linux-kernel - Re: [PATCH v4 1/4] dt-bindings: net: Add Realtek MDIO controller

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] [day] [month] [year] [list]
Message-ID: <b03b5d67-2df6-40da-adea-2339445797b2@alliedtelesis.co.nz>
Date: Thu, 23 Jan 2025 23:40:30 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: Andrew Lunn <andrew@...n.ch>
CC: Krzysztof Kozlowski <krzk@...nel.org>, "lee@...nel.org" <lee@...nel.org>,
	"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"tsbogend@...ha.franken.de" <tsbogend@...ha.franken.de>,
	"hkallweit1@...il.com" <hkallweit1@...il.com>, "linux@...linux.org.uk"
	<linux@...linux.org.uk>, "sander@...nheule.net" <sander@...nheule.net>,
	"markus.stockhausen@....de" <markus.stockhausen@....de>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>
Subject: Re: [PATCH v4 1/4] dt-bindings: net: Add Realtek MDIO controller


On 24/01/2025 11:08, Andrew Lunn wrote:
>>>> +        properties:
>>>> +          realtek,port:
>>>> +            $ref: /schemas/types.yaml#/definitions/uint32
>>>> +            description:
>>>> +              The MDIO communication on the RTL9300 is abstracted by the switch. At
>>>> +              the software level communication uses the switch port to address the
>>>> +              PHY with the actual MDIO bus and address having been setup via the
>>>> +              parent mdio-bus and reg property.
>>> I don't quite get why this cannot be the 'reg' property. I understood that
>>> 'reg' of this node is not really used? Or you meant here this 'reg', not
>>> parent's 'reg'?
>> It's is a bit confusing (any suggestions for improving the description
>> and/or commit message are welcome).
> I don't know if it will actually help, but....
>
> We have two entangled configurations here.
>
> 1) You have 4 MDIO busses which you need to describe using mdio.yaml
>     In this binding, reg is the address of the device on the bus, in
>     the range 0-31.
>
> 2) The hardware was a pool of PHYs which you can map to address on the
>     MDIO busses.
>
> Rather than combining them, maybe it would be better to keep them
> separate. It is probably more error prone, but simpler to
> understand. And hopefully errors result in PHYs not being found during
> probe, so the problems are obvious.
>
> Maybe you can actually use phandles. You have the usual MDIO bus
> nodes:
>
>      mdio@...30000 {
>          #address-cells = <1>;
>          #size-cells = <0>;
>
>          ethphy0: ethernet-phy@1 {
>              reg = <1>;
>          };
>
>          ethphy1: ethernet-phy@3 {
>              reg = <3>;
>          };
>      };
>
>      mdio@...40000 {
>          #address-cells = <1>;
>          #size-cells = <0>;
>
>          ethphy2: ethernet-phy@1 {
>              reg = <1>;
>          };
>
>          ethphy3: ethernet-phy@3 {
>              reg = <3>;
>          };
>      };
>
>      mdio@...50000 {
>         ...
>      }
>
>      mdio@...60000 {
>         ...
>      }
>
>      And then a node which is a list of PHY phandles:
>
>      [&ethphy0, &ethphy1, &ethphy2, &ethphy3, ....]
>
> The 0th entry in the list tells you have to map the 0th PHY in the
> pool to an MDIO bus and address. Follow the phandle to get the MDIO
> bus and the address on the bus.

A fuller dts would be something like this (for the 8-port board I have 
in front of me)

                 mdio-controller@...0 {
                         compatible = "realtek,rtl9301-mdio";
                         reg = <0xca00>;

                         mdio-bus@0 {
                                 reg = <0x00>;
                                 ethphy0: ethernet-phy@0 {
                                         reg = <0x00>;
                                         compatible = 
"ethernet-phy-ieee802.3-c45";
                                 };
                                 ethphy1: ethernet-phy@1 {
                                         reg = <0x01>;
                                         compatible = 
"ethernet-phy-ieee802.3-c45";
                                 };
                                 ethphy2: ethernet-phy@2 {
                                         reg = <0x02>;
                                         compatible = 
"ethernet-phy-ieee802.3-c45";
                                 };
                                 ethphy3: ethernet-phy@3 {
                                         reg = <0x03>;
                                         compatible = 
"ethernet-phy-ieee802.3-c45";
                                 };
                         };
                         mdio-bus@1 {
                                 reg = <0x01>;
                                 ethphy4: ethernet-phy@8 {
                                         reg = <0x00>;
                                         compatible = 
"ethernet-phy-ieee802.3-c45";
                                 };
                                 ethphy5: ethernet-phy@9 {
                                         reg = <0x01>;
                                         compatible = 
"ethernet-phy-ieee802.3-c45";
                                 };
                                 ethphy6: ethernet-phy@10 {
                                         reg = <0x02>;
                                         compatible = 
"ethernet-phy-ieee802.3-c45";
                                 };
                                 ethphy7: ethernet-phy@11 {
                                         reg = <0x03>;
                                         compatible = 
"ethernet-phy-ieee802.3-c45";
                                 };
                         };
                         realtek,ports = [&ethphy0, &ethphy1, &ethphy2, 
&ethphy3,
                                                    /* need a gap here 
as there are 4 unused ports*/
                                                    0, 0, 0, 0,
                                                    &ethphy4, &ethphy5, 
&ethphy6, &ethphy7];
                 };

I could probably make it work but I'm not sure that it is any more 
understandable.