Re: [PATCH] genirq: use relaxed access by default for irq_reg_{readl,writel}

[Marc Zyngier <maz@kernel.org>]

On Mon, 29 Jan 2024 14:45:02 +0000,
Lucas Stach <l.stach@pengutronix.de> wrote:
> 
> irqchip access does not require any memory ordering between other
> memory transactions and the IRQ controller peripheral access.
> As all architectures now implement the relaxed MMIO accessors we
> can switch the irq_reg_{readl,writel} helpers to use them, in
> order to avoid potentially costly barriers in the IRQ handling
> hotpath.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  include/linux/irq.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 90081afa10ce..fa1597db7887 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -1218,7 +1218,7 @@ static inline void irq_reg_writel(struct irq_chip_generic *gc,
>  	if (gc->reg_writel)
>  		gc->reg_writel(val, gc->reg_base + reg_offset);
>  	else
> -		writel(val, gc->reg_base + reg_offset);
> +		writel_relaxed(val, gc->reg_base + reg_offset);
>  }
>  
>  static inline u32 irq_reg_readl(struct irq_chip_generic *gc,
> @@ -1227,7 +1227,7 @@ static inline u32 irq_reg_readl(struct irq_chip_generic *gc,
>  	if (gc->reg_readl)
>  		return gc->reg_readl(gc->reg_base + reg_offset);
>  	else
> -		return readl(gc->reg_base + reg_offset);
> +		return readl_relaxed(gc->reg_base + reg_offset);
>  }
>  

If this relaxation is introduced, it really should be documented and
require a buy-in, because unsuspecting drivers may implicitly depend
on the stronger ordering.

I'm a strong advocate of the relaxed ordering, but changing this
wholesale is potentially dangerous.

	M.

-- 
Without deviation from the norm, progress is not possible.