Re: [PATCH net-next v11 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings

On Thu, 2026-08-06 at 14:38 +0000, [email protected] wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The schema lacks an `mdio` node definition while setting `unevaluatedProperties: false` at the root, which inadvertently prevents instantiation of the integrated PHYs referenced in the example.
> - [Medium] Invalid constraint and usage of RGMII internal delay properties in combination with `phy-mode = "rgmii"`.
> --
> 
> --- Patch [2]: [PATCH net-next v11 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings ---
> commit 36eb3256c1e8bdeb280de04a105905650e50438f
> Author: Jens Emil Schulz Østergaard <[email protected]>
> 
> dt-bindings: net: lan9645x: add LAN9645X switch bindings
> 
> Add bindings for LAN9645X switch. We use a fallback compatible for the
> smallest SKU microchip,lan96455s-switch.
> 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml
> > new file mode 100644
> > index 0000000000000..043fb48922b41
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml
> > @@ -0,0 +1,111 @@
> [ ... ]
> > +        properties:
> > +          rx-internal-delay-ps:
> > +            const: 2000
> > +
> > +          tx-internal-delay-ps:
> > +            const: 2000
> 
> [Severity: Medium]
> Is it intended to restrict the rx-internal-delay-ps and tx-internal-delay-ps
> properties to exactly 2000?

Yes, these are the only delay values supported by the mac.

> 
> [ ... ]
> > +required:
> > +  - compatible
> > +  - reg
> > +  - ethernet-ports
> > +
> > +unevaluatedProperties: false
> 
> [Severity: Medium]
> Since the switch has integrated PHYs, will users need to define them in an
> mdio node to correctly resolve the phy-handle references?
> 
> If so, won't this unevaluatedProperties: false at the root level reject the
> device tree, as the mdio node is not defined in the properties block or
> inherited from dsa.yaml?

No, any peripherals such as MDIO controllers are meant to be child nodes of
an MFD device, as siblings of this switch node. The MDIO bus devices will
not be subnodes of this switch node, so they are not meant to be accomodated
by this schema.

> 
> [ ... ]
> > +            ethernet-port@7 {
> > +                reg = <7>;
> > +                phy-mode = "rgmii";
> > +                ethernet = <&cpu_host_port>;
> > +                rx-internal-delay-ps = <2000>;
> > +                tx-internal-delay-ps = <2000>;
> 
> [Severity: Medium]
> Can this combination of phy-mode = "rgmii" and a 2000ps internal delay be used
> here?
> 
> According to the ethernet controller bindings, phy-mode = "rgmii" implies
> the delay is implemented on the PCB. Adding a 2000ps internal MAC delay on
> top of this might result in a double delay.
> 
> If the hardware has fixed internal delays, should this use
> phy-mode = "rgmii-id" instead?

No, the intention here is to describe the situation where the CPU port and the
conduit are connected mac-to-mac. Documentation/networking/phy.rst:


* PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting any
  internal delay by itself, it assumes that either the Ethernet MAC (if capable)
  or the PCB traces insert the correct 1.5-2ns delay

RGMII_ID means the PHY should insert delays on both rx and tx, which would be
incorrect here.

> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260805-dsa_lan9645x_switch_driver_base-v11-0-007ebc983a0a@microchip.com?part=2