diff mbox series

[v2,1/2] dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC

Message ID 20240814114106.2809876-3-kevin_chen@aspeedtech.com
State Changes Requested
Headers show
Series Add support for AST2700 INTC driver | expand

Checks

Context Check Description
robh/checkpatch warning total: 3 errors, 1 warnings, 71 lines checked
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Kevin Chen Aug. 14, 2024, 11:41 a.m. UTC 
The ASPEED AST27XX interrupt controller(INTC) contain second level and
third level interrupt controller. The third level INTC combines 32 interrupt
sources into 1 interrupt into parent interrupt controller. The second
level INTC doing hand shake with third level INTC.
---
 .../aspeed,ast2700-intc.yaml                  | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml

Comments

Krzysztof Kozlowski Aug. 14, 2024, 2:04 p.m. UTC | #1 
On 14/08/2024 13:41, Kevin Chen wrote:
> The ASPEED AST27XX interrupt controller(INTC) contain second level and
> third level interrupt controller. The third level INTC combines 32 interrupt
> sources into 1 interrupt into parent interrupt controller. The second
> level INTC doing hand shake with third level INTC.


> +maintainers:
> +  - Kevin Chen <kevin_chen@aspeedtech.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - aspeed,ast2700-intc-ic
> +
> +  reg:
> +    minItems: 1

That's unconstrained. Instead: maxItems: 1

> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 2
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 10
> +    description:
> +      It contains two types of interrupt controller. The first type is multiple
> +      interrupt sources into parent interrupt controller. The second type is 
> +      1 interrupt source to parent interrupt controller.

I think I asked already - list the items with description.

Why the number is flexible?

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-controller
> +  - '#interrupt-cells'
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +    
> +        interrupt-controller@12101b00 {
> +          compatible = "aspeed,ast2700-intc-ic";

Messed indentation.



Best regards,
Krzysztof
Rob Herring (Arm) Aug. 18, 2024, 3:20 p.m. UTC | #2 
On Wed, Aug 14, 2024 at 07:41:05PM +0800, Kevin Chen wrote:
> The ASPEED AST27XX interrupt controller(INTC) contain second level and
> third level interrupt controller. The third level INTC combines 32 interrupt
> sources into 1 interrupt into parent interrupt controller. The second
> level INTC doing hand shake with third level INTC.

Missing Signed-off-by. checkpatch.pl also reports trailing whitespace.
Kevin Chen Oct. 7, 2024, 10:48 a.m. UTC | #3 
> > The ASPEED AST27XX interrupt controller(INTC) contain second level and
> > third level interrupt controller. The third level INTC combines 32
> > interrupt sources into 1 interrupt into parent interrupt controller.
> > The second level INTC doing hand shake with third level INTC.
> 
> 
> > +maintainers:
> > +  - Kevin Chen <kevin_chen@aspeedtech.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - aspeed,ast2700-intc-ic
> > +
> > +  reg:
> > +    minItems: 1
> 
> That's unconstrained. Instead: maxItems: 1
Agree.

> 
> > +
> > +  interrupt-controller: true
> > +
> > +  '#interrupt-cells':
> > +    const: 2
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 10
> > +    description:
> > +      It contains two types of interrupt controller. The first type is multiple
> > +      interrupt sources into parent interrupt controller. The second type is
> > +      1 interrupt source to parent interrupt controller.
> 
> I think I asked already - list the items with description.
> 
> Why the number is flexible?
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(#192~#197) if interrupt in INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in one INTC0.
INTC1 is used to assert INTC0 if interrupt of modules asserted. There are 32 module interrupts used in one INTC1.

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupt-controller
> > +  - '#interrupt-cells'
> > +  - interrupts
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    bus {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        interrupt-controller@12101b00 {
> > +          compatible = "aspeed,ast2700-intc-ic";
> 
> Messed indentation.
Agree. Would change to the following.
examples:
  - |
    #include <dt-bindings/interrupt-controller/arm-gic.h>

    bus {
        #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>,
                         <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>;
        };
    };

> 
> Best regards,
> Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
new file mode 100644
index 000000000000..9a76d5c3b66b
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml
@@ -0,0 +1,71 @@ 
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aspeed AST2700 Interrupt Controller
+
+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.
+
+maintainers:
+  - Kevin Chen <kevin_chen@aspeedtech.com>
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2700-intc-ic
+
+  reg:
+    minItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+
+  interrupts:
+    minItems: 1
+    maxItems: 10
+    description:
+      It contains two types of interrupt controller. The first type is multiple
+      interrupt sources into parent interrupt controller. The second type is 
+      1 interrupt source to parent interrupt controller.
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - '#interrupt-cells'
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    bus {
+        #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>,
+                       <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>;
+        };
+    };