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

From: Vladimir Oltean <olteanv@gmail.com>
To: Kurt Kanzenbach <kurt@linutronix.de>
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>,
	Kamil Alkhouri <kamil.alkhouri@hs-offenburg.de>,
	ilias.apalodimas@linaro.org
Subject: Re: [PATCH net-next v6 4/7] net: dsa: hellcreek: Add support for hardware timestamping
Date: Thu, 8 Oct 2020 12:44:40 +0300	[thread overview]
Message-ID: <20201008094440.oede2fucgpgcfx6a@skbuf> (raw)
In-Reply-To: <87362pjev0.fsf@kurt>

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.

> 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.

> >> > 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.

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.