diff mbox series

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

Message ID 20250131010151.2527688-3-chris.packham@alliedtelesis.co.nz (mailing list archive)
State Superseded
Headers show
Series RTL9300 MDIO driver | expand

Commit Message

Chris Packham Jan. 31, 2025, 1:01 a.m. UTC
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@alliedtelesis.co.nz>
---

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(+)

Comments

Daniel Golle Jan. 31, 2025, 6:35 a.m. UTC | #1
Hi Chris,

afaik net-next is still closed right now, but lets discuss the series as RFC
in the meantime maybe, right?

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@alliedtelesis.co.nz>
> ---
> 
> 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?

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.

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).


> +          };
> +          ethernet-phy@1 {
> +            reg = <1>;
> +            realtek,port = <0>;
> +          };
> +        };
> +      };
>      };
>  
> -- 
> 2.48.1
> 
>
Krzysztof Kozlowski Jan. 31, 2025, 7:33 a.m. UTC | #2
On Fri, Jan 31, 2025 at 02:01:49PM +1300, Chris Packham wrote:
>  .../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#
> +

So this mdio-controller looks duplicated now. Why is it needed?

>    '#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#
> +
> +

Just one blank line.


Best regards,
Krzysztof
Chris Packham Feb. 2, 2025, 8:14 p.m. UTC | #3
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@alliedtelesis.co.nz>
>> ---
>>
>> 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@alliedtelesis.co.nz/
diff mbox series

Patch

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>;
+          };
+          ethernet-phy@1 {
+            reg = <1>;
+            realtek,port = <0>;
+          };
+        };
+      };
     };