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

[Marek Vasut <marex@nabladev.com>]

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.