Re: [PATCH v1 12/18] hw/arm/aspeed_ast27x0: Support two levels of INTC controllers for AST2700 A1

[Andrew Jeffery]

On Tue, 2025-02-04 at 09:43 +0000, Jamin Lin wrote:
> Hi Andrew,
> 
> > -----Original Message-----
> > From: Andrew Jeffery <andrew@codeconstruct.com.au>
> > Sent: Thursday, January 30, 2025 12:20 PM
> > To: Jamin Lin <jamin_lin@aspeedtech.com>; Cédric Le Goater <clg@kaod.org>;
> > Peter Maydell <peter.maydell@linaro.org>; Steven Lee
> > <steven_lee@aspeedtech.com>; Troy Lee <leetroy@gmail.com>; Joel Stanley
> > <joel@jms.id.au>; open list:ASPEED BMCs <qemu-arm@nongnu.org>; open
> > list:All patches CC here <qemu-devel@nongnu.org>
> > Cc: Troy Lee <troy_lee@aspeedtech.com>; Yunlin Tang
> > <yunlin.tang@aspeedtech.com>
> > Subject: Re: [PATCH v1 12/18] hw/arm/aspeed_ast27x0: Support two levels of
> > INTC controllers for AST2700 A1
> > 
> > On Tue, 2025-01-21 at 15:04 +0800, Jamin Lin wrote:
> > > The design of INTC controllers has significantly changed in AST2700 A1.
> > > 
> > > There are a total of 480 interrupt sources in AST2700 A1. For
> > > interrupt numbers from 0 to 127, they can route directly to PSP, SSP,
> > > and TSP. Due to the limitation of interrupt numbers of processors, the
> > > interrupts are merged every
> > > 32 sources for interrupt numbers greater than 127.
> > > 
> > > There are two levels of interrupt controllers, INTC0 and INTC1. The
> > > interrupt sources of INTC0 are the interrupt numbers from INTC_0 to
> > > INTC_127 and interrupts from INTC1. The interrupt sources of INTC1 are
> > > the interrupt numbers greater than INTC_127. INTC1 controls the interrupts
> > INTC_128 to INTC_319 only.
> > > 
> > > Currently, only GIC 192 to 201 are supported, and their source
> > > interrupts are from INTC1 and connected to INTC0 at input pin 0 and
> > > output pins 0 to 9 for GIC 192-201.
> > > 
> > > To support both AST2700 A1 and A0, INTC0 input pins 1 to 9 and output
> > > pins
> > > 10 to 18 remain to support GIC 128-136, which source interrupts from 
> > > INTC0.
> > > These will be removed if we decide not to support AST2700 A0 in the 
> > > future.
> > > 
> > > +---------------------------------------------------------------------------------------+
> > > >                            AST2700 A1
> > Design
> > > > > 
> > > 
> > > 
> > 
> > > > > 
> > > 
> > >                     +--------------------------+
> > 
> > > > > 
> > > 
> > >                     |         INTC1            |
> > 
> > > > +---------------+             |
> > > 
> > >                     |                          |
> >       |
> > > > orgates[0]   |             |
> > > >    orgates[0]+----> |inpin[0]+------->outpin[0]+------> |
> > 0
> > > > >              |
> > > >    orgates[1]|----> |inpin[1]|------->outpin[1]|------> | 1   0-31
> > > > bits +--+          |
> > > >    orgates[2]|----> |inpin[2]|------->outpin[2]|------> |
> > 2
> > > > >   |          |
> > > >    orgates[3]|----> |inpin[3]|------->outpin[3]|------> |
> > 3
> > > > >   |          |
> > > >    orgates[4]|----> |inpin[4]|------->outpin[4]|------> |
> > 4
> > > > >   |          |
> > > >    orgates[5]+----> |inpin[5]+------->outpin[5]+------> |
> > 5
> > > > >   |          |
> > > 
> > >                     |                          |
> > 
> > > > > ---------------|  |          |
> > > 
> > >                     +--------------------------+
> > 
> > > > >           |
> > > > 
> > > > +-------------------------------------------------------------------
> > > > ----|          |
> > > 
> > >    |
> > 
> > > > > 
> > > 
> > >    |
> > 
> > > > > 
> > > >    |                +------------------------------+
> > > > +-----------------+    |
> > > 
> > >    |                |            INTC0             |
> >            |
> > > > GIC         |    |
> > > >    |                |inpin[0:0]--------->outpin[0] +--------->
> > > 192
> > > > >     |
> > > >    |                |inpin[0:1]|-------->outpin[1] |--------->
> > > 193
> > > > >     |
> > > >    |                |inpin[0:2]|-------->outpin[2] |--------->
> > > 194
> > > > >     |
> > > >    |                |inpin[0:3]|-------->outpin[3] |--------->
> > > 195
> > > > >     |
> > > >    >--------------> |inpin[0:4]|-------->outpin[4] |--------->
> > > 196
> > > > >     |
> > > >                     |inpin[0:5]|-------->outpin[5] |--------->
> > > 197
> > > > >     |
> > > >                     |inpin[0:6]|-------->outpin[6] |--------->
> > > 198
> > > > >     |
> > > >                     |inpin[0:7]|-------->outpin[7] |--------->
> > > 199
> > > > >     |
> > > >                     |inpin[0:8]|-------->outpin[8] |--------->
> > > 200
> > > > >     |
> > > >                     |inpin[0:9]|-------->outpin[9] |--------->
> > > 201
> > > > >     |
> > > +---------------------------------------------------------------------------------------+
> > > +---------------------------------------------------------------------------------------+
> > > >   orgates[1]|-----> |inpin[1]|---------->outpin[10]|--------->
> > > 128
> > > > >     |
> > > >   orgates[2]|-----> |inpin[2]|---------->outpin[11]|--------->
> > > 129
> > > > >     |
> > > >   orgates[3]|-----> |inpin[3]|---------->outpin[12]|--------->
> > > 130
> > > > >     |
> > > >   orgates[4]|-----> |inpin[4]|---------->outpin[13]|--------->
> > > 131
> > > > >     |
> > > >   orgates[5]|-----> |inpin[5]|---------->outpin[14]|--------->
> > > 132
> > > > >     |
> > > >   orgates[6]|-----> |inpin[6]|---------->outpin[15]|--------->
> > > 133
> > > > >     |
> > > >   orgates[7]|-----> |inpin[7]|---------->outpin[16]|--------->
> > > 134
> > > > >     |
> > > >   orgates[8]|-----> |inpin[8]|---------->outpin[17]|--------->
> > > 135
> > > > >     |
> > > >   orgates[9]+-----> |inpin[9]|---------->outpin[18]+--------->
> > > 136
> > > > >     |
> > > >                     +------------------------------+
> > > > +-----------------+    |
> > > 
> > > 
> > 
> > > > > 
> > > >                     AST2700 A0
> > Design
> > 
> > > > > 
> > > 
> > > 
> > 
> > > > > 
> > > +---------------------------------------------------------------------------------------+
> > > 
> > 
> > Okay, so I think this is the diagram and discussion I asked for as 
> > documentation
> > earlier. I still prefer it doesn't just live in a commit message, that you 
> > pull it out
> > to a separate document that we can easily point to and evolve.
> > 
> INTC drivers owners update DTS binding document here, 
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20241016022410.1154574-2-kevin_chen@aspeedtech.com/

