Re: [PATCH v5 3/8] dt-bindings: mfd: realtek,rtd1xxx: Add interrupt controller child node

[Krzysztof Kozlowski <krzk@kernel.org>]

On 22/10/2026 08:44, Yu-Chun Lin [林祐君] wrote:
> Hi Krzysztof,
> 
>>>>>  patternProperties:
>>>>> +  "^interrupt-controller":
>>>>> +    $ref: /schemas/interrupt-controller/realtek,rtd1319-intc-iso.yaml
>>>>> +    unevaluatedProperties: false
>>>>> +
>>>>>    '@[0-9a-f]+$':
>>>>>      type: object
>>>>
>>>> What is this for then? Should it be removed?
>>>>
>>>
>>> The original syscon compatible has other child nodes that do have 'reg'
>>> properties and unit addresses, so this pattern is still needed for them.
>>>
>>>>>
>>>>> @@ -76,3 +80,27 @@ required:
>>>>>    - reg
>>>>>
>>>>>  additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> +  - |
>>>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>>>> +
>>>>> +    syscon@7000 {
>>>>> +        compatible = "realtek,rtd1625-iso", "syscon", "simple-mfd";
>>>>> +        reg = <0x7000 0x1000>;
>>>>> +        ranges = <0x0 0x7000 0x1000>;
>>>>> +        #address-cells = <1>;
>>>>> +        #size-cells = <1>;
>>>>
>>>> Why do you have cells it your child has no addressing?
>>>>
>>>>> +
>>>>> +        interrupt-controller {
>>>>
>>>> So the binding requires children with unit address, yes?
>>>>
>>>
>>> The interrupt-controller does not need a unit address because it
>>> accesses the registers via the parent's regmap.
>>
>> That's not really relevant. Child drivers still access via parent's regmap and
>> have unit address. This binding has bus addressing for children, so use it. Do
>> not add random or inconsistent code.
>>
> 
> The registers for interrupt controller are quite scattered within the parent
> syscon block. Take rtd1625-intc-misc as an example, the offsets are 0x4,
> 0xc, 0x80, and 0x88 After adding unit-address and reg.
> It would be like:
> 
> interrupt-controller@4 {
>     compatible = "realtek,rtd1625-intc-misc";
>     reg = <0x4 0x4>,
>           <0xc 0x4>,
>           <0x80 0x4>,
>           <0x88 0x4>;
>     interrupt-controller;

That would mean that this device is instead part of the parent. If you
do not have separate address space, how is it a separate device?

>     ...
> };
> 
> Could you please confirm if this is the correct and preferred approach?
> 
>>>
>>> Will update the example to remove the unnecessary properties from the
>>> parent as follows:
>>>
>>> syscon@7000 {
>>>     compatible = "realtek,rtd1625-iso", "syscon", "simple-mfd";
>>>     reg = <0x7000 0x1000>;
>>>
>>
>> Still incomplete. Where are other devices?
>>
> 
> In the past, Andreas used a generic '@[0-9a-f]+$"' to match child nodes.
> Because of this, in older SoCs (e.g., rtd129x.dtsi, rtd16xx.dtsi,
> rtd139x.dtsi), devices like reset-controller, watchdog, and serial were
> allowed and placed under the iso or misc syscon nodes.
> 
> However, I have some doubts about continuing to use that exact structure:
> 
> 1. We previously discussed and agreed [1] that devices like serial should
> actually be placed under rbus, not syscon.
> 2. For the SoCs introduced in this patch (rtd1319,rtd1319d, rtd1325,
> rtd1625), the drivers for the other child devices (like watchdog) are not
> upstreamed yet.
> 
> To properly fix the "incomplete example" for the simple-mfd structure,
> could you please advise:
> 
> 1. Should I replace the generic '@[0-9a-f]+$' in the YAML binding with
> explicit device definitions for these specific child nodes?
> 2. For the example section, should I just populate it with placeholders
> (e.g., watchdog) referencing Andreas's older DTs (like rtd129x.dtsi) to
> illustrate the MFD structure?
> 
> [1] https://lore.kernel.org/lkml/15951264289d44f797ebd4e05238b380@realtek.com/

If you know that existing pattern is not really correct, please do not
grow it.

Best regards,
Krzysztof