Re: [PATCH 2/4] dt-bindings: net: dsa: document internal MDIO bus

[Arınç ÜNAL]

On 13.08.2023 15:59, Arınç ÜNAL wrote:
> On 13.08.2023 14:53, Vladimir Oltean wrote:
> > On Sat, Aug 12, 2023 at 10:20:43PM +0300, Arınç ÜNAL wrote:
> > > diff --git a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
> > > index 4d5f5cc6d031e2..82dda8fae8b16e 100644
> > > --- a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
> > > +++ b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
> > > @@ -72,6 +72,10 @@ properties:
> > >             - compatible
> > >             - reg
> > > +  mdio:
> > > +    $ref: dsa.yaml#/properties/mdio
> > > +    unevaluatedProperties: false
> >
> > sja1105 does not support an "mdio" child property. I haven't checked the
> > others. Don't add properties that aren't supported.
>
> Adding the mdio property to the dsa.yaml schema will allow it on all of the
> schemas that refer to dsa.yaml such as this one. This addition here is only
> to disallow additional properties under the mdio property for this specific
> schema.
>
> That said, my understanding is that the internal MDIO bus exists on all of
> the switches controlled by DSA. Whether each individual DSA subdriver
> supports registering it does not matter in terms of documenting the
> internal MDIO bus for all DSA switches.

On top of this, I'd argue to document the internal MDIO bus on the
ethernet-switch.yaml schema instead.

Arınç

> SJA1110 uses the mdios property instead because it's got two internal mdio
> buses, which is why I invalidate the mdio property for it. If SJA1105 has
> also got two internal mdio buses, let me know.
>
> > > +
> > >   patternProperties:
> > >     "^(ethernet-)?ports$":
> > >       patternProperties:
> > >
> > > The nxp,sja1105.yaml schema also needed some changes.
> > >
> > > dt-bindings: net: dsa: nxp,sja1105: improve internal MDIO bus bindings
> > >
> > > SJA1110 Ethernet Switch uses the mdios property for its internal MDIO bus.
> > > Therefore, disallow the mdios property for SJA1105, and the mdio property
> > > for SJA1110.
> > >
> > > Require the phy-handle property on the non-CPU ports if the mdios property
> > > is being used.
> > >
> > > Refer to dsa.yaml#/properties/mdio to point the human readers to the
> > > description on the dsa.yaml schema.
> > >
> > > ---
> > >   .../bindings/net/dsa/nxp,sja1105.yaml         | 20 ++++++++++++++++++-
> > >   1 file changed, 19 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
> > > index 82dda8fae8b16e..7d92350f1065b2 100644
> > > --- a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
> > > +++ b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
> > > @@ -52,7 +52,7 @@ properties:
> > >       patternProperties:
> > >         "^mdio@[0-1]$":
> > > -        $ref: /schemas/net/mdio.yaml#
> > > +        $ref: dsa.yaml#/properties/mdio
> > >           unevaluatedProperties: false
> > >           properties:
> > > @@ -128,14 +128,32 @@ allOf:
> > >       then:
> > >         properties:
> > >           spi-cpol: false
> > > +        mdios: false
> > > +
> > >         required:
> > >           - spi-cpha
> > >       else:
> > >         properties:
> > >           spi-cpha: false
> > > +        mdio: false
> > > +
> > >         required:
> > >           - spi-cpol
> > > +  - if:
> > > +      required: [ mdios ]
> > > +    then:
> > > +      patternProperties:
> > > +        "^(ethernet-)?ports$":
> > > +          patternProperties:
> > > +            "^(ethernet-)?port@[0-9]+$":
> > > +              if:
> > > +                not:
> > > +                  required: [ ethernet ]
> > > +              then:
> > > +                required:
> > > +                  - phy-handle
> >
> > For sja1105, phylink-compatible bindings (phy-handle, fixed-link or managed)
> > are required for all ports (user, dsa or cpu).
> >
> > Also, sja1105 does not populate the slave_mii_bus, so it never uses the
> > fallback where ports implicitly connect to an internal PHY if no phylink
> > bindings are present.
>
> I'll handle these accordingly with your answer to my question above.
>
> Arınç