All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Organov <sorganov@gmail.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: richardcochran@gmail.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Fugang Duan <fugang.duan@nxp.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>
Subject: Re: [PATCH  3/5] net: fec: initialize clock with 0 rather than current kernel time
Date: Mon, 06 Jul 2020 21:24:24 +0300	[thread overview]
Message-ID: <874kqksdrb.fsf@osv.gnss.ru> (raw)
In-Reply-To: <20200706152721.3j54m73bm673zlnj@skbuf> (Vladimir Oltean's message of "Mon, 6 Jul 2020 18:27:21 +0300")

Vladimir Oltean <olteanv@gmail.com> writes:

> Hi Sergey,
>
> On Mon, Jul 06, 2020 at 05:26:14PM +0300, Sergey Organov wrote:
>> Initializing with 0 makes it much easier to identify time stamps from
>> otherwise uninitialized clock.
>> 
>> Initialization of PTP clock with current kernel time makes little sense as
>> PTP time scale differs from UTC time scale that kernel time represents.
>> It only leads to confusion when no actual PTP initialization happens, as
>> these time scales differ in a small integer number of seconds (37 at the
>> time of writing.)
>> 
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>
> Reading your patch, I got reminded of my own attempt of making an
> identical change to the ptp_qoriq driver:
>
> https://www.spinics.net/lists/netdev/msg601625.html
>
> Could we have some sort of kernel-wide convention, I wonder (even though
> it might be too late for that)? After your patch, I can see equal
> amounts of confusion of users expecting some boot-time output of
> $(phc_ctl /dev/ptp0 get) as it used to be, and now getting something
> else.
>
> There's no correct answer, I'm afraid.

IMHO, the correct answer would be keep non-initialized clock at 0. No
ticking.

> Whatever the default value of the clock may be, it's bound to be
> confusing for some reason, _if_ the reason why you're investigating it
> in the first place is a driver bug. Also, I don't really see how your
> change to use Jan 1st 1970 makes it any less confusing.

When I print the clocks in application, I see seconds and milliseconds
part since epoch. With this patch seconds count from 0, that simply
match uptime. Easy to tell from any other (malfunctioning) clock.

Here is the description of confusion and improvement. I spent half a day
not realizing that I sometimes get timestamps from the wrong PTP clock.
Part of the problem is that kernel time at startup, when it is used for
initialization of the PTP clock, is in fact somewhat random, and it
could be off by a few seconds. Now, when in application I get time stamp
that is almost right, and then another one that is, say, 9 seconds off,
what should I think? Right, that I drive PTP clock wrongly.

Now, when one of those timestamps is almost 0, I see immediately I got
time from wrong PTP clock, rather than wrong time from correct PTP
clock.

Thanks,
-- Sergey

  reply	other threads:[~2020-07-06 18:24 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 14:26 [PATCH 0/5] net: fec: fix external PTP PHY support Sergey Organov
2020-07-06 14:26 ` [PATCH 1/5] net: fec: properly support external PTP PHY for hardware time stamping Sergey Organov
2020-07-06 15:08   ` Vladimir Oltean
2020-07-06 15:21     ` Sergey Organov
2020-07-06 15:47       ` Vladimir Oltean
2020-07-06 18:33         ` Sergey Organov
2020-07-07  7:04           ` Vladimir Oltean
2020-07-07 15:29             ` Sergey Organov
2020-07-08 11:00             ` Richard Cochran
2020-07-08 10:55           ` Richard Cochran
2020-07-06 14:26 ` [PATCH 2/5] net: fec: enable to use PPS feature without " Sergey Organov
2020-07-07  4:05   ` [EXT] " Andy Duan
2020-07-07 14:29     ` Sergey Organov
2020-07-06 14:26 ` [PATCH 3/5] net: fec: initialize clock with 0 rather than current kernel time Sergey Organov
2020-07-06 15:27   ` Vladimir Oltean
2020-07-06 18:24     ` Sergey Organov [this message]
2020-07-07  6:36       ` Vladimir Oltean
2020-07-07 16:07         ` Sergey Organov
2020-07-07 16:43           ` Vladimir Oltean
2020-07-07 17:09             ` Sergey Organov
2020-07-07 17:12               ` Vladimir Oltean
2020-07-07 17:56                 ` Sergey Organov
2020-07-08 11:15                   ` Richard Cochran
2020-07-08 12:14                     ` Sergey Organov
2020-07-08 11:11             ` Richard Cochran
2020-07-08 11:04     ` Richard Cochran
2020-07-08 12:24       ` Sergey Organov
2020-07-08 12:37       ` Sergey Organov
2020-07-08 14:48         ` Richard Cochran
2020-07-08 17:18           ` Sergey Organov
2020-07-06 14:26 ` [PATCH 4/5] net: fec: get rid of redundant code in fec_ptp_set() Sergey Organov
2020-07-07  4:08   ` [EXT] " Andy Duan
2020-07-07 14:43     ` Sergey Organov
2020-07-08  5:34       ` Andy Duan
2020-07-08  8:48         ` Sergey Organov
2020-07-08  8:57           ` Andy Duan
2020-07-08 12:26             ` Sergey Organov
2020-07-06 14:26 ` [PATCH 5/5] net: fec: replace snprintf() with strlcpy() in fec_ptp_init() Sergey Organov
2020-07-11 12:08 ` [PATCH v2 net] net: fec: fix hardware time stamping by external devices Sergey Organov
2020-07-11 23:19   ` Vladimir Oltean
2020-07-12 14:16     ` Sergey Organov
2020-07-12 14:47       ` Andrew Lunn
2020-07-12 15:01       ` Vladimir Oltean
2020-07-12 17:29         ` Sergey Organov
2020-07-12 19:33           ` Vladimir Oltean
2020-07-12 22:32             ` Sergey Organov
2020-07-12 23:15               ` Vladimir Oltean
2020-07-14 12:39                 ` Sergey Organov
2020-07-14 14:23                   ` Vladimir Oltean
2020-07-14 14:35                     ` Sergey Organov
2020-07-14 14:44                     ` Vladimir Oltean
2020-07-14 16:18                       ` Sergey Organov
2020-07-14 14:01   ` Richard Cochran
2020-07-14 14:27     ` Sergey Organov
2020-07-14 16:28 ` [PATCH v3 " Sergey Organov
2020-07-16 18:24   ` Jakub Kicinski
2020-07-16 20:38     ` Sergey Organov
2020-07-16 21:06       ` Jakub Kicinski
2020-07-16 21:18         ` Sergey Organov
2020-07-15 15:42 ` [PATCH net-next v2 0/4] net: fec: a few improvements Sergey Organov
2020-07-15 15:42   ` [PATCH net-next v2 1/4] net: fec: enable to use PPS feature without time stamping Sergey Organov
2020-07-15 15:42   ` [PATCH net-next v2 2/4] net: fec: initialize clock with 0 rather than current kernel time Sergey Organov
2020-07-15 15:42   ` [PATCH net-next v2 3/4] net: fec: get rid of redundant code in fec_ptp_set() Sergey Organov
2020-07-15 15:43   ` [PATCH net-next v2 4/4] net: fec: replace snprintf() with strlcpy() in fec_ptp_init() Sergey Organov
2020-07-16  3:00   ` [EXT] [PATCH net-next v2 0/4] net: fec: a few improvements Andy Duan
2020-07-16 18:37     ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874kqksdrb.fsf@osv.gnss.ru \
    --to=sorganov@gmail.com \
    --cc=davem@davemloft.net \
    --cc=fugang.duan@nxp.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=richardcochran@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.