| 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
| ||
|
Message-ID:
<TY2PPF5CB9A1BE6CDE8AF88A638BEC06B41F2FFA@TY2PPF5CB9A1BE6.apcprd06.prod.outlook.com>
Date: Sun, 26 Oct 2025 03:57:06 +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>,
"jk@...econstruct.com.au" <jk@...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 v5 1/3] dt-bindings: interrupt-controller: aspeed,ast2700:
Add support for INTC hierarchy
> Subject: Re: [PATCH v5 1/3] dt-bindings: interrupt-controller: aspeed,ast2700:
> Add support for INTC hierarchy
>
> On Thu, Oct 23, 2025 at 06:57:01AM +0000, Ryan Chen wrote:
> > Hello Rob.
> > Thank you for your detailed review and comments.
> >
> > > Subject: Re: [PATCH v5 1/3] dt-bindings: interrupt-controller:
> aspeed,ast2700:
> > > Add support for INTC hierarchy
> > >
> > > On Wed, Oct 22, 2025 at 02:55:05PM +0800, Ryan Chen wrote:
> > > > AST2700 contains two-level interrupt controllers (INTC0 and
> > > > INTC1), each with its own register space and handling different
> > > > sets of peripherals.
> > >
> > > This is a mess!
> > >
> > > How does this relate to the existing "aspeed,ast2700-intc-ic"? Its
> > > schema has a block diagram of connections which I can understand. This
> does not.
> > >
> > > The use of child nodes here is questionable. A variable number of
> > > interrupt banks is not a reason to have child nodes. I'm only
> > > guessing that's what's happening here because you haven't explained it.
> >
> > Let me clarify the hardware structure and the purpose of these bindings.
> >
> > The AST2700 SoC includes two top-level interrupt controller modules,
> > INTC0 and INTC1. (aspeed,ast2700-intc0, aspeed,ast2700-intc1) Each of
> > them provides routing selection and register protection features.
> > Within each INTCx block, there are multiple sub-blocks called intc-ic,
> > each handling multi-interrupt sources.
> > ("aspeed,ast2700-intc0-ic", "aspeed,ast2700-intc1-ic")
> >
> > Cascading occurs between the child banks:
> > Level 1 : intc0-ic have multi-interrupts connect to GIC (root) Level 2
> > : multi Intc1-ic# connect to intc0-ic The parent intc0/1 nodes expose
> > register regions for routing and protection control, serving as
> > containers for their intc-ic children.
>
> Being a 2nd vs. 3rd level interrupt controller is not a reason for different
> compatibles. The programming model is obviously the same for both as you
> essentially have 0 driver changes. Having N banks of 32 interrupts vs. 1 bank of
> 32 interrupts is not a reason to have multiple intcN-ic nodes. That is a very
> common difference between instances of the same interrupt controller such as
> the GIC.
>
> What you need to do is simply extend your driver to support N banks of
> 32 interrupts. That's what almost every other irqchip driver with more than 32
> interrupts does. If you are lucky, then the offset to each bank's registers is just
> hwirq/32 * <bank stride> and the number of banks can be calculated from the
> length of 'reg'. If you are not lucky, then you could put 1 'reg' entry for each
> bank.
>
> AFAICT, the existing binding in aspeed,ast2700-intc.yaml should work for you.
>
> >
> > The following simplified diagram shows the hierarchy:
> >
> >
> > +----------+ +----------+
> > | intc0 | | intc1 |
> > - - - - - - - - - - - - - - - - -+---- -----+- - - +------ - -+
> > +-----------------------+ | | | |
> > | +-------+ +---------+ | | | | |
> > | | | | | | | | | |
> > | | PSP +-+ GIC | | | | | |
> > | | | | | | | | | |
> > | +-------+ | | | | | | |
> > | | | | +----------+ | |
> > | | 192~201 <-|------+ <-------+ intc1-ic |
> > | +---------+ | | | | |
> > +-----------------------+ | intc0-ic <-------+ intc1-ic |
> > | | |
> |
> > | <-------+ intc1-ic |
> > +----------+ .....
>
> You already match on intc0 and handle 32 interrupts. Now you are adding
> intc0-ic to match on and handling the same 32 interrupts?
Thank your guidance, understood your point. the currently is met requirement.
For currently aspeed,ast2700-intc.yaml I still need update following.
#1. Interrupts:
- maxItems: 6
+ minItems: 1
+ maxItems: 10
The 1 level request multi-interrupt to root GIC, the max is 10.
2nd level only request 1 to level 1 intc-ic.
in level 1: will be need 10, 2nd level only need 1.
Level1 :
intc0_11: interrupt-controller@...01b00 {
compatible = "aspeed,ast2700-intc-ic";
reg = <0x0 0x12101b00 0x0 0x10>;
#interrupt-cells = <1>;
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>;
};
Level 2:
intc1_0: interrupt-controller@...18100 {
compatible = "aspeed,ast2700-intc-ic";
reg = <0x0 0x14c18100 0x0 0x10>;
#interrupt-cells = <1>;
interrupt-controller;
interrupts-extended = <&intc0_11 0>;
};
#2. '#interrupt-cells':
-const: 2
+const: 1
Due to the driver irq-aspeed-intc.c not support any trigger type.
>
> Rob
Powered by blists - more mailing lists