Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes
@ 2026-07-10  6:43 Luke Howard
  2026-07-10  6:43 ` [PATCH net-next v2 1/2] net: dsa: mv88e6xxx: use ARRIVAL1 counter for all peer delay messages Luke Howard
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Luke Howard @ 2026-07-10  6:43 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
	Richard Cochran
  Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel,
	Luke Howard

Two fixes for improving the reliably of hardware timestamp acquisition
on Marvell switches. In our tests this eliminated missed timestamps in
ptp4l.

Signed-off-by: Luke Howard <lukeh@padl.com>
---
Changes in v2:
- Lineraize SKBs before retrieving embedded timestamp
- Hoist driver register lock out of per-frame loop
- Link to v1: https://patch.msgid.link/20260703-mv88e6xxx-ptp-fixes-v1-0-0138581889a9@padl.com

To: Andrew Lunn <andrew@lunn.ch>
To: Vladimir Oltean <olteanv@gmail.com>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Richard Cochran <richardcochran@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Luke Howard (2):
      net: dsa: mv88e6xxx: use ARRIVAL1 counter for all peer delay messages
      net: dsa: mv88e6xxx: embedded PTP timestamp support

 drivers/net/dsa/mv88e6xxx/chip.c     |   3 +
 drivers/net/dsa/mv88e6xxx/chip.h     |   8 ++
 drivers/net/dsa/mv88e6xxx/hwtstamp.c | 140 +++++++++++++++++++++++++++++++----
 drivers/net/dsa/mv88e6xxx/hwtstamp.h |  14 ++++
 4 files changed, 152 insertions(+), 13 deletions(-)
---
base-commit: fe3e786ef4eb6e47d2901f568a27bd920477bbe9
change-id: 20260630-mv88e6xxx-ptp-fixes-1732570b8829

Best regards,
--  
Luke Howard <lukeh@padl.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 1/2] net: dsa: mv88e6xxx: use ARRIVAL1 counter for all peer delay messages
  2026-07-10  6:43 [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes Luke Howard
@ 2026-07-10  6:43 ` Luke Howard
  2026-07-10  6:43 ` [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support Luke Howard
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Luke Howard @ 2026-07-10  6:43 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
	Richard Cochran
  Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel,
	Luke Howard

mv88e6xxx switches have two arrival timestamp counters for timestamping
PTP event messages. This permits more than one arriving event message's
timestamp to be captured. This is useful for the case where event
messages from a grandmaster arrive at the same time as PDelayReq/PDelayResp
messages from a peer.

Previously only PDelayResp messages were assigned to the second arrival
counter; this patch does so for PDelayReq messages as well.

Signed-off-by: Luke Howard <lukeh@padl.com>
---
 drivers/net/dsa/mv88e6xxx/hwtstamp.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
index 6e6472a3b75ad..57ff77496864f 100644
--- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c
+++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
@@ -319,9 +319,16 @@ static void mv88e6xxx_rxtstamp_work(struct mv88e6xxx_chip *chip,
 				   &ps->rx_queue2);
 }
 
-static int is_pdelay_resp(const struct ptp_header *hdr)
+static bool is_pdelay_msg(const struct ptp_header *hdr)
 {
-	return (hdr->tsmt & 0xf) == 3;
+	switch (ptp_get_msgtype(hdr, PTP_CLASS_V2)) {
+	case PTP_MSGTYPE_PDELAY_REQ:
+		fallthrough;
+	case PTP_MSGTYPE_PDELAY_RESP:
+		return true;
+	default:
+		return false;
+	}
 }
 
 bool mv88e6xxx_port_rxtstamp(struct dsa_switch *ds, int port,
@@ -343,7 +350,7 @@ bool mv88e6xxx_port_rxtstamp(struct dsa_switch *ds, int port,
 
 	SKB_PTP_TYPE(skb) = type;
 
-	if (is_pdelay_resp(hdr))
+	if (is_pdelay_msg(hdr))
 		skb_queue_tail(&ps->rx_queue2, skb);
 	else
 		skb_queue_tail(&ps->rx_queue, skb);
@@ -584,8 +591,9 @@ int mv88e6xxx_hwtstamp_setup(struct mv88e6xxx_chip *chip)
 	if (err)
 		return err;
 
-	/* Use ARRIVAL1 for peer delay response messages. */
+	/* Use ARRIVAL1 for peer delay messages. */
 	err = mv88e6xxx_ptp_write(chip, MV88E6XXX_PTP_TS_ARRIVAL_PTR,
+				  MV88E6XXX_PTP_MSGTYPE_PDLAY_REQ |
 				  MV88E6XXX_PTP_MSGTYPE_PDLAY_RES);
 	if (err)
 		return err;

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support
  2026-07-10  6:43 [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes Luke Howard
  2026-07-10  6:43 ` [PATCH net-next v2 1/2] net: dsa: mv88e6xxx: use ARRIVAL1 counter for all peer delay messages Luke Howard
