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

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

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@broadcom.com>
> Reviewed-by: Michael Chan <michael.chan@broadcom.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@linux.dev>