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

[Ryan Chen]

> 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. :/

The following is datasheet description. 

AST2700 Interrupt Controller Hierarchy (from datasheet):

INTC0 and INTC1 are AMBA slave devices on the AHB bus,
each with their own register space. 480 interrupt sources: INTn (n=0~479)
INT0~127 can be routed directly to PSP, SSP, or TSP.
INT128~319 are handled by INTC1, which have multiple instances
(INTC1_0, INTC1_1, ...)
INTC1 outputs are routed into INTC0; INTC0 outputs go to the GIC.

This structure means:
INTC0 receives INT0~127 and also all outputs from INTC1.
INTC1 handles a subset of interrupt sources, and its output 
is routed as an input to INTC0.

Block Diagram / Interrupt Chain:
GIC
 |
 v
INTC0 (parent, aspeed,ast2700-intc0)
 |
 +-- INTC0_0 (aspeed,ast2700-intc-ic) --> [module0, module1, ...]
 |
 +-- ...
 |
 +-- INTC0_11 (aspeed,ast2700-intc-ic)
      |
      v
   INTC1 (parent, aspeed,ast2700-intc1)
    |
    +-- INTC1_0 (aspeed,ast2700-intc-ic) --> [moduleA, moduleB, ...]
    |
    +-- ...
    |
    +-- INTC1_5 (aspeed,ast2700-intc-ic) --> [moduleY, moduleZ, ...]

| Device Tree Node | Hardware Block  | Output Routed To    |
| -------          | ---------------      | ----------------------     |
| intc0          | INTC0 @12100000 | GIC               |
| intc1          | INTC1 @14c18000 | INTC0 input (cascaded)|

intc0 uses interrupts to connect to the GIC (top-level parent)
intc1 uses interrupts-extended to connect to an input on INTC0
(second-level, cascaded)

This approach ensures the software and device tree reflect the
actual hardware interrupt paths.
It allows the kernel to correctly map register space and handle interrupt
delivery, and makes future debug/maintenance straightforward.

If there are any details you'd like clarified or if you recommend a 
different device tree structure, please let me know!

> 
> >>
> >> 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 at 12101b00 {
> >     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 at 14c18100 {
> >     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?
> 

The change from interrupts to interrupts-extended does not change the 
source or meaning of the interrupt itself.
For first-level INTC-IC nodes, the parent is the GIC, so we use interrupts.

For second-level (cascaded) INTC-IC nodes, the parent is an input on INTC0, 
so interrupts-extended is required to correctly reflect the hardware chain as
described in the datasheet.
This ensures the DT matches the hardware hierarchy—the actual interrupt source 
and routing path are not changed, only described more precisely.

> > };
> > 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?

The oneOf schema allows the binding to support both scenarios,
matching the hardware and software requirements.

first-level INTC-IC nodes required [interrupts]
second-level (cascaded) required [interrupts-extended]

> 
> 
> Best regards,
> Krzysztof