Re: [PATCH 1/2] linux/interrupt.h: allow "guard" notation to disable and reenable IRQ with valid IRQ check

[Thomas Gleixner <tglx@kernel.org>]

On Thu, Jan 22 2026 at 00:23, Marek Vasut wrote:

$Subject: linux/interrupt.h is not a proper prefix. Just use 'genirq:'

Also what means 'allow ....'? Subject lines should provide a concise
summary of the change, i.e. something like:

        Provide conditional disable_irq guard variant

> Introduce disable_valid_irq scoped guard. This is an extension
> of disable_irq scoped guard, which disables and enables IRQs
> around a scope. The disable_valid_irq scoped guard does almost
> the same, except it handles the case where IRQ is not valid,
> in which case it does not do anything. This is meant to be used
> by for example touch controller drivers, which can do both IRQ
> driven and polling mode of operation, and this makes their code
> slighly simpler.

How many of those have this pattern? From a cursory look I found not
even one as your example is neither applicable to upstream nor to next.

> +static inline void disable_valid_irq(unsigned int irq)
> +{
> +	if (irq > 0)
> +		disable_irq(irq);
> +}
> +
> +static inline void enable_valid_irq(unsigned int irq)
> +{
> +	if (irq > 0)
> +		enable_irq(irq);
> +}
> +
> +DEFINE_LOCK_GUARD_1(disable_valid_irq, int,

'int' because it matches the function argument type of the inlines,
right?

> +		    disable_valid_irq(*_T->lock), enable_valid_irq(*_T->lock))

disable_valid_irq is a pretty non-intuitive name if you look at it just
by reading a usage site. It's not really improving the readability of
the code, it's in fact obscuring it as the reader has to actually look
up what the hell this means and then stumble upon a completely
undocumented lock guard define.

I'm all for using guards, but using guards just for the sake of using
guards is not a really good approach.

Thanks,

        tglx