netdev - Re: [PATCH net-next 3/3] bnxt: Enforce PTP software freq adjustments only when in non-RTC mode

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 for Android: free password hash cracker in your pocket

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

Message-ID: <10a31caf-9ceb-8d13-4cf5-b0e2ffef948d@linux.dev>
Date:   Tue, 21 Mar 2023 11:18:09 +0000
From:   Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To:     Pavan Chebbi <pavan.chebbi@...adcom.com>,
        michael.chan@...adcom.com, kuba@...nel.org
Cc:     davem@...emloft.net, edumazet@...gle.com, gospo@...adcom.com,
        netdev@...r.kernel.org, pabeni@...hat.com, richardcochran@...il.com
Subject: Re: [PATCH net-next 3/3] bnxt: Enforce PTP software freq adjustments
 only when in non-RTC mode

On 21/03/2023 10:32, Pavan Chebbi wrote:
> Currently driver performs software based frequency adjustments
> when RTC capability is not discovered or when in shared PHC mode.
> But there may be some old firmware versions that still support
> hardware freq adjustments without RTC capability being exposed.
> In this situation driver will use non-realtime mode even on single
> host NICs.
> 
> Hence enforce software frequency adjustments only when running in
> shared PHC mode. Make suitable changes for cyclecounter for the
> same.
> 
> Signed-off-by: Pavan Chebbi <pavan.chebbi@...adcom.com>
> Reviewed-by: Michael Chan <michael.chan@...adcom.com>
> ---
>   drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> index a3a3978a4d1c..b79a186f864c 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> @@ -230,7 +230,7 @@ static int bnxt_ptp_adjfine(struct ptp_clock_info *ptp_info, long scaled_ppm)
>   						ptp_info);
>   	struct bnxt *bp = ptp->bp;
>   
> -	if (BNXT_PTP_USE_RTC(bp))
> +	if (!BNXT_MH(ptp->bp))

bp is already resolved and stored in variable, it's better to use it

>   		return bnxt_ptp_adjfine_rtc(bp, scaled_ppm);
>   
>   	spin_lock_bh(&ptp->ptp_lock);
> @@ -861,9 +861,15 @@ static void bnxt_ptp_timecounter_init(struct bnxt *bp, bool init_tc)
>   		memset(&ptp->cc, 0, sizeof(ptp->cc));
>   		ptp->cc.read = bnxt_cc_read;
>   		ptp->cc.mask = CYCLECOUNTER_MASK(48);
> -		ptp->cc.shift = BNXT_CYCLES_SHIFT;
> -		ptp->cc.mult = clocksource_khz2mult(BNXT_DEVCLK_FREQ, ptp->cc.shift);
> -		ptp->cmult = ptp->cc.mult;
> +		if (BNXT_MH(ptp->bp)) {

and here, bp is the first argument to the function, why you do resolve 
again?

> +			/* Use timecounter based non-real time mode */
> +			ptp->cc.shift = BNXT_CYCLES_SHIFT;
> +			ptp->cc.mult = clocksource_khz2mult(BNXT_DEVCLK_FREQ, ptp->cc.shift);
> +			ptp->cmult = ptp->cc.mult;
> +		} else {
> +			ptp->cc.shift = 0;
> +			ptp->cc.mult = 1;
> +		}
>   		ptp->next_overflow_check = jiffies + BNXT_PHC_OVERFLOW_PERIOD;
>   	}
>   	if (init_tc)

Otherwise looks good!

Acked-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>