Re: [PATCH net-next 3/3] ptp: idt82p33: optimize _idt82p33_adjfine

From: Richard Cochran
Date: Wed Nov 04 2020 - 11:47:02 EST


On Wed, Nov 04, 2020 at 11:01:49AM -0500, min.li.xe@xxxxxxxxxxx wrote:
> From: Min Li <min.li.xe@xxxxxxxxxxx>
>
> Use div_s64 so that the neg_adj is not needed.

Back in the day, I coded the neg_adj because there was some issue with
signed 64 bit division that I can't recall now. Either div_s64 didn't
exist or it was buggy on some archs... there was _some_ reason.

So unless you are sure that this works on all platforms, I would leave
it alone.

Thanks,
Richard


> Signed-off-by: Min Li <min.li.xe@xxxxxxxxxxx>
> ---
> drivers/ptp/ptp_idt82p33.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/ptp/ptp_idt82p33.c b/drivers/ptp/ptp_idt82p33.c
> index b1528a0..e970379d 100644
> --- a/drivers/ptp/ptp_idt82p33.c
> +++ b/drivers/ptp/ptp_idt82p33.c
> @@ -320,7 +320,6 @@ static int _idt82p33_adjfine(struct idt82p33_channel *channel, long scaled_ppm)
> {
> struct idt82p33 *idt82p33 = channel->idt82p33;
> unsigned char buf[5] = {0};
> - int neg_adj = 0;
> int err, i;
> s64 fcw;
>
> @@ -340,16 +339,9 @@ static int _idt82p33_adjfine(struct idt82p33_channel *channel, long scaled_ppm)
> * FCW = -------------
> * 168 * 2^4
> */
> - if (scaled_ppm < 0) {
> - neg_adj = 1;
> - scaled_ppm = -scaled_ppm;
> - }
>
> fcw = scaled_ppm * 244140625ULL;
> - fcw = div_u64(fcw, 2688);
> -
> - if (neg_adj)
> - fcw = -fcw;
> + fcw = div_s64(fcw, 2688);
>
> for (i = 0; i < 5; i++) {
> buf[i] = fcw & 0xff;
> --
> 2.7.4
>