[PATCH v2 0/2] irqchip/irq-realtek-rtl: Add multicore support

[PATCH v2 0/2] irqchip/irq-realtek-rtl: Add multicore support

[Markus Stockhausen]

The Realtek Otto switch series consists of multiple devices.

- RTL838x: single core (Realtek proprietary IRQ controller)
- RTL839x: multi core (Realtek proprietary IRQ controller)
- RTL930x: multi core (Realtek proprietary IRQ controller)
- RTL931x: multi core (MIPS GIC controller)

The first three devices are supported by the irq-realtek-rtl
driver. Until now it only supports single core operation. So
the multi core devices cannot be driven in SMP mode.

Add multi core support to the driver.  

Remark: Device tree documentation already knows about this
feature [1]. But it never made it into the driver.

[1] https://elixir.bootlin.com/linux/v7.1-rc6/source/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---

v1 -> v2:
  - Replace IRQ with interrupt in commit messages
  - Use unsigned int for hw_irq and cpu
  - Convert raw_spin_lock_irqsave/restore() in mask/unmask functions
    to guard(raw_spinlock)()
  - Convert raw_spin_lock_irqsave/restore() in mapping function
    to guard(raw_spinlock_irqsave)()
  - Use loop scoped variable in realtek_rtl_of_init()
  - Drop mask calculation in realtek_ictl_unmask_irq(). The affinity
    setter already takes care of that. With this properly set,
    reading irq_data_get_effective_affinity_mask() is enough.
  - Fix style issues and adapt to maintainer tip handbook
v1: https://lore.kernel.org/all/20260512184646.1896480-1-markus.stockhausen@gmx.de/


Markus Stockhausen (2):
  irqchip/irq-realtek-rtl: Add/simplify register helpers
  irqchip/irq-realtek-rtl: Add multicore support

 drivers/irqchip/irq-realtek-rtl.c | 116 ++++++++++++++++++++----------
 1 file changed, 79 insertions(+), 37 deletions(-)

-- 
2.54.0

[PATCH v2 1/2] irqchip/irq-realtek-rtl: Add/simplify register helpers

[Markus Stockhausen]

The Realtek interrupt controller has two important registers
that are used by the driver in several places

- GIMR: global interrupt mask register
- IRR: Interrupt routing registers

The usage of these registers is very inconsistent. GIMR is
addressed directly while IRR has a helper that needs a macro
as an input. Harmonize this by providing consistent helpers
that improve code readability.

The callers of these helpers use classic lock/unlock functions
and sometimes use the wrong locking helper. E.g. irqsave
variants are used in mask/unmask although not needed. Adapt
and fix the surrounding call locations.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/irqchip/irq-realtek-rtl.c | 64 +++++++++++++++----------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/irqchip/irq-realtek-rtl.c b/drivers/irqchip/irq-realtek-rtl.c
index 942c1f8c363d..4e2996eb671e 100644
--- a/drivers/irqchip/irq-realtek-rtl.c
+++ b/drivers/irqchip/irq-realtek-rtl.c
@@ -37,10 +37,29 @@ static void __iomem *realtek_ictl_base;
 #define IRR_OFFSET(idx)		(4 * (3 - (idx * 4) / 32))
 #define IRR_SHIFT(idx)		((idx * 4) % 32)
 
