zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.67k stars 6.52k forks source link

flash_stm32_ospi: OSPI wr in OPI/STR mode is for 32bit address only #49406

Closed tobias-aunbol closed 2 years ago

tobias-aunbol commented 2 years ago

Describe the bug I'm using littleFS mounted on the external flash MX25LM51245 on the b_u585i_iot02a board. After https://github.com/zephyrproject-rtos/zephyr/pull/46794/commits/a6bce5e9bfc2d9dc20fc60d42446f5c9082b0e47 I keep getting error:

flash_stm32_ospi: OSPI wr in OPI/STR mode is for 32bit address only from flash driver. When i'm trying to either make a new directory or a new file with littleFS. Don't know if there are some new configurations that i should be aware of or this is a bad mix of littleFS and the ospi driver? Target: B-U585I-IOT02A board with the MX25LM51245 SPI flash My board .dts has the following nodes of interest but the littleFS node can also just be added to b_u585i_io202a.dts: ``` fstab { compatible = "zephyr,fstab"; lfs_cfg: lfs { compatible = "zephyr,fstab,littlefs"; mount-point = "/lfs"; partition = <&external_storage_partition>; read-size = <16>; prog-size = <16>; cache-size = <64>; lookahead-size = <64>; block-cycles = <512>; automount; }; }; mx25lm51245: ospi-nor-flash@0 { compatible = "st,stm32-ospi-nor"; label = "MX25LM51245"; reg = <0>; ospi-max-frequency = ; size = ; spi-bus-width = ; data-rate = ; four-byte-opcodes; status = "okay"; partitions { compatible = "fixed-partitions"; #address-cells = <1>; #size-cells = <1>; external_storage_partition: partition@0 { label = "ext-storage"; reg = <0x00000000 DT_SIZE_M(4)>; }; }; ``` **To Reproduce** Steps to reproduce the behavior: 1. Add the lfs_cfg node to b_u585i_io02a.dts 2. Beside my own application i have tested zephyr/samples/drivers/flash_shell 3. Add CONFIG_FILE_SYSTEM=y CONFIG_FILE_SYSTEM_LITTLEFS=y CONFIG_FILE_SYSTEM_SHELL=y to prj.conf for the flash_shell sample 4. west build -p -b b_u585i_io2a.dts /zephyr/samples/drivers/flash_shell 5. west flash 6. Boot target and watch terminal 7. littlefs should succesfully be automounted and everything looks alright: ``` uart:~$ *** Booting Zephyr OS build zephyr-v3.1.0-3677-gd1fdee7b67d2 *** Found flash controller flash-controller@40022000. Flash I/O commands can be run. flash_stm32_ospi: OSPI flash config is OPI / DTR ISM330DHCX: chip id 0x6b littlefs: littlefs partition at /lfs littlefs: LittleFS version 2.4, disk version 2.0 littlefs: FS at MX25LM51245:0x0 is 1024 0x1000-byte blocks with 512 cycle littlefs: sizes: rd 16 ; pr 16 ; ca 64 ; la 64 littlefs: /lfs mounted littlefs: Automount /lfs succeeded ``` 8. All write attemps fail try: fs mkdir /lfs/test or fs write /lfs/test.txt 01 02 03 04 0a these will give the following errors: ``` Error creating dir[-5] flash_stm32_ospi: OSPI wr in OPI/STR mode is for 32bit address only fs: failed to create directory (-5) ``` **Expected behavior** Should be able to successfully make new directory or write new file using littleFS **Impact** Not able to use littleFS on MX25LM51245 **Environment (please complete the following information):** - Linux Ubuntu - Zephyr SDK - SHA: a6bce5e9bfc2d9dc20fc60d42446f5c9082b0e47
FRASTM commented 2 years ago

Trying this samples/drivers/flash_shell with corrresponding b_u585i_iot02a.conf and b_u585i_iot02a.overlay gives :

uart:~$ *** Booting Zephyr OS build zephyr-v3.1.0-3811-g3217d9737530  ***
Found flash controller flash-controller@40022000.
Flash I/O commands can be run.

uart:~$ fs pwd
/
uart:~$ fs mkdir /lfs/test
Error creating dir[-2]
<err> fs: mount point not found!!
uart:~$ 
tobias-aunbol commented 2 years ago

Trying this samples/drivers/flash_shell with corrresponding b_u585i_iot02a.conf and b_u585i_iot02a.overlay gives :

uart:~$ *** Booting Zephyr OS build zephyr-v3.1.0-3811-g3217d9737530  ***
Found flash controller flash-controller@40022000.
Flash I/O commands can be run.

uart:~$ fs pwd
/
uart:~$ fs mkdir /lfs/test
Error creating dir[-2]
<err> fs: mount point not found!!
uart:~$ 

@FRASTM This looks like that something isn't right with the configuration under the LFS node in the device tree, have you included the LFS node in the b_u585i_iot02.dts and is the partition phandle in the lfs node pointing to the partition on the external flash? When i run the sample with the configuration I desribed, the littleFS at least is correctly mounted.

FRASTM commented 2 years ago

Trying this samples/drivers/flash_shell with corrresponding b_u585i_iot02a.conf and b_u585i_iot02a.overlay gives :

uart:~$ *** Booting Zephyr OS build zephyr-v3.1.0-3811-g3217d9737530  ***
Found flash controller flash-controller@40022000.
Flash I/O commands can be run.

uart:~$ fs pwd
/
uart:~$ fs mkdir /lfs/test
Error creating dir[-2]
<err> fs: mount point not found!!
uart:~$ 

@FRASTM This looks like that something isn't right with the configuration under the LFS node in the device tree, have you included the LFS node in the b_u585i_iot02.dts and is the partition phandle in the lfs node pointing to the partition on the external flash? When i run the sample with the configuration I desribed, the littleFS at least is correctly mounted.

config put in the samples/drivers/flash_shell/boards : b_u585i_iot02a.conf b_u585i_iot02a.overlay

@tobias-aunbol could you please amend

tobias-aunbol commented 2 years ago

@FRASTM yes that looks fine, it's the same settings as i have been testing with on my b_u585i_iot02a board, i still haven't experienced the -2 mount point not found error yet though.

Edit: I just digged a bit in it, and i think the reason why you get mount error is because that the mx25lm51245 is already defined in b_u585i_io2a-common.dtsi with the sfdo-bfp property defined. If i also have this define in my node i also get mount error and the following error messages:

<err> littlefs: WEST_TOPDIR/modules/fs/littlefs/lfs.c:1076: Corrupted dir pair at {0x0, 0x1}
<wrn> littlefs: can't mount (LFS -84); formatting
<err> flash_stm32_ospi: Error: wrong sector size 0x2
<err> littlefs: format failed (LFS -5)
<err> fs: fs mount error (-5)
<err> littlefs: Automount /lfs failed: -5

After some investigations and debugging i can also see that it maybe has something to do with the sfdp property in the node. Because if I delete the sfdp property (which i have in my board). The dev pointer to flash_stm32_ospi_data->address_width will make the addressSize be set to HAL_OSPI_ADDRESS_24_BITS in stm32_ospi_hal_address_size function in the driver which will eventually make ospi_write_access fail.

