[PATCH net-next v2 0/2] Add Marvell PHY PTP support

[PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Kory Maincent]

Add PTP basic support for Marvell 88E151x PHYs.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Kory Maincent (1):
      net: phy: Move Marvell PHY drivers to its own subdirectory

Russell King (1):
      net: phy: Add Marvell PHY PTP support

 MAINTAINERS                                        |   2 +-
 drivers/net/phy/Kconfig                            |  23 +-
 drivers/net/phy/Makefile                           |   5 +-
 drivers/net/phy/marvell/Kconfig                    |  35 +
 drivers/net/phy/marvell/Makefile                   |   7 +
 drivers/net/phy/{ => marvell}/marvell-88q2xxx.c    |   0
 drivers/net/phy/{ => marvell}/marvell-88x2222.c    |   0
 drivers/net/phy/{ => marvell}/marvell10g.c         |   0
 .../net/phy/{marvell.c => marvell/marvell_main.c}  |  18 +-
 drivers/net/phy/marvell/marvell_ptp.c              | 725 +++++++++++++++++++++
 drivers/net/phy/marvell/marvell_ptp.h              |  26 +
 drivers/net/phy/marvell/marvell_tai.c              | 279 ++++++++
 drivers/net/phy/marvell/marvell_tai.h              |  36 +
 13 files changed, 1128 insertions(+), 28 deletions(-)
---
base-commit: e9363f5834ade5fa092751ed42080edfdf4ff93e
change-id: 20250124-feature_marvell_ptp-f8730c6a94c2

Best regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[PATCH net-next v2 1/2] net: phy: Move Marvell PHY drivers to its own subdirectory

[Kory Maincent]

Move the Marvell PHY drivers to a dedicated directory to improve
organization and maintainability.

As part of this cleanup, and in preparation for adding PTP support in
the marvell driver, rename marvell.c to marvell_main.c.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 MAINTAINERS                                        |  2 +-
 drivers/net/phy/Kconfig                            | 23 +---------------------
 drivers/net/phy/Makefile                           |  5 +----
 drivers/net/phy/marvell/Kconfig                    | 23 ++++++++++++++++++++++
 drivers/net/phy/marvell/Makefile                   |  6 ++++++
 drivers/net/phy/{ => marvell}/marvell-88q2xxx.c    |  0
 drivers/net/phy/{ => marvell}/marvell-88x2222.c    |  0
 drivers/net/phy/{ => marvell}/marvell10g.c         |  0
 .../net/phy/{marvell.c => marvell/marvell_main.c}  |  0
 9 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4c5c2e2c127877a8283793637b0e935ceec27aff..b57df9a87de798c2eab139214f01253ddc1d2708 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14302,7 +14302,7 @@ M:	Russell King <linux@armlinux.org.uk>
 M:	Marek Behún <kabel@kernel.org>
 L:	netdev@vger.kernel.org
 S:	Maintained
-F:	drivers/net/phy/marvell10g.c
+F:	drivers/net/phy/marvell/marvell10g.c
 
 MARVELL MVEBU THERMAL DRIVER
 M:	Miquel Raynal <miquel.raynal@bootlin.com>
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index d29f9f7fd2e110415496f322b2936c903cbc4d9c..bccffa3a48fc88ca08c26753e12645bd824e9ff4 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -235,28 +235,7 @@ config LSI_ET1011C_PHY
 	help
 	  Supports the LSI ET1011C PHY.
 
-config MARVELL_PHY
-	tristate "Marvell Alaska PHYs"
-	help
-	  Currently has a driver for the 88E1XXX
-
-config MARVELL_10G_PHY
-	tristate "Marvell Alaska 10Gbit PHYs"
-	help
-	  Support for the Marvell Alaska MV88X3310 and compatible PHYs.
-
-config MARVELL_88Q2XXX_PHY
-	tristate "Marvell 88Q2XXX PHY"
-	depends on HWMON || HWMON=n
-	help
-	  Support for the Marvell 88Q2XXX 100/1000BASE-T1 Automotive Ethernet
-	  PHYs.
-
-config MARVELL_88X2222_PHY
-	tristate "Marvell 88X2222 PHY"
-	help
-	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
-	  Transceiver.
+source "drivers/net/phy/marvell/Kconfig"
 
 config MAXLINEAR_GPHY
 	tristate "Maxlinear Ethernet PHYs"
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 23ce205ae91d88ef28fa24ec3689bed1be16a0be..1c0f271b26bee2abb965640b41865bb0f4fda6b6 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -70,10 +70,7 @@ obj-$(CONFIG_ICPLUS_PHY)	+= icplus.o
 obj-$(CONFIG_INTEL_XWAY_PHY)	+= intel-xway.o
 obj-$(CONFIG_LSI_ET1011C_PHY)	+= et1011c.o
 obj-$(CONFIG_LXT_PHY)		+= lxt.o
-obj-$(CONFIG_MARVELL_10G_PHY)	+= marvell10g.o
-obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
-obj-$(CONFIG_MARVELL_88Q2XXX_PHY)	+= marvell-88q2xxx.o
-obj-$(CONFIG_MARVELL_88X2222_PHY)	+= marvell-88x2222.o
+obj-y				+= marvell/
 obj-$(CONFIG_MAXLINEAR_GPHY)	+= mxl-gpy.o
 obj-y				+= mediatek/
 obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
diff --git a/drivers/net/phy/marvell/Kconfig b/drivers/net/phy/marvell/Kconfig
new file mode 100644
index 0000000000000000000000000000000000000000..a85bc9e4311e6bedd4a89db9527aca82d55a0762
--- /dev/null
+++ b/drivers/net/phy/marvell/Kconfig
@@ -0,0 +1,23 @@
+# SPDX-License-Identifier: GPL-2.0
+config MARVELL_PHY
+	tristate "Marvell Alaska PHYs"
+	help
+	  Currently has a driver for the 88E1XXX
+
+config MARVELL_10G_PHY
+	tristate "Marvell Alaska 10Gbit PHYs"
+	help
+	  Support for the Marvell Alaska MV88X3310 and compatible PHYs.
+
+config MARVELL_88Q2XXX_PHY
+	tristate "Marvell 88Q2XXX PHY"
+	depends on HWMON || HWMON=n
+	help
+	  Support for the Marvell 88Q2XXX 100/1000BASE-T1 Automotive Ethernet
+	  PHYs.
+
+config MARVELL_88X2222_PHY
+	tristate "Marvell 88X2222 PHY"
+	help
+	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
+	  Transceiver.
diff --git a/drivers/net/phy/marvell/Makefile b/drivers/net/phy/marvell/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..2c3622b053d1f54eba518b06730b797fb103ee06
--- /dev/null
+++ b/drivers/net/phy/marvell/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_MARVELL_10G_PHY)	+= marvell10g.o
+marvell-y			:= marvell_main.o
+obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
+obj-$(CONFIG_MARVELL_88Q2XXX_PHY)	+= marvell-88q2xxx.o
+obj-$(CONFIG_MARVELL_88X2222_PHY)	+= marvell-88x2222.o
diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell/marvell-88q2xxx.c
similarity index 100%
rename from drivers/net/phy/marvell-88q2xxx.c
rename to drivers/net/phy/marvell/marvell-88q2xxx.c
diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell/marvell-88x2222.c
similarity index 100%
rename from drivers/net/phy/marvell-88x2222.c
rename to drivers/net/phy/marvell/marvell-88x2222.c
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell/marvell10g.c
similarity index 100%
rename from drivers/net/phy/marvell10g.c
rename to drivers/net/phy/marvell/marvell10g.c
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell/marvell_main.c
similarity index 100%
rename from drivers/net/phy/marvell.c
rename to drivers/net/phy/marvell/marvell_main.c

-- 
2.34.1

[PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

From: Russell King <rmk+kernel@armlinux.org.uk>

From: Russell King <rmk+kernel@armlinux.org.uk>

Add PTP basic support for Marvell 88E151x PHYs. These PHYs support
timestamping the egress and ingress of packets, but does not support
any packet modification.

The PHYs support hardware pins for providing an external clock for the
TAI counter, and a separate pin that can be used for event capture or
generation of a trigger (either a pulse or periodic).  This code does
not support either of these modes.

The driver takes inspiration from the Marvell 88E6xxx DSA and DP83640
drivers.  The hardware is very similar to the implementation found in
the 88E6xxx DSA driver, but the access methods are very different,
although it may be possible to create a library that both can use
along with accessor functions.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Add support for interruption.
Fix L2 PTP encapsulation frame detection.
Fix first PTP timestamp being dropped.
Fix Kconfig to depends on MARVELL_PHY.
Update comments to use kdoc.

Co-developed-by: Kory Maincent <kory.maincent@bootlin.com>
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Russell I don't know which email I should use, so I keep your old SOB.
---
 drivers/net/phy/marvell/Kconfig        |  12 +
 drivers/net/phy/marvell/Makefile       |   1 +
 drivers/net/phy/marvell/marvell_main.c |  18 +-
 drivers/net/phy/marvell/marvell_ptp.c  | 725 +++++++++++++++++++++++++++++++++
 drivers/net/phy/marvell/marvell_ptp.h  |  26 ++
 drivers/net/phy/marvell/marvell_tai.c  | 279 +++++++++++++
 drivers/net/phy/marvell/marvell_tai.h  |  36 ++
 7 files changed, 1096 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell/Kconfig b/drivers/net/phy/marvell/Kconfig
index a85bc9e4311e6bedd4a89db9527aca82d55a0762..2e4f42cfed9d1dfa16c1140e244c63b484e98ff9 100644
--- a/drivers/net/phy/marvell/Kconfig
+++ b/drivers/net/phy/marvell/Kconfig
@@ -4,6 +4,18 @@ config MARVELL_PHY
 	help
 	  Currently has a driver for the 88E1XXX
 
+config MARVELL_PHY_PTP
+	bool "Marvell PHY PTP support"
+	depends on NETWORK_PHY_TIMESTAMPING
+	depends on PTP_1588_CLOCK
+	depends on MARVELL_PHY
+	help
+	  Support PHY timestamping on Marvell 88E1510, 88E1512, 88E1514
+	  and 88E1518 PHYs.
+
+	  N.B. In order for this to be fully functional, your MAC driver
+	  must call the skb_tx_timestamp() function.
+
 config MARVELL_10G_PHY
 	tristate "Marvell Alaska 10Gbit PHYs"
 	help
diff --git a/drivers/net/phy/marvell/Makefile b/drivers/net/phy/marvell/Makefile
index 2c3622b053d1f54eba518b06730b797fb103ee06..ec0540376d6dae09fd25e8c562eda72bb572a752 100644
--- a/drivers/net/phy/marvell/Makefile
+++ b/drivers/net/phy/marvell/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_MARVELL_10G_PHY)	+= marvell10g.o
 marvell-y			:= marvell_main.o
+marvell-$(CONFIG_MARVELL_PHY_PTP)	+= marvell_ptp.o marvell_tai.o
 obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
 obj-$(CONFIG_MARVELL_88Q2XXX_PHY)	+= marvell-88q2xxx.o
 obj-$(CONFIG_MARVELL_88X2222_PHY)	+= marvell-88x2222.o
diff --git a/drivers/net/phy/marvell/marvell_main.c b/drivers/net/phy/marvell/marvell_main.c
index 623292948fa706a2b0d8b98919ead8b609bbd949..7b52c153d78a2e8de081922d1eb6a097e6c595c1 100644
--- a/drivers/net/phy/marvell/marvell_main.c
+++ b/drivers/net/phy/marvell/marvell_main.c
@@ -38,6 +38,8 @@
 #include <asm/irq.h>
 #include <linux/uaccess.h>
 
+#include "marvell_ptp.h"
+
 #define MII_MARVELL_PHY_PAGE		22
 #define MII_MARVELL_COPPER_PAGE		0x00
 #define MII_MARVELL_FIBER_PAGE		0x01
@@ -425,6 +427,15 @@ static irqreturn_t marvell_handle_interrupt(struct phy_device *phydev)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t m88e1510_handle_interrupt(struct phy_device *phydev)
+{
+	irqreturn_t handled;
+
+	handled = marvell_ptp_irq(phydev);
+	handled |= marvell_handle_interrupt(phydev);
+	return handled;
+}
+
 static int marvell_set_polarity(struct phy_device *phydev, int polarity)
 {
 	u16 val;
@@ -3655,6 +3666,10 @@ static int m88e1510_probe(struct phy_device *phydev)
 	if (err)
 		return err;
 
+	err = marvell_ptp_probe(phydev);
+	if (err)
+		return err;
+
 	return phy_sfp_probe(phydev, &m88e1510_sfp_ops);
 }
 
