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

[Chris Packham <Chris.Packham@xxxxxxxxxxxxxxxxxxx>]

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@5c030000 {
>          #address-cells = <1>;
>          #size-cells = <0>;
>
>          ethphy0: ethernet-phy@1 {
>              reg = <1>;
>          };
>
>          ethphy1: ethernet-phy@3 {
>              reg = <3>;
>          };
>      };
>
>      mdio@5c040000 {
>          #address-cells = <1>;
>          #size-cells = <0>;
>
>          ethphy2: ethernet-phy@1 {
>              reg = <1>;
>          };
>
>          ethphy3: ethernet-phy@3 {
>              reg = <3>;
>          };
>      };
>
>      mdio@5c050000 {
>         ...
>      }
>
>      mdio@5c060000 {
>         ...
>      }
>
>      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@ca00 {
                         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.