But i thought, (can't remember why) that the sfdp-bfp bytes was retrieved at runtime if this property wasn't defined. So if the default sfdp-bfp values from b_u585_iot02-common.dtsi gives mounting error, and the runtime retrieved parameters makes ospi_write_access fails, then what are the correct values and how to find them?

tobias-aunbol commented 2 years ago

Update: Here's my findings so far.

I have investigated and tried to understand SFDP, and i can see that there is a sample provided in samples/drivers/jesd216 where the paramters can be read out. To use this it requires the flash driver to implement sfdp_read from the flash.h interface. This function is actually already implemented in the driver but not referenced to the flash API. I would propose a change like this, which is same way it has been done in nrf_qspi_nor.c :

static const struct flash_driver_api flash_stm32_ospi_driver_api = {
    .read = flash_stm32_ospi_read,
    .write = flash_stm32_ospi_write,
    .erase = flash_stm32_ospi_erase,
    .get_parameters = flash_stm32_ospi_get_parameters,
    #if defined(CONFIG_FLASH_JESD216_API) // This is the change
    .sfdp_read = ospi_read_sfdp, //This is the change
#endif /* CONFIG_FLASH_JESD216_API */
#if defined(CONFIG_FLASH_PAGE_LAYOUT)
    .page_layout = flash_stm32_ospi_pages_layout,
#endif
};

This will not solve the forementioned problem but was just something that i stumpled upon. Then i could run the sample and read out the SFDP parameters from the MX25lm51245. I haven't been investigating fully into this yet and i don't know the exact consequences of them, but there were several things i think is interessting:

It looks like the minor number when reading from device is 8, but when in the default sfdp in b_u585_iot02-common.dtsi this is set to 6. This is maybe not a bug, but it shows that their is something that is not alligned with the provided sfdp and what can actually be read from the device.

The most interessting thing, and what is related the most to the problem is that when reading out DW1 i get: 0xff8a20e5 in bin: 111 1111 1000 1010 0010 0000 1110 0101 When i read the jesd216F standard: https://www.jedec.org/system/files/docs/JESD216F-02.pdf (page 40) i can see that bit [18:17] is Address bytes and this is set to 01 (marked with bold) which according to the standard is 3- or 4byte addressing.

in stm32_ospi_hal_address_size in the flash driver is the address_width set based on:

static void spi_nor_process_bfp_addrbytes(const struct device *dev,
                      const uint8_t jesd216_bfp_addrbytes)
{
    struct flash_stm32_ospi_data *data = dev->data;

    if (jesd216_bfp_addrbytes == JESD216_SFDP_BFP_DW1_ADDRBYTES_VAL_4B) {
        data->address_width = 4U;
    } else {
        data->address_width = 3U;
    }

JESD216_SFDP_BFP_DW1_ADDRBYTES_VAL_4B is defined in jesd216.h:

#define JESD216_SFDP_BFP_DW1_ADDRBYTES_VAL_3B 0
#define JESD216_SFDP_BFP_DW1_ADDRBYTES_VAL_3B4B 1
#define JESD216_SFDP_BFP_DW1_ADDRBYTES_VAL_4B 2

the check in the function should (for what i can see) be based on JESD216_SFDP_BFP_DW1_ADDRBYTES_VAL_3B4B or the HAL_OSPI_ADDRESS_32_BITS shouldn't be set according to the address_width.

I would definitly like some feedback on this if possible: @FRASTM @erwango

FRASTM commented 2 years ago

I have investigated and tried to understand SFDP, and i can see that there is a sample provided in samples/drivers/jesd216 where the paramters can be read out. To use this it requires the flash driver to implement sfdp_read from the flash.h interface. This function is actually already implemented in the driver but not referenced to the flash API. I would propose a change like this, which is same way it has been done in nrf_qspi_nor.c :

That's included in the https://github.com/zephyrproject-rtos/zephyr/pull/47962

tobias-aunbol commented 2 years ago

@FRASTM @erwango will #47962 also include a change so the data->address_width won't be set to 3U if it's JESD216_SFDP_BFP_DW1_ADDRBYTES_VAL_3B4B as i mentioned in my last comment?. Because if so, then i think this will fix this issue. I tried to test #47962 but this is getting me hardfault, which i already have commented on #47962.

FRASTM commented 2 years ago

Yes, finally, the static void spi_nor_process_bfp_addrbytes() should it be :

    if (jesd216_bfp_addrbytes == JESD216_SFDP_BFP_DW1_ADDRBYTES_VAL_3B4B) {
        data->address_width = 4U;
    } else {
        data->address_width = 3U;
    }

This is the commit 20c0b2267553327a7e5e8961fbbc6fb2d4186a7b

FRASTM commented 2 years ago

However, still not getting the fs :

*** Booting Zephyr OS build zephyr-v3.1.0-4719-gb3ee03749d4c  ***    
Found flash controller flash-controller@40022000.                            
Flash I/O commands can be run.                                               
[00:00:00.140,000] <inf> flash_stm32_ospi: OSPI flash config is OPI / DTR    
[00:00:00.140,000] <inf> flash_stm32_ospi: Read SFDP from octoFlash          
[00:00:00.140,000] <inf> flash_stm32_ospi: Read SFDP from octoFlash          
uart:~$ fs pwd                                                               
fs: command not found                                                        
uart:~$ 
tobias-aunbol commented 2 years ago

@FRASTM it looks like that no filesystem is mounted or activated are you reproducing by the steps i provided and remember the .overlay files yourself provided earlier in this thread? I just tested the PR #47962 and everything looks fine here so i will close this issue.