Re: [PATCH v6 1/2] dt-bindings: arm: stm32: Document st,stm32mp23/25-syscfg subnodes and cells

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `clocks: false` restriction in the `else` block incorrectly forbids the `clocks` property for `st,stm32-syscfg`, breaking validation of existing in-tree device trees.
- [High] Making `phy@2400`, `phy@2800`, `#address-cells`, and `#size-cells` unconditionally `required` contradicts their optional nature and breaks existing device trees.
- [Medium] The patternProperties entry contains a $ref to a schema file (`st,stm32-usb2phy.yaml`) that does not exist in the repository.
--

commit ddc0637029fe22061a9355ce24276a2cff5c6682
Author: Marek Vasut <marex@nabladev.com>

dt-bindings: arm: stm32: Document st,stm32mp23/25-syscfg subnodes and cells

The st,stm32mp23-syscfg and st,stm32mp25-syscfg can have subnodes, which
represent the USB 2.0 FEMTO-PHY. Document subnodes pattern properties and
address/size cells.

> diff --git a/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml b/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
> index 95d2319afe235..a559f8b03b842 100644
> --- a/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
> +++ b/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
> @@ -39,6 +39,17 @@ properties:

[ ... ]

> +patternProperties:
> +  "^phy@2[48]00$":
> +    type: object
> +    $ref: /schemas/phy/st,stm32-usb2phy.yaml

[Severity: Medium]
Does this reference target a schema file (st,stm32-usb2phy.yaml) that is
missing from the repository? Schema validation tooling might fail to resolve
this reference.

>  required:
>    - compatible
>    - reg
> @@ -54,13 +65,30 @@ allOf:
>      then:
>        required:
>          - clocks
> +    else:
> +      properties:
> +        clocks: false

[Severity: High]
Does this block incorrectly forbid the clocks property for other compatibles,
breaking validation of existing in-tree device trees?

For example, platforms like STM32F429 use the st,stm32-syscfg compatible and
define clocks:

arch/arm/boot/dts/st/stm32f429.dtsi:syscfg {
    ...
    clocks = <&rcc 0 STM32F4_APB2_CLOCK(SYSCFG)>;
    ...
}

Because st,stm32-syscfg does not match the preceding if condition, it falls
into this else block, which will cause dtbs_check to fail.

>    - if:
>        properties:
>          compatible:
> -          const: st,stm32mp25-syscfg
> +          contains:
> +            enum:
> +              - st,stm32mp23-syscfg
> +              - st,stm32mp25-syscfg
>      then:
>        required:
> +        - "#address-cells"
>          - "#clock-cells"
> +        - "#size-cells"
> +        - phy@2400
> +        - phy@2800

[Severity: High]
Does making these properties unconditionally required contradict their optional
nature implied by "can have subnodes" in the commit message?

Existing devicetrees without these subnodes will fail validation. For
example, in arch/arm64/boot/dts/st/stm32mp251.dtsi:

syscfg: syscon@44230000 {
	compatible = "st,stm32mp25-syscfg", "syscon";
	reg = <0x44230000 0x10000>;
	#clock-cells = <0>;
};

This node will immediately fail dtbs_check because it lacks the newly
mandated phy@2400, phy@2800, #address-cells, and #size-cells properties.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902124018.606544-1-marex@nabladev.com?part=1