Re: [PATCH net-next v6 4/7] net: dsa: hellcreek: Add support for hardware timestamping

From: Kamil Alkhouri <kamil.alkhouri@hs-offenburg.de>
To: Kurt Kanzenbach <kurt@linutronix.de>,
	Vladimir Oltean <olteanv@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Richard Cochran <richardcochran@gmail.com>,
	ilias.apalodimas@linaro.org
Subject: Re: [PATCH net-next v6 4/7] net: dsa: hellcreek: Add support for hardware timestamping
Date: Thu, 08 Oct 2020 14:55:57 +0200	[thread overview]
Message-ID: <f040ba36070dd1e07b05cc63a392d8267ce4efe2.camel@hs-offenburg.de> (raw)
In-Reply-To: <87lfghhw9u.fsf@kurt>

Hello dears,

On Thu, 2020-10-08 at 12:01 +0200, Kurt Kanzenbach wrote:
> On Thu Oct 08 2020, Vladimir Oltean wrote:
> > On Thu, Oct 08, 2020 at 10:34:11AM +0200, Kurt Kanzenbach wrote:
> > > On Wed Oct 07 2020, Vladimir Oltean wrote:
> > > > On Wed, Oct 07, 2020 at 12:39:49PM +0200, Kurt Kanzenbach
> > > > wrote:
> > > > > For instance the hellcreek switch has actually three ptp
> > > > > hardware
> > > > > clocks and the time stamping can be configured to use either
> > > > > one of
> > > > > them.
> > > > 
> > > > The sja1105 also has a corrected and an uncorrected PTP clock
> > > > that can
> > > > take timestamps. Initially I had thought I'd be going to spend
> > > > some time
> > > > figuring out multi-PHC support, but now I don't see any
> > > > practical reason
> > > > to use the uncorrected PHC for anything.
> > > 
> > > Just out of curiosity: How do you implement 802.1AS then? My
> > > understanding is that the free-running clock has to be used for
> > > the
> > 
> > Has to be? I couldn't find that wording in IEEE 802.1AS-2011.
> 
> It doesn't has to be, it *should* be. That's at least the outcome we
> had
> after lots of discussions. Actually Kamil (on Cc) is the expert on
> this
> topic.

According to 802.1AS-2011 (10.1.1): "The LocalClock entity is a free-
running clock (see 3.3) that provides a common time to the time-aware
system, relative to an arbitrary epoch.", "... All timestamps are taken
relative to the LocalClock entity". The same statement holds true for
802.1AS-2020 (10.1.2.1).

> > > calculation of the peer delays and such meaning there should be a
> > > way to
> > > get access to both PHCs or having some form of cross timestamping
> > > available.
> > > 
> > > The hellcreek switch can take cross snapshots of all three ptp
> > > clocks in
> > > hardware for that purpose.
> > 
> > Well, at the end of the day, all the other TSN offloads (tc-taprio,
> > tc-gate) will still have to use the synchronized PTP clock, so what
> > we're doing is we're simply letting that clock be synchronized by
> > ptp4l.
> 
> Yes, the synchronized clock is of course needed for the traffic
> scheduling and so on. This is what we do here in this code as well.
> Only
> the synchronized one is exported to user space and used. However, the
> multi PHCs issue should be addressed as well at some point.
> 
> > > > > > So when you'll poll for TX timestamps, you'll receive a TX
> > > > > > timestamp from the PHY and another one from the switch, and
> > > > > > those will
> > > > > > be in a race with one another, so you won't know which one
> > > > > > is which.
> > > > > 
> > > > > OK. So what happens if the driver will accept to disable
> > > > > hardware
> > > > > timestamping? Is there anything else that needs to be
> > > > > implemented? Are
> > > > > there (good) examples?
> > > > 
> > > > It needs to not call skb_complete_tx_timestamp() and friends.
> > > > 
> > > > For PHY timestamping, it also needs to invoke the correct
> > > > methods for RX
> > > > and for TX, where the PHY timestamping hooks will get called. I
> > > > don't
> > > > think that DSA is compatible yet with PHY timestamping, but it
> > > > is
> > > > probably a trivial modification.
> > > 
> > > Hmm? If DSA doesn't support PHY timestamping how are other DSA
> > > drivers
> > > dealing with it then? I'm getting really confused.
> > 
> > They aren't dealing with it, of course.
> > 
> > > Furthermore, there is no hellcreek hardware available with
> > > timestamping
> > > capable PHYs. How am I supposed to even test this?
> > > 
> > > For now, until there is hardware available, PHY timestamping is
> > > not
> > > supported with the hellcreek switch.
> > 
> > I was just pointing out that this is something you'll certainly
> > have to
> > change if somebody will want PHY timestamping.
> 
> Understood.
> 
> > Even without hardware, you _could_ probably test that DSA is doing
> > the
> > right thing by simply adding the PTP timestamping ops to a PHY
> > driver
> > that you own, and inject dummy timestamps. The expectation becomes
> > that
> > user space gets those dummy timestamps, and not the ones emitted by
> > your
> > switch.
> 
> Of course it can be mocked. Whenever somebody wants to do PHY
> timestamping with a hellcreek switch this issue can be re-visited.
> 
> Thanks,
> Kurt