From: Christian Eggers <ceggers@arri.de>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Richard Cochran <richardcochran@gmail.com>,
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: Wed, 11 Nov 2020 22:50:22 +0100 [thread overview]
Message-ID: <2269731.xUjYGUPlbM@n95hx1g2> (raw)
In-Reply-To: <20201110193245.uwsmrqzio5hco7fb@skbuf>
Hi Vladimir,
On Tuesday, 10 November 2020, 20:32:45 CET, Vladimir Oltean wrote:
> On Tue, Nov 10, 2020 at 06:40:45PM +0200, Vladimir Oltean wrote:
> > I am fairly confident that this is how your hardware works, because
> > that's also how peer delay wants to be timestamped, it seems.
>
> So I was confident and also wrong, it appears. My KSZ9477 datasheet
> says:
>
> In the host-to-switch direction, the 4-byte timestamp field is always
> present when PTP is enabled, as shown in Figure 4-6. This is true for
> all packets sent by the host, including IBA packets. The host uses this
> field to insert the receive timestamp from PTP Pdelay_Req messages into
> the Pdelay_Resp messages. For all other traffic and PTP message types,
> the host should populate the timestamp field with zeros.
>
> Hm. Does that mean that the switch updates the originTimestamp field of
> the Sync frames by itself?
IMHO this is the best solution. User space / driver do not know the exact time
(would require slow I2C transfer). So inserting the time in hardware seems to
be the better solution. Maybe this is what the data sheet meant with "egress
timestamp insertion".
> Ok... Very interesting that they decided to
> introduce a field in the tail tag for a single type of PTP message.
> But something is still wrong if you need to special-case the negative
> correctionField, it looks like the arithmetic is not done on the correct
> number of bits, either by the driver or by the hardware.
>
Maybe I found the formula which is (should) applied to the correction field
for PDelayResp:
correction = correction_old + now + residental_delay + tx_latency - tail_tag
<correction_old>: current value from the PTP header
<now>: Time of the PTP clock when entering the switch
<residental_delay>: Switching delay
<Tx latency>: Delay between time stamping unit and wire (configurable via
register)
<tail_tag>: Time stamp in the tail tag
The new correction value has been captured with Wireshark. For the measurement
I simply halted the internal PTP clock and set it to zero (so it's value is
exactly known). In the port's TX time stamp unit I got a non-zero time stamp
anyway (between, 10 to 40 ns), so this must be the residential delay.
I tested with different values for the correction field and the tail_tag.
Negative values are no problem, but the calculation seems no to consider all
bits.
Measurements:
- PTP clock: 0 (frozen)
- Residential delay: variable
- TX delay: 45 ns (default)
- correction = 0xffff ffff ffff.ffff (-1.xxx ns)
correction = correction + now + residental_delay + tx_latency - tail_tag
= -1 + 0 40 + 45 0
= 84 (0000 0000 0054)
wireshark: (0000 0000 0054) --> correct
correction = correction + now + residental_delay + tx_latency - tail_tag
= -1 + 0 32 + 45 1.000
= -924 (FFFF FFFF FC64)
wireshark: (0000 FFFF FC64) --> wrong
correction = correction + now + residental_delay + tx_latency - tail_tag
= -1 + 0 24 + 45 2.000.000.000
= -1.926.258.108 (FFFF 8D2F A244)
wireshark: (0000 7B9A CA44) --> wrong
- correction = 0xffff ffff 0000.0000 (-65536.0 ns)
correction = correction + now + residental_delay + tx_latency - tail_tag
= -65536 0 24 + 45 0
= -65467 (FFFF FFFF 0045)
wireshark: (FFFF FFFF 0045) --> correct
correction = correction + now + residental_delay + tx_latency - tail_tag
= -65536 + 0 32 + 45 1.000
= -66459 (FFFF FFFE FC65)
wireshark: (0000 FFFE FC65) --> wrong
Please note that the tail tag consist of 2 bits for seconds and 30 bit
nanoseconds. So the value of 2.000.000.000 means 1s + 926.258.176 ns.
As you are better in 2's complement as me, you can give me some more
combinations for testing if you need. But in the end it looks like I should
keep T2 in the tail tag.
> And zeroing out the correctionField of the Pdelay_Resp on transmission,
> to put that value into t_Tail_Tag? How can you squeeze a 48-bit value
> into a 32-bit value without truncation?
Only the lower bits are used. As long as PDelayResp doesn't take more than 4
seconds, this should be enough.
regards
Christian
next prev parent reply other threads:[~2020-11-11 21:53 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-19 17:24 [RFC PATCH 0/9] net: dsa: microchip: PTP support for KSZ956x Christian Eggers
2020-10-19 17:24 ` [RFC PATCH net-next 1/9] dt-bindings: net: dsa: convert ksz bindings document to yaml Christian Eggers
2020-10-21 6:52 ` Kurt Kanzenbach
2020-10-21 8:46 ` Christian Eggers
2020-10-22 0:16 ` Vladimir Oltean
2020-10-22 0:40 ` Florian Fainelli
2020-10-22 10:54 ` Kurt Kanzenbach
2020-10-22 12:37 ` Vladimir Oltean
2020-10-22 19:17 ` Florian Fainelli
2020-10-26 13:54 ` Rob Herring
2020-10-19 17:24 ` [RFC PATCH net-next 2/9] net: dsa: microchip: split ksz_common.h Christian Eggers
2020-10-19 17:24 ` [RFC PATCH net-next 3/9] net: dsa: microchip: rename ksz9477.c to ksz9477_main.o Christian Eggers
2020-10-19 17:24 ` [RFC PATCH net-next 4/9] dt-bindings: net: dsa: microchip,ksz: add interrupt property Christian Eggers
2020-10-19 17:24 ` [RFC PATCH net-next 5/9] net: dsa: microchip: ksz9477: basic interrupt support Christian Eggers
2020-10-19 17:24 ` [RFC PATCH net-next 6/9] net: dsa: microchip: ksz9477: add Posix clock support for chip PTP clock Christian Eggers
2020-10-19 17:24 ` [RFC PATCH net-next 7/9] net: dsa: microchip: ksz9477: add hardware time stamping support Christian Eggers
2020-10-20 0:10 ` Vladimir Oltean
2020-10-20 8:39 ` Christian Eggers
2020-10-21 23:39 ` Vladimir Oltean
2020-10-22 2:42 ` Richard Cochran
2020-10-22 7:30 ` Christian Eggers
2020-10-22 10:17 ` Christian Eggers
2020-10-30 18:24 ` Vladimir Oltean
2020-11-01 9:35 ` Christian Eggers
2020-11-01 11:10 ` Vladimir Oltean
2020-11-01 22:14 ` Christian Eggers
2020-11-01 23:41 ` Vladimir Oltean
2020-11-01 23:55 ` Vladimir Oltean
2020-11-02 10:35 ` Christian Eggers
2020-11-02 12:28 ` Vladimir Oltean
2020-10-22 3:02 ` Richard Cochran
2020-10-22 9:01 ` Vladimir Oltean
2020-10-22 10:50 ` Vladimir Oltean
2020-10-22 11:11 ` Christian Eggers
2020-10-22 11:32 ` Vladimir Oltean
2020-10-22 14:34 ` Richard Cochran
2020-11-05 20:18 ` Christian Eggers
2020-11-10 1:42 ` Vladimir Oltean
2020-11-10 14:36 ` Christian Eggers
2020-11-10 16:40 ` Vladimir Oltean
2020-11-10 19:32 ` Vladimir Oltean
2020-11-11 21:49 ` Christian Eggers
2020-11-11 21:50 ` Christian Eggers [this message]
2020-11-12 15:28 ` Christian Eggers
2020-11-12 15:38 ` Vladimir Oltean
2020-11-17 11:27 ` Christian Eggers
2020-10-19 17:24 ` [RFC PATCH net-next 8/9] net: dsa: microchip: ksz9477: add Pulse Per Second (PPS) support Christian Eggers
2020-10-19 17:46 ` Vladimir Oltean
2020-10-20 8:38 ` Christian Eggers
2020-10-19 17:24 ` [RFC PATCH net-next 9/9] net: dsa: microchip: ksz9477: add periodic output support Christian Eggers
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=2269731.xUjYGUPlbM@n95hx1g2 \
--to=ceggers@arri.de \
--cc=Tristram.Ha@microchip.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=andrew@lunn.ch \
--cc=codrin.ciubotariu@microchip.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=f.fainelli@gmail.com \
--cc=george.mccollister@gmail.com \
--cc=helmut.grohne@intenta.de \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marex@denx.de \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pbarker@konsulko.com \
--cc=richardcochran@gmail.com \
--cc=robh+dt@kernel.org \
--cc=vivien.didelot@gmail.com \
--cc=woojung.huh@microchip.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).