irqchip/ast2700-intc: Add AST2700 INTC debugfs support and yaml update
Subsystems (3)
Ryan Chen
reviewerv4 cover irqchip/ast2700-intc: Add AST2700 INTC debugfs support and yaml update
This patch series adds device tree bindings and driver support for the AST2700 SoC!|s two interrupt controllers (INTC0 and INTC1), along with debugfs entries for runtime inspection of routing and register protection status, and bindings example refine.
v4:
- aspeed,ast2700-intc.yaml
- Clarify the relationship between INTC0/INTC1 parent nodes, the aspeed,ast2700-intc-ic child nodes, and the GIC.
- Add a block diagram and DT examples showing the cascaded wiring (GIC <- INTC0 <- INTC1 children).
- Mirrors the datasheet-described topology and register map, including the separation of INTC0/INTC1 regions.
- Lets DT unambiguously express first-level (GIC parent) and cascaded second-level (INTC0 parent) interrupt controllers via examples that use interrupts for INTC0 children and interrupts-extended for INTC1 children routed into INTC0.
- irq-ast2700-intc.c
- Drop all string decoding and human readable tables. Debugfs now dumps raw routing/protection registers only.
- Split into a separate source file and made it modular
- If the compatible not match ast2700-intc0/1, bail out return -ENODEV.
v3:
- aspeed,ast2700-intc.yaml
- Clarify the relationship between INTC0/INTC1 parent nodes, the aspeed,ast2700-intc-ic child nodes, and the GIC.
- Add a block diagram and DT examples showing the cascaded wiring
(GIC <- INTC0 <- INTC1 children).
- Mirrors the datasheet-described topology and register map, including the separation of INTC0/INTC1 regions and their routing/protection registers.
- Lets DT unambiguously express first-level (GIC parent) and cascaded second-level (INTC0 parent) interrupt controllers via examples that use interrupts for INTC0 children and interrupts-extended for INTC1 children routed into INTC0.
- irq-aspeed-intc.c
- separate c file from irq-aspeed-intc.c
- make m
v2:
- fix dt bindingcheck
Ryan Chen (2):
dt-bindings: interrupt-controller: aspeed: Add parent compatibles and
refine documentation
Irqchip/ast2700-intc: add debugfs support and AST2700 INTC0/INTC1
routing/protection display
.../aspeed,ast2700-intc.yaml | 158 +++++++++++----- drivers/irqchip/Kconfig | 6 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-ast2700-intc.c | 174 ++++++++++++++++++ 4 files changed, 296 insertions(+), 43 deletions(-) create mode 100644 drivers/irqchip/irq-ast2700-intc.c
Signature · 1 line
2.34.1
Ryan Chen
reviewerPATCH v4 1/2 dt-bindings: interrupt-controller: aspeed: Add parent compatibles and refine documentation
AST2700 contains two independent top-level interrupt controllers (INTC0, INTC1). Each occupies its own register space and handles different sets of peripherals. Above them, the PSP (CA35) GIC is the root interrupt aggregator. In hardware, INTC1 outputs are routed into INTC0, and INTC0 outputs are routed into the GIC.
Introduce distinct compatibles for these parent blocks so the DT can model the hierarchy and register space layout accurately:
- aspeed,ast2700-intc0 (parent node at 0x12100000)
- aspeed,ast2700-intc1 (parent node at 0x14c18000)
The existing child compatible:
- aspeed,ast2700-intc-ic
continues to describe the interrupt-controller instances within each INTC block (e.g. INTC0_0..INTC0_11 and INTC1_0..INTC1_5).
--- .../aspeed,ast2700-intc.yaml | 158 +++++++++++++----- 1 file changed, 115 insertions(+), 43 deletions(-)
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +115 -43
@@ -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 <[email protected]>
@@ -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: + The first cell is the IRQ number, the second cell is the trigger + type. + + interrupts: + minItems: 1 + maxItems: 6 + description: | + The interrupts provided by this interrupt controller. + + interrupts-extended: + 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@12100000 { + 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@1b00 { + 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@14c18000 { + compatible = "aspeed,ast2700-intc1"; + reg = <0 0x14c18000 0 0x400>; + ranges = <0x0 0x0 0x0 0x14c18000 0x0 0x400>; #address-cells = <2>; #size-cells = <2>; - interrupt-controller@12101b00 { - 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)>; }; + }; };
Signature · 1 line
2.34.1
Re: [PATCH v4 1/2] dt-bindings: interrupt-controller: aspeed: Add parent compatibles and refine documentation
Quoted 30 lines · Ryan Chen
AST2700 contains two independent top-level interrupt controllers (INTC0, INTC1). Each occupies its own register space and handles different sets of peripherals. Above them, the PSP (CA35) GIC is the root interrupt aggregator. In hardware, INTC1 outputs are routed into INTC0, and INTC0 outputs are routed into the GIC.
Introduce distinct compatibles for these parent blocks so the DT can model the hierarchy and register space layout accurately:
- aspeed,ast2700-intc0 (parent node at
0x12100000)
- aspeed,ast2700-intc1 (parent node at0x14c18000)The existing child compatible:
- aspeed,ast2700-intc-ic
continues to describe the interrupt-controller instances within each INTC block (e.g. INTC0_0..INTC0_11 and INTC1_0..INTC1_5).
Signed-off-by Ryan Chen <[email protected]>--- .../aspeed,ast2700-intc.yaml | 158 +++++++++++++----- 1 file changed, 115 insertions(+), 43 deletions(-)Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +0
-0@@ -10,6 +10,33 @@ description: This interrupt controller hardware is second level interrupt controller that
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml description: · line 20
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +8
-0is 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 | + +-----+ +---------+
Same problem as last time. This tells me intc0 has not children...
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml description: · line 30
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +10
-0+ +---------+ + | |---module0 + | INTC0_0 |---module1 + | |---... + +---------+---module31 + |---.... | + +---------+ + | | +---------+ + | INTC0_11| +---| INTC1 | + | | +---------+
...This tells that inc1 has no children (only intc0_11, which you said is aspeed,ast2700-intc-ic !!!)....
(keep scrolling)
Quoted 51 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +14
-24+ +---------+ +---------+---module0 + | INTC1_0 |---module1 + | |---... + +---------+---module31 + ... + +---------+---module0 + | INTC1_5 |---module1 + | |---... + +---------+---module31 maintainers: - Kevin Chen <[email protected]>@@ -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
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml maintainers: · line 62
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +4
-5- ... - | +---------+---module0 - +---| INTC1_5 |---module2 - | |---... - +---------+---module31 + ranges: true + +patternProperties: + "^interrupt-controller@":
... but this tells me that intc0 and intc1 has children.
Quoted 24 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +24
-0+ 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: + The first cell is the IRQ number, the second cell is the trigger + type. + + interrupts: + minItems: 1 + maxItems: 6 + description: |
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml maintainers: · line 96
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +10
-0+ The interrupts provided by this interrupt controller. + + interrupts-extended: + 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.
No, you do not define two. Only interrupts.
Quoted 27 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +13
-3+ + 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>; +
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml examples: · line 131
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +10
-0+ intc0: interrupt-controller@12100000 { + 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@1b00 { + compatible = "aspeed,ast2700-intc-ic"; + reg = <0 0x12101b00 0 0x10>;
... and that's quite wrong unit address. Also no resources in the parent, so this entire split seems superficial and incorrect.
This binding is not improving. You are not responding to REAL problems described to you. What's more, you send it in a way making our life difficult, look:
b4 diff '[email protected]' Using cached copy of the lookup
---
Analyzing 3 messages in the thread
Could not find lower series to compare against.
Best regards,
Krzysztof
Ryan Chen
reviewerRE: [PATCH v4 1/2] dt-bindings: interrupt-controller: aspeed: Add parent compatibles and refine documentation
Quoted 167 lines
Subject: Re: [PATCH v4 1/2] dt-bindings: interrupt-controller: aspeed: Add parent compatibles and refine documentation
Quoted 14 lines · Ryan Chen
AST2700 contains two independent top-level interrupt controllers (INTC0, INTC1). Each occupies its own register space and handles different sets of peripherals. Above them, the PSP (CA35) GIC is the root interrupt aggregator. In hardware, INTC1 outputs are routed into INTC0, and INTC0 outputs are routed into the GIC.
Introduce distinct compatibles for these parent blocks so the DT can model the hierarchy and register space layout accurately:
- aspeed,ast2700-intc0 (parent node at
0x12100000)
- aspeed,ast2700-intc1 (parent node at0x14c18000)The existing child compatible:
Quoted 8 lines
- aspeed,ast2700-intc-ic
continues to describe the interrupt-controller instances within each INTC block (e.g. INTC0_0..INTC0_11 and INTC1_0..INTC1_5).
Signed-off-by Ryan Chen <[email protected]>--- .../aspeed,ast2700-intc.yaml | 158+++++++++++++-----
Quoted 13 lines
1 file changed, 115 insertions(+), 43 deletions(-)diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270
0-intc.yamlb/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yamlindex55636d06a674..81304b53c112100644
---
a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,as +++t2700-intc.yamlReview description: · line 10
Quoted 1 line
(quoted diff) +0
-0@@ -10,6 +10,33 @@ description: This interrupt controller hardware is second level interrupt controllerthat
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml description: · line 20
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +8
-0is 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 | + +-----+ +---------+Same problem as last time. This tells me intc0 has not children...
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml description: · line 30
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +10
-0+ +---------+ + | |---module0 + | INTC0_0 |---module1 + | |---... + +---------+---module31 + |---.... | + +---------+ + | | +---------+ + | INTC0_11| +---| INTC1 | + | | +---------+...This tells that inc1 has no children (only
intc0_11, which you said is aspeed,ast2700-intc-ic !!!)....
(keep scrolling)Quoted 29 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +9
-0+ +---------+ +---------+---module0 + | INTC1_0 |---module1 + | |---... + +---------+---module31 + ... + +---------+---module0 + | INTC1_5 |---module1 + | |---... + +---------+---module31 maintainers: - Kevin Chen <[email protected]> @@ -17,49 +44,70 @@maintainers:
properties:
compatible:
enum:Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +5
-3- - aspeed,ast2700-intc-ic + - aspeed,ast2700-intc0 + - aspeed,ast2700-intc1 reg: maxItems: 1 - interrupt-controller: true + '#address-cells': + const: 2 - '#interrupt-cells': + '#size-cells':Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml maintainers:
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +0
-9const: 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 enableand raw
Quoted 12 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +0
-12- 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 - | | |---... - | +---------+---module31Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml maintainers: · line 62
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +4
-5- ... - | +---------+---module0 - +---| INTC1_5 |---module2 - | |---... - +---------+---module31 + ranges: true + +patternProperties: + "^interrupt-controller@":... but this tells me that intc0 and intc1 has children.
Quoted 21 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +21
-0+ 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: + The first cell is the IRQ number, the second cell is the trigger + type. + + interrupts:Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml maintainers:
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +10
-0+ minItems: 1 + maxItems: 6 + description: | + The interrupts provided by this interrupt controller. + + interrupts-extended: + minItems: 1 + maxItems: 6 + description: | + This property is required when defining a cascaded interruptcontroller
Quoted 7 lines
Quoted 1 line
+ that is connected under another interrupt controller. It specifies
the
Quoted 1 line
+ parent interrupt(s) in the upstream controller to which this
controller
Quoted 1 line
+ is connected.
No, you do not define two. Only interrupts.
Thanks, I did my homework, it is no need.
Quoted 34 lines
Quoted 34 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +19
-3+ + 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@12100000 { + compatible = "aspeed,ast2700-intc0"; + reg = <0 0x12100000 0 0x4000>; + ranges = <0x0 0x0 0x0 0x12100000 0x0 0x4000>; + #address-cells = <2>; + #size-cells = <2>;
Quoted 8 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml examples: · line 131
Quoted 4 lines
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +4
-0+ + intc0_11: interrupt-controller@1b00 { + compatible = "aspeed,ast2700-intc-ic"; + reg = <0 0x12101b00 0 0x10>;... and that's quite wrong unit address. Also no resources in the parent, so this entire split seems superficial and incorrect.
Sorry, it is my mistake.
Quoted 4 lines
This binding is not improving. You are not responding to REAL problems described to you. What's more, you send it in a way making our life difficult, look:
Sorry, My intent was to describe the SoC register space more precisely with dtsi, by having intc0/intc1 as parent nodes and the multiple intc-ic instances as children. But if this approach is not acceptable, I will drop the parent node split and keep only the multi-instance aspeed,ast2700-intc-ic nodes, as before.
Please let me know if you think there is still a way to keep the hierarchy useful. Thanks your guidance.
Quoted 9 lines
b4 diff '[email protected]' Using cached copy of the lookup
---
Analyzing 3 messages in the thread
Could not find lower series to compare against.Best regards,
Krzysztof
Andrew Jeffery
maintainerRe: [PATCH v4 1/2] dt-bindings: interrupt-controller: aspeed: Add parent compatibles and refine documentation
In reply to Krzysztof KozlowskiQuoted 58 lines · Krzysztof Kozlowski
Quoted 35 lines · Ryan Chen
AST2700 contains two independent top-level interrupt controllers (INTC0, INTC1). Each occupies its own register space and handles different sets of peripherals. Above them, the PSP (CA35) GIC is the root interrupt aggregator. In hardware, INTC1 outputs are routed into INTC0, and INTC0 outputs are routed into the GIC.
Introduce distinct compatibles for these parent blocks so the DT can model the hierarchy and register space layout accurately:
- aspeed,ast2700-intc0 (parent node at
0x12100000)
- aspeed,ast2700-intc1 (parent node at0x14c18000)The existing child compatible:
- aspeed,ast2700-intc-ic
continues to describe the interrupt-controller instances within each INTC block (e.g. INTC0_0..INTC0_11 and INTC1_0..INTC1_5).
Signed-off-by Ryan Chen <[email protected]>---
.../aspeed,ast2700-intc.yaml | 158 +++++++++++++-----1 file changed, 115 insertions(+), 43 deletions(-)(quoted diff) +0
-0
controller/aspeed,ast2700-intc.yaml
b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
index55636d06a674..81304b53c112100644Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml +0
-0@@ -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.
Quoted 8 lines
+ 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 |
+ +-----+ +---------+Same problem as last time. This tells me intc0 has not children...
Quoted 10 lines
+ +---------+
+ | |---module0 + | INTC0_0 |---module1 + | |---... + +---------+---module31 + |---.... | + +---------+ + | | +---------+ + | INTC0_11| +---| INTC1 | + | | +---------+
Quoted 14 lines
...This tells that inc1 has no children (only
intc0_11, which you said is aspeed,ast2700-intc-ic !!!)....
(keep scrolling)Quoted 10 lines
+ +---------+ +---------+---module0 + |
INTC1_0|---module1 + | |---... + +---------+---module31 + ...
+ +---------+---module0 + |INTC1_5|---module1 + | |---... + +---------+---module31
I've taken a look at the datasheet and had a bit of a think about how to make progress here.
My feeling is this diagram (and the previous one) undersells the complexity of the design by quite some margin. It's probably best to start by zooming out quite a lot:
The AST2700 SoC contains (at least) 4 distinct processors:
1. The quad-core ARM Cortex-A35 (PSP: Primary Service Processor)
2. A Cortex-M4 (SSP: Secondary Service Processor)
3. Another Cortex-M4 (TSP: Tertiary Service Processor)
4. The BootMCU - a RISC-V processor to execute the mask ROM
While the PSP GIC shown in the diagram above is one possible interrupt destination, many of the 480 interrupt sources in the package can be routed to the dedicated interrupt controller of any of these four processors. Likewise, many peripherals of the SoC are mapped into the physical address space of each processor. The routing is handled by the two interrupt controller blocks described in the binding text above:
+--------------------+
| +-------+ +------+ |
| | | | | |
| | PSP +-+ GIC <-+----------+
| | | | | | |
| +-------+ +------+ | |
+--------------------+ |
|
+--------------------+ |
| +-------+ +------+ | | +-------+
| | | | | | | | | INTx
| | SSP +-+ NVIC <-+----------+----------| INTC0 <-----------
| | | | | | | | |
| +-------+ +------+ | | +---^---+
+--------------------+ | |
| |
+--------------------+ | |
| +-------+ +------+ | | |
| | | | | | | |
| | TSP +-+ NVIC <-+----------+ |
| | | | | | |
| +-------+ +------+ | |
+--------------------+ |
|
+--------------------+ |
| +------+ +-------+ | +---+---+
| | | | | | | | INTy
| | BMCU +-+ APLIC <-+---------------------+ INTC1 <-----------
| | | | | | | |
| +------+ +-------+ | +-------+
+--------------------+There's a split in the interrupt domain: 0 <= INTx <= 127 < INTy.
The PSP GIC, SSP NVIC and TSP NVIC destinations for each INTx source is selected by a corresponding mux in INTC0.
The destination for each INTy source is selected by a corresponding mux in INTC1, where the possible destinations are:
1. A shared interrupt line Ma, routed through INTC0 to the PSP GIC
2. A mux in INTC0, providing a second level of indirection
3. A shared interrupt line Md, routed through INTC0 to the PSP GIC
4. A shared interrupt line Me, routed through INTC0 to the PSP GIC
5. A shared interrupt line Mb, routed through INTC0 to the SSP NVIC
6. A shared interrupt line Mc, routed through INTC0 to the TSP NVIC
7. The BootMCU APLIC
Each shared interrupt line from INTC1 to INTC0 merges up to 32 interrupt sources.
INTC0 and INTC1 are both mapped into the physical address space of each processor. However, to prevent any one of them interfering with interrupts dedicated to another, the controller's register map is divided into distinct regions whose access is constrained to a specific processor. The enable and status registers in INTC0 for shared line sets Ma, Md and Me are constrained to the PSP, Mb to the SSP, and Mc to the TSP.
+-------+ +-------+
| INTC0 | | INTC1 |
- - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- - - -+- -
+--------------------+ | | | |
| +-------+ +------+ | | | | |
| | | | | | | | | |
| | PSP +-+ GIC | | | | | |
| | | | | | | | | |
| +-------+ | | | | | | |
| | | | +-------+ | |
| | 192 <-|-----------+ Ma <------------|---+ |
| | | | +-------+ | | |
| | 208 <-|-----------+ Md <------------|---+ |
| | | | +-------+ | | |
| | 224 <-|-----------+ Me <------------|---+ |
| | | | +-------+ | | |
| +------+ | | | | | |
+--------------------+ | | | | |
- - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- -|- -+- -
+--------------------+ | | | | |
| +-------+ +------+ | +-------+ | +---|------------
| | SSP +-+ NVIC <-|-----------+ Mb <------------|---+ | INT128
| +-------+ +------+ | +-------+ | | |
+--------------------+ | | | | |
- - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- -|- -+- -
+--------------------+ | | | | |
| +-------+ +------+ | +-------+ | | |
| | TSP +-+ NVIC <-|-----------+ Mc <------------|---+ |
| +-------+ +------+ | +-------+ | |
+--------------------+ | | | |
- - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- - - -+- -
+--------------------+ | | | |
| +------+ +-------+ | | | | |
| | BMCU +-+ APLIC | | | | | |
| +------+ +-------+ | | | | |
+--------------------+ | | | |
- - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- - - -+- -
+-------+ +-------+INTC1 mux destination 2 above allows the PSP to reconfigure the routes at runtime at INTC0, rather than submit to a route that might've been protected by firmware early in the boot chain, so long as the boot firmware chose the appropriate route in INTC1. INTC0 merges these interrupts in sets of 32 and operates in much the same way as INTC1 in the diagram above, though cannot route to the BootMCU.
I'd prefer we acknowledge all this in the binding, and do enough to allow the kernel to configure routing as it wishes. While in some applications the routes may be constrained by firmware, the platform- specific portion of the devicetree can be written reflect this.
From some experimentation this shouldn't require anything bespoke. Using interrupts-extended we can represent the route configuration with a phandle to the upstream controller on the cascaded node. To avoid some arbitrary interrupt index choices at the node for INTC0, I think it's worth describing the register sets for interrupt sets Ma, Md and Me as subnodes of INTC0 with their own interrupt resources. This feels reasonably tidy, as the selection of the Ma, Md or Me sets completely determines its ultimate index at the PSP GIC. Doing so also removes them from needing to be described if any changes to some default route configuration are required by the platform, necessitating overriding the interrupts-extended property of the INTC0 node.
The only curiosity of this approach is that the interrupt-controller nodes for the non-PSP processors need to be described so we can reference them via phandles for the purpose of routing the interrupts. As these controllers are not mapped in the physical address space of the PSP we need the devicetree to inform the kernel as much.
Here's an example pseudo-devicetree. Of course there are elements that need more work, but I feel we can mine it for parts.
Cheers,
Andrew
/ {
primary {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges;
intc0: interrupt-controller@12100000 {
compatible = "aspeed,ast2700-intc0-a1";
reg = <0x12100000 0x1b00>;
#address-cells = <1>;
#size-cells = <1>;
ranges;
interrupt-controller;
#interrupt-cells = <2>;
interrupts-extended =
#if GIC
/* GICINT0 */ <&gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
#elif SSP
/* SSPINT0 */ <&ssp_nvic 0 0>,
#else /* TSP */
/* TSPINT0 */ <&tsp_nvic 0 0>,#endif
/* ... */
#if GIC_128 /* Route merged 128-159 interrupts to GICINT128 */
/* 128 */ <&gic GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>,
#elif GIC_160 /* Route merged 128-159 interrupts to GICINT160 */
/* 128 */ <&gic GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
#elif GIC_176 /* Route merged 128-159 interrupts to GICINT176 */
/* 128 */ <&gic GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>,
#elif SSP
/* 128 */ <&ssp_nvic 128 0>,
#else /* TSP */
/* 128 */ <&tsp_nvic 128 0>,#endif
/* ... */
/* 186 */ <&gic 186 0>;
intcm0: interrupt-controller@12101b00 {
compatible = "aspeed,ast2700-intcm-a1";
reg = <0x12101b00 0x10>;
interrupt-controller;
#interrupt-cells = <2>;
interrupt-parent = <&gic>;
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>;
<GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 201 IRQ_TYPE_LEVEL_HIGH>;
};
intcm30: interrupt-controller@12101b10 {
compatible = "aspeed,ast2700-intcm-a1";
reg = <0x12101b10 0x10>;
interrupt-controller;
#interrupt-cells = <2>;
interrupt-parent = <&gic>;
interrupts =
<GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 211 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 212 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 213 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 214 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 216 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 217 IRQ_TYPE_LEVEL_HIGH>;
};
intcm40: interrupt-controller@12101b20 {
compatible = "aspeed,ast2700-intcm-a1";
reg = <0x12101b20 0x10>;
interrupt-controller;
#interrupt-cells = <2>;
interrupt-parent = <&gic>;
interrupts =
<GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 227 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 228 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 230 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 232 IRQ_TYPE_LEVEL_HIGH>;
<GIC_SPI 233 IRQ_TYPE_LEVEL_HIGH>;
};
};
intc1: interrupt-controller@14c18000 {
compatible = "aspeed,ast2700-intc1-a1";
reg = <0x14c18000 0x1000>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts-extended =
#if M0
/* 0 */ <&intcm0 0 IRQ_TYPE_LEVEL_HIGH>,
#elif C0
/* 0 */ <&intc0 128 IRQ_TYPE_LEVEL_HIGH>,
#elif M10
/* 0 */ <&ssp_nvic 160 0>,
#elif M20
/* 0 */ <&tsp_nvic 160 0>,
#elif M30
/* 0 */ <&intcm30 0 IRQ_TYPE_LEVEL_HIGH>,
#elif M40
/* 0 */ <&intcm40 0 IRQ_TYPE_LEVEL_HIGH>,
#else /* B */
/* 0 */ <&aplic 128 IRQ_TYPE_LEVEL_HIGH>,
#endif
/* ... */
};
vuart1: serial@1e787000 {
compatible = "aspeed,ast2700-vuart";
reg = <0x14c30000 0x1000>;
reg-shift = <2>;
interrupts-extended = <&intc1 17 IRQ_TYPE_LEVEL_HIGH>;
};
gic: interrupt-controller@fff01000 {
compatible = "arm,gic-400";
#interrupt-cells = <3>;
#address-cells = <0>;
interrupt-controller;
reg = <0x0 0xfff01000 0 0x1000>,
<0x0 0xfff02000 0 0x2000>,
<0x0 0xfff04000 0 0x2000>,
<0x0 0xfff06000 0 0x2000>;
interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
};};
secondary {
#address-cells = <2>;
/* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/of/address.c?h=v6.16#n491 */
#size-cells = <0>;
/* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/of/address.c?h=v6.16#n430 */ ssp_nvic: interrupt-controller@1,e000e100 {
compatible = "arm,v7m-nvic";
#interrupt-cells = <2>;
#address-cells = <0>;
interrupt-controller;
reg = <1 0xe000e100>;
};};
tertiary {
#address-cells = <2>;
#size-cells = <0>; tsp_nvic: interrupt-controller@2,e000e100 {
compatible = "arm,v7m-nvic";
#interrupt-cells = <2>;
#address-cells = <0>;
interrupt-controller;
reg = <2 0xe000e100>;
};};
bootmcu {
#address-cells = <2>;
#size-cells = <0>; aplic1: interrupt-controller@3,d000000 {
compatible = "riscv,aplic";
interrupts-extended = <&cpu1_intc 9>,
reg = <3 0xd000000>;
interrupt-controller;
#interrupt-cells = <2>;
riscv,num-sources = <480>;
}; };
};
Ryan Chen
reviewerRE: [PATCH v4 1/2] dt-bindings: interrupt-controller: aspeed: Add parent compatibles and refine documentation
Quoted 231 lines
Subject: Re: [PATCH v4 1/2] dt-bindings: interrupt-controller: aspeed: Add parent compatibles and refine documentation
Quoted 65 lines · Krzysztof Kozlowski
Quoted 42 lines · Ryan Chen
AST2700 contains two independent top-level interrupt controllers (INTC0, INTC1). Each occupies its own register space and handles different sets of peripherals. Above them, the PSP (CA35) GIC is the root interrupt aggregator. In hardware, INTC1 outputs are routed into INTC0, and INTC0 outputs are routed into the GIC.
Introduce distinct compatibles for these parent blocks so the DT can model the hierarchy and register space layout accurately:
- aspeed,ast2700-intc0 (parent node at
0x12100000)
- aspeed,ast2700-intc1 (parent node at0x14c18000)The existing child compatible:
- aspeed,ast2700-intc-ic
continues to describe the interrupt-controller instances within each INTC block (e.g. INTC0_0..INTC0_11 and INTC1_0..INTC1_5).
Signed-off-by Ryan Chen <[email protected]>---
.../aspeed,ast2700-intc.yaml | 158
+++++++++++++-----1 file changed, 115 insertions(+), 43 deletions(-)(quoted diff) +0
-0
controller/aspeed,ast2700-intc.yamlb/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
index55636d06a674..81304b53c112100644
---
a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed, +++ast2700-intc.yaml(quoted diff) +0
-0@@ -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.Quoted 8 lines
+ 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 |
+ +-----+ +---------+Same problem as last time. This tells me intc0 has not children...
Quoted 10 lines
+ +---------+
+ | |---module0 + | INTC0_0 |---module1 + | |---... + +---------+---module31 + |---.... | + +---------+ + | | +---------+ + | INTC0_11| +---| INTC1 | + | | +---------+Quoted 14 lines
...This tells that inc1 has no children (only
intc0_11, which you said is aspeed,ast2700-intc-ic !!!)....
(keep scrolling)Quoted 10 lines
+ +---------+ +---------+---module0 + |
INTC1_0|---module1 + | |---... + +---------+---module31 + ...
+ +---------+---module0 + |INTC1_5|---module1 + | |---... + +---------+---module31I've taken a look at the datasheet and had a bit of a think about how to make progress here.
My feeling is this diagram (and the previous one) undersells the complexity of the design by quite some margin. It's probably best to start by zooming out quite a lot:
The AST2700 SoC contains (at least) 4 distinct processors:
1. The quad-core ARM Cortex-A35 (PSP: Primary Service Processor) 2. A Cortex-M4 (SSP: Secondary Service Processor) 3. Another Cortex-M4 (TSP:
Tertiary Service Processor) 4. The BootMCU - a RISC-V processor to execute the mask ROMWhile the PSP GIC shown in the diagram above is one possible interrupt destination, many of the 480 interrupt sources in the package can be routed to the dedicated interrupt controller of any of these four processors. Likewise, many peripherals of the SoC are mapped into the physical address space of each processor. The routing is handled by the two interrupt controller blocks described in the binding text above:
+--------------------+ | +-------+ +------+ | | | | | | | | | PSP +-+ GIC <-+----------+ | | | | | | | | +-------+ +------+ | | +--------------------+ | | +--------------------+ | | +-------+ +------+ | | +-------+ | | | | | | | | | INTx | | SSP +-+ NVIC <-+----------+----------| INTC0 <----------- | | | | | | | | | | +-------+ +------+ | | +---^---+ +--------------------+ | | | | +--------------------+ | | | +-------+ +------+ | | | | | | | | | | | | | TSP +-+ NVIC <-+----------+ | | | | | | | | | +-------+ +------+ | | +--------------------+ | | +--------------------+ | | +------+ +-------+ | +---+---+ | | | | | | | | INTy | | BMCU +-+ APLIC <-+---------------------+ INTC1 <----------- | | | | | | | | | +------+ +-------+ | +-------+ +--------------------+There's a split in the interrupt domain: 0 <= INTx <= 127 < INTy.
The PSP GIC, SSP NVIC and TSP NVIC destinations for each INTx source is selected by a corresponding mux in INTC0.
The destination for each INTy source is selected by a corresponding mux in INTC1, where the possible destinations are:
1. A shared interrupt line Ma, routed through INTC0 to the PSP GIC 2. A mux in INTC0, providing a second level of indirection 3. A shared interrupt line Md, routed through INTC0 to the PSP GIC 4. A shared interrupt line Me, routed through INTC0 to the PSP GIC 5. A shared interrupt line Mb, routed through INTC0 to the SSP NVIC 6. A shared interrupt line Mc, routed through INTC0 to the TSP NVIC 7. The BootMCU APLIC
Each shared interrupt line from INTC1 to INTC0 merges up to 32 interrupt sources.
INTC0 and INTC1 are both mapped into the physical address space of each processor. However, to prevent any one of them interfering with interrupts dedicated to another, the controller's register map is divided into distinct regions whose access is constrained to a specific processor. The enable and status registers in INTC0 for shared line sets Ma, Md and Me are constrained to the PSP, Mb to the SSP, and Mc to the TSP.
+-------+ +-------+ | INTC0 | | INTC1 | - - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- - - -+- - +--------------------+ | | | | | +-------+ +------+ | | | | | | | | | | | | | | | | | PSP +-+ GIC | | | | | | | | | | | | | | | | | +-------+ | | | | | | | | | | | +-------+ | | | | 192 <-|-----------+ Ma <------------|---+ | | | | | +-------+ | | | | | 208 <-|-----------+ Md <------------|---+ | | | | | +-------+ | | | | | 224 <-|-----------+ Me <------------|---+ | | | | | +-------+ | | | | +------+ | | | | | | +--------------------+ | | | | | - - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- -|- -+- - +--------------------+ | | | | | | +-------+ +------+ | +-------+ | +---|------------ | | SSP +-+ NVIC <-|-----------+ Mb <------------|---+ | INT128 | +-------+ +------+ | +-------+ | | | +--------------------+ | | | | | - - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- -|- -+- - +--------------------+ | | | | | | +-------+ +------+ | +-------+ | | | | | TSP +-+ NVIC <-|-----------+ Mc <------------|---+ | | +-------+ +------+ | +-------+ | | +--------------------+ | | | | - - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- - - -+- - +--------------------+ | | | | | +------+ +-------+ | | | | | | | BMCU +-+ APLIC | | | | | | | +------+ +-------+ | | | | | +--------------------+ | | | | - - - - - - - - - - - - - - - - - -+- - - -+- - - - - - +- - - -+- - +-------+ +-------+INTC1 mux destination 2 above allows the PSP to reconfigure the routes at runtime at INTC0, rather than submit to a route that might've been protected by firmware early in the boot chain, so long as the boot firmware chose the appropriate route in INTC1. INTC0 merges these interrupts in sets of 32 and operates in much the same way as INTC1 in the diagram above, though cannot route to the BootMCU.
I'd prefer we acknowledge all this in the binding, and do enough to allow the kernel to configure routing as it wishes. While in some applications the routes may be constrained by firmware, the platform- specific portion of the devicetree can be written reflect this.
From some experimentation this shouldn't require anything bespoke. Using interrupts-extended we can represent the route configuration with a phandle to the upstream controller on the cascaded node. To avoid some arbitrary interrupt index choices at the node for INTC0, I think it's worth describing the register sets for interrupt sets Ma, Md and Me as subnodes of INTC0 with their own interrupt resources. This feels reasonably tidy, as the selection of the Ma, Md or Me sets completely determines its ultimate index at the PSP GIC. Doing so also removes them from needing to be described if any changes to some default route configuration are required by the platform, necessitating overriding the interrupts-extended property of the INTC0 node.
The only curiosity of this approach is that the interrupt-controller nodes for the non-PSP processors need to be described so we can reference them via phandles for the purpose of routing the interrupts.
As these controllers are not mapped in the physical address space of the PSP we need the devicetree to inform the kernel as much.
Quoted 6 lines
Here's an example pseudo-devicetree. Of course there are elements that need more work, but I feel we can mine it for parts.
Cheers,
Andrew
Hello Andrew,
Thanks a lot for diagram draw. I agree with your point.
The binding must reflect the full routing capability of INTC0/INTC1, not just a simple parent/child split.
So that the reason I add a driver for routing in "aspeed,ast2700-intc0/1".
https://lore.kernel.org/all/[email protected]/
- aspeed,ast2700-intc0 (parent node at 0x12100000) - routing driver included
- aspeed,ast2700-intc-ic 0x1b00
- aspeed,ast2700-intc-ic 0x1b10
- aspeed,ast2700-intc-ic 0x1b20
- aspeed,ast2700-intc1 (parent node at 0x14c18000) - routing driver included
- aspeed,ast2700-intc-ic 0x100
- aspeed,ast2700-intc-ic 0x110
- aspeed,ast2700-intc-ic 0x120
- aspeed,ast2700-intc-ic 0x130
- aspeed,ast2700-intc-ic 0x140
- aspeed,ast2700-intc-ic 0x150
Ryan Chen
Quoted 191 lines
/ {
primary { compatible = "simple-bus"; #address-cells = <1>; #size-cells = <1>; ranges; intc0: interrupt-controller@12100000 { compatible = "aspeed,ast2700-intc0-a1"; reg = <0x12100000 0x1b00>; #address-cells = <1>; #size-cells = <1>; ranges; interrupt-controller; #interrupt-cells = <2>; interrupts-extended = #if GIC /* GICINT0 */ <&gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>, #elif SSP /* SSPINT0 */ <&ssp_nvic 0 0>, #else /* TSP */ /* TSPINT0 */ <&tsp_nvic 0 0>,#endif
/* ... */#if GIC_128 /* Route merged 128-159 interrupts to GICINT128 */ /* 128 */ <&gic GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>, #elif GIC_160 /* Route merged 128-159 interrupts to GICINT160 */ /* 128 */ <&gic GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>, #elif GIC_176 /* Route merged 128-159 interrupts to GICINT176 */ /* 128 */ <&gic GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, #elif SSP /* 128 */ <&ssp_nvic 128 0>, #else /* TSP */ /* 128 */ <&tsp_nvic 128 0>,#endif
/* ... */ /* 186 */ <&gic 186 0>; intcm0: interrupt-controller@12101b00 { compatible = "aspeed,ast2700-intcm-a1"; reg = <0x12101b00 0x10>; interrupt-controller; #interrupt-cells = <2>; interrupt-parent = <&gic>; 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>; <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 201 IRQ_TYPE_LEVEL_HIGH>; }; intcm30: interrupt-controller@12101b10 { compatible = "aspeed,ast2700-intcm-a1"; reg = <0x12101b10 0x10>; interrupt-controller; #interrupt-cells = <2>; interrupt-parent = <&gic>; interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 211 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 212 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 213 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 214 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 216 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 217 IRQ_TYPE_LEVEL_HIGH>; }; intcm40: interrupt-controller@12101b20 { compatible = "aspeed,ast2700-intcm-a1"; reg = <0x12101b20 0x10>; interrupt-controller; #interrupt-cells = <2>; interrupt-parent = <&gic>; interrupts = <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 227 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 228 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 230 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 232 IRQ_TYPE_LEVEL_HIGH>; <GIC_SPI 233 IRQ_TYPE_LEVEL_HIGH>; }; }; intc1: interrupt-controller@14c18000 { compatible = "aspeed,ast2700-intc1-a1"; reg = <0x14c18000 0x1000>; interrupt-controller; #interrupt-cells = <2>; interrupts-extended = #if M0 /* 0 */ <&intcm0 0 IRQ_TYPE_LEVEL_HIGH>, #elif C0 /* 0 */ <&intc0 128 IRQ_TYPE_LEVEL_HIGH>, #elif M10 /* 0 */ <&ssp_nvic 160 0>, #elif M20 /* 0 */ <&tsp_nvic 160 0>, #elif M30 /* 0 */ <&intcm30 0 IRQ_TYPE_LEVEL_HIGH>, #elif M40 /* 0 */ <&intcm40 0 IRQ_TYPE_LEVEL_HIGH>, #else /* B */ /* 0 */ <&aplic 128 IRQ_TYPE_LEVEL_HIGH>, #endif /* ... */ }; vuart1: serial@1e787000 { compatible = "aspeed,ast2700-vuart"; reg = <0x14c30000 0x1000>; reg-shift = <2>; interrupts-extended = <&intc1 17 IRQ_TYPE_LEVEL_HIGH>; }; gic: interrupt-controller@fff01000 { compatible = "arm,gic-400"; #interrupt-cells = <3>; #address-cells = <0>; interrupt-controller; reg = <0x0 0xfff01000 0 0x1000>, <0x0 0xfff02000 0 0x2000>, <0x0 0xfff04000 0 0x2000>, <0x0 0xfff06000 0 0x2000>; interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>; };};
secondary { #address-cells = <2>; /* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/ of/address.c?h=v6.16#n491 */ #size-cells = <0>; /*https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/
of/address.c?h=v6.16#n430 */ssp_nvic: interrupt-controller@1,e000e100 { compatible = "arm,v7m-nvic"; #interrupt-cells = <2>; #address-cells = <0>; interrupt-controller; reg = <1 0xe000e100>; };};
tertiary { #address-cells = <2>; #size-cells = <0>;tsp_nvic: interrupt-controller@2,e000e100 { compatible = "arm,v7m-nvic"; #interrupt-cells = <2>; #address-cells = <0>; interrupt-controller; reg = <2 0xe000e100>; };};
bootmcu { #address-cells = <2>; #size-cells = <0>;aplic1: interrupt-controller@3,d000000 { compatible = "riscv,aplic"; interrupts-extended = <&cpu1_intc 9>, reg = <3 0xd000000>; interrupt-controller; #interrupt-cells = <2>; riscv,num-sources = <480>; };};
};
Ryan Chen
reviewerPATCH v4 2/2 Irqchip/ast2700-intc: add debugfs support and AST2700 INTC0/INTC1 routing/protection display
In reply to Ryan ChenAST2700 INTC0/INTC1 nodes ("aspeed,ast2700-intc0/1") not only include the interrupt controller child node ("aspeed,ast2700-intc-ic"), but also provide interrupt routing and register protection features. Adds debugfs entries for interrupt routing and protection status for AST2700 INTC0/INTC1.
--- drivers/irqchip/Kconfig | 6 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-ast2700-intc.c | 174 +++++++++++++++++++++++++++++ 3 files changed, 181 insertions(+) create mode 100644 drivers/irqchip/irq-ast2700-intc.c
drivers/irqchip/Kconfig +6 -0
@@ -97,6 +97,12 @@ config AL_FIC help Support Amazon's Annapurna Labs Fabric Interrupt Controller. +config AST2700_INTC + tristate "AST2700 Interrupt Controller" + depends on ARCH_ASPEED + help + Support AST2700 Interrupt Controller. + config ATMEL_AIC_IRQ bool select GENERIC_IRQ_CHIP
drivers/irqchip/Makefile +1 -0
@@ -88,6 +88,7 @@ obj-$(CONFIG_LS_EXTIRQ) += irq-ls-extirq.o obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o irq-aspeed-scu-ic.o obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-intc.o +obj-$(CONFIG_AST2700_INTC) += irq-ast2700-intc.o obj-$(CONFIG_STM32MP_EXTI) += irq-stm32mp-exti.o obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o
drivers/irqchip/irq-ast2700-intc.c new file +174 -0
174 lines not on this page · raw message
Signature · 1 line
2.34.1