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

[Marek Vasut]

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.