The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 0/2] irqchip/irq-realtek-rtl: Add multicore support
@ 2026-05-12 18:46 Markus Stockhausen
  2026-05-12 18:46 ` [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register helpers Markus Stockhausen
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Markus Stockhausen @ 2026-05-12 18:46 UTC (permalink / raw)
  To: tglx, linux-kernel; +Cc: 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.  

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register helpers
  2026-05-12 18:46 [PATCH 0/2] irqchip/irq-realtek-rtl: Add multicore support Markus Stockhausen
@ 2026-05-12 18:46 ` Markus Stockhausen
  2026-06-03 15:57   ` Thomas Gleixner
  2026-05-12 18:46 ` [PATCH 2/2] irqchip/irq-realtek-rtl: Add multicore support Markus Stockhausen
  2026-05-22 17:46 ` AW: [PATCH 0/2] " Markus Stockhausen
  2 siblings, 1 reply; 9+ messages in thread
From: Markus Stockhausen @ 2026-05-12 18:46 UTC (permalink / raw)
  To: tglx, linux-kernel; +Cc: Markus Stockhausen

The Realtek IRQ 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 an
macro as an input. Harmonize this by providing consistent
helpers that improve code readability.

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

diff --git a/drivers/irqchip/irq-realtek-rtl.c b/drivers/irqchip/irq-realtek-rtl.c
index 942c1f8c363d..7fb8dd4c5670 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(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(int hwirq)
+{
+	u32 gimr;
+
+	gimr = readl(REG(RTL_ICTL_GIMR));
+	gimr &= ~BIT(hwirq);
+	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);
@@ -51,28 +70,18 @@ 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));
-
+	enable_gimr(i->hwirq);
 	raw_spin_unlock_irqrestore(&irq_lock, flags);
 }
 
 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));
-
+	disable_gimr(i->hwirq);
 	raw_spin_unlock_irqrestore(&irq_lock, flags);
 }
 
@@ -89,7 +98,7 @@ static int intc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
 	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);
+	write_irr(hw, 1);
 	raw_spin_unlock_irqrestore(&irq_lock, flags);
 
 	return 0;
@@ -135,9 +144,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 (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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] irqchip/irq-realtek-rtl: Add multicore support
  2026-05-12 18:46 [PATCH 0/2] irqchip/irq-realtek-rtl: Add multicore support Markus Stockhausen
  2026-05-12 18:46 ` [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register helpers Markus Stockhausen
@ 2026-05-12 18:46 ` Markus Stockhausen
  2026-06-03 16:02   ` Thomas Gleixner
  2026-05-22 17:46 ` AW: [PATCH 0/2] " Markus Stockhausen
  2 siblings, 1 reply; 9+ messages in thread
From: Markus Stockhausen @ 2026-05-12 18:46 UTC (permalink / raw)
  To: tglx, linux-kernel; +Cc: Markus Stockhausen

The Realtek IRQ 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 | 91 +++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 22 deletions(-)

diff --git a/drivers/irqchip/irq-realtek-rtl.c b/drivers/irqchip/irq-realtek-rtl.c
index 7fb8dd4c5670..e0e07074bf6f 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(int hw_irq)
+static inline void enable_gimr(int cpu, 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(int hwirq)
+static inline void disable_gimr(int cpu, int hwirq)
 {
 	u32 gimr;
 
-	gimr = readl(REG(RTL_ICTL_GIMR));
+	gimr = readl(REG(cpu, RTL_ICTL_GIMR));
 	gimr &= ~BIT(hwirq);
-	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(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;
@@ -70,35 +71,73 @@ static void write_irr(int hw_irq, u32 value)
 static void realtek_ictl_unmask_irq(struct irq_data *i)
 {
 	unsigned long flags;
+	cpumask_t cpus;
+	int cpu;
+
+	cpumask_and(&cpus, &realtek_ictl_cpu_configurable,
+		    irq_data_get_effective_affinity_mask(i));
 
 	raw_spin_lock_irqsave(&irq_lock, flags);
-	enable_gimr(i->hwirq);
+	for_each_cpu(cpu, &cpus)
+		enable_gimr(cpu, i->hwirq);
 	raw_spin_unlock_irqrestore(&irq_lock, flags);
 }
 
 static void realtek_ictl_mask_irq(struct irq_data *i)
 {
 	unsigned long flags;
+	int cpu;
 
 	raw_spin_lock_irqsave(&irq_lock, flags);
-	disable_gimr(i->hwirq);
+	for_each_cpu(cpu, &realtek_ictl_cpu_configurable)
+		disable_gimr(cpu, i->hwirq);
 	raw_spin_unlock_irqrestore(&irq_lock, flags);
 }
 
+static int realtek_ictl_irq_affinity(struct irq_data *i,
+				     const struct cpumask *dest,
+				     bool force)
+{
+	cpumask_t cpu_configure;
+	cpumask_t cpu_disable;
+	cpumask_t cpu_enable;
+	unsigned long flags;
+	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);
+
+	raw_spin_lock_irqsave(&irq_lock, flags);
+	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);
+	raw_spin_unlock_irqrestore(&irq_lock, flags);
+
+	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,
+	.irq_set_affinity = realtek_ictl_irq_affinity,
 };
 
 static int intc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
 {
 	unsigned long flags;
+	int cpu;
 
 	irq_set_chip_and_handler(irq, &realtek_ictl_irq, handle_level_irq);
 
 	raw_spin_lock_irqsave(&irq_lock, flags);
-	write_irr(hw, 1);
+	for_each_cpu(cpu, &realtek_ictl_cpu_configurable)
+		write_irr(cpu, hw, 1);
 	raw_spin_unlock_irqrestore(&irq_lock, flags);
 
 	return 0;
@@ -112,12 +151,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);
+	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();
@@ -139,16 +179,23 @@ static int __init realtek_rtl_of_init(struct device_node *node, struct device_no
 	unsigned int soc_irq;
 	int parent_irq;
 
-	realtek_ictl_base = of_iomap(node, 0);
-	if (!realtek_ictl_base)
-		return -ENXIO;
-
-	/* Disable all cascaded interrupts and clear routing */
-	for (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 (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 (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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* AW: [PATCH 0/2] irqchip/irq-realtek-rtl: Add multicore support
  2026-05-12 18:46 [PATCH 0/2] irqchip/irq-realtek-rtl: Add multicore support Markus Stockhausen
  2026-05-12 18:46 ` [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register helpers Markus Stockhausen
  2026-05-12 18:46 ` [PATCH 2/2] irqchip/irq-realtek-rtl: Add multicore support Markus Stockhausen
@ 2026-05-22 17:46 ` Markus Stockhausen
  2 siblings, 0 replies; 9+ messages in thread
From: Markus Stockhausen @ 2026-05-22 17:46 UTC (permalink / raw)
  To: tglx, linux-kernel

Hi Thomas,

> Von: Markus Stockhausen <markus.stockhausen@gmx.de> 
> Gesendet: Dienstag, 12. Mai 2026 20:47
> An: tglx@kernel.org; linux-kernel@vger.kernel.org
> Cc: Markus Stockhausen <markus.stockhausen@gmx.de>
> Betreff: [PATCH 0/2] irqchip/irq-realtek-rtl: Add multicore support
>
> 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.  

Maybe you can give feedback on this series?

Thanks in advance.

Markus


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register helpers
  2026-05-12 18:46 ` [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register helpers Markus Stockhausen
@ 2026-06-03 15:57   ` Thomas Gleixner
  2026-06-04 12:32     ` AW: " Markus Stockhausen
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2026-06-03 15:57 UTC (permalink / raw)
  To: Markus Stockhausen, linux-kernel; +Cc: Markus Stockhausen

On Tue, May 12 2026 at 20:46, Markus Stockhausen wrote:

> The Realtek IRQ controller has two important registers that

s/IRQ/interrupt/

This is not a SMS service.

>  
> -static void write_irr(void __iomem *irr0, int idx, u32 value)
> +static inline void enable_gimr(int hw_irq)

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(int hwirq)

Ditto

> +{
> +	u32 gimr;
> +
> +	gimr = readl(REG(RTL_ICTL_GIMR));
> +	gimr &= ~BIT(hwirq);
> +	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);
> @@ -51,28 +70,18 @@ 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);

Please convert that to

       guard(raw_spinlock)(&lock);

while at it. No _irqsave required as mask/unmask are invoked with the
interrupt descriptor lock held and interrupts disabled.

>  static void realtek_ictl_mask_irq(struct irq_data *i)
>  {
>  	unsigned long flags;
> -	u32 value;
>  
>  	raw_spin_lock_irqsave(&irq_lock, flags);

Ditto

> -
> -	value = readl(REG(RTL_ICTL_GIMR));
> -	value &= ~BIT(i->hwirq);
> -	writel(value, REG(RTL_ICTL_GIMR));
> -
> +	disable_gimr(i->hwirq);
>  	raw_spin_unlock_irqrestore(&irq_lock, flags);
>  }
>  
> @@ -89,7 +98,7 @@ static int intc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
>  	irq_set_chip_and_handler(irq, &realtek_ictl_irq, handle_level_irq);
>  
       guard(raw_spinlock_irq)(&lock);

_irq because this is task context.

>  	raw_spin_lock_irqsave(&irq_lock, flags);
> -	write_irr(REG(RTL_ICTL_IRR0), hw, 1);
> +	write_irr(hw, 1);
>  	raw_spin_unlock_irqrestore(&irq_lock, flags);
>  
>  	return 0;
> @@ -135,9 +144,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 (soc_irq = 0; soc_irq < RTL_ICTL_NUM_INPUTS; soc_irq++) {

Please make that

       for (unsigned int soc_irq = 0; ....

and remove the declaration at the top of the function.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] irqchip/irq-realtek-rtl: Add multicore support
  2026-05-12 18:46 ` [PATCH 2/2] irqchip/irq-realtek-rtl: Add multicore support Markus Stockhausen
@ 2026-06-03 16:02   ` Thomas Gleixner
  2026-06-04 11:50     ` AW: " Markus Stockhausen
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2026-06-03 16:02 UTC (permalink / raw)
  To: Markus Stockhausen, linux-kernel; +Cc: Markus Stockhausen

On Tue, May 12 2026 at 20:46, Markus Stockhausen wrote:

> The Realtek IRQ driver currently supports only single core

s/IRQ/interrupt/g

> 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
>
>  
> -static inline void enable_gimr(int hw_irq)
> +static inline void enable_gimr(int cpu, int hw_irq)

unsigned int cpu - All over the place.

>  {
>  	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(int hwirq)
> +static inline void disable_gimr(int cpu, int hwirq)
>  {
>  	u32 gimr;
>  
> -	gimr = readl(REG(RTL_ICTL_GIMR));
> +	gimr = readl(REG(cpu, RTL_ICTL_GIMR));
>  	gimr &= ~BIT(hwirq);
> -	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(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;
> @@ -70,35 +71,73 @@ static void write_irr(int hw_irq, u32 value)
>  static void realtek_ictl_unmask_irq(struct irq_data *i)
>  {
>  	unsigned long flags;
> +	cpumask_t cpus;
> +	int cpu;
> +
> +	cpumask_and(&cpus, &realtek_ictl_cpu_configurable,
> +		    irq_data_get_effective_affinity_mask(i));

What is this cpumask_and() for? The affinity setter already ensures that
the effective mask is a subset of configurable, no?

>  
> +static int realtek_ictl_irq_affinity(struct irq_data *i,
> +				     const struct cpumask *dest,
> +				     bool force)

Please use the full 100 characters.

> +{
> +	cpumask_t cpu_configure;
> +	cpumask_t cpu_disable;
> +	cpumask_t cpu_enable;

https://docs.kernel.org/process/maintainer-tip.html#variable-declarations

> +	unsigned long flags;
> +	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);
> +
> +	raw_spin_lock_irqsave(&irq_lock, flags);

	scoped_guard(raw_spinlock, ....) {

> +	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);

See bracket rules in the documentation I linked to.


  	}

> +	raw_spin_unlock_irqrestore(&irq_lock, flags);
> +
> +	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,
> +	.irq_set_affinity = realtek_ictl_irq_affinity,

Please fix up the struct initializer according to documentation.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* AW: [PATCH 2/2] irqchip/irq-realtek-rtl: Add multicore support
  2026-06-03 16:02   ` Thomas Gleixner
@ 2026-06-04 11:50     ` Markus Stockhausen
  0 siblings, 0 replies; 9+ messages in thread
From: Markus Stockhausen @ 2026-06-04 11:50 UTC (permalink / raw)
  To: 'Thomas Gleixner', linux-kernel

Hi Thomas,

thanks for the clear guidance and the notes about
using the proper locking functions. That simplified
the creation v2 of the series a lot.

Markus


^ permalink raw reply	[flat|nested] 9+ messages in thread

* AW: [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register helpers
  2026-06-03 15:57   ` Thomas Gleixner
@ 2026-06-04 12:32     ` Markus Stockhausen
  2026-06-04 12:39       ` Thomas Gleixner
  0 siblings, 1 reply; 9+ messages in thread
From: Markus Stockhausen @ 2026-06-04 12:32 UTC (permalink / raw)
  To: 'Thomas Gleixner'; +Cc: linux-kernel

> Von: Thomas Gleixner <tglx@kernel.org> 
> Gesendet: Mittwoch, 3. Juni 2026 17:57
> An: Markus Stockhausen <markus.stockhausen@gmx.de>;
linux-kernel@vger.kernel.org
> Cc: Markus Stockhausen <markus.stockhausen@gmx.de>
> Betreff: Re: [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register
helpers
> ...
> > -
> > -	value = readl(REG(RTL_ICTL_GIMR));
> > -	value &= ~BIT(i->hwirq);
> > -	writel(value, REG(RTL_ICTL_GIMR));
> > -
> > +	disable_gimr(i->hwirq);
> >  	raw_spin_unlock_irqrestore(&irq_lock, flags);
> >  }
> >  
> > @@ -89,7 +98,7 @@ static int intc_map(struct irq_domain *d, unsigned int
irq, irq_hw_number_t hw)
> >  	irq_set_chip_and_handler(irq, &realtek_ictl_irq, handle_level_irq);
> >  
>        guard(raw_spinlock_irq)(&lock);
>
> _irq because this is task context.
>
> >  	raw_spin_lock_irqsave(&irq_lock, flags);
> > -	write_irr(REG(RTL_ICTL_IRR0), hw, 1);
> > +	write_irr(hw, 1);
> >  	raw_spin_unlock_irqrestore(&irq_lock, flags);
> >  
> >  	return 0;

The above is the only one of your guides I'm unsure about. 
With the whole series adapted and applied I finally get 

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_irq)(&irq_lock);
	for_each_cpu(cpu, &realtek_ictl_cpu_configurable)
		write_irr(cpu, hw, 1);

	return 0;
}

Boot gives a warning.

[    0.009035] ------------[ cut here ]------------
[    0.014075] WARNING: CPU: 0 PID: 0 at init/main.c:1059
start_kernel+0x3a0/0x510
[    0.022115] Interrupts were enabled early
[    0.026483] Modules linked in:
[    0.029846] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.18.33 #0
NONE
[    0.029871] Hardware name: Linksys LGS328C
[    0.029878] Stack : 80b7dddc 00000031 00000000 00000001 00000000 00000000
00000000 00000000
[    0.029929]         00000000 00000000 00000000 00000000 00000000 00000001
80b7dd98 00000000
[    0.029975]         00000000 00000000 80a846d4 80b7dc30 00000000 ffffefff
00000001 00000031
[    0.030022]         00000033 80b7dbf4 00000033 00000264 00000001 00000000
80a846d4 80b7dea8
[    0.030069]         00000000 80c23ec4 80b95238 80100000 00000000 80b9d4d0
00000000 813a0000
[    0.030116]         ...
[    0.030125] Call Trace:
[    0.030129] [<80114e38>] show_stack+0x28/0xf0
[    0.030163] [<8010e0ec>] dump_stack_lvl+0x70/0xb0
[    0.030197] [<801390cc>] __warn+0x9c/0x114
[    0.030227] [<801392c4>] warn_slowpath_fmt+0x180/0x188
[    0.030246] [<80c23ec4>] start_kernel+0x3a0/0x510

This goes away with switching to guard(raw_spinlock_irqsave)().

Anything I'm missing here?

Thanks in advance.

Markus


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: AW: [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register helpers
  2026-06-04 12:32     ` AW: " Markus Stockhausen
@ 2026-06-04 12:39       ` Thomas Gleixner
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2026-06-04 12:39 UTC (permalink / raw)
  To: Markus Stockhausen; +Cc: linux-kernel

On Thu, Jun 04 2026 at 14:32, Markus Stockhausen wrote:

>> Von: Thomas Gleixner <tglx@kernel.org> 
>> Gesendet: Mittwoch, 3. Juni 2026 17:57
>> An: Markus Stockhausen <markus.stockhausen@gmx.de>;
> linux-kernel@vger.kernel.org
>> Cc: Markus Stockhausen <markus.stockhausen@gmx.de>
>> Betreff: Re: [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register
> helpers
>> ...
>> > -
>> > -	value = readl(REG(RTL_ICTL_GIMR));
>> > -	value &= ~BIT(i->hwirq);
>> > -	writel(value, REG(RTL_ICTL_GIMR));
>> > -
>> > +	disable_gimr(i->hwirq);
>> >  	raw_spin_unlock_irqrestore(&irq_lock, flags);
>> >  }
>> >  
>> > @@ -89,7 +98,7 @@ static int intc_map(struct irq_domain *d, unsigned int
> irq, irq_hw_number_t hw)
>> >  	irq_set_chip_and_handler(irq, &realtek_ictl_irq, handle_level_irq);
>> >  
>>        guard(raw_spinlock_irq)(&lock);
>>
>> _irq because this is task context.
>>
>> >  	raw_spin_lock_irqsave(&irq_lock, flags);
>> > -	write_irr(REG(RTL_ICTL_IRR0), hw, 1);
>> > +	write_irr(hw, 1);
>> >  	raw_spin_unlock_irqrestore(&irq_lock, flags);
>> >  
>> >  	return 0;
>
> The above is the only one of your guides I'm unsure about. 
> With the whole series adapted and applied I finally get 
>
> 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_irq)(&irq_lock);
> 	for_each_cpu(cpu, &realtek_ictl_cpu_configurable)
> 		write_irr(cpu, hw, 1);
>
> 	return 0;
> }
>
> Boot gives a warning.

Indeed. If map() is invoked during early boot, you need _irqsave.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-06-04 12:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12 18:46 [PATCH 0/2] irqchip/irq-realtek-rtl: Add multicore support Markus Stockhausen
2026-05-12 18:46 ` [PATCH 1/2] irqchip/irq-realtek-rtl: Add/simplify register helpers Markus Stockhausen
2026-06-03 15:57   ` Thomas Gleixner
2026-06-04 12:32     ` AW: " Markus Stockhausen
2026-06-04 12:39       ` Thomas Gleixner
2026-05-12 18:46 ` [PATCH 2/2] irqchip/irq-realtek-rtl: Add multicore support Markus Stockhausen
2026-06-03 16:02   ` Thomas Gleixner
2026-06-04 11:50     ` AW: " Markus Stockhausen
2026-05-22 17:46 ` AW: [PATCH 0/2] " Markus Stockhausen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox