[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<OS8PR06MB7541F7F2775C3BF46C97957BF25FA@OS8PR06MB7541.apcprd06.prod.outlook.com>
Date: Wed, 23 Jul 2025 07:56:37 +0000
From: Ryan Chen <ryan_chen@...eedtech.com>
To: Rob Herring <robh@...nel.org>
CC: Thomas Gleixner <tglx@...utronix.de>, 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
> Subject: Re: [PATCH v3 1/2] dt-bindings: interrupt-controller: aspeed: Add
> parent node compatibles and refine documentation
>
> On Tue, Jul 22, 2025 at 05:51:55PM +0800, Ryan Chen wrote:
> > The AST2700 SoC contains two independent top-level interrupt
> > controllers
> > (INTC0 and INTC1), each responsible for handling different peripheral
> > groups and occupying separate register spaces. Above them, PSP(CA35)
> > GIC controller acts as the root interrupt aggregator. Accurately
> > describing this hierarchical hardware structure in the device tree
> > requires distinct compatible strings for the parent nodes of INTC0 and INTC1.
> >
> > - Adds 'aspeed,ast2700-intc0' and 'aspeed,ast2700-intc1' compatible
> > strings for parent interrupt controller nodes. (in addition to the
> > existing 'aspeed,ast2700-intc-ic' for child nodes)
> > - Clarifies the relationship and function of INTC0 parent
> > (intc0_0~x: child), INTC1 parent (intc1_0~x: child), and the GIC in
> > the documentation.
> > - Updates block diagrams and device tree examples to illustrate the
> > hierarchy and compatible usage.
> > - Refines documentation and example formatting.
> >
> > This change allows the device tree and driver to distinguish between
> > parent (top-level) and child (group) interrupt controller nodes,
> > enabling more precise driver matching SOC register space allocation.
>
> That's nice, but is an ABI break.
Sorry, Is it an ABI break?
I keep the compatible aspeed,ast2700-intc-ic, not change it.
Just move it to be child, and make aspeed,ast2700-intc0/1 to be parent for aspeed,ast2700-intc-ic.
Older : https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/interrupt-controller/aspeed%2Cast2700-intc.yaml#L70C2-L86C7
bus {
#address-cells = <2>;
#size-cells = <2>;
interrupt-controller@...01b00 {
compatible = "aspeed,ast2700-intc-ic";
reg = <0 0x12101b00 0 0x10>;
#interrupt-cells = <2>;
interrupt-controller;
interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
};
};
New parent and child
bus {
#address-cells = <2>;
#size-cells = <2>;
intc0: interrupt-controller@...00000 {
compatible = "aspeed,ast2700-intc0";
reg = <0 0x12100000 0 0x4000>;
ranges = <0x0 0x0 0x0 0x12100000 0x0 0x4000>;
#address-cells = <2>;
#size-cells = <2>;
intc0_11: interrupt-controller@...0 {
compatible = "aspeed,ast2700-intc-ic";
reg = <0 0x12101b00 0 0x10>;
#interrupt-cells = <2>;
interrupt-controller;
interrupts = <GIC_SPI 192 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
<GIC_SPI 193 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
<GIC_SPI 194 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
<GIC_SPI 195 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
<GIC_SPI 196 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
<GIC_SPI 197 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
};
};
intc1: interrupt-controller@...18000 {
compatible = "aspeed,ast2700-intc1";
reg = <0 0x14c18000 0 0x400>;
ranges = <0x0 0x0 0x0 0x14c18000 0x0 0x400>;
#address-cells = <2>;
#size-cells = <2>;
intc1_0: interrupt-controller@100 {
compatible = "aspeed,ast2700-intc-ic";
reg = <0x0 0x100 0x0 0x10>;
#interrupt-cells = <2>;
interrupt-controller;
interrupts-extended = <&intc0_11 0 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
};
};
};
>
> >
> > Signed-off-by: Ryan Chen <ryan_chen@...eedtech.com>
> > ---
> > .../aspeed,ast2700-intc.yaml | 158
> +++++++++++++-----
> > 1 file changed, 115 insertions(+), 43 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270
> > 0-intc.yaml
> > b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270
> > 0-intc.yaml index 55636d06a674..bdc4d8835843 100644
> > ---
> > a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270
> > 0-intc.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,as
> > +++ t2700-intc.yaml
> > @@ -10,6 +10,33 @@ description:
> > This interrupt controller hardware is second level interrupt controller
> that
> > is hooked to a parent interrupt controller. It's useful to combine multiple
> > interrupt sources into 1 interrupt to parent interrupt controller.
> > + Depend to which INTC0 or INTC1 used.
> > + INTC0 and INTC1 are two kinds of interrupt controller with enable
> > + and raw status registers for use.
> > + 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
> >
> > maintainers:
> > - Kevin Chen <kevin_chen@...eedtech.com> @@ -17,49 +44,70 @@
> > maintainers:
> > properties:
> > compatible:
> > enum:
> > - - aspeed,ast2700-intc-ic
> > + - aspeed,ast2700-intc0
> > + - aspeed,ast2700-intc1
> >
> > reg:
> > maxItems: 1
> >
> > - interrupt-controller: true
> > + '#address-cells':
> > + const: 2
> >
> > - '#interrupt-cells':
> > + '#size-cells':
> > const: 2
> > - description:
> > - The first cell is the IRQ number, the second cell is the trigger
> > - type as defined in interrupt.txt in this directory.
> > -
> > - interrupts:
> > - maxItems: 6
> > - description: |
> > - Depend to which INTC0 or INTC1 used.
> > - INTC0 and INTC1 are two kinds of interrupt controller with enable
> and raw
> > - status registers for use.
> > - INTC0 is used to assert GIC if interrupt in INTC1 asserted.
> > - INTC1 is used to assert INTC0 if interrupt of modules asserted.
> > - +-----+ +-------+ +---------+---module0
> > - | GIC |---| INTC0 |--+--| INTC1_0 |---module2
> > - | | | | | | |---...
> > - +-----+ +-------+ | +---------+---module31
> > - |
> > - | +---------+---module0
> > - +---| INTC1_1 |---module2
> > - | | |---...
> > - | +---------+---module31
> > - ...
> > - | +---------+---module0
> > - +---| INTC1_5 |---module2
> > - | |---...
> > - +---------+---module31
> >
> > + ranges: true
> > +
> > +patternProperties:
> > + "^interrupt-controller@":
> > + type: object
> > + description: Interrupt group child nodes
> > + additionalProperties: false
> > +
> > + properties:
> > + compatible:
> > + enum:
> > + - aspeed,ast2700-intc-ic
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupt-controller: true
> > +
> > + '#interrupt-cells':
> > + const: 2
> > + description: |
>
> Don't need '|'.
>
> > + The first cell is the IRQ number, the second cell is the trigger
> > + type as defined in interrupt.txt in this directory.
>
> Don't add links to legacy documents.
>
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 6
> > + description: |
> > + The interrupts provided by this interrupt controller.
> > +
> > + interrupts-extended:
>
> Why do you have both interrupts and interrupts-extended? They are mutually
> exclusive and both are auto-magically supported. The schemas only have to
> document 'interrupts'.
>
> > + minItems: 1
> > + maxItems: 6
> > + description: |
> > + This property is required when defining a cascaded interrupt
> controller
> > + that is connected under another interrupt controller. It specifies
> the
> > + parent interrupt(s) in the upstream controller to which this
> controller
> > + is connected.
> > +
> > + oneOf:
> > + - required: [interrupts]
> > + - required: [interrupts-extended]
> > +
> > + required:
> > + - compatible
> > + - reg
> > + - interrupt-controller
> > + - '#interrupt-cells'
> >
> > required:
> > - compatible
> > - reg
> > - - interrupt-controller
> > - - '#interrupt-cells'
> > - - interrupts
> >
> > additionalProperties: false
> >
> > @@ -68,19 +116,43 @@ examples:
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> >
> > bus {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + intc0: interrupt-controller@...00000 {
>
> This node isn't an interrupt-controller.
>
> > + compatible = "aspeed,ast2700-intc0";
> > + reg = <0 0x12100000 0 0x4000>;
> > + ranges = <0x0 0x0 0x0 0x12100000 0x0 0x4000>;
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + intc0_11: interrupt-controller@...0 {
> > + compatible = "aspeed,ast2700-intc-ic";
> > + reg = <0 0x12101b00 0 0x10>;
> > + #interrupt-cells = <2>;
> > + interrupt-controller;
> > + interrupts = <GIC_SPI 192 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > + <GIC_SPI 193 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > + <GIC_SPI 194 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > + <GIC_SPI 195 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > + <GIC_SPI 196 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > + <GIC_SPI 197 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>;
> > + };
> > + };
> > +
> > + intc1: interrupt-controller@...18000 {
> > + compatible = "aspeed,ast2700-intc1";
> > + reg = <0 0x14c18000 0 0x400>;
> > + ranges = <0x0 0x0 0x0 0x14c18000 0x0 0x400>;
> > #address-cells = <2>;
> > #size-cells = <2>;
> >
> > - interrupt-controller@...01b00 {
> > - compatible = "aspeed,ast2700-intc-ic";
> > - reg = <0 0x12101b00 0 0x10>;
> > - #interrupt-cells = <2>;
> > - interrupt-controller;
> > - interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>,
> > - <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>,
> > - <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>,
> > - <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>,
> > - <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>,
> > - <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
> > + intc1_0: interrupt-controller@100 {
> > + compatible = "aspeed,ast2700-intc-ic";
> > + reg = <0x0 0x100 0x0 0x10>;
> > + #interrupt-cells = <2>;
> > + interrupt-controller;
> > + interrupts-extended = <&intc0_11 0
> (GIC_CPU_MASK_SIMPLE(4)
> > + | IRQ_TYPE_LEVEL_HIGH)>;
> > };
> > + };
> > };
> > --
> > 2.34.1
> >
Powered by blists - more mailing lists