On Fri, Mar 25, 2022 at 12:52:02AM +0000, Keller, Jacob E wrote: > > -----Original Message----- > > From: Vladimir Oltean <olte...@gmail.com> > > Sent: Thursday, March 24, 2022 5:45 PM > > To: Keller, Jacob E <jacob.e.kel...@intel.com> > > Cc: ch...@chriscaudle.org; linuxptp-users@lists.sourceforge.net > > Subject: Re: [Linuxptp-users] phc2sys not synchr. to right time and date > > > > On Fri, Mar 25, 2022 at 12:27:53AM +0000, Keller, Jacob E wrote: > > > > (there might be other reasons why his setup is working, but) > > > > Some kernel Ethernet drivers have a bad habit of writing the current > > > > (UTC) system time (derived from the battery-backed RTC) into the PHC > > > > current (TAI) time at boot. You're off by 37 seconds, but hey, at least > > > > it's 2022, so the time _must_ be right! > > > > https://elixir.bootlin.com/linux/latest/source/drivers/ptp/ptp_qoriq.c#L504 > > > > > > If there's a strong recommendation against doing this, should we go > > > fix the in-tree drivers to avoid doing so? > > > > Everybody seems to have their own opinion on the topic. > > > > At one point I submitted a patch to change the ptp_qoriq driver (shared > > by a bunch of Freescale/NXP Ethernet drivers) to stop doing this, > > Richard said it's not worth it to make the change. > > > > I think most of the Intel parts also initialize the time at driver load. > > > Then Sergey Organov came with a FEC driver bug which made the FEC emit a > > duplicate TX timestamp each time the attached PHY also emitted a TX > > timestamp. He said "hey, I would have figured out much quicker what was > > going on, if the FEC didn't have the same rough time base as the PHY, > > despite me not attempting to synchronize it to anything!". Richard > > became more responsive to this argument and, as far as I see, is now > > suggesting driver writers to let the PTP time be what it is when the > > driver probes, instead of setting it to a basically different random value. > > > > Leaving whatever the PTP time is alone? This typically would have it > be zero in Intel hardware, which would get reported as offset since > 1970..
Yes, that would be the idea. > > As for myself, I had an issue with the stmmac driver where it was doing > > this PTP time reinitialization with the system time, but not only at > > boot, but rather each time the timestamping ioctl was sent by user space > > (i.e. each time I started ptp4l). That was actively bothering me, > > because I was also running phc2sys in the background, and the offset > > kept resetting to 37 every time I ran a test application which enabled > > timestamping, so I had to change that with a kernel patch. > > Yes that sounds like absolutely horrid approach. Definitely shouldn't > get setting the time outside of initialization. > > > > > I expect that if I return with the ptp_qoriq patch now, after all the > > discussions that took place in the meantime, it would be accepted, > > at least for the sake of uniformity if nothing else. > > > > In general, we all seem to agree that the initial PTP time is largely > > irrelevant until you start looking at it for some particular reason > > (debugging). Or at least, almost everyone seems to agree. I remember > > Miroslav Lichvar argued that an initial offset of 37 seconds is somehow > > better than an initial offset from 1970 from 2022 because of the smaller > > clock jump? I admit I didn't quite get that. > > Smaller offsets are easier to correct. At least in some of the Intel > hardware it can do atomic corrections for small offsets, but cannot do > so for large offsets. At least in the NXP world I'm living in, this problem doesn't really exist. A clock step is the same kind of operation regardless of offset it needs to correct (either non-atomic/read-modify-write: ptp_qoriq, or atomic: sja1105). As a side note I find it a bit strange to implement an atomic clock step method in hardware but limit the offset... I realize this isn't ideal (because kernel and user space isn't updated in lockstep), but maybe to account for both cases, ptp4l could make the initial servo's clockadj_step() be a clockadj_set_time()? _______________________________________________ Linuxptp-users mailing list Linuxptp-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-users
