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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sat, 9 Mar 2024 12:45:06 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Niklas Söderlund
 <niklas.soderlund+renesas@...natech.se>, Sergey Shtylyov
 <s.shtylyov@....ru>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Geert Uytterhoeven <geert+renesas@...der.be>, netdev@...r.kernel.org,
 devicetree@...r.kernel.org
Cc: linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: net: renesas,etheravb: Add MDIO bus reset
 properties

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@...00000 {
>             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@...00000 {
>             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