linux-kernel - Re: [PATCH 2/2] irqchip/aspeed-scu-ic: Add support for AST2700 SCU interrupt controllers

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: <5c46082a-c31b-4d12-a1d6-f315c8ce7aaa@linux.ibm.com>
Date: Wed, 6 Aug 2025 08:41:30 -0500
From: Eddie James <eajames@...ux.ibm.com>
To: Ryan Chen <ryan_chen@...eedtech.com>, 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>,
        "linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
        "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>
Subject: Re: [PATCH 2/2] irqchip/aspeed-scu-ic: Add support for AST2700 SCU
 interrupt controllers


On 8/6/25 2:14 AM, Ryan Chen wrote:
>> Subject: Re: [PATCH 2/2] irqchip/aspeed-scu-ic: Add support for AST2700 SCU
>> interrupt controllers
>>
>> On Mon, Aug 04, 2025 at 01:34:45PM +0800, Ryan Chen wrote:
>>> The AST2700 SoC follows the multi-instance interrupt controller
>>> architecture introduced in the AST2600, where each SCU interrupt group
>>> (IC0–IC3) is treated as an independent interrupt domain.
>>>
>>> Unlike the AST2600, which uses a combined register for interrupt
>>> enable and status bits, the AST2700 separates these into distinct
>>> registers: one for interrupt enable (IER) and another for interrupt
>>> status (ISR). This architectural change requires explicit handling of split
>> registers for interrupt control.
>>> - Register definitions and configuration for AST2700 SCU IC instances
>>>    (compatible: aspeed,ast2700-scu-ic0/1/2/3)
>>> - Initialization logic for handling split IER/ISR registers
>>> - Chained IRQ handling and mask/unmask logic
>>> - Table-driven registration using IRQCHIP_DECLARE per compatible
>>>
>>> Signed-off-by: Ryan Chen <ryan_chen@...eedtech.com>
>>> ---
>>>   drivers/irqchip/irq-aspeed-scu-ic.c | 240
>>> ++++++++++++++++++++++------
>>>   1 file changed, 195 insertions(+), 45 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-aspeed-scu-ic.c
>>> b/drivers/irqchip/irq-aspeed-scu-ic.c
>>> index 1c7045467c48..b6f3ba269c5b 100644
>>> --- a/drivers/irqchip/irq-aspeed-scu-ic.c
>>> +++ b/drivers/irqchip/irq-aspeed-scu-ic.c
>>> @@ -1,6 +1,6 @@
>>>   // SPDX-License-Identifier: GPL-2.0-or-later
>>>   /*
>>> - * Aspeed AST24XX, AST25XX, and AST26XX SCU Interrupt Controller
>>> + * Aspeed AST24XX, AST25XX, AST26XX, AST27XX SCU Interrupt Controller
>>>    * Copyright 2019 IBM Corporation
>>>    *
>>>    * Eddie James <eajames@...ux.ibm.com> @@ -34,11 +34,42 @@
>>>   	GENMASK(5, ASPEED_AST2600_SCU_IC1_SHIFT)
>>>   #define ASPEED_AST2600_SCU_IC1_NUM_IRQS	2
>>>
>>> +#define ASPEED_AST2700_SCU_IC0_EN_REG	0x1d0
>>> +#define ASPEED_AST2700_SCU_IC0_STS_REG	0x1d4
>>> +#define ASPEED_AST2700_SCU_IC0_SHIFT	0
>>> +#define ASPEED_AST2700_SCU_IC0_ENABLE	\
>>> +	GENMASK(3, ASPEED_AST2700_SCU_IC0_SHIFT)
>>> +#define ASPEED_AST2700_SCU_IC0_NUM_IRQS	4
>>> +
>>> +#define ASPEED_AST2700_SCU_IC1_EN_REG	0x1e0
>>> +#define ASPEED_AST2700_SCU_IC1_STS_REG	0x1e4
>>> +#define ASPEED_AST2700_SCU_IC1_SHIFT	0
>>> +#define ASPEED_AST2700_SCU_IC1_ENABLE	\
>>> +	GENMASK(3, ASPEED_AST2700_SCU_IC1_SHIFT)
>>> +#define ASPEED_AST2700_SCU_IC1_NUM_IRQS	4
>>> +
>>> +#define ASPEED_AST2700_SCU_IC2_EN_REG	0x104
>>> +#define ASPEED_AST2700_SCU_IC2_STS_REG	0x100
>>> +#define ASPEED_AST2700_SCU_IC2_SHIFT	0
>>> +#define ASPEED_AST2700_SCU_IC2_ENABLE	\
>>> +	GENMASK(3, ASPEED_AST2700_SCU_IC2_SHIFT)
>>> +#define ASPEED_AST2700_SCU_IC2_NUM_IRQS	4
>>> +
>>> +#define ASPEED_AST2700_SCU_IC3_EN_REG	0x10c
>>> +#define ASPEED_AST2700_SCU_IC3_STS_REG	0x108
>>> +#define ASPEED_AST2700_SCU_IC3_SHIFT	0
>>> +#define ASPEED_AST2700_SCU_IC3_ENABLE	\
>>> +	GENMASK(1, ASPEED_AST2700_SCU_IC3_SHIFT)
>>> +#define ASPEED_AST2700_SCU_IC3_NUM_IRQS	2
>>> +
>> The reason for ic0/ic1 compatibles before was the enable field was different.
>> Now it's at least at the same shift. Do you really need a different value for IC3?
>>
> OK, I can remove this define.
>
>> The register addresses should come from "reg". I don't understand why they
>> are hardcoded in the driver.
> The original code register is come from parent. scu_ic->scu = syscon_node_to_regmap(node->parent);
> I keep the original code logic, and add AST2700.


Hi Ryan,


How much is common with 2500/2600? I wonder if it would be easier to 
just create a new driver only for AST2700, instead of all in the same 
file/driver?


Thanks,

Eddie


>> Rob