-
Notifications
You must be signed in to change notification settings - Fork 12
ot_spi_device
: always upload payload for SW flash commands
#224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
References for review:
The documentation of the upload
field loosely suggests that uploading only really cares about the address, but it doesn't make that very clear:
If upload field in the command info entry is set, the cmdparse activates the upload submodule when the opcode is received. addr_en, addr_4B_affected, and addr_4b_forced (TBD) affect the upload functionality. The three address related configs defines the command address field size.
RTL:
payload_en
is loaded into thecmd_info
from the regs: https://github.com/lowRISC/opentitan/blob/e4ce7cf160f508d5787ed2e272872326c9f9159c/hw/ip/spi_device/rtl/spi_device.sv#L641- The
cmd_info
is passed through SPI command parsing (which only observes the opcode): https://github.com/lowRISC/opentitan/blob/e4ce7cf160f508d5787ed2e272872326c9f9159c/hw/ip/spi_device/rtl/spi_cmdparse.sv#L253 spi_passthrough
(unimplemented in QEMU) does use thepayload_en
: https://github.com/lowRISC/opentitan/blob/e4ce7cf160f508d5787ed2e272872326c9f9159c/hw/ip/spi_device/rtl/spi_passthrough.sv#L810 etc.- SPI read commands use the
payload_en
to determine the number of bits read from the FIFO at a time: https://github.com/lowRISC/opentitan/blob/e4ce7cf160f508d5787ed2e272872326c9f9159c/hw/ip/spi_device/rtl/spi_readcmd.sv#L472-L486 - SPI upload basically uses none of the
cmd_info
fields, including ignoringpayload_en
. It only uses theaddr_mode
andbusy
fields: https://github.com/lowRISC/opentitan/blob/e4ce7cf160f508d5787ed2e272872326c9f9159c/hw/ip/spi_device/rtl/spid_upload.sv#L212-L225
e2fd360
to
a44d544
Compare
Changed to check |
The SPI device design only considers the `PAYLOAD_EN` bit of a slot for the passthrough mode, not for uploaded flash commands which always upload their payloads. Signed-off-by: James Wainwright <james.wainwright@lowrisc.org>
a44d544
to
03bfc14
Compare
It seems that clang-format complains about the line length on the comments. qemu/.gitlab-ci.d/opentitan/build.yml Line 66 in 9bc668a
|
Ah my bad, I didn't re-run |
I keep doing this mistake, I think we need to address it @ CI level. |
The SPI device design only considers the
PAYLOAD_EN
bit of a slot for the passthrough mode, not for uploaded flash commands which always upload their payloads.