[PATCH net-next 0/2] net: dsa: mv88e6xxx: various hwstamp fixes

[PATCH net-next 0/2] net: dsa: mv88e6xxx: various hwstamp fixes

[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>
---
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 | 132 +++++++++++++++++++++++++++++++----
 drivers/net/dsa/mv88e6xxx/hwtstamp.h |  14 ++++
 4 files changed, 144 insertions(+), 13 deletions(-)
---
base-commit: b8ea7da314c2efcb9c2f559ed65b7a36c869d68e
change-id: 20260630-mv88e6xxx-ptp-fixes-1732570b8829

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

[PATCH net-next 1/2] net: dsa: mv88e6xxx: use ARRIVAL1 counter for all peer delay messages

[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

Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: use ARRIVAL1 counter for all peer delay messages

[Andrew Lunn]

On Fri, Jul 03, 2026 at 04:41:58PM +1000, Luke Howard wrote:
> 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>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support

[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 | 122 +++++++++++++++++++++++++++++++----
 drivers/net/dsa/mv88e6xxx/hwtstamp.h |  14 ++++
 4 files changed, 135 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..341a9e33021d7 100644
--- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c
+++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
@@ -8,6 +8,8 @@
  *      Erik Hons <erik.hons@ni.com>
  *      Brandon Streiff <brandon.streiff@ni.com>
  *      Dane Wagner <dane.wagner@ni.com>
+ *
+ * Copyright (c) 2025 PADL Software Pty Ltd
  */
 
 #include "chip.h"
@@ -15,6 +17,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 +248,63 @@ 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) {
+		*ns = (u64)get_unaligned_be32(&skb->data[skb->len - 4]);
+		skb_trim(skb, skb->len - 4);
+	} else if (arr_ts_mode + 4 <= sizeof(*hdr)) {
+		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);
+	spin_lock_irqsave(&rxq->lock, flags);
+	skb_queue_splice_tail_init(rxq, &received);
+	spin_unlock_irqrestore(&rxq->lock, flags);
+
+	for ( ; skb; skb = __skb_dequeue(&received)) {
+		if (parse_embedded_ts(chip->info->arr_ts_mode, skb, &ns)) {
+			mv88e6xxx_reg_lock(chip);
+			ns = timecounter_cyc2time(&chip->tstamp_tc, ns);
+			mv88e6xxx_reg_unlock(chip);
+			shwt = skb_hwtstamps(skb);
+			memset(shwt, 0, sizeof(*shwt));
+			shwt->hwtstamp = ns_to_ktime(ns);
+		}
+		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 +367,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 +423,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 +603,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 +687,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

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support

[Andrew Lunn]

> +	/* 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.

So how do you indicate trailer?

> +++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
> @@ -8,6 +8,8 @@
>   *      Erik Hons <erik.hons@ni.com>
>   *      Brandon Streiff <brandon.streiff@ni.com>
>   *      Dane Wagner <dane.wagner@ni.com>
> + *
> + * Copyright (c) 2025 PADL Software Pty Ltd

It is unusual to add a copyright for the passed.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support

[Luke Howard]

> On 4 Jul 2026, at 1:13 am, Andrew Lunn <andrew@lunn.ch> wrote:
> 
>> + /* 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.
> 
> So how do you indicate trailer?

ArrTSMode being 1 (matches switch register interpretation).

Will remove (c).

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support

[Maxime Chevallier]

Hi Luke,

On 7/3/26 08:41, Luke Howard wrote:
> 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>

[...]

> +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) {
> +		*ns = (u64)get_unaligned_be32(&skb->data[skb->len - 4]);

One thing is that you need to make sure you're dealing with a linear skb
when accessing the trailer, which can be done by calling skb_linearize(skb).

Then you should use skb_tail_pointer(skb) to get to the trailer.

You can take as examples the DSA taggers that pull the tag from the tail, such as
the tag_trailer [1], tag_ksz, tag_rtl8_4, etc. 

And finally you may have to use the chksum variants of the skb_trim such as
pskb_trim_rcsum(), like done in the above-mentionned taggers.

[1] : https://elixir.bootlin.com/linux/v7.1.2/source/net/dsa/tag_trailer.c#L28

Thanks,

Maxime