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

Nicolai Buchwitz
2026-04-10T07:29:50Z

On 9.4.2026 17:26, Marek Vasut wrote:
> 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.

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>