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

[Paolo Abeni <pabeni@redhat.com>]

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