Re: [PATCH v5 2/4] dt-bindings: mfd: Add MDIO interface to rtl9301-switch

[Chris Packham]

On 31/01/2025 19:35, Daniel Golle wrote:
> Hi Chris,
>
> afaik net-next is still closed right now, but lets discuss the series as RFC
> in the meantime maybe, right?

Yes sure. I probably should have tagged these as net-next even with or 
without RFC.

> On Fri, Jan 31, 2025 at 02:01:49PM +1300, Chris Packham wrote:
>> The MDIO controller is part of the switch on the RTL9300 family of
>> devices. Add a $ref to the mfd binding for these devices.
>>
>> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
>> ---
>>
>> Notes:
>>      This patch is dependent on "dt-bindings: net: Add Realtek MDIO
>>      controller" which adds the realtek,rtl9301-mdio.yaml binding.
>>      
>>      Changes in v5:
>>      - Note dependency on realtek,rtl9301-mdio.yaml patch
>>      - Add back reg property to the mdio-controller node.
>>      Changes in v4:
>>      - There is a single MDIO controller that has MDIO buses as children
>>      Changes in v3:
>>      - None
>>      Changes in v2:
>>      - None
>>
>>   .../bindings/mfd/realtek,rtl9301-switch.yaml  | 29 +++++++++++++++++++
>>   1 file changed, 29 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/realtek,rtl9301-switch.yaml b/Documentation/devicetree/bindings/mfd/realtek,rtl9301-switch.yaml
>> index f053303ab1e6..89e10213a4ee 100644
>> --- a/Documentation/devicetree/bindings/mfd/realtek,rtl9301-switch.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/realtek,rtl9301-switch.yaml
>> @@ -28,6 +28,9 @@ properties:
>>     reg:
>>       maxItems: 1
>>   
>> +  mdio-controller:
>> +    $ref: /schemas/net/realtek,rtl9301-mdio.yaml#
>> +
>>     '#address-cells':
>>       const: 1
>>   
>> @@ -41,6 +44,10 @@ patternProperties:
>>     'i2c@[0-9a-f]+$':
>>       $ref: /schemas/i2c/realtek,rtl9301-i2c.yaml#
>>   
>> +  'mdio-controller@[0-9a-f]+$':
>> +    $ref: /schemas/net/realtek,rtl9301-mdio.yaml#
>> +
>> +
>>   required:
>>     - compatible
>>     - reg
>> @@ -110,5 +117,27 @@ examples:
>>             };
>>           };
>>         };
>> +
>> +      mdio-controller@ca00 {
>> +        compatible = "realtek,rtl9301-mdio";
>> +        reg = <0xca00 0x200>;
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        mdio-bus@0 {
>> +          reg = <0>;
>> +          #address-cells = <1>;
>> +          #size-cells = <0>;
>> +
>> +          ethernet-phy@0 {
>> +            reg = <0>;
>> +            realtek,port = <1>;
> Aren't all those PHYs referenced as phandles by DSA switch ports?

I'm still tiptoeing around whether this thing will be DSA or 
switchdev[1]. In theory the RTL9300 could be either although the 
specific design I'm working uses the internal CPU core so it's more 
switchdev like. Binding wise the mdio-bus arrangement would be fairly 
similar in either case.

> Imho it would be better to not introduce a new property but instead
> let the driver of the mdio-controller parse the DSA switch description
> and follow the existing 'phy-handle' properties in order to infer the
> mapping of all ports to all PHYs, and by that then be able to also
> know the reverse mapping.
> You could reference the switch node in the mdio-controller node.

As it stands the switch node is the parent of the mdio-controller (that 
may actually help as presumably I can go via the parent rather than a 
phandle). I've kind of avoided doing anything involving too much of the 
switch because I was hoping to land the mdio driver independently. Maybe 
I still can as long as I define the binding for the switch block now. Is 
is the done thing for one node in the dts to parse information from a 
second?

>
> That would avoid redundant information in the device tree, as we
> would then only have one mapping instead of having it two times
> (once by the usual 'phy-handle' property of the DSA user port and
> another time reverse using your newly introduce 'realtek,port'
> property of each ethernet-phy).

Yes that makes sense. It does mean I need to start defining the binding 
for the actual switch portion which I've been putting off. Time to roll 
up those sleeves.

>
>> +          };
>> +          ethernet-phy@1 {
>> +            reg = <1>;
>> +            realtek,port = <0>;
>> +          };
>> +        };
>> +      };
>>       };
>>   
>> -- 
>> 2.48.1
>>
>>
[1] - 
https://lore.kernel.org/lkml/b15b15ce-ae24-4e04-83ab-87017226f558@xxxxxxxxxxxxxxxxxxx/