diff mbox series

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

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

Commit Message

Chris Packham Jan. 20, 2025, 4:02 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:
    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  | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Krzysztof Kozlowski Jan. 22, 2025, 8:14 a.m. UTC | #1
On Mon, Jan 20, 2025 at 05:02:12PM +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>
> ---
> 

You need to explain merging dependencies. Nothing in cover letter,
nothing here, but this *CANNOT* be merged independently.

> Notes:
>     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  | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/realtek,rtl9301-switch.yaml b/Documentation/devicetree/bindings/mfd/realtek,rtl9301-switch.yaml
> index f053303ab1e6..c19d2c209434 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
>  
> @@ -110,5 +113,26 @@ examples:
>            };
>          };
>        };
> +
> +      mdio-controller {

No, no resources here, no unit address. Look at other nodes - they have
the resource, the address. Mixing such nodes is clear indication this is
not correct hardware description and you do this only for Linux.

Fold child device into parent.

Best regards,
Krzysztof
Chris Packham Jan. 22, 2025, 8:53 p.m. UTC | #2
On 22/01/2025 21:14, Krzysztof Kozlowski wrote:
> On Mon, Jan 20, 2025 at 05:02:12PM +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>
>> ---
>>
> You need to explain merging dependencies. Nothing in cover letter,
> nothing here, but this *CANNOT* be merged independently.
OK. I'll make sure to add a note here and to the series cover letter.
>> Notes:
>>      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  | 24 +++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/realtek,rtl9301-switch.yaml b/Documentation/devicetree/bindings/mfd/realtek,rtl9301-switch.yaml
>> index f053303ab1e6..c19d2c209434 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
>>   
>> @@ -110,5 +113,26 @@ examples:
>>             };
>>           };
>>         };
>> +
>> +      mdio-controller {
> No, no resources here, no unit address. Look at other nodes - they have
> the resource, the address. Mixing such nodes is clear indication this is
> not correct hardware description and you do this only for Linux.
>
> Fold child device into parent.

In this particular case all the mdio stuff is actually contained to a 
range starting at offset 0xca00. I dropped it because it was simpler in 
the driver to use the full 16-bit address rather than trying to use 
offsets from the base address that didn't correspond to the datasheet. 
As you've highlighted that's making the dt-binding impose driver 
specifics so would adding back `mdio-controller@ca00` and `reg = <0xca00 
0x200>;` be OK even if the driver doesn't actually use them?

>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Jan. 31, 2025, 7:26 a.m. UTC | #3
On 22/01/2025 21:53, Chris Packham wrote:
>>>         };
>>> +
>>> +      mdio-controller {
>> No, no resources here, no unit address. Look at other nodes - they have
>> the resource, the address. Mixing such nodes is clear indication this is
>> not correct hardware description and you do this only for Linux.
>>
>> Fold child device into parent.
> 
> In this particular case all the mdio stuff is actually contained to a 
> range starting at offset 0xca00. I dropped it because it was simpler in 
> the driver to use the full 16-bit address rather than trying to use 
> offsets from the base address that didn't correspond to the datasheet. 
> As you've highlighted that's making the dt-binding impose driver 
> specifics so would adding back `mdio-controller@ca00` and `reg = <0xca00 
> 0x200>;` be OK even if the driver doesn't actually use them?


If this matches the hardware, then yes.

Best regards,
Krzysztof
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..c19d2c209434 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
 
@@ -110,5 +113,26 @@  examples:
           };
         };
       };
+
+      mdio-controller {
+        compatible = "realtek,rtl9301-mdio";
+        #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>;
+          };
+        };
+      };
     };