Re: [PATCH net-next v3 1/5] dt-bindings: net: maxlinear: add MxL86211C VDDP supply

[netdev-bot+sashiko@kernel.org]

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] At this commit, the binding adds `vddp-supply`
  (Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml,…
- [Low] Before this patch, maxlinear,gpy2xx.yaml had no `select`, no
  `properties/compatible` and no `$nodename`.

--

dt-bindings: net: maxlinear: add MxL86211C VDDP supply

This commit adds an optional vddp-supply property to the MaxLinear GPY2xx
PHY binding and limits it to the MxL86211C (ethernet-phy-idc133.5400). It
also adds a select statement, so the schema now applies to nodes with that
PHY ID or with the maxlinear,use-broken-interrupts property. A second
example showing the supply is added too.

> diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> index 8a3713abd1ca..53f430e12151 100644
> --- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> @@ -10,10 +10,37 @@ maintainers:
>    - Andrew Lunn <andrew@lunn.ch>
>    - Michael Walle <michael@walle.cc>
>  
> +select:
> +  anyOf:
> +    - properties:
> +        compatible:
> +          contains:
> +            const: ethernet-phy-idc133.5400
> +      required:
> +        - compatible
> +    - required:
> +        - maxlinear,use-broken-interrupts

[Severity: Low]
Will this select branch make the first example in this file fail
dt_binding_check?

Before this patch the schema had no select, no compatible and no
$nodename. dtschema therefore generated a select that matched nothing, and
the existing dependency was never checked:

dependencies:
  maxlinear,use-broken-interrupts: [ interrupts ]

With this branch, every node that has maxlinear,use-broken-interrupts is
now selected. The first example has that property, but it uses
interrupts-extended instead of interrupts:

        ethernet-phy@0 {
            reg = <0>;
            interrupts-extended = <&intc 0>;
            maxlinear,use-broken-interrupts;
        };

If the dtschema interrupts fixup only rewrites properties and required, and
not dependencies, wouldn't this report interrupts as a missing dependency
of maxlinear,use-broken-interrupts? Real DTs that wire the GPY2xx IRQ with
interrupts-extended would get the same dtbs_check error.

The example and the dependency are unchanged at the end of the series.
Could the dependency accept either interrupts or interrupts-extended (for
example a oneOf of required: [interrupts] and
required: [interrupts-extended])? Or could the example use interrupts?

> +
>  allOf:

[ ... ]

>  properties:
> +  vddp-supply:
> +    description: |
> +      Optional regulator supplying the VDDP pin. Its voltage configures the
> +      MDIO, MDC, and MDINT pad voltage and must be either 1.8 V or 3.3 V.
> +

[Severity: Low]
Does anything use vddp-supply at this commit? The MxL86211C entry in
gpy_drivers[] in drivers/net/phy/mxl-gpy.c still uses gpy_probe() and
gpy21x_config_init(). Neither of them gets or enables a regulator.

Suppose a DT points vddp-supply at a switchable regulator that lacks
regulator-always-on. Could the rail stay off, or be turned off by the
unused-regulator cleanup, leaving the MDIO, MDC and MDINT pads without
power?

The next patch in the series, "net: phy: mxl-gpy: add MxL86211C VDDP
supply", fixes this. It adds mxl86211c_probe(), which calls
devm_regulator_get_optional(dev, "vddp") and regulator_enable(). It also
adds mxl86211c_config_init(), which reapplies the pad voltage after reset.
So only a kernel built at exactly this commit is affected.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020093655.344585-1-robert.marko%40sartura.hr