-
Notifications
You must be signed in to change notification settings - Fork 7.8k
drivers: flash: flash_mcux_flexspi: add support for W25Q512JV #80261
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
drivers: flash: flash_mcux_flexspi: add support for W25Q512JV #80261
Conversation
@@ -886,6 +886,37 @@ static int flash_flexspi_nor_check_jedec(struct flash_flexspi_nor_data *data, | |||
|
|||
/* Switch on manufacturer and vendor ID */ | |||
switch (vendor_id & 0xFFFF) { | |||
case 0x40ef: |
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.
not going to request changes to this PR about this right now since it's a bug fix and what im gonna ask about is more than a minor tweak... , but this doesn't seem scalable here, we dont really want all this code for many different flash chips in the image, do we ? and do we expect the flexspi hardware connection to change dynamically (runtime) ? ie, why isnt these luts somehow determined statically
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.
No, it isn't scalable at all. The SFDP driver design was initially intended to save us from statically defined LUTs, but most flash chips that support 4 byte addressing do it via a "vendor defined command set"- so for example, this W25Q512JV worked fine with 3 byte addressing, but that doesn't let you access all 64 MB of the flash chip, hence the need for this switch case.
Long term the solution is to move the FlexSPI under the MSPI API, so we can just write MSPI drivers for all these flash chips.
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.
SFDP has codes for standard instructions but JESD216 but mentions that vendors often chosen to extend set of instructions; there is table of instructions that support 4 byte addressin, but in this case this is standard 0xec Fast Read 1s-4s-4s.
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.
I'm not sure I follow. Does JESD216 define a DWORD that can be read to determine if the flash uses standard 4 byte addressing instructions (IE 0xEC)? I could not find one in the standard.
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.
I have jesd216.F from 2022, and there is chapter: 6.7 "JEDEC 4-byte Address Instruction Parameter Header and Table", p67.
You can also search for "Support for (1S-1S-4S) Page Program Command, Instruction=34h"
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.
But I think that since you have already limited the setup to specific vendor/memory, there is no need to query and you can just enforce directly what you want.
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.
I have jesd216.F from 2022, and there is chapter: 6.7 "JEDEC 4-byte Address Instruction Parameter Header and Table", p67.
You can also search for "Support for (1S-1S-4S) Page Program Command, Instruction=34h"
Yup, you are right- this is the proper way to do this. I agree that for now this approach is ok, but this is how we should handle SFDP probes for QSPI flash in the future I think.
Add support for the W25Q512JV with the FLEXSPI, using a custom LUT table. Signed-off-by: Daniel DeGrasse <[email protected]>
WS25Q512JV can only run at 104MHz at 3.3V, unless the read parameter bits are changed. Since we don't reprogram these currently, reduce max frequency to safe value Signed-off-by: Daniel DeGrasse <[email protected]>
30dd65f
to
68ba6f1
Compare
@de-nordic could you take a look at this PR? |
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.
@danieldegrasse Looks OK to me, but I can only verify characteristics of the memory, to my understanding, datasheet and JESD216.
The ownership here is on NXP platform maintainers, I am only in control of API.
(btw, we need to figure out to stop doing SoC vendor specific SPI memory device drivers)
Strongly agree- I want to move this to use the MSPI API in the (hopefully) near term. |
Add support for the W25Q512JV with the FLEXSPI, using a custom LUT table.