shi: Enable SHI interrupt from CS interrupt

Enable the SHI interrupt only after we have received and begun
processing our host command. Disable the SHI interrupt once our
transaction is complete (with either success or error status). This will
prevent the SHI interrupt from being asserted at the same time as the CS
interrupt, which can lead to the SHI interrupt being serviced first.
Also, it avoids needless, non-useful SHI interrupts during error
transactions.

BUG=chrome-os-partner:55710,chrome-os-partner:55795,chrome-os-partner:56254
BRANCH=None
TEST=Manual on gru. Stress test flashrom w/ unpowered Donette attached
(for host command spam), verify no errors encountered after 100 minutes.
Also verify host command interface functions properly after sysjump.

Change-Id: I41e3deb382897cd4286e6ac96f4f3066bf7a94a7
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/371510
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>
Reviewed-by: Mulin Chao <mlchao@nuvoton.com>

1 files changed, 43 insertions, 20 deletions

diff --git a/chip/npcx/shi.c b/chip/npcx/shi.c
index 6cf030cf7c..154bc3a5d7 100644
--- a/chip/npcx/shi.c
+++ b/chip/npcx/shi.c
@@ -195,13 +195,15 @@ static uint8_t shi_read_buf_pointer(void);
  */
 static void shi_send_response_packet(struct host_packet *pkt)
 {
+	/*
+	 * Disable interrupts. This routine is not called from interrupt
+	 * context and buffer underrun will likely occur if it is
+	 * preempted after writing its initial reply byte. Also, we must be
+	 * sure our state doesn't unexpectedly change, in case we're expected
+	 * to take RESP_NOT_RDY actions.
+	 */
+	interrupt_disable();
 	if (state == SHI_STATE_PROCESSING) {
-		/*
-		* Disable interrupts. This routine is not called from interrupt
-		* context and buffer underrun will likely occur if it is
-		* preempted after writing its initial reply byte.
-		*/
-		interrupt_disable();
 
 		/* Append our past-end byte, which we reserved space for. */
 		((uint8_t *) pkt->response)[pkt->response_size + 0] =
@@ -226,8 +228,6 @@ static void shi_send_response_packet(struct host_packet *pkt)
 #ifdef NPCX_SHI_BYPASS_OVER_256B
 		}
 #endif
-
-		interrupt_enable();
 	}
 	/*
 	 * If we're not processing, then the AP has already terminated the
@@ -239,6 +239,8 @@ static void shi_send_response_packet(struct host_packet *pkt)
 		DEBUG_CPRINTF("END\n");
 	} else
 		DEBUG_CPRINTS("Unexpected state %d in response handler", state);
+
+	interrupt_enable();
 }
 
 void shi_handle_host_package(void)
@@ -293,9 +295,6 @@ void shi_handle_host_package(void)
 /* Parse header for version of spi-protocol */
 static void shi_parse_header(void)
 {
-	/* Disable SHI interrupt until we're sure the size of request package.*/
-	task_disable_irq(NPCX_IRQ_SHI);
-
 	/* We're now inside a transaction */
 	state = SHI_STATE_RECEIVING;
 	DEBUG_CPRINTF("RV-");
@@ -330,9 +329,6 @@ static void shi_parse_header(void)
 		/* Computing total bytes need to receive */
 		shi_params.sz_request = pkt_size;
 
-		/* Enable SHI interrupt & handle request package */
-		task_enable_irq(NPCX_IRQ_SHI);
-
 		shi_handle_host_package();
 	} else {
 		/* Invalid version number */
@@ -501,12 +497,6 @@ static void shi_bad_received_data(void)
 		CPRINTF("%02x ", in_msg[i]);
 	CPRINTF("]\n");
 
-	/*
-	 * Enable SHI interrupt again since we disable it
-	 * at the begin of SHI_STATE_RECEIVING state
-	 */
-	task_enable_irq(NPCX_IRQ_SHI);
-
 	/* Reset shi's state machine for error recovery */
 	shi_reset_prepare();
 
@@ -559,6 +549,14 @@ void shi_int_handler(void)
 			shi_fill_out_status(EC_SPI_NOT_READY);
 
 			state = SHI_STATE_CNL_RESP_NOT_RDY;
+
+			/*
+			 * Disable SHI interrupt, it will remain disabled
+			 * until shi_send_response_packet() is called and
+			 * CS is asserted for a new transaction.
+			 */
+			task_disable_irq(NPCX_IRQ_SHI);
+
 			DEBUG_CPRINTF("CNL-");
 			return;
 		/* Next transaction but we're not ready */
@@ -684,6 +682,12 @@ void shi_cs_event(enum gpio_signal signal)
 
 	DEBUG_CPRINTF("CSL-");
 
+	/*
+	 * Enable SHI interrupt - we will either succeed to parse our host
+	 * command or reset on failure from here.
+	 */
+	task_enable_irq(NPCX_IRQ_SHI);
+
 	/* Read first three bytes to parse which protocol is receiving */
 	shi_parse_header();
 }
@@ -699,6 +703,9 @@ static void shi_reset_prepare(void)
 {
 	uint16_t i;
 
+	/* We no longer care about SHI interrupts, so disable them. */
+	task_disable_irq(NPCX_IRQ_SHI);
+
 	/* Initialize parameters of next transaction */
 	shi_params.rx_msg = in_msg;
 	shi_params.tx_msg = out_msg;
@@ -735,6 +742,7 @@ static void shi_reset_prepare(void)
 static void shi_enable(void)
 {
 	int gpio_flags;
+	int shi_cs_level;
 
 	shi_reset_prepare();
 
@@ -754,8 +762,23 @@ static void shi_enable(void)
 	 */
 	SET_BIT(NPCX_DEVALT(ALT_GROUP_C), NPCX_DEVALTC_SHI_SL);
 
+	/*
+	 * If CS is already asserted prior to enabling our GPIO interrupt then
+	 * we have missed the falling edge and we need to handle the
+	 * deassertion interrupt - to do so, we'll enable the SHI interrupt
+	 * later.
+	 */
+	shi_cs_level = gpio_get_level(GPIO_SHI_CS_L);
+
 	/* Enable SHI_CS_L interrupt */
 	gpio_enable_interrupt(GPIO_SHI_CS_L);
+
+	/*
+	 * Check the CS level again, in case it was asserted after our previous
+	 * check.
+	 */
+	if (!shi_cs_level || !gpio_get_level(GPIO_SHI_CS_L))
+		task_enable_irq(NPCX_IRQ_SHI);
 }
 DECLARE_HOOK(HOOK_CHIPSET_RESUME, shi_enable, HOOK_PRIO_DEFAULT);