Skip to content
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

[sw/silicon_creator] Add SFDP table definition and READ_SFDP command #11742

Merged
merged 1 commit into from
Mar 29, 2022

Conversation

alphan
Copy link
Contributor

@alphan alphan commented Mar 28, 2022

This PR adds the initial definition of the SFDP table that OpenTitan will use and the READ_SFDP command. The values are from the doc (SFDP and the BFPT headers) and the spreadsheet (basic flash parameters table) that I've been working on.

Following issues track some open items:

Signed-off-by: Alphan Ulusoy [email protected]

@alphan alphan force-pushed the spi-device-sfdp branch 2 times, most recently from e58cc40 to 9f508fa Compare March 28, 2022 20:24
@alphan alphan marked this pull request as ready for review March 28, 2022 21:02
@alphan alphan requested review from cfrantz and a team as code owners March 28, 2022 21:02
@alphan alphan requested review from timothytrippel, a-will and eunchan and removed request for a team March 28, 2022 21:02
@alphan
Copy link
Contributor Author

alphan commented Mar 28, 2022

@eunchan, can you take a quick look at the changes in spi_device_init() and cmd_info_set() to confirm that I'm setting the cmd_info registers correctly? (None of the the commands have payload so far so I don't touch corresponding bits yet.) Thanks!

@eunchan
Copy link
Contributor

eunchan commented Mar 28, 2022

Yep . Looks good to me Thanks!

Copy link
Contributor

@timothytrippel timothytrippel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I will defer to @a-will's judgment given his experience with the spi_device DIFs.

sw/device/silicon_creator/lib/drivers/spi_device.c Outdated Show resolved Hide resolved
.address = false,
.dummy_cycles = 0,
});
// Configure READ_SFDP command (CMD_INFO_4).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment: I wonder if we ought to have regtool generate constants / macros that associate cmd_info slots with the specific hardware-handled commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I think the current design let's us customize the actual opcode but you have to write it to the correct cmd_info register. @eunchan I'll add a link to this comment in #11740.

Comment on lines +327 to +333
/**
* BFPT 17th Word
* --------------
* [31: 0]: Fast read (1S-8S-8S) (1S-1S-8S) (not supported, 0x0)
*/
#define BFPT_WORD_17(X) \
X(31, 0, kBfptNotSupported)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Given the non-support of anything past word 16, one wonders if we should find a JEDEC parameter table version that doesn't include words [17..23] and use that version and save a bit of parameter space.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I'll try to find something from a credible source when I get the chance but jedec.org is not very helpful when it comes to previous versions of the specs.

@alphan alphan merged commit 2c1f602 into lowRISC:master Mar 29, 2022
@alphan alphan deleted the spi-device-sfdp branch March 29, 2022 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants