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

[Thomas Gleixner <tglx@kernel.org>]

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