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

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Sergey Shtylyov" <s.shtylyov@omp.ru>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Geert Uytterhoeven" <geert+renesas@glider.be>,
	netdev@vger.kernel.org, devicetree@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH] dt-bindings: net: renesas,etheravb: Add MDIO bus reset properties
Date: Sat, 9 Mar 2024 12:45:06 +0100	[thread overview]
Message-ID: <1375d840-6e42-4e60-896c-265fe6a21705@linaro.org> (raw)
In-Reply-To: <20240309012538.719518-1-niklas.soderlund+renesas@ragnatech.se>

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.

Plus, where is the user of all this?

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

Best regards,
Krzysztof