flash_stm32_ospi.c: Unable to erase flash partition using flash_map API

[tobias-aunbol]

Describe the bug
When using:
flash_area_erase(fa, 0. fa->fa_size)
flash_stm32_ospi returns error with:

LOG_ERR("Error: wrong sector size 0x%x", size);
and returning error code -ENOTSUP

This is due to a check on the SPI_NOR_SECTOR_SIZE and dev_cfg->flash_size in flash_stm32_ospi_erase

Which only make it (for what i can see) able to erase either a sector or the whole chip.
As for the datasheet the erase command can be used on sector (4K byte), (64K byte) and whole chip.

I have successfully erased the whole chip by using the flash_driver_api.
The problem with that is, that i would rather use the flash_map API as this make a cleaner cut to the rest of the application, and i would like to use different partitions, and could be nice to be able to erase data on specific partition. The partition size is alligned with the sector/block size.

Target:
B-U585I-IOT02A board with the MX25LM51245 SPI flash

To Reproduce

1. Make a flash partition in .dts.
   From application:
2. Get a reference to it by using flash_area_open
3. Call flash_area_erase with reference from flash_area_open:
   ex. flash_area_erase(fa, 0, fa->fa_size)

Expected behavior
Expected driver to clear sectors from start address (offset) to partition size.
Of course this is expected that a partition size alligned with the sector size /block size is defined in partition in .dts.
If partition size isn't matching then maybe a fail is appropriate or erasing the closest possible without writing into next partition.

Impact
Not able to use flash_stm32_ospi with flash_map API

Environment (please complete the following information):

• OS: Linux
• Toolchain Zephyr SDK
• SHA: a5cd5f4a0c9e48c78600d291a73d7c5669cba236

Additional context

from .dts file:

