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

[Sebastian Andrzej Siewior]

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.

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.

Sebastian