-static void write_irr(void __iomem *irr0, int idx, u32 value)
+static inline void enable_gimr(unsigned int hw_irq)
 {
-	unsigned int offset = IRR_OFFSET(idx);
-	unsigned int shift = IRR_SHIFT(idx);
+	u32 gimr;
+
+	gimr = readl(REG(RTL_ICTL_GIMR));
+	gimr |= BIT(hw_irq);
+	writel(gimr, REG(RTL_ICTL_GIMR));
+}
+
+static inline void disable_gimr(unsigned int hw_irq)
+{
+	u32 gimr;
+
+	gimr = readl(REG(RTL_ICTL_GIMR));
+	gimr &= ~BIT(hw_irq);
+	writel(gimr, REG(RTL_ICTL_GIMR));
+}
+
+static void write_irr(int hw_irq, u32 value)
+{
+	void __iomem *irr0 = REG(RTL_ICTL_IRR0);
+	unsigned int offset = IRR_OFFSET(hw_irq);
+	unsigned int shift = IRR_SHIFT(hw_irq);
 	u32 irr;
 
 	irr = readl(irr0 + offset) & ~(0xf << shift);
@@ -50,30 +69,14 @@ static void write_irr(void __iomem *irr0, int idx, u32 value)
 
 static void realtek_ictl_unmask_irq(struct irq_data *i)
 {
-	unsigned long flags;
-	u32 value;
-
-	raw_spin_lock_irqsave(&irq_lock, flags);
-
-	value = readl(REG(RTL_ICTL_GIMR));
-	value |= BIT(i->hwirq);
-	writel(value, REG(RTL_ICTL_GIMR));
-
-	raw_spin_unlock_irqrestore(&irq_lock, flags);
+	guard(raw_spinlock)(&irq_lock);
+	enable_gimr(i->hwirq);
 }
 
 static void realtek_ictl_mask_irq(struct irq_data *i)
 {
-	unsigned long flags;
-	u32 value;
-
-	raw_spin_lock_irqsave(&irq_lock, flags);
-
-	value = readl(REG(RTL_ICTL_GIMR));
-	value &= ~BIT(i->hwirq);
-	writel(value, REG(RTL_ICTL_GIMR));
-
-	raw_spin_unlock_irqrestore(&irq_lock, flags);
+	guard(raw_spinlock)(&irq_lock);
+	disable_gimr(i->hwirq);
 }
 
 static struct irq_chip realtek_ictl_irq = {
@@ -84,13 +87,10 @@ static struct irq_chip realtek_ictl_irq = {
 
 static int intc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
 {
-	unsigned long flags;
-
 	irq_set_chip_and_handler(irq, &realtek_ictl_irq, handle_level_irq);
 
-	raw_spin_lock_irqsave(&irq_lock, flags);
-	write_irr(REG(RTL_ICTL_IRR0), hw, 1);
-	raw_spin_unlock_irqrestore(&irq_lock, flags);
+	guard(raw_spinlock_irqsave)(&irq_lock);
+	write_irr(hw, 1);
 
 	return 0;
 }
@@ -127,7 +127,6 @@ static int __init realtek_rtl_of_init(struct device_node *node, struct device_no
 {
 	struct of_phandle_args oirq;
 	struct irq_domain *domain;
-	unsigned int soc_irq;
 	int parent_irq;
 
 	realtek_ictl_base = of_iomap(node, 0);
@@ -135,9 +134,10 @@ static int __init realtek_rtl_of_init(struct device_node *node, struct device_no
 		return -ENXIO;
 
 	/* Disable all cascaded interrupts and clear routing */
-	writel(0, REG(RTL_ICTL_GIMR));
-	for (soc_irq = 0; soc_irq < RTL_ICTL_NUM_INPUTS; soc_irq++)
-		write_irr(REG(RTL_ICTL_IRR0), soc_irq, 0);
+	for (unsigned int soc_irq = 0; soc_irq < RTL_ICTL_NUM_INPUTS; soc_irq++) {
+		disable_gimr(soc_irq);
+		write_irr(soc_irq, 0);
+	}
 
 	if (WARN_ON(!of_irq_count(node))) {
 		/*
-- 
2.54.0

[PATCH v2 2/2] irqchip/irq-realtek-rtl: Add multicore support

[Markus Stockhausen]

The Realtek interrupt driver currently supports only single core
systems. So the higher end devices like RTL839x and RTL930x with
dual VPEs must be driven with NR_CPU=1. Enhance the driver to
support multicore (dual VPE) systems. For this:

- Extend the register map for multiple cores
- Search for multiple CPU cores in the devicetree
- Improve the register helpers to support multiple cores
- Add an affinity setter
- Enhance the IRQ handler for multiple cores

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
---
 drivers/irqchip/irq-realtek-rtl.c | 92 ++++++++++++++++++++++---------
 1 file changed, 67 insertions(+), 25 deletions(-)

diff --git a/drivers/irqchip/irq-realtek-rtl.c b/drivers/irqchip/irq-realtek-rtl.c
index 4e2996eb671e..eb7842932c9d 100644
--- a/drivers/irqchip/irq-realtek-rtl.c
+++ b/drivers/irqchip/irq-realtek-rtl.c
@@ -23,10 +23,11 @@
 
 #define RTL_ICTL_NUM_INPUTS	32
 
-#define REG(x)		(realtek_ictl_base + x)
+#define REG(cpu, x)		(realtek_ictl_base[cpu] + x)
 
 static DEFINE_RAW_SPINLOCK(irq_lock);
-static void __iomem *realtek_ictl_base;
+static void __iomem *realtek_ictl_base[NR_CPUS];
+static cpumask_t realtek_ictl_cpu_configurable;
 
 /*
  * IRR0-IRR3 store 4 bits per interrupt, but Realtek uses inverted numbering,
@@ -37,27 +38,27 @@ static void __iomem *realtek_ictl_base;
 #define IRR_OFFSET(idx)		(4 * (3 - (idx * 4) / 32))
 #define IRR_SHIFT(idx)		((idx * 4) % 32)
 
-static inline void enable_gimr(unsigned int hw_irq)
+static inline void enable_gimr(unsigned int cpu, unsigned int hw_irq)
 {
 	u32 gimr;
 
-	gimr = readl(REG(RTL_ICTL_GIMR));
+	gimr = readl(REG(cpu, RTL_ICTL_GIMR));
 	gimr |= BIT(hw_irq);
-	writel(gimr, REG(RTL_ICTL_GIMR));
+	writel(gimr, REG(cpu, RTL_ICTL_GIMR));
 }
 
-static inline void disable_gimr(unsigned int hw_irq)
+static inline void disable_gimr(unsigned int cpu, unsigned int hw_irq)
 {
 	u32 gimr;
 
-	gimr = readl(REG(RTL_ICTL_GIMR));
+	gimr = readl(REG(cpu, RTL_ICTL_GIMR));
 	gimr &= ~BIT(hw_irq);
-	writel(gimr, REG(RTL_ICTL_GIMR));
+	writel(gimr, REG(cpu, RTL_ICTL_GIMR));
 }
 
-static void write_irr(int hw_irq, u32 value)
+static void write_irr(unsigned int cpu, int hw_irq, u32 value)
 {
-	void __iomem *irr0 = REG(RTL_ICTL_IRR0);
+	void __iomem *irr0 = REG(cpu, RTL_ICTL_IRR0);
 	unsigned int offset = IRR_OFFSET(hw_irq);
 	unsigned int shift = IRR_SHIFT(hw_irq);
 	u32 irr;
@@ -69,28 +70,61 @@ static void write_irr(int hw_irq, u32 value)
 
 static void realtek_ictl_unmask_irq(struct irq_data *i)
 {
+	unsigned int cpu;
+
 	guard(raw_spinlock)(&irq_lock);
-	enable_gimr(i->hwirq);
+	for_each_cpu(cpu, irq_data_get_effective_affinity_mask(i))
+		enable_gimr(cpu, i->hwirq);
 }
 
 static void realtek_ictl_mask_irq(struct irq_data *i)
 {
+	unsigned int cpu;
+
 	guard(raw_spinlock)(&irq_lock);
-	disable_gimr(i->hwirq);
+	for_each_cpu(cpu, &realtek_ictl_cpu_configurable)
+		disable_gimr(cpu, i->hwirq);
+}
+
+static int realtek_ictl_irq_affinity(struct irq_data *i, const struct cpumask *dest, bool force)
+{
+	cpumask_t cpu_configure, cpu_disable, cpu_enable;
+	unsigned int cpu;
+
+	cpumask_and(&cpu_configure, cpu_present_mask, &realtek_ictl_cpu_configurable);
+	cpumask_and(&cpu_enable, &cpu_configure, dest);
+	cpumask_andnot(&cpu_disable, &cpu_configure, dest);
+
+	scoped_guard(raw_spinlock, &irq_lock) {
+		for_each_cpu(cpu, &cpu_disable)
+			disable_gimr(cpu, i->hwirq);
+		for_each_cpu(cpu, &cpu_enable) {
+			if (!irqd_irq_masked(i))
+				enable_gimr(cpu, i->hwirq);
+		}
+	}
+
+	irq_data_update_effective_affinity(i, &cpu_enable);
+
+	return IRQ_SET_MASK_OK;
 }
 
 static struct irq_chip realtek_ictl_irq = {
-	.name = "realtek-rtl-intc",
-	.irq_mask = realtek_ictl_mask_irq,
-	.irq_unmask = realtek_ictl_unmask_irq,
+	.name			= "realtek-rtl-intc",
+	.irq_mask		= realtek_ictl_mask_irq,
+	.irq_unmask		= realtek_ictl_unmask_irq,
+	.irq_set_affinity	= realtek_ictl_irq_affinity,
 };
 
 static int intc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
 {
+	unsigned int cpu;
+
 	irq_set_chip_and_handler(irq, &realtek_ictl_irq, handle_level_irq);
 
 	guard(raw_spinlock_irqsave)(&irq_lock);
-	write_irr(hw, 1);
+	for_each_cpu(cpu, &realtek_ictl_cpu_configurable)
+		write_irr(cpu, hw, 1);
 
 	return 0;
 }
@@ -103,12 +137,13 @@ static const struct irq_domain_ops irq_domain_ops = {
 static void realtek_irq_dispatch(struct irq_desc *desc)
 {
 	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned int cpu = smp_processor_id();
 	struct irq_domain *domain;
 	unsigned long pending;
 	unsigned int soc_int;
 
 	chained_irq_enter(chip, desc);
-	pending = readl(REG(RTL_ICTL_GIMR)) & readl(REG(RTL_ICTL_GISR));
+	pending = readl(REG(cpu, RTL_ICTL_GIMR)) & readl(REG(cpu, RTL_ICTL_GISR));
 
 	if (unlikely(!pending)) {
 		spurious_interrupt();
@@ -129,16 +164,23 @@ static int __init realtek_rtl_of_init(struct device_node *node, struct device_no
 	struct irq_domain *domain;
 	int parent_irq;
 
-	realtek_ictl_base = of_iomap(node, 0);
-	if (!realtek_ictl_base)
-		return -ENXIO;
-
-	/* Disable all cascaded interrupts and clear routing */
-	for (unsigned int soc_irq = 0; soc_irq < RTL_ICTL_NUM_INPUTS; soc_irq++) {
-		disable_gimr(soc_irq);
-		write_irr(soc_irq, 0);
+	cpumask_clear(&realtek_ictl_cpu_configurable);
+
+	for (unsigned int cpu = 0; cpu < NR_CPUS; cpu++) {
+		realtek_ictl_base[cpu] = of_iomap(node, cpu);
+		if (realtek_ictl_base[cpu]) {
+			cpumask_set_cpu(cpu, &realtek_ictl_cpu_configurable);
+			/* Disable all cascaded interrupts and clear routing */
+			for (unsigned int soc_irq = 0; soc_irq < RTL_ICTL_NUM_INPUTS; soc_irq++) {
+				disable_gimr(cpu, soc_irq);
+				write_irr(cpu, soc_irq, 0);
+			}
+		}
 	}
 
+	if (cpumask_empty(&realtek_ictl_cpu_configurable))
+		return -ENXIO;
+
 	if (WARN_ON(!of_irq_count(node))) {
 		/*
 		 * If DT contains no parent interrupts, assume MIPS CPU IRQ 2
-- 
2.54.0

Re: [PATCH v2 2/2] irqchip/irq-realtek-rtl: Add multicore support

[Thomas Gleixner]

On Thu, Jun 04 2026 at 15:06, Markus Stockhausen wrote:

>  static void realtek_ictl_unmask_irq(struct irq_data *i)
>  {
> +	unsigned int cpu;
> +
>	 guard(raw_spinlock)(&irq_lock);
> -	enable_gimr(i->hwirq);
> +	for_each_cpu(cpu, irq_data_get_effective_affinity_mask(i))
> +		enable_gimr(cpu, i->hwirq);
>  }

Ok.

>  static void realtek_ictl_mask_irq(struct irq_data *i)
>  {
> +	unsigned int cpu;
> +
>	 guard(raw_spinlock)(&irq_lock);
> -	disable_gimr(i->hwirq);
> +	for_each_cpu(cpu, &realtek_ictl_cpu_configurable)
> +		disable_gimr(cpu, i->hwirq);
> +}

Why not using the effective mask here? CPUs which are not in the
effective mask are disabled already.

> +static int realtek_ictl_irq_affinity(struct irq_data *i, const struct cpumask *dest, bool force)
> +{
> +	cpumask_t cpu_configure, cpu_disable, cpu_enable;
> +	unsigned int cpu;

Looking deeper at this specific part:

> +	cpumask_and(&cpu_configure, cpu_present_mask, &realtek_ictl_cpu_configurable);

Why do you need that?

cpu_configurable should arguably never contain non-present CPUs.

If you want to protect against a bonkers device tree, then do so in the
probe function.

But ....

> +	cpumask_and(&cpu_enable, &cpu_configure, dest);
> +	cpumask_andnot(&cpu_disable, &cpu_configure, dest);

Assume that cpu_configure and dest do not overlap, then you end up with
_zero_ target CPUs, i.e. a non-working interrupt.

The general asssumption of the core interrupt code is that except for
strict per CPU interrupts, which are managed separately, interrupts can
be routed to any online CPU.

So I think your init logic is broken:

> +		realtek_ictl_base[cpu] = of_iomap(node, cpu);
> +		if (realtek_ictl_base[cpu]) {

If this mapping fails, then you should fail the initialization and your
loop around that should do:

            for_each_present_cpu()

and not try to figure that out via NR_CPUS:

> +	for (unsigned int cpu = 0; cpu < NR_CPUS; cpu++) {

and the failed mapping logic. At the point where the driver is
initialized the present CPU mask is stable and authoritative.

If a mapping for a present CPU fails, then the driver has to fail the
initialization as you otherwise run into the stale interrupt situation
described above.

With that fixed you can drop this whole realtek_ictl_cpu_configurable
dance as the core will never set a non-present CPU in the destination
mask. It even guarantees that the CPUs in the mask are online unless
@force = true. The latter is only for scenarios where pseudo per CPU
interrupts have to be affined before a CPU goes online, so irrelevant
for your use case.

> +	scoped_guard(raw_spinlock, &irq_lock) {
> +		for_each_cpu(cpu, &cpu_disable)
> +			disable_gimr(cpu, i->hwirq);
> +		for_each_cpu(cpu, &cpu_enable) {
> +			if (!irqd_irq_masked(i))
> +				enable_gimr(cpu, i->hwirq);
> +		}
> +	}

This can be simplified to:

	if (!irqd_irq_masked(i))
        	realtek_ictl_mask_irq(i);

	irq_data_update_effective_affinity(i, &dest);

	if (!irqd_irq_masked(i))
        	realtek_ictl_unmask_irq(i);

Sorry that I failed to catch those things right away.

Thanks,

        tglx

AW: [PATCH v2 2/2] irqchip/irq-realtek-rtl: Add multicore support

[Markus Stockhausen]

> Von: Thomas Gleixner <tglx@kernel.org> 
> Gesendet: Donnerstag, 4. Juni 2026 17:59
> An: Markus Stockhausen <markus.stockhausen@gmx.de>;
linux-kernel@vger.kernel.org
> Cc: Markus Stockhausen <markus.stockhausen@gmx.de>
> Betreff: Re: [PATCH v2 2/2] irqchip/irq-realtek-rtl: Add multicore support
> ...
> With that fixed you can drop this whole realtek_ictl_cpu_configurable
> dance as the core will never set a non-present CPU in the destination
> mask. It even guarantees that the CPUs in the mask are online unless
> @force = true. The latter is only for scenarios where pseudo per CPU
> interrupts have to be affined before a CPU goes online, so irrelevant
> for your use case.

Thanks für the explanation. That paved the way for a much 
simpler v3 series. BTW - We need those pseudo per CPU interrupts.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/dr
ivers/irqchip/irq-mips-gic.c?id=2250db8628a0d8293ad2e0671138b848a185fba1

Markus

Re: AW: [PATCH v2 2/2] irqchip/irq-realtek-rtl: Add multicore support

[Thomas Gleixner]

On Thu, Jun 04 2026 at 20:22, Markus Stockhausen wrote:
>> Von: Thomas Gleixner <tglx@kernel.org> 
>> Gesendet: Donnerstag, 4. Juni 2026 17:59
>> An: Markus Stockhausen <markus.stockhausen@gmx.de>;
> linux-kernel@vger.kernel.org
>> Cc: Markus Stockhausen <markus.stockhausen@gmx.de>
>> Betreff: Re: [PATCH v2 2/2] irqchip/irq-realtek-rtl: Add multicore support
>> ...
>> With that fixed you can drop this whole realtek_ictl_cpu_configurable
>> dance as the core will never set a non-present CPU in the destination
>> mask. It even guarantees that the CPUs in the mask are online unless
>> @force = true. The latter is only for scenarios where pseudo per CPU
>> interrupts have to be affined before a CPU goes online, so irrelevant
>> for your use case.
>
> Thanks für the explanation. That paved the way for a much 
> simpler v3 series. BTW - We need those pseudo per CPU interrupts.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/irqchip/irq-mips-gic.c?id=2250db8628a0d8293ad2e0671138b848a185fba1

Fine, but that code makes no sense:

	if (force)
		cpu = cpumask_first(cpumask);
	else
		cpu = cpumask_first_and(cpumask, cpu_online_mask);

because in the !force case @cpumask is guaranteed to be not empty and a
subset of cpu_online mask.

I missed that back then probably because the long since then fixed
original core code could supply arbitrary masks to the drivers and my
memory still stuck to this scheme.

Thanks,

        tglx