drivers: flash: flexspi: Add 4-byte addressing support for MT25 family #82532

Finomnis · 2024-12-04T10:11:06Z

Currently, MT25 flashes are running in 3-byte mode.

This is not compatible with the chip we use in our project (MT25QU01GBBB), as only 128 Mbit of its 1 Gbit can be addressed.

The new LUT configuration is identical to the one used in other projects of ours, based on Azure RTOS. Of course, I still always appreciate more people testing it on their devices, in case someone else has a MT25 chip somewhere.

Link to datasheet for LUT reviews: https://mm.digikey.com/Volume0/opasdata/d220001/medias/docus/2293/MT25QU01GBBB_DS.pdf

danieldegrasse

Two minor nits, but the LUT table looks right.

danieldegrasse · 2024-12-04T13:50:17Z

drivers/flash/flash_mcux_flexspi_nor.c

+				kFLEXSPI_Command_SDR, kFLEXSPI_1PAD, SPI_NOR_CMD_SE_4B,
+				kFLEXSPI_Command_RADDR_SDR, kFLEXSPI_1PAD, 32);
+		flexspi_lut[ERASE_BLOCK][0] = FLEXSPI_LUT_SEQ(
+				kFLEXSPI_Command_SDR, kFLEXSPI_1PAD, 0xDC,


Tiny nit- can use SPI_NOR_CMD_BE_4B here.

danieldegrasse · 2024-12-04T13:53:14Z

drivers/flash/flash_mcux_flexspi_nor.c

+				kFLEXSPI_Command_SDR, kFLEXSPI_1PAD, SPI_NOR_CMD_RDSR,
+				kFLEXSPI_Command_READ_SDR, kFLEXSPI_1PAD, 0x01);
+		/* Device has no QE bit, 1-4-4 and 1-1-4 is always enabled */
+		return flash_flexspi_nor_quad_enable(data, flexspi_lut, JESD216_DW15_QER_NONE);


This is effectively a NOP, right? We can probably drop this call.

From my understanding, yes.

Let's just drop it then, and leave a comment there so that developers aren't confused in the future as to why this is skipped

danieldegrasse · 2024-12-04T13:55:37Z

drivers/flash/flash_mcux_flexspi_nor.c

@@ -1093,6 +1093,36 @@ static int flash_flexspi_nor_check_jedec(struct flash_flexspi_nor_data *data,
 				kFLEXSPI_Command_READ_SDR, kFLEXSPI_1PAD, 0x01);
 		/* Device uses bit 6 of status reg 1 for QE */
 		return flash_flexspi_nor_quad_enable(data, flexspi_lut, JESD216_DW15_QER_VAL_S1B6);
+	case 0xbb20:


Just FYI- I have this PR open: #82448. If this merges first I can update the value here- I would expect it to be 0x18bb20, but could you verify that on your end?

Ours is 0x21bb20.

Ah, I misread your initial comment- I thought you had an 128Mbit flash, not 1Gbit (I guess in hindsight, if you had a 128Mbit flash this PR wouldn't be needed...). Going off this table, that value makes sense:

Makes sense. So I guess we could add 0x19 to 0x22?

Yes, we should be able to add them all to the switch statement pretty safely :)

Note, NNba20 and NNbb20 are the same devices 0xba and 0xbb are voltages for these specific devices.
NN == 0x18 means that you have 128Mbit device, which works with 24 bit addressing everything above 0x18 will require 4 byte.
Although, I do not see any comments that the 0x18 type devices would not work with 4 byte addressing.

@de-nordic Meaning, we should add a case 0xba20: there as well?

You can, because both should be covered.

Finomnis · 2024-12-04T14:29:23Z

@danieldegrasse We are getting weird erase/write issues on our device, which are fixed if we add a k_sleep(20) in the flash_flexspi_nor_wait_bus_busy. So I reverted this change to 'Draft' and we'll investigate.

