drivers: add STM32 Quad SPI driver

[mnkp]

This commit adds support for STM32 Quad SPI driver targeting single, dual or quad SPI Flash memories.

The driver is functional, basic tests were performed on a custom board with AT25SL641 flash memory, however in the current state it is still a proof of concept.

New features:

• the driver provides basic support for reading SFDP (Serial Flash Discoverable Parameters) from the external flash.

Limitations:

• the driver relies on STM32 HAL QSPI driver working in interrupt mode. Unfortunately, the implementation of the interrupt mode is quite inefficient. The ST driver is unable to send data to Quad SPI peripheral fast enough to sustain standard SPI (1-1-1) mode with Quad SPI module working at half AHB speed. The transmission succeeds due to clock stalling.
• due to speed limitations the drivers supports currently SPI mode only.
• the driver doesn't support fast read operations.
• the driver doesn't make use of a built in status polling mode, instead manually sends status query commands

The DTS node describing external flash connected to Quad SPI peripheral is defined as

&quadspi{
        status = "okay";

        at25sl641: qspi-nor-flash@0 {
                compatible = "jedec,spi-nor";
                label = "AT25SL641";
                reg = <0>;
                spi-max-frequency = <133000000>;
                /* 64 Megabits = 8 Megabytes */
                size = <0x4000000>;
                jedec-id = [1f 43 17];
        };
};

where qspi-nor-flash@0 is fixed. This should be reworked, however requires further discussion.

[mnkp]

@arvid-r, @QSQCaito

[zephyrbot]

All checks are passing now.

checkpatch (informational only, not a failure)

-:16: WARNING:MISSING_BREAK: Possible switch case/default not preceded by break or fallthrough comment
#16: FILE: drivers/clock_control/clock_stm32_ll_common.c:107:
+	case STM32_CLOCK_BUS_AHB3:

-:334: WARNING:LONG_LINE: line over 80 characters
#334: FILE: drivers/flash/flash_stm32_qspi.c:256:
+		dev_data->flash_params.size = (size_n < 31) ? (1 << size_n) : (1 << 31);

-:463: WARNING:LONG_LINE: line over 80 characters
#463: FILE: drivers/flash/flash_stm32_qspi.c:385:
+			to_write = SPI_NOR_PAGE_SIZE - (addr % SPI_NOR_PAGE_SIZE);

