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

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

Hi Kryzysztof,

On 23.07.2025 08:22, Krzysztof Kozlowski wrote:
> On 22/07/2025 20:25, Jonas Jelonek wrote:
>>>> Since you have a lot of expertise on that and I obviously fail to find
>>>> documentation that helps me to do that properly, could you give me some hints
>>>> on how that has to look? I'd really appreciate this.
>>> So in your if:then: block where you narrow mst-id, you add on same level
>>> as properties:
>>>
>>> patternProperties:
>>>   YOUR_REGEX: false
>> How I thought of narrowing that in the first place was to make mst-id required
>> for RTL9310 but optional for RTL9300. In terms of describing the hardware, this
>> is valid for RTL9300 too (but there's no need for the driver or anything else to
>> know that).
>>
>> But I don't mind if you'd rather have it only defined in the 'then' block, or
>> just disallowed for RTL9300, effectively forbidding the usage for RTL9300.
>>
>> Either way, it seems I'm still doing it wrong with the regex. Adding as you
>> suggested:
>>
>> if:
>>     properties:
>>         compatible:
>>             contains:
>>                 const: realtek,rtl9301-i2c
>> then:
>>     patternProperties:
>>         '^i2c@([0-9]|1[0-1])$': false
>>
>> breaks validation of the RTL9300 example. Probably I don't see how this
>> is expected to look like in a working state.
> RTL9300 has 8 controllers, so why are you disallowing them? We talk here
> only about new stuff. Why would you change EXISTING behavior when adding
> something new?
>
> You need pattern matching redundant children for existing device.
>

It wasn't my intention to disallow that, but I'm struggling with these specifics
of the dt-bindings to accomplish what is requested. Anyway, I think your comment
gave me a good hint to solve that now.

if:
    properties:
        compatible:
            contains:
                const: realtek,rtl9301-i2c
then:
    patternProperties:
        '^i2c@([8-9]|1[0-1])$': false

This seems to do the trick, dt_binding_check is fine with this. Is this how you
intended it to look like?


Talking about 'not changing existing behaviour', is it fine to allow the mst-id
property for the existing RTL9300 too or should that really only be allowed for
what I add here now?

> Best regards,
> Krzysztof

Best regards,
Jonas Jelonek