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

Marek Vasut
2026-04-10T01:22:19Z

On 4/9/26 8:52 AM, Nicolai Buchwitz wrote:

Hello Nicolai,

>> @@ -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.
That is exactly why I wrapped the IRQ handler, because the BH should be
disabled ONLY around the IRQ handler, not around the other call sites.