-:531: WARNING:LONG_LINE: line over 80 characters
#531: FILE: drivers/flash/flash_stm32_qspi.c:453:
+			for (int i = ARRAY_SIZE(flash_params->sector_layout) - 1; i >= 0; i--) {

-:532: WARNING:LONG_LINE: line over 80 characters
#532: FILE: drivers/flash/flash_stm32_qspi.c:454:
+				u32_t block_size = 1 << flash_params->sector_layout[i].size_n;

-:535: WARNING:LONG_LINE: line over 80 characters
#535: FILE: drivers/flash/flash_stm32_qspi.c:457:
+				    && SPI_NOR_IS_ADDR_ALIGNED(addr, block_size)) {

-:538: WARNING:LONG_LINE: line over 80 characters
#538: FILE: drivers/flash/flash_stm32_qspi.c:460:
+					cmd_erase.Instruction = flash_params->sector_layout[i].erase_cmd;

-:783: WARNING:LONG_LINE: line over 80 characters
#783: FILE: drivers/flash/flash_stm32_qspi.c:705:
+	ret = qspi_process_jedec_flash_parameter_table(dev, jedec_pt_addr, jedec_pt_len);

-:790: WARNING:LONG_LINE: line over 80 characters
#790: FILE: drivers/flash/flash_stm32_qspi.c:712:
+	dev_data->layout.pages_size = 1 << dev_data->flash_params.sector_layout[0].size_n;

-:791: WARNING:LONG_LINE: line over 80 characters
#791: FILE: drivers/flash/flash_stm32_qspi.c:713:
+	dev_data->layout.pages_count = dev_data->flash_params.size / dev_data->layout.pages_size;

-:815: WARNING:LONG_LINE: line over 80 characters
#815: FILE: drivers/flash/flash_stm32_qspi.c:737:
+		.spi_max_frequency = DT_PROP(QSPI_FLASH_MODULE(id, 0), spi_max_frequency), \

-:911: WARNING:LONG_LINE: line over 80 characters
#911: FILE: drivers/flash/sfdp.h:58:
+static inline u8_t sfdp_get_param_header_pt_length(union sfdp_header *param_header)

-:922: WARNING:LONG_LINE: line over 80 characters
#922: FILE: drivers/flash/sfdp.h:69:
+static inline off_t sfdp_get_param_header_pt_pointer(union sfdp_header *param_header)

-:958: WARNING:LONG_LINE: line over 80 characters
#958: FILE: drivers/flash/sfdp.h:105:
+static inline u8_t sfdp_pt_1v0_dw8_get_sector_type_2_erase_opcode(union sfdp_dword dw)

-:978: WARNING:LONG_LINE: line over 80 characters
#978: FILE: drivers/flash/sfdp.h:125:
+static inline u8_t sfdp_pt_1v0_dw8_get_sector_type_1_erase_opcode(union sfdp_dword dw)

-:998: WARNING:LONG_LINE: line over 80 characters
#998: FILE: drivers/flash/sfdp.h:145:
+static inline u8_t sfdp_pt_1v0_dw9_get_sector_type_4_erase_opcode(union sfdp_dword dw)

-:1018: WARNING:LONG_LINE: line over 80 characters
#1018: FILE: drivers/flash/sfdp.h:165:
+static inline u8_t sfdp_pt_1v0_dw9_get_sector_type_3_erase_opcode(union sfdp_dword dw)

-:1042: WARNING:LONG_LINE_COMMENT: line over 80 characters
#1042: FILE: drivers/flash/spi_nor.h:38:
+#define SPI_NOR_CMD_RSFDP       0x5A    /* Read Serial Flash Discovery Parameter */

- total: 0 errors, 18 warnings, 1199 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPDX_LICENSE_TAG SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

[arvid-r]

Thanks! Interesting. sfdp.h looks like it could be reused by others too, right?

Correct me if I'm wrong, but this is a slightly different approach than the current Nordic driver, by using compatible = "jedec,spi-nor" for the chip itself. The nordic implementation instead uses this in the devicetree (with a binding on the chip rather than the flash-controller node):

mx25r64: mx25r6435f@0 {
		compatible = "nordic,qspi-nor";
		reg = <0>;
		writeoc = "pp4io";
		readoc = "read4io";
		sck-frequency = <8000000>;
		label = "MX25R64";
		jedec-id = [c2 28 17];
		size = <67108864>;
		has-be32k;
		has-dpd;
		t-enter-dpd = <10000>;
		t-exit-dpd = <35000>;
	};

Or is this case different somehow?

[mnkp]

Thanks! Interesting. sfdp.h looks like it could be reused by others too, right?

Yes, sfdp.h is meant to be used on all platforms. So far, I didn't have a chance to test it on a big-endian system but it's supposed to work there too. There are quite a lot of parameters that can be extracted from SFDP table and this file is going to grow a lot. I've decided to use inline functions that take a dedicated union sfdp_dword type parameter. But that's just one way to do it. Chromium guys use the macro approach sfdp.h. That makes the file more compact and in fact more readable. However, my feeling is that macro based approach will generate larger code size on big-endian platforms. But again, I didn't test it. Compiler optimizations may be good enough that at the end it will not matter. Comments are welcomed.

Correct me if I'm wrong, but this is a slightly different approach than the current Nordic driver, by using compatible = "jedec,spi-nor" for the chip itself.

The compatible = "jedec,spi-nor" indicates that the node should be handled by drivers/flash/spi_nor.c driver, which of course is incorrect. The compatible needs to be changed. However, I wasn't sure how. The node is supposed to describe an external flash which is independent from the QSPI module to which it is connected. Going the Nordic way and adding a new compatible = "stm32,qspi-nor" didn't feel right. We would duplicate compatibles that are meant to describe the same external flash component. We could use compatible = "jedec,qspi-nor" but we don't have at the moment generic QSPI API, nor the generic qspi-nor driver that would handle the node. Another possibility would be to use child bindings and do not add a new compatible at all. After all the driver that handles the node is the flash controller itself. In this case st,stm32-quadspi. This part needs a bit more discussion.

[pabigot]

The compatible = "jedec,spi-nor" indicates that the node should be handled by drivers/flash/spi_nor.c driver, which of course is incorrect.

That's not entirely accurate. jedec,spi-nor is the compatible used in Linux to describe these devices. It is not at all intended to be bound to that particular driver: use of CONFIG_SPI_NOR=y selects the implementation. The core parts of the binding could just as well be used with a CONFIG_STM32_QSPI_NOR=y or other driver.

nordic,qspi-nor extends a common subset of jedec,spi-nor because the Nordic peripheral can only be treated as a generic QSPI controller through rather tortured misuse of the fallback "just run this command" capability. It's really designed as a high-level flash interface, and the configuration options it requires reflect that. Because we weren't using SFDP for reasons mentioned in #25851 it was necessary to add a lot of other material to tell the driver how to deal with specific devices.

Assuming we do move to using SFDP we may be able to reduce the differences in the devicetree descriptions and use jedec,spi-nor universally. I'd prototyped Nordic QSPI configuration using SFDP content over a year ago in a different framework. Something like that could work in Zephyr as well: nrf_qspi_nor would still be the driver that would be used, but the devicetree node could be limited to the common jedec,spi-nor content.

I haven't yet looked at this driver nor the NXP one but I would expect moving to SFDP would allow those to be described compatibly as well.

Thanks for kicking this off. It's past time we started widening the support for flash devices in Zephyr.

[arvid-r]

Ok, makes sense. I think I will try to rework the NXP case then, to use jedec,spi-nor and then extend it to something like nxp,qspi-nor if needed. That should be easy to rework later if we go for SFDP. Sorry, talking about #25669 here, but these are very much related.

[arvid-r]

Will this work with MCUBOOT? It looks for DT_CHOSEN_ZEPHYR_FLASH_CONTROLLER_LABEL. But I think that is rather something that should change on the MCUBOOT side? @pabigot

[mnkp]

The compatible = "jedec,spi-nor" indicates that the node should be handled by drivers/flash/spi_nor.c driver, which of course is incorrect.

That's not entirely accurate. jedec,spi-nor is the compatible used in Linux to describe these devices. It is not at all intended to be bound to that particular driver: use of CONFIG_SPI_NOR=y selects the implementation. The core parts of the binding could just as well be used with a CONFIG_STM32_QSPI_NOR=y or other driver.

The device tree specification states:
The compatible property value consists of one or more strings that define the specific programming model for the device. This list of strings should be used by a client program for device driver selection. The property value consists of a concatenated list of null terminated strings, from most specific to most general. They allow a device to express its compatibility with a family of similar devices, potentially allowing a single device driver to match against several devices.

While we don't use the value of the compatible property to select the device driver in Zephyr (yet), we shouldn't purposefully violate the specification.

That said, in Linux the spi-nor driver is able to handle both SPI and QSPI flash devices and they consistently use jedec,spi-nor compatible for the flash nodes.

[erwango]

@mnkp, about the issue reported on QSPI mode, can you raise an issue in https://github.com/STMicroelectronics/STM32CubeL4/issues ?
I think it will be the best way to get fast and direct feedback.

[mnkp]

The compatible = "jedec,spi-nor" indicates that the node should be handled by drivers/flash/spi_nor.c driver, which of course is incorrect.

Let's move the discussion regarding compatible property to #25958 since the problem is not limited to a single driver.

[mnkp]

@mnkp, about the issue reported on QSPI mode, can you raise an issue in https://github.com/STMicroelectronics/STM32CubeL4/issues ?
I think it will be the best way to get fast and direct feedback.

Thanks, I will. However, considering the amount of data QSPI module needs to transfer I reckon that using DMA is the only viable option for this driver. I didn't do it in this first, draft PR version since setting up DMA is more complex. I will try to update the implementation for the non draft PR.

[nzmichaelh]

@mnkp FYI I'm looking at adding QSPI support for the SAME51 to control the 2 MiB NOR flash on the ItsyBitsy M4. I did it by forking spi_nor.c as the QSPI peripheral has a very different interface to the Zephyr SPI API.

The WIP commits are at a803bd1 and b1743bd

I'd much rather not fork though, such as having a common NOR flash library or driver and only having the low level command and address code be SAM0 specific.

[mtahirbutt]

Hi, I am trying mx25r6435f via QSPI to run on disco_l475_iot1 board, but i do not see signal acitivty on CS, SO, SI, SCLK. I used the correct pinmux for BSP, any idea about that?
I used this overlay file,
&quadspi{
status = "okay";

    mx25r6435f: qspi-nor-flash@0 {
            compatible = "jedec,spi-nor";
            label = "MX25R6435F";
            reg = <0>;
            spi-max-frequency = <80000000>;
            /* 64 Megabits = 8 Megabytes */
            size = <0x4000000>;
            jedec-id = [1f 43 17];
    };

};

[hjuul]

@mnkp, any plans to continue work on this PR now that #25851 is merged?

[giansta]

@mnkp Your branch is working well on our STM32L462VE based custom board with AT25SL641 flash memory,
We only needed to add support for the device in flash_map_default for MCUboot. The change is simple, we have it in our branch https://github.com/giansta/zephyr/tree/flash_stm32_qspi_gs, commit ef22c47.
It would be nice to have it merged here.
Thanks!

[mtahirbutt]

Hi, I am trying mx25r6435f via QSPI to run on disco_l475_iot1 board, but i do not see signal acitivty on CS, SO, SI, SCLK. I used the correct pinmux for BSP, any idea about that?
I used this overlay file,
&quadspi{
status = "okay";

    mx25r6435f: qspi-nor-flash@0 {
            compatible = "jedec,spi-nor";
            label = "MX25R6435F";
            reg = <0>;
            spi-max-frequency = <80000000>;
            /* 64 Megabits = 8 Megabytes */
            size = <0x4000000>;
            jedec-id = [1f 43 17];
    };

};

it works with disco_l475_iot1 with single SPI. thanks. but I am not able to write desired data to flash. it writes only '0' with PP flash command. someone has some idea what is wrong with writing.

[giansta]

@mnkp Your branch is working well on our STM32L462VE based custom board with AT25SL641 flash memory,
We only needed to add support for the device in flash_map_default for MCUboot. The change is simple, we have it in our branch https://github.com/giansta/zephyr/tree/flash_stm32_qspi_gs, commit ef22c47.
It would be nice to have it merged here.
Thanks!

@mnkp When our branch https://github.com/giansta/zephyr/tree/flash_stm32_qspi_gs was rebased to latest Zephyr master, we needed no more any change to have a running bootloader (thanks to commit 07fd283). So your branch just needs alignment to latest Zephyr (like we did in our commit f4fb6cd) to be merged.
Thanks.

[r2r0]

@mnkp Do you have any plans to continue work on this PR?
If not then do you have any objections to issue PRs based on your work?

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[mnkp]

Superseded by #30864.