danieldegrasse

Thanks for fixing all the other usages of 0xDC :)

danieldegrasse · 2024-12-04T14:32:46Z

@danieldegrasse We are getting weird erase/write issues on our device, which are fixed if we add a k_sleep(20) in the flash_flexspi_nor_wait_bus_busy. So I reverted this change to 'Draft' and we'll investigate.

Very odd- just a note, I would be very careful calling kernel APIs within the driver. If you are executing in place from this flash chip (I assume so, given the board you are using), then attempting to run code stored in flash while the flash device is programming can cause issues- the sequence that happens is something like so:

flash starts programming sequence, and will now ignore the "read" command
core attempts to fetch code from flash to execute
FlexSPI sends read command to flash, flash ignores command. FlexSPI will return 0x0 for data read
Core executes these instructions as mov r0, r0- control flow is broken, but this will only manifest later on as a crash for unclear reasons

Finomnis · 2024-12-04T14:36:02Z

Very odd- just a note, I would be very careful calling kernel APIs within the driver. If you are executing in place from this flash chip (I assume so, given the board you are using), then attempting to run code stored in flash while the flash device is programming can cause issues- the sequence that happens is something like so:

You are absolutely right, of course. But we only did that for testing purposes, and it made our functional write/read tests succeed. Very odd.

Finomnis · 2024-12-04T15:46:26Z

@danieldegrasse It works with a 10MHz clock, but fails with everything larger. Might be a cs setup/hold time issue on our side or similar. Still wanna make sure it's not an issue in the LUT before merging this.

It feels like the 'Write Finished' register flag gets cleared too early, or at least it's perceived by the CPU that way. I already checked the LUT for it though and it seems fine. I'm not sure what the issue is.

Finomnis · 2024-12-04T15:59:57Z

@danieldegrasse Btw, what's the reason that flash_mcux_flexspi_nor.c and spi_nor.c are only loosely coupled? A lot of JEDEC stuff is kind of implemented twice, like the whole 4-byte handling seems to be configured in spi_nor.c as well. And CONFIG_FLASH_MCUX_FLEXSPI_NOR does not even activate or interact with CONFIG_SPI_NOR or CONFIG_SPI_NOR_SFDP.

danieldegrasse · 2024-12-04T16:05:37Z

@danieldegrasse It works with 1MHz clock, but fails with everything larger. Might be a cs setup/hold time issue on our side or similar. Still wanna make sure it's not an issue in the LUT before merging this.

Very strange. I've been looking at the datasheet as well, the LUT really seems correct to me. Is it possible there is some other bit that needs to be checked? I'm looking at the flag status register and wonder how bit 7 behaves

Finomnis · 2024-12-04T16:08:56Z

I'm looking at the flag status register and wonder how bit 7 behaves

Page 19, note 2: Status register bit 0 is the inverse of flag status register bit 7.

Finomnis · 2024-12-05T09:18:24Z

@danieldegrasse We currently assume it's a bug on our side, so I put this PR back to non-draft. Ready to be reviewed and merged.

danieldegrasse · 2024-12-05T20:22:38Z

@danieldegrasse Btw, what's the reason that flash_mcux_flexspi_nor.c and spi_nor.c are only loosely coupled? A lot of JEDEC stuff is kind of implemented twice, like the whole 4-byte handling seems to be configured in spi_nor.c as well. And CONFIG_FLASH_MCUX_FLEXSPI_NOR does not even activate or interact with CONFIG_SPI_NOR or CONFIG_SPI_NOR_SFDP.

So generally the reason is that spi_nor.c only handles serial SPI communication, (IE 1-1-1), not parallel busses like 1-4-4 or similar. Historically, there has not been an API for interfacing with parallel SPI controllers- instead, the FlexSPI had a vendor specific API designed for it, as NXP needed to write drivers for multiple flash chips.

