On Tue, Jul 30, 2019 at 12:04:29PM +0200, Hubert Feurstein wrote: > diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c > index 768d256f7c9f..51cdf4712517 100644 > --- a/drivers/net/dsa/mv88e6xxx/ptp.c > +++ b/drivers/net/dsa/mv88e6xxx/ptp.c > @@ -15,11 +15,38 @@ > #include "hwtstamp.h" > #include "ptp.h" > > -/* Raw timestamps are in units of 8-ns clock periods. */ > -#define CC_SHIFT 28 > -#define CC_MULT (8 << CC_SHIFT) > -#define CC_MULT_NUM (1 << 9) > -#define CC_MULT_DEM 15625ULL > +/* The adjfine API clamps ppb between [-32,768,000, 32,768,000], and
That is not true. > + * therefore scaled_ppm between [-2,147,483,648, 2,147,483,647]. > + * Set the maximum supported ppb to a round value smaller than the maximum. > + * > + * Percentually speaking, this is a +/- 0.032x adjustment of the > + * free-running counter (0.968x to 1.032x). > + */ > +#define MV88E6XXX_MAX_ADJ_PPB 32000000 I had set an arbitrary limit of 1000 ppm. I can't really see any point in raising the limit. > +/* Family MV88E6250: > + * Raw timestamps are in units of 10-ns clock periods. > + * > + * clkadj = scaled_ppm * 10*2^28 / (10^6 * 2^16) > + * simplifies to > + * clkadj = scaled_ppm * 2^7 / 5^5 > + */ > +#define MV88E6250_CC_SHIFT 28 > +#define MV88E6250_CC_MULT (10 << MV88E6250_CC_SHIFT) > +#define MV88E6250_CC_MULT_NUM (1 << 7) > +#define MV88E6250_CC_MULT_DEM 3125ULL > + > +/* Other families: > + * Raw timestamps are in units of 8-ns clock periods. > + * > + * clkadj = scaled_ppm * 8*2^28 / (10^6 * 2^16) > + * simplifies to > + * clkadj = scaled_ppm * 2^9 / 5^6 > + */ > +#define MV88E6XXX_CC_SHIFT 28 > +#define MV88E6XXX_CC_MULT (8 << MV88E6XXX_CC_SHIFT) > +#define MV88E6XXX_CC_MULT_NUM (1 << 9) > +#define MV88E6XXX_CC_MULT_DEM 15625ULL > > #define TAI_EVENT_WORK_INTERVAL msecs_to_jiffies(100) > > @@ -179,24 +206,14 @@ static void mv88e6352_tai_event_work(struct work_struct > *ugly) > static int mv88e6xxx_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) > { > struct mv88e6xxx_chip *chip = ptp_to_chip(ptp); > - int neg_adj = 0; > - u32 diff, mult; > - u64 adj; > + s64 adj; > > - if (scaled_ppm < 0) { > - neg_adj = 1; > - scaled_ppm = -scaled_ppm; > - } Please don't re-write this logic. It is written like that for a reason. > - mult = CC_MULT; > - adj = CC_MULT_NUM; > - adj *= scaled_ppm; > - diff = div_u64(adj, CC_MULT_DEM); Just substitute CC_MULT* with your new chip->ptp_cc_mult* and leave the rest alone. Thanks, Richard