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

[Thomas Gleixner]

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