Re: [PATCH 1/5] dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: net: maxlinear: PHY incorrectly modeled as regulator provider instead of consumer
- [Medium] dt-bindings: net: maxlinear: missing 'vddp-supply' definition breaks dt-schema validation
- [Low] dt-bindings: net: maxlinear: schema property 'mdio-regulator' lacks top-level definition
--

commit 03684e9eca992ab6bfa5c8f894e3de06a76d36a7
Author: Robert Marko <robert.marko@sartura.hr>

dt-bindings: net: maxlinear: add MxL86211C MDIO I/O regulator
    
Document the MDIO I/O voltage regulator exposed by the MxL86211C PHY.
The regulator configures the MDIO, MDC, and MDINT pad voltage between 1.8 V
and 3.3 V.

> diff --git a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> index 8a3713abd1ca9..0645e885f1747 100644
> --- a/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> +++ b/Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml
> @@ -12,6 +12,29 @@ maintainers:
>  
>  allOf:
>    - $ref: ethernet-phy.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ethernet-phy-idc133.5400
> +      required:
> +        - compatible
> +
> +    then:
> +      properties:
> +        mdio-regulator:

[Severity: Low]
Is it standard practice to define properties exclusively inside the allOf
conditional branch?

Standard dt-schema convention requires all properties to be explicitly defined
in the top-level properties block, using conditional blocks only to apply
restrictions or to disallow the property for specific compatibles.

> +          type: object
> +          description: |
> +            Initial data for the MxL86211C MDIO I/O voltage regulator. Set
> +            this to 1.8 V or 3.3 V to configure the MDIO, MDC, and MDINT
> +            pad voltage. The board must supply the VDDP pin at the selected
> +            voltage.
> +          $ref: /schemas/regulator/regulator.yaml#

[Severity: Medium]
Does this misrepresent the physical hardware topology by modeling the PHY
as a regulator provider?

Since the documentation states "The board must supply the VDDP pin", the PHY
is a power consumer. In Device Tree, an external hardware supply must be
modeled as a consumer property (e.g., vddp-supply). Modeling this as an
mdio-regulator provider node referencing regulator.yaml appears to be
an architectural abuse of the framework.

Shouldn't the PHY driver read this supply via regulator_get_voltage() to
configure its registers instead of registering a dummy regulator provider?

[Severity: Medium]
Will valid device trees that accurately model the VDDP hardware supply fail
dt_binding_check validation?

The documentation explicitly mandates that the board must supply the VDDP pin,
meaning a correct hardware description requires a vddp-supply property.
However, the schema contains unevaluatedProperties: false but omits the
definition of vddp-supply.

Since neither this schema nor ethernet-phy.yaml implicitly allows vddp-supply,
won't this cause validation errors for users who correctly model the physical
hardware?

> +          unevaluatedProperties: false

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930122305.293686-1-robert.marko@sartura.hr?part=1