linux-kernel - Re: [PATCH v3 1/2] dt-bindings: interrupt-controller: aspeed: Add parent node compatibles and refine documentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b8e570ab-313f-4f20-bedb-a1191c672435@kernel.org>
Date: Sun, 27 Jul 2025 11:36:23 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Ryan Chen <ryan_chen@...eedtech.com>, Thomas Gleixner
 <tglx@...utronix.de>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Joel Stanley <joel@....id.au>,
 Andrew Jeffery <andrew@...econstruct.com.au>,
 Kevin Chen <kevin_chen@...eedtech.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>,
 "linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>
Subject: Re: [PATCH v3 1/2] dt-bindings: interrupt-controller: aspeed: Add
 parent node compatibles and refine documentation

On 27/07/2025 03:47, Ryan Chen wrote:
>> Subject: Re: [PATCH v3 1/2] dt-bindings: interrupt-controller: aspeed: Add parent
>> node compatibles and refine documentation
>>
>> On 23/07/2025 10:08, Ryan Chen wrote:
>>>> Subject: Re: [PATCH v3 1/2] dt-bindings: interrupt-controller:
>>>> aspeed: Add parent node compatibles and refine documentation
>>>>
>>>> On 22/07/2025 11:51, Ryan Chen wrote:
>>>>> +  INTC0 is used to assert GIC if interrupt in INTC1 asserted.
>>>>> +  INTC1 is used to assert INTC0 if interrupt of modules asserted.
>>>>> +  +-----+   +---------+
>>>>> +  | GIC |---|  INTC0  |
>>>>> +  +-----+   +---------+
>>>>> +            +---------+
>>>>> +            |         |---module0
>>>>> +            | INTC0_0 |---module1
>>>>> +            |         |---...
>>>>> +            +---------+---module31
>>>>> +            |---....  |
>>>>> +            +---------+
>>>>> +            |         |     +---------+
>>>>> +            | INTC0_11| +---| INTC1   |
>>>>> +            |         |     +---------+
>>>>> +            +---------+     +---------+---module0
>>>>> +                            | INTC1_0 |---module1
>>>>> +                            |         |---...
>>>>> +                            +---------+---module31
>>>>> +                            ...
>>>>> +                            +---------+---module0
>>>>> +                            | INTC1_5 |---module1
>>>>> +                            |         |---...
>>>>> +                            +---------+---module31
>>>>
>>>> You binding also said intc1 is the parent of intc-ic, so where is here intc-ic?
>>>>
>>>> This diagram and new binding do not match at all.
>>>
>>> The corresponded compatible is following.
>>>
>>>   +-----+   +---------+
>>>   | GIC |---|  INTC0  | -> (parent : aspeed,ast2700-intc0)
>>>   +-----+   +---------+
>>>             +---------+
>>>             |        |---module0
>>>             | INTC0_0 |---module1
>>> 			(child : aspeed,ast2700-intc-ic)
>>>             |        |---...
>>>             +---------+---module31
>>>             |---....  |
>>>             +---------+
>>>             |         |    					 +---------+
>>>             | INTC0_11 | +----------------------------	| INTC1   |  -> ->
>> (parent : aspeed,ast2700-intc1)
>>
>> AGAIN (second time): that's not what your binding said.
>>
>> Your binding is explicit here, which is what we want in general. It says that inct1 is
>> one of the parents of intc-ic.

... and you never addressed that. :/

>>
>> Let me be clear, because you will be dragging this talk with irrelevant arguments
>> forever - changing this binding is close to no. If you come with correct arguments,
>> maybe would work. But the main point is that you probably do not have to even
>> change the binding to achieve proper hardware description. Work on that.
>>
> 
> If I do not change the binding, I think the yaml and dts can still fit the interrupt
> nesting architecture by using both interrupts and interrupts-extended.
> 
> For first-level controllers, use the standard interrupts property
> (e.g. with the GIC as the parent).
> 
> For second-level INTC-IC instances, use interrupts-extended to refer to the
> first-level INTC-IC, following common Linux practice for stacked interrupt controllers.
> For example:
> dts
> // First level
> intc0_11: interrupt-controller@...01b00 {
>     compatible = "aspeed,ast2700-intc-ic";
>     reg = <...>;
>     interrupt-controller;
>     #interrupt-cells = <2>;
>     interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>, ...;
> };
> 
> // Second level, cascaded
> intc1_0: interrupt-controller@...18100 {
>     compatible = "aspeed,ast2700-intc-ic";
>     reg = <...>;
>     interrupt-controller;
>     #interrupt-cells = <2>;
>     interrupts-extended = <&intc0_11 0 IRQ_TYPE_LEVEL_HIGH>;

This looks like changing the meaning of the interrupt. What was the
interrupt here before? What interrupt is here now?

> };
> In yaml, I can use:
> oneOf:
>   - required: [interrupts]
>   - required: [interrupts-extended]
> This allows both cases to be valid.


Hm? Since when you need both cases?


Best regards,
Krzysztof