lowRISC / opentitan

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

[spi_host] Allow SPI host tests to work with multiple flash chips #23625

Open jwnrt opened 3 weeks ago

jwnrt commented 3 weeks ago

Description

The spi_host_winbond_flash_test checks the manufacturer and device IDs of a Winbond flash chip connected to the SPI host.

The device ID is hardcoded but the test should work with many kinds of flashes.

With the new CW340 FPGAs added to CI, we will need to add more flash chips. The kind we have are still manufactured by Winbond but have different IDs. We would like to remove the explicit device ID check from this test to allow it to work with our second Winbond flash part.

Opening this issue since it needs to wait until after the current quiet period.

a-will commented 3 weeks ago

One way to do this would be to rely on the SFDP to configure the test (but report the JEDEC ID in the log, so you can identify the chip for failures).

General support for all the SFDP fields can be a semi-significant undertaking, though.

jwnrt commented 3 weeks ago

One way to do this would be to rely on the SFDP to configure the test (but report the JEDEC ID in the log, so you can identify the chip for failures).

Do you mean that the device ID is also in the SFDP and we can compare the JEDEC ID to that instead of our hardcoded value?

a-will commented 3 weeks ago

One way to do this would be to rely on the SFDP to configure the test (but report the JEDEC ID in the log, so you can identify the chip for failures).

Do you mean that the device ID is also in the SFDP and we can compare the JEDEC ID to that instead of our hardcoded value?

No, I mean that the SFDP tells us how to drive the SPI flash chip (e.g. what the command opcodes are to do various things, whether a chip supports quad mode, how to enable, etc.).

A table of known JEDEC IDs isn't particularly scalable, so it might not make sense to check it. Instead, we'd know it should fail if you get all zeroes or all ones. We do get the flash memory density from the SFDP, though, so maybe we could check one byte of the JEDEC ID.

jwnrt commented 3 weeks ago

No, I mean that the SFDP tells us how to drive the SPI flash chip (e.g. what the command opcodes are to do various things, whether a chip supports quad mode, how to enable, etc.).

@AlexJones0 and I checked the SFDP tables and it looks like you can get the op-code for quad reads but not writes from the standard tables. Our sw/device/tests{/pmod}/spi_host_*_flash_tests hard code these IDs, but reading them from the SFDP tables requires knowledge about the manufacturer-specific tables.

We could embed the tables for a few manufacturers and read the op-codes, but this doesn't help us much.

I agree we should replace the device_id check with a check for all-0 or all-f but we still need to check the manufacturer ID I think.

We do get the flash memory density from the SFDP, though, so maybe we could check one byte of the JEDEC ID.

I'm not sure what you mean by this, sorry.

a-will commented 3 weeks ago

We do get the flash memory density from the SFDP, though, so maybe we could check one byte of the JEDEC ID.

I'm not sure what you mean by this, sorry.

The command to read the JEDEC ID is a tuple of 3 values: (manufacturer ID, memory type, memory capacity). Potentially, that last byte could be checked with the info from the SFDP.

jwnrt commented 3 weeks ago

Thanks, that makes sense.

(Closing was a mis-click)