Re: [PATCH net 1/2] net: fec: Restart PPS after link state change

[Paolo Abeni]

On 9/30/24 10:20, Csókás Bence wrote:
> On 2024. 09. 25. 6:37, Wei Fang wrote:
> > > +/* Restore PTP functionality after a reset */ void
> > > +fec_ptp_restore_state(struct fec_enet_private *fep) {
> > > +	unsigned long flags;
> > > +
> > > +	spin_lock_irqsave(&fep->tmreg_lock, flags);
> > > +
> > > +	/* Reset turned it off, so adjust our status flag */
> > > +	fep->pps_enable = 0;
> > > +
> > > +	spin_unlock_irqrestore(&fep->tmreg_lock, flags);
> > > +
> > > +	/* Restart PPS if needed */
> > > +	if (fep->ptp_saved_state.pps_enable) {
> >
> > It's better to put " fep->pps_enable = 0" here so that it does
> > not need to be set when PPS is disabled.
>
> It doesn't hurt to set it to 0 when it's already 0, and it saves us
> having to unlock separately in the if {} and else blocks. Plus, after
> reset, PPS will be turned off unconditionally, since the actual HW gets
> reset.

I agree with Csókás, the proposed code looks simpler and more readable.

I'm applying this.

Thanks!

Paolo