Re: [PATCH v4 2/3] dt-bindings: i2c: realtek,rtl9301-i2c: extend for RTL9310 support

[Jonas Jelonek <jelonek.jonas@gmail.com>]

Hi Rob,

On 31.07.2025 00:29, Rob Herring wrote:
> On Tue, Jul 29, 2025 at 07:51:44AM +0000, Jonas Jelonek wrote:
>>  
>>    reg:
>>      items:
>> @@ -35,8 +45,15 @@ properties:
>>    "#size-cells":
>>      const: 0
>>  
>> +  realtek,mst-id:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      Realtek-internal ID of the I2C controller/master.
>> +    minimum: 1
>> +    maximum: 2
> We normally try to avoid instance IDs. Why not just a property defining 
> the SCL # to use.
>

I argued with the following reasons to use the master id:
- Realtek mentions 'Master 1' and 'Master 2' a lot in their documentation while
   the SCL number is barely mentioned and probably also confusing in their
   docs [1]
- other specifics could be inferred from that master id, not only the SCL
   number. While this is not relevant yet, future SoC series might need this
   if support for them can be added in this driver

However, using `realtek,scl-num` would be perfectly fine for now if you'd
really like to avoid this master ID. Either way, this property has to match the
other settings anyway (i. e. unit address) to make this actually work.
The master ID only would have made this more flexible if not only the SCL
number is different, but this actually isn't relevant for RTL9300 and RTL9310.

>> +
>>  patternProperties:
>> -  '^i2c@[0-7]$':
>> +  '^i2c@([0-9]|1[0-1])$':
> Unit-addresses are typically hex.

Will fix that.

>>  required:
>>    - compatible
>>    - reg
>> @@ -68,3 +104,15 @@ examples:
>>          #size-cells = <0>;
>>        };
>>      };
>> +  - |
>> +    i2c@100c {
>> +      compatible = "realtek,rtl9310-i2c";
>> +      reg = <0x100c 0x18>;
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +      realtek,mst-id = <1>;
>> +
>> +      i2c@0 {
>> +        reg = <0>;
>> +      };
>> +    };
> Is this really different enough to justify another example?

Probably not, would be only to showcase the subtle differences between
RTL9300 and RTL9310.
I just added it, temporarily with another unit address, to check with
dt_binding_check if this is working.


Best,
Jonas Jelonek