lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.5k stars 745 forks source link

[spi_device] Synchronize control and status bits at regular intervals #20754

Closed a-will closed 7 months ago

a-will commented 8 months ago

Description

spi_device still does not handle status bits like a host would expect of a SPI flash device. The commit phase of status bit synchronization currently only occurs on the rising edge of CSB, but the ReadStatus commands need to allow for changes to the WIP bit without the deassertion of CSB.

This design confuses the visibility of status bits with the requirements for the passthrough gate. It is only the passthrough gate that must wait for the deassertion of CSB to switch on/off.

Instead, for the purposes of representing the status registers, commit staged bits at every 8th SCK edge. Keep the current staging mechanism for synchronization, and also keep the passthrough gate commit on CSB de-assertion.

Semi-related, in that it could use a similar commit point: https://github.com/lowRISC/opentitan/issues/15543

a-will commented 8 months ago

If we want to keep the commit of the WEN bit to happen during the WREN / WRDI commands, we'll also need to squash the hardware update to bypass sck_status_staged (and override the staged bit going into the sck_status_committed flops -- hardware updates always take priority).

This likely would mean making sck_status_staged be a combinatorial function, instead of a flopped signal that feeds the sck_status_committed flops.

a-will commented 8 months ago

In addition, we may want to expose to software when software-initiated writes are dropped. The short async FIFO that brings software updates into the SCK domain is not guaranteed to make forward progress, as it relies on the host to issue SPI transactions.

Notice the ignored ready condition here: https://github.com/lowRISC/opentitan/blob/748235cbb6af38d2e6cf04726704682a6aab363c/hw/ip/spi_device/rtl/spid_status.sv#L187-L195

a-will commented 7 months ago

In addition to the above, the Read Status Register commands currently rotate through all three status registers for even the original RDSR command. This is atypical behavior that precludes the conventional polling enabled by a variety of SPI flash devices.

I think we should make SPI Device IP only read from the targeted 8-bit register for all variants of Read Status Register {1,2,3}.