Re: [PATCH] dt-bindings: net: renesas,etheravb: Add MDIO bus reset properties

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Krzysztof,

Thanks for your reply.

On 2024-03-09 12:45:06 +0100, Krzysztof Kozlowski wrote:
> On 09/03/2024 02:25, Niklas Söderlund wrote:
> > The bindings for Renesas Ethernet AVB are from 2015 and contain some
> > oddities that are impossible to get right without breaking existing
> > bindings. One such thing is that the MDIO bus properties that should be
> > its own node are mixed with the node for the IP for Ethernet AVB.
> > 
> > Instead of a separate node for the MDIO bus,
> > 
> >     avb: ethernet@e6800000 {
> >             compatible = "renesas,etheravb-r8a7795",
> >                          "renesas,etheravb-rcar-gen3";
> >             reg = <0xe6800000 0x800>, <0xe6a00000 0x10000>;
> > 
> >             ...
> > 
> >             phy-handle = <&phy0>;
> > 
> >             mdio {
> >                 #address-cells = <1>;
> >                 #size-cells = <0>;
> > 
> >                 phy0: ethernet-phy@0 {
> >                     ...
> >                 };
> >             };
> >     };
> > 
> > The Ethernet AVB mix it in one,
> > 
> >     avb: ethernet@e6800000 {
> >             compatible = "renesas,etheravb-r8a7795",
> >                          "renesas,etheravb-rcar-gen3";
> >             reg = <0xe6800000 0x800>, <0xe6a00000 0x10000>;
> > 
> >             ...
> > 
> >             phy-handle = <&phy0>;
> > 
> >             #address-cells = <1>;
> >             #size-cells = <0>;
> > 
> >             phy0: ethernet-phy@0 {
> >                 ...
> >             };
> >     };
> > 
> > This forces to all MDIO bus properties needed to be described in the
> > Ethernet AVB bindings directly. However not all MDIO bus properties are
> > described as they were not needed. This change adds the MDIO bus
> > properties to reset the MDIO bus in preparation for them being used.
> 
> That is not exactly what you wrote in the binding. Binding suggests you
> have per device GPIO, which in your design is shared but the commit msg
> says it is some sort of bus reset.
> 
> These are two different things, because in first case you could have a
> design using two GPIOs, not one. Then your binding is completely wrong.

I copied the description from mdio.yaml as the intention is the two are 
the same. But on a second read I agree it's confusing.

> 
> Plus, where is the user of all this?

I think you found it in, for posterity it's in '[PATCH] arm64: dts: 
renesas: white-hawk: ethernet: Describe adv1 and avb2'.

> 
> I think you should rather correct the binding to use mdio node and add
> appropriate handling in the driver, keeping backward compatibility.

I had not consider that, the driver can indeed support both modes and 
only allow MDIO bus properties if the mdio sub-node is present. I will 
do so, thanks for the suggestion.

-- 
Kind Regards,
Niklas Söderlund