Missing 'label' on nodes with 'compatible = "jedec,spi-nor"'

[ulfalizer]

dts/bindings/mtd/jedec,spi-nor.yaml includes dts/bindings/spi/spi-device.yaml, which says that any node with that binding must have a label property, but none of the nodes with compatible = "jedec,spi-nor" has a label property.

List for reference:

$ git grep jedec,spi-nor
boards/arm/bl654_dvk/bl654_dvk.dts:             compatible = "jedec,spi-nor";
boards/arm/mimxrt1015_evk/mimxrt1015_evk.dts:           compatible = "adesto,at25sf128a", "jedec,spi-nor";
boards/arm/mimxrt1020_evk/mimxrt1020_evk.dts:           compatible = "issi,is25wp064", "jedec,spi-nor";
boards/arm/mimxrt1050_evk/mimxrt1050_evk_qspi.dts:              compatible = "issi,is25wp064", "jedec,spi-nor";
boards/arm/mimxrt1060_evk/mimxrt1060_evk.dts:           compatible = "issi,is25wp064", "jedec,spi-nor";
boards/arm/nrf52840_pca10056/nrf52840_pca10056.dts:             compatible = "jedec,spi-nor";
boards/arm/particle_argon/dts/mesh_feather.dtsi:                compatible = "jedec,spi-nor";
boards/arm/particle_boron/dts/mesh_feather.dtsi:                compatible = "jedec,spi-nor";
boards/arm/particle_xenon/dts/mesh_feather.dtsi:                compatible = "jedec,spi-nor";
boards/riscv32/hifive1/hifive1.dts:             compatible = "issi,is25lp128", "jedec,spi-nor";
boards/riscv32/hifive1_revb/hifive1_revb.dts:           compatible = "issi,is25lp128", "jedec,spi-nor";
boards/riscv32/qemu_riscv32/qemu_riscv32.dts:           compatible = "issi,is25lp128", "jedec,spi-nor";
boards/x86/arduino_101/arduino_101.dts:         compatible = "winbond,w25q16", "jedec,spi-nor";
boards/xtensa/intel_s1000_crb/intel_s1000_crb.dts:              compatible = "jedec,spi-nor";
dts/arm/nxp/nxp_rt1064.dtsi:            compatible = "winbond,w25q32jvwj", "jedec,spi-nor";
dts/bindings/mtd/jedec,spi-nor.yaml:    constraint: "jedec,spi-nor"

This leads to failures in #17660, which turns missing category: required properties into an error.

I wonder if some dummy label string could be put in, even if it isn't used.

[ulfalizer]

CC @pabigot @galak

[pabigot]

I have always maintained that Zephyr misuses the label property, but I was told by @galak this was to be fixed when tooling was complete. Are we there yet?

All we really need is some handle that can be used in device_get_binding(DT_HANDLE_PATH) to get the device associated with PATH in the devicetree. Nobody should ever need to see the value of that handle outside of debugging.

Ideally it wouldn't be a string (a unique integer assigned when the binding set was generated would be adequate), but I don't expect that to change. Whatever it is, there's no reason the devicetree scripting can't synthesize a unique label when one is required but missing.

The scripting should be checking that label is unique across all nodes for which there are devices anyway, so it could synthesize a short unique string, or simply represent the path to the node (wasting ROM).

In any case, requiring that device tree nodes provide a user-supplied label property because we have no other way to identify the node is, in my opinion, bad design.

[pabigot]

I've recently been making properties optional when I can have the driver implementation provide a reasonable default, and required when the driver must be given a value. For jedec,spi-nor the size, erase-block-size, and write-block-size properties are required, but cannot be defaulted in the driver.

I think the devicetree tooling interferes in the case of flash-type devices because it synthesizes size-related properties from partition table data even if they don't appear explicitly. (I'm not sure about this.)

It is unclear to me whether category presence checks are before or after this synthesis.

Extending this existing practice to label, SPI (and other) devices must have a label so we can get their driver, but the tooling can synthesize one. So a proposal:

• Change the tooling so that if a node needs a label (e.g., having one defined in the yaml whether it's required or optional), but does not have one in the devicetree source, the scripting will synthesize one from <compatible>#<instance>. E.g. jedec,spi-nor#0.

If category presence is checked before synthesis, then just change spi-base.yaml to make label optional.

If that isn't viable then:

• Close this PR by adding label = "jedec,spi-nor#0"; to all the nodes that require it.

[ulfalizer]

@pabigot
Which quickfix do you prefer, and could you submit a PR for it?

Doesn't need to be high art at this point. Could add a comment explaining what's going on maybe. Trying to batch-fix all of those required-but-missing properties to be able to plug it for good.

Could improve the design later.