Re: [net,PATCH v2] net: ks8851: Reinstate disabling of BHs around IRQ handler

Nicolai Buchwitz
2026-04-09T06:52:22Z

Hi Marek

On 8.4.2026 18:24, Marek Vasut wrote:

> [...]

> + bool no_bh_in_irq_handler;

> [...]

> +/**
> + * ks8851_irq_nobh - IRQ handler with BH disabled
> + * @irq: IRQ number
> + * @_ks: cookie
> + *
> + * Wrapper which calls ks8851_irq() with BH disabled.
> + */
> +static irqreturn_t ks8851_irq_nobh(int irq, void *_ks)
> +{
> + irqreturn_t ret;
> +
> + local_bh_disable();
> + ret = ks8851_irq(irq, _ks);
> + local_bh_enable();
> +
> + return ret;
> +}
> +
> /**
> * ks8851_flush_tx_work - flush outstanding TX work
> * @ks: The device state
> @@ -408,7 +426,9 @@ static int ks8851_net_open(struct net_device *dev)
> unsigned long flags;
> int ret;
>
> - ret = request_threaded_irq(dev->irq, NULL, ks8851_irq,
> + ret = request_threaded_irq(dev->irq, NULL,
> + ks->no_bh_in_irq_handler ?
> + ks8851_irq_nobh : ks8851_irq,

This works, but wouldn't it be simpler to put the BH disable
into the PAR lock/unlock directly?

static void ks8851_lock_par(...)
{
local_bh_disable();
spin_lock_irqsave(&ksp->lock, *flags);
}

static void ks8851_unlock_par(...)
{
spin_unlock_irqrestore(&ksp->lock, *flags);
local_bh_enable();
}

No flag, no wrapper, no conditional in request_threaded_irq.
And it protects all PAR lock/unlock callsites, not just the
IRQ handler.

> [...]

Regards
Nicolai

Tested-by: Nicolai Buchwitz <nb@tipi-net.de> # KS8851 SPI, non-RT