Re: [Linuxptp-users] Interesting (very wrong) PTP output

[Keller, J. E <jac...@in...>]

> -----Original Message-----
> From: Christian Riesch [mailto:chr...@om...]
> Sent: Thursday, June 07, 2012 2:55 AM
> To: ric...@gm...; jak...@in...
> Cc: lin...@li...
> Subject: Re: [Linuxptp-users] Interesting (very wrong) PTP output
> 
> Hi Jake and Richard,
> 
> >On Tue, Jun 05, 2012 at 11:31:24PM +0000, Keller, Jacob E wrote:
> >> It seems like somehow the p/i servo code is incorrect because once we
> >> hit max adjust it never seems to move away from it.
> >
> > The servo clamps the adjustment to the maximum. It looks like it the
> > servo output is always outside of the limit. You can inspect s->drift
> > to get a better idea what is going on.
> 
> I agree, looks like something is going wrong with s->drift, it seems to
> get set to a very high value and does not recover from that.
> 
> Since the value of s->drift is clamped to the maximum adjustment in pi.c
> when
> s->count == 4, it must be the initial calculation for s->count == 2.
> s->This
> calculation tries to get a good estimate for s->drift to allow a faster
> settling of the control loop. However, there is no clamping of the value
> of s->drift in this case.
> 
> What are your scripts doing before ptp4l is started? Do they do some clock
> adjustments? Note that the initial calculation of s->drift only works if
> the clock is initially set to 0 ppb, otherwise some correction of the
> calculated s->drift must be done.
> 
> Finally, if we calculate an estimate for s->drift, we should also apply it
> to the clock.
> 
> Please find below a patch (it's only an quick hack and only compile-
> tested) that
> a) sets the clock to 0 ppb before doing anything else,
> b) clamps s->drift to the maximum adjustment value at the first
> calculation
>    (estimation),
> c) applies the estimate to the clock.
> 
> Regards,
> Christian
> 
> ---
>  clock.c |    3 +--
>  pi.c    |   25 +++++++++++++++++--------
>  2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/clock.c b/clock.c
> index e3797bf..c8216c3 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -475,13 +475,12 @@ enum servo_state clock_synchronize(struct clock *c,
>  		c->master_offset, state, adj, c->path_delay);
> 
>  	switch (state) {
> -	case SERVO_UNLOCKED:
> -		break;
>  	case SERVO_JUMP:
>  		clock_step(c->clkid, -c->master_offset);
>  		c->t1 = tmv_zero();
>  		c->t2 = tmv_zero();
>  		break;
> +	case SERVO_UNLOCKED:
>  	case SERVO_LOCKED:
>  		clock_ppb(c->clkid, -adj);
>  		break;
> diff --git a/pi.c b/pi.c
> index 33766b1..743ae71 100644
> --- a/pi.c
> +++ b/pi.c
> @@ -59,28 +59,37 @@ static double pi_sample(struct servo *servo,
> 
>  	switch (s->count) {
>  	case 0:
> -		s->offset[0] = offset;
> -		s->local[0] = local_ts;
>  		*state = SERVO_UNLOCKED;
> -		s->count = 1;
>  		break;
>  	case 1:
> -		s->offset[1] = offset;
> -		s->local[1] = local_ts;
> +		s->offset[0] = offset;
> +		s->local[0] = local_ts;
>  		*state = SERVO_UNLOCKED;
>  		s->count = 2;
>  		break;
>  	case 2:
> -		s->drift = (s->offset[1] - s->offset[0]) /
> -			(s->local[1] - s->local[0]);
> +		s->offset[1] = offset;
> +		s->local[1] = local_ts;
>  		*state = SERVO_UNLOCKED;
>  		s->count = 3;
>  		break;
>  	case 3:
> -		*state = SERVO_JUMP;
> +		s->drift = (s->offset[1] - s->offset[0]) /
> +			(s->local[1] - s->local[0]);
> +		if (s->drift < -s->maxppb) {
> +			s->drift = -s->maxppb;
> +		} else if (s->drift > s->maxppb) {
> +			s->drift = s->maxppb;
> +		}
> +		ppb = s->drift;
> +		*state = SERVO_UNLOCKED;
>  		s->count = 4;
>  		break;
>  	case 4:
> +		*state = SERVO_JUMP;
> +		s->count = 5;
> +		break;
> +	case 5:
>  		ki_term = s->ki * offset;
>  		ppb = s->kp * offset + s->drift + ki_term;
>  		if (ppb < -s->maxppb) {
> --
> 1.7.4.1
> 
> 
> --------------------------------------------------------------------------
> ----
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> Linuxptp-users mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linuxptp-users

I found an issue with the driver for the network adapter, that fixed the issue. I am not sure what this patch is trying to do. But I do think applying a ppb adjustment of 0 to the device originally can be useful to ensure its in a correct state during the unlocked stage.

- Jake