Re: [PATCH net-next v15 2/9] dt-bindings: phy: document the serdes PHY on sa8255p

Re: [PATCH net-next v15 2/9] dt-bindings: phy: document the serdes PHY on sa8255p

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In the new binding
  Documentation/devicetree/bindings/phy/qcom,sa8255p-dwmac-sgmii-…

--

dt-bindings: phy: document the serdes PHY on sa8255p

This commit adds a new binding for the SGMII/SerDes PHY on Qualcomm sa8255p
platforms. The hardware is the same as sa8775p rev3, but here firmware
manages the PHY over SCMI.

> diff --git a/Documentation/devicetree/bindings/phy/qcom,sa8255p-dwmac-sgmii-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sa8255p-dwmac-sgmii-phy.yaml
> new file mode 100644
> index 0000000000000..9e846e3815555
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,sa8255p-dwmac-sgmii-phy.yaml

[ ... ]

> +  power-domains:
> +    maxItems: 1
> +
> +  power-domain-names:
> +    maxItems: 1

[Severity: Low]
This isn't a bug, but should power-domain-names pin its value here?

The property is listed under required, and the example uses "serdes".
However, the only constraint is maxItems: 1. A node with
power-domain-names = "foo" would still pass dt_binding_check, so the value
of a required property is left undefined in the ABI.

The other firmware-managed sa8255p bindings fix their names with const
entries. For example, qcom,sa8255p-geni-i2c.yaml has:

  power-domain-names:
    items:
      - const: power
      - const: perf

qcom,sa8255p-ethqos.yaml, added later in this series, does the same with
core and mdio.

power-domains also has no description, so the binding doesn't say what the
single domain represents.

Could this use items: - const: serdes for power-domain-names, and add an
items: - description: entry for power-domains? Another option is to drop
power-domain-names, since there is only one entry.

The driver added later in the series (phy-qcom-sgmii-eth-scmi.c) doesn't
look the domain up by name. The driver core attaches the single domain
automatically, so an unexpected name causes no runtime failure. The concern
is limited to the schema and the ABI definition.

> +
> +  "#phy-cells":
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +  - power-domains
> +  - power-domain-names

[ ... ]

> +        power-domains = <&scmi7_dvfs 0>;
> +        power-domain-names = "serdes";
> +    };

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923-qcom-sa8255p-emac-v15-0-e82f33720737%40oss.qualcomm.com