dts: arm: st: add npgios to all stm32 gpio controllers

[mrrosen]

For almost all STM32 GPIO controllers, the number of supported GPIO pins managed by a single controller is 16 (with some exceptions for fewer). However, the default for ngpios in the device tree bindings for gpio-controllers is 32; leading to inaccuracies in handling GPIO for these controllers, such as presenting too many GPIOs in the GPIO shell. This patch adds the ngpios property for all currently supported STM32 GPIO controllers.

[gautierg-st]

Wouldn't it be better to override the default value of 32 and set it to 16 for STM32? (If it's possible to override a default?)
We would avoid repeating the same line in practically all gpio nodes.

[mrrosen]

Wouldn't it be better to override the default value of 32 and set it to 16 for STM32? (If it's possible to override a default?)
We would avoid repeating the same line in practically all gpio nodes.

I agree but it didn't seem like it was possible to override defaults in the devicetree bindings and the default comes from gpio-controller.

And there is some interesting discussion as to whether or not default should be used in this way: #25164

It's a bit unclear to me how this should end up as on the one hand, it's possible the STM33 driver just has this set somehow but on the other, things close to the driver but not exactly the driver (or, the gpio shell) needs this information as well to enumerate the right number of pins for each controller.

So while what I ended up doing here isn't wrong in the end, I do agree it's rather verbose when almost all STM32 GPIO controllers support 16 pins.

[FRASTM]

could this be grouped in one change on st,stm32-gpio.yaml, like the following:

include:
  - name: base.yaml
  - name: gpio-controller.yaml
    property-blocklist:
      - ngpios

properties:
  ngpios:
    type: int
    default: 16

[mrrosen]

could this be grouped in one change on st,stm32-gpio.yaml, like the following:

include:
  - name: base.yaml
  - name: gpio-controller.yaml
    property-blocklist:
      - ngpios

properties:
  ngpios:
    type: int
    default: 16

That's an interesting way of doing it I didn't think of; but it does have the down side that fields (like the help text) are either lost for this binding or have to kept in sync with gpio-controller.yaml, right?

I do also wonder why then we wouldn't do this with all properties that are common then (like gpio-controller just defaulted true or #gpip-cells defaulted to 2 since those are duplicated for all gpio nodes too). I think it gets back to the discussion on that issue I linked where it didn't seem like it was desired to use the bindings as a replacement for actually setting values in the device tree but more just documenting things; and I'm not sure if Zephyr in general has changed it's mind on that since that thread is rather old.

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

[erwango]

Thanks @mrrosen. Sorry for not reviewing this earlier but I preferred to avoid tree wide changes before v3.7.0

I'm fine with updating ngpios for stm32, but then what about fixing the number in the stm32 compatible as done here for instance:

zephyr/dts/bindings/gpio/nxp,pcal6416a.yaml

Lines 10 to 13 in e79ae38

	properties:
	ngpios:
	required: true
	const: 16

[erwango]

I do also wonder why then we wouldn't do this with all properties that are common then (like gpio-controller just defaulted true or #gpip-cells defaulted to 2 since those are duplicated for all gpio nodes too).

Because ngpio is already provided with a default value in gpio-controller.yaml which doesn't match STM32 value. Fixing it the same method feels as the reasonable approach.

[erwango]

Please use default method.

[mrrosen]

@erwango @FRASTM @gautierg-st

Ive changed the PR to align to the review discussion. Further reviewing the discussion in some of the places I linked; its does seems like this is more the Zephyr method to handling devicetree defaults (I would say that alot more properties can be defaulted as a result but those can be handled as they arise/need updating).

I would say however that while I dont really mind the bindings as a source of values for properties, I think its the fact that Zephyr's build doesnt seem to produce a final DTS with all the defaulted properties in place is rather unfortunate -- part of the idea using devicetree I felt was to let other external tools possible use it and with defaults used as they are here, that information is lost as no dts file ever actually has them. Or I am missing it, but I didnt see it in zephyr/zephyr.dts as I hoped I would...

[erwango]

I think its the fact that Zephyr's build doesnt seem to produce a final DTS with all the defaulted properties in place is rather unfortunate -- part of the idea using devicetree I felt was to let other external tools possible use it and with defaults used as they are here, that information is lost as no dts file ever actually has them

That's indeed a relevant point, which however targets zephyr policy around defaults as a whole. Don't hesitate to raise an issue if you'd like to get this discussion to a wider audience.

[gautierg-st]

It doesn't seem necessary to do that. See the example provided by @erwango in nxp,pcal6416a.yaml, I don't see a property-blocklist (and it presumably works).

[mrrosen]

Since that used const/required, it basically just added a const to the property binding. However, if there was a tree that used it (I actually dont see this driver in any board/shield), it would still need to be in the tree; its just anything other than ngpios = <8>; would be an error (missing it would be a violation of required and any other value a violation of const: 8)

[mrrosen]

Thanks @mrrosen. Sorry for not reviewing this earlier but I preferred to avoid tree wide changes before v3.7.0

I'm fine with updating ngpios for stm32, but then what about fixing the number in the stm32 compatible as done here for instance:

zephyr/dts/bindings/gpio/nxp,pcal6416a.yaml

Lines 10 to 13 in e79ae38

	properties:
	ngpios:
	required: true
	const: 16

The const solution doesnt really work for a few reasons from my quick tests:

1. Its typically paired with required and if so, it must appear in the tree. If the goal is to not define it tree but rely on the value from the bindings, this doesnt work
2. Just defining const without required does not include the value in the tree, nor should it from the description of that key: https://docs.zephyrproject.org/latest/build/dts/bindings-syntax.html#const. Unlike default, const is more of just a constraint than a definition.
3. While very rare, there are STM32 GPIO controllers that do not implement all 16 IOs, at least as far as their documentation describes; so its better if for those exceptions, the value can be changed to the correct one for which defining by const does not allow

And I cannot define a default without blocking the property as it leads to an error processing the bindings (I did try just to see and it does not work). I think what was done here is the best use of a binding-defined value which avoids needing to define it in most trees as thats what everyone here was looking for.