Re: [PATCH] net: lan743x_ptp: convert to ktime_get_clocktai_ts64

From: Arnd Bergmann <arnd@arndb.de>
To: Richard Cochran <richardcochran@gmail.com>
Cc: Bryan.Whitehead@microchip.com,
	UNGLinuxDriver@microchiptechnology.mail.onmicrosoft.com,
	David Miller <davem@davemloft.net>,
	YueHaibing <yuehaibing@huawei.com>,
	Networking <netdev@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net: lan743x_ptp: convert to ktime_get_clocktai_ts64
Date: Fri, 17 Aug 2018 21:29:56 +0200	[thread overview]
Message-ID: <CAK8P3a1XEDzGtxyZ3Q6=jZe+w67r_ZakksAEQVDC8pYSCne55A@mail.gmail.com> (raw)
In-Reply-To: <20180817162558.GB22210@eeboy>

On Fri, Aug 17, 2018 at 6:26 PM Richard Cochran
<richardcochran@gmail.com> wrote:
>
> On Wed, Aug 15, 2018 at 08:50:03PM +0000, Bryan.Whitehead@microchip.com wrote:
> > Sounds reasonable to me. I will yield to Richard's insight.
> > But it would be nice if requirements like these were documented.
>
> I think the only reason for initializing to the system UTC (as most
> drivers do) is historical.  The first Intel driver was simply copied.
> I've been thinking recently that we should standardize this.  I'm open
> for suggestions on how to do this. Remember that the system time is
> likely wrong at driver initialization time.

Ah, so you mean the other drivers in fact do initialize the hardware clock,
they just set it to UTC instead of TAI? I must have looked wrong then,
but I see it now in most implementations (exceptions include ravb, sfc
and stmmac).

This certainly seems to be an "interesting" problem, given that the Linux
implementations (other than the new lan743x) then start out with UTC,
while the PTP spec mandates TAI. So even if the system clock is
perfectly synchronized to UTC at boot, we set the PTP hardware 37
seconds slow.

It would not be hard to change all PTP drivers to explicitly initialize to
TAI, but that might create its own set of problems if random code
depends on the current behavior.

I also see that "phc_ctl /dev/ptp0 set" defaults to CLOCK_REALTIME
and has no option to use CLOCK_TAI instead. How is this meant to
work? I see a lot of other code that tries to deal with leap seconds and
the tai offset, so I hope I was just misreading that code.

       Arnd