Re: [PATCH net-next v4 1/4] dt-bindings: net: dp83867: add compatible and reference ethernet-phy.yaml

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] This patch adds `select: properties: compatible: contains:
  const: ethernet-phy-id2000.a231` to…

--

dt-bindings: net: dp83867: add compatible and reference ethernet-phy.yaml

This patch adds a compatible for the DP83867 PHY ID, with an optional
ethernet-phy-ieee802.3-c22 fallback. It also adds a select on the PHY ID,
so dtschema now applies the binding to nodes. The $ref changes from
ethernet-controller.yaml to ethernet-phy.yaml.

> diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.yaml b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> index 4bc1f98fd9fe5..2cd8311ece5e5 100644
> --- a/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> @@ -8,7 +8,16 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
>  title: TI DP83867 ethernet PHY
>  
>  allOf:
> -  - $ref: ethernet-controller.yaml#
> +  - $ref: ethernet-phy.yaml#
> +
> +# only the PHY ID, not every PHY with the c22 fallback
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        const: ethernet-phy-id2000.a231
> +  required:
> +    - compatible

[Severity: Medium]
With this select, the schema now applies to every in-tree node that has
"ethernet-phy-id2000.a231". The binding still ends with:

    unevaluatedProperties: false

Several Xilinx ZynqMP devicetrees set #phy-cells on their DP83867 nodes.
For example, arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts has:

    phy0: ethernet-phy@21 {
        #phy-cells = <1>;
        compatible = "ethernet-phy-id2000.a231";

The same pattern is in zynqmp-zcu102-revB.dts, zynqmp-zcu104-revA.dts,
zynqmp-zcu104-revC.dts, zynqmp-zcu106-revA.dts, zynqmp-zcu111-revA.dts,
zynqmp-sck-kv-g-revA.dtso and zynqmp-sck-kv-g-revB.dtso. It is also in
zynqmp-sck-kr-g-revA.dtso and zynqmp-sck-kr-g-revB.dtso, with two PHYs
in each.

#phy-cells is not defined in ti,dp83867.yaml or in ethernet-phy.yaml. With
this patch, will make dtbs_check report "Unevaluated properties are not
allowed ('#phy-cells' was unexpected)" for all 12 of these nodes?

The commit message says:

    Reference
    ethernet-phy.yaml instead, which also describes reset-gpios and the
    other generic PHY properties existing devicetrees use.

That doesn't seem to hold for #phy-cells.

The later patches in the series keep the select and
unevaluatedProperties: false. They don't change these DTS files either, so
the mismatch is still there at the end of the series.

Should the series either drop #phy-cells from the Xilinx devicetrees, or
allow it in the schema? It looks bogus on an MDIO PHY that isn't a generic
PHY provider.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-v7-2-topic-dp83867-v4-0-a268f83b1ec1%40pengutronix.de