Re: [PATCH] Use a spinlock to guard `fep->ptp_clk_on`

[Andrew Lunn <andrew@lunn.ch>]

On Thu, Sep 01, 2022 at 10:06:03AM +0200, Csókás Bence wrote:
> 
> On 2022. 09. 01. 9:51, Csókás Bence wrote:
> > 
> > On 2022. 08. 31. 18:24, Andrew Lunn wrote:
> >  >>> Please keep to reverse christmas tree
> >  >>
> >  >> checkpatch didn't tell me that was a requirement... Easy to fix though.
> >  >
> >  > checkpatch does not have the smarts to detect this. And it is a netdev
> >  > only thing.
> > 
> > I thought checkpatch also has the per-subsystem rules, too.
> > 
> >  > There is also a different between not being able to sleep, and not
> >  > being able to process an interrupt for some other hardware. You got a
> >  > report about taking a mutex in atomic context. That just means you
> >  > cannot sleep, probably because a spinlock is held. That is very
> >  > different to not being able to handle interrupts. You only need
> >  > spin_lock_irqsave() if the interrupt handler also needs the same spin
> >  > lock to protect it from a thread using the spin lock.
> > 
> > Alright, I will switch to plain `spin_lock()` then.
> 
> By the way, what about `&fep->tmreg_lock`? Should that also be switched to
> `spin_lock()`? If not, how should I handle the nested locking? Calling
> `spin_lock_irqsave(&fep->tmreg_lock)` after `spin_lock(&&fep->ptp_clk_lock)`
> seems pointless. Should I lock with `spin_lock_irqsave(&fep->ptp_clk_lock)`
> there?

Richard was making the point, do you need two locks?

What are the locks protecting? Could you use one lock for both use
cases? Should the other lock also not be an _irqsave()?

       Andrew