The flash_mcux_flexspi_nor.c driver historically only supported flash chips that used standard 3 byte addressing in 1-4-4 mode (and truthfully, was designed for the IS25WP064 flash chip on some NXP EVKs, as far as I can tell). I reworked the driver to support SFDP in an attempt to broaden the range of flash chips the driver could support.

Currently, this driver has a few issues (as you've pointed out):

this switch statement keeps getting larger as we handle more and more flash chips with different lookup table settings
there is a lot of duplication of the SFDP algorithms

Now there is an API in upstream for interfacing with parallel SPI controllers, the MSPI API. The path forwards is likely the following:

implement support for the MSPI API using the FlexSPI controller (I have started work on this here: [WIP] Add NXP FlexSPI driver using the MSPI API #82580)
implement support for the MSPI API using the SPI API (a shim driver like this will allow us to use SPI devices with the MSPI API in 1-1-1 mode)
add a driver using SFDP with the MSPI API for flash chips

Long term, we can also implement drivers for specific flash chips using the MSPI API as well. This will allow us to move away from the flash_mcux_flexspi_xxx drivers that are specific to the FlexSPI only

danieldegrasse · 2024-12-10T16:22:17Z

Since #82448 merged, this PR now needs to encode 3 bytes of the JEDEC ID rather than 2

Finomnis · 2024-12-10T22:04:39Z

@de-nordic Would you mind verifying that I don't have a typo in my JEDEC IDs or Product Package Codes?

Currently, MT25 flashes were running in 3-byte mode. This is not compatible with the chip we use in our project (MT25QU01GBBB), as only 128 Mbit of its 1 Gbit can be addressed. Signed-off-by: Martin Stumpf <[email protected]>

Finomnis · 2024-12-10T22:13:22Z

@danieldegrasse Sorry, just fixed the wording of a comment, will stop editing now.

zephyrbot added area: Flash platform: NXP Drivers NXP Semiconductors, drivers labels Dec 4, 2024

zephyrbot requested review from danieldegrasse, dbaluta, de-nordic, decsny, dleach02, manuargue, MarkWangChinese, mmahadevan108 and rghaddab December 4, 2024 10:11

zephyrbot assigned de-nordic Dec 4, 2024

mstumpf-vected force-pushed the flexspi_flash_mt25 branch 2 times, most recently from 7a5f297 to d2416e2 Compare December 4, 2024 10:46

zephyrbot added the Release Notes To be mentioned in the release notes label Dec 4, 2024

zephyrbot requested review from fabiobaltieri and kartben December 4, 2024 10:47

mstumpf-vected force-pushed the flexspi_flash_mt25 branch from d2416e2 to 344500d Compare December 4, 2024 11:10

Finomnis marked this pull request as draft December 4, 2024 13:27

danieldegrasse previously approved these changes Dec 4, 2024

View reviewed changes

mstumpf-vected dismissed danieldegrasse’s stale review via 779affa December 4, 2024 14:23

mstumpf-vected force-pushed the flexspi_flash_mt25 branch 2 times, most recently from 779affa to f3e0e56 Compare December 4, 2024 14:27

danieldegrasse reviewed Dec 4, 2024

View reviewed changes

Finomnis marked this pull request as ready for review December 4, 2024 18:33

Finomnis requested a review from danieldegrasse December 5, 2024 10:58

danieldegrasse previously approved these changes Dec 5, 2024

View reviewed changes

de-nordic previously approved these changes Dec 10, 2024

View reviewed changes

de-nordic added this to the v4.1.0 milestone Dec 10, 2024

mstumpf-vected dismissed stale reviews from de-nordic and danieldegrasse via f4e131f December 10, 2024 12:16

mstumpf-vected force-pushed the flexspi_flash_mt25 branch from f3e0e56 to f4e131f Compare December 10, 2024 12:16

Finomnis force-pushed the flexspi_flash_mt25 branch 2 times, most recently from f4e131f to b7cd010 Compare December 10, 2024 22:03

Finomnis requested review from de-nordic and danieldegrasse December 10, 2024 22:03

danieldegrasse previously approved these changes Dec 10, 2024

View reviewed changes

Finomnis dismissed danieldegrasse’s stale review via fc36d41 December 10, 2024 22:05

Finomnis force-pushed the flexspi_flash_mt25 branch 3 times, most recently from 3cfb082 to 3d37180 Compare December 10, 2024 22:09

Finomnis force-pushed the flexspi_flash_mt25 branch from 3d37180 to c3debf1 Compare December 10, 2024 22:11

Finomnis requested a review from danieldegrasse December 10, 2024 22:13

danieldegrasse approved these changes Dec 10, 2024

View reviewed changes

de-nordic approved these changes Dec 16, 2024

View reviewed changes

kartben merged commit a9bdaf4 into zephyrproject-rtos:main Dec 16, 2024
24 checks passed

Finomnis deleted the flexspi_flash_mt25 branch December 16, 2024 08:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: flash: flexspi: Add 4-byte addressing support for MT25 family #82532

drivers: flash: flexspi: Add 4-byte addressing support for MT25 family #82532

Finomnis commented Dec 4, 2024 •

edited

Loading

danieldegrasse left a comment

danieldegrasse Dec 4, 2024

Finomnis Dec 4, 2024

danieldegrasse Dec 4, 2024

Finomnis Dec 4, 2024

danieldegrasse Dec 4, 2024

Finomnis Dec 4, 2024

danieldegrasse Dec 4, 2024

Finomnis Dec 4, 2024 •

edited

Loading

danieldegrasse Dec 4, 2024

Finomnis Dec 4, 2024

danieldegrasse Dec 4, 2024

de-nordic Dec 4, 2024

Finomnis Dec 9, 2024

de-nordic Dec 10, 2024

Finomnis Dec 10, 2024

Finomnis commented Dec 4, 2024

danieldegrasse left a comment

danieldegrasse commented Dec 4, 2024

Finomnis commented Dec 4, 2024

Finomnis commented Dec 4, 2024 •

edited

Loading

Finomnis commented Dec 4, 2024 •

edited

Loading

danieldegrasse commented Dec 4, 2024

Finomnis commented Dec 4, 2024 •

edited

Loading

Finomnis commented Dec 5, 2024

danieldegrasse commented Dec 5, 2024

danieldegrasse commented Dec 10, 2024

Finomnis commented Dec 10, 2024 •

edited

Loading

Finomnis commented Dec 10, 2024

drivers: flash: flexspi: Add 4-byte addressing support for MT25 family #82532

drivers: flash: flexspi: Add 4-byte addressing support for MT25 family #82532

Conversation

Finomnis commented Dec 4, 2024 • edited Loading

danieldegrasse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Finomnis Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Finomnis commented Dec 4, 2024

danieldegrasse left a comment

Choose a reason for hiding this comment

danieldegrasse commented Dec 4, 2024

Finomnis commented Dec 4, 2024

Finomnis commented Dec 4, 2024 • edited Loading

Finomnis commented Dec 4, 2024 • edited Loading

danieldegrasse commented Dec 4, 2024

Finomnis commented Dec 4, 2024 • edited Loading

Finomnis commented Dec 5, 2024

danieldegrasse commented Dec 5, 2024

danieldegrasse commented Dec 10, 2024

Finomnis commented Dec 10, 2024 • edited Loading

Finomnis commented Dec 10, 2024

Finomnis commented Dec 4, 2024 •

edited

Loading

Finomnis Dec 4, 2024 •

edited

Loading

Finomnis commented Dec 4, 2024 •

edited

Loading

Finomnis commented Dec 4, 2024 •

edited

Loading

Finomnis commented Dec 4, 2024 •

edited

Loading

Finomnis commented Dec 10, 2024 •

edited

Loading