[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250723050120.GA1231854-robh@kernel.org>
Date: Wed, 23 Jul 2025 00:01:20 -0500
From: Rob Herring <robh@...nel.org>
To: Ryan Chen <ryan_chen@...eedtech.com>
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, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.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 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.
>
> 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,ast2700-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
> index 55636d06a674..bdc4d8835843 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-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