Re: [RFC PATCH net-next 7/9] net: dsa: microchip: ksz9477: add hardware time stamping support

From: Vladimir Oltean <olteanv@gmail.com>
To: Richard Cochran <richardcochran@gmail.com>
Cc: Christian Eggers <ceggers@arri.de>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Helmut Grohne <helmut.grohne@intenta.de>,
	Paul Barker <pbarker@konsulko.com>,
	Codrin Ciubotariu <codrin.ciubotariu@microchip.com>,
	George McCollister <george.mccollister@gmail.com>,
	Marek Vasut <marex@denx.de>,
	Tristram Ha <Tristram.Ha@microchip.com>,
	"David S . Miller" <davem@davemloft.net>,
	Woojung Huh <woojung.huh@microchip.com>,
	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH net-next 7/9] net: dsa: microchip: ksz9477: add hardware time stamping support
Date: Thu, 22 Oct 2020 13:50:14 +0300	[thread overview]
Message-ID: <20201022105014.gflswfpie4qvbw3h@skbuf> (raw)
In-Reply-To: <20201022090126.h64hfnlajqelveku@skbuf>

On Thu, Oct 22, 2020 at 12:01:26PM +0300, Vladimir Oltean wrote:
> On Wed, Oct 21, 2020 at 08:02:17PM -0700, Richard Cochran wrote:
> > On Thu, Oct 22, 2020 at 02:39:35AM +0300, Vladimir Oltean wrote:
> > > So how _does_ that work for TI PHYTER?
> > >
> > > As far as we understand, the PHYTER appears to autonomously mangle PTP packets
> > > in the following way:
> > > - subtracting t2 on RX from the correctionField of the Pdelay_Req
> > > - adding t3 on TX to the correctionField of the Pdelay_Resp
> >
> > The Phyter does not support peer-to-peer one step.
> 
> Ok, that's my mistake for not double-checking, sorry.
> 
> > The only driver that implements it is ptp_ines.c.
> >
> > And *that* driver/HW implements it correctly.
> 
> Is there documentation available for this timestamping block? I might be
> missing some data, as the kernel driver is fairly pass-through for the
> TX timestamping of the Pdelay_Resp, so the hardware might just 'do the
> right thing'. I believe the answer lies within the timestamper's
> per-port RX FIFO. Just guessing here, but I suspect that the RX FIFO
> doesn't get cleared immediately after the host queries it, and the
> hardware caches the last 100 events from the pool and uses the RX
> timestamps of Pdelay_Req as t2 in the process of updating the
> correctionField of Pdelay_Resp on TX. Would that be correct?

Ah, no, I think I'm wrong. As per your own description here:
https://lwn.net/Articles/807970/

If the hardware supports p2p one-step, it subtracts the ingress time
stamp value from the Pdelay_Request correction field.  The user space
software stack then simply copies the correction field into the
Pdelay_Response, and on transmission the hardware adds the egress time
stamp into the correction field.

So we were correct about the behavior, just not about the target
hardware.

So, that's just not how this hardware works. What do you recommend?
Keeping a FIFO of Pdelay_Req RX timestamps, and matching them to
Pdelay_Resp messages on TX, all of that contained within tag_ksz.c?