@@ -3916,11 +3931,12 @@ static struct phy_driver marvell_drivers[] = {
 		.features = PHY_GBIT_FIBRE_FEATURES,
 		.flags = PHY_POLL_CABLE_TEST,
 		.probe = m88e1510_probe,
+		.remove = marvell_ptp_remove,
 		.config_init = m88e1510_config_init,
 		.config_aneg = m88e1510_config_aneg,
 		.read_status = marvell_read_status,
 		.config_intr = marvell_config_intr,
-		.handle_interrupt = marvell_handle_interrupt,
+		.handle_interrupt = m88e1510_handle_interrupt,
 		.get_wol = m88e1318_get_wol,
 		.set_wol = m88e1318_set_wol,
 		.resume = marvell_resume,
diff --git a/drivers/net/phy/marvell/marvell_ptp.c b/drivers/net/phy/marvell/marvell_ptp.c
new file mode 100644
index 0000000000000000000000000000000000000000..e5d0378a97c4daaae9ad574a46de6b3afa82cbca
--- /dev/null
+++ b/drivers/net/phy/marvell/marvell_ptp.c
@@ -0,0 +1,725 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Marvell PTP driver for 88E1510, 88E1512, 88E1514 and 88E1518 PHYs
+ *
+ * Ideas taken from 88E6xxx DSA and DP83640 drivers. This file
+ * implements the packet timestamping support only (PTP).  TAI
+ * support is separate.
+ */
+#include <linux/if_vlan.h>
+#include <linux/list.h>
+#include <linux/netdevice.h>
+#include <linux/net_tstamp.h>
+#include <linux/phy.h>
+#include <linux/ptp_classify.h>
+#include <linux/ptp_clock_kernel.h>
+#include <linux/uaccess.h>
+
+#include "marvell_ptp.h"
+#include "marvell_tai.h"
+
+#define TX_TIMEOUT_MS	40
+#define RX_TIMEOUT_MS	40
+
+#define MARVELL_PAGE_PTP_PORT_1			8
+#define PTP1_PORT_CONFIG_0			0
+#define PTP1_PORT_CONFIG_0_DISTSPECCHECK	BIT(11)
+#define PTP1_PORT_CONFIG_0_DISTSOVERWRITE	BIT(1)
+#define PTP1_PORT_CONFIG_0_DISPTP		BIT(0)
+#define PTP1_PORT_CONFIG_1			1
+#define PTP1_PORT_CONFIG_1_IPJUMP(x)		(((x) & 0x3f) << 8)
+#define PTP1_PORT_CONFIG_1_ETJUMP(x)		((x) & 0x1f)
+#define PTP1_PORT_CONFIG_2			2
+#define PTP1_PORT_CONFIG_2_DEPINTEN		BIT(1)
+#define PTP1_PORT_CONFIG_2_ARRINTEN		BIT(0)
+#define PTP1_ARR_STATUS0			8
+#define PTP1_ARR_STATUS1			12
+
+#define MARVELL_PAGE_PTP_PORT_2			9
+#define PTP2_DEP_STATUS				0
+
+#define MARVELL_PAGE_PTP_GLOBAL_CONFIG		14
+#define PTP_GLOBAL_CONFIG1			1
+
+struct marvell_ptp_cb {
+	unsigned long timeout;
+	u16 seq;
+};
+
+#define MARVELL_PTP_CB(skb)	((struct marvell_ptp_cb *)(skb)->cb)
+
+struct marvell_rxts {
+	struct list_head node;
+	u64 ns;
+	u16 seq;
+};
+
+struct marvell_ptp {
+	struct marvell_tai *tai;
+	struct list_head tai_node;
+	struct phy_device *phydev;
+	struct mii_timestamper mii_ts;
+
+	/* We only support one outstanding transmit skb */
+	struct sk_buff *tx_skb;
+	enum hwtstamp_tx_types tx_type;
+
+	struct mutex rx_mutex; /* Protect rx_queue, rx_pend and rx_free */
+	struct list_head rx_free;
+	struct list_head rx_pend;
+	struct sk_buff_head rx_queue;
+	enum hwtstamp_rx_filters rx_filter;
+	struct marvell_rxts rx_ts[64];
+
+	struct delayed_work ts_work;
+};
+
+struct marvell_ts {
+	u32 time;
+	u16 stat;
+#define MV_STATUS_INTSTATUS_MASK		0x0006
+#define MV_STATUS_INTSTATUS_NORMAL		0x0000
+#define MV_STATUS_VALID				BIT(0)
+	u16 seq;
+};
+
+/**
+ * marvell_read_tstamp - read the status, timestamp and sequence
+ * @phydev: Pointer to the phy_device structure
+ * @ts: Pointer to marvell_ts structure
+ * @page: Registers page number
+ * @reg: Register index
+ *
+ * Read the status, timestamp and PTP common header sequence from the PHY.
+ * Apparently, reading these are atomic, but there is no mention how the
+ * PHY treats this access as atomic. So, we set the DisTSOverwrite bit
+ * when configuring the PHY.
+ *
+ * Return: 1 on success, 0 for invalid status, failure value on error
+ */
+static int marvell_read_tstamp(struct phy_device *phydev,
+			       struct marvell_ts *ts,
+			       u8 page, u8 reg)
+{
+	int oldpage;
+	int ret = 0;
+
+	/* Read status register */
+	oldpage = phy_select_page(phydev, page);
+	if (oldpage >= 0) {
+		ret = __phy_read(phydev, reg);
+		if (ret < 0)
+			goto restore;
+
+		ts->stat = ret;
+		if (!(ts->stat & MV_STATUS_VALID)) {
+			ret = 0;
+			goto restore;
+		}
+
+		/* Read low timestamp */
+		ret = __phy_read(phydev, reg + 1);
+		if (ret < 0)
+			goto restore;
+
+		ts->time = ret;
+
+		/* Read high timestamp */
+		ret = __phy_read(phydev, reg + 2);
+		if (ret < 0)
+			goto restore;
+
+		ts->time |= ret << 16;
+
+		/* Read sequence */
+		ret = __phy_read(phydev, reg + 3);
+		if (ret < 0)
+			goto restore;
+
+		ts->seq = ret;
+
+		/* Clear valid */
+		__phy_write(phydev, reg, 0);
+
+		/* Return 1 as 0 is for invalid status */
+		ret = 1;
+	}
+restore:
+	return phy_restore_page(phydev, oldpage, ret);
+}
+
+static struct marvell_ptp *mii_ts_to_ptp(struct mii_timestamper *mii_ts)
+{
+	return container_of(mii_ts, struct marvell_ptp, mii_ts);
+}
+
+/**
+ * marvell_ptp_rx - Deliver skb with timestamp
+ * @skb: rx socket buffer
+ * @ns: Timestamp in ns
+ *
+ * Deliver a skb with its timestamp back to the networking core
+ */
+static void marvell_ptp_rx(struct sk_buff *skb, u64 ns)
+{
+	struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb);
+
+	memset(shhwtstamps, 0, sizeof(*shhwtstamps));
+	shhwtstamps->hwtstamp = ns_to_ktime(ns);
+	netif_rx(skb);
+}
+
+/**
+ * marvell_ptp_get_rxts - Get a rx timestamp entry
+ * @ptp: Pointer to marvell_ptp structure
+ *
+ * Get a rx timestamp entry. Try the free list, and if that fails,
+ * steal the oldest off the pending list.
+ *
+ * Return: marvell_rxts list element or zero
+ */
+static struct marvell_rxts *marvell_ptp_get_rxts(struct marvell_ptp *ptp)
+{
+	if (!list_empty(&ptp->rx_free))
+		return list_first_entry(&ptp->rx_free, struct marvell_rxts,
+					node);
+
+	return list_last_entry(&ptp->rx_pend, struct marvell_rxts, node);
+}
+
+/**
+ * marvell_ptp_rx_ts - Check for a rx timestamp entry
+ * @ptp: Pointer to marvell_ptp structure
+ *
+ * Check for a rx timestamp entry, try to find the corresponding skb and
+ * deliver it, otherwise add the rx timestamp to the queue of pending
+ * timestamps.
+ *
+ * Return: 1 if found, 0 if no rx timestamp, -1 in case of timestamp overrun
+ */
+static int marvell_ptp_rx_ts(struct marvell_ptp *ptp)
+{
+	struct marvell_rxts *rxts;
+	struct marvell_ts ts;
+	struct sk_buff *skb;
+	bool found = false;
+	u64 ns;
+	int err;
+
+	err = marvell_read_tstamp(ptp->phydev, &ts, MARVELL_PAGE_PTP_PORT_1,
+				  PTP1_ARR_STATUS0);
+	if (err <= 0)
+		return 0;
+
+	if ((ts.stat & MV_STATUS_INTSTATUS_MASK) !=
+	    MV_STATUS_INTSTATUS_NORMAL) {
+		dev_warn(&ptp->phydev->mdio.dev,
+			 "rx timestamp overrun (%x)\n", ts.stat);
+		return -1;
+	}
+
+	ns = marvell_tai_cyc2time(ptp->tai, ts.time);
+
+	mutex_lock(&ptp->rx_mutex);
+
+	/* Search the rx queue for a matching skb */
+	skb_queue_walk(&ptp->rx_queue, skb) {
+		if (MARVELL_PTP_CB(skb)->seq == ts.seq) {
+			__skb_unlink(skb, &ptp->rx_queue);
+			found = true;
+			break;
+		}
+	}
+
+	if (!found) {
+		rxts = marvell_ptp_get_rxts(ptp);
+		rxts->ns = ns;
+		rxts->seq = ts.seq;
+		list_move(&rxts->node, &ptp->rx_pend);
+	}
+
+	mutex_unlock(&ptp->rx_mutex);
+
+	if (found)
+		marvell_ptp_rx(skb, ns);
+
+	return 1;
+}
+
+/**
+ * marvell_ptp_rxtstamp - Validate rx packet
+ * @mii_ts: Pointer to mii_timestamper structure
+ * @skb: rx socket buffer
+ * @type: PTP class type
+ *
+ * Check whether the packet is suitable for timestamping, and if so,
+ * try to find a pending timestamp for it. If no timestamp is found,
+ * queue the packet with a timeout.
+ *
+ * Return: true if a ptp packet detected and rx filter is enabled false
+ *	   otherwise.
+ */
+static bool marvell_ptp_rxtstamp(struct mii_timestamper *mii_ts,
+				 struct sk_buff *skb, int type)
+{
+	struct marvell_ptp *ptp = mii_ts_to_ptp(mii_ts);
+	struct ptp_header *ptp_hdr;
+	struct marvell_rxts *rxts;
+	bool found = false;
+	u16 seqid;
+	u64 ns;
+
+	if (ptp->rx_filter == HWTSTAMP_FILTER_NONE)
+		return false;
+
+	ptp_hdr = ptp_parse_header(skb, type);
+	if (!ptp_hdr)
+		return false;
+
+	seqid = ntohs(ptp_hdr->sequence_id);
+
+	mutex_lock(&ptp->rx_mutex);
+
+	/* Search the pending receive timestamps for a matching seqid */
+	list_for_each_entry(rxts, &ptp->rx_pend, node) {
+		if (rxts->seq == seqid) {
+			found = true;
+			ns = rxts->ns;
+			/* Move this timestamp entry to the free list */
+			list_move_tail(&rxts->node, &ptp->rx_free);
+			break;
+		}
+	}
+
+	if (!found) {
+		/* Store the seqid and queue the skb. Do this under the lock
+		 * to ensure we don't miss any timestamps appended to the
+		 * rx_pend list.
+		 */
+		MARVELL_PTP_CB(skb)->seq = seqid;
+		MARVELL_PTP_CB(skb)->timeout = jiffies +
+			msecs_to_jiffies(RX_TIMEOUT_MS);
+		__skb_queue_tail(&ptp->rx_queue, skb);
+	}
+
+	mutex_unlock(&ptp->rx_mutex);
+
+	if (found) {
+		/* We found the corresponding timestamp. If we can add the
+		 * timestamp, do we need to go through the netif_rx()
+		 * path, or would it be more efficient to add the timestamp
+		 * and return "false" here?
+		 */
+		marvell_ptp_rx(skb, ns);
+	} else {
+		if (ptp->phydev->irq < 0)
+			schedule_delayed_work(&ptp->ts_work, 2);
+	}
+
+	return true;
+}
+
+/**
+ * marvell_ptp_rx_expire - Manage expired socket
+ * @ptp: Pointer to marvell_ptp structure
+ *
+ * Move any expired skbs on to our own list, and then hand the contents of
+ * our list to netif_rx() - this avoids calling netif_rx() with our
+ * mutex held.
+ */
+static void marvell_ptp_rx_expire(struct marvell_ptp *ptp)
+{
+	struct sk_buff_head list;
+	struct sk_buff *skb;
+
+	__skb_queue_head_init(&list);
+
+	mutex_lock(&ptp->rx_mutex);
+	while ((skb = skb_dequeue(&ptp->rx_queue)) != NULL) {
+		if (!time_is_before_jiffies(MARVELL_PTP_CB(skb)->timeout)) {
+			__skb_queue_head(&ptp->rx_queue, skb);
+			break;
+		}
+		__skb_queue_tail(&list, skb);
+	}
+	mutex_unlock(&ptp->rx_mutex);
+
+	while ((skb = __skb_dequeue(&list)) != NULL)
+		netif_rx(skb);
+}
+
+/**
+ * marvell_ptp_txtstamp_complete - Complete tx timestamp
+ * @ptp: Pointer to marvell_ptp structure
+ *
+ * Complete the transmit timestamping. This is called to read the transmit
+ * timestamp from the PHY, and report back the transmitted timestamp.
+ *
+ * Return: 1 on success, 0 on tx timestamp timeout case, -1 on error
+ */
+static int marvell_ptp_txtstamp_complete(struct marvell_ptp *ptp)
+{
+	struct skb_shared_hwtstamps shhwtstamps;
+	struct sk_buff *skb = ptp->tx_skb;
+	struct marvell_ts ts;
+	int err;
+	u64 ns;
+
+	err = marvell_read_tstamp(ptp->phydev, &ts, MARVELL_PAGE_PTP_PORT_2,
+				  PTP2_DEP_STATUS);
+	if (err < 0)
+		goto fail;
+
+	if (err == 0) {
+		if (time_is_before_jiffies(MARVELL_PTP_CB(skb)->timeout)) {
+			dev_warn(&ptp->phydev->mdio.dev,
+				 "tx timestamp timeout\n");
+			goto free;
+		}
+		return 0;
+	}
+
+	/* Check the status */
+	if ((ts.stat & MV_STATUS_INTSTATUS_MASK) !=
+	    MV_STATUS_INTSTATUS_NORMAL) {
+		dev_warn(&ptp->phydev->mdio.dev,
+			 "tx timestamp overrun (%x)\n", ts.stat);
+		goto free;
+	}
+
+	/* Reject if the sequence number doesn't match */
+	if (ts.seq != MARVELL_PTP_CB(skb)->seq) {
+		dev_warn(&ptp->phydev->mdio.dev,
+			 "tx timestamp unexpected sequence id\n");
+		goto free;
+	}
+
+	ptp->tx_skb = NULL;
+
+	/* Set the timestamp */
+	ns = marvell_tai_cyc2time(ptp->tai, ts.time);
+	memset(&shhwtstamps, 0, sizeof(shhwtstamps));
+	shhwtstamps.hwtstamp = ns_to_ktime(ns);
+	skb_complete_tx_timestamp(skb, &shhwtstamps);
+	return 1;
+
+fail:
+	dev_err_ratelimited(&ptp->phydev->mdio.dev,
+			    "failed reading PTP: %pe\n", ERR_PTR(err));
+free:
+	dev_kfree_skb_any(skb);
+	ptp->tx_skb = NULL;
+	return -1;
+}
+
+/**
+ * marvell_ptp_do_txtstamp - Prepared tx socket with timestamping
+ * @mii_ts: Pointer to mii_timestamper structure
+ * @skb: tx socket buffer
+ * @type: PTP class type
+ *
+ * Check whether the skb will be timestamped on transmit. We only support
+ * a single outstanding skb. Add it if the slot is available.
+ *
+ * Return: True if packet can be transmitted false otherwise
+ */
+static bool marvell_ptp_do_txtstamp(struct mii_timestamper *mii_ts,
+				    struct sk_buff *skb, int type)
+{
+	struct marvell_ptp *ptp = mii_ts_to_ptp(mii_ts);
+	struct ptp_header *ptp_hdr;
+
+	if (ptp->tx_type != HWTSTAMP_TX_ON)
+		return false;
+
+	if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
+		return false;
+
+	ptp_hdr = ptp_parse_header(skb, type);
+	if (!ptp_hdr)
+		return false;
+
+	MARVELL_PTP_CB(skb)->seq = ntohs(ptp_hdr->sequence_id);
+	MARVELL_PTP_CB(skb)->timeout = jiffies +
+		msecs_to_jiffies(TX_TIMEOUT_MS);
+
+	if (cmpxchg(&ptp->tx_skb, NULL, skb))
+		return false;
+
+	/* DP83640 marks the skb for hw timestamping. Since the MAC driver
+	 * may call skb_tx_timestamp() but may not support timestamping
+	 * itself, it may not set this flag. So, we need to do this here.
+	 */
+	skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+	if (ptp->phydev->irq < 0)
+		schedule_delayed_work(&ptp->ts_work, 2);
+
+	return true;
+}
+
+static void marvell_ptp_txtstamp(struct mii_timestamper *mii_ts,
+				 struct sk_buff *skb, int type)
+{
+	if (!marvell_ptp_do_txtstamp(mii_ts, skb, type))
+		kfree_skb(skb);
+}
+
+static int marvell_ptp_hwtstamp(struct mii_timestamper *mii_ts,
+				struct kernel_hwtstamp_config *config,
+				struct netlink_ext_ack *extack)
+{
+	struct marvell_ptp *ptp = mii_ts_to_ptp(mii_ts);
+	u16 cfg0 = PTP1_PORT_CONFIG_0_DISPTP;
+	u16 cfg2 = 0;
+	int err;
+
+	if (config->flags)
+		return -EINVAL;
+
+	switch (config->tx_type) {
+	case HWTSTAMP_TX_OFF:
+		break;
+
+	case HWTSTAMP_TX_ON:
+		cfg0 = 0;
+		if (ptp->phydev->irq >= 0)
+			cfg2 |= PTP1_PORT_CONFIG_2_DEPINTEN;
+		break;
+
+	default:
+		return -ERANGE;
+	}
+
+	switch (config->rx_filter) {
+	case HWTSTAMP_FILTER_NONE:
+		break;
+
+	case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
+	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
+	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
+	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
+	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
+	case HWTSTAMP_FILTER_PTP_V2_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
+		/* We accept 802.1AS, IEEE 1588v1 and IEEE 1588v2. We could
+		 * filter on 802.1AS using the transportSpecific field, but
+		 * that affects the transmit path too.
+		 */
+		config->rx_filter = HWTSTAMP_FILTER_SOME;
+		cfg0 = 0;
+		if (ptp->phydev->irq >= 0)
+			cfg2 |= PTP1_PORT_CONFIG_2_ARRINTEN;
+		break;
+
+	default:
+		return -ERANGE;
+	}
+
+	err = phy_modify_paged(ptp->phydev, MARVELL_PAGE_PTP_PORT_1,
+			       PTP1_PORT_CONFIG_0,
+			       PTP1_PORT_CONFIG_0_DISPTP, cfg0);
+	if (err)
+		return err;
+
+	err = phy_write_paged(ptp->phydev, MARVELL_PAGE_PTP_PORT_1,
+			      PTP1_PORT_CONFIG_2, cfg2);
+	if (err)
+		return err;
+
+	ptp->tx_type = config->tx_type;
+	ptp->rx_filter = config->rx_filter;
+
+	return 0;
+}
+
+static int marvell_ptp_ts_info(struct mii_timestamper *mii_ts,
+			       struct kernel_ethtool_ts_info *ts_info)
+{
+	struct marvell_ptp *ptp = mii_ts_to_ptp(mii_ts);
+
+	ts_info->so_timestamping = SOF_TIMESTAMPING_TX_HARDWARE |
+				   SOF_TIMESTAMPING_RX_HARDWARE |
+				   SOF_TIMESTAMPING_RAW_HARDWARE;
+	ts_info->phc_index = ptp_clock_index(ptp->tai->ptp_clock);
+	ts_info->tx_types = BIT(HWTSTAMP_TX_OFF) |
+			    BIT(HWTSTAMP_TX_ON);
+	ts_info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) |
+			      BIT(HWTSTAMP_FILTER_SOME);
+
+	return 0;
+}
+
+static int marvell_ptp_port_config(struct phy_device *phydev)
+{
+	int err;
+
+	/* Disable transport specific check (if the PTP common header)
+	 * Disable timestamp overwriting (so we can read a stable entry.)
+	 * Disable PTP
+	 */
+	err = phy_write_paged(phydev, MARVELL_PAGE_PTP_PORT_1,
+			      PTP1_PORT_CONFIG_0,
+			      PTP1_PORT_CONFIG_0_DISTSPECCHECK |
+			      PTP1_PORT_CONFIG_0_DISTSOVERWRITE |
+			      PTP1_PORT_CONFIG_0_DISPTP);
+	if (err < 0)
+		return err;
+
+	/* Set ether-type jump to 12 (to ether protocol)
+	 * Set IP jump to 2 (to skip over ether protocol)
+	 * Does this mean it won't pick up on VLAN packets?
+	 */
+	err = phy_write_paged(phydev, MARVELL_PAGE_PTP_PORT_1,
+			      PTP1_PORT_CONFIG_1,
+			      PTP1_PORT_CONFIG_1_ETJUMP(12) |
+			      PTP1_PORT_CONFIG_1_IPJUMP(2));
+	if (err < 0)
+		return err;
+
+	/* Timestamp only sync (MessageID 1) and delay_req (MessageID 2)
+	 * PTP frames
+	 */
+	err = phy_write_paged(phydev, MARVELL_PAGE_PTP_GLOBAL_CONFIG,
+			      PTP_GLOBAL_CONFIG1, 0x3);
+	if (err < 0)
+		return err;
+
+	/* Disable all interrupts */
+	phy_write_paged(phydev, MARVELL_PAGE_PTP_PORT_1,
+			PTP1_PORT_CONFIG_2, 0);
+
+	return 0;
+}
+
+static void marvell_ptp_port_disable(struct phy_device *phydev)
+{
+	/* Disable PTP */
+	phy_write_paged(phydev, MARVELL_PAGE_PTP_PORT_1,
+			PTP1_PORT_CONFIG_0, PTP1_PORT_CONFIG_0_DISPTP);
+
+	/* Disable interrupts */
+	phy_write_paged(phydev, MARVELL_PAGE_PTP_PORT_1,
+			PTP1_PORT_CONFIG_2, 0);
+}
+
+/**
+ * marvell_ptp_irq - PTP IRQ handler
+ * @phydev: Pointer to phy_device structure
+ *
+ * Return: IRQ_HANDLED/IRQ_NONE
+ */
+irqreturn_t marvell_ptp_irq(struct phy_device *phydev)
+{
+	struct marvell_ptp *ptp;
+	irqreturn_t ret = IRQ_NONE;
+
+	if (!phydev->mii_ts)
+		return ret;
+
+	ptp = mii_ts_to_ptp(phydev->mii_ts);
+	if (marvell_ptp_rx_ts(ptp))
+		ret = IRQ_HANDLED;
+
+	if (ptp->tx_skb && marvell_ptp_txtstamp_complete(ptp))
+		ret = IRQ_HANDLED;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(marvell_ptp_irq);
+
+static void marvell_ptp_worker(struct work_struct *work)
+{
+	struct marvell_ptp *ptp = container_of(work, struct marvell_ptp,
+					       ts_work.work);
+
+	if (ptp->tx_skb)
+		marvell_ptp_txtstamp_complete(ptp);
+
+	marvell_ptp_rx_ts(ptp);
+
+	marvell_ptp_rx_expire(ptp);
+
+	if (!skb_queue_empty(&ptp->rx_queue) || ptp->tx_skb)
+		schedule_delayed_work(&ptp->ts_work, 2);
+}
+
+int marvell_ptp_probe(struct phy_device *phydev)
+{
+	struct marvell_ptp *ptp;
+	int err, i;
+
+	ptp = devm_kzalloc(&phydev->mdio.dev, sizeof(*ptp), GFP_KERNEL);
+	if (!ptp)
+		return -ENOMEM;
+
+	ptp->phydev = phydev;
+	ptp->mii_ts.rxtstamp = marvell_ptp_rxtstamp;
+	ptp->mii_ts.txtstamp = marvell_ptp_txtstamp;
+	ptp->mii_ts.hwtstamp = marvell_ptp_hwtstamp;
+	ptp->mii_ts.ts_info = marvell_ptp_ts_info;
+
+	/* No phy interrupt */
+	if (phydev->irq < 0)
+		INIT_DELAYED_WORK(&ptp->ts_work, marvell_ptp_worker);
+	mutex_init(&ptp->rx_mutex);
+	INIT_LIST_HEAD(&ptp->rx_free);
+	INIT_LIST_HEAD(&ptp->rx_pend);
+	skb_queue_head_init(&ptp->rx_queue);
+
+	for (i = 0; i < ARRAY_SIZE(ptp->rx_ts); i++)
+		list_add_tail(&ptp->rx_ts[i].node, &ptp->rx_free);
+
+	/* Get the TAI for this PHY. */
+	err = marvell_tai_get(&ptp->tai, phydev);
+	if (err)
+		return err;
+
+	/* Configure this PTP port */
+	err = marvell_ptp_port_config(phydev);
+	if (err) {
+		marvell_tai_put(ptp->tai);
+		return err;
+	}
+
+	phydev->mii_ts = &ptp->mii_ts;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(marvell_ptp_probe);
+
+void marvell_ptp_remove(struct phy_device *phydev)
+{
+	struct marvell_ptp *ptp;
+
+	if (!phydev->mii_ts)
+		return;
+
+	/* Disconnect from the net subsystem - we assume there is no
+	 * packet activity at this point.
+	 */
+	ptp = mii_ts_to_ptp(phydev->mii_ts);
+	phydev->mii_ts = NULL;
+
+	if (phydev->irq < 0)
+		cancel_delayed_work_sync(&ptp->ts_work);
+
+	/* Free or dequeue all pending skbs */
+	if (ptp->tx_skb)
+		kfree_skb(ptp->tx_skb);
+
+	skb_queue_purge(&ptp->rx_queue);
+
+	/* Ensure that the port is disabled */
+	marvell_ptp_port_disable(phydev);
+	marvell_tai_put(ptp->tai);
+}
+EXPORT_SYMBOL_GPL(marvell_ptp_remove);
+
+MODULE_AUTHOR("Russell King");
+MODULE_DESCRIPTION("Marvell PHY PTP library");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/phy/marvell/marvell_ptp.h b/drivers/net/phy/marvell/marvell_ptp.h
new file mode 100644
index 0000000000000000000000000000000000000000..678f7a499a19eafc8d5b8de32a3dfbdc3094acf8
--- /dev/null
+++ b/drivers/net/phy/marvell/marvell_ptp.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef MARVELL_PTP_H
+#define MARVELL_PTP_H
+
+#if IS_ENABLED(CONFIG_MARVELL_PHY_PTP)
+void marvell_ptp_check(struct phy_device *phydev);
+int marvell_ptp_probe(struct phy_device *phydev);
+void marvell_ptp_remove(struct phy_device *phydev);
+irqreturn_t marvell_ptp_irq(struct phy_device *phydev);
+#else
+static inline int marvell_ptp_probe(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static inline void marvell_ptp_remove(struct phy_device *phydev)
+{
+}
+
+static inline irqreturn_t marvell_ptp_irq(struct phy_device *phydev)
+{
+	return IRQ_NONE;
+}
+#endif
+
+#endif
diff --git a/drivers/net/phy/marvell/marvell_tai.c b/drivers/net/phy/marvell/marvell_tai.c
new file mode 100644
index 0000000000000000000000000000000000000000..f6608a0986ca7edd297b4de6699114109e5e6d4c
--- /dev/null
+++ b/drivers/net/phy/marvell/marvell_tai.c
@@ -0,0 +1,279 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Marvell PTP driver for 88E1510, 88E1512, 88E1514 and 88E1518 PHYs
+ *
+ * This file implements TAI support as a PTP clock. Timecounter/cyclecounter
+ * representation taken from Marvell 88E6xxx DSA driver. We may need to share
+ * the TAI between multiple PHYs in a multiport PHY.
+ */
+#include <linux/ktime.h>
+#include <linux/slab.h>
+#include <linux/phy.h>
+#include "marvell_tai.h"
+
+#define MARVELL_PAGE_MISC			6
+#define GCR					20
+#define GCR_PTP_POWER_DOWN			BIT(9)
+#define GCR_PTP_REF_CLOCK_SOURCE		BIT(8)
+#define GCR_PTP_INPUT_SOURCE			BIT(7)
+#define GCR_PTP_OUTPUT				BIT(6)
+
+#define MARVELL_PAGE_TAI_GLOBAL			12
+#define TAI_CONFIG_0				0
+#define TAI_CONFIG_0_EVENTCAPOV			BIT(15)
+#define TAI_CONFIG_0_TRIGGENINTEN		BIT(9)
+#define TAI_CONFIG_0_EVENTCAPINTEN		BIT(8)
+
+#define TAI_CONFIG_9				9
+#define TAI_CONFIG_9_EVENTERR			BIT(9)
+#define TAI_CONFIG_9_EVENTCAPVALID		BIT(8)
+
+#define TAI_EVENT_CAPTURE_TIME_LO		10
+#define TAI_EVENT_CAPTURE_TIME_HI		11
+
+#define MARVELL_PAGE_PTP_GLOBAL			14
+#define PTPG_CONFIG_0				0
+#define PTPG_CONFIG_1				1
+#define PTPG_CONFIG_2				2
+#define PTPG_CONFIG_3				3
+#define PTPG_CONFIG_3_TSATSFD			BIT(0)
+#define PTPG_STATUS				8
+#define PTPG_READPLUS_COMMAND			14
+#define PTPG_READPLUS_DATA			15
+
+static DEFINE_SPINLOCK(tai_list_lock);
+static LIST_HEAD(tai_list);
+
+static struct marvell_tai *cc_to_tai(const struct cyclecounter *cc)
+{
+	return container_of(cc, struct marvell_tai, cyclecounter);
+}
+
+/* Read the global time registers using the readplus command */
+static u64 marvell_tai_clock_read(const struct cyclecounter *cc)
+{
+	struct marvell_tai *tai = cc_to_tai(cc);
+	struct phy_device *phydev = tai->phydev;
+	int err, oldpage, lo, hi;
+
+	oldpage = phy_select_page(phydev, MARVELL_PAGE_PTP_GLOBAL);
+	if (oldpage >= 0) {
+		/* 88e151x says to write 0x8e0e */
+		ptp_read_system_prets(tai->sts);
+		err = __phy_write(phydev, PTPG_READPLUS_COMMAND, 0x8e0e);
+		ptp_read_system_postts(tai->sts);
+		lo = __phy_read(phydev, PTPG_READPLUS_DATA);
+		hi = __phy_read(phydev, PTPG_READPLUS_DATA);
+	}
+	err = phy_restore_page(phydev, oldpage, err);
+
+	if (err || lo < 0 || hi < 0)
+		return 0;
+
+	return lo | hi << 16;
+}
+
+u64 marvell_tai_cyc2time(struct marvell_tai *tai, u32 cyc)
+{
+	u64 ns;
+
+	mutex_lock(&tai->mutex);
+	ns = timecounter_cyc2time(&tai->timecounter, cyc);
+	mutex_unlock(&tai->mutex);
+
+	return ns;
+}
+
+static struct marvell_tai *ptp_to_tai(struct ptp_clock_info *ptp)
+{
+	return container_of(ptp, struct marvell_tai, caps);
+}
+
+static int marvell_tai_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
+{
+	struct marvell_tai *tai = ptp_to_tai(ptp);
+	bool neg;
+	u32 diff;
+	u64 adj;
+
+	neg = scaled_ppm < 0;
+	if (neg)
+		scaled_ppm = -scaled_ppm;
+
+	adj = tai->cc_mult_num;
+	adj *= scaled_ppm;
+	diff = div_u64(adj, tai->cc_mult_den);
+
+	mutex_lock(&tai->mutex);
+	timecounter_read(&tai->timecounter);
+	tai->cyclecounter.mult = neg ? tai->cc_mult - diff :
+				       tai->cc_mult + diff;
+	mutex_unlock(&tai->mutex);
+
+	return 0;
+}
+
+static int marvell_tai_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct marvell_tai *tai = ptp_to_tai(ptp);
+
+	mutex_lock(&tai->mutex);
+	timecounter_adjtime(&tai->timecounter, delta);
+	mutex_unlock(&tai->mutex);
+
+	return 0;
+}
+
+static int marvell_tai_gettimex64(struct ptp_clock_info *ptp,
+				  struct timespec64 *ts,
+				  struct ptp_system_timestamp *sts)
+{
+	struct marvell_tai *tai = ptp_to_tai(ptp);
+	u64 ns;
+
+	mutex_lock(&tai->mutex);
+	tai->sts = sts;
+	ns = timecounter_read(&tai->timecounter);
+	tai->sts = NULL;
+	mutex_unlock(&tai->mutex);
+
+	*ts = ns_to_timespec64(ns);
+
+	return 0;
+}
+
+static int marvell_tai_settime64(struct ptp_clock_info *ptp,
+				 const struct timespec64 *ts)
+{
+	struct marvell_tai *tai = ptp_to_tai(ptp);
+	u64 ns = timespec64_to_ns(ts);
+
+	mutex_lock(&tai->mutex);
+	timecounter_init(&tai->timecounter, &tai->cyclecounter, ns);
+	mutex_unlock(&tai->mutex);
+
+	return 0;
+}
+
+/* Periodically read the timecounter to keep the time refreshed. */
+static long marvell_tai_aux_work(struct ptp_clock_info *ptp)
+{
+	struct marvell_tai *tai = ptp_to_tai(ptp);
+
+	mutex_lock(&tai->mutex);
+	timecounter_read(&tai->timecounter);
+	mutex_unlock(&tai->mutex);
+
+	return tai->half_overflow_period;
+}
+
+/* Configure the global (shared between ports) configuration for the PHY. */
+static int marvell_tai_global_config(struct phy_device *phydev)
+{
+	int err;
+
+	/* Power up PTP */
+	err = phy_modify_paged(phydev, MARVELL_PAGE_MISC, GCR,
+			       GCR_PTP_POWER_DOWN, 0);
+	if (err)
+		return err;
+
+	/* Set ether-type for IEEE1588 packets */
+	err = phy_write_paged(phydev, MARVELL_PAGE_PTP_GLOBAL,
+			      PTPG_CONFIG_0, ETH_P_1588);
+	if (err < 0)
+		return err;
+
+	/* MsdIDTSEn - Enable timestamping on all PTP MessageIDs */
+	err = phy_write_paged(phydev, MARVELL_PAGE_PTP_GLOBAL,
+			      PTPG_CONFIG_1, ~0);
+	if (err < 0)
+		return err;
+
+	/* TSArrPtr - Point to Arr0 registers */
+	err = phy_write_paged(phydev, MARVELL_PAGE_PTP_GLOBAL,
+			      PTPG_CONFIG_2, 0);
+	if (err < 0)
+		return err;
+
+	/* TSAtSFD - timestamp at SFD */
+	err = phy_write_paged(phydev, MARVELL_PAGE_PTP_GLOBAL,
+			      PTPG_CONFIG_3, PTPG_CONFIG_3_TSATSFD);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+int marvell_tai_get(struct marvell_tai **taip, struct phy_device *phydev)
+{
+	struct marvell_tai *tai;
+	unsigned long overflow_ms;
+	int err;
+
+	err = marvell_tai_global_config(phydev);
+	if (err < 0)
+		return err;
+
+	tai = kzalloc(sizeof(*tai), GFP_KERNEL);
+	if (!tai)
+		return -ENOMEM;
+
+	mutex_init(&tai->mutex);
+
+	tai->phydev = phydev;
+
+	/* This assumes a 125MHz clock */
+	tai->cc_mult = 8 << 28;
+	tai->cc_mult_num = 1 << 9;
+	tai->cc_mult_den = 15625U;
+
+	tai->cyclecounter.read = marvell_tai_clock_read;
+	tai->cyclecounter.mask = CYCLECOUNTER_MASK(32);
+	tai->cyclecounter.mult = tai->cc_mult;
+	tai->cyclecounter.shift = 28;
+
+	overflow_ms = (1ULL << 32 * tai->cc_mult * 1000) >>
+			tai->cyclecounter.shift;
+	tai->half_overflow_period = msecs_to_jiffies(overflow_ms / 2);
+
+	timecounter_init(&tai->timecounter, &tai->cyclecounter,
+			 ktime_to_ns(ktime_get_real()));
+
+	tai->caps.owner = THIS_MODULE;
+	snprintf(tai->caps.name, sizeof(tai->caps.name), "Marvell PHY");
+	/* max_adj of 1000000 is what MV88E6xxx DSA uses */
+	tai->caps.max_adj = 1000000;
+	tai->caps.adjfine = marvell_tai_adjfine;
+	tai->caps.adjtime = marvell_tai_adjtime;
+	tai->caps.gettimex64 = marvell_tai_gettimex64;
+	tai->caps.settime64 = marvell_tai_settime64;
+	tai->caps.do_aux_work = marvell_tai_aux_work;
+
+	tai->ptp_clock = ptp_clock_register(&tai->caps, &phydev->mdio.dev);
+	if (IS_ERR(tai->ptp_clock)) {
+		kfree(tai);
+		return PTR_ERR(tai->ptp_clock);
+	}
+
+	ptp_schedule_worker(tai->ptp_clock, tai->half_overflow_period);
+
+	spin_lock(&tai_list_lock);
+	list_add_tail(&tai->tai_node, &tai_list);
+	spin_unlock(&tai_list_lock);
+
+	*taip = tai;
+
+	return 0;
+}
+
+void marvell_tai_put(struct marvell_tai *tai)
+{
+	ptp_clock_unregister(tai->ptp_clock);
+
+	spin_lock(&tai_list_lock);
+	list_del(&tai->tai_node);
+	spin_unlock(&tai_list_lock);
+
+	kfree(tai);
+}
diff --git a/drivers/net/phy/marvell/marvell_tai.h b/drivers/net/phy/marvell/marvell_tai.h
new file mode 100644
index 0000000000000000000000000000000000000000..bab8b430bcdd4db685b1025187d793ec3091fed8
--- /dev/null
+++ b/drivers/net/phy/marvell/marvell_tai.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef MARVELL_TAI_H
+#define MARVELL_TAI_H
+
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/ptp_clock_kernel.h>
+#include <linux/timecounter.h>
+
+struct phy_device;
+
+struct marvell_tai {
+	struct list_head tai_node;
+	struct phy_device *phydev;
+
+	struct ptp_clock_info caps;
+	struct ptp_clock *ptp_clock;
+
+	u32 cc_mult_num;
+	u32 cc_mult_den;
+	u32 cc_mult;
+
+	struct mutex mutex; /* Portect timecouter and cyclecounter */
+	struct timecounter timecounter;
+	struct cyclecounter cyclecounter;
+	long half_overflow_period;
+
+	/* Used while reading the TAI */
+	struct ptp_system_timestamp *sts;
+};
+
+u64 marvell_tai_cyc2time(struct marvell_tai *tai, u32 cyc);
+int marvell_tai_get(struct marvell_tai **taip, struct phy_device *phydev);
+void marvell_tai_put(struct marvell_tai *tai);
+
+#endif

-- 
2.34.1

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

On Mon, 07 Apr 2025 16:03:01 +0200
Kory Maincent <kory.maincent@bootlin.com> wrote:

> From: Russell King <rmk+kernel@armlinux.org.uk>
> 
> From: Russell King <rmk+kernel@armlinux.org.uk>

It seems b4 is automatically adding another From tag is the Author is different.
Sorry for that.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Simon Horman]

On Mon, Apr 07, 2025 at 04:03:01PM +0200, Kory Maincent wrote:
> From: Russell King <rmk+kernel@armlinux.org.uk>
> 
> From: Russell King <rmk+kernel@armlinux.org.uk>
> 
> Add PTP basic support for Marvell 88E151x PHYs. These PHYs support
> timestamping the egress and ingress of packets, but does not support
> any packet modification.
> 
> The PHYs support hardware pins for providing an external clock for the
> TAI counter, and a separate pin that can be used for event capture or
> generation of a trigger (either a pulse or periodic).  This code does
> not support either of these modes.
> 
> The driver takes inspiration from the Marvell 88E6xxx DSA and DP83640
> drivers.  The hardware is very similar to the implementation found in
> the 88E6xxx DSA driver, but the access methods are very different,
> although it may be possible to create a library that both can use
> along with accessor functions.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> Add support for interruption.
> Fix L2 PTP encapsulation frame detection.
> Fix first PTP timestamp being dropped.
> Fix Kconfig to depends on MARVELL_PHY.
> Update comments to use kdoc.
> 
> Co-developed-by: Kory Maincent <kory.maincent@bootlin.com>
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Hi Kory,

Some minor feedback from my side.

> ---
> 
> Russell I don't know which email I should use, so I keep your old SOB.

Russell's SOB seems to be missing.

...

> diff --git a/drivers/net/phy/marvell/marvell_tai.c b/drivers/net/phy/marvell/marvell_tai.c

...

> +/* Read the global time registers using the readplus command */
> +static u64 marvell_tai_clock_read(const struct cyclecounter *cc)
> +{
> +	struct marvell_tai *tai = cc_to_tai(cc);
> +	struct phy_device *phydev = tai->phydev;
> +	int err, oldpage, lo, hi;
> +
> +	oldpage = phy_select_page(phydev, MARVELL_PAGE_PTP_GLOBAL);
> +	if (oldpage >= 0) {
> +		/* 88e151x says to write 0x8e0e */
> +		ptp_read_system_prets(tai->sts);
> +		err = __phy_write(phydev, PTPG_READPLUS_COMMAND, 0x8e0e);
> +		ptp_read_system_postts(tai->sts);
> +		lo = __phy_read(phydev, PTPG_READPLUS_DATA);
> +		hi = __phy_read(phydev, PTPG_READPLUS_DATA);
> +	}

If the condition above is not met then err, lo, and hi may be used
uninitialised below.

Flagged by W=1 builds with clang 20.1.2, and Smatch.

> +	err = phy_restore_page(phydev, oldpage, err);
> +
> +	if (err || lo < 0 || hi < 0)
> +		return 0;
> +
> +	return lo | hi << 16;
> +}

...

> +int marvell_tai_get(struct marvell_tai **taip, struct phy_device *phydev)
> +{
> +	struct marvell_tai *tai;
> +	unsigned long overflow_ms;
> +	int err;
> +
> +	err = marvell_tai_global_config(phydev);
> +	if (err < 0)
> +		return err;
> +
> +	tai = kzalloc(sizeof(*tai), GFP_KERNEL);
> +	if (!tai)
> +		return -ENOMEM;
> +
> +	mutex_init(&tai->mutex);
> +
> +	tai->phydev = phydev;
> +
> +	/* This assumes a 125MHz clock */
> +	tai->cc_mult = 8 << 28;
> +	tai->cc_mult_num = 1 << 9;
> +	tai->cc_mult_den = 15625U;
> +
> +	tai->cyclecounter.read = marvell_tai_clock_read;
> +	tai->cyclecounter.mask = CYCLECOUNTER_MASK(32);
> +	tai->cyclecounter.mult = tai->cc_mult;
> +	tai->cyclecounter.shift = 28;
> +
> +	overflow_ms = (1ULL << 32 * tai->cc_mult * 1000) >>
> +			tai->cyclecounter.shift;
> +	tai->half_overflow_period = msecs_to_jiffies(overflow_ms / 2);
> +
> +	timecounter_init(&tai->timecounter, &tai->cyclecounter,
> +			 ktime_to_ns(ktime_get_real()));
> +
> +	tai->caps.owner = THIS_MODULE;
> +	snprintf(tai->caps.name, sizeof(tai->caps.name), "Marvell PHY");
> +	/* max_adj of 1000000 is what MV88E6xxx DSA uses */
> +	tai->caps.max_adj = 1000000;
> +	tai->caps.adjfine = marvell_tai_adjfine;
> +	tai->caps.adjtime = marvell_tai_adjtime;
> +	tai->caps.gettimex64 = marvell_tai_gettimex64;
> +	tai->caps.settime64 = marvell_tai_settime64;
> +	tai->caps.do_aux_work = marvell_tai_aux_work;
> +
> +	tai->ptp_clock = ptp_clock_register(&tai->caps, &phydev->mdio.dev);
> +	if (IS_ERR(tai->ptp_clock)) {
> +		kfree(tai);

tai is freed on the line above, but dereferenced on the line below.

Flagged by Smatch.

> +		return PTR_ERR(tai->ptp_clock);
> +	}
> +
> +	ptp_schedule_worker(tai->ptp_clock, tai->half_overflow_period);
> +
> +	spin_lock(&tai_list_lock);
> +	list_add_tail(&tai->tai_node, &tai_list);
> +	spin_unlock(&tai_list_lock);
> +
> +	*taip = tai;
> +
> +	return 0;
> +}

...

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Tue, Apr 08, 2025 at 04:49:34PM +0100, Simon Horman wrote:
> On Mon, Apr 07, 2025 at 04:03:01PM +0200, Kory Maincent wrote:
> > From: Russell King <rmk+kernel@armlinux.org.uk>
> > 
> > From: Russell King <rmk+kernel@armlinux.org.uk>
> > 
> > Add PTP basic support for Marvell 88E151x PHYs. These PHYs support
> > timestamping the egress and ingress of packets, but does not support
> > any packet modification.
> > 
> > The PHYs support hardware pins for providing an external clock for the
> > TAI counter, and a separate pin that can be used for event capture or
> > generation of a trigger (either a pulse or periodic).  This code does
> > not support either of these modes.
> > 
> > The driver takes inspiration from the Marvell 88E6xxx DSA and DP83640
> > drivers.  The hardware is very similar to the implementation found in
> > the 88E6xxx DSA driver, but the access methods are very different,
> > although it may be possible to create a library that both can use
> > along with accessor functions.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > 
> > Add support for interruption.
> > Fix L2 PTP encapsulation frame detection.
> > Fix first PTP timestamp being dropped.
> > Fix Kconfig to depends on MARVELL_PHY.
> > Update comments to use kdoc.
> > 
> > Co-developed-by: Kory Maincent <kory.maincent@bootlin.com>
> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> 
> Hi Kory,
> 
> Some minor feedback from my side.
> 
> > ---
> > 
> > Russell I don't know which email I should use, so I keep your old SOB.
> 
> Russell's SOB seems to be missing.

... and anyway, I haven't dropped my patches, I'm waiting for the
fundamental issue with merging Marvell PHY PTP support destroying the
ability to use MVPP2 PTP support to be solved, and then I will post
my patches.

They aren't dead, I'm just waiting for the issues I reported years ago
with the PTP infrastructure to be resolved - and to be tested as
resolved.

I'm still not convinced that they have been given Kory's responses to
me (some of which I honestly don't understand), but I will get around
to doing further testing to see whether enabling Marvell PHY PTP
support results in MVPP2 support becoming unusable.

Kory's lack of communication with me has been rather frustrating.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

On Tue, 8 Apr 2025 18:32:04 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Tue, Apr 08, 2025 at 04:49:34PM +0100, Simon Horman wrote:
> > On Mon, Apr 07, 2025 at 04:03:01PM +0200, Kory Maincent wrote:  
> > > From: Russell King <rmk+kernel@armlinux.org.uk>
> > > 
> > > From: Russell King <rmk+kernel@armlinux.org.uk>
> > > 
> > > Add PTP basic support for Marvell 88E151x PHYs. These PHYs support
> > > timestamping the egress and ingress of packets, but does not support
> > > any packet modification.
> > > 
> > > The PHYs support hardware pins for providing an external clock for the
> > > TAI counter, and a separate pin that can be used for event capture or
> > > generation of a trigger (either a pulse or periodic).  This code does
> > > not support either of these modes.
> > > 
> > > The driver takes inspiration from the Marvell 88E6xxx DSA and DP83640
> > > drivers.  The hardware is very similar to the implementation found in
> > > the 88E6xxx DSA driver, but the access methods are very different,
> > > although it may be possible to create a library that both can use
> > > along with accessor functions.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > 
> > > Add support for interruption.
> > > Fix L2 PTP encapsulation frame detection.
> > > Fix first PTP timestamp being dropped.
> > > Fix Kconfig to depends on MARVELL_PHY.
> > > Update comments to use kdoc.
> > > 
> > > Co-developed-by: Kory Maincent <kory.maincent@bootlin.com>
> > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>  
> > 
> > Hi Kory,
> > 
> > Some minor feedback from my side.
> >   
> > > ---
> > > 
> > > Russell I don't know which email I should use, so I keep your old SOB.  
> > 
> > Russell's SOB seems to be missing.  
> 
> ... and anyway, I haven't dropped my patches, I'm waiting for the
> fundamental issue with merging Marvell PHY PTP support destroying the
> ability to use MVPP2 PTP support to be solved, and then I will post
> my patches.
> 
> They aren't dead, I'm just waiting for the issues I reported years ago
> with the PTP infrastructure to be resolved - and to be tested as
> resolved.
> 
> I'm still not convinced that they have been given Kory's responses to
> me (some of which I honestly don't understand), but I will get around
> to doing further testing to see whether enabling Marvell PHY PTP
> support results in MVPP2 support becoming unusable.
> 
> Kory's lack of communication with me has been rather frustrating.

You were in CC in all the series I sent and there was not a lot of review and
testing on your side. I know you seemed a lot busy at that time but I don't
understand what communication is missing here? 

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Wed, Apr 09, 2025 at 10:18:08AM +0200, Kory Maincent wrote:
> On Tue, 8 Apr 2025 18:32:04 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Tue, Apr 08, 2025 at 04:49:34PM +0100, Simon Horman wrote:
> > > On Mon, Apr 07, 2025 at 04:03:01PM +0200, Kory Maincent wrote:  
> > > > From: Russell King <rmk+kernel@armlinux.org.uk>
> > > > 
> > > > From: Russell King <rmk+kernel@armlinux.org.uk>
> > > > 
> > > > Add PTP basic support for Marvell 88E151x PHYs. These PHYs support
> > > > timestamping the egress and ingress of packets, but does not support
> > > > any packet modification.
> > > > 
> > > > The PHYs support hardware pins for providing an external clock for the
> > > > TAI counter, and a separate pin that can be used for event capture or
> > > > generation of a trigger (either a pulse or periodic).  This code does
> > > > not support either of these modes.
> > > > 
> > > > The driver takes inspiration from the Marvell 88E6xxx DSA and DP83640
> > > > drivers.  The hardware is very similar to the implementation found in
> > > > the 88E6xxx DSA driver, but the access methods are very different,
> > > > although it may be possible to create a library that both can use
> > > > along with accessor functions.
> > > > 
> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > > 
> > > > Add support for interruption.
> > > > Fix L2 PTP encapsulation frame detection.
> > > > Fix first PTP timestamp being dropped.
> > > > Fix Kconfig to depends on MARVELL_PHY.
> > > > Update comments to use kdoc.
> > > > 
> > > > Co-developed-by: Kory Maincent <kory.maincent@bootlin.com>
> > > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>  
> > > 
> > > Hi Kory,
> > > 
> > > Some minor feedback from my side.
> > >   
> > > > ---
> > > > 
> > > > Russell I don't know which email I should use, so I keep your old SOB.  
> > > 
> > > Russell's SOB seems to be missing.  
> > 
> > ... and anyway, I haven't dropped my patches, I'm waiting for the
> > fundamental issue with merging Marvell PHY PTP support destroying the
> > ability to use MVPP2 PTP support to be solved, and then I will post
> > my patches.
> > 
> > They aren't dead, I'm just waiting for the issues I reported years ago
> > with the PTP infrastructure to be resolved - and to be tested as
> > resolved.
> > 
> > I'm still not convinced that they have been given Kory's responses to
> > me (some of which I honestly don't understand), but I will get around
> > to doing further testing to see whether enabling Marvell PHY PTP
> > support results in MVPP2 support becoming unusable.
> > 
> > Kory's lack of communication with me has been rather frustrating.
> 
> You were in CC in all the series I sent and there was not a lot of review and
> testing on your side. I know you seemed a lot busy at that time but I don't
> understand what communication is missing here? 

I don't spend much time at the physical location where the hardware that
I need to test your long awaited code is anymore. That means the
opportunities to test it are *rare*.

So far, each time I've tested your code, it's been broken. This really
doesn't help.

If you want me to do anything more in a timely manner, like test fixes,
you need to get them to me by the end of this week, otherwise I won't
again be able to test them for a while.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

On Wed, 9 Apr 2025 09:33:09 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Apr 09, 2025 at 10:18:08AM +0200, Kory Maincent wrote:
> > On Tue, 8 Apr 2025 18:32:04 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> >   
> > > On Tue, Apr 08, 2025 at 04:49:34PM +0100, Simon Horman wrote:  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
> > > 
> > > ... and anyway, I haven't dropped my patches, I'm waiting for the
> > > fundamental issue with merging Marvell PHY PTP support destroying the
> > > ability to use MVPP2 PTP support to be solved, and then I will post
> > > my patches.
> > > 
> > > They aren't dead, I'm just waiting for the issues I reported years ago
> > > with the PTP infrastructure to be resolved - and to be tested as
> > > resolved.
> > > 
> > > I'm still not convinced that they have been given Kory's responses to
> > > me (some of which I honestly don't understand), but I will get around
> > > to doing further testing to see whether enabling Marvell PHY PTP
> > > support results in MVPP2 support becoming unusable.
> > > 
> > > Kory's lack of communication with me has been rather frustrating.  
> > 
> > You were in CC in all the series I sent and there was not a lot of review
> > and testing on your side. I know you seemed a lot busy at that time but I
> > don't understand what communication is missing here?   
> 
> I don't spend much time at the physical location where the hardware that
> I need to test your long awaited code is anymore. That means the
> opportunities to test it are *rare*.
> 
> So far, each time I've tested your code, it's been broken. This really
> doesn't help.
> 
> If you want me to do anything more in a timely manner, like test fixes,
> you need to get them to me by the end of this week, otherwise I won't
> again be able to test them for a while.

You could try again with Vlad patch adding support to ndo_hwtstamp_get/set to
the mvpp2 drivers.
https://github.com/vladimiroltean/linux/commit/5bde95816f19cf2872367ecdbef1efe476e4f833

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Wed, Apr 09, 2025 at 10:48:58AM +0200, Kory Maincent wrote:
> On Wed, 9 Apr 2025 09:33:09 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Wed, Apr 09, 2025 at 10:18:08AM +0200, Kory Maincent wrote:
> > > On Tue, 8 Apr 2025 18:32:04 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > >   
> > > > On Tue, Apr 08, 2025 at 04:49:34PM +0100, Simon Horman wrote:  
> >  [...]  
> >  [...]  
> >  [...]  
> >  [...]  
> >  [...]  
> > > > 
> > > > ... and anyway, I haven't dropped my patches, I'm waiting for the
> > > > fundamental issue with merging Marvell PHY PTP support destroying the
> > > > ability to use MVPP2 PTP support to be solved, and then I will post
> > > > my patches.
> > > > 
> > > > They aren't dead, I'm just waiting for the issues I reported years ago
> > > > with the PTP infrastructure to be resolved - and to be tested as
> > > > resolved.
> > > > 
> > > > I'm still not convinced that they have been given Kory's responses to
> > > > me (some of which I honestly don't understand), but I will get around
> > > > to doing further testing to see whether enabling Marvell PHY PTP
> > > > support results in MVPP2 support becoming unusable.
> > > > 
> > > > Kory's lack of communication with me has been rather frustrating.  
> > > 
> > > You were in CC in all the series I sent and there was not a lot of review
> > > and testing on your side. I know you seemed a lot busy at that time but I
> > > don't understand what communication is missing here?   
> > 
> > I don't spend much time at the physical location where the hardware that
> > I need to test your long awaited code is anymore. That means the
> > opportunities to test it are *rare*.
> > 
> > So far, each time I've tested your code, it's been broken. This really
> > doesn't help.
> > 
> > If you want me to do anything more in a timely manner, like test fixes,
> > you need to get them to me by the end of this week, otherwise I won't
> > again be able to test them for a while.
> 
> You could try again with Vlad patch adding support to ndo_hwtstamp_get/set to
> the mvpp2 drivers.
> https://github.com/vladimiroltean/linux/commit/5bde95816f19cf2872367ecdbef1efe476e4f833

Well, I'm not sure PTP is working correctly.

On one machine (SolidRun Hummingboard 2), I'm running ptpd v2:

2025-04-09 11:34:19.590032 ptpd2[7284].startup (info)      (___) Configuration OK
2025-04-09 11:34:19.594624 ptpd2[7284].startup (info)      (___) Successfully acquired lock on /var/run/ptpd2.lock
2025-04-09 11:34:19.596099 ptpd2[7284].startup (notice)    (___) PTPDv2 started successfully on end0 using "masteronly" preset (PID 7284)
2025-04-09 11:34:19.596347 ptpd2[7284].startup (info)      (___) TimingService.PTP0: PTP service init
# Timestamp, State, Clock ID, One Way Delay, Offset From Master, Slave to Master, Master to Slave, Observed Drift, Last packet Received, One Way Delay Mean, One Way Delay Std Dev, Offset From Master Mean, Offset From Master Std Dev, Observed Drift Mean, Observed Drift Std Dev, raw delayMS, raw delaySM
2025-04-09 11:34:19.596685, init,
2025-04-09 11:34:19.699787 ptpd2[7284].end0 (notice)    (lstn_init) Now in state: PTP_LISTENING
2025-04-09 11:34:19.699915, lstn_init,  1
2025-04-09 11:34:29.596621 ptpd2[7284].end0 (notice)    (lstn_init) TimingService.PTP0: elected best TimingService
2025-04-09 11:34:29.596758 ptpd2[7284].end0 (info)      (lstn_init) TimingService.PTP0: acquired clock control
2025-04-09 11:34:31.701104 ptpd2[7284].end0 (notice)    (mst) Now in state: PTP_MASTER, Best master: d063b4fffe0243c3(unknown)/1 (self)
2025-04-09 11:34:31.701369, mst, d063b4fffe0243c3(unknown)/1

with this configuration:

ptpengine:interface=end0
ptpengine:preset=masteronly
ptpengine:multicast_ttl=1
clock:no_adjust=y
clock:no_reset=y

On the test machine (Macchiatobin), I'm running linuxptp ptp4l:

# ./ptp4l -i eth2 -m -s -l 7
...
ptp4l[2701.638]: master offset     -30111 s2 freq  -91915 path delay     63039
ptp4l[2701.725]: port 1: delay timeout
ptp4l[2701.726]: delay   filtered      63039   raw      40846
ptp4l[2702.253]: port 1: delay timeout
ptp4l[2702.254]: delay   filtered      63039   raw      43806
ptp4l[2702.638]: master offset      29689 s2 freq  -41148 path delay     63039
ptp4l[2703.638]: master offset     -14050 s2 freq  -75981 path delay     63039
ptp4l[2703.993]: port 1: delay timeout
ptp4l[2703.993]: delay   filtered      62371   raw      48094
ptp4l[2704.255]: port 1: delay timeout
ptp4l[2704.255]: delay   filtered      61726   raw      49767
ptp4l[2704.638]: master offset      16434 s2 freq  -49712 path delay     61726
ptp4l[2705.570]: port 1: delay timeout
ptp4l[2705.571]: delay   filtered      61726   raw      68302
ptp4l[2705.638]: master offset     -33159 s2 freq  -94374 path delay     61726
ptp4l[2706.638]: master offset      28762 s2 freq  -42401 path delay     61726
ptp4l[2707.254]: port 1: delay timeout

The "delay timeout" and random master offsets doesn't look like PTP is
working correctly.

tcpdump on the Macchiatobin shows:

13:08:34.701122 d0:63:b4:02:43:c3 > 01:00:5e:00:01:81, ethertype IPv4 (0x0800), length 86: 192.168.0.240.319 > 224.0.1.129.319: PTPv2, v1 compat : no, msg type : sync msg, length : 44, domain : 0, reserved1 : 0, Flags [two step], NS correction : 0, sub NS correction : 0, reserved2 : 0, clock identity : 0xd063b4fffe0243c3, port id : 1, seq id : 5642, control : 0 (Sync), log message interval : 0, originTimeStamp : 1744200514 seconds, 700022395 nanoseconds
13:08:34.701123 d0:63:b4:02:43:c3 > 01:00:5e:00:01:81, ethertype IPv4 (0x0800), length 86: 192.168.0.240.320 > 224.0.1.129.320: PTPv2, v1 compat : no, msg type : follow up msg, length : 44, domain : 0, reserved1 : 0, Flags [none], NS correction : 0, sub NS correction : 0, reserved2 : 0, clock identity : 0xd063b4fffe0243c3, port id : 1, seq id : 5642, control : 2 (Follow_Up), log message interval : 0, preciseOriginTimeStamp : 1744200514 seconds, 700078731 nanoseconds
13:08:35.146133 00:51:82:11:33:02 > 01:00:5e:00:01:81, ethertype IPv4 (0x0800), length 86: 192.168.1.96.319 > 224.0.1.129.319: PTPv2, v1 compat : no, msg type : delay req msg, length : 44, domain : 0, reserved1 : 0, Flags [none], NS correction : 0, sub NS correction : 0, reserved2 : 0, clock identity : 0x5182fffe113302, port id : 1, seq id : 13, control : 1 (Delay_Req), log message interval : 127, originTimeStamp : 0 seconds, 0 nanoseconds
13:08:35.146529 d0:63:b4:02:43:c3 > 01:00:5e:00:01:81, ethertype IPv4 (0x0800), length 96: 192.168.0.240.320 > 224.0.1.129.320: PTPv2, v1 compat : no, msg type : delay resp msg, length : 54, domain : 0, reserved1 : 0, Flags [none], NS correction : 0, sub NS correction : 0, reserved2 : 0, clock identity : 0xd063b4fffe0243c3, port id : 1, seq id : 13, control : 3 (Delay_Resp), log message interval : 0, receiveTimeStamp : 1744200515 seconds, 145324449 nanoseconds, port identity : 0x5182fffe113302, port id : 1
13:08:35.701268 d0:63:b4:02:43:c3 > 01:00:5e:00:01:81, ethertype IPv4 (0x0800), length 106: 192.168.0.240.320 > 224.0.1.129.320: PTPv2, v1 compat : no, msg type : announce msg, length : 64, domain : 0, reserved1 : 0, Flags [none], NS correction : 0, sub NS correction : 0, reserved2 : 0, clock identity : 0xd063b4fffe0243c3, port id : 1, seq id : 2821, control : 5 (Other), log message interval : 1, originTimeStamp : 0 seconds 0 nanoseconds, origin cur utc :0, rsvd : 130, gm priority_1 : 128, gm clock class : 13, gm clock accuracy : 254, gm clock variance : 65535, gm priority_2 : 128, gm clock id : 0xd063b4fffe0243c3, steps removed : 0, time source : 0xa0
13:08:35.701268 d0:63:b4:02:43:c3 > 01:00:5e:00:01:81, ethertype IPv4 (0x0800), length 86: 192.168.0.240.319 > 224.0.1.129.319: PTPv2, v1 compat : no, msg type : sync msg, length : 44, domain : 0, reserved1 : 0, Flags [two step], NS correction : 0, sub NS correction : 0, reserved2 : 0, clock identity : 0xd063b4fffe0243c3, port id : 1, seq id : 5643, control : 0 (Sync), log message interval : 0, originTimeStamp : 1744200515 seconds, 700230163 nanoseconds

So we can see that ptpdv2 is responding to the delay requests, but it
seems that ptp4l doesn't see them, but it is seeing the other messages
from the HB2 running in master mode. I don't have time to investigate
any further until later today, and then again not until tomorrow
evening.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

On Wed, 9 Apr 2025 13:16:12 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Apr 09, 2025 at 10:48:58AM +0200, Kory Maincent wrote:
> > On Wed, 9 Apr 2025 09:33:09 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> >   
> > > On Wed, Apr 09, 2025 at 10:18:08AM +0200, Kory Maincent wrote:  
>  [...]  
>  [...]  
> > >  [...]  
> > >  [...]  
> > >  [...]  
> > >  [...]  
> > >  [...]    
>  [...]  
>  [...]  
> > > 
> > > I don't spend much time at the physical location where the hardware that
> > > I need to test your long awaited code is anymore. That means the
> > > opportunities to test it are *rare*.
> > > 
> > > So far, each time I've tested your code, it's been broken. This really
> > > doesn't help.
> > > 
> > > If you want me to do anything more in a timely manner, like test fixes,
> > > you need to get them to me by the end of this week, otherwise I won't
> > > again be able to test them for a while.  
> > 
> > You could try again with Vlad patch adding support to ndo_hwtstamp_get/set
> > to the mvpp2 drivers.
> > https://github.com/vladimiroltean/linux/commit/5bde95816f19cf2872367ecdbef1efe476e4f833
> >  
> 
> Well, I'm not sure PTP is working correctly.
> 
> On one machine (SolidRun Hummingboard 2), I'm running ptpd v2:

...
 
> So we can see that ptpdv2 is responding to the delay requests, but it
> seems that ptp4l doesn't see them, but it is seeing the other messages
> from the HB2 running in master mode. I don't have time to investigate
> any further until later today, and then again not until tomorrow
> evening.

Ok, thanks for the tests and these information.
Did you run ptp4l with this patch applied and did you switch to Marvell PHY PTP
source?

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Wed, Apr 09, 2025 at 02:38:20PM +0200, Kory Maincent wrote:
> Ok, thanks for the tests and these information.
> Did you run ptp4l with this patch applied and did you switch to Marvell PHY PTP
> source?

This was using mvpp2, but I have my original patch as part of my kernel
rather than your patch.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

On Wed, 9 Apr 2025 14:35:17 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Apr 09, 2025 at 02:38:20PM +0200, Kory Maincent wrote:
> > Ok, thanks for the tests and these information.
> > Did you run ptp4l with this patch applied and did you switch to Marvell PHY
> > PTP source?  
> 
> This was using mvpp2, but I have my original patch as part of my kernel
> rather than your patch.

So you are only testing the mvpp2 PTP. It seems there is something broken with
it. I don't think it is related to my work.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Wed, Apr 09, 2025 at 06:04:14PM +0200, Kory Maincent wrote:
> On Wed, 9 Apr 2025 14:35:17 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Wed, Apr 09, 2025 at 02:38:20PM +0200, Kory Maincent wrote:
> > > Ok, thanks for the tests and these information.
> > > Did you run ptp4l with this patch applied and did you switch to Marvell PHY
> > > PTP source?  
> > 
> > This was using mvpp2, but I have my original patch as part of my kernel
> > rather than your patch.
> 
> So you are only testing the mvpp2 PTP. It seems there is something broken with
> it. I don't think it is related to my work.

Yes, and it has worked - but probably was never tested with PTPDv2 but
with linuxptp. As it was more than five years ago when I worked on this
stuff, I just can't remember the full details of the test setup I used.

I think the reason I gave up running PTP on my network is the problems
that having the NIC bound into a Linux bridge essentially means that
you can't participate in PTP on that machine. That basically means a
VM host machine using a bridge device for the guests can't use PTP
to time sync itself.

Well, it looks like the PHY based timestamping also isn't working -
ptp4l says its failing to timestamp transmitted packets, but having
added debug, the driver _is_ timestamping them, so the timestamps
are getting lost somewhere in the networking layer, or are too late
for ptp4l, which only waits 1ms, and the schedule_delayed_work(, 2) 
will be about 20ms at HZ=100. Increasing the wait in ptp4l to 100ms
still doesn't appear to get a timestamp. According to the timestamps
on the debug messages, it's only taking 10ms to return the timestamp.

So, at the moment, ptp looks entirely non-functional. Or the userspace
tools are broken.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Wed, Apr 09, 2025 at 06:34:35PM +0100, Russell King (Oracle) wrote:
> On Wed, Apr 09, 2025 at 06:04:14PM +0200, Kory Maincent wrote:
> > On Wed, 9 Apr 2025 14:35:17 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > 
> > > On Wed, Apr 09, 2025 at 02:38:20PM +0200, Kory Maincent wrote:
> > > > Ok, thanks for the tests and these information.
> > > > Did you run ptp4l with this patch applied and did you switch to Marvell PHY
> > > > PTP source?  
> > > 
> > > This was using mvpp2, but I have my original patch as part of my kernel
> > > rather than your patch.
> > 
> > So you are only testing the mvpp2 PTP. It seems there is something broken with
> > it. I don't think it is related to my work.
> 
> Yes, and it has worked - but probably was never tested with PTPDv2 but
> with linuxptp. As it was more than five years ago when I worked on this
> stuff, I just can't remember the full details of the test setup I used.
> 
> I think the reason I gave up running PTP on my network is the problems
> that having the NIC bound into a Linux bridge essentially means that
> you can't participate in PTP on that machine. That basically means a
> VM host machine using a bridge device for the guests can't use PTP
> to time sync itself.
> 
> Well, it looks like the PHY based timestamping also isn't working -
> ptp4l says its failing to timestamp transmitted packets, but having
> added debug, the driver _is_ timestamping them, so the timestamps
> are getting lost somewhere in the networking layer, or are too late
> for ptp4l, which only waits 1ms, and the schedule_delayed_work(, 2) 
> will be about 20ms at HZ=100. Increasing the wait in ptp4l to 100ms
> still doesn't appear to get a timestamp. According to the timestamps
> on the debug messages, it's only taking 10ms to return the timestamp.
> 
> So, at the moment, ptp looks entirely non-functional. Or the userspace
> tools are broken.

Right, got to the bottom of it at last. I hate linuxptp / ptp4l. The
idea that one looks at the source, sees this:

                res = poll(&pfd, 1, sk_tx_timeout);
                if (res < 1) {
                        pr_err(res ? "poll for tx timestamp failed: %m" :
                                     "timed out while polling for tx timestamp");
                        pr_err("increasing tx_timestamp_timeout may correct "
                               "this issue, but it is likely caused by a driver bug");

finds this in the same file:

int sk_tx_timeout = 1;

So it seemed obvious and logical that increasing that initialiser would
increase the _default_ timeout... but no, that's not the case, because,
ptp4l.c does:

        sk_tx_timeout = config_get_int(cfg, NULL, "tx_timestamp_timeout");

unconditionally, and config.c has a table of config options along with
their defaults... meaning that initialiser above for sk_tx_timeout
means absolutely nothing, and one _has_ to use a config file.

With that fixed, ptp4l's output looks very similar to that with mvpp2 -
which doesn't inspire much confidence that the ptp stack is operating
properly with the offset and frequency varying all over the place, and
the "delay timeout" messages spamming frequently. I'm also getting
ptp4l going into fault mode - so PHY PTP is proving to be way more
unreliable than mvpp2 PTP. :(

Now, the one thing I can't get rid of is the receive timestamp
overflow warning - this occurs whenever e.g. ptp4l is restarted,
and is caused by there being no notification that PTP isn't being
used anymore.

Consequently, we end up with the PHY queuing a timestamp for a Sync
packet which it sees on the network, but because nothing is wanting
the packets (because e.g. ptp4l has been stopped) there's no packets
queued into the receive queue to take this timestamp, so we stop
polling the PHY for timestamps.

If we continue to rapidly poll the PHY, then we could needlessly
waste cycles - because nothing tells us "we have no one wanting
hardware timestamps anymore" which seems to be a glaring hole in
the PTP design.

Not setting DISTSOVERWRITE seems like a solution, but that seems to
lead to issues with timestamps being lost.

Well, having spent much of the afternoon and all evening on this,
and all I see are problems that don't seem to have solutions.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Richard Cochran]

On Wed, Apr 09, 2025 at 11:38:00PM +0100, Russell King (Oracle) wrote:

> Right, got to the bottom of it at last. I hate linuxptp / ptp4l.

So don't use it.  Nobody is forcing you.

Thanks,
Richard

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Wed, Apr 09, 2025 at 09:16:19PM -0700, Richard Cochran wrote:
> On Wed, Apr 09, 2025 at 11:38:00PM +0100, Russell King (Oracle) wrote:
> 
> > Right, got to the bottom of it at last. I hate linuxptp / ptp4l.
> 
> So don't use it.  Nobody is forcing you.

What else is there to test PTP support that is better?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Richard Cochran]

On Thu, Apr 10, 2025 at 08:44:06AM +0100, Russell King (Oracle) wrote:
> What else is there to test PTP support that is better?

There is nothing else, as far as I know.  So I guess you are stuck
with ptp4l.

Sorry,
Richard

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

On Wed, 9 Apr 2025 23:38:00 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Apr 09, 2025 at 06:34:35PM +0100, Russell King (Oracle) wrote:
> > On Wed, Apr 09, 2025 at 06:04:14PM +0200, Kory Maincent wrote:  
> > > On Wed, 9 Apr 2025 14:35:17 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > >   
>  [...]  
>  [...]  
>  [...]  
> > > 
> > > So you are only testing the mvpp2 PTP. It seems there is something broken
> > > with it. I don't think it is related to my work.  
> > 
> > Yes, and it has worked - but probably was never tested with PTPDv2 but
> > with linuxptp. As it was more than five years ago when I worked on this
> > stuff, I just can't remember the full details of the test setup I used.
> > 
> > I think the reason I gave up running PTP on my network is the problems
> > that having the NIC bound into a Linux bridge essentially means that
> > you can't participate in PTP on that machine. That basically means a
> > VM host machine using a bridge device for the guests can't use PTP
> > to time sync itself.
> > 
> > Well, it looks like the PHY based timestamping also isn't working -
> > ptp4l says its failing to timestamp transmitted packets, but having
> > added debug, the driver _is_ timestamping them, so the timestamps
> > are getting lost somewhere in the networking layer, or are too late
> > for ptp4l, which only waits 1ms, and the schedule_delayed_work(, 2) 
> > will be about 20ms at HZ=100. Increasing the wait in ptp4l to 100ms
> > still doesn't appear to get a timestamp. According to the timestamps
> > on the debug messages, it's only taking 10ms to return the timestamp.
> > 
> > So, at the moment, ptp looks entirely non-functional. Or the userspace
> > tools are broken.  
> 
> Right, got to the bottom of it at last. I hate linuxptp / ptp4l. The
> idea that one looks at the source, sees this:
> 
>                 res = poll(&pfd, 1, sk_tx_timeout);
>                 if (res < 1) {
>                         pr_err(res ? "poll for tx timestamp failed: %m" :
>                                      "timed out while polling for tx
> timestamp"); pr_err("increasing tx_timestamp_timeout may correct "
>                                "this issue, but it is likely caused by a
> driver bug");
> 
> finds this in the same file:
> 
> int sk_tx_timeout = 1;
> 
> So it seemed obvious and logical that increasing that initialiser would
> increase the _default_ timeout... but no, that's not the case, because,
> ptp4l.c does:
> 
>         sk_tx_timeout = config_get_int(cfg, NULL, "tx_timestamp_timeout");
> 
> unconditionally, and config.c has a table of config options along with
> their defaults... meaning that initialiser above for sk_tx_timeout
> means absolutely nothing, and one _has_ to use a config file.
> 
> With that fixed, ptp4l's output looks very similar to that with mvpp2 -
> which doesn't inspire much confidence that the ptp stack is operating
> properly with the offset and frequency varying all over the place, and
> the "delay timeout" messages spamming frequently. I'm also getting
> ptp4l going into fault mode - so PHY PTP is proving to be way more
> unreliable than mvpp2 PTP. :(

That's really weird. On my board the Marvell PHY PTP is more reliable than MACB.
Even by disabling the interrupt.
What is the state of the driver you are using? 

On my board using the PHY PTP:
# ethtool --set-hwtimestamp-cfg eth0 index 0 qualifier precise
# ptp4l -m -i eth0 -s -2 
ptp4l[1111.786]: selected /dev/ptp0 as PTP clock
ptp4l[1111.829]: port 1 (eth0): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[1111.830]: port 0 (/var/run/ptp4l): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[1111.831]: port 0 (/var/run/ptp4lro): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[1117.896]: selected local clock 2ace59.fffe.e52d8b as best master
ptp4l[1159.456]: port 1 (eth0): new foreign master 0080e1.fffe.4253d0-1
ptp4l[1163.456]: selected best master clock 0080e1.fffe.4253d0
ptp4l[1163.456]: port 1 (eth0): LISTENING to UNCALIBRATED on RS_SLAVE
ptp4l[1165.456]: master offset 797590212440464546 s0 freq      -0 path delay      1368
ptp4l[1166.456]: master offset 797590212440460565 s1 freq   -3981 path delay      1353
ptp4l[1167.456]: master offset        -24 s2 freq   -4005 path delay      1353
ptp4l[1167.456]: port 1 (eth0): UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
ptp4l[1168.456]: master offset        -19 s2 freq   -4007 path delay      1353
ptp4l[1169.457]: master offset        821 s2 freq   -3173 path delay       528
ptp4l[1170.457]: master offset          2 s2 freq   -3745 path delay       528
ptp4l[1171.457]: master offset       -295 s2 freq   -4042 path delay       578
ptp4l[1172.457]: master offset       -262 s2 freq   -4097 path delay       578
ptp4l[1173.457]: master offset       -147 s2 freq   -4061 path delay       553
ptp4l[1174.457]: master offset        -42 s2 freq   -4000 path delay       525
ptp4l[1175.457]: master offset        -38 s2 freq   -4008 path delay       525
ptp4l[1176.457]: master offset        -10 s2 freq   -3992 path delay       522
ptp4l[1177.457]: master offset        -29 s2 freq   -4014 path delay       525
ptp4l[1178.457]: master offset        -23 s2 freq   -4017 path delay       525
ptp4l[1179.457]: master offset          0 s2 freq   -4000 path delay       522
ptp4l[1180.458]: master offset          4 s2 freq   -3996 path delay       523
ptp4l[1181.458]: master offset         -8 s2 freq   -4007 path delay       523
ptp4l[1182.458]: master offset         10 s2 freq   -3992 path delay       521
ptp4l[1183.458]: master offset          9 s2 freq   -3990 path delay       521
ptp4l[1184.458]: master offset         -7 s2 freq   -4003 path delay       523
ptp4l[1185.458]: master offset          4 s2 freq   -3994 path delay       523
ptp4l[1186.458]: master offset        -15 s2 freq   -4012 path delay       525

> Now, the one thing I can't get rid of is the receive timestamp
> overflow warning - this occurs whenever e.g. ptp4l is restarted,
> and is caused by there being no notification that PTP isn't being
> used anymore.
> 
> Consequently, we end up with the PHY queuing a timestamp for a Sync
> packet which it sees on the network, but because nothing is wanting
> the packets (because e.g. ptp4l has been stopped) there's no packets
> queued into the receive queue to take this timestamp, so we stop
> polling the PHY for timestamps.
> 
> If we continue to rapidly poll the PHY, then we could needlessly
> waste cycles - because nothing tells us "we have no one wanting
> hardware timestamps anymore" which seems to be a glaring hole in
> the PTP design.

Maybe ptp4l should disable the tx timestamp mode and the rx filter mode if it
stops. 

> Not setting DISTSOVERWRITE seems like a solution, but that seems to
> lead to issues with timestamps being lost.
> 
> Well, having spent much of the afternoon and all evening on this,
> and all I see are problems that don't seem to have solutions.

Maxime is also working on a Macchiatobin, I will ask him to test on his side.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Thu, Apr 10, 2025 at 11:17:54AM +0200, Kory Maincent wrote:
> On Wed, 9 Apr 2025 23:38:00 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > On Wed, Apr 09, 2025 at 06:34:35PM +0100, Russell King (Oracle) wrote:
> > > On Wed, Apr 09, 2025 at 06:04:14PM +0200, Kory Maincent wrote:  
> > > > On Wed, 9 Apr 2025 14:35:17 +0100
> > > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > >   
> >  [...]  
> >  [...]  
> >  [...]  
> > > > 
> > > > So you are only testing the mvpp2 PTP. It seems there is something broken
> > > > with it. I don't think it is related to my work.  
> > > 
> > > Yes, and it has worked - but probably was never tested with PTPDv2 but
> > > with linuxptp. As it was more than five years ago when I worked on this
> > > stuff, I just can't remember the full details of the test setup I used.
> > > 
> > > I think the reason I gave up running PTP on my network is the problems
> > > that having the NIC bound into a Linux bridge essentially means that
> > > you can't participate in PTP on that machine. That basically means a
> > > VM host machine using a bridge device for the guests can't use PTP
> > > to time sync itself.
> > > 
> > > Well, it looks like the PHY based timestamping also isn't working -
> > > ptp4l says its failing to timestamp transmitted packets, but having
> > > added debug, the driver _is_ timestamping them, so the timestamps
> > > are getting lost somewhere in the networking layer, or are too late
> > > for ptp4l, which only waits 1ms, and the schedule_delayed_work(, 2) 
> > > will be about 20ms at HZ=100. Increasing the wait in ptp4l to 100ms
> > > still doesn't appear to get a timestamp. According to the timestamps
> > > on the debug messages, it's only taking 10ms to return the timestamp.
> > > 
> > > So, at the moment, ptp looks entirely non-functional. Or the userspace
> > > tools are broken.  
> > 
> > Right, got to the bottom of it at last. I hate linuxptp / ptp4l. The
> > idea that one looks at the source, sees this:
> > 
> >                 res = poll(&pfd, 1, sk_tx_timeout);
> >                 if (res < 1) {
> >                         pr_err(res ? "poll for tx timestamp failed: %m" :
> >                                      "timed out while polling for tx
> > timestamp"); pr_err("increasing tx_timestamp_timeout may correct "
> >                                "this issue, but it is likely caused by a
> > driver bug");
> > 
> > finds this in the same file:
> > 
> > int sk_tx_timeout = 1;
> > 
> > So it seemed obvious and logical that increasing that initialiser would
> > increase the _default_ timeout... but no, that's not the case, because,
> > ptp4l.c does:
> > 
> >         sk_tx_timeout = config_get_int(cfg, NULL, "tx_timestamp_timeout");
> > 
> > unconditionally, and config.c has a table of config options along with
> > their defaults... meaning that initialiser above for sk_tx_timeout
> > means absolutely nothing, and one _has_ to use a config file.
> > 
> > With that fixed, ptp4l's output looks very similar to that with mvpp2 -
> > which doesn't inspire much confidence that the ptp stack is operating
> > properly with the offset and frequency varying all over the place, and
> > the "delay timeout" messages spamming frequently. I'm also getting
> > ptp4l going into fault mode - so PHY PTP is proving to be way more
> > unreliable than mvpp2 PTP. :(
> 
> That's really weird. On my board the Marvell PHY PTP is more reliable than MACB.
> Even by disabling the interrupt.
> What is the state of the driver you are using? 

Right, it seems that some of the problems were using linuxptp v3.0
rather than v4.4, which seems to work better (in that it doesn't
seem to time out and drop into fault mode.)

With v4.4, if I try:

# ./ptp4l -i eth2 -m -s -2
ptp4l[322.396]: selected /dev/ptp0 as PTP clock
ptp4l[322.453]: port 1 (eth2): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[322.454]: port 0 (/var/run/ptp4l): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[322.455]: port 0 (/var/run/ptp4lro): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[328.797]: selected local clock 005182.fffe.113302 as best master

that's all I see. If I drop the -2, then:

# ./ptp4l -i eth2 -m -s
ptp4l[405.516]: selected /dev/ptp0 as PTP clock
ptp4l[405.521]: port 1 (eth2): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[405.522]: port 0 (/var/run/ptp4l): INITIALIZING to LISTENING on INIT_COMPL
ETE
ptp4l[405.523]: port 0 (/var/run/ptp4lro): INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[405.833]: port 1 (eth2): new foreign master d063b4.fffe.0243c3-1
Marvell 88E1510 f212a200.mdio-mii:00: rx timestamp overrun (q=0 stat=0x5 seq=227)
ptp4l[405.884]: port 1 (eth2): received SYNC without timestamp
ptp4l[409.833]: selected best master clock d063b4.fffe.0243c3
ptp4l[409.834]: foreign master not using PTP timescale
ptp4l[409.834]: running in a temporal vortex
ptp4l[409.834]: port 1 (eth2): LISTENING to UNCALIBRATED on RS_SLAVE
ptp4l[410.840]: master offset   -5184050 s0 freq  +10360 path delay     55766
ptp4l[411.841]: master offset   -5255393 s1 freq  -60982 path delay     55766
ptp4l[412.840]: master offset      61793 s2 freq    +811 path delay     55766
ptp4l[412.841]: port 1 (eth2): UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
ptp4l[413.840]: master offset     -56367 s2 freq  -98811 path delay     73450
ptp4l[414.840]: master offset      62566 s2 freq   +3212 path delay     73450
ptp4l[415.840]: master offset     -18947 s2 freq  -59531 path delay     68353
ptp4l[416.840]: master offset      18277 s2 freq  -27991 path delay     62059
ptp4l[417.840]: master offset      -8628 s2 freq  -49413 path delay     62059
ptp4l[418.840]: master offset      44759 s2 freq   +1385 path delay     55766
ptp4l[419.840]: master offset     -40592 s2 freq  -70538 path delay     55766
ptp4l[420.840]: master offset      44689 s2 freq   +2565 path delay     42890
ptp4l[421.840]: master offset     -41672 s2 freq  -70389 path delay     42890
...
ptp4l[485.840]: master offset     -32192 s2 freq  -72387 path delay     47615
ptp4l[486.840]: master offset      58486 s2 freq   +8633 path delay     47615
ptp4l[487.840]: master offset     -57279 s2 freq  -89586 path delay     53535
ptp4l[488.840]: master offset      49431 s2 freq     -60 path delay     53535
ptp4l[489.840]: master offset     -55336 s2 freq  -89997 path delay     58247
ptp4l[490.840]: master offset      52156 s2 freq    +894 path delay     58247
ptp4l[491.840]: master offset     -56897 s2 freq  -92512 path delay     65986
ptp4l[492.840]: master offset      53392 s2 freq    +707 path delay     65986
ptp4l[493.840]: master offset     -35477 s2 freq  -72144 path delay     71031
ptp4l[494.840]: master offset      10634 s2 freq  -36676 path delay     71031
ptp4l[495.840]: master offset     -17451 s2 freq  -61571 path delay     71031
ptp4l[496.840]: master offset      52024 s2 freq   +2669 path delay     71031
ptp4l[497.840]: master offset     -36239 s2 freq  -69987 path delay     71031
ptp4l[498.840]: master offset      10968 s2 freq  -33652 path delay     71031
ptp4l[499.840]: master offset     -21116 s2 freq  -62445 path delay     61292
ptp4l[500.840]: master offset      56971 s2 freq   +9307 path delay     39904
ptp4l[501.840]: master offset     -29442 s2 freq  -60015 path delay     39904
ptp4l[502.840]: master offset      49644 s2 freq  +10239 path delay     37320
ptp4l[503.912]: master offset     -30912 s2 freq  -55424 path delay     37934
ptp4l[504.840]: master offset     -20782 s2 freq  -54568 path delay     41265

and from that you can see that the offset and frequency are very much
all over the place, not what you would expect from something that is
supposed to be _hardware_ timestamped - which is why I say that NTP
seems to be superior to PTP at least here.

With mvpp2, it's a very similar story:

ptp4l[628.834]: master offset      38211 s2 freq  -29874 path delay     62949
ptp4l[629.834]: master offset     -41111 s2 freq  -97733 path delay     66289
ptp4l[630.834]: master offset      33131 s2 freq  -35824 path delay     63864
ptp4l[631.834]: master offset     -55578 s2 freq -114594 path delay     63864
ptp4l[632.833]: master offset      34110 s2 freq  -41579 path delay     57582
ptp4l[633.834]: master offset     -13137 s2 freq  -78593 path delay     60047
ptp4l[634.834]: master offset      55063 s2 freq  -14334 path delay     49425
ptp4l[635.834]: master offset     -41302 s2 freq  -94180 path delay     49425
ptp4l[636.833]: master offset      11798 s2 freq  -53471 path delay     42796
ptp4l[637.834]: master offset     -31575 s2 freq  -93304 path delay     42796
ptp4l[638.833]: master offset      24722 s2 freq  -46480 path delay     46230
ptp4l[639.834]: master offset     -35568 s2 freq  -99353 path delay     52896
ptp4l[640.834]: master offset      56812 s2 freq  -17644 path delay     52896
ptp4l[641.834]: master offset     -63429 s2 freq -120841 path delay     66734
ptp4l[642.834]: master offset      56669 s2 freq  -19772 path delay     62778
ptp4l[643.834]: master offset     -31006 s2 freq  -90446 path delay     62778
ptp4l[644.834]: master offset      40576 s2 freq  -28166 path delay     54047
ptp4l[645.834]: master offset     -33082 s2 freq  -89651 path delay     54047
ptp4l[646.833]: master offset       7230 s2 freq  -59264 path delay     50476
ptp4l[647.834]: master offset     -19581 s2 freq  -83906 path delay     50476
ptp4l[648.833]: master offset      17652 s2 freq  -52547 path delay     50476
ptp4l[649.834]: master offset     -13170 s2 freq  -78073 path delay     50476
ptp4l[650.833]: master offset      18712 s2 freq  -50142 path delay     47967

Again, offset all over the place, frequency also showing that it doesn't
stabilise.

This _could_ be because of the master clock being random - but then it's
using the FEC PTP implementation with PTPD v2 - maybe either the FEC
implementation is buggy or maybe it's PTPD v2 causing this. I have no
idea how I can debug this - and I'm not going to invest in a "grand
master" PTP clock on a whim just to find out that isn't the problem.

I thought... maybe I can use the PTP implementation in a Marvell
switch as the network master, but the 88E6176 doesn't support PTP.

Maybe I can use an x86 platform... nope:

# ethtool -T enp0s25
Time stamping parameters for enp0s25:
Capabilities:
        software-transmit
        software-receive
        software-system-clock
PTP Hardware Clock: none
Hardware Transmit Timestamp Modes: none
Hardware Receive Filter Modes: none

Anyway, let's try taking a tcpdump on the x86 machine of the sync
packets and compare the deviation of the software timestamp to that
of the hardware timestamp (all deviations relative to the first
packet part seconds):

16:30:30.577298 - originTimeStamp : 1744299061 seconds, 762464622 nanoseconds
16:30:31.577270 - originTimeStamp : 1744299062 seconds, 762363987 nanoseconds
   -28us						-100.635us
16:30:32.577303 - originTimeStamp : 1744299063 seconds, 762429696 nanoseconds
   +85us						-34.926us
16:30:33.577236 - originTimeStamp : 1744299064 seconds, 762328728 nanoseconds
   -62us						-135.894us
16:30:34.577280 - originTimeStamp : 1744299065 seconds, 762398770 nanoseconds
   -18us						-65.852us

We can see here that the timestamp from the software receive is far
more regular than the origin timestamp in the packets, which, in
combination with the randomness of both mvpp2 and the 88e151x PTP
trying to sync with it, makes me question whether there is something
fundamentally wrong with the FEC PTP implementation / PTPDv2.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

On Thu, 10 Apr 2025 16:41:06 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Thu, Apr 10, 2025 at 11:17:54AM +0200, Kory Maincent wrote:
> > On Wed, 9 Apr 2025 23:38:00 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:  
> > > On Wed, Apr 09, 2025 at 06:34:35PM +0100, Russell King (Oracle) wrote:  

> > > 
> > > With that fixed, ptp4l's output looks very similar to that with mvpp2 -
> > > which doesn't inspire much confidence that the ptp stack is operating
> > > properly with the offset and frequency varying all over the place, and
> > > the "delay timeout" messages spamming frequently. I'm also getting
> > > ptp4l going into fault mode - so PHY PTP is proving to be way more
> > > unreliable than mvpp2 PTP. :(  
> > 
> > That's really weird. On my board the Marvell PHY PTP is more reliable than
> > MACB. Even by disabling the interrupt.
> > What is the state of the driver you are using?   
> 
> Right, it seems that some of the problems were using linuxptp v3.0
> rather than v4.4, which seems to work better (in that it doesn't
> seem to time out and drop into fault mode.)
> 
> With v4.4, if I try:
> 
> # ./ptp4l -i eth2 -m -s -2
> ptp4l[322.396]: selected /dev/ptp0 as PTP clock
> ptp4l[322.453]: port 1 (eth2): INITIALIZING to LISTENING on INIT_COMPLETE
> ptp4l[322.454]: port 0 (/var/run/ptp4l): INITIALIZING to LISTENING on
> INIT_COMPLETE ptp4l[322.455]: port 0 (/var/run/ptp4lro): INITIALIZING to
> LISTENING on INIT_COMPLETE ptp4l[328.797]: selected local clock
> 005182.fffe.113302 as best master
> 
> that's all I see. If I drop the -2, then:

It seems you are still using your Marvell PHY drivers without my change.
PTP L2 was broken on your first patch and I fixed it.
I have the same result without the -2 which mean ptp4l uses UDP IPV4.
 
> and from that you can see that the offset and frequency are very much
> all over the place, not what you would expect from something that is
> supposed to be _hardware_ timestamped - which is why I say that NTP
> seems to be superior to PTP at least here.
> 
> With mvpp2, it's a very similar story:

> ptp4l[628.834]: master offset      38211 s2 freq  -29874 path delay     62949
> ptp4l[629.834]: master offset     -41111 s2 freq  -97733 path delay     66289
> ptp4l[630.834]: master offset      33131 s2 freq  -35824 path delay     63864
> ptp4l[631.834]: master offset     -55578 s2 freq -114594 path delay     63864
> ptp4l[632.833]: master offset      34110 s2 freq  -41579 path delay     57582
> ptp4l[633.834]: master offset     -13137 s2 freq  -78593 path delay     60047
> ptp4l[634.834]: master offset      55063 s2 freq  -14334 path delay     49425
> ptp4l[635.834]: master offset     -41302 s2 freq  -94180 path delay     49425

I can't tell about mvpp2 as I don't have board with this MAC but these values
seem really high and vary a lot. As this behavior is similar between the Marvell
PHY or the mvpp2 MAC maybe the issue comes indeed from your link partner. 

> Again, offset all over the place, frequency also showing that it doesn't
> stabilise.
> 
> This _could_ be because of the master clock being random - but then it's
> using the FEC PTP implementation with PTPD v2 - maybe either the FEC
> implementation is buggy or maybe it's PTPD v2 causing this. I have no
> idea how I can debug this - and I'm not going to invest in a "grand
> master" PTP clock on a whim just to find out that isn't the problem.
> 
> I thought... maybe I can use the PTP implementation in a Marvell
> switch as the network master, but the 88E6176 doesn't support PTP.
> 
> Maybe I can use an x86 platform... nope:
> 
> # ethtool -T enp0s25
> Time stamping parameters for enp0s25:
> Capabilities:
>         software-transmit
>         software-receive
>         software-system-clock

Still you could try with timestamping from software on the link partner.
On my side I am using a STM32MP157-DK as link partner.

If I set the DK board as PTP master and tell it to use software PTP (-S
parameter) it is still more reliable than yours.
ptp4l[4419.134]: master offset        136 s2 freq   -1984 path delay    118390
ptp4l[4420.134]: master offset       1757 s2 freq    -322 path delay    115888
ptp4l[4421.134]: master offset      -1154 s2 freq   -2706 path delay    115888
ptp4l[4422.134]: master offset      -1652 s2 freq   -3551 path delay    115888
ptp4l[4423.134]: master offset      -1199 s2 freq   -3593 path delay    115252

> PTP Hardware Clock: none
> Hardware Transmit Timestamp Modes: none
> Hardware Receive Filter Modes: none
> 
> Anyway, let's try taking a tcpdump on the x86 machine of the sync
> packets and compare the deviation of the software timestamp to that
> of the hardware timestamp (all deviations relative to the first
> packet part seconds):
> 
> 16:30:30.577298 - originTimeStamp : 1744299061 seconds, 762464622 nanoseconds
> 16:30:31.577270 - originTimeStamp : 1744299062 seconds, 762363987 nanoseconds
>    -28us						-100.635us
> 16:30:32.577303 - originTimeStamp : 1744299063 seconds, 762429696 nanoseconds
>    +85us						-34.926us
> 16:30:33.577236 - originTimeStamp : 1744299064 seconds, 762328728 nanoseconds
>    -62us						-135.894us
> 16:30:34.577280 - originTimeStamp : 1744299065 seconds, 762398770 nanoseconds
>    -18us						-65.852us
> 
> We can see here that the timestamp from the software receive is far
> more regular than the origin timestamp in the packets, which, in
> combination with the randomness of both mvpp2 and the 88e151x PTP
> trying to sync with it, makes me question whether there is something
> fundamentally wrong with the FEC PTP implementation / PTPDv2.

So we come to the same conclusion, the issue comes from your link partner! ;)

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Thu, Apr 10, 2025 at 06:02:05PM +0200, Kory Maincent wrote:
> On Thu, 10 Apr 2025 16:41:06 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Thu, Apr 10, 2025 at 11:17:54AM +0200, Kory Maincent wrote:
> > > On Wed, 9 Apr 2025 23:38:00 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:  
> > > > On Wed, Apr 09, 2025 at 06:34:35PM +0100, Russell King (Oracle) wrote:  
> 
> > > > 
> > > > With that fixed, ptp4l's output looks very similar to that with mvpp2 -
> > > > which doesn't inspire much confidence that the ptp stack is operating
> > > > properly with the offset and frequency varying all over the place, and
> > > > the "delay timeout" messages spamming frequently. I'm also getting
> > > > ptp4l going into fault mode - so PHY PTP is proving to be way more
> > > > unreliable than mvpp2 PTP. :(  
> > > 
> > > That's really weird. On my board the Marvell PHY PTP is more reliable than
> > > MACB. Even by disabling the interrupt.
> > > What is the state of the driver you are using?   
> > 
> > Right, it seems that some of the problems were using linuxptp v3.0
> > rather than v4.4, which seems to work better (in that it doesn't
> > seem to time out and drop into fault mode.)
> > 
> > With v4.4, if I try:
> > 
> > # ./ptp4l -i eth2 -m -s -2
> > ptp4l[322.396]: selected /dev/ptp0 as PTP clock
> > ptp4l[322.453]: port 1 (eth2): INITIALIZING to LISTENING on INIT_COMPLETE
> > ptp4l[322.454]: port 0 (/var/run/ptp4l): INITIALIZING to LISTENING on
> > INIT_COMPLETE ptp4l[322.455]: port 0 (/var/run/ptp4lro): INITIALIZING to
> > LISTENING on INIT_COMPLETE ptp4l[328.797]: selected local clock
> > 005182.fffe.113302 as best master
> > 
> > that's all I see. If I drop the -2, then:
> 
> It seems you are still using your Marvell PHY drivers without my change.
> PTP L2 was broken on your first patch and I fixed it.
> I have the same result without the -2 which mean ptp4l uses UDP IPV4.

I'm not sure what you're referring to.

There isn't any change for the packet offsets in your patch in
https://termbin.com/gzei

You added configuration of the PTP global config 1 register, which
is already in my patches - I was writing ~0 to that, you were writing
3. This only changes which PTP MSGID values get timestamped. I've been
trying your value of 3 here just in case it was significant.

You changed to use ptp_parse_header() (which didn't exist at the time
you took my patch) and I had already updated my patches to use when
this new helper was introduced.

The overflow_ns change you made in https://termbin.com/6a18 doesn't
apply to my code, because my code became:

        overflow_ns = BIT_ULL(32) * param->cc_mult;
        overflow_ns >>= param->cc_shift;
        tai->half_overflow_period = nsecs_to_jiffies64(overflow_ns / 2);

with overflow_ns being a u64.

I think that covers everything that has a functional change in terms
of packet parsing either by the driver or by the hardware, so I'm not
sure what problem you are referring to, or what your fix for it is -
maybe it's not in either of these two patches?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Thu, Apr 10, 2025 at 07:16:47PM +0100, Russell King (Oracle) wrote:
> On Thu, Apr 10, 2025 at 06:02:05PM +0200, Kory Maincent wrote:
> > On Thu, 10 Apr 2025 16:41:06 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > 
> > > On Thu, Apr 10, 2025 at 11:17:54AM +0200, Kory Maincent wrote:
> > > > On Wed, 9 Apr 2025 23:38:00 +0100
> > > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:  
> > > > > On Wed, Apr 09, 2025 at 06:34:35PM +0100, Russell King (Oracle) wrote:  
> > 
> > > > > 
> > > > > With that fixed, ptp4l's output looks very similar to that with mvpp2 -
> > > > > which doesn't inspire much confidence that the ptp stack is operating
> > > > > properly with the offset and frequency varying all over the place, and
> > > > > the "delay timeout" messages spamming frequently. I'm also getting
> > > > > ptp4l going into fault mode - so PHY PTP is proving to be way more
> > > > > unreliable than mvpp2 PTP. :(  
> > > > 
> > > > That's really weird. On my board the Marvell PHY PTP is more reliable than
> > > > MACB. Even by disabling the interrupt.
> > > > What is the state of the driver you are using?   
> > > 
> > > Right, it seems that some of the problems were using linuxptp v3.0
> > > rather than v4.4, which seems to work better (in that it doesn't
> > > seem to time out and drop into fault mode.)
> > > 
> > > With v4.4, if I try:
> > > 
> > > # ./ptp4l -i eth2 -m -s -2
> > > ptp4l[322.396]: selected /dev/ptp0 as PTP clock
> > > ptp4l[322.453]: port 1 (eth2): INITIALIZING to LISTENING on INIT_COMPLETE
> > > ptp4l[322.454]: port 0 (/var/run/ptp4l): INITIALIZING to LISTENING on
> > > INIT_COMPLETE ptp4l[322.455]: port 0 (/var/run/ptp4lro): INITIALIZING to
> > > LISTENING on INIT_COMPLETE ptp4l[328.797]: selected local clock
> > > 005182.fffe.113302 as best master
> > > 
> > > that's all I see. If I drop the -2, then:
> > 
> > It seems you are still using your Marvell PHY drivers without my change.
> > PTP L2 was broken on your first patch and I fixed it.
> > I have the same result without the -2 which mean ptp4l uses UDP IPV4.
> 
> I'm not sure what you're referring to.

Okay, turns out to be nothing to do with any fixes in my code or not
(even though I still don't know what the claimed brokenness you
refer to actually was.)

It turns out to be that ptpdv2 sends PTP packets using IPv4 UDP *or*
L2, and was using IPv4 UDP. Adding "ptpengine:transport=ethernet" to
the ptpdv2 configuration allows ptp4l -2 to then work.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

On Thu, 10 Apr 2025 20:40:12 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Thu, Apr 10, 2025 at 07:16:47PM +0100, Russell King (Oracle) wrote:
> > On Thu, Apr 10, 2025 at 06:02:05PM +0200, Kory Maincent wrote:  
> > > On Thu, 10 Apr 2025 16:41:06 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> > > It seems you are still using your Marvell PHY drivers without my change.
> > > PTP L2 was broken on your first patch and I fixed it.
> > > I have the same result without the -2 which mean ptp4l uses UDP IPV4.  
> > 
> > I'm not sure what you're referring to.  
> 
> Okay, turns out to be nothing to do with any fixes in my code or not
> (even though I still don't know what the claimed brokenness you
> refer to actually was.)

If I remember well you need the PTP global config 1 register set to 3 to have
the L2 PTP working.

> It turns out to be that ptpdv2 sends PTP packets using IPv4 UDP *or*
> L2, and was using IPv4 UDP. Adding "ptpengine:transport=ethernet" to
> the ptpdv2 configuration allows ptp4l -2 to then work.

Ack.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Fri, Apr 11, 2025 at 10:01:27AM +0200, Kory Maincent wrote:
> On Thu, 10 Apr 2025 20:40:12 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Thu, Apr 10, 2025 at 07:16:47PM +0100, Russell King (Oracle) wrote:
> > > On Thu, Apr 10, 2025 at 06:02:05PM +0200, Kory Maincent wrote:  
> > > > On Thu, 10 Apr 2025 16:41:06 +0100
> > > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > > > It seems you are still using your Marvell PHY drivers without my change.
> > > > PTP L2 was broken on your first patch and I fixed it.
> > > > I have the same result without the -2 which mean ptp4l uses UDP IPV4.  
> > > 
> > > I'm not sure what you're referring to.  
> > 
> > Okay, turns out to be nothing to do with any fixes in my code or not
> > (even though I still don't know what the claimed brokenness you
> > refer to actually was.)
> 
> If I remember well you need the PTP global config 1 register set to 3 to have
> the L2 PTP working.

The PTP global config 1 register determines which message IDs get
timestamped, both for incoming and outgoing messages.

Setting it to 0x3 means that only Sync and Delay_Req messages only
get stamped, irrespective of whether userspace wants to stamp
messages in the transmit path with other message IDs.

With it set to ~0 as I have it, this means all PTP messages are
candidates for being stamped.

As this is a global register, which can be shared between ports (not
for 1510, but may be for other PHYs or DSA), and we have no idea which
messages will need to be stamped, setting it to ~0 is sensible, and
it's also what would be expected of the PTP layers, because we report
back to userspace HWTSTAMP_FILTER_SOME which means "return value: time
stamp all packets requested plus some others" (from the documentation).

I'm currently using 0x0203 (sync, delay_req, delay_resp) and it's
working mostly fine, although I do from time to time see rx overruns
and sometimes tx timestamps missed. I'm trying to fix that before I
post updated patches.

It should be fine with other values too, and should have no effect
whether L2 and L4 are used. DSA sets this to 0x0f, which uses the
same hardware, and I assume is well tested:

        /* MV88E6XXX_PTP_MSG_TYPE is a mask of PTP message types to
         * timestamp. This affects all ports that have timestamping enabled,
         * but the timestamp config is per-port; thus we configure all events
         * here and only support the HWTSTAMP_FILTER_*_EVENT filter types.
         */
        err = mv88e6xxx_ptp_write(chip, MV88E6XXX_PTP_MSGTYPE,
                                  MV88E6XXX_PTP_MSGTYPE_ALL_EVENT);

#define MV88E6XXX_PTP_MSGTYPE_ALL_EVENT         0x000f

with peer delay response messages directed to the second arrival
timestamp registers:

        /* Use ARRIVAL1 for peer delay response messages. */
        err = mv88e6xxx_ptp_write(chip, MV88E6XXX_PTP_TS_ARRIVAL_PTR,
                                  MV88E6XXX_PTP_MSGTYPE_PDLAY_RES);

#define MV88E6XXX_PTP_MSGTYPE_PDLAY_RES         0x0008

Please re-test with other values and report how it doesn't work.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

On Tue, 8 Apr 2025 16:49:34 +0100
Simon Horman <horms@kernel.org> wrote:

> On Mon, Apr 07, 2025 at 04:03:01PM +0200, Kory Maincent wrote:
> > From: Russell King <rmk+kernel@armlinux.org.uk>
> > 
> > From: Russell King <rmk+kernel@armlinux.org.uk>
> > 
> > Add PTP basic support for Marvell 88E151x PHYs. These PHYs support
> > timestamping the egress and ingress of packets, but does not support
> > any packet modification.
> > 
> > The PHYs support hardware pins for providing an external clock for the
> > TAI counter, and a separate pin that can be used for event capture or
> > generation of a trigger (either a pulse or periodic).  This code does
> > not support either of these modes.
> > 
> > The driver takes inspiration from the Marvell 88E6xxx DSA and DP83640
> > drivers.  The hardware is very similar to the implementation found in
> > the 88E6xxx DSA driver, but the access methods are very different,
> > although it may be possible to create a library that both can use
> > along with accessor functions.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > 
> > Add support for interruption.
> > Fix L2 PTP encapsulation frame detection.
> > Fix first PTP timestamp being dropped.
> > Fix Kconfig to depends on MARVELL_PHY.
> > Update comments to use kdoc.
> > 
> > Co-developed-by: Kory Maincent <kory.maincent@bootlin.com>
> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>  
> 
> Hi Kory,
> 
> Some minor feedback from my side.
> 
> > ---
> > 
> > Russell I don't know which email I should use, so I keep your old SOB.  
> 
> Russell's SOB seems to be missing.

It is, 5 lines higher, but maybe you prefer to have them all together. 

> 
> ...
> 
> > diff --git a/drivers/net/phy/marvell/marvell_tai.c
> > b/drivers/net/phy/marvell/marvell_tai.c  
> 
> ...
> 
> > +/* Read the global time registers using the readplus command */
> > +static u64 marvell_tai_clock_read(const struct cyclecounter *cc)
> > +{
> > +	struct marvell_tai *tai = cc_to_tai(cc);
> > +	struct phy_device *phydev = tai->phydev;
> > +	int err, oldpage, lo, hi;
> > +
> > +	oldpage = phy_select_page(phydev, MARVELL_PAGE_PTP_GLOBAL);
> > +	if (oldpage >= 0) {
> > +		/* 88e151x says to write 0x8e0e */
> > +		ptp_read_system_prets(tai->sts);
> > +		err = __phy_write(phydev, PTPG_READPLUS_COMMAND, 0x8e0e);
> > +		ptp_read_system_postts(tai->sts);
> > +		lo = __phy_read(phydev, PTPG_READPLUS_DATA);
> > +		hi = __phy_read(phydev, PTPG_READPLUS_DATA);
> > +	}  
> 
> If the condition above is not met then err, lo, and hi may be used
> uninitialised below.
> 
> Flagged by W=1 builds with clang 20.1.2, and Smatch.

Indeed thanks!

> > +	tai->caps.max_adj = 1000000;
> > +	tai->caps.adjfine = marvell_tai_adjfine;
> > +	tai->caps.adjtime = marvell_tai_adjtime;
> > +	tai->caps.gettimex64 = marvell_tai_gettimex64;
> > +	tai->caps.settime64 = marvell_tai_settime64;
> > +	tai->caps.do_aux_work = marvell_tai_aux_work;
> > +
> > +	tai->ptp_clock = ptp_clock_register(&tai->caps, &phydev->mdio.dev);
> > +	if (IS_ERR(tai->ptp_clock)) {
> > +		kfree(tai);  
> 
> tai is freed on the line above, but dereferenced on the line below.
> 
> Flagged by Smatch.

Indeed thanks!

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Simon Horman]

On Wed, Apr 09, 2025 at 10:07:57AM +0200, Kory Maincent wrote:
> On Tue, 8 Apr 2025 16:49:34 +0100
> Simon Horman <horms@kernel.org> wrote:
> 
> > On Mon, Apr 07, 2025 at 04:03:01PM +0200, Kory Maincent wrote:
> > > From: Russell King <rmk+kernel@armlinux.org.uk>
> > > 
> > > From: Russell King <rmk+kernel@armlinux.org.uk>
> > > 
> > > Add PTP basic support for Marvell 88E151x PHYs. These PHYs support
> > > timestamping the egress and ingress of packets, but does not support
> > > any packet modification.
> > > 
> > > The PHYs support hardware pins for providing an external clock for the
> > > TAI counter, and a separate pin that can be used for event capture or
> > > generation of a trigger (either a pulse or periodic).  This code does
> > > not support either of these modes.
> > > 
> > > The driver takes inspiration from the Marvell 88E6xxx DSA and DP83640
> > > drivers.  The hardware is very similar to the implementation found in
> > > the 88E6xxx DSA driver, but the access methods are very different,
> > > although it may be possible to create a library that both can use
> > > along with accessor functions.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > 
> > > Add support for interruption.
> > > Fix L2 PTP encapsulation frame detection.
> > > Fix first PTP timestamp being dropped.
> > > Fix Kconfig to depends on MARVELL_PHY.
> > > Update comments to use kdoc.
> > > 
> > > Co-developed-by: Kory Maincent <kory.maincent@bootlin.com>
> > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>  
> > 
> > Hi Kory,
> > 
> > Some minor feedback from my side.
> > 
> > > ---
> > > 
> > > Russell I don't know which email I should use, so I keep your old SOB.  
> > 
> > Russell's SOB seems to be missing.
> 
> It is, 5 lines higher, but maybe you prefer to have them all together. 

This thread seems to have subsequently gone elsewhere.
But, FTR, yes, I would prefer all tags together.

...

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Mon, Apr 07, 2025 at 04:03:01PM +0200, Kory Maincent wrote:
> From: Russell King <rmk+kernel@armlinux.org.uk>
> 
> From: Russell King <rmk+kernel@armlinux.org.uk>
> 
> Add PTP basic support for Marvell 88E151x PHYs. These PHYs support
> timestamping the egress and ingress of packets, but does not support
> any packet modification.
> 
> The PHYs support hardware pins for providing an external clock for the
> TAI counter, and a separate pin that can be used for event capture or
> generation of a trigger (either a pulse or periodic).  This code does
> not support either of these modes.
> 
> The driver takes inspiration from the Marvell 88E6xxx DSA and DP83640
> drivers.  The hardware is very similar to the implementation found in
> the 88E6xxx DSA driver, but the access methods are very different,
> although it may be possible to create a library that both can use
> along with accessor functions.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> Add support for interruption.
> Fix L2 PTP encapsulation frame detection.
> Fix first PTP timestamp being dropped.
> Fix Kconfig to depends on MARVELL_PHY.
> Update comments to use kdoc.

Would you mind forwarding me the changes you actually made so I can
integrate them into the version I have (which is structured quite
differently from - what I assume - is a much older version of my
patches please?

The PTP IP is re-used not only in Marvell PHY drivers but also their
DSA drivers, and having it all in drivers/net/phy/ as this version
has does not make sense.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Kory Maincent]

On Wed, 9 Apr 2025 16:34:36 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Mon, Apr 07, 2025 at 04:03:01PM +0200, Kory Maincent wrote:
> > From: Russell King <rmk+kernel@armlinux.org.uk>
> > 
> > From: Russell King <rmk+kernel@armlinux.org.uk>
> > 
> > Add PTP basic support for Marvell 88E151x PHYs. These PHYs support
> > timestamping the egress and ingress of packets, but does not support
> > any packet modification.
> > 
> > The PHYs support hardware pins for providing an external clock for the
> > TAI counter, and a separate pin that can be used for event capture or
> > generation of a trigger (either a pulse or periodic).  This code does
> > not support either of these modes.
> > 
> > The driver takes inspiration from the Marvell 88E6xxx DSA and DP83640
> > drivers.  The hardware is very similar to the implementation found in
> > the 88E6xxx DSA driver, but the access methods are very different,
> > although it may be possible to create a library that both can use
> > along with accessor functions.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > 
> > Add support for interruption.
> > Fix L2 PTP encapsulation frame detection.
> > Fix first PTP timestamp being dropped.
> > Fix Kconfig to depends on MARVELL_PHY.
> > Update comments to use kdoc.  
> 
> Would you mind forwarding me the changes you actually made so I can
> integrate them into the version I have (which is structured quite
> differently from - what I assume - is a much older version of my
> patches please?

I don't know how you want this but here is a diff with your first patch sent
mainline:
https://termbin.com/gzei
There is also a small fix in the tai part:
https://termbin.com/6a18

> The PTP IP is re-used not only in Marvell PHY drivers but also their
> DSA drivers, and having it all in drivers/net/phy/ as this version
> has does not make sense.

Ok, didn't know that.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Andrew Lunn]

On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:
> Add PTP basic support for Marvell 88E151x PHYs.

Russell has repeatedly said this will cause regressions in some setups
where there are now two PTP implementations, and the wrong one will be
chosen by default. I would expect some comments in the commit message
explaining how this has been addressed, so it is clear a regression
will not happen.

     Andrew

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Kory Maincent]

On Mon, 7 Apr 2025 16:08:04 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:
> > Add PTP basic support for Marvell 88E151x PHYs.  
> 
> Russell has repeatedly said this will cause regressions in some setups
> where there are now two PTP implementations, and the wrong one will be
> chosen by default. I would expect some comments in the commit message
> explaining how this has been addressed, so it is clear a regression
> will not happen.

This was fixed by the following patch series which have parts that get merged
along the way to version 21. It adds support to select the hardware PTP
provider and change the default to MAC PTP for newly introduced PHY PTP support
(default_timestamp flag in phy_device struct).
https://lore.kernel.org/netdev/20241212-feature_ptp_netnext-v21-0-2c282a941518@bootlin.com/

I will add the description in v3. I will wait at least one week to let people
review the patch.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Russell King (Oracle)]

On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:
> Add PTP basic support for Marvell 88E151x PHYs.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Is the PTP selection stuff actually sorted now? Last time I tested it
after it having been merged into the kernel for a while, it didn't work,
and I reported that fact. You haven't told me that you now expect it to
work.

I don't want this merged until such time that we can be sure that MVPP2
platforms can continue using the MVPP2 PTP support, which to me means
that the PTP selection between a MAC and PHY needs to work.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Kory Maincent]

On Mon, 7 Apr 2025 17:02:28 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:
> > Add PTP basic support for Marvell 88E151x PHYs.
> > 
> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>  
> 
> Is the PTP selection stuff actually sorted now? Last time I tested it
> after it having been merged into the kernel for a while, it didn't work,
> and I reported that fact. You haven't told me that you now expect it to
> work.

The last part of the series, the PTP selection support wasn't merged when you
tested it, although the default PTP choice that causes your regression was
merged.
Now it is fully merged, even the ethtool support.
https://lore.kernel.org/netdev/mjn6eeo6lestvo6z3utb7aemufmfhn5alecyoaz46dt4pwjn6v@4aaaz6qpqd4b/

The only issue is the rtln warning from the phy_detach function. About it, I
have already sent you the work I have done throwing ASSERT_RTNL in phy_detach.
Maybe I should resend it as RFC.

> I don't want this merged until such time that we can be sure that MVPP2
> platforms can continue using the MVPP2 PTP support, which to me means
> that the PTP selection between a MAC and PHY needs to work.

It should works, the default PTP will be the MAC PTP and you will be able to
select the current PTP between MAC and PHY with the following command:
# ethtool --set-hwtimestamp-cfg eth0 index 0 qualifier precise
Time stamping configuration for eth0:
Hardware timestamp provider index: 0
Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
Hardware Transmit Timestamp Mode:
	off
Hardware Receive Filter Mode:
	none
Hardware Flags: none
# ethtool --set-hwtimestamp-cfg eth0 index 1 qualifier precise
Time stamping configuration for eth0:
Hardware timestamp provider index: 1
Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
Hardware Transmit Timestamp Mode:
	off
Hardware Receive Filter Mode:
	none
Hardware Flags: none

You can list the PTPs with the dump command:
# ethtool --show-time-stamping "*"

You will need to stop phc2sys and ptp4l during these change as linuxptp may
face some issues during the PTP change.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Russell King (Oracle)]

On Mon, Apr 07, 2025 at 06:20:28PM +0200, Kory Maincent wrote:
> On Mon, 7 Apr 2025 17:02:28 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:
> > > Add PTP basic support for Marvell 88E151x PHYs.
> > > 
> > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>  
> > 
> > Is the PTP selection stuff actually sorted now? Last time I tested it
> > after it having been merged into the kernel for a while, it didn't work,
> > and I reported that fact. You haven't told me that you now expect it to
> > work.
> 
> The last part of the series, the PTP selection support wasn't merged when you
> tested it, although the default PTP choice that causes your regression was
> merged.
> Now it is fully merged, even the ethtool support.
> https://lore.kernel.org/netdev/mjn6eeo6lestvo6z3utb7aemufmfhn5alecyoaz46dt4pwjn6v@4aaaz6qpqd4b/
> 
> The only issue is the rtln warning from the phy_detach function. About it, I
> have already sent you the work I have done throwing ASSERT_RTNL in phy_detach.
> Maybe I should resend it as RFC.
> 
> > I don't want this merged until such time that we can be sure that MVPP2
> > platforms can continue using the MVPP2 PTP support, which to me means
> > that the PTP selection between a MAC and PHY needs to work.
> 
> It should works, the default PTP will be the MAC PTP and you will be able to
> select the current PTP between MAC and PHY with the following command:
> # ethtool --set-hwtimestamp-cfg eth0 index 0 qualifier precise
> Time stamping configuration for eth0:
> Hardware timestamp provider index: 0
> Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> Hardware Transmit Timestamp Mode:
> 	off
> Hardware Receive Filter Mode:
> 	none
> Hardware Flags: none
> # ethtool --set-hwtimestamp-cfg eth0 index 1 qualifier precise
> Time stamping configuration for eth0:
> Hardware timestamp provider index: 1
> Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> Hardware Transmit Timestamp Mode:
> 	off
> Hardware Receive Filter Mode:
> 	none
> Hardware Flags: none
> 
> You can list the PTPs with the dump command:
> # ethtool --show-time-stamping "*"
> 
> You will need to stop phc2sys and ptp4l during these change as linuxptp may
> face some issues during the PTP change.

I'm preferring to my emails in connection with:

https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk

when I tested your work last time, it seemed that what was merged hadn't
even been tested. In the last email, you said you'd look into it, but I
didn't hear anything further. Have the problems I reported been
addressed?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Kory Maincent]

On Mon, 7 Apr 2025 17:32:43 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Mon, Apr 07, 2025 at 06:20:28PM +0200, Kory Maincent wrote:
> > On Mon, 7 Apr 2025 17:02:28 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> >   
> > > On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:  
>  [...]  
> > > 
> > > Is the PTP selection stuff actually sorted now? Last time I tested it
> > > after it having been merged into the kernel for a while, it didn't work,
> > > and I reported that fact. You haven't told me that you now expect it to
> > > work.  
> > 
> > The last part of the series, the PTP selection support wasn't merged when
> > you tested it, although the default PTP choice that causes your regression
> > was merged.
> > Now it is fully merged, even the ethtool support.
> > https://lore.kernel.org/netdev/mjn6eeo6lestvo6z3utb7aemufmfhn5alecyoaz46dt4pwjn6v@4aaaz6qpqd4b/
> > 
> > The only issue is the rtln warning from the phy_detach function. About it, I
> > have already sent you the work I have done throwing ASSERT_RTNL in
> > phy_detach. Maybe I should resend it as RFC.
> >   
> > > I don't want this merged until such time that we can be sure that MVPP2
> > > platforms can continue using the MVPP2 PTP support, which to me means
> > > that the PTP selection between a MAC and PHY needs to work.  
> > 
> > It should works, the default PTP will be the MAC PTP and you will be able to
> > select the current PTP between MAC and PHY with the following command:
> > # ethtool --set-hwtimestamp-cfg eth0 index 0 qualifier precise
> > Time stamping configuration for eth0:
> > Hardware timestamp provider index: 0
> > Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> > Hardware Transmit Timestamp Mode:
> > 	off
> > Hardware Receive Filter Mode:
> > 	none
> > Hardware Flags: none
> > # ethtool --set-hwtimestamp-cfg eth0 index 1 qualifier precise
> > Time stamping configuration for eth0:
> > Hardware timestamp provider index: 1
> > Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> > Hardware Transmit Timestamp Mode:
> > 	off
> > Hardware Receive Filter Mode:
> > 	none
> > Hardware Flags: none
> > 
> > You can list the PTPs with the dump command:
> > # ethtool --show-time-stamping "*"
> > 
> > You will need to stop phc2sys and ptp4l during these change as linuxptp may
> > face some issues during the PTP change.  
> 
> I'm preferring to my emails in connection with:
> 
> https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> 
> when I tested your work last time, it seemed that what was merged hadn't
> even been tested. In the last email, you said you'd look into it, but I
> didn't hear anything further. Have the problems I reported been
> addressed?

It wasn't merged it was 19th version and it worked and was tested, but not
with the best development design. I have replied to you that I will do some
change in v20 to address this.
https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/

It gets finally merged in v21.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Russell King (Oracle)]

On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:
> On Mon, 7 Apr 2025 17:32:43 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Mon, Apr 07, 2025 at 06:20:28PM +0200, Kory Maincent wrote:
> > > On Mon, 7 Apr 2025 17:02:28 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > >   
> > > > On Mon, Apr 07, 2025 at 04:02:59PM +0200, Kory Maincent wrote:  
> >  [...]  
> > > > 
> > > > Is the PTP selection stuff actually sorted now? Last time I tested it
> > > > after it having been merged into the kernel for a while, it didn't work,
> > > > and I reported that fact. You haven't told me that you now expect it to
> > > > work.  
> > > 
> > > The last part of the series, the PTP selection support wasn't merged when
> > > you tested it, although the default PTP choice that causes your regression
> > > was merged.
> > > Now it is fully merged, even the ethtool support.
> > > https://lore.kernel.org/netdev/mjn6eeo6lestvo6z3utb7aemufmfhn5alecyoaz46dt4pwjn6v@4aaaz6qpqd4b/
> > > 
> > > The only issue is the rtln warning from the phy_detach function. About it, I
> > > have already sent you the work I have done throwing ASSERT_RTNL in
> > > phy_detach. Maybe I should resend it as RFC.
> > >   
> > > > I don't want this merged until such time that we can be sure that MVPP2
> > > > platforms can continue using the MVPP2 PTP support, which to me means
> > > > that the PTP selection between a MAC and PHY needs to work.  
> > > 
> > > It should works, the default PTP will be the MAC PTP and you will be able to
> > > select the current PTP between MAC and PHY with the following command:
> > > # ethtool --set-hwtimestamp-cfg eth0 index 0 qualifier precise
> > > Time stamping configuration for eth0:
> > > Hardware timestamp provider index: 0
> > > Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> > > Hardware Transmit Timestamp Mode:
> > > 	off
> > > Hardware Receive Filter Mode:
> > > 	none
> > > Hardware Flags: none
> > > # ethtool --set-hwtimestamp-cfg eth0 index 1 qualifier precise
> > > Time stamping configuration for eth0:
> > > Hardware timestamp provider index: 1
> > > Hardware timestamp provider qualifier: Precise (IEEE 1588 quality)
> > > Hardware Transmit Timestamp Mode:
> > > 	off
> > > Hardware Receive Filter Mode:
> > > 	none
> > > Hardware Flags: none
> > > 
> > > You can list the PTPs with the dump command:
> > > # ethtool --show-time-stamping "*"
> > > 
> > > You will need to stop phc2sys and ptp4l during these change as linuxptp may
> > > face some issues during the PTP change.  
> > 
> > I'm preferring to my emails in connection with:
> > 
> > https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> > 
> > when I tested your work last time, it seemed that what was merged hadn't
> > even been tested. In the last email, you said you'd look into it, but I
> > didn't hear anything further. Have the problems I reported been
> > addressed?
> 
> It wasn't merged it was 19th version and it worked and was tested, but not
> with the best development design. I have replied to you that I will do some
> change in v20 to address this.
> https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/
> 
> It gets finally merged in v21.

Okay, so I'm pleased to report that this now works on the Macchiatobin:

# ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump tsinfo-get --json '{"header":{"dev-name":"eth2"}}' 
[{'header': {'dev-index': 5, 'dev-name': 'eth2'},
  'hwtstamp-provider': {'index': 2, 'qualifier': 0},
  'phc-index': 2,
  'rx-filters': {'bits': {'bit': [{'index': 0, 'name': 'none'},
                                  {'index': 1, 'name': 'all'}]},
                 'nomask': True,
                 'size': 16},
  'timestamping': {'bits': {'bit': [{'index': 0, 'name': 'hardware-transmit'},
                                    {'index': 1, 'name': 'software-transmit'},
                                    {'index': 2, 'name': 'hardware-receive'},
                                    {'index': 3, 'name': 'software-receive'},
                                    {'index': 4,
                                     'name': 'software-system-clock'},
                                    {'index': 6,
                                     'name': 'hardware-raw-clock'}]},
                   'nomask': True,
                   'size': 18},
  'tx-types': {'bits': {'bit': [{'index': 0, 'name': 'off'},
                                {'index': 1, 'name': 'on'},
                                {'index': 2, 'name': 'onestep-sync'},
                                {'index': 3, 'name': 'onestep-p2p'}]},
               'nomask': True,
               'size': 4}},
 {'header': {'dev-index': 5, 'dev-name': 'eth2'},
  'hwtstamp-provider': {'index': 0, 'qualifier': 0},
  'phc-index': 0,
  'rx-filters': {'bits': {'bit': [{'index': 0, 'name': 'none'},
                                  {'index': 2, 'name': 'some'}]},
                 'nomask': True,
                 'size': 16},
  'timestamping': {'bits': {'bit': [{'index': 0, 'name': 'hardware-transmit'},
                                    {'index': 2, 'name': 'hardware-receive'},
                                    {'index': 3, 'name': 'software-receive'},
                                    {'index': 4,
                                     'name': 'software-system-clock'},
                                    {'index': 6,
                                     'name': 'hardware-raw-clock'}]},
                   'nomask': True,
                   'size': 18},
  'tx-types': {'bits': {'bit': [{'index': 0, 'name': 'off'},
                                {'index': 1, 'name': 'on'}]},
               'nomask': True,
               'size': 4}}]

where phc 2 is the mvpp2 clock, and phc 0 is the PHY.

# ethtool -T eth2
Time stamping parameters for eth2:
Capabilities:
        hardware-transmit
        software-transmit
        hardware-receive
        software-receive
        software-system-clock
        hardware-raw-clock
PTP Hardware Clock: 2
Hardware Transmit Timestamp Modes:
        off
        on
        onestep-sync
        onestep-p2p
Hardware Receive Filter Modes:
        none
        all

So I guess that means that by default it's using PHC 2, and thus using
the MVPP2 PTP implementation - which is good, it means that when we add
Marvell PHY support, this won't switch to the PHY implementation.

Now, testing ethtool:

$ ./ethtool --get-hwtimestamp-cfg eth2
netlink error: Operation not supported

Using ynl:

# ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump tsconfig-get --json '{"header":{"dev-name":"eth2"}}'
[]

So, It's better, something still isn't correct as there's no
configuration. Maybe mvpp2 needs updating first? If that's the case,
then we're not yet in a position to merge PHY PTP support.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Kory Maincent]

On Tue, 8 Apr 2025 21:38:19 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:
> > On Mon, 7 Apr 2025 17:32:43 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > I'm preferring to my emails in connection with:
> > > 
> > > https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> > > 
> > > when I tested your work last time, it seemed that what was merged hadn't
> > > even been tested. In the last email, you said you'd look into it, but I
> > > didn't hear anything further. Have the problems I reported been
> > > addressed?  
> > 
> > It wasn't merged it was 19th version and it worked and was tested, but not
> > with the best development design. I have replied to you that I will do some
> > change in v20 to address this.
> > https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/
> > 
> > It gets finally merged in v21.  
> 
> Okay, so I'm pleased to report that this now works on the Macchiatobin:
> 
> where phc 2 is the mvpp2 clock, and phc 0 is the PHY.

Great, thank you for the testing!

> 
> # ethtool -T eth2
> Time stamping parameters for eth2:
> Capabilities:
>         hardware-transmit
>         software-transmit
>         hardware-receive
>         software-receive
>         software-system-clock
>         hardware-raw-clock
> PTP Hardware Clock: 2
> Hardware Transmit Timestamp Modes:
>         off
>         on
>         onestep-sync
>         onestep-p2p
> Hardware Receive Filter Modes:
>         none
>         all
> 
> So I guess that means that by default it's using PHC 2, and thus using
> the MVPP2 PTP implementation - which is good, it means that when we add
> Marvell PHY support, this won't switch to the PHY implementation.

Yes.

> 
> Now, testing ethtool:
> 
> $ ./ethtool --get-hwtimestamp-cfg eth2
> netlink error: Operation not supported
> 
> Using ynl:
> 
> # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> 
> So, It's better, something still isn't correct as there's no
> configuration. Maybe mvpp2 needs updating first? If that's the case,
> then we're not yet in a position to merge PHY PTP support.

Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
Vlad had made some work to update all net drivers to these NDOs but he never
send it mainline:
https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9

I have already try to ping him on this but without success.
Vlad any idea on when you could send your series upstream?

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Russell King (Oracle)]

On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:
> On Tue, 8 Apr 2025 21:38:19 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:
> > > On Mon, 7 Apr 2025 17:32:43 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > > I'm preferring to my emails in connection with:
> > > > 
> > > > https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> > > > 
> > > > when I tested your work last time, it seemed that what was merged hadn't
> > > > even been tested. In the last email, you said you'd look into it, but I
> > > > didn't hear anything further. Have the problems I reported been
> > > > addressed?  
> > > 
> > > It wasn't merged it was 19th version and it worked and was tested, but not
> > > with the best development design. I have replied to you that I will do some
> > > change in v20 to address this.
> > > https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/
> > > 
> > > It gets finally merged in v21.  
> > 
> > Okay, so I'm pleased to report that this now works on the Macchiatobin:
> > 
> > where phc 2 is the mvpp2 clock, and phc 0 is the PHY.
> 
> Great, thank you for the testing!
> 
> > 
> > # ethtool -T eth2
> > Time stamping parameters for eth2:
> > Capabilities:
> >         hardware-transmit
> >         software-transmit
> >         hardware-receive
> >         software-receive
> >         software-system-clock
> >         hardware-raw-clock
> > PTP Hardware Clock: 2
> > Hardware Transmit Timestamp Modes:
> >         off
> >         on
> >         onestep-sync
> >         onestep-p2p
> > Hardware Receive Filter Modes:
> >         none
> >         all
> > 
> > So I guess that means that by default it's using PHC 2, and thus using
> > the MVPP2 PTP implementation - which is good, it means that when we add
> > Marvell PHY support, this won't switch to the PHY implementation.
> 
> Yes.
> 
> > 
> > Now, testing ethtool:
> > 
> > $ ./ethtool --get-hwtimestamp-cfg eth2
> > netlink error: Operation not supported
> > 
> > Using ynl:
> > 
> > # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> > tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> > 
> > So, It's better, something still isn't correct as there's no
> > configuration. Maybe mvpp2 needs updating first? If that's the case,
> > then we're not yet in a position to merge PHY PTP support.
> 
> Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> Vlad had made some work to update all net drivers to these NDOs but he never
> send it mainline:
> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> 
> I have already try to ping him on this but without success.
> Vlad any idea on when you could send your series upstream?

Right, and that means that the kernel is not yet ready to support
Marvell PHY PTP, because all the pre-requisits to avoid breaking
mvpp2 have not yet been merged.

So that's a NAK on this series from me.

I'd have thought this would be obvious given my well known stance
on why I haven't merged Marvell PHY PTP support before.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Vladimir Oltean]

On Wed, Apr 09, 2025 at 09:35:59AM +0100, Russell King (Oracle) wrote:
> On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:
> > On Tue, 8 Apr 2025 21:38:19 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > 
> > > On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:
> > > > On Mon, 7 Apr 2025 17:32:43 +0100
> > > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > > > I'm preferring to my emails in connection with:
> > > > > 
> > > > > https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> > > > > 
> > > > > when I tested your work last time, it seemed that what was merged hadn't
> > > > > even been tested. In the last email, you said you'd look into it, but I
> > > > > didn't hear anything further. Have the problems I reported been
> > > > > addressed?  
> > > > 
> > > > It wasn't merged it was 19th version and it worked and was tested, but not
> > > > with the best development design. I have replied to you that I will do some
> > > > change in v20 to address this.
> > > > https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/
> > > > 
> > > > It gets finally merged in v21.  
> > > 
> > > Okay, so I'm pleased to report that this now works on the Macchiatobin:
> > > 
> > > where phc 2 is the mvpp2 clock, and phc 0 is the PHY.
> > 
> > Great, thank you for the testing!
> > 
> > > 
> > > # ethtool -T eth2
> > > Time stamping parameters for eth2:
> > > Capabilities:
> > >         hardware-transmit
> > >         software-transmit
> > >         hardware-receive
> > >         software-receive
> > >         software-system-clock
> > >         hardware-raw-clock
> > > PTP Hardware Clock: 2
> > > Hardware Transmit Timestamp Modes:
> > >         off
> > >         on
> > >         onestep-sync
> > >         onestep-p2p
> > > Hardware Receive Filter Modes:
> > >         none
> > >         all
> > > 
> > > So I guess that means that by default it's using PHC 2, and thus using
> > > the MVPP2 PTP implementation - which is good, it means that when we add
> > > Marvell PHY support, this won't switch to the PHY implementation.
> > 
> > Yes.
> > 
> > > 
> > > Now, testing ethtool:
> > > 
> > > $ ./ethtool --get-hwtimestamp-cfg eth2
> > > netlink error: Operation not supported
> > > 
> > > Using ynl:
> > > 
> > > # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> > > tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> > > 
> > > So, It's better, something still isn't correct as there's no
> > > configuration. Maybe mvpp2 needs updating first? If that's the case,
> > > then we're not yet in a position to merge PHY PTP support.
> > 
> > Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> > Vlad had made some work to update all net drivers to these NDOs but he never
> > send it mainline:
> > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> > 
> > I have already try to ping him on this but without success.
> > Vlad any idea on when you could send your series upstream?
> 
> Right, and that means that the kernel is not yet ready to support
> Marvell PHY PTP, because all the pre-requisits to avoid breaking
> mvpp2 have not yet been merged.
> 
> So that's a NAK on this series from me.
> 
> I'd have thought this would be obvious given my well known stance
> on why I haven't merged Marvell PHY PTP support before.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

I will try to update and submit that patch set over the course of this
weekend.

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Kory Maincent]

On Wed, 9 Apr 2025 11:38:35 +0300
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:

> On Wed, Apr 09, 2025 at 09:35:59AM +0100, Russell King (Oracle) wrote:
> > On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:  
> > > On Tue, 8 Apr 2025 21:38:19 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > >   
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
> > > 
> > > Great, thank you for the testing!
> > >   
>  [...]  
> > > 
> > > Yes.
> > >   
>  [...]  
> > > 
> > > Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> > > Vlad had made some work to update all net drivers to these NDOs but he
> > > never send it mainline:
> > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> > > 
> > > I have already try to ping him on this but without success.
> > > Vlad any idea on when you could send your series upstream?  
> > 
> > Right, and that means that the kernel is not yet ready to support
> > Marvell PHY PTP, because all the pre-requisits to avoid breaking
> > mvpp2 have not yet been merged.
> > 
> > So that's a NAK on this series from me.
> > 
> > I'd have thought this would be obvious given my well known stance
> > on why I haven't merged Marvell PHY PTP support before.
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!  
> 
> I will try to update and submit that patch set over the course of this
> weekend.

That's great, thanks for the update status!

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Russell King (Oracle)]

On Wed, Apr 09, 2025 at 11:38:35AM +0300, Vladimir Oltean wrote:
> On Wed, Apr 09, 2025 at 09:35:59AM +0100, Russell King (Oracle) wrote:
> > On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:
> > > On Tue, 8 Apr 2025 21:38:19 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > 
> > > > On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:
> > > > > On Mon, 7 Apr 2025 17:32:43 +0100
> > > > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > > > > I'm preferring to my emails in connection with:
> > > > > > 
> > > > > > https://lore.kernel.org/r/ZzTMhGDoi3WcY6MR@shell.armlinux.org.uk
> > > > > > 
> > > > > > when I tested your work last time, it seemed that what was merged hadn't
> > > > > > even been tested. In the last email, you said you'd look into it, but I
> > > > > > didn't hear anything further. Have the problems I reported been
> > > > > > addressed?  
> > > > > 
> > > > > It wasn't merged it was 19th version and it worked and was tested, but not
> > > > > with the best development design. I have replied to you that I will do some
> > > > > change in v20 to address this.
> > > > > https://lore.kernel.org/all/20241113171443.697ac278@kmaincent-XPS-13-7390/
> > > > > 
> > > > > It gets finally merged in v21.  
> > > > 
> > > > Okay, so I'm pleased to report that this now works on the Macchiatobin:
> > > > 
> > > > where phc 2 is the mvpp2 clock, and phc 0 is the PHY.
> > > 
> > > Great, thank you for the testing!
> > > 
> > > > 
> > > > # ethtool -T eth2
> > > > Time stamping parameters for eth2:
> > > > Capabilities:
> > > >         hardware-transmit
> > > >         software-transmit
> > > >         hardware-receive
> > > >         software-receive
> > > >         software-system-clock
> > > >         hardware-raw-clock
> > > > PTP Hardware Clock: 2
> > > > Hardware Transmit Timestamp Modes:
> > > >         off
> > > >         on
> > > >         onestep-sync
> > > >         onestep-p2p
> > > > Hardware Receive Filter Modes:
> > > >         none
> > > >         all
> > > > 
> > > > So I guess that means that by default it's using PHC 2, and thus using
> > > > the MVPP2 PTP implementation - which is good, it means that when we add
> > > > Marvell PHY support, this won't switch to the PHY implementation.
> > > 
> > > Yes.
> > > 
> > > > 
> > > > Now, testing ethtool:
> > > > 
> > > > $ ./ethtool --get-hwtimestamp-cfg eth2
> > > > netlink error: Operation not supported
> > > > 
> > > > Using ynl:
> > > > 
> > > > # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> > > > tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> > > > 
> > > > So, It's better, something still isn't correct as there's no
> > > > configuration. Maybe mvpp2 needs updating first? If that's the case,
> > > > then we're not yet in a position to merge PHY PTP support.
> > > 
> > > Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> > > Vlad had made some work to update all net drivers to these NDOs but he never
> > > send it mainline:
> > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> > > 
> > > I have already try to ping him on this but without success.
> > > Vlad any idea on when you could send your series upstream?
> > 
> > Right, and that means that the kernel is not yet ready to support
> > Marvell PHY PTP, because all the pre-requisits to avoid breaking
> > mvpp2 have not yet been merged.
> > 
> > So that's a NAK on this series from me.
> > 
> > I'd have thought this would be obvious given my well known stance
> > on why I haven't merged Marvell PHY PTP support before.
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> 
> I will try to update and submit that patch set over the course of this
> weekend.

Thanks. Friday is really the last day for me for an uncertain period
thereafter where I won't be able to do any further testing.

I don't run PTP on my network as it's IMHO not as good as NTP with the
hardware I have. It's also been five years since I last had something
setup, which was when I was working on Marvell PHY support, All the
knowledge I had back then for PTP support has been "swapped out" into
/dev/null. I don't even remember which machines I was using, and thus
have no idea if they're even still connected to the network.

As this has already been blocked on this for five years, I don't think
it's unreasonable that it takes longer, so please don't feel that you
need to get them done by Friday.

Just be aware that I won't be able to test again for an uncertain
period thereafter.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Kory Maincent]

On Wed, 9 Apr 2025 09:35:59 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:
> > On Tue, 8 Apr 2025 21:38:19 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> >   
> > > On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:  
>  [...]  
>  [...]  
>  [...]  
> > > 
> > > Okay, so I'm pleased to report that this now works on the Macchiatobin:
> > > 
> > > where phc 2 is the mvpp2 clock, and phc 0 is the PHY.  
> > 
> > Great, thank you for the testing!
> >   
> > > 
> > > # ethtool -T eth2
> > > Time stamping parameters for eth2:
> > > Capabilities:
> > >         hardware-transmit
> > >         software-transmit
> > >         hardware-receive
> > >         software-receive
> > >         software-system-clock
> > >         hardware-raw-clock
> > > PTP Hardware Clock: 2
> > > Hardware Transmit Timestamp Modes:
> > >         off
> > >         on
> > >         onestep-sync
> > >         onestep-p2p
> > > Hardware Receive Filter Modes:
> > >         none
> > >         all
> > > 
> > > So I guess that means that by default it's using PHC 2, and thus using
> > > the MVPP2 PTP implementation - which is good, it means that when we add
> > > Marvell PHY support, this won't switch to the PHY implementation.  
> > 
> > Yes.
> >   
> > > 
> > > Now, testing ethtool:
> > > 
> > > $ ./ethtool --get-hwtimestamp-cfg eth2
> > > netlink error: Operation not supported
> > > 
> > > Using ynl:
> > > 
> > > # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> > > tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> > > 
> > > So, It's better, something still isn't correct as there's no
> > > configuration. Maybe mvpp2 needs updating first? If that's the case,
> > > then we're not yet in a position to merge PHY PTP support.  
> > 
> > Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> > Vlad had made some work to update all net drivers to these NDOs but he never
> > send it mainline:
> > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> > 
> > I have already try to ping him on this but without success.
> > Vlad any idea on when you could send your series upstream?  
> 
> Right, and that means that the kernel is not yet ready to support
> Marvell PHY PTP, because all the pre-requisits to avoid breaking
> mvpp2 have not yet been merged.

Still I don't understand how this break mvpp2.
As you just tested this won't switch to the PHY PTP implementation. The old
usage of using SIOCG/SHWTSTAMP will still work, you simply won't be able to use
the new netlink feature of switching between the two PTP as long as
ndo_hwtstamp_get/set NDOs is not supported in mvpp2.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Russell King (Oracle)]

On Wed, Apr 09, 2025 at 10:46:37AM +0200, Kory Maincent wrote:
> On Wed, 9 Apr 2025 09:35:59 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Wed, Apr 09, 2025 at 10:31:30AM +0200, Kory Maincent wrote:
> > > On Tue, 8 Apr 2025 21:38:19 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > >   
> > > > On Mon, Apr 07, 2025 at 06:39:14PM +0200, Kory Maincent wrote:  
> >  [...]  
> >  [...]  
> >  [...]  
> > > > 
> > > > Okay, so I'm pleased to report that this now works on the Macchiatobin:
> > > > 
> > > > where phc 2 is the mvpp2 clock, and phc 0 is the PHY.  
> > > 
> > > Great, thank you for the testing!
> > >   
> > > > 
> > > > # ethtool -T eth2
> > > > Time stamping parameters for eth2:
> > > > Capabilities:
> > > >         hardware-transmit
> > > >         software-transmit
> > > >         hardware-receive
> > > >         software-receive
> > > >         software-system-clock
> > > >         hardware-raw-clock
> > > > PTP Hardware Clock: 2
> > > > Hardware Transmit Timestamp Modes:
> > > >         off
> > > >         on
> > > >         onestep-sync
> > > >         onestep-p2p
> > > > Hardware Receive Filter Modes:
> > > >         none
> > > >         all
> > > > 
> > > > So I guess that means that by default it's using PHC 2, and thus using
> > > > the MVPP2 PTP implementation - which is good, it means that when we add
> > > > Marvell PHY support, this won't switch to the PHY implementation.  
> > > 
> > > Yes.
> > >   
> > > > 
> > > > Now, testing ethtool:
> > > > 
> > > > $ ./ethtool --get-hwtimestamp-cfg eth2
> > > > netlink error: Operation not supported
> > > > 
> > > > Using ynl:
> > > > 
> > > > # ./ynl/cli.py --spec netlink/specs/ethtool.yaml --no-schema --dump
> > > > tsconfig-get --json '{"header":{"dev-name":"eth2"}}' []
> > > > 
> > > > So, It's better, something still isn't correct as there's no
> > > > configuration. Maybe mvpp2 needs updating first? If that's the case,
> > > > then we're not yet in a position to merge PHY PTP support.  
> > > 
> > > Indeed mvpp2 has not been update to support the ndo_hwtstamp_get/set NDOs.
> > > Vlad had made some work to update all net drivers to these NDOs but he never
> > > send it mainline:
> > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v9
> > > 
> > > I have already try to ping him on this but without success.
> > > Vlad any idea on when you could send your series upstream?  
> > 
> > Right, and that means that the kernel is not yet ready to support
> > Marvell PHY PTP, because all the pre-requisits to avoid breaking
> > mvpp2 have not yet been merged.
> 
> Still I don't understand how this break mvpp2.
> As you just tested this won't switch to the PHY PTP implementation.

How do I know that from the output? Nothing in the output appears to
tells me which PTP implementation will be used.

Maybe you have some understanding that makes this obvious that I don't
have.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Kory Maincent]

On Wed, 9 Apr 2025 10:29:52 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Apr 09, 2025 at 10:46:37AM +0200, Kory Maincent wrote:
> > On Wed, 9 Apr 2025 09:35:59 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> > > Right, and that means that the kernel is not yet ready to support
> > > Marvell PHY PTP, because all the pre-requisits to avoid breaking
> > > mvpp2 have not yet been merged.  
> > 
> > Still I don't understand how this break mvpp2.
> > As you just tested this won't switch to the PHY PTP implementation.  
> 
> How do I know that from the output? Nothing in the output appears to
> tells me which PTP implementation will be used.
> 
> Maybe you have some understanding that makes this obvious that I don't
> have.

You are right there is no report of the PTP source device info in ethtool.
With all the design change of the PTP series this has not made through my brain
that we lost this information along the way.

You can still know the source like that but that's not the best.
# ls -l /sys/class/ptp

It will be easy to add the source name support in netlink but which names are
better report to the user?
- dev_name of the netdev->dev and phydev->mdio.dev?
  Maybe not the best naming for the phy PTP source
  (ff0d0000.ethernet-ffffffff:01)
- "PHY" + the PHY ID and "MAC" string?
- Another idea? 

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Maxime Chevallier]

On Wed, 9 Apr 2025 14:23:09 +0200
Kory Maincent <kory.maincent@bootlin.com> wrote:

> On Wed, 9 Apr 2025 10:29:52 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Wed, Apr 09, 2025 at 10:46:37AM +0200, Kory Maincent wrote:  
> > > On Wed, 9 Apr 2025 09:35:59 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:  
> 
> > > > Right, and that means that the kernel is not yet ready to support
> > > > Marvell PHY PTP, because all the pre-requisits to avoid breaking
> > > > mvpp2 have not yet been merged.    
> > > 
> > > Still I don't understand how this break mvpp2.
> > > As you just tested this won't switch to the PHY PTP implementation.    
> > 
> > How do I know that from the output? Nothing in the output appears to
> > tells me which PTP implementation will be used.
> > 
> > Maybe you have some understanding that makes this obvious that I don't
> > have.  
> 
> You are right there is no report of the PTP source device info in ethtool.
> With all the design change of the PTP series this has not made through my brain
> that we lost this information along the way.
> 
> You can still know the source like that but that's not the best.
> # ls -l /sys/class/ptp
> 
> It will be easy to add the source name support in netlink but which names are
> better report to the user?
> - dev_name of the netdev->dev and phydev->mdio.dev?
>   Maybe not the best naming for the phy PTP source
>   (ff0d0000.ethernet-ffffffff:01)
> - "PHY" + the PHY ID and "MAC" string?

How about an enum instead of a string indicating the device type, and if
PHY, the phy_index ? (phy ID has another meaning :) )

Maxime

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Kory Maincent]

On Wed, 9 Apr 2025 14:46:54 +0200
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> On Wed, 9 Apr 2025 14:23:09 +0200
> Kory Maincent <kory.maincent@bootlin.com> wrote:
> 
> > On Wed, 9 Apr 2025 10:29:52 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> >   
> > > On Wed, Apr 09, 2025 at 10:46:37AM +0200, Kory Maincent wrote:    
>  [...]  
> >   
>  [...]  
>  [...]  
> > > 
> > > How do I know that from the output? Nothing in the output appears to
> > > tells me which PTP implementation will be used.
> > > 
> > > Maybe you have some understanding that makes this obvious that I don't
> > > have.    
> > 
> > You are right there is no report of the PTP source device info in ethtool.
> > With all the design change of the PTP series this has not made through my
> > brain that we lost this information along the way.
> > 
> > You can still know the source like that but that's not the best.
> > # ls -l /sys/class/ptp
> > 
> > It will be easy to add the source name support in netlink but which names
> > are better report to the user?
> > - dev_name of the netdev->dev and phydev->mdio.dev?
> >   Maybe not the best naming for the phy PTP source
> >   (ff0d0000.ethernet-ffffffff:01)
> > - "PHY" + the PHY ID and "MAC" string?  
> 
> How about an enum instead of a string indicating the device type, and if
> PHY, the phy_index ? (phy ID has another meaning :) )

This will raise the same question I faced during the ptp series mainline
process. In Linux, the PTP is managed through netdev or phylib API.
In case of a NIC all is managed through netdev. So if a NIC has a PTP at the PHY
layer how should we report that? As MAC PTP because it goes thought netdev, as
PHY PTP but without phyindex?
That's why maybe using netlink string could assure we won't have UAPI breakage
in the future due to weird cases.
What do you think?
 
Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Maxime Chevallier]

On Wed, 9 Apr 2025 16:49:20 +0200
Kory Maincent <kory.maincent@bootlin.com> wrote:

> On Wed, 9 Apr 2025 14:46:54 +0200
> Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
> 
> > On Wed, 9 Apr 2025 14:23:09 +0200
> > Kory Maincent <kory.maincent@bootlin.com> wrote:
> >   
> > > On Wed, 9 Apr 2025 10:29:52 +0100
> > > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > >     
> > > > On Wed, Apr 09, 2025 at 10:46:37AM +0200, Kory Maincent wrote:      
> >  [...]    
> > >     
> >  [...]  
> >  [...]    
> > > > 
> > > > How do I know that from the output? Nothing in the output appears to
> > > > tells me which PTP implementation will be used.
> > > > 
> > > > Maybe you have some understanding that makes this obvious that I don't
> > > > have.      
> > > 
> > > You are right there is no report of the PTP source device info in ethtool.
> > > With all the design change of the PTP series this has not made through my
> > > brain that we lost this information along the way.
> > > 
> > > You can still know the source like that but that's not the best.
> > > # ls -l /sys/class/ptp
> > > 
> > > It will be easy to add the source name support in netlink but which names
> > > are better report to the user?
> > > - dev_name of the netdev->dev and phydev->mdio.dev?
> > >   Maybe not the best naming for the phy PTP source
> > >   (ff0d0000.ethernet-ffffffff:01)
> > > - "PHY" + the PHY ID and "MAC" string?    
> > 
> > How about an enum instead of a string indicating the device type, and if
> > PHY, the phy_index ? (phy ID has another meaning :) )  
> 
> This will raise the same question I faced during the ptp series mainline
> process. In Linux, the PTP is managed through netdev or phylib API.
> In case of a NIC all is managed through netdev. So if a NIC has a PTP at the PHY
> layer how should we report that? As MAC PTP because it goes thought netdev, as
> PHY PTP but without phyindex?

Are you referring to the case where the PHY is transparently handled by
the MAC driver (i.e. controlled through a firmware of some sort) ?

In such case, how do you even know that timestamping is done in a PHY,
as the kernel doesn't know the PHY even exists ? The
HWTSTAMP_SOURCE_XXX enum either says it's from PHYLIB or NETDEV. As
PHYs handled by firmwares don't go through phylib, I'd say reporting
"PHY with no index" won't be accurate.

In such case I'd probably expect the NIC driver to register several
hwtstamp_provider with different qualifiers

> That's why maybe using netlink string could assure we won't have UAPI breakage
> in the future due to weird cases.
> What do you think?

Well I'd say this is the same for enums, nothing prevents you from
adding more values to your enum ?

Maxime

Re: [PATCH net-next v2 0/2] Add Marvell PHY PTP support

[Kory Maincent]

On Wed, 9 Apr 2025 17:10:55 +0200
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> On Wed, 9 Apr 2025 16:49:20 +0200
> Kory Maincent <kory.maincent@bootlin.com> wrote:
> 
> > On Wed, 9 Apr 2025 14:46:54 +0200
> > Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
> >   
> > > On Wed, 9 Apr 2025 14:23:09 +0200
> > > Kory Maincent <kory.maincent@bootlin.com> wrote:

> > > How about an enum instead of a string indicating the device type, and if
> > > PHY, the phy_index ? (phy ID has another meaning :) )    
> > 
> > This will raise the same question I faced during the ptp series mainline
> > process. In Linux, the PTP is managed through netdev or phylib API.
> > In case of a NIC all is managed through netdev. So if a NIC has a PTP at
> > the PHY layer how should we report that? As MAC PTP because it goes thought
> > netdev, as PHY PTP but without phyindex?  
> 
> Are you referring to the case where the PHY is transparently handled by
> the MAC driver (i.e. controlled through a firmware of some sort) ?

Yes I was.
 
> In such case, how do you even know that timestamping is done in a PHY,
> as the kernel doesn't know the PHY even exists ? The
> HWTSTAMP_SOURCE_XXX enum either says it's from PHYLIB or NETDEV. As
> PHYs handled by firmwares don't go through phylib, I'd say reporting
> "PHY with no index" won't be accurate.
> 
> In such case I'd probably expect the NIC driver to register several
> hwtstamp_provider with different qualifiers
> 
> > That's why maybe using netlink string could assure we won't have UAPI
> > breakage in the future due to weird cases.
> > What do you think?  
> 
> Well I'd say this is the same for enums, nothing prevents you from
> adding more values to your enum ?

Thanks! I am ok with that.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com