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

[Marek Vasut <marex@nabladev.com>]

On 4/14/26 10:55 AM, Sebastian Andrzej Siewior wrote:
> On 2026-04-13 18:03:38 [+0200], To Marek Vasut wrote:
>> On 2026-04-13 17:31:34 [+0200], Marek Vasut wrote:
>>>> I don't see why it needs to disable interrupts.
>>>
>>> Because when the lock is held, the PAR code shouldn't be interrupted by an
>>> interrupt, otherwise it would completely mess up the state of the KS8851
>>> MAC. The spinlock does not protect only the IRQ handler, it protects also
>>> ks8851_start_xmit_par() and ks8851_write_mac_addr() and
>>> ks8851_read_mac_addr() and ks8851_net_open() and ks8851_net_stop() and other
>>> sites which call ks8851_lock()/ks8851_unlock() which cannot be executed
>>> concurrently, but where BHs can be enabled.
>>
>> I need check this once brain is at full power again. But which
>> interrupt? Your interrupt is threaded. So that should be okay.
> 
> I don't understand. There is no point in using spin_lock_irqsave() in
> ks8851_lock_par(). You don't protect against interrupts because none of
> the user actually run in an interrupt. As far as I can see, the
> interrupt is threaded and the mdio phy link checks should come from the
> workqueue.

Ha, now that the IRQ handler is indeed only threaded, I can use 
spin_lock_bh() indeed. I will send a V3 like that.

> What is wrong is that the ndo_start_xmit callback can be invoked from a
> softirq and such you must disable BHs while acquiring a lock which can
> be accessed from both contexts. Therefore spin_lock() is not sufficient,
> it needs the _bh() and _irq() brings no additional value here.