Re: [net-next v3 3/5] dt-bindings: net: starfive,jh7110-dwmac: Add jhb100 sgmii rx clk

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch erroneously relaxes clock constraints for the base `starfive,jh7110-dwmac` hardware, incorrectly allowing it to specify 6 clocks.
- [Low] Replacing the global `items` array for `clock-names` with duplicated conditional blocks is a binding anti-pattern.
--

commit 868beb354c5de56282a3dbb82e7d79b8745d272e
Author: Minda Chen <minda.chen@starfivetech.com>

dt-bindings: net: starfive,jh7110-dwmac: Add jhb100 sgmii rx clk

This commit updates the starfive,jh7110-dwmac binding to support the jhb100
SGMII interface. It adds a 6th clock for the new SGMII interface while
noting that existing RMII/RGMII interfaces still use 5 clocks.

> --- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> @@ -39,20 +39,18 @@ properties:
[ ... ]
>    clock-names:
> -    items:
> -      - const: stmmaceth
> -      - const: pclk
> -      - const: ptp_ref
> -      - const: tx
> -      - const: gtx
> +    minItems: 5
> +    maxItems: 6

This isn't a bug, but does it make sense to keep the full 6-element list
defined here at the top level and rely purely on minItems and maxItems
overrides inside the conditional blocks?

Standard device tree binding practice usually defines the complete items array
once at the top level. By removing the items array here and moving it into
the conditionals, the string constants are duplicated across multiple blocks.

[ ... ]
> @@ -111,6 +121,26 @@ allOf:
>            contains:
>              const: starfive,jh7110-dwmac
>      then:
> +      properties:
> +        clocks:
> +          minItems: 5
> +          maxItems: 6
> +
> +        clock-names:
> +          oneOf:
> +            - items:
> +                - const: stmmaceth
> +                - const: pclk
> +                - const: ptp_ref
> +                - const: tx
> +                - const: gtx
> +            - items:
> +                - const: stmmaceth
> +                - const: pclk
> +                - const: ptp_ref
> +                - const: tx
> +                - const: gtx
> +                - const: sgmii_rx

Does this inadvertently relax the clock constraints for the base
starfive,jh7110-dwmac hardware?

The commit message explicitly states that the 6th clock is a requirement
only for the new jhb100 SGMII interface, while other interfaces use 5 clocks.

Since jhb100 uses starfive,jh7110-dwmac as a fallback, this outer then
block applies to both hardware variants. Because there is no inner else
block for the pure starfive,jh7110-dwmac to restrict maxItems back to 5,
will a pure jh7110 device tree now be erroneously permitted to specify 6
clocks and silently pass schema validation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507094115.8355-1-minda.chen@starfivetech.com?part=3