mx25lm51245: ospi-nor-flash@0 {
		compatible = "st,stm32-ospi-nor";
		label = "MX25LM51245";
		reg = <0>;
		ospi-max-frequency = <DT_FREQ_M(50)>;
		size = <DT_SIZE_M(64)>;
		spi-bus-width = <OSPI_OPI_MODE>;
		data-rate = <OSPI_DTR_TRANSFER>;
		status = "okay";

		partitions {
			compatible = "fixed-partitions";
			#address-cells = <1>;
			#size-cells = <1>;

			external_storage_partition: partition@0 {
				label = "extstorage";
				reg = <0x00000000 0x500000>; //also tested with 1000 (works) and 4000

Test made:

void test_flash_area_erase(const struct flash_area* fa, unsigned int fa_id) {

    int err = flash_area_open(fa_id, &fa);

    if(!err && fa != nullptr) {
        
        err = flash_area_erase(fa, 0, fa->fa_size);
        
        if(err)
            LOG_ERR("Error erasing flash area: %d", err);
        
        flash_area_close(fa);
    }
    else {
        LOG_ERR("Unable to open flash area: %d", err);
    }

}

void main(void) {

    LOG_INF("test_flash_area_erase");

    const struct flash_area* m_flash_area = nullptr;
    unsigned int flash_area_id = FLASH_AREA_ID(extstorage);

    test_flash_area_erase(m_flash_area, flash_area_id);


}

This output the following:

[00:00:00.141,000] <inf> flash_stm32_ospi: OSPI flash config is OPI / DTR
[00:00:00.142,000] <inf> littlefs: littlefs partition at /lfs
*** Booting Zephyr OS build zephyr-v3.1.0-690-ga5cd5f4a0c9e  ***
[00:00:00.142,000] <inf>  app: test_flash_area_erase
[00:00:00.142,000] <err> flash_stm32_ospi: Error: wrong sector size 0x500000
[00:00:00.142,000] <err> app: Error erasing flash area: -134

[Laczen]

@tobias-aunbol, the definition of the external partition seems wrong, this is not aligned to the erase-block.

[tobias-aunbol]

@Laczen thanks a lot for your comment. Can you please elaborate further? Is it the size that you are thinking of?

[Laczen]

@Laczen thanks a lot for your comment. Can you please elaborate further? Is it the size that you are thinking of?

Yes, the size of 0x4FFFFF, this can't be correct.

[tobias-aunbol]

I'm no expert (yet ;) in using external SPI flashes so thanks alot! This should then be the beginning of the next block or sector to be alligned with the erase-block? so 0x500000?

[Laczen]

I'm no expert (yet ;) in using external SPI flashes so thanks alot! This should then be the beginning of the next block or sector to be alligned with the erase-block? so 0x500000?

It is the same for internal flashes (and also for arrays, ...). When you specify a size the largest index you can write to is size - 1 (because the index starts at 0). So yes 0x500000, but you can start with trying with 0x1000 (4kB), or 0x4000 (16kB). Set it to a value that you know is supported.

[tobias-aunbol]

oh my god of course, thanks a lot @Laczen, I actually feel kind of stupid now, this fixed the issue. Thanks for pointing it out. I'll mark this issue as close

[Laczen]

No problem, glad that your problem is resolved.

[tobias-aunbol]

Alright i jumped the conclusion a bit to quick, i made a new test again this morning with fresh eyes, and even though my partition wasn't alligned with the erase-block. The flash_stm32_ospi.c still returns with the same error. So i'll reopen the issue

[Laczen]

@tobias-aunbol, so even with a aligned erase-block (both offset and size are aligned to erase blocks) you are getting an error ? Could you please update the issue with the latest description of the setup that fails.

[tobias-aunbol]

@Laczen yeah unfortunatly, i have made a small update to the description

[Laczen]

@tobias-aunbol, could you add a little test program that you are using to validate (or are you running a zephyr test) ?

[Laczen]

@Laczen yeah unfortunatly, i have made a small update to the description

And it is still reporting that the sector size is wrong ?

[Laczen]

@tobias-aunbol, IMO the problem is in the flash_stm32_ospi driver, and is not related to flash_map:
https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/flash/flash_stm32_ospi.c#L726

This check should not validate that size != SECTOR_SIZE but should verify that size is not a multiple of SECTOR_SIZE (and maybe sector size should not come a CONFIG but should be read from the device.

@GeorgeCGV, could you check this ?

[tobias-aunbol]

@Laczen I have updated with my test software. I totally agree with you on that it's related to the flash_stm32_ospi.c driver. Because if i run the same test with a internal flash partition it works as expected

[Laczen]

@Laczen I have updated with my test software. I totally agree with you on that it's related to the flash_stm32_ospi.c driver. Because if i run the same test with a internal flash partition it works as expected

To verify it should work for a specific partition size that is equal to 4kB (that I think is SPI_NOR_SECTOR_SIZE).

And to correct it you should replace size != SPI_NOR_SECTOR_SIZE line 726 with (size % SPI_NOR_SECTOR_SIZE) != 0

[tobias-aunbol]

@Laczen I have updated with my test software. I totally agree with you on that it's related to the flash_stm32_ospi.c driver. Because if i run the same test with a internal flash partition it works as expected

To verify it should work for a specific partition size that is equal to 4kB (that I think is SPI_NOR_SECTOR_SIZE).

And to correct it you should replace size != SPI_NOR_SECTOR_SIZE line 726 with (size % SPI_NOR_SECTOR_SIZE) != 0

Yes i agree with you, i just ran the test with size 0x1000, and that outputs:
[00:00:00.141,000] flash_stm32_ospi: OSPI flash config is OPI / DTR
[00:00:00.142,000] littlefs: littlefs partition at /lfs
*** Booting Zephyr OS build zephyr-v3.1.0-690-ga5cd5f4a0c9e ***
[00:00:00.142,000] app: test_flash_area_erase

And you proposed changes seems to work

[FRASTM]

PR to come with the proposed change :

@@ -724,7 +724,7 @@ static int flash_stm32_ospi_erase(const struct device *dev, off_t addr,
 		return -EINVAL;
 	}
 
-	if ((size != SPI_NOR_SECTOR_SIZE) && (size < dev_cfg->flash_size)) {
+	if (((size % SPI_NOR_SECTOR_SIZE) != 0) && (size < dev_cfg->flash_size)) {
 		LOG_ERR("Error: wrong sector size 0x%x", size);
 		return -ENOTSUP;
 	}

[FRASTM]

BTW, did you face any other issue during your test ( refering to #46740) with the b_u585i_iot02a like :

JEDEC OSPI-NOR SPI flash testing                                                
==========================                                                      
SPI flash driver MX25LM51245 was not found! 

[FRASTM]

@tobias-aunbol, IMO the problem is in the flash_stm32_ospi driver, and is not related to flash_map: https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/flash/flash_stm32_ospi.c#L726

This check should not validate that size != SECTOR_SIZE but should verify that size is not a multiple of SECTOR_SIZE (and maybe sector size should not come a CONFIG but should be read from the device.

@GeorgeCGV, could you check this ?

--> I agree that the sector size and other octo flash info should come from the MX25LM51245 device. But unfortunately it does not answer correctly to the read SFDP command (JESD216_SFDP_MAGIC does not match). That's why we put a dummy sfdp-bfp table in the DTS node. Even though this table is probably not correctly reversely filled to hold all the expected info driver requires.
Actually, the octoflash SPI_NOR_SECTOR_SIZE, is 0x1000 (from the MX25LM51245 datasheet).

[Laczen]

@FRASTM It is ok to use SPI_NOR_SECTOR_SIZE.