drivers: flash_stm32_qspi: fix DT accessor for flash size

[arbrauns]

The flash size is the second part (size) of the first reg value, not the first part (address) of a nonexistent second reg value.

This was introduced in #68274, and I don't understand how it has ever worked. I'm getting the following errors:

$PRJDIR/build/zephyr/include/generated/zephyr/devicetree_generated.h:14745:39: error: 'DT_N_S_soc_S_quadspi_a0001000_S_flash_90000000_REG_IDX_1_VAL_ADDRESS' undeclared here (not in a function); did you mean 'DT_N_S_soc_S_quadspi_a0001000_S_flash_90000000_REG_IDX_0_VAL_ADDRESS'?
14745 | #define DT_N_INST_0_st_stm32_qspi_nor DT_N_S_soc_S_quadspi_a0001000_S_flash_90000000
      |                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
$PRJDIR/zephyr/include/zephyr/devicetree.h:4785:33: note: in definition of macro 'DT_CAT4'
 4785 | #define DT_CAT4(a1, a2, a3, a4) a1 ## a2 ## a3 ## a4
      |                                 ^~
$PRJDIR/zephyr/include/zephyr/devicetree.h:4125:44: note: in expansion of macro 'DT_REG_ADDR_BY_IDX'
 4125 | #define DT_INST_REG_ADDR_BY_IDX(inst, idx) DT_REG_ADDR_BY_IDX(DT_DRV_INST(inst), idx)
      |                                            ^~~~~~~~~~~~~~~~~~
$PRJDIR/zephyr/include/zephyr/sys/util_internal.h:105:36: note: in expansion of macro 'DT_N_INST_0_st_stm32_qspi_nor'
  105 | #define UTIL_PRIMITIVE_CAT(a, ...) a##__VA_ARGS__
      |                                    ^
$PRJDIR/zephyr/include/zephyr/sys/util_internal.h:104:26: note: in expansion of macro 'UTIL_PRIMITIVE_CAT'
  104 | #define UTIL_CAT(a, ...) UTIL_PRIMITIVE_CAT(a, __VA_ARGS__)
      |                          ^~~~~~~~~~~~~~~~~~
$PRJDIR/zephyr/include/zephyr/devicetree.h:336:31: note: in expansion of macro 'UTIL_CAT'
  336 | #define DT_INST(inst, compat) UTIL_CAT(DT_N_INST, DT_DASH(inst, compat))
      |                               ^~~~~~~~
$PRJDIR/zephyr/include/zephyr/devicetree.h:3604:27: note: in expansion of macro 'DT_INST'
 3604 | #define DT_DRV_INST(inst) DT_INST(inst, DT_DRV_COMPAT)
      |                           ^~~~~~~
$PRJDIR/zephyr/include/zephyr/devicetree.h:4125:63: note: in expansion of macro 'DT_DRV_INST'
 4125 | #define DT_INST_REG_ADDR_BY_IDX(inst, idx) DT_REG_ADDR_BY_IDX(DT_DRV_INST(inst), idx)
      |                                                               ^~~~~~~~~~~
$PRJDIR/zephyr/drivers/flash/flash_stm32_qspi.c:1567:23: note: in expansion of macro 'DT_INST_REG_ADDR_BY_IDX'
 1567 |         .flash_size = DT_INST_REG_ADDR_BY_IDX(0, 1),
      |                       ^~~~~~~~~~~~~~~~~~~~~~~

[arbrauns]

I'll have to investigate further, something's weird here. This fixes the issue for me locally on top of v3.7.0 with a couple patches that shouldn't make a difference, but causes problems in CI.

[arbrauns]

Oooh, it's because my board sets #size-cells = <1>; in the quadspi node, but the sample doesn't, which causes the size to be interpreted as the next address cell. What's the correct way here?

[erwango]

Oooh, it's because my board sets #size-cells = <1>; in the quadspi node, but the sample doesn't, which causes the size to be interpreted as the next address cell. What's the correct way here?

Could you elaborate (which board, which sample, ...) ?
Ideally it should be fixed in a way that minimizes the probability that someone else meet this issue again ;-). For instance fixing in board or even soc if only linked to hw and not to the application...

[arbrauns]

All boards/samples/etc using compatible = "st,stm32-qspi-nor";/compatible = "st,stm32-ospi-nor";, see the files changed in #68274 and #68533 for a good list.

They all have a reg property of the form reg = <0x70000000 DT_SIZE_M(64)>;, which to me suggests one address and one size cell, but their parent node doesn't have a #size-cells property, so it defaults to 0. Either the reg properties should be changed to reg = <0x70000000>, <DT_SIZE_M(64)>; to better reflect the reality of one address cell and no size cells, or (preferably) #size-cells = <1>; should be added to their parent nodes.

[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.

[arbrauns]

@erwango: would appreciate some guidance here

[erwango]

Agreed on the analysis.
I suggest to add:

  "#address-cells":
    required: true
    const: 1
  "#size-cells":
    required: true
    const: 1

in impacted compatibles bindings, and fix dts description. Note that this would likely require a note in migration guidelines as this may break out of tree boards definitions.

[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.

[mathieuchopstm]

Driver modification LGTM, but as suggested by @erwango, the modification should be made at SoC DTSI level instead, e.g., here for STM32H7:

zephyr/dts/arm/st/h7/stm32h7.dtsi

Lines 1068 to 1076 in 561ee12

	quadspi: quadspi@52005000 {
	compatible = "st,stm32-qspi";
	#address-cells = <0x1>;
	#size-cells = <0x0>;
	reg = <0x52005000 0x34>;
	interrupts = <92 0>;
	clocks = <&rcc STM32_CLOCK(AHB3, 14U)>;
	status = "disabled";
	};

This should fix the issue without needing to modify any board's DTS file.

[erwango]

@arbrauns I think we're there.

Note that it should also be fixed for drivers/flash/flash_stm32_xspi.c, bu tthen there would be a conflict with #85561 and potentially #85564.

So I propose that we keep these separate.
@gautierg-st Would you mind checking this on flash_stm32_xspi once above mentionned PRs are merged ?