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

[Chris Packham]

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@xxxxxxxxxxxxxxxxxxx>
>> ---
>>
> 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
>