Re: [Linuxptp-users] master offset is increased in some condition

[Richard C. <ric...@gm...>]

On Mon, Mar 12, 2012 at 02:42:10PM +0900, Takahiro Shimizu wrote:
> Hello,
> 
> I faced the issue that the master offset is increased in some condition.

...
 
> This issue is happened when adj is -50000000 or 50000000.
> If the adj is -50000000 with s2(SERVO_LOCKED), clock_ppb is called with
> -adj.
> 
> enum servo_state clock_synchronize(struct clock *c,
> :
> :
> case SERVO_LOCKED:
> clock_ppb(c->clkid, -adj);
> :
> 
> 
> static void clock_ppb(clockid_t clkid, double ppb)
> {
> struct timex tx;
> memset(&tx, 0, sizeof(tx));
> tx.modes = ADJ_FREQUENCY;
> tx.freq = (long) (ppb * 65.536);
> if (clock_adjtime(clkid, &tx) < 0)
> pr_err("failed to adjust the clock: %m");
> }
> If ppb is 50000000, then the result is 50000000 * 65.536 = 3276800000.
> tx.freq = (long)3276800000 = -1018167296.
> I think the tx.freq is overflowed. This is unexpected result.
> 
> I think tx.freq should be under 0x7fffffff.
> If so, the max adh should be 0x7fffffff/65.536 = 32767999.9847...

Yes, you are right, for 32 bit machines.

The problem is that the PHC driver API has the max_adj in ppb, but the
NTP adjtime API has the frequency adjustment field as ppm with a 16
bit fractional part.

In the PHC driver API the field is four bytes (type s32), but in the
NTP adjtime API the field is either four or eight bytes (type long),
depending on the machine architecture.
 
> If I chage the EG20T ptp driver, the result seems OK.
> 
> static struct ptp_clock_info ptp_pch_caps = {
> .owner = THIS_MODULE,
> .name = "PCH timer",
> // .max_adj = 50000000,
> .max_adj = 32767999, // Changed the max_adj by TS.
> 
> (Question)
> 1. Is my modification correct ?
> 2. Is it better to correct it in another part?

I think we should change ptp4l to limit the max_adj according to
the driver's max_adj and sizeof(long).

Thanks,
Richard