@ 2026-07-10  6:43 ` Luke Howard
  2026-07-19 10:12   ` Vladimir Oltean
  2026-07-16 10:57 ` [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes Simon Horman
  2026-07-17  7:47 ` Luke Howard
  3 siblings, 1 reply; 12+ messages in thread
From: Luke Howard @ 2026-07-10  6:43 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
	Richard Cochran
  Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel,
	Luke Howard

mv88e6xxx switches can support embedding PTP timestamps directly
in the frame, either as a trailer or at a configurable offset
(typically the reserved bytes in the PTP header).

Add support for this on the 88E6341 and 88E6352 switches, being
those on which I was able to verify this. Other switch chips may
also work. The arrival timestamp offsets are relative to the PTP
common header and will work for both L2 and L3 PTP packets; the
respective headers are skipped by the number of bytes set in the
ETJump and IPJump registers, which are initialized to sensible
defaults and are VLAN tag-aware.

(Note: the 6352 datasheet incorrectly states that ETJump and
IPJump are initialized to zero. They are initialized to 12 and
2 respectively; this is corrected in the 6341 data sheet.)

Signed-off-by: Luke Howard <lukeh@padl.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c     |   3 +
 drivers/net/dsa/mv88e6xxx/chip.h     |   8 +++
 drivers/net/dsa/mv88e6xxx/hwtstamp.c | 130 +++++++++++++++++++++++++++++++----
 drivers/net/dsa/mv88e6xxx/hwtstamp.h |  14 ++++
 4 files changed, 143 insertions(+), 12 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 80b877c74513d..c3277c1f3d785 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -28,6 +28,7 @@
 #include <linux/of_mdio.h>
 #include <linux/platform_data/mv88e6xxx.h>
 #include <linux/property.h>
+#include <linux/ptp_classify.h>
 #include <linux/netdevice.h>
 #include <linux/gpio/consumer.h>
 #include <linux/phylink.h>
@@ -6370,6 +6371,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.edsa_support = MV88E6XXX_EDSA_SUPPORTED,
 		.ptp_support = true,
 		.ops = &mv88e6341_ops,
