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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <998a4b7e-8d29-4702-87fb-726117369240@redhat.com>
Date: Tue, 1 Oct 2024 11:20:49 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Csókás Bence <csokas.bence@...lan.hu>,
 Wei Fang <wei.fang@....com>, Frank Li <frank.li@....com>
Cc: Shenwei Wang <shenwei.wang@....com>, Clark Wang <xiaoning.wang@....com>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Richard Cochran <richardcochran@...il.com>,
 "David S. Miller" <davem@...emloft.net>,
 "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net 1/2] net: fec: Restart PPS after link state change

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