dts: bindings: mtd: add erase-block-size property to spi-nor

[cfriedt]

Add an optional erase-block-size property to the jedec,spi-nor Devicetree binding.

This ensures that devices whose slot1_partition is on a SPI NOR flash device do not encounter the following warning from mcuboot.

Unable to determine size of slot1 partition, cannot calculate minimum sector usage

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[JarmouniA]

Was already proposed in #95152

[cfriedt]

Was already proposed in #95152

Hmmm... That PR seems to address write block size rather than erase block size (although I haven't read through all of the comments).

[JarmouniA]

Hmmm... That PR seems to address write block size rather than erase block size (although I haven't read through all of the comments).

Yes my bad, didn't pay close attention, but I think the same rule applies to the two.

[cfriedt]

Yes my bad, didn't pay close attention, but I think the same rule applies to the two.

Is the argument there that the write or erase block size of the spi nor flash device is a global software property?

Different flash devices can and do have different write or erase block sizes, so configuring that globally in software is not really representative of real world scenarios.

This change is mainly to fix a warning that arises when building MCUBoot, which should be avoided if easily possible, like by adding an (optional) DT property.

Without the DT binding, the warning can cause problems in some build environments, which is kind of a bug. I would be happy to file an issue for it.

[de-nordic]

why are we adding MCuboot parameter? I hate these MCUboot configuration scripts.
I think I will accept both this and the #95152, just to be over with this. But this is so incorrect to pick that info for the mcuboot.

We are basically adding this param to make warning go away, without reflecting the change in device driver.
I am not even sure we are naming it correctly, because this would me minimal erase block size? SPI mems often have entire range of erase blocks size, others have only single value. The driver selects which command to use, depending on range.
Should this param then limit driver to use only this size and command for it and not try to bundle pages together, or is that minimal one allowed and pages are bundled to at least have this size?

[cfriedt]

why are we adding MCuboot parameter?
We are basically adding this param to make warning go away, without reflecting the change in device driver.

It provides an (optional) compile-time-constant value for the erase size, which certainly helps the application (be it mcuboot or a user app). It's possible that patching MCUBoot would also work (although that patch might be more complex).

Would be great if the driver could make use of it, but I know that JEDEC is capable of reporting multiple erase sizes as well.

Of course, if spi_nor_erase() is called with an incorrect erase size, then a runtime error would be reported as well.

zephyr/drivers/flash/spi_nor.c

Line 1046 in 5d02fac

/* size must be a multiple of sectors */

[JarmouniA]

The proposed property already exists in a generic flash binding

zephyr/dts/bindings/mtd/soc-nv-flash.yaml

Line 8 in 169cf86

erase-block-size:

In my opinion, the external Flash nodes that are generating the MCUboot warning should just add the soc-nv-flash compatible and define erase-block-size. Same goes for write-block-size.

[cfriedt]

In my opinion, the external Flash nodes that are generating the MCUboot warning should just add the soc-nv-flash compatible and define erase-block-size. Same goes for write-block-size.

Just learned of that recently as well.

So the idea is to create an soc-nv-flash node under the jedec-spi-nor instance, and then declare the write and erase sizes under that. It's a little bit of a workaround, but I guess if it works.

[JarmouniA]

So the idea is to create an soc-nv-flash node under the jedec-spi-nor instance, and then declare the write and erase sizes under that. It's a little bit of a workaround, but I guess if it works.

Either create a child flash node, of the controller node, with compatible soc-nv-flash and declare the properties, which is the correct way to do it in my opinion, because the spi-nor is the controller, and the erase & write sizes are properties of the flash chip, but we will have to change the zephyr,flash chosen to the new child node,

Or just add soc-nv-flash as another compatible of the SPI NOR controller node and declare the properties there, but it will break the assumptions of some macros like

zephyr/include/zephyr/devicetree/fixed-partitions.h

Line 86 in fb7a74e

#define DT_MEM_FROM_FIXED_PARTITION(node_id) \

zephyr/include/zephyr/devicetree/fixed-partitions.h

Line 133 in fb7a74e

#define DT_FIXED_PARTITION_ADDR(node_id) \

[cfriedt]

Using the soc-nv-flash workaround.