Re: [PATCH 7/8] dt-bindings: i2c: realtek,rtl9301-i2c: extend for RTL9607C support

[Rustam Adilov <adilov@disroot.org>]

Hello,

On 2026-03-15 08:56, Krzysztof Kozlowski wrote:
> On Sat, Mar 14, 2026 at 01:26:27PM +0500, Rustam Adilov wrote:
>> Add the "realtek,rtl9607-i2c" compatible for i2c controller on the
>> RTL9607C SoC series.
>> 
>> Add a clocks property to the properties since RTL9607C requires it
>> along with the realtek,scl.
>> 
>> Signed-off-by: Rustam Adilov <adilov@disroot.org>
>> ---
>>  .../bindings/i2c/realtek,rtl9301-i2c.yaml         | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml b/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
>> index f9a449fee2b0..5873cfdc5b3e 100644
>> --- a/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
>> +++ b/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
>> @@ -15,6 +15,8 @@ description:
>>    assigned to either I2C controller.
>>    RTL9310 SoCs have equal capabilities but support 12 common SDA lines which
>>    can be assigned to either I2C controller.
>> +  RTL9607C SoCs have equal capabilities but each controller only supports 1
>> +  SCL/SDA line.
>>  
>>  properties:
>>    compatible:
>> @@ -34,6 +36,7 @@ properties:
>>        - enum:
>>            - realtek,rtl9301-i2c
>>            - realtek,rtl9310-i2c
>> +          - realtek,rtl9607-i2c
>>  
>>    reg:
>>      items:
>> @@ -51,6 +54,9 @@ properties:
>>        The SCL line number of this I2C controller.
>>      enum: [ 0, 1 ]
>>  
>> +  clocks:
>> +    maxItems: 1
>> +
>>  patternProperties:
>>    '^i2c@[0-9ab]$':
>>      $ref: /schemas/i2c/i2c-controller.yaml
>> @@ -81,6 +87,15 @@ allOf:
>>      then:
>>        patternProperties:
>>          '^i2c@[89ab]$': false
> 
> Other devices do not have any clock input? Hard to believe, but if that
> was the case then previous "if:then:" should disallow this. But if you
> assume they have clock, which is expected, then document this in the
> commit msg. Now you just silently add clock to each of existing variant
> without any explanation WHY.

As far as i can tell, that does seem to be the case that they don't need the
clocks input. In RTL9300 and RL9310 there is a simple SCL_FREQ bit field that
sets the frequency from preselected option (100khz, 400kHz, 2.5MHz and 50kHz) and
it is gotten straight from the clock-frequency of the i2c child nodes.
But in RTL9607C there is CLK_DIV bit field instead and it is set to the calculated
value of "clk_get_rate(i2c->clk) / i2c->bus_freq - 1" and that is why it requires
clocks.

With this, what should be changed in this i2c bindings file? I was under assumption
that even if only RTL9607 requires the clocks i would still need to add it to the 
properties along with the "if:then:" but if not then i can remove it.
> 
> Best regards,
> Krzysztof

Best,
Rustam