The diagram there is relatively straight-forward, but I suspect the
devil is in the details.

> 
> 
> > I'm a little hazy on some of your notation in diagram though. Can you 
> > explain
> > your use of pipes ("|"), plusses ("+"), the "orgates" to the left of INTC1 
> > (what
> > are they ORing?), and the choice of 5 lines into the "orgates[0]" box? Also 
> > why
> > does the "orgates[0]" arrow point where it does on INTC0?
> > 
> I created this block diagram using the asciiflow tool and both ("I") and 
> "("+") symbols are identical.
> I will update the diagram and replace ("+") with ("|").
> Sorry to make you confuse.

Ah, right.

> 
> The design of the OR gates for GICINT 196 is as follows:

196? You discuss 192 below.

> It has interrupt sources ranging from 0 to 31, with its output pin connected 
> to INTC_IO "T0 GICINTC_192".
> The output pin is then connected to INTC_CPU "GIC_192_201" at bit 0, and its 
> bit 0 output should be connected to GIC 192.
> 
> The design of INTC_CPU GIC_192_201 have 10 output pins, mapped as following:
> Bit 0 --> GIC 192
> BIT 1 -> GIC 193
> BIT 2-> GIC 194
> 
> Our firmware only utilizes bits 0 to 5, so I have only illustrated bits 0 to 
> 5 in the diagram. However, I have implemented bits 0 to 9 in the code.

Ah, okay, this is helpful context.

> 
> Then, orgates[0] "or" the output pins of INTC_IO (To GIC192, To GIC193, To 
> GIC194, To GIC195, To GIC196) 
> 
> ETH1    +-----------+
> +--------v+0         3+
>   ETH2    |          4|
> +-------->+1         5|
>   ETH3    |          6|
> +-------->+2        19|
>   UART0   |         20|
> +-------->+7        21|
>   UART1   |         22|
> +-------->+8        23|
>   UART2   |         24|
> +-------->+9        25|
>   UART3   |         26|
> +--------->10       27|
>   UART5   |         28|
> +-------->+11       29|
>   UART6   |           |
> +-------->+12       30|
>   UART7   |         31|
> +-------->+13         |
>   UART8   |  OR[0:31] |
> +--------->14         |
>   UART9   |           |
> +-------->+15         |
>   UART10  |           |
> +-------->+16         |
>   UART11  |           |
> +-------->+17         |
>   UART12  |           |
> +--------->18         |
>           |           |
>           |           |
>           |           |
>           +-----------+
> 
> https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/arch/arm64/boot/dts/aspeed/aspeed-g7.dtsi#L483
> https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/arch/arm64/boot/dts/aspeed/aspeed-g7.dtsi#L1610

Let me absorb this and develop my own understanding in conjunction with
the datasheet. It may take a few days.

Andrew