Cr50: Clear the TX FIFO when the SPS CS deasserts

When the SPI slave chip select is deasserted, it means that the
SPI master doesn't want to hear any more from the EC. We need to
clear any bytes left in the TX FIFO, so that the next SPI
transaction doesn't send those leftover bytes out.

Since the EC's SPI protocol for host commands uses software flow
control, those leftover bytes could screw up the messages. I
expanded a comment explaining how that works.

BUG=chrome-os-partner:40969
BRANCH=none
TEST=make buildall

And, with the EC connected to the build host via an FTDI USB-to-SPI
adapater, I used the extra/sps_errs/ test program to see the
original problem and that this CL fixed it:

  cd extra/sps_errs
  make
  ./prog -v
  ./prog -v -c 22
  ./prog -v

This sends a complete EC_CMD_HELLO message, then a truncated
message, then sends the whole message. Before this change to
sps.c, the third message response begins with the leftover bytes
from the aborted second message.

Bad third message:

  Transfer(12) =>
     03 64 01 00 00 00 04 00 a5 a5 a5 a5
  Transfer(12) <=
     a9 a8 a7 a6 f8 f8 f8 f8 f8 f8 f9 f9
     ^^ ^^ ^^ ^^

Good third message:

  Transfer(12) =>
     03 64 01 00 00 00 04 00 a5 a5 a5 a5
  Transfer(12) <=
     f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f9 f9
     ^^ ^^ ^^ ^^

Change-Id: Id6e431f91be0204921edee2f774b6c487966ddff
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/284746
Reviewed-by: Vadim Bendebury <vbendeb@google.com>

1 files changed, 12 insertions, 8 deletions

diff --git a/chip/g/sps.c b/chip/g/sps.c
index b4152da076..c68e3b0c60 100644
--- a/chip/g/sps.c
+++ b/chip/g/sps.c
@@ -167,17 +167,18 @@ static void sps_enable(void)
 	GWRITE_FIELD(SPS, FIFO_CTRL, TXFIFO_EN, 1);
 
 	/*
-	 * Wait until we have a few bytes in the FIFO before waking up. Note
-	 * that if the master wants to read bytes from us, it may have to clock
-	 * in at least RXFIFO_THRESHOLD + 1 bytes before we notice that it's
-	 * asking.
+	 * Wait until we have a few bytes in the FIFO before waking up. There's
+	 * a tradeoff here: If the master wants to talk to us it will have to
+	 * clock in at least RXFIFO_THRESHOLD + 1 bytes before we notice. On
+	 * the other hand, if we set this too low we waste a lot of time
+	 * handling interrupts before we have enough bytes to know what the
+	 * master is saying.
 	 */
-	GREG32(SPS, RXFIFO_THRESHOLD) = 8;
+	GREG32(SPS, RXFIFO_THRESHOLD) = 7;
 	GWRITE_FIELD(SPS, ICTRL, RXFIFO_LVL, 1);
 
 	/* Also wake up when the master has finished talking to us, so we can
-	 * drain any remaining bytes in the RX FIFO. Too late for TX, of
-	 * course. */
+	 * drain any remaining bytes in the RX FIFO. */
 	GWRITE_FIELD(SPS, ISTATE_CLR, CS_DEASSERT, 1);
 	GWRITE_FIELD(SPS, ICTRL, CS_DEASSERT, 1);
 }
@@ -245,8 +246,11 @@ DECLARE_IRQ(GC_IRQNUM_SPS0_RXFIFO_LVL_INTR, _sps0_interrupt, 1);
 
 void _sps0_cs_deassert_interrupt(void)
 {
-	/* Make sure the receive FIFO is drained. */
+	/* Notify the registered handler (and drain the RX FIFO) */
 	sps_rx_interrupt(0);
+	/* Clear the TX FIFO manually, so the next transaction doesn't
+	 * start by clocking out any bytes left over from this one. */
+	GREG32(SPS, TXFIFO_WPTR) = GREG32(SPS, TXFIFO_RPTR);
 	/* Clear the interrupt bit */
 	GWRITE_FIELD(SPS, ISTATE_CLR, CS_DEASSERT, 1);
 }