Re: [RFC PATCH net-next 1/7] dt-bindings: net: add Airoha EN8811H PHY MCU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The schema disables `reset-gpios` on the PHY node to prevent firmware erasure but fails to disable the `resets` and `reset-names` properties.
- [Medium] The strict `unevaluatedProperties: false` on the child PHY node rejects valid EN8811H-specific PHY properties.
--

commit 6002751a738ff01594839da605cd448738f78458
Author: Aleksei Sviridkin <f@lex.la>
Subject: dt-bindings: net: add Airoha EN8811H PHY MCU

This commit adds a device tree binding for the Airoha EN8811H PHY's MD32
MCU. It describes the MCU as an MDIO device that owns the reset line,
with the PHY function operating on a child bus, ensuring the PHY is only
visible when firmware is running.

> diff --git a/Documentation/devicetree/bindings/net/airoha,en8811h-mcu.yaml b/Documentation/devicetree/bindings/net/airoha,en8811h-mcu.yaml
> new file mode 100644
> index 0000000000000..7be0ab8622661
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/airoha,en8811h-mcu.yaml
[ ... ]
> +  mdio:
> +    $ref: mdio.yaml#
> +    description:
> +      The bus the PHY function answers on while firmware is running, at
> +      the same address this node occupies on the parent bus. Only that
> +      one address is passed through.
> +
> +    patternProperties:
> +      '^ethernet-phy@[0-9a-f]+$':
> +        type: object
> +        $ref: ethernet-phy.yaml#
> +        unevaluatedProperties: false

[Severity: Medium]
Does this strict evaluation constraint inadvertently block valid PHY-specific
properties?

By restricting the properties exclusively to those evaluated in
ethernet-phy.yaml#, the schema rejects specific properties defined in the
Airoha EN8811H PHY schema, such as rx-polarity from phy-common-props.yaml
or airoha,pnswap-rx.

Could this cause dt_binding_check to fail when a valid device tree includes
these EN8811H-specific PHY properties on the child node?

> +
> +        properties:
> +          reset-gpios: false
> +          reset-assert-us: false
> +          reset-deassert-us: false

[Severity: Medium]
The schema disables reset-gpios to prevent firmware erasure on the PHY
node, but does it also need to disable the standard resets and reset-names
properties?

If a device tree configures a hardware reset controller for the child
ethernet-phy node using the standard resets property, it will silently pass
validation. At runtime, the PHY framework might pulse this reset during
detachment, wiping the MCU's volatile firmware.

Should resets: false and reset-names: false be added here to fully close
this validation loophole?

> +
> +        description:
> +          The reset line belongs to the MCU node above. A reset described
> +          here would be pulsed by this node's own lifecycle, and that erases
> +          the firmware that created this PHY.
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788711797.git.f@lex.la?part=1