From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3F321C433E0 for ; Fri, 19 Jun 2020 13:40:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 26B8320DD4 for ; Fri, 19 Jun 2020 13:40:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1733010AbgFSNkH (ORCPT ); Fri, 19 Jun 2020 09:40:07 -0400 Received: from vps0.lunn.ch ([185.16.172.187]:48906 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726124AbgFSNkH (ORCPT ); Fri, 19 Jun 2020 09:40:07 -0400 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1jmHFR-001HN0-TZ; Fri, 19 Jun 2020 15:40:01 +0200 Date: Fri, 19 Jun 2020 15:40:01 +0200 From: Andrew Lunn To: Kurt Kanzenbach Cc: Vivien Didelot , Florian Fainelli , "David S. Miller" , Jakub Kicinski , netdev@vger.kernel.org, Rob Herring , devicetree@vger.kernel.org, Sebastian Andrzej Siewior , Richard Cochran , Kamil Alkhouri , ilias.apalodimas@linaro.org Subject: Re: [RFC PATCH 3/9] net: dsa: hellcreek: Add PTP clock support Message-ID: <20200619134001.GC304147@lunn.ch> References: <20200618064029.32168-1-kurt@linutronix.de> <20200618064029.32168-4-kurt@linutronix.de> <20200618172304.GG240559@lunn.ch> <878sgjqx4r.fsf@kurt> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <878sgjqx4r.fsf@kurt> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Jun 19, 2020 at 10:26:44AM +0200, Kurt Kanzenbach wrote: > Hi Andrew, > > On Thu Jun 18 2020, Andrew Lunn wrote: > >> +static u64 __hellcreek_ptp_clock_read(struct hellcreek *hellcreek) > >> +{ > >> + u16 nsl, nsh, secl, secm, sech; > >> + > >> + /* Take a snapshot */ > >> + hellcreek_ptp_write(hellcreek, PR_COMMAND_C_SS, PR_COMMAND_C); > >> + > >> + /* The time of the day is saved as 96 bits. However, due to hardware > >> + * limitations the seconds are not or only partly kept in the PTP > >> + * core. That's why only the nanoseconds are used and the seconds are > >> + * tracked in software. Anyway due to internal locking all five > >> + * registers should be read. > >> + */ > >> + sech = hellcreek_ptp_read(hellcreek, PR_SS_SYNC_DATA_C); > >> + secm = hellcreek_ptp_read(hellcreek, PR_SS_SYNC_DATA_C); > >> + secl = hellcreek_ptp_read(hellcreek, PR_SS_SYNC_DATA_C); > >> + nsh = hellcreek_ptp_read(hellcreek, PR_SS_SYNC_DATA_C); > >> + nsl = hellcreek_ptp_read(hellcreek, PR_SS_SYNC_DATA_C); > >> + > >> + return (u64)nsl | ((u64)nsh << 16); > > > > Hi Kurt > > > > What are the hardware limitations? There seems to be 48 bits for > > seconds? That allows for 8925104 years? > > In theory, yes. Due to hardware hardware considerations only a few or > none of these bits are used for the seconds. The rest is zero. Meaning > that the wraparound is not 8925104 years, but at e.g. 8 seconds when > using 3 bits for the seconds. Please add this to the comment. > >> +static u64 __hellcreek_ptp_gettime(struct hellcreek *hellcreek) > >> +{ > >> + u64 ns; > >> + > >> + ns = __hellcreek_ptp_clock_read(hellcreek); > >> + if (ns < hellcreek->last_ts) > >> + hellcreek->seconds++; > >> + hellcreek->last_ts = ns; > >> + ns += hellcreek->seconds * NSEC_PER_SEC; > > > > So the assumption is, this gets called at least once per second. And > > if that does not happen, there is no recovery. The second is lost. > > Yes, exactly. If a single overflow is missed, then the time is wrong. > > > > > I'm just wondering if there is something more robust using what the > > hardware does provide, even if the hardware is not perfect. > > I don't think there's a more robust way to do this. The overflow period > is a second which should be enough time to catch the overflow even if > the system is loaded. We did long running tests for days and the > mechanism worked fine. We could also consider to move the delayed work > to a dedicated thread which could be run with real time (SCHED_FIFO) > priority. But, I didn't see the need for it. If the hardware does give you 3 working bits for the seconds, you could make use of that for a consistency check. If nothing else, you could do a dev_err(dev, 'PTP time is FUBAR'); Andrew