wip grammarly

blog/content/post/2023-06-07-twpm-spi-fix.md

@@ -29,35 +29,33 @@ short-distance communication between various devices. SPI is one of the
 interfaces used by TPM chips for communication with PC motherboard. SPI uses 4
 lines for communication: MOSI, MISO, SCK, SS, which are described down below.
 
-For the device to work as a TPM module, it must implement TPM protocol. TPM
-protocol works by transmitting variable-length frames over SPI, 4-byte TPM
-header contains length of data payload as well transfer direction (read or
-write). Another important feature of TPM protocol are the wait states - SPI
-slave can hold transmittion by pulling MISO line down.
-
-TODO: TPM header image
-
-TPMs typically operate at frequency up to 24 MHz, this is also the maximum
-frequency required by the PTP spec (TBD: link, describe what is PPT spec). Such
-a high frequency as well as non-standard SPI features - wait-states and
-variable-length frames pose a significant challenge, neither the platform we are
-using is the easy one.
+The device must implement the TPM protocol to work as a TPM module. TPM protocol
+works by transmitting variable-length frames over SPI. The 4-byte TPM header
+contains fields describing the length of the data payload, the address of the
+target TPM register, and the transfer direction (read or write). TPM  protocol
+has its own means of handling flow control (as there isn't a standard flow
+control mechanism on SPI) and for doing bus aborts.
+
+TPMs typically operate at frequencies from 10 MHz up to 24 MHz. TPM must be able
+to operate at 24 MHz to comply with TCG PTP specification and be compatible with
+most of the PCs on the market. Getting SPI right on such high frequencies is a
+significant challenge, especially when operating as a slave. TPM-specific
+features complicate things further.
 
 ## Limitations of STM32L476
 
-STM32L476 has SPI capable of frequencies up to a (theoretical) limit of 40 MHz
-which is a half of maximum clock that can be provided to Cortex-M and AHB/APB
-buses. There are other limiting factors such as maximum GPIO speed, DMA transfer
-speed and performance of the firmware itself.
+STM32L476 has SPI capable of frequencies up to a (theoretical) limit of 40 MHz,
+which is half of the maximum clock that can be provided to Cortex-M and AHB/APB
+buses. Other limiting factors include maximum GPIO speed, DMA transfer speed,
+and performance of the firmware itself.
 
 ## Creating SPI slave on Zephyr
 
-Zephyr provides support for SPI master, and experimental support for SPI slave.
-From earlier tests which I will not describe here, I can tell that SPI slave
-works at frequencies up to 100 KHz and becomes unstable at higher frequencies.
+Zephyr is our platform of choice primarily due to its portability (we will
+target non-STM32 platforms too). I will briefly describe the outcome of my early
+tests done on Zephyr and why it was terrible.
 
-For further tests, I will a create simple application that sends some random
-data:
+The application just transmits a static sequence of bytes:
 
 ```c
 const struct device *spi_dev = NULL;
@@ -98,7 +96,7 @@ void main(void) {
 }
 ```
 
-We enable support SPI slave and DMA.
+It is necessary to enable some configs:
 
 ```shell
 CONFIG_GPIO=y
@@ -142,16 +140,16 @@ spi_write failed: -11
 spi_write failed: -11
 ```
 
-The problem is a timeout - the driver waits 1 second for transfer to complete
-and then fails. While this is a desirable behaviour for SPI master, it is not
-desirable for slave. Master itself decides when data is transmitted, so when
-transfer doesn't complete in reasonable time, for sure there is something wrong.
-Slave must wait for master to start data transmission, which could take any
-time. Right now, we are stuck in endless loop, where transfer is queued,
-aborted, then queued again.
+This is the first problem with Zephyr's SPI driver - each transfer has a
+one-second timeout. While this may be desirable behavior for SPI master (it
+could be used for error recovery, for example, to power cycle the slave if it
+doesn't respond), it breaks SPI slave. The slave must be ready to give a
+response when the transfer commences - appropriate data must already be loaded
+in FIFO. Here we get stuck in an endless loop, queuing the transfer, aborting
+it, and queuing it again.
 
-To fix the problem, we can modify `wait_dma_rx_tx_done` in `spi_ll_tm32.c`, the
-origin function looks like this:
+The problem can be worked around by patching the `wait_dma_rx_tx_done` function
+in `spi_ll_stm32.c`. The original function looks like this:
 
 ```c
 static int wait_dma_rx_tx_done(const struct device *dev)
@@ -167,8 +165,7 @@ static int wait_dma_rx_tx_done(const struct device *dev)
 ...
 ```
 
-The problem lies in the call to `k_sem_take`, just replace `K_MSEC(1000)` with
-`K_FOREVER`.
+Just replace `K_MSEC(1000)` with `K_FOREVER`.
 
 Now running `spitest` at 100 KHz yields the following result: