Re: [Intel-wired-lan] [PATCH net-next v2] igb: refactor igb_ptp_adjfine_82580 to use diff_by_scaled_ppm

[Jacob Keller <jacob.e.keller@intel.com>]

On 3/8/2023 2:36 AM, Andrii Staikov wrote:
> Driver's .adjfine interface functions use adjust_by_scaled_ppm and
> diff_by_scaled_ppm introduced in commit 1060707e3809
> ("ptp: introduce helpers to adjust by scaled parts per million")
> to calculate the required adjustment in a concise manner,
> but not igb_ptp_adjfine_82580.
> Fix it by introducing IGB_82580_BASE_PERIODand changing function logic
> to use diff_by_scaled_ppm.
> 
> Fixes: 1060707e3809 ("ptp: introduce helpers to adjust by scaled parts per million")
> Signed-off-by: Andrii Staikov <andrii.staikov@intel.com>

Thanks for this. I missed this function when doing the refactor above.
This change looks correct to me.

I'm not sure this would qualify as a bug fix sent to net, but:

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> ---
> v2: Changing target to net-queue and reflowing commit message
> to fit 75 characters per line.

The subject still says this targets net-next, but I think that's fine.

> ---
>  drivers/net/ethernet/intel/igb/igb_ptp.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
> index 6f471b91f562..405886ee5261 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -67,6 +67,7 @@
>  #define INCVALUE_82576_MASK		GENMASK(E1000_TIMINCA_16NS_SHIFT - 1, 0)
>  #define INCVALUE_82576			(16u << IGB_82576_TSYNC_SHIFT)
>  #define IGB_NBITS_82580			40
> +#define IGB_82580_BASE_PERIOD		0x800000000
>  
>  static void igb_ptp_tx_hwtstamp(struct igb_adapter *adapter);
>  static void igb_ptp_sdp_init(struct igb_adapter *adapter);
> @@ -209,17 +210,11 @@ static int igb_ptp_adjfine_82580(struct ptp_clock_info *ptp, long scaled_ppm)
>  	struct igb_adapter *igb = container_of(ptp, struct igb_adapter,
>  					       ptp_caps);
>  	struct e1000_hw *hw = &igb->hw;
> -	int neg_adj = 0;
> +	bool neg_adj;
>  	u64 rate;
>  	u32 inca;
>  
> -	if (scaled_ppm < 0) {
> -		neg_adj = 1;
> -		scaled_ppm = -scaled_ppm;
> -	}
> -	rate = scaled_ppm;
> -	rate <<= 13;
> -	rate = div_u64(rate, 15625);
> +	neg_adj = diff_by_scaled_ppm(IGB_82580_BASE_PERIOD, scaled_ppm, &rate);
>  
>  	inca = rate & INCVALUE_MASK;
>  	if (neg_adj)
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan