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

[Nicolai Buchwitz]

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@xxxxxxxxxxx>  # KS8851 SPI, non-RT