+		.arr_ts_mode = offsetof(struct ptp_header, reserved2),
 	},
 
 	[MV88E6350] = {
@@ -6447,6 +6449,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.edsa_support = MV88E6XXX_EDSA_SUPPORTED,
 		.ptp_support = true,
 		.ops = &mv88e6352_ops,
+		.arr_ts_mode = offsetof(struct ptp_header, reserved2),
 	},
 	[MV88E6361] = {
 		.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6361,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index e966e7c4cc5de..b6a90eba81c43 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -180,6 +180,14 @@ struct mv88e6xxx_info {
 	 * port 0, 1 means internal PHYs range starts at port 1, etc
 	 */
 	unsigned int internal_phys_offset;
+
+	/* Arrival Time Stamp Mode (ArrTSMode); see the ArrTSMode encoding in
+	 * hwtstamp.h. Zero (the default) leaves arrival time stamps in the
+	 * switch registers; non-zero embeds them in the frame, either appended
+	 * as a trailer or overwritten at that byte offset past the start of the
+	 * PTP common header.
+	 */
+	unsigned int arr_ts_mode;
 };
 
 struct mv88e6xxx_atu_entry {
diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
index 57ff77496864f..1fbed34075797 100644
--- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c
+++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
@@ -15,6 +15,7 @@
 #include "hwtstamp.h"
 #include "ptp.h"
 #include <linux/ptp_classify.h>
+#include <linux/unaligned.h>
 
 #define SKB_PTP_TYPE(__skb) (*(unsigned int *)((__skb)->cb))
 
@@ -245,6 +246,73 @@ static int seq_match(struct sk_buff *skb, u16 ts_seqid)
 	return ts_seqid == ntohs(hdr->sequence_id);
 }
 
+static bool parse_embedded_ts(unsigned int arr_ts_mode,
+			      struct sk_buff *skb, u64 *ns)
+{
+	struct ptp_header *hdr;
+
+	*ns = 0;
+
+	/* APPEND means the switch appended the time stamp as a 4-byte trailer
+	 * (not all switches support this). Any other non-zero value is the byte
+	 * offset past the start of the PTP common header at which the switch
+	 * overwrote the time stamp in place (e.g. the reserved header bytes).
+	 */
+	if (arr_ts_mode == MV88E6XXX_PTP_ARR_TS_MODE_APPEND && skb->len >= 4) {
+		if (skb_linearize(skb))
+			return false;
+
+		*ns = (u64)get_unaligned_be32(skb_tail_pointer(skb) - 4);
+		if (pskb_trim_rcsum(skb, skb->len - 4))
+			return false;
+	} else if (arr_ts_mode + 4 <= sizeof(*hdr)) {
+		if (skb_linearize(skb))
+			return false;
+
+		hdr = ptp_parse_header(skb, SKB_PTP_TYPE(skb));
+		if (!hdr)
+			return false;
+
+		*ns = (u64)get_unaligned_be32((u8 *)hdr + arr_ts_mode);
+		memset((u8 *)hdr + arr_ts_mode, 0, 4);
+	} else {
+		return false;
+	}
+
+	return true;
+}
+
+static void mv88e6xxx_get_rxts_embedded(struct mv88e6xxx_chip *chip,
+					struct mv88e6xxx_port_hwtstamp *ps,
+					struct sk_buff *skb)
+{
+	struct sk_buff_head *rxq = &ps->rx_queue;
+	struct skb_shared_hwtstamps *shwt;
+	struct sk_buff_head received;
+	unsigned long flags;
+	u64 ns;
+
+	__skb_queue_head_init(&received);
+	__skb_queue_head(&received, skb);
+	spin_lock_irqsave(&rxq->lock, flags);
+	skb_queue_splice_tail_init(rxq, &received);
+	spin_unlock_irqrestore(&rxq->lock, flags);
+
+	mv88e6xxx_reg_lock(chip);
+	skb_queue_walk(&received, skb) {
+		if (!parse_embedded_ts(chip->info->arr_ts_mode, skb, &ns))
+			continue;
+		ns = timecounter_cyc2time(&chip->tstamp_tc, ns);
+		shwt = skb_hwtstamps(skb);
+		memset(shwt, 0, sizeof(*shwt));
+		shwt->hwtstamp = ns_to_ktime(ns);
+	}
+	mv88e6xxx_reg_unlock(chip);
+
+	while ((skb = __skb_dequeue(&received)))
+		netif_rx(skb);
+}
+
 static void mv88e6xxx_get_rxts(struct mv88e6xxx_chip *chip,
 			       struct mv88e6xxx_port_hwtstamp *ps,
 			       struct sk_buff *skb, u16 reg,
@@ -307,8 +375,21 @@ static void mv88e6xxx_rxtstamp_work(struct mv88e6xxx_chip *chip,
 	const struct mv88e6xxx_ptp_ops *ptp_ops = chip->info->ops->ptp_ops;
 	struct sk_buff *skb;
 
-	skb = skb_dequeue(&ps->rx_queue);
+	if (chip->info->arr_ts_mode) {
+		/* If arr_ts_mode is set, the timestamps are embedded in the
+		 * frames so a register read is not required. We still need a
+		 * work queue rather than processing inline because
+		 * timecounter_cyc2time() takes the global mutex and this
+		 * cannot be called from mv88e6xxx_port_rxtstamp().
+		 */
+		skb = skb_dequeue(&ps->rx_queue);
+		if (skb)
+			mv88e6xxx_get_rxts_embedded(chip, ps, skb);
 
+		return;
+	}
+
+	skb = skb_dequeue(&ps->rx_queue);
 	if (skb)
 		mv88e6xxx_get_rxts(chip, ps, skb, ptp_ops->arr0_sts_reg,
 				   &ps->rx_queue);
@@ -350,7 +431,7 @@ bool mv88e6xxx_port_rxtstamp(struct dsa_switch *ds, int port,
 
 	SKB_PTP_TYPE(skb) = type;
 
-	if (is_pdelay_msg(hdr))
+	if (!chip->info->arr_ts_mode && is_pdelay_msg(hdr))
 		skb_queue_tail(&ps->rx_queue2, skb);
 	else
 		skb_queue_tail(&ps->rx_queue, skb);
@@ -530,14 +611,37 @@ int mv88e6165_global_enable(struct mv88e6xxx_chip *chip)
 
 int mv88e6352_hwtstamp_port_disable(struct mv88e6xxx_chip *chip, int port)
 {
-	return mv88e6xxx_port_ptp_write(chip, port, MV88E6XXX_PORT_PTP_CFG0,
-					MV88E6XXX_PORT_PTP_CFG0_DISABLE_PTP);
+	int err;
+
+	err = mv88e6xxx_port_ptp_write(chip, port, MV88E6XXX_PORT_PTP_CFG0,
+				       MV88E6XXX_PORT_PTP_CFG0_DISABLE_PTP);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_port_ptp_write(chip, port, MV88E6XXX_PORT_PTP_CFG2, 0);
+	if (err)
+		return err;
+
+	return 0;
 }
 
 int mv88e6352_hwtstamp_port_enable(struct mv88e6xxx_chip *chip, int port)
 {
-	return mv88e6xxx_port_ptp_write(chip, port, MV88E6XXX_PORT_PTP_CFG0,
-					MV88E6XXX_PORT_PTP_CFG0_DISABLE_TSPEC_MATCH);
+	int err;
+
+	if (chip->info->arr_ts_mode) {
+		err = mv88e6xxx_port_ptp_write(chip, port, MV88E6XXX_PORT_PTP_CFG2,
+					       chip->info->arr_ts_mode << 8);
+		if (err)
+			return err;
+	}
+
+	err = mv88e6xxx_port_ptp_write(chip, port, MV88E6XXX_PORT_PTP_CFG0,
+				       MV88E6XXX_PORT_PTP_CFG0_DISABLE_TSPEC_MATCH);
+	if (err)
+		return err;
+
+	return 0;
 }
 
 static int mv88e6xxx_hwtstamp_port_setup(struct mv88e6xxx_chip *chip, int port)
@@ -591,12 +695,14 @@ int mv88e6xxx_hwtstamp_setup(struct mv88e6xxx_chip *chip)
 	if (err)
 		return err;
 
-	/* Use ARRIVAL1 for peer delay messages. */
-	err = mv88e6xxx_ptp_write(chip, MV88E6XXX_PTP_TS_ARRIVAL_PTR,
-				  MV88E6XXX_PTP_MSGTYPE_PDLAY_REQ |
-				  MV88E6XXX_PTP_MSGTYPE_PDLAY_RES);
-	if (err)
-		return err;
+	if (!chip->info->arr_ts_mode) {
+		/* Use ARRIVAL1 for peer delay messages. */
+		err = mv88e6xxx_ptp_write(chip, MV88E6XXX_PTP_TS_ARRIVAL_PTR,
+					  MV88E6XXX_PTP_MSGTYPE_PDLAY_REQ |
+					  MV88E6XXX_PTP_MSGTYPE_PDLAY_RES);
+		if (err)
+			return err;
+	}
 
 	/* 88E6341 devices default to timestamping at the PHY, but this has
 	 * a hardware issue that results in unreliable timestamps. Force
diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.h b/drivers/net/dsa/mv88e6xxx/hwtstamp.h
index c359821d5a6ea..c25f53923e768 100644
--- a/drivers/net/dsa/mv88e6xxx/hwtstamp.h
+++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.h
@@ -68,6 +68,20 @@
 #define MV88E6XXX_PORT_PTP_CFG2_DEP_IRQ_EN		0x0002
 #define MV88E6XXX_PORT_PTP_CFG2_ARR_IRQ_EN		0x0001
 
+/* Arrival Time Stamp Mode (ArrTSMode), CFG2 bits [15:8]: configures how the
+ * switch embeds the arrival time stamp (PTPArr0Time) into enabled PTP event
+ * frames.
+ *   0x00        frame modification disabled (time stamp read from registers)
+ *   0x01        append the 4-byte time stamp at the end of the frame,
+ *               growing the frame by four bytes
+ *   0x04..0xEF  overwrite the 4-byte time stamp in place, that many bytes past
+ *               the start of the PTP common header, without growing the frame
+ *               (offsetof(struct ptp_header, reserved2) targets the reserved
+ *               bytes of the header)
+ *   others      reserved
+ */
+#define MV88E6XXX_PTP_ARR_TS_MODE_APPEND		0x01
+
 /* Offset 0x03: PTP LED Configuration */
 #define MV88E6XXX_PORT_PTP_LED_CFG	0x03
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes
  2026-07-10  6:43 [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes Luke Howard
  2026-07-10  6:43 ` [PATCH net-next v2 1/2] net: dsa: mv88e6xxx: use ARRIVAL1 counter for all peer delay messages Luke Howard
  2026-07-10  6:43 ` [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support Luke Howard
@ 2026-07-16 10:57 ` Simon Horman
  2026-07-17  7:47 ` Luke Howard
  3 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2026-07-16 10:57 UTC (permalink / raw)
  To: Luke Howard
  Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
	Richard Cochran, Cedric Jehasse, Kieran Tyrrell, Max Holtmann,
	Max Hunter, Christoph Mellauner, Simon Gapp, netdev, linux-kernel

On Fri, Jul 10, 2026 at 04:43:40PM +1000, Luke Howard wrote:
> Two fixes for improving the reliably of hardware timestamp acquisition
> on Marvell switches. In our tests this eliminated missed timestamps in
> ptp4l.
> 
> Signed-off-by: Luke Howard <lukeh@padl.com>
> ---
> Changes in v2:
> - Lineraize SKBs before retrieving embedded timestamp
> - Hoist driver register lock out of per-frame loop
> - Link to v1: https://patch.msgid.link/20260703-mv88e6xxx-ptp-fixes-v1-0-0138581889a9@padl.com

For the series:

Reviewed-by: Simon Horman <horms@kernel.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes
  2026-07-10  6:43 [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes Luke Howard
                   ` (2 preceding siblings ...)
  2026-07-16 10:57 ` [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes Simon Horman
@ 2026-07-17  7:47 ` Luke Howard
  2026-07-19  9:53   ` Vladimir Oltean
  3 siblings, 1 reply; 12+ messages in thread
From: Luke Howard @ 2026-07-17  7:47 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
	Richard Cochran
  Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel


> ---
> Luke Howard (2):
>    net: dsa: mv88e6xxx: embedded PTP timestamp support

This can be improved: there is no need to extract the embedded timestamp in the PTP worker as it can be done directly in mv88e6xxx_port_rxtstamp() (provided tstamp_{cc,tc} are protected by a spinlock). DSA can deliver the frame normally, similar to ocelot_ptp_rx_timestamp().

Deferring to the worker can reorder frames such that PTP general messages arrive before the timestamped event messages, which confuses some other PTP implementations such as gptp2d [1].

This optimisation of course only works for ArrTSMode because there is no MDIO read required.

Luke

[1] https://github.com/xl4-shiro/excelfore-gptp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes
  2026-07-17  7:47 ` Luke Howard
@ 2026-07-19  9:53   ` Vladimir Oltean
  2026-07-19 11:22     ` Luke Howard
  0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2026-07-19  9:53 UTC (permalink / raw)
  To: Luke Howard
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Vivien Didelot, Gregory CLEMENT, Andrew Lunn, Richard Cochran,
	Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel

On Fri, Jul 17, 2026 at 05:47:49PM +1000, Luke Howard wrote:
> 
> > ---
> > Luke Howard (2):
> >    net: dsa: mv88e6xxx: embedded PTP timestamp support
> 
> This can be improved: there is no need to extract the embedded
> timestamp in the PTP worker as it can be done directly in
> mv88e6xxx_port_rxtstamp() (provided tstamp_{cc,tc} are protected by a
> spinlock). DSA can deliver the frame normally, similar to
> ocelot_ptp_rx_timestamp().

ocelot_ptp_rx_timestamp() accesses MMIO-based registers, which can be
done atomically.
mv88e6xxx_ptp_clock_read() accesses MDIO bus registers, and the MDIO bus
is sleepable. Fundamental difference.

Your hardware only provides 32 bits of partial timestamp, so
mv88e6xxx_ptp_clock_read() will always be needed one way or another, to
recover the full 64 bits. Either through tstamp_{cc,tc} or through
direct calls.

> Deferring to the worker can reorder frames such that PTP general
> messages arrive before the timestamped event messages, which confuses
> some other PTP implementations such as gptp2d [1].

True, this is a caveat, but event messages and general messages can
already take different network paths, especially with PTP over IP where
they go through different UDP ports (even if for gPTP that is not the case).
The PTP user space implementation needs to be prepared to handle this.

> This optimisation of course only works for ArrTSMode because there is
> no MDIO read required.

I don't understand this comment given the partial 32-bit timestamp
limitation.

> Luke
> 
> [1] https://github.com/xl4-shiro/excelfore-gptp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support
  2026-07-10  6:43 ` [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support Luke Howard
@ 2026-07-19 10:12   ` Vladimir Oltean
  2026-07-19 11:35     ` Luke Howard
  0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2026-07-19 10:12 UTC (permalink / raw)
  To: Luke Howard
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Vivien Didelot, Gregory CLEMENT, Andrew Lunn, Richard Cochran,
	Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel

On Fri, Jul 10, 2026 at 04:43:42PM +1000, Luke Howard wrote:
> @@ -6370,6 +6371,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.edsa_support = MV88E6XXX_EDSA_SUPPORTED,
>  		.ptp_support = true,
>  		.ops = &mv88e6341_ops,
> +		.arr_ts_mode = offsetof(struct ptp_header, reserved2),
>  	},
>  
>  	[MV88E6350] = {
> @@ -6447,6 +6449,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.edsa_support = MV88E6XXX_EDSA_SUPPORTED,
>  		.ptp_support = true,
>  		.ops = &mv88e6352_ops,
> +		.arr_ts_mode = offsetof(struct ptp_header, reserved2),
>  	},
>  	[MV88E6361] = {
>  		.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6361,
> diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.h b/drivers/net/dsa/mv88e6xxx/hwtstamp.h
> index c359821d5a6ea..c25f53923e768 100644
> --- a/drivers/net/dsa/mv88e6xxx/hwtstamp.h
> +++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.h
> @@ -68,6 +68,20 @@
>  #define MV88E6XXX_PORT_PTP_CFG2_DEP_IRQ_EN		0x0002
>  #define MV88E6XXX_PORT_PTP_CFG2_ARR_IRQ_EN		0x0001
>  
> +/* Arrival Time Stamp Mode (ArrTSMode), CFG2 bits [15:8]: configures how the
> + * switch embeds the arrival time stamp (PTPArr0Time) into enabled PTP event
> + * frames.
> + *   0x00        frame modification disabled (time stamp read from registers)
> + *   0x01        append the 4-byte time stamp at the end of the frame,
> + *               growing the frame by four bytes
> + *   0x04..0xEF  overwrite the 4-byte time stamp in place, that many bytes past
> + *               the start of the PTP common header, without growing the frame
> + *               (offsetof(struct ptp_header, reserved2) targets the reserved
> + *               bytes of the header)
> + *   others      reserved
> + */
> +#define MV88E6XXX_PTP_ARR_TS_MODE_APPEND		0x01
> +
>  /* Offset 0x03: PTP LED Configuration */
>  #define MV88E6XXX_PORT_PTP_LED_CFG	0x03

We try to keep the data path protocol between the switch and the host
API compatible, and identifiable by /sys/class/net/<conduit>/dsa/tagging.
I would argue that the protocol where PTP timestamps are in registers,
vs where they are in PTP header reserved fields, vs where they are
appended as trailers, are 3 different protocols and should not be
presented as "edsa".

Two different switches both use the "edsa" protocol (say MV88E6390 and
MV88E6352), yet one expects timestamps in registers and the other in the
PTP header reserved fields. Strange.

In some instances, user space cares (though perhaps not imminently).
For example, libpcap/tcpdump might get confused by a non-zero arr_ts_mode.
Or an XDP/AF_XDP implementation of the edsa protocol might not understand
(because it has no access to this information) what arr_ts_mode the
underlying switch is configured for, and thus where to get timestamps from.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes
  2026-07-19  9:53   ` Vladimir Oltean
@ 2026-07-19 11:22     ` Luke Howard
  2026-07-19 22:57       ` Vladimir Oltean
  0 siblings, 1 reply; 12+ messages in thread
From: Luke Howard @ 2026-07-19 11:22 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Vivien Didelot, Gregory CLEMENT, Andrew Lunn, Richard Cochran,
	Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel

Hi Vladimir,

> ocelot_ptp_rx_timestamp() accesses MMIO-based registers, which can be
> done atomically.
> mv88e6xxx_ptp_clock_read() accesses MDIO bus registers, and the MDIO bus
> is sleepable. Fundamental difference.
> 
> Your hardware only provides 32 bits of partial timestamp, so
> mv88e6xxx_ptp_clock_read() will always be needed one way or another, to
> recover the full 64 bits. Either through tstamp_{cc,tc} or through
> direct calls.

This still happens from overflow_work().

>> Deferring to the worker can reorder frames such that PTP general
>> messages arrive before the timestamped event messages, which confuses
>> some other PTP implementations such as gptp2d [1].
> 
> True, this is a caveat, but event messages and general messages can
> already take different network paths, especially with PTP over IP where
> they go through different UDP ports (even if for gPTP that is not the case).
> The PTP user space implementation needs to be prepared to handle this.

Good point. So perhaps processing the embedded timestamp inline doesn’t confer much benefit. ptp4l (which we use) handles out-of-order messages fine.

>> This optimisation of course only works for ArrTSMode because there is
>> no MDIO read required.
> 
> I don't understand this comment given the partial 32-bit timestamp
> limitation.

Better phrased as no MDIO read to recover the arrival timestamp.

Luke

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support
  2026-07-19 10:12   ` Vladimir Oltean
@ 2026-07-19 11:35     ` Luke Howard
  2026-07-19 23:04       ` Vladimir Oltean
  0 siblings, 1 reply; 12+ messages in thread
From: Luke Howard @ 2026-07-19 11:35 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Vivien Didelot, Gregory CLEMENT, Andrew Lunn, Richard Cochran,
	Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel


> We try to keep the data path protocol between the switch and the host
> API compatible, and identifiable by /sys/class/net/<conduit>/dsa/tagging.
> I would argue that the protocol where PTP timestamps are in registers,
> vs where they are in PTP header reserved fields, vs where they are
> appended as trailers, are 3 different protocols and should not be
> presented as "edsa".

For context: I added ArrTSMode support because even with the PTP worker process priority bumped, ptp4l frequently missed RX timestamps.

I can add these as non-default tagging variants that can be selected by the user. "edsa-arrts-trailer" and "edsa-arrts-ptp-reserved”?

Luke

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes
  2026-07-19 11:22     ` Luke Howard
@ 2026-07-19 22:57       ` Vladimir Oltean
  2026-07-20  1:13         ` Luke Howard
  0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2026-07-19 22:57 UTC (permalink / raw)
  To: Luke Howard
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Vivien Didelot, Gregory CLEMENT, Andrew Lunn, Richard Cochran,
	Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel

On Sun, Jul 19, 2026 at 09:22:39PM +1000, Luke Howard wrote:
> Hi Vladimir,
> 
> > ocelot_ptp_rx_timestamp() accesses MMIO-based registers, which can be
> > done atomically.
> > mv88e6xxx_ptp_clock_read() accesses MDIO bus registers, and the MDIO bus
> > is sleepable. Fundamental difference.
> > 
> > Your hardware only provides 32 bits of partial timestamp, so
> > mv88e6xxx_ptp_clock_read() will always be needed one way or another, to
> > recover the full 64 bits. Either through tstamp_{cc,tc} or through
> > direct calls.
> 
> This still happens from overflow_work().

Ok. My mistake.

> >> Deferring to the worker can reorder frames such that PTP general
> >> messages arrive before the timestamped event messages, which confuses
> >> some other PTP implementations such as gptp2d [1].
> > 
> > True, this is a caveat, but event messages and general messages can
> > already take different network paths, especially with PTP over IP where
> > they go through different UDP ports (even if for gPTP that is not the case).
> > The PTP user space implementation needs to be prepared to handle this.
> 
> Good point. So perhaps processing the embedded timestamp inline
> doesn’t confer much benefit. ptp4l (which we use) handles out-of-order
> messages fine.

If you're intending the in-band timestamps as a fix for gptp2d's
limitations, then no. As a general optimization - maybe.

> >> This optimisation of course only works for ArrTSMode because there is
> >> no MDIO read required.
> > 
> > I don't understand this comment given the partial 32-bit timestamp
> > limitation.
> 
> Better phrased as no MDIO read to recover the arrival timestamp.

Yeah, the timecounter/cyclecounter requires refreshes at least once
every 32-bit wraparound/2 time in order for the trick to work. But since
that is set up, it frees up the need to access MDIO per packet, since it
is guaranteed that cycle_now - tc->cycle_last is no larger than half the
wraparound time, which permits distinguishing timestamps taken by
hardware before tc->cycle_last from those taken after tc->cycle_last,
and correctly converting both to a valid 64-bit time base. Quite clever,
actually, I didn't fully understand the first time.

The only problem with timecounter/cyclecounter is that the hardware
clock remains free-running, which means you won't be able to apply a
time-synchronized taprio schedule (if the hw supports that).

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support
  2026-07-19 11:35     ` Luke Howard
@ 2026-07-19 23:04       ` Vladimir Oltean
  0 siblings, 0 replies; 12+ messages in thread
From: Vladimir Oltean @ 2026-07-19 23:04 UTC (permalink / raw)
  To: Luke Howard
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Vivien Didelot, Gregory CLEMENT, Andrew Lunn, Richard Cochran,
	Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel

On Sun, Jul 19, 2026 at 09:35:45PM +1000, Luke Howard wrote:
> 
> > We try to keep the data path protocol between the switch and the host
> > API compatible, and identifiable by /sys/class/net/<conduit>/dsa/tagging.
> > I would argue that the protocol where PTP timestamps are in registers,
> > vs where they are in PTP header reserved fields, vs where they are
> > appended as trailers, are 3 different protocols and should not be
> > presented as "edsa".
> 
> For context: I added ArrTSMode support because even with the PTP
> worker process priority bumped, ptp4l frequently missed RX timestamps.
> 
> I can add these as non-default tagging variants that can be selected
> by the user. "edsa-arrts-trailer" and "edsa-arrts-ptp-reserved”?

I guess that would be fine, except:
- worth clarifying that you're talking about the Reserved2 field (rather
  than Reserved1 or Reserved3)
- any reason justifying the introduction of "edsa-arrts-trailed", or
  would it be just for completeness? As a user, what would make me
  decide between "edsa-arrts-trailer" and "edsa-arrts-ptp-reserved2"?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes
  2026-07-19 22:57       ` Vladimir Oltean
@ 2026-07-20  1:13         ` Luke Howard
  0 siblings, 0 replies; 12+ messages in thread
From: Luke Howard @ 2026-07-20  1:13 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Vivien Didelot, Gregory CLEMENT, Andrew Lunn, Richard Cochran,
	Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel


>> Good point. So perhaps processing the embedded timestamp inline
>> doesn’t confer much benefit. ptp4l (which we use) handles out-of-order
>> messages fine.
> 
> If you're intending the in-band timestamps as a fix for gptp2d's
> limitations, then no. As a general optimization - maybe.

Delivering it inline makes more sense to me but it is a more intrusive change. I don’t have a strong opinion either way. I’m sure gptp2d could be fixed were someone motivated.

> The only problem with timecounter/cyclecounter is that the hardware
> clock remains free-running, which means you won't be able to apply a
> time-synchronized taprio schedule (if the hw supports that).

Yes, many Marvell switches support 802.1Qbv but there is no kernel support at this time. For 802.1AS, you want both kernel and user counters to be free running, so neighborRateRatio can be correctly reported.

Luke

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-07-20  1:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10  6:43 [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes Luke Howard
2026-07-10  6:43 ` [PATCH net-next v2 1/2] net: dsa: mv88e6xxx: use ARRIVAL1 counter for all peer delay messages Luke Howard
2026-07-10  6:43 ` [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support Luke Howard
2026-07-19 10:12   ` Vladimir Oltean
2026-07-19 11:35     ` Luke Howard
2026-07-19 23:04       ` Vladimir Oltean
2026-07-16 10:57 ` [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes Simon Horman
2026-07-17  7:47 ` Luke Howard
2026-07-19  9:53   ` Vladimir Oltean
2026-07-19 11:22     ` Luke Howard
2026-07-19 22:57       ` Vladimir Oltean
2026-07-20  1:13         ` Luke Howard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox