lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