| 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:
<OS8PR06MB7541981CAE586D85F37FB651F231A@OS8PR06MB7541.apcprd06.prod.outlook.com>
Date: Mon, 18 Aug 2025 05:48:05 +0000
From: Ryan Chen <ryan_chen@...eedtech.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
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>, 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 v4 1/2] dt-bindings: interrupt-controller: aspeed: Add
parent compatibles and refine documentation
> Subject: Re: [PATCH v4 1/2] dt-bindings: interrupt-controller: aspeed: Add
> parent compatibles and refine documentation
>
> 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,ast270
> > 0-intc.yaml
> > b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270
> > 0-intc.yaml index 55636d06a674..81304b53c112 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 |
> > + +-----+ +---------+
>
> 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
> >
> > 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@":
>
> ... but this tells me that intc0 and intc1 has children.
>
> > + 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:
> > + The first cell is the IRQ number, the second cell is the trigger
> > + type.
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 6
> > + description: |
> > + The interrupts provided by this interrupt controller.
> > +
> > + interrupts-extended:
> > + 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.
>
> No, you do not define two. Only interrupts.
Thanks, I did my homework, it is no need.
>
> > +
> > + 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 {
> > + 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>;
>
>
> ... and that's quite wrong unit address. Also no resources in the parent, so this
> entire split seems superficial and incorrect.
Sorry, it is my mistake.
>
> This binding is not improving. You are not responding to REAL problems
> described to you. What's more, you send it in a way making our life difficult,
> look:
Sorry, My intent was to describe the SoC register space more precisely with dtsi,
by having intc0/intc1 as parent nodes and the multiple intc-ic instances as children.
But if this approach is not acceptable, I will drop the parent node split and keep
only the multi-instance aspeed,ast2700-intc-ic nodes, as before.
Please let me know if you think there is still a way to keep the hierarchy useful.
Thanks your guidance.
>
> b4 diff '20250812100830.145578-2-ryan_chen@...eedtech.com'
> Using cached copy of the lookup
> ---
> Analyzing 3 messages in the thread
> Could not find lower series to compare against.
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists