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: <1a2ca78746e00c2ec4bfc2953a897c48376ed36f.camel@codeconstruct.com.au>
Date: Tue, 09 Sep 2025 18:11:19 +0930
From: Andrew Jeffery <andrew@...econstruct.com.au>
To: Krzysztof Kozlowski <krzk@...nel.org>, Ryan Chen
 <ryan_chen@...eedtech.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski
	 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Joel Stanley
	 <joel@....id.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 v4 1/2] dt-bindings: interrupt-controller: aspeed: Add
 parent compatibles and refine documentation

On Thu, 2025-08-14 at 10:03 +0200, Krzysztof Kozlowski wrote:
> On Tue, Aug 12, 2025 at 06:08:29PM +0800, Ryan Chen wrote:
> > 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).
> > 
> > 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..81304b53c112 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  |
> > +  +-----+   +---------+
> 
> Same problem as last time. This tells me intc0 has not children...
> 
> > +            +---------+
> > +            |         |---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)
> 
> > +            +---------+     +---------+---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@...00000 {
      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@...01b00 {
        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@...01b10 {
        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@...01b20 {
        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@...18000 {
      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@...87000 {
      compatible = "aspeed,ast2700-vuart";
      reg = <0x14c30000 0x1000>;
      reg-shift = <2>;
      interrupts-extended = <&intc1 17 IRQ_TYPE_LEVEL_HIGH>;
    };

    gic: interrupt-controller@...01000 {
      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>;
    };
  };
};

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