[PATCH net-next v1 0/3] r8169: add PTP support for RTL8127a

[PATCH net-next v1 0/3] r8169: add PTP support for RTL8127a

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

This series adds IEEE 1588 PTP support to r8169 for RTL8127a
(RTL_GIGA_MAC_VER_80): a PTP hardware clock and hardware TX/RX
timestamping of PTPv2 event messages. PPS and other auxiliary
functions are intentionally left out.

The PTP block is accessed through the PHY OCP interface, which may now
be used from atomic context, so patch 1 converts the GPHY_OCP access to
an atomic poll and serializes it with a dedicated spinlock.

Patch 2 registers the PHC (adjfine, adjtime, gettimex64, settime64)
while the interface is up.

Patch 3 adds hardware timestamping via ndo_hwtstamp_get/set, reports
the capabilities via get_ts_info and the TX timestamp counters via
get_ts_stats.

Javen Xu (3):
  r8169: make PHY OCP access safe in atomic context
  r8169: add PTP hardware clock support for RTL8127a
  r8169: add hardware timestamping support for RTL8127a

 drivers/net/ethernet/realtek/Kconfig      |   1 +
 drivers/net/ethernet/realtek/Makefile     |   2 +-
 drivers/net/ethernet/realtek/r8169.h      |   2 +
 drivers/net/ethernet/realtek/r8169_main.c | 113 ++++-
 drivers/net/ethernet/realtek/r8169_ptp.c  | 556 ++++++++++++++++++++++
 drivers/net/ethernet/realtek/r8169_ptp.h  |  58 +++
 6 files changed, 722 insertions(+), 10 deletions(-)
 create mode 100644 drivers/net/ethernet/realtek/r8169_ptp.c
 create mode 100644 drivers/net/ethernet/realtek/r8169_ptp.h

-- 
2.43.0

[PATCH net-next v1 1/3] r8169: make PHY OCP access safe in atomic context

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

r8168_phy_ocp_read() and r8168_phy_ocp_write() wait for the GPHY_OCP
handshake with rtl_loop_wait(), which may sleep (fsleep(25)). Upcoming
PTP support for RTL8127A accesses the PTP block through the same PHY
OCP interface from atomic context (spinlock held, NAPI RX path), and
concurrently with phylib MDIO accesses.

Busy-wait for the handshake with readl_poll_timeout_atomic() instead,
keeping the previous maximum wait time of 250us, and serialize the
GPHY_OCP register accesses with a new spinlock.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
 drivers/net/ethernet/realtek/r8169_main.c | 37 +++++++++++++++++++----
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index cd4ebc654dd0..96d3a2bd7094 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -20,6 +20,7 @@
 #include <linux/if_vlan.h>
 #include <linux/in.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/ip.h>
 #include <linux/tcp.h>
 #include <linux/interrupt.h>
@@ -881,6 +882,7 @@ struct rtl8169_private {
 	} wk;
 
 	raw_spinlock_t mac_ocp_lock;
+	spinlock_t phy_ocp_lock;	/* serialize GPHY_OCP access */
 	struct mutex led_lock;	/* serialize LED ctrl RMW access */
 
 	unsigned supports_gmii:1;
@@ -1234,30 +1236,52 @@ static bool rtl_ocp_reg_failure(u32 reg)
 	return WARN_ONCE(reg & 0xffff0001, "Invalid ocp reg %x!\n", reg);
 }
 
-DECLARE_RTL_COND(rtl_ocp_gphy_cond)
+/* PHY OCP registers may also be accessed from atomic context (e.g. by the
+ * PTP code), therefore busy-wait for completion instead of sleeping.
+ */
+static int rtl_phy_ocp_wait(struct rtl8169_private *tp, bool high)
 {
-	return RTL_R32(tp, GPHY_OCP) & OCPAR_FLAG;
+	u32 val;
+	int ret;
+
+	ret = readl_poll_timeout_atomic(tp->mmio_addr + GPHY_OCP, val,
+					!!(val & OCPAR_FLAG) == high, 1, 250);
+	if (ret && net_ratelimit())
+		netdev_err(tp->dev, "PHY OCP access timeout\n");
+
+	return ret;
 }
 
 static void r8168_phy_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
 {
+	unsigned long flags;
+
 	if (rtl_ocp_reg_failure(reg))
 		return;
 
+	spin_lock_irqsave(&tp->phy_ocp_lock, flags);
 	RTL_W32(tp, GPHY_OCP, OCPAR_FLAG | (reg << 15) | data);
-
-	rtl_loop_wait_low(tp, &rtl_ocp_gphy_cond, 25, 10);
+	rtl_phy_ocp_wait(tp, false);
+	spin_unlock_irqrestore(&tp->phy_ocp_lock, flags);
 }
 
 static int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg)
 {
+	unsigned long flags;
+	int val;
+
 	if (rtl_ocp_reg_failure(reg))
 		return 0;
 
+	spin_lock_irqsave(&tp->phy_ocp_lock, flags);
 	RTL_W32(tp, GPHY_OCP, reg << 15);
+	if (rtl_phy_ocp_wait(tp, true))
+		val = -ETIMEDOUT;
+	else
+		val = RTL_R32(tp, GPHY_OCP) & 0xffff;
+	spin_unlock_irqrestore(&tp->phy_ocp_lock, flags);
 
-	return rtl_loop_wait_high(tp, &rtl_ocp_gphy_cond, 25, 10) ?
-		(RTL_R32(tp, GPHY_OCP) & 0xffff) : -ETIMEDOUT;
+	return val;
 }
 
 static void __r8168_mac_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
@@ -6816,6 +6840,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->ocp_base = OCP_STD_PHY_BASE;
 
 	raw_spin_lock_init(&tp->mac_ocp_lock);
+	spin_lock_init(&tp->phy_ocp_lock);
 	mutex_init(&tp->led_lock);
 
 	/* Get the *optional* external "ether_clk" used on some boards */
-- 
2.43.0

[PATCH net-next v1 2/3] r8169: add PTP hardware clock support for RTL8127a

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

RTL8127a has a PTP clock in its PHY, accessed through the PHY OCP
interface. Register it as a PTP hardware clock supporting adjfine,
adjtime, gettimex64 and settime64.

The clock is registered when the interface is opened and unregistered
when it is closed. It is initialized from the system time every time
the chip is brought up.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
 drivers/net/ethernet/realtek/Kconfig      |   1 +
 drivers/net/ethernet/realtek/Makefile     |   2 +-
 drivers/net/ethernet/realtek/r8169.h      |   2 +
 drivers/net/ethernet/realtek/r8169_main.c |  20 +-
 drivers/net/ethernet/realtek/r8169_ptp.c  | 291 ++++++++++++++++++++++
 drivers/net/ethernet/realtek/r8169_ptp.h  |  32 +++
 6 files changed, 345 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/realtek/r8169_ptp.c
 create mode 100644 drivers/net/ethernet/realtek/r8169_ptp.h

diff --git a/drivers/net/ethernet/realtek/Kconfig b/drivers/net/ethernet/realtek/Kconfig
index 5818a9a500a8..985594a72fce 100644
--- a/drivers/net/ethernet/realtek/Kconfig
+++ b/drivers/net/ethernet/realtek/Kconfig
@@ -84,6 +84,7 @@ config 8139_OLD_RX_RESET
 config R8169
 	tristate "Realtek 8169/8168/8101/8125 ethernet support"
 	depends on PCI
+	depends on PTP_1588_CLOCK_OPTIONAL
 	select FW_LOADER
 	select CRC32
 	select REALTEK_PHY
diff --git a/drivers/net/ethernet/realtek/Makefile b/drivers/net/ethernet/realtek/Makefile
index 12a9c399f40c..6434804775dc 100644
--- a/drivers/net/ethernet/realtek/Makefile
+++ b/drivers/net/ethernet/realtek/Makefile
@@ -5,7 +5,7 @@
 
 obj-$(CONFIG_8139CP) += 8139cp.o
 obj-$(CONFIG_8139TOO) += 8139too.o
-r8169-y += r8169_main.o r8169_firmware.o r8169_phy_config.o
+r8169-y += r8169_main.o r8169_firmware.o r8169_phy_config.o r8169_ptp.o
 r8169-$(CONFIG_R8169_LEDS) += r8169_leds.o
 obj-$(CONFIG_R8169) += r8169.o
 obj-$(CONFIG_RTASE) += rtase/
diff --git a/drivers/net/ethernet/realtek/r8169.h b/drivers/net/ethernet/realtek/r8169.h
index fb772cc04310..f9d9d048c5c4 100644
--- a/drivers/net/ethernet/realtek/r8169.h
+++ b/drivers/net/ethernet/realtek/r8169.h
@@ -95,3 +95,5 @@ int rtl8125_get_led_mode(struct rtl8169_private *tp, int index);
 int rtl8125_set_led_mode(struct rtl8169_private *tp, int index, u16 mode);
 struct r8169_led_classdev *rtl8125_init_leds(struct net_device *ndev);
 void r8169_remove_leds(struct r8169_led_classdev *leds);
+int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg);
+void r8168_phy_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data);
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 96d3a2bd7094..fc3cfa09a57e 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -37,6 +37,7 @@
 
 #include "r8169.h"
 #include "r8169_firmware.h"
+#include "r8169_ptp.h"
 
 #define FIRMWARE_8168D_1	"rtl_nic/rtl8168d-1.fw"
 #define FIRMWARE_8168D_2	"rtl_nic/rtl8168d-2.fw"
@@ -888,6 +889,7 @@ struct rtl8169_private {
 	unsigned supports_gmii:1;
 	unsigned aspm_manageable:1;
 	unsigned dash_enabled:1;
+	unsigned ptp_support:1;
 	dma_addr_t counters_phys_addr;
 	struct rtl8169_counters *counters;
 	struct rtl8169_tc_offsets tc_offset;
@@ -902,6 +904,8 @@ struct rtl8169_private {
 	struct phylink *phylink;
 	struct phylink_config phylink_config;
 	struct phylink_pcs pcs;
+
+	struct rtl8169_ptp ptp;
 };
 
 typedef void (*rtl_generic_fct)(struct rtl8169_private *tp);
@@ -1252,7 +1256,7 @@ static int rtl_phy_ocp_wait(struct rtl8169_private *tp, bool high)
 	return ret;
 }
 
-static void r8168_phy_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
+void r8168_phy_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
 {
 	unsigned long flags;
 
@@ -1265,7 +1269,7 @@ static void r8168_phy_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
 	spin_unlock_irqrestore(&tp->phy_ocp_lock, flags);
 }
 
-static int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg)
+int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg)
 {
 	unsigned long flags;
 	int val;
@@ -5780,6 +5784,9 @@ static void rtl8169_up(struct rtl8169_private *tp)
 	rtl8169_napi_enable(tp);
 	enable_work(&tp->wk.work);
 	rtl_reset_work(tp);
+
+	if (tp->ptp_support)
+		rtl8169_ptp_set_local_time(&tp->ptp);
 }
 
 static int rtl8169_close(struct net_device *dev)
@@ -5791,6 +5798,8 @@ static int rtl8169_close(struct net_device *dev)
 
 	phylink_stop(tp->phylink);
 	netif_stop_queue(dev);
+	if (tp->ptp_support)
+		rtl8169_ptp_stop(&tp->ptp);
 	rtl8169_down(tp);
 	for (int i = 0; i < tp->num_rx_rings; i++)
 		rtl8169_rx_clear(tp, &tp->rx_ring[i]);
@@ -5861,6 +5870,8 @@ static int rtl_open(struct net_device *dev)
 	}
 	rtl8169_up(tp);
 	phylink_start(tp->phylink);
+	if (tp->ptp_support)
+		rtl8169_ptp_init(&tp->ptp);
 	rtl8169_init_counter_offsets(tp);
 	netif_start_queue(dev);
 out:
@@ -6904,6 +6915,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->dash_type = rtl_get_dash_type(tp);
 	tp->dash_enabled = rtl_dash_is_enabled(tp);
 
+	tp->ptp_support = IS_ENABLED(CONFIG_PTP_1588_CLOCK) &&
+			  tp->mac_version == RTL_GIGA_MAC_VER_80;
+	if (tp->ptp_support)
+		rtl8169_ptp_setup(&tp->ptp, tp, dev, &pdev->dev);
+
 	if (rtl_hw_support_rss(tp)) {
 		tp->rss_data = devm_kzalloc(&pdev->dev, sizeof(*tp->rss_data),
 					    GFP_KERNEL);
diff --git a/drivers/net/ethernet/realtek/r8169_ptp.c b/drivers/net/ethernet/realtek/r8169_ptp.c
new file mode 100644
index 000000000000..6201170e0c83
--- /dev/null
+++ b/drivers/net/ethernet/realtek/r8169_ptp.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* r8169_ptp.c: RealTek 8169/8168/8101/8125 ethernet driver.
+ *
+ * PTP hardware clock support for RTL8127A.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/iopoll.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/timekeeping.h>
+
+#include "r8169.h"
+#include "r8169_ptp.h"
+
+/* PTP registers, accessed through the PHY OCP interface */
+#define PTP_CTL				0xe400
+#define PTP_CTL_EN			BIT(0)
+#define PTP_CTL_TYPE_3			0x003f
+#define PTP_INER			0xe402
+#define PTP_INSR			0xe404
+#define PTP_INT_RX_TS			BIT(2)
+#define PTP_INT_TX_TS			BIT(3)
+#define PTP_SYNCE_CTL			0xe406
+#define PTP_SYNCE_EN			BIT(0)
+#define PTP_GEN_CFG			0xe408
+#define PTP_GEN_CFG_INT_CLR_MODE	BIT(0)
+#define PTP_CLK_CFG			0xe410
+#define PTP_CLK_CFG_EXEC		BIT(0)
+#define PTP_CLK_CFG_MODE		GENMASK(3, 1)
+/* Time registers, PTP_CFG_TIME for the clock, PTP_TRX_TS_TIME for packets */
+#define PTP_CFG_TIME			0xe412
+#define PTP_TRX_TS_TIME			0xe446
+#define PTP_TIME_NS_LO			0x0
+#define PTP_TIME_NS_HI			0x2
+#define PTP_TIME_S_LO			0x4
+#define PTP_TIME_S_MI			0x6
+#define PTP_TIME_S_HI			0x8
+#define PTP_TIME_NS_HI_MASK		GENMASK(13, 0)
+#define PTP_TIME_NSEC_MASK		GENMASK(29, 0)
+#define PTP_TIME_SEC_MASK		GENMASK_ULL(47, 0)
+#define PTP_RATE_HI_MASK		GENMASK(21, 16)
+
+#define PTP_MAX_ADJ			488281
+
+/* PTP_CLK_CFG clock adjust modes */
+enum rtl8169_ptp_clkadj_mode {
+	PTP_CLKADJ_DIRECT_READ		= 4,
+	PTP_CLKADJ_DIRECT_WRITE		= 6,
+	PTP_CLKADJ_INC_STEP		= 8,
+	PTP_CLKADJ_DEC_STEP		= 10,
+	PTP_CLKADJ_RATE_WRITE		= 14,
+};
+
+static u16 rtl8169_ptp_read(struct rtl8169_ptp *ptp, u32 reg)
+{
+	return r8168_phy_ocp_read(ptp->tp, reg);
+}
+
+static void rtl8169_ptp_write(struct rtl8169_ptp *ptp, u32 reg, u16 val)
+{
+	r8168_phy_ocp_write(ptp->tp, reg, val);
+}
+
+static void rtl8169_ptp_modify(struct rtl8169_ptp *ptp, u32 reg, u16 mask,
+			       u16 set)
+{
+	u16 val = rtl8169_ptp_read(ptp, reg);
+
+	rtl8169_ptp_write(ptp, reg, (val & ~mask) | set);
+}
+
+/* Poll until (reg & mask) is non-zero (set == true) or zero (set == false).
+ * Returns the last register value read, or -ETIMEDOUT.
+ */
+static int rtl8169_ptp_poll(struct rtl8169_ptp *ptp, u32 reg, u16 mask,
+			    bool set)
+{
+	u16 val;
+	int ret;
+
+	ret = read_poll_timeout_atomic(rtl8169_ptp_read, val,
+				       !!(val & mask) == set, 1, 1000, false,
+				       ptp, reg);
+	if (ret) {
+		if (net_ratelimit())
+			netdev_err(ptp->dev, "PTP reg 0x%04x poll timeout\n",
+				   reg);
+		return ret;
+	}
+
+	return val;
+}
+
+static void rtl8169_ptp_clkadj(struct rtl8169_ptp *ptp,
+			       enum rtl8169_ptp_clkadj_mode mode)
+{
+	rtl8169_ptp_modify(ptp, PTP_CLK_CFG, PTP_CLK_CFG_MODE,
+			   PTP_CLK_CFG_EXEC | mode);
+	rtl8169_ptp_poll(ptp, PTP_CLK_CFG, PTP_CLK_CFG_EXEC, false);
+}
+
+static void rtl8169_ptp_read_time(struct rtl8169_ptp *ptp, u32 base,
+				  struct timespec64 *ts)
+{
+	u64 sec;
+	u32 nsec;
+
+	nsec = rtl8169_ptp_read(ptp, base + PTP_TIME_NS_HI) &
+	       PTP_TIME_NS_HI_MASK;
+	nsec <<= 16;
+	nsec |= rtl8169_ptp_read(ptp, base + PTP_TIME_NS_LO);
+
+	sec = rtl8169_ptp_read(ptp, base + PTP_TIME_S_HI);
+	sec <<= 16;
+	sec |= rtl8169_ptp_read(ptp, base + PTP_TIME_S_MI);
+	sec <<= 16;
+	sec |= rtl8169_ptp_read(ptp, base + PTP_TIME_S_LO);
+
+	ts->tv_sec = sec;
+	ts->tv_nsec = nsec;
+}
+
+static void rtl8169_ptp_write_time(struct rtl8169_ptp *ptp,
+				   const struct timespec64 *ts)
+{
+	u64 sec = ts->tv_sec & PTP_TIME_SEC_MASK;
+	u32 nsec = ts->tv_nsec & PTP_TIME_NSEC_MASK;
+
+	rtl8169_ptp_write(ptp, PTP_CFG_TIME + PTP_TIME_NS_LO, nsec);
+	rtl8169_ptp_write(ptp, PTP_CFG_TIME + PTP_TIME_NS_HI, nsec >> 16);
+	rtl8169_ptp_write(ptp, PTP_CFG_TIME + PTP_TIME_S_LO, sec);
+	rtl8169_ptp_write(ptp, PTP_CFG_TIME + PTP_TIME_S_MI, sec >> 16);
+	rtl8169_ptp_write(ptp, PTP_CFG_TIME + PTP_TIME_S_HI, sec >> 32);
+}
+
+static void rtl8169_ptp_hwtstamp_enable(struct rtl8169_ptp *ptp, bool enable)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ptp->lock, flags);
+
+	rtl8169_ptp_modify(ptp, PTP_SYNCE_CTL, PTP_SYNCE_EN, 0);
+	if (enable) {
+		rtl8169_ptp_modify(ptp, PTP_INER, 0,
+				   PTP_INT_RX_TS | PTP_INT_TX_TS);
+		rtl8169_ptp_modify(ptp, PTP_GEN_CFG, 0,
+				   PTP_GEN_CFG_INT_CLR_MODE);
+		rtl8169_ptp_write(ptp, PTP_INSR, 0xffff);
+		rtl8169_ptp_write(ptp, PTP_CTL, PTP_CTL_TYPE_3 | BIT(12));
+	} else {
+		rtl8169_ptp_modify(ptp, PTP_INER,
+				   PTP_INT_RX_TS | PTP_INT_TX_TS, 0);
+		rtl8169_ptp_modify(ptp, PTP_CTL, PTP_CTL_EN, 0);
+	}
+	rtl8169_ptp_modify(ptp, 0xa640, 0, BIT(15));
+
+	spin_unlock_irqrestore(&ptp->lock, flags);
+}
+
+static void rtl8169_ptp_settime(struct rtl8169_ptp *ptp,
+				const struct timespec64 *ts)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ptp->lock, flags);
+	rtl8169_ptp_write_time(ptp, ts);
+	rtl8169_ptp_clkadj(ptp, PTP_CLKADJ_DIRECT_WRITE);
+	spin_unlock_irqrestore(&ptp->lock, flags);
+}
+
+static int rtl8169_ptp_adjfine(struct ptp_clock_info *info, long scaled_ppm)
+{
+	struct rtl8169_ptp *ptp = container_of(info, struct rtl8169_ptp, info);
+	s32 ppb = scaled_ppm_to_ppb(scaled_ppm);
+	unsigned long flags;
+	u32 rate;
+
+	rate = div_u64((u64)abs(ppb) << 32, NSEC_PER_SEC);
+	if (ppb < 0)
+		rate = -rate;
+
+	spin_lock_irqsave(&ptp->lock, flags);
+	rtl8169_ptp_write(ptp, PTP_CFG_TIME + PTP_TIME_NS_LO, rate);
+	rtl8169_ptp_write(ptp, PTP_CFG_TIME + PTP_TIME_NS_HI,
+			  FIELD_GET(PTP_RATE_HI_MASK, rate));
+	rtl8169_ptp_clkadj(ptp, PTP_CLKADJ_RATE_WRITE);
+	spin_unlock_irqrestore(&ptp->lock, flags);
+
+	return 0;
+}
+
+static int rtl8169_ptp_adjtime(struct ptp_clock_info *info, s64 delta)
+{
+	struct rtl8169_ptp *ptp = container_of(info, struct rtl8169_ptp, info);
+	struct timespec64 d = ns_to_timespec64(abs(delta));
+	unsigned long flags;
+
+	spin_lock_irqsave(&ptp->lock, flags);
+	rtl8169_ptp_write_time(ptp, &d);
+	rtl8169_ptp_clkadj(ptp, delta < 0 ? PTP_CLKADJ_DEC_STEP :
+					    PTP_CLKADJ_INC_STEP);
+	spin_unlock_irqrestore(&ptp->lock, flags);
+
+	return 0;
+}
+
+static int rtl8169_ptp_gettimex64(struct ptp_clock_info *info,
+				  struct timespec64 *ts,
+				  struct ptp_system_timestamp *sts)
+{
+	struct rtl8169_ptp *ptp = container_of(info, struct rtl8169_ptp, info);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ptp->lock, flags);
+	ptp_read_system_prets(sts);
+	rtl8169_ptp_clkadj(ptp, PTP_CLKADJ_DIRECT_READ);
+	ptp_read_system_postts(sts);
+	rtl8169_ptp_read_time(ptp, PTP_CFG_TIME, ts);
+	spin_unlock_irqrestore(&ptp->lock, flags);
+
+	return 0;
+}
+
+static int rtl8169_ptp_settime64(struct ptp_clock_info *info,
+				 const struct timespec64 *ts)
+{
+	struct rtl8169_ptp *ptp = container_of(info, struct rtl8169_ptp, info);
+
+	rtl8169_ptp_settime(ptp, ts);
+
+	return 0;
+}
+
+static const struct ptp_clock_info rtl8169_ptp_info = {
+	.owner		= THIS_MODULE,
+	.max_adj	= PTP_MAX_ADJ,
+	.adjfine	= rtl8169_ptp_adjfine,
+	.adjtime	= rtl8169_ptp_adjtime,
+	.gettimex64	= rtl8169_ptp_gettimex64,
+	.settime64	= rtl8169_ptp_settime64,
+};
+
+void rtl8169_ptp_set_local_time(struct rtl8169_ptp *ptp)
+{
+	struct timespec64 ts;
+
+	ktime_get_real_ts64(&ts);
+	rtl8169_ptp_settime(ptp, &ts);
+}
+
+void rtl8169_ptp_init(struct rtl8169_ptp *ptp)
+{
+	struct ptp_clock *clock;
+
+	ptp->info = rtl8169_ptp_info;
+	snprintf(ptp->info.name, sizeof(ptp->info.name), "%pm",
+		 ptp->dev->dev_addr);
+
+	clock = ptp_clock_register(&ptp->info, ptp->parent);
+	if (IS_ERR(clock)) {
+		netdev_err(ptp->dev, "failed to register PTP clock: %pe\n",
+			   clock);
+		return;
+	}
+	ptp->clock = clock;
+
+	rtl8169_ptp_hwtstamp_enable(ptp, false);
+}
+
+void rtl8169_ptp_stop(struct rtl8169_ptp *ptp)
+{
+	if (!ptp->clock)
+		return;
+
+	rtl8169_ptp_hwtstamp_enable(ptp, false);
+
+	ptp_clock_unregister(ptp->clock);
+	ptp->clock = NULL;
+}
+
+void rtl8169_ptp_setup(struct rtl8169_ptp *ptp, struct rtl8169_private *tp,
+		       struct net_device *dev, struct device *parent)
+{
+	ptp->tp = tp;
+	ptp->dev = dev;
+	ptp->parent = parent;
+	spin_lock_init(&ptp->lock);
+}
diff --git a/drivers/net/ethernet/realtek/r8169_ptp.h b/drivers/net/ethernet/realtek/r8169_ptp.h
new file mode 100644
index 000000000000..e0b342b82283
--- /dev/null
+++ b/drivers/net/ethernet/realtek/r8169_ptp.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* r8169_ptp.h: RealTek 8169/8168/8101/8125 ethernet driver.
+ *
+ * PTP hardware clock support.
+ */
+
+#ifndef _R8169_PTP_H
+#define _R8169_PTP_H
+
+#include <linux/ptp_clock_kernel.h>
+#include <linux/spinlock.h>
+
+struct device;
+struct net_device;
+struct rtl8169_private;
+
+struct rtl8169_ptp {
+	struct rtl8169_private *tp;
+	struct net_device *dev;
+	struct device *parent;
+	struct ptp_clock_info info;
+	struct ptp_clock *clock;
+	spinlock_t lock;	/* serialize PTP register access sequences */
+};
+
+void rtl8169_ptp_setup(struct rtl8169_ptp *ptp, struct rtl8169_private *tp,
+		       struct net_device *dev, struct device *parent);
+void rtl8169_ptp_init(struct rtl8169_ptp *ptp);
+void rtl8169_ptp_stop(struct rtl8169_ptp *ptp);
+void rtl8169_ptp_set_local_time(struct rtl8169_ptp *ptp);
+
+#endif /* _R8169_PTP_H */
-- 
2.43.0

[PATCH net-next v1 3/3] r8169: add hardware timestamping support for RTL8127a

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

Add support for hardware timestamping of PTPv2 event messages on
RTL8127a:

- RX: the PHY latches the timestamp of the last received event message
  per message type. Read it back for PTP event messages and attach it
  to the skb.
- TX: one packet at a time can be timestamped. The hardware signals the
  availability of the TX timestamp in PTP_INSR, which is polled from a
  work item. Requests are dropped if no timestamp shows up within 15s.

The hardware does not filter by message type, so all PTPv2 RX filters
are reported as HWTSTAMP_FILTER_PTP_V2_EVENT.

TX timestamp statistics are exposed via ethtool get_ts_stats.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
 drivers/net/ethernet/realtek/r8169_main.c |  56 ++++-
 drivers/net/ethernet/realtek/r8169_ptp.c  | 267 +++++++++++++++++++++-
 drivers/net/ethernet/realtek/r8169_ptp.h  |  28 ++-
 3 files changed, 348 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index fc3cfa09a57e..2ffb877ac2b4 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5146,6 +5146,10 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	skb_tx_timestamp(skb);
 
+	if (tp->ptp_support &&
+	    unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
+		rtl8169_ptp_tx_tstamp(&tp->ptp, skb);
+
 	/* Force memory writes to complete before releasing descriptor */
 	dma_wmb();
 
@@ -5483,6 +5487,10 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 			rtl8169_rx_hash(tp, desc, skb);
 			skb_record_rx_queue(skb, ring - tp->rx_ring);
 		}
+
+		if (tp->ptp_support)
+			rtl8169_ptp_rx_tstamp(&tp->ptp, skb);
+
 		rtl8169_rx_csum(tp, skb, status);
 		skb->protocol = eth_type_trans(skb, dev);
 
@@ -6091,6 +6099,29 @@ static void rtl_remove_one(struct pci_dev *pdev)
 	rtl_rar_set(tp, tp->dev->perm_addr);
 }
 
+static int rtl8169_hwtstamp_get(struct net_device *dev,
+				struct kernel_hwtstamp_config *config)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (!tp->ptp_support)
+		return -EOPNOTSUPP;
+
+	return rtl8169_ptp_hwtstamp_get(&tp->ptp, config);
+}
+
+static int rtl8169_hwtstamp_set(struct net_device *dev,
+				struct kernel_hwtstamp_config *config,
+				struct netlink_ext_ack *extack)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (!tp->ptp_support)
+		return -EOPNOTSUPP;
+
+	return rtl8169_ptp_hwtstamp_set(&tp->ptp, config, extack);
+}
+
 static const struct net_device_ops rtl_netdev_ops = {
 	.ndo_open		= rtl_open,
 	.ndo_stop		= rtl8169_close,
@@ -6105,6 +6136,8 @@ static const struct net_device_ops rtl_netdev_ops = {
 	.ndo_set_mac_address	= rtl_set_mac_address,
 	.ndo_eth_ioctl		= rtl8169_ioctl,
 	.ndo_set_rx_mode	= rtl_set_rx_mode,
+	.ndo_hwtstamp_get	= rtl8169_hwtstamp_get,
+	.ndo_hwtstamp_set	= rtl8169_hwtstamp_set,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= rtl8169_netpoll,
 #endif
@@ -6796,6 +6829,26 @@ static void rtl8169_get_channels(struct net_device *dev,
 	}
 }
 
+static int rtl8169_get_ts_info(struct net_device *dev,
+			       struct kernel_ethtool_ts_info *info)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (!tp->ptp_support)
+		return ethtool_op_get_ts_info(dev, info);
+
+	return rtl8169_ptp_get_ts_info(&tp->ptp, info);
+}
+
+static void rtl8169_get_ts_stats(struct net_device *dev,
+				 struct ethtool_ts_stats *ts_stats)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (tp->ptp_support)
+		rtl8169_ptp_get_ts_stats(&tp->ptp, ts_stats);
+}
+
 static const struct ethtool_ops rtl8169_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_MAX_FRAMES,
@@ -6810,7 +6863,8 @@ static const struct ethtool_ops rtl8169_ethtool_ops = {
 	.get_strings		= rtl8169_get_strings,
 	.get_sset_count		= rtl8169_get_sset_count,
 	.get_ethtool_stats	= rtl8169_get_ethtool_stats,
-	.get_ts_info		= ethtool_op_get_ts_info,
+	.get_ts_info		= rtl8169_get_ts_info,
+	.get_ts_stats		= rtl8169_get_ts_stats,
 	.nway_reset		= rtl8169_nway_reset,
 	.get_eee		= rtl8169_get_eee,
 	.set_eee		= rtl8169_set_eee,
diff --git a/drivers/net/ethernet/realtek/r8169_ptp.c b/drivers/net/ethernet/realtek/r8169_ptp.c
index 6201170e0c83..d31802583761 100644
--- a/drivers/net/ethernet/realtek/r8169_ptp.c
+++ b/drivers/net/ethernet/realtek/r8169_ptp.c
@@ -1,15 +1,18 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* r8169_ptp.c: RealTek 8169/8168/8101/8125 ethernet driver.
  *
- * PTP hardware clock support for RTL8127A.
+ * PTP hardware clock and timestamping support for RTL8127A.
  */
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/ethtool.h>
 #include <linux/iopoll.h>
 #include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
+#include <linux/ptp_classify.h>
+#include <linux/skbuff.h>
 #include <linux/timekeeping.h>
 
 #include "r8169.h"
@@ -30,6 +33,16 @@
 #define PTP_CLK_CFG			0xe410
 #define PTP_CLK_CFG_EXEC		BIT(0)
 #define PTP_CLK_CFG_MODE		GENMASK(3, 1)
+#define PTP_TRX_TS_STA			0xe430
+#define PTP_TRX_TS_RD			BIT(0)
+#define PTP_TRX_TS_SEL_RX		BIT(1)
+#define PTP_TRX_TS_MSGTYPE		GENMASK(3, 2)
+#define PTP_TRX_TS_EXEC			BIT(4)
+#define PTP_TRX_TS_TX_PDLYRSP_RDY	BIT(12)
+#define PTP_TRX_TS_TX_PDLYREQ_RDY	BIT(13)
+#define PTP_TRX_TS_TX_DLYREQ_RDY	BIT(14)
+#define PTP_TRX_TS_TX_SYNC_RDY		BIT(15)
+#define PTP_TRX_TS_TX_RDY		GENMASK(15, 12)
 /* Time registers, PTP_CFG_TIME for the clock, PTP_TRX_TS_TIME for packets */
 #define PTP_CFG_TIME			0xe412
 #define PTP_TRX_TS_TIME			0xe446
@@ -44,6 +57,12 @@
 #define PTP_RATE_HI_MASK		GENMASK(21, 16)
 
 #define PTP_MAX_ADJ			488281
+#define RTL8169_PTP_TX_TIMEOUT		(15 * HZ)
+
+/* ptp->state bits */
+enum rtl8169_ptp_state {
+	RTL8169_PTP_TX_IN_PROGRESS,
+};
 
 /* PTP_CLK_CFG clock adjust modes */
 enum rtl8169_ptp_clkadj_mode {
@@ -136,6 +155,48 @@ static void rtl8169_ptp_write_time(struct rtl8169_ptp *ptp,
 	rtl8169_ptp_write(ptp, PTP_CFG_TIME + PTP_TIME_S_HI, sec >> 32);
 }
 
+/* Latch the RX or TX timestamp of the given message type into the
+ * PTP_TRX_TS_TIME registers.
+ */
+static void rtl8169_ptp_trx_ts_cmd(struct rtl8169_ptp *ptp, u16 cmd)
+{
+	rtl8169_ptp_modify(ptp, PTP_TRX_TS_STA,
+			   PTP_TRX_TS_SEL_RX | PTP_TRX_TS_MSGTYPE,
+			   PTP_TRX_TS_RD | cmd);
+	rtl8169_ptp_poll(ptp, PTP_TRX_TS_STA, PTP_TRX_TS_RD, false);
+}
+
+static void rtl8169_ptp_egress_time(struct rtl8169_ptp *ptp,
+				    struct timespec64 *ts)
+{
+	u8 msgtype;
+	int val;
+
+	val = rtl8169_ptp_poll(ptp, PTP_TRX_TS_STA, PTP_TRX_TS_TX_RDY, true);
+	if (val < 0)
+		val = 0;
+
+	switch (val & PTP_TRX_TS_TX_RDY) {
+	case PTP_TRX_TS_TX_PDLYRSP_RDY:
+		msgtype = PTP_MSGTYPE_PDELAY_RESP;
+		break;
+	case PTP_TRX_TS_TX_PDLYREQ_RDY:
+		msgtype = PTP_MSGTYPE_PDELAY_REQ;
+		break;
+	case PTP_TRX_TS_TX_DLYREQ_RDY:
+		msgtype = PTP_MSGTYPE_DELAY_REQ;
+		break;
+	case PTP_TRX_TS_TX_SYNC_RDY:
+	default:
+		msgtype = PTP_MSGTYPE_SYNC;
+		break;
+	}
+
+	rtl8169_ptp_trx_ts_cmd(ptp, FIELD_PREP(PTP_TRX_TS_MSGTYPE, msgtype) |
+				    PTP_TRX_TS_EXEC);
+	rtl8169_ptp_read_time(ptp, PTP_TRX_TS_TIME, ts);
+}
+
 static void rtl8169_ptp_hwtstamp_enable(struct rtl8169_ptp *ptp, bool enable)
 {
 	unsigned long flags;
@@ -243,6 +304,201 @@ static const struct ptp_clock_info rtl8169_ptp_info = {
 	.settime64	= rtl8169_ptp_settime64,
 };
 
+static void rtl8169_ptp_tx_complete(struct rtl8169_ptp *ptp,
+				    struct skb_shared_hwtstamps *hwts)
+{
+	struct sk_buff *skb = ptp->tx_skb;
+
+	ptp->tx_skb = NULL;
+	clear_bit_unlock(RTL8169_PTP_TX_IN_PROGRESS, &ptp->state);
+
+	if (hwts)
+		skb_tstamp_tx(skb, hwts);
+	dev_kfree_skb_any(skb);
+}
+
+static void rtl8169_ptp_tx_work(struct work_struct *work)
+{
+	struct rtl8169_ptp *ptp = container_of(work, struct rtl8169_ptp,
+					       tx_work);
+	struct skb_shared_hwtstamps hwts = {};
+	struct timespec64 ts;
+	unsigned long flags;
+	bool ready;
+
+	if (!ptp->tx_skb)
+		return;
+
+	if (time_is_before_jiffies(ptp->tx_start + RTL8169_PTP_TX_TIMEOUT)) {
+		spin_lock_irqsave(&ptp->lock, flags);
+		rtl8169_ptp_write(ptp, PTP_INSR, PTP_INT_TX_TS);
+		spin_unlock_irqrestore(&ptp->lock, flags);
+
+		ptp->tx_timeouts++;
+		rtl8169_ptp_tx_complete(ptp, NULL);
+		return;
+	}
+
+	spin_lock_irqsave(&ptp->lock, flags);
+	ready = rtl8169_ptp_read(ptp, PTP_INSR) & PTP_INT_TX_TS;
+	if (ready) {
+		rtl8169_ptp_write(ptp, PTP_INSR, PTP_INT_TX_TS);
+		rtl8169_ptp_egress_time(ptp, &ts);
+	}
+	spin_unlock_irqrestore(&ptp->lock, flags);
+
+	if (!ready) {
+		schedule_work(&ptp->tx_work);
+		return;
+	}
+
+	hwts.hwtstamp = timespec64_to_ktime(ts);
+	ptp->tx_pkts++;
+	rtl8169_ptp_tx_complete(ptp, &hwts);
+}
+
+void rtl8169_ptp_tx_tstamp(struct rtl8169_ptp *ptp, struct sk_buff *skb)
+{
+	if (ptp->config.tx_type != HWTSTAMP_TX_ON ||
+	    test_and_set_bit_lock(RTL8169_PTP_TX_IN_PROGRESS, &ptp->state)) {
+		ptp->tx_skipped++;
+		return;
+	}
+
+	skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+	ptp->tx_skb = skb_get(skb);
+	ptp->tx_start = jiffies;
+	schedule_work(&ptp->tx_work);
+}
+
+void rtl8169_ptp_rx_tstamp(struct rtl8169_ptp *ptp, struct sk_buff *skb)
+{
+	unsigned int ptp_class;
+	struct ptp_header *hdr;
+	struct timespec64 ts;
+	unsigned long flags;
+	u8 msgtype;
+
+	if (!READ_ONCE(ptp->rx_enabled))
+		return;
+
+	ptp_class = ptp_classify_raw(skb);
+	if (ptp_class == PTP_CLASS_NONE)
+		return;
+
+	skb_reset_mac_header(skb);
+	hdr = ptp_parse_header(skb, ptp_class);
+	if (unlikely(!hdr))
+		return;
+
+	/* Only event messages are timestamped by the hardware */
+	msgtype = ptp_get_msgtype(hdr, ptp_class);
+	if (msgtype > PTP_MSGTYPE_PDELAY_RESP)
+		return;
+
+	spin_lock_irqsave(&ptp->lock, flags);
+	rtl8169_ptp_trx_ts_cmd(ptp, PTP_TRX_TS_SEL_RX |
+				    FIELD_PREP(PTP_TRX_TS_MSGTYPE, msgtype) |
+				    PTP_TRX_TS_EXEC);
+	rtl8169_ptp_read_time(ptp, PTP_TRX_TS_TIME, &ts);
+	spin_unlock_irqrestore(&ptp->lock, flags);
+
+	skb_hwtstamps(skb)->hwtstamp = timespec64_to_ktime(ts);
+}
+
+int rtl8169_ptp_hwtstamp_get(struct rtl8169_ptp *ptp,
+			     struct kernel_hwtstamp_config *config)
+{
+	hwtstamp_config_to_kernel(config, &ptp->config);
+
+	return 0;
+}
+
+int rtl8169_ptp_hwtstamp_set(struct rtl8169_ptp *ptp,
+			     struct kernel_hwtstamp_config *config,
+			     struct netlink_ext_ack *extack)
+{
+	bool enable = false;
+	bool rx_enabled;
+
+	if (!ptp->clock)
+		return -ENETDOWN;
+
+	if (config->flags)
+		return -EINVAL;
+
+	switch (config->tx_type) {
+	case HWTSTAMP_TX_ON:
+		enable = true;
+		break;
+	case HWTSTAMP_TX_OFF:
+		break;
+	default:
+		return -ERANGE;
+	}
+
+	switch (config->rx_filter) {
+	case HWTSTAMP_FILTER_NONE:
+		rx_enabled = false;
+		break;
+	case HWTSTAMP_FILTER_PTP_V2_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
+	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
+	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
+		config->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
+		rx_enabled = true;
+		enable = true;
+		break;
+	default:
+		return -ERANGE;
+	}
+
+	if (ptp->config.tx_type != config->tx_type ||
+	    ptp->config.rx_filter != config->rx_filter) {
+		hwtstamp_config_from_kernel(&ptp->config, config);
+		rtl8169_ptp_hwtstamp_enable(ptp, enable);
+	}
+	WRITE_ONCE(ptp->rx_enabled, rx_enabled);
+
+	return 0;
+}
+
+int rtl8169_ptp_get_ts_info(struct rtl8169_ptp *ptp,
+			    struct kernel_ethtool_ts_info *info)
+{
+	info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE |
+				SOF_TIMESTAMPING_TX_HARDWARE |
+				SOF_TIMESTAMPING_RX_HARDWARE |
+				SOF_TIMESTAMPING_RAW_HARDWARE;
+
+	if (ptp->clock)
+		info->phc_index = ptp_clock_index(ptp->clock);
+
+	info->tx_types = BIT(HWTSTAMP_TX_OFF) | BIT(HWTSTAMP_TX_ON);
+	info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) |
+			   BIT(HWTSTAMP_FILTER_PTP_V2_EVENT) |
+			   BIT(HWTSTAMP_FILTER_PTP_V2_L4_EVENT) |
+			   BIT(HWTSTAMP_FILTER_PTP_V2_SYNC) |
+			   BIT(HWTSTAMP_FILTER_PTP_V2_L4_SYNC) |
+			   BIT(HWTSTAMP_FILTER_PTP_V2_DELAY_REQ) |
+			   BIT(HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ);
+
+	return 0;
+}
+
+void rtl8169_ptp_get_ts_stats(struct rtl8169_ptp *ptp,
+			      struct ethtool_ts_stats *stats)
+{
+	stats->pkts = ptp->tx_pkts;
+	stats->lost = ptp->tx_timeouts;
+	stats->err = ptp->tx_skipped;
+}
+
 void rtl8169_ptp_set_local_time(struct rtl8169_ptp *ptp)
 {
 	struct timespec64 ts;
@@ -267,6 +523,8 @@ void rtl8169_ptp_init(struct rtl8169_ptp *ptp)
 	}
 	ptp->clock = clock;
 
+	ptp->config.tx_type = HWTSTAMP_TX_OFF;
+	ptp->config.rx_filter = HWTSTAMP_FILTER_NONE;
 	rtl8169_ptp_hwtstamp_enable(ptp, false);
 }
 
@@ -275,7 +533,13 @@ void rtl8169_ptp_stop(struct rtl8169_ptp *ptp)
 	if (!ptp->clock)
 		return;
 
+	ptp->config.tx_type = HWTSTAMP_TX_OFF;
+	ptp->config.rx_filter = HWTSTAMP_FILTER_NONE;
+	WRITE_ONCE(ptp->rx_enabled, false);
 	rtl8169_ptp_hwtstamp_enable(ptp, false);
+	cancel_work_sync(&ptp->tx_work);
+	if (ptp->tx_skb)
+		rtl8169_ptp_tx_complete(ptp, NULL);
 
 	ptp_clock_unregister(ptp->clock);
 	ptp->clock = NULL;
@@ -288,4 +552,5 @@ void rtl8169_ptp_setup(struct rtl8169_ptp *ptp, struct rtl8169_private *tp,
 	ptp->dev = dev;
 	ptp->parent = parent;
 	spin_lock_init(&ptp->lock);
+	INIT_WORK(&ptp->tx_work, rtl8169_ptp_tx_work);
 }
diff --git a/drivers/net/ethernet/realtek/r8169_ptp.h b/drivers/net/ethernet/realtek/r8169_ptp.h
index e0b342b82283..cb3d6a04096b 100644
--- a/drivers/net/ethernet/realtek/r8169_ptp.h
+++ b/drivers/net/ethernet/realtek/r8169_ptp.h
@@ -1,18 +1,24 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /* r8169_ptp.h: RealTek 8169/8168/8101/8125 ethernet driver.
  *
- * PTP hardware clock support.
+ * PTP hardware clock and timestamping support.
  */
 
 #ifndef _R8169_PTP_H
 #define _R8169_PTP_H
 
+#include <linux/net_tstamp.h>
 #include <linux/ptp_clock_kernel.h>
 #include <linux/spinlock.h>
+#include <linux/workqueue.h>
 
 struct device;
+struct ethtool_ts_stats;
+struct kernel_ethtool_ts_info;
 struct net_device;
+struct netlink_ext_ack;
 struct rtl8169_private;
+struct sk_buff;
 
 struct rtl8169_ptp {
 	struct rtl8169_private *tp;
@@ -21,6 +27,15 @@ struct rtl8169_ptp {
 	struct ptp_clock_info info;
 	struct ptp_clock *clock;
 	spinlock_t lock;	/* serialize PTP register access sequences */
+	struct hwtstamp_config config;
+	bool rx_enabled;
+	struct work_struct tx_work;
+	struct sk_buff *tx_skb;
+	unsigned long tx_start;
+	unsigned long state;
+	u64 tx_pkts;
+	u64 tx_timeouts;
+	u64 tx_skipped;
 };
 
 void rtl8169_ptp_setup(struct rtl8169_ptp *ptp, struct rtl8169_private *tp,
@@ -28,5 +43,16 @@ void rtl8169_ptp_setup(struct rtl8169_ptp *ptp, struct rtl8169_private *tp,
 void rtl8169_ptp_init(struct rtl8169_ptp *ptp);
 void rtl8169_ptp_stop(struct rtl8169_ptp *ptp);
 void rtl8169_ptp_set_local_time(struct rtl8169_ptp *ptp);
+void rtl8169_ptp_tx_tstamp(struct rtl8169_ptp *ptp, struct sk_buff *skb);
+void rtl8169_ptp_rx_tstamp(struct rtl8169_ptp *ptp, struct sk_buff *skb);
+int rtl8169_ptp_hwtstamp_get(struct rtl8169_ptp *ptp,
+			     struct kernel_hwtstamp_config *config);
+int rtl8169_ptp_hwtstamp_set(struct rtl8169_ptp *ptp,
+			     struct kernel_hwtstamp_config *config,
+			     struct netlink_ext_ack *extack);
+int rtl8169_ptp_get_ts_info(struct rtl8169_ptp *ptp,
+			    struct kernel_ethtool_ts_info *info);
+void rtl8169_ptp_get_ts_stats(struct rtl8169_ptp *ptp,
+			      struct ethtool_ts_stats *stats);
 
 #endif /* _R8169_PTP_H */
-- 
2.43.0

Re: [PATCH net-next v1 1/3] r8169: make PHY OCP access safe in atomic context

[Andrew Lunn]

On Mon, Nov 09, 2026 at 10:30:20AM +0800, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
> 
> r8168_phy_ocp_read() and r8168_phy_ocp_write() wait for the GPHY_OCP
> handshake with rtl_loop_wait(), which may sleep (fsleep(25)). Upcoming
> PTP support for RTL8127A accesses the PTP block through the same PHY
> OCP interface from atomic context (spinlock held, NAPI RX path), and
> concurrently with phylib MDIO accesses.
> 
> Busy-wait for the handshake with readl_poll_timeout_atomic() instead,
> keeping the previous maximum wait time of 250us, and serialize the
> GPHY_OCP register accesses with a new spinlock.

There are many drivers which need to access PHY registers via blocking
MDIO accesses. Why is this driver different?

	Andrew

Re: [PATCH net-next v1 2/3] r8169: add PTP hardware clock support for RTL8127a

[Andrew Lunn]

On Mon, Nov 09, 2026 at 10:30:21AM +0800, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
> 
> RTL8127a has a PTP clock in its PHY, accessed through the PHY OCP
> interface. Register it as a PTP hardware clock supporting adjfine,
> adjtime, gettimex64 and settime64.

What PHY is this? Why is the PHY driver not exporting the clock, like
other PHY drivers do.

	Andrew

RE: [PATCH net-next v1 1/3] r8169: make PHY OCP access safe in atomic context

[Javen]

>
>On Mon, Nov 09, 2026 at 10:30:20AM +0800, javen wrote:
>> From: Javen Xu <javen_xu@realsil.com.cn>
>>
>> r8168_phy_ocp_read() and r8168_phy_ocp_write() wait for the GPHY_OCP
>> handshake with rtl_loop_wait(), which may sleep (fsleep(25)). Upcoming
>> PTP support for RTL8127A accesses the PTP block through the same PHY
>> OCP interface from atomic context (spinlock held, NAPI RX path), and
>> concurrently with phylib MDIO accesses.
>>
>> Busy-wait for the handshake with readl_poll_timeout_atomic() instead,
>> keeping the previous maximum wait time of 250us, and serialize the
>> GPHY_OCP register accesses with a new spinlock.
>
>There are many drivers which need to access PHY registers via blocking MDIO
>accesses. Why is this driver different?

In v1, RX timestamp was read in rtl_rx(), i.e. in NAPI context and tx is in ndo_start_xmit. Since neither of those fast-paths can sleep, I had to export the raw PHY OCP accessors and wrap them in a spinlock.

I will try to move all the code to phy driver and all accesses will use phy_read_mmd()/phy_write_mmd() and will be serialized by a mutex.

Thanks,
Javen

>
>        Andrew

Re: [PATCH net-next v1 1/3] r8169: make PHY OCP access safe in atomic context

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] When the PHY OCP write handshake never completes, the CPU now
  busy-spins with local IRQs disabled for the full timeout, and the real…

--

r8169: make PHY OCP access safe in atomic context

This commit replaces the sleeping rtl_loop_wait() handshake in
r8168_phy_ocp_read() and r8168_phy_ocp_write() with a busy-wait based on
readl_poll_timeout_atomic(). It also serializes GPHY_OCP accesses with a
new phy_ocp_lock spinlock, so that the upcoming RTL8127A PTP code can
access PHY OCP registers from atomic context.

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index cd4ebc654dd0b..96d3a2bd70947 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c

[ ... ]

> @@ -1234,30 +1236,52 @@ static bool rtl_ocp_reg_failure(u32 reg)
>  	return WARN_ONCE(reg & 0xffff0001, "Invalid ocp reg %x!\n", reg);
>  }
>  
> -DECLARE_RTL_COND(rtl_ocp_gphy_cond)
> +/* PHY OCP registers may also be accessed from atomic context (e.g. by the
> + * PTP code), therefore busy-wait for completion instead of sleeping.
> + */
> +static int rtl_phy_ocp_wait(struct rtl8169_private *tp, bool high)
>  {
> -	return RTL_R32(tp, GPHY_OCP) & OCPAR_FLAG;
> +	u32 val;
> +	int ret;
> +
> +	ret = readl_poll_timeout_atomic(tp->mmio_addr + GPHY_OCP, val,
> +					!!(val & OCPAR_FLAG) == high, 1, 250);
> +	if (ret && net_ratelimit())
> +		netdev_err(tp->dev, "PHY OCP access timeout\n");
> +
> +	return ret;
>  }
>  
>  static void r8168_phy_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
>  {
> +	unsigned long flags;
> +
>  	if (rtl_ocp_reg_failure(reg))
>  		return;
>  
> +	spin_lock_irqsave(&tp->phy_ocp_lock, flags);
>  	RTL_W32(tp, GPHY_OCP, OCPAR_FLAG | (reg << 15) | data);
> -
> -	rtl_loop_wait_low(tp, &rtl_ocp_gphy_cond, 25, 10);
> +	rtl_phy_ocp_wait(tp, false);
> +	spin_unlock_irqrestore(&tp->phy_ocp_lock, flags);
>  }

[Severity: Low]
What happens here when the device stops responding and MMIO reads return
all-ones, for example after surprise removal, a PCIe link down or during
AER recovery?

In that case OCPAR_FLAG always reads as set. rtl_phy_ocp_wait(tp, false)
never sees its condition met and spins until the timeout, with local
interrupts disabled by spin_lock_irqsave().

Is the timeout actually 250us? poll_timeout_us_atomic() in
include/linux/iopoll.h only subtracts the udelay() time, plus 1ns per
iteration, from the budget:

		if (__delay_us) { \
			udelay(__delay_us); \
			if (__timeout_us) \
				__left_ns -= __delay_ns; \
		} \
		cpu_relax(); \
		if (__timeout_us) \
			__left_ns--; \

The time spent in the non-posted readl() is not counted. So each write
that times out would keep interrupts off for about 250 x (1us + readl
latency). That is longer than the commit message says:

  "keeping the previous maximum wait time of 250us"

Long write sequences would hit one such window per write. Examples are
the rtl_fw PHY firmware download, the r8169_phy_config.c tables via
rtl_writephy()->r8168g_mdio_write(), phylib MDIO ops such as
r8169_mdio_write_reg_c45(), and the r8127 SFP init.

Before this patch, the same failure path slept in rtl_loop_wait() via
fsleep(25) and left interrupts enabled.

The read path in r8168_phy_ocp_read() doesn't seem to have this problem,
because an all-ones value meets the "flag high" condition right away.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109023022.770-1-javen_xu%40realsil.com.cn

Re: [PATCH net-next v1 2/3] r8169: add PTP hardware clock support for RTL8127a

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 7 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 4 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The PHC stays registered and callable while the NIC is runtime-
  suspended, and its callbacks never take a runtime-PM reference or…
- [Medium] rtl8169_ptp_read() returns u16, but it wraps
  r8168_phy_ocp_read(), which returns int and gives -ETIMEDOUT when the…
- [Medium] rtl8169_ptp_clkadj() is void and drops the -ETIMEDOUT from
  rtl8169_ptp_poll() when PTP_CLK_CFG_EXEC never self-clears.
- [Medium] In rtl_init_one(), tp->ptp_support is set only from
  IS_ENABLED(CONFIG_PTP_1588_CLOCK) && tp->mac_version ==…
- [Medium] The PTP block configuration is applied in only one place,
  rtl8169_ptp_init(), once per ndo_open, and only after rtl8169_up() has…
- [Low] The commit message does not mention that every open and close on
  RTL8127A now writes PTP/SyncE control registers and an undocumented…
- [Low] In rtl8169_ptp_gettimex64(), ptp_read_system_prets() and
  ptp_read_system_postts() bracket all of…

--

r8169: add PTP hardware clock support for RTL8127a

This adds a PTP hardware clock for RTL8127A. The clock is in the PHY and
is accessed through the PHY OCP interface. It supports adjfine, adjtime,
gettimex64 and settime64. It is registered on open, unregistered on close,
and seeded from system time each time the chip is brought up.

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 96d3a2bd70947..fc3cfa09a57e7 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c

[ ... ]

> @@ -5780,6 +5784,9 @@ static void rtl8169_up(struct rtl8169_private *tp)
>  	rtl8169_napi_enable(tp);
>  	enable_work(&tp->wk.work);
>  	rtl_reset_work(tp);
> +
> +	if (tp->ptp_support)
> +		rtl8169_ptp_set_local_time(&tp->ptp);
>  }
>  
>  static int rtl8169_close(struct net_device *dev)
> @@ -5791,6 +5798,8 @@ static int rtl8169_close(struct net_device *dev)
>  
>  	phylink_stop(tp->phylink);
>  	netif_stop_queue(dev);
> +	if (tp->ptp_support)
> +		rtl8169_ptp_stop(&tp->ptp);
>  	rtl8169_down(tp);
>  	for (int i = 0; i < tp->num_rx_rings; i++)
>  		rtl8169_rx_clear(tp, &tp->rx_ring[i]);
> @@ -5861,6 +5870,8 @@ static int rtl_open(struct net_device *dev)
>  	}
>  	rtl8169_up(tp);
>  	phylink_start(tp->phylink);
> +	if (tp->ptp_support)
> +		rtl8169_ptp_init(&tp->ptp);
>  	rtl8169_init_counter_offsets(tp);
>  	netif_start_queue(dev);
>  out:

[Severity: High]
Can the PHC be used while the NIC is runtime-suspended?

The clock is registered here and is only unregistered in rtl8169_close().
Runtime and system suspend go through rtl8169_net_suspend(), which calls
rtl8169_down() but does nothing with the PHC:

drivers/net/ethernet/realtek/r8169_main.c:rtl8169_net_suspend() {
    ...
	netif_device_detach(tp->dev);

	if (netif_running(tp->dev))
		rtl8169_down(tp);
    ...
}

If the interface is up with no carrier, rtl8169_runtime_idle() schedules
a suspend:

	if (!netif_running(tp->dev) || !netif_carrier_ok(tp->dev))
		pm_schedule_suspend(device, 10000);

Once the chip is in D3, /dev/ptpN is still live. clock_gettime(),
clock_adjtime() and the PTP_SYS_OFFSET* ioctls still reach gettimex64,
settime64, adjtime and adjfine. None of these take a runtime-PM
reference or check pm_runtime_active(). rtl8169_get_stats64() and the
LED helpers in this driver do guard their hardware access that way.

With MMIO returning all-ones:

- r8168_phy_ocp_read() returns 0xffff.
- Each r8168_phy_ocp_write() spins for its full 250 us timeout.
- The EXEC poll in rtl8169_ptp_poll() never sees bit 0 clear.

read_poll_timeout_atomic() counts only the udelay time, so the poll
runs about 1000 polled OCP reads. All of this happens under
spin_lock_irqsave(&ptp->lock).

Wouldn't that keep IRQs off for milliseconds on every PHC call? After
that, gettimex64 would return 0 with tv_nsec = 0x3fffffff and
tv_sec = 0xffffffffffff. settime, adjtime and adjfine would also report
success without doing anything.

Resume seems to have two more problems. rtl8169_runtime_resume() calls
rtl8169_up() without RTNL or ptp->lock:

	if (tp->TxDescArray)
		rtl8169_up(tp);

phy_ocp_lock only serializes single register accesses. So phy_init_hw()
and genphy_soft_reset() in rtl8169_init_phy() can interleave with a
multi-register PTP sequence running on another CPU.

rtl8169_up() then calls rtl8169_ptp_set_local_time(), which steps the
PHC back to CLOCK_REALTIME on every runtime or system resume. Doesn't
that discard the time a PTP servo had set? The adjfine rate is also
never cached or restored.

This is unchanged at the end of the series.

> @@ -6904,6 +6915,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	tp->dash_type = rtl_get_dash_type(tp);
>  	tp->dash_enabled = rtl_dash_is_enabled(tp);
>  
> +	tp->ptp_support = IS_ENABLED(CONFIG_PTP_1588_CLOCK) &&
> +			  tp->mac_version == RTL_GIGA_MAC_VER_80;
> +	if (tp->ptp_support)
> +		rtl8169_ptp_setup(&tp->ptp, tp, dev, &pdev->dev);
> +

[Severity: Medium]
Should this also check tp->sfp_mode?

On the RTL8127ATF fiber variant, rtl_get_sfp_mode() returns
RTL_SFP_8127_ATF. r8169_mdio_register() is only called for RTL_SFP_NONE,
so there is no phydev, and rtl8169_up() skips rtl8169_init_phy().

A PHC is still registered in that case. The PTP block, which the commit
message says is "in its PHY", is still programmed on every open and
resume through rtl8169_ptp_set_local_time() and
rtl8169_ptp_hwtstamp_enable().

What happens if that block isn't clocked or isn't in the data path in
fiber mode? The clkadj polls would time out with IRQs off and log
errors on every open and resume, and userspace would get a bogus PHC.
Later in the series, hardware timestamping is also advertised for this
variant.

It isn't clear whether the PTP block at 0xe400 works in fiber mode, but
the code treats both modes the same way.

>  	if (rtl_hw_support_rss(tp)) {
>  		tp->rss_data = devm_kzalloc(&pdev->dev, sizeof(*tp->rss_data),
>  					    GFP_KERNEL);
> diff --git a/drivers/net/ethernet/realtek/r8169_ptp.c b/drivers/net/ethernet/realtek/r8169_ptp.c
> new file mode 100644
> index 0000000000000..6201170e0c83e
> --- /dev/null
> +++ b/drivers/net/ethernet/realtek/r8169_ptp.c
> @@ -0,0 +1,291 @@

[ ... ]

> +static u16 rtl8169_ptp_read(struct rtl8169_ptp *ptp, u32 reg)
> +{
> +	return r8168_phy_ocp_read(ptp->tp, reg);
> +}

[Severity: Medium]
r8168_phy_ocp_read() returns int and gives -ETIMEDOUT when the GPHY_OCP
handshake times out:

	if (rtl_phy_ocp_wait(tp, true))
		val = -ETIMEDOUT;
	else
		val = RTL_R32(tp, GPHY_OCP) & 0xffff;

Doesn't the u16 return type here turn that into the register value
0xff92? Callers would then treat it as real data:

- rtl8169_ptp_modify() writes (0xff92 & ~mask) | set back to the
  register. For PTP_CLK_CFG that sets bits 4-15. PTP_SYNCE_CTL,
  PTP_INER, PTP_CTL and 0xa640 in rtl8169_ptp_hwtstamp_enable() are
  affected the same way.
- rtl8169_ptp_poll(..., PTP_CLK_CFG_EXEC, false) sees bit 0 clear in
  0xff92, so a failed read counts as command completion.
- rtl8169_ptp_read_time() ORs 0xff92 into the nsec and sec fields, so
  gettimex64 returns a made-up time.

Later in the series, the same helper also feeds the RX/TX timestamp
paths.

[ ... ]

> +static void rtl8169_ptp_clkadj(struct rtl8169_ptp *ptp,
> +			       enum rtl8169_ptp_clkadj_mode mode)
> +{
> +	rtl8169_ptp_modify(ptp, PTP_CLK_CFG, PTP_CLK_CFG_MODE,
> +			   PTP_CLK_CFG_EXEC | mode);
> +	rtl8169_ptp_poll(ptp, PTP_CLK_CFG, PTP_CLK_CFG_EXEC, false);
> +}

[Severity: Medium]
The -ETIMEDOUT from rtl8169_ptp_poll() is dropped here. Because
rtl8169_ptp_clkadj() is void, gettimex64, settime64, adjtime and
adjfine all return 0 even when the command never completed. Should the
error be passed back to the PTP core?

PTP_CFG_TIME holds both the operands and the results:

- rtl8169_ptp_write_time() fills it for DIRECT_WRITE, INC_STEP and
  DEC_STEP.
- rtl8169_ptp_adjfine() writes the rate word into NS_LO/NS_HI.
- rtl8169_ptp_gettimex64() reads the result from it after DIRECT_READ.

If DIRECT_READ doesn't complete, can gettimex64 return a leftover
adjtime delta, adjfine rate word or settime value as the current PHC
time, with status 0? The next command could also be issued while EXEC
is still set.

This is unchanged at the end of the series.

[ ... ]

> +static void rtl8169_ptp_hwtstamp_enable(struct rtl8169_ptp *ptp, bool enable)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ptp->lock, flags);
> +
> +	rtl8169_ptp_modify(ptp, PTP_SYNCE_CTL, PTP_SYNCE_EN, 0);
> +	if (enable) {
> +		rtl8169_ptp_modify(ptp, PTP_INER, 0,
> +				   PTP_INT_RX_TS | PTP_INT_TX_TS);
> +		rtl8169_ptp_modify(ptp, PTP_GEN_CFG, 0,
> +				   PTP_GEN_CFG_INT_CLR_MODE);
> +		rtl8169_ptp_write(ptp, PTP_INSR, 0xffff);
> +		rtl8169_ptp_write(ptp, PTP_CTL, PTP_CTL_TYPE_3 | BIT(12));
> +	} else {
> +		rtl8169_ptp_modify(ptp, PTP_INER,
> +				   PTP_INT_RX_TS | PTP_INT_TX_TS, 0);
> +		rtl8169_ptp_modify(ptp, PTP_CTL, PTP_CTL_EN, 0);
> +	}
> +	rtl8169_ptp_modify(ptp, 0xa640, 0, BIT(15));

[Severity: Low]
This isn't a bug, but the commit message doesn't say that every open and
close on RTL8127A now calls rtl8169_ptp_hwtstamp_enable(ptp, false).
That call:

- clears PTP_SYNCE_EN, regardless of what firmware or BIOS configured;
- masks the RX/TX timestamp interrupts;
- clears PTP_CTL_EN;
- sets BIT(15) of PHY OCP register 0xa640.

Could the commit message explain why SyncE is turned off? Could 0xa640
get a #define and a comment saying what bit 15 does and whether it
changes PHY behaviour beyond PTP?

The same writes are still there at the end of the series.

> +
> +	spin_unlock_irqrestore(&ptp->lock, flags);
> +}

[ ... ]

> +static int rtl8169_ptp_gettimex64(struct ptp_clock_info *info,
> +				  struct timespec64 *ts,
> +				  struct ptp_system_timestamp *sts)
> +{
> +	struct rtl8169_ptp *ptp = container_of(info, struct rtl8169_ptp, info);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ptp->lock, flags);
> +	ptp_read_system_prets(sts);
> +	rtl8169_ptp_clkadj(ptp, PTP_CLKADJ_DIRECT_READ);
> +	ptp_read_system_postts(sts);

[Severity: Low]
Do prets/postts cover more than the latching event here?
rtl8169_ptp_clkadj() does three things:

1. an OCP read of PTP_CLK_CFG for the read-modify-write;
2. the OCP write that triggers the latch;
3. the EXEC-clear poll loop, with at least one more OCP read plus
   udelay(1) per iteration.

Each PHY OCP access is an MMIO write plus a busy-poll on OCPAR_FLAG. The
window is therefore several OCP round trips wide and not centred on the
latch point. Wouldn't that bias the midpoint phc2sys uses and inflate
the uncertainty reported by PTP_SYS_OFFSET_EXTENDED?

This is unchanged at the end of the series.

> +	rtl8169_ptp_read_time(ptp, PTP_CFG_TIME, ts);
> +	spin_unlock_irqrestore(&ptp->lock, flags);
> +
> +	return 0;
> +}

[ ... ]

> +void rtl8169_ptp_init(struct rtl8169_ptp *ptp)
> +{
> +	struct ptp_clock *clock;
> +
> +	ptp->info = rtl8169_ptp_info;
> +	snprintf(ptp->info.name, sizeof(ptp->info.name), "%pm",
> +		 ptp->dev->dev_addr);
> +
> +	clock = ptp_clock_register(&ptp->info, ptp->parent);
> +	if (IS_ERR(clock)) {
> +		netdev_err(ptp->dev, "failed to register PTP clock: %pe\n",
> +			   clock);
> +		return;
> +	}
> +	ptp->clock = clock;
> +
> +	rtl8169_ptp_hwtstamp_enable(ptp, false);

[Severity: Medium]
This is the only place the PTP block configuration is applied. It runs
once per ndo_open, after rtl8169_up() has already seeded the clock with
rtl8169_ptp_set_local_time().

On runtime or system resume, rtl8169_up() runs phy_init_hw(),
r8169_hw_phy_config() and genphy_soft_reset() again and re-seeds the
PHC time, but never re-applies this configuration. If the PHY reset or
power-down clears these registers, won't the hardware state after
resume differ from what was set up at open?

Later in the series, ptp->config keeps the user's hwtstamp setting, but
rtl8169_ptp_hwtstamp_set() only reprograms the hardware when the config
changes. Timestamping could therefore stay disabled after resume with
no error.

Separately, 0xa640 BIT(15) is set in both the enable and disable
branches of rtl8169_ptp_hwtstamp_enable(). Is there any path that clears
it?

> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109023022.770-1-javen_xu%40realsil.com.cn

Re: [PATCH net-next v1 3/3] r8169: add hardware timestamping support for RTL8127a

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 15 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 4 · Low: 9

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Timestamping is not stopped or restored on suspend, runtime
  suspend, resume or shutdown.
- [High] TX hardware timestamps are not tied to the skb that requested
  them.
- [Medium] The polling scheme for TX and RX timestamps is CPU-intensive
  and runs with IRQs off.
- [Medium] rtl8169_ptp_rx_tstamp() reads the PHY's per-message-type 'last
  received' latch when NAPI processes the skb, which can be well after…
- [Medium] Failed or all-ones register reads are treated as valid
  timestamps.
- [Medium] rtl8169_ptp_get_ts_info() reads ptp->clock and calls
  ptp_clock_index(ptp->clock) without RTNL or any driver lock.
- [Low] rtl8169_ptp_get_ts_info() and rtl8169_ptp_hwtstamp_set() disagree
  on the supported RX filters.
- [Low] The uAPI behaves inconsistently when no PHC is registered.
- [Low] The RX path stamps PTPv1 general messages, even though the driver
  only accepts PTPv2 filters and reports PTP_V2_EVENT.
- [Low] rtl8169_ptp_get_ts_stats() maps the driver counters onto
  ethtool_ts_stats fields in ways that contradict their kernel-doc.
- [Low] In rtl8169_start_xmit(), skb_tx_timestamp(skb) runs before
  rtl8169_ptp_tx_tstamp() sets SKBTX_IN_PROGRESS.
- [Low] rtl8169_ptp_tx_tstamp() runs in ndo_start_xmit with only the TX
  queue lock and reads ptp->config.tx_type with a plain load.
- [Low] ptp->tx_pkts and ptp->tx_timeouts are u64 counters incremented in
  rtl8169_ptp_tx_work(), and ptp->tx_skipped is incremented in…
- [Low] rtl8169_ptp_trx_ts_cmd() does a read-modify-write of
  PTP_TRX_TS_STA through rtl8169_ptp_modify().
- [Low] This patch makes rtl8169_ptp_hwtstamp_enable(true) reachable for
  the first time.

--

r8169: add hardware timestamping support for RTL8127a

This commit adds hardware timestamping of PTPv2 event messages on
RTL8127a. RX timestamps are read from per-message-type PHY latches in the
NAPI path. TX timestamps are handled one packet at a time and polled from
a work item with a 15 second timeout. The commit also wires up
ndo_hwtstamp_get/set, get_ts_info and get_ts_stats.

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index fc3cfa09a57e7..2ffb877ac2b49 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5146,6 +5146,10 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
>  
>  	skb_tx_timestamp(skb);
>  
> +	if (tp->ptp_support &&
> +	    unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
> +		rtl8169_ptp_tx_tstamp(&tp->ptp, skb);

[Severity: Low]
Should SKBTX_IN_PROGRESS be set before skb_tx_timestamp() runs here?

rtl8169_ptp_tx_tstamp() sets SKBTX_IN_PROGRESS only after
skb_tx_timestamp() has already produced the software timestamp. Because of
that, this check in __skb_tstamp_tx() can never suppress it:

    if (!hwtstamps && !(tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW) &&
        skb_shinfo(orig_skb)->tx_flags & SKBTX_IN_PROGRESS)
        return;

A socket that asks for both software and hardware TX timestamps without
SOF_TIMESTAMPING_OPT_TX_SWHW gets an extra software timestamp.
Documentation/networking/timestamping.rst says skb_tx_timestamp() gives a
software timestamp only when SKBTX_IN_PROGRESS is not set.

> +
>  	/* Force memory writes to complete before releasing descriptor */
>  	dma_wmb();

[ ... ]

> diff --git a/drivers/net/ethernet/realtek/r8169_ptp.c b/drivers/net/ethernet/realtek/r8169_ptp.c
> index 6201170e0c83e..d318025837612 100644
> --- a/drivers/net/ethernet/realtek/r8169_ptp.c
> +++ b/drivers/net/ethernet/realtek/r8169_ptp.c

[ ... ]

> @@ -136,6 +155,48 @@ static void rtl8169_ptp_write_time(struct rtl8169_ptp *ptp,
>  	rtl8169_ptp_write(ptp, PTP_CFG_TIME + PTP_TIME_S_HI, sec >> 32);
>  }
>  
> +/* Latch the RX or TX timestamp of the given message type into the
> + * PTP_TRX_TS_TIME registers.
> + */
> +static void rtl8169_ptp_trx_ts_cmd(struct rtl8169_ptp *ptp, u16 cmd)
> +{
> +	rtl8169_ptp_modify(ptp, PTP_TRX_TS_STA,
> +			   PTP_TRX_TS_SEL_RX | PTP_TRX_TS_MSGTYPE,
> +			   PTP_TRX_TS_RD | cmd);

[Severity: Low]
This read-modify-write of PTP_TRX_TS_STA masks only SEL_RX and MSGTYPE.
The TX_RDY status bits 15:12 are written back exactly as they were read.
What is the access type of those bits?

If they are write-1-to-clear, an RX timestamp read from NAPI could clear a
pending TX ready bit. That happens when the read falls between a TX
timestamp event and rtl8169_ptp_tx_work() consuming it.
rtl8169_ptp_tx_work() would then see PTP_INSR.TX_TS with no TX_RDY bit. It
would poll for 1ms with interrupts off and fall back to the SYNC latch.

If they are sticky read/write bits, writing 1 back means they never clear.
Ready bits then pile up and rtl8169_ptp_egress_time() falls into the
multi-bit default case.

> +	rtl8169_ptp_poll(ptp, PTP_TRX_TS_STA, PTP_TRX_TS_RD, false);
> +}
> +
> +static void rtl8169_ptp_egress_time(struct rtl8169_ptp *ptp,
> +				    struct timespec64 *ts)
> +{
> +	u8 msgtype;
> +	int val;
> +
> +	val = rtl8169_ptp_poll(ptp, PTP_TRX_TS_STA, PTP_TRX_TS_TX_RDY, true);
> +	if (val < 0)
> +		val = 0;

[Severity: Medium]
Can a failed or all-ones register read be delivered as a valid timestamp
here?

If the TX_RDY poll times out, val is forced to 0. The switch then falls
through to the SYNC latch, which is read and delivered anyway.

rtl8169_ptp_read() returns u16, so an -ETIMEDOUT from
r8168_phy_ocp_read() becomes 0xff92. Bits 15:12 are set in that value, so
this poll for PTP_TRX_TS_TX_RDY succeeds at once on a failed read.

rtl8169_ptp_trx_ts_cmd() ignores the result of the poll that waits for
PTP_TRX_TS_RD to clear. If the latch command never completes,
PTP_TRX_TS_TIME still holds the old contents. Both
rtl8169_ptp_rx_tstamp() and rtl8169_ptp_tx_work() then report those as
this packet's timestamp, and tx_pkts counts it as a success.

If the device is in D3 or has been removed from the bus,
r8168_phy_ocp_read() returns 0xffff. rtl8169_ptp_read_time() then builds
nsec = 0x3fffffff, which is above NSEC_PER_SEC and never range checked,
and sec = 2^48 - 1. timespec64_to_ktime() turns that into KTIME_MAX.

Should these error paths drop the timestamp and count the request as lost
or err instead?

> +
> +	switch (val & PTP_TRX_TS_TX_RDY) {

[ ... ]

> @@ -243,6 +304,201 @@ static const struct ptp_clock_info rtl8169_ptp_info = {

[ ... ]

> +static void rtl8169_ptp_tx_work(struct work_struct *work)
> +{

[ ... ]

> +	spin_lock_irqsave(&ptp->lock, flags);
> +	ready = rtl8169_ptp_read(ptp, PTP_INSR) & PTP_INT_TX_TS;
> +	if (ready) {
> +		rtl8169_ptp_write(ptp, PTP_INSR, PTP_INT_TX_TS);
> +		rtl8169_ptp_egress_time(ptp, &ts);
> +	}
> +	spin_unlock_irqrestore(&ptp->lock, flags);
> +
> +	if (!ready) {
> +		schedule_work(&ptp->tx_work);
> +		return;
> +	}

[Severity: Medium]
When the timestamp is not ready, this requeues the work at once with no
delay, until tx_start + 15 * HZ. Doesn't that turn every unanswered
request into a 15 second busy loop?

Each pass takes ptp->lock with spin_lock_irqsave() and does a GPHY_OCP
transaction, which can busy-wait up to 250us in rtl_phy_ocp_wait(). Some
requests never get an answer from the hardware, for example a non-PTP skb
or a frame dropped on link down. For those, a kworker keeps spinning for
the whole timeout and floods the OCP mailbox it shares with phylib.

Would a delayed_work, or ptp_schedule_worker() with do_aux_work and a poll
interval, bound this? The commit message only says the timestamp "is
polled from a work item".

The RX side has a similar cost. rtl8169_ptp_rx_tstamp() runs from rtl_rx()
for every frame that ptp_classify_raw() accepts with msgtype <= 3. Under
spin_lock_irqsave() it does:

- a read-modify-write of PTP_TRX_TS_STA
- a read_poll_timeout_atomic() of up to 1000us
- five time register reads

A remote sender decides whether a frame takes this path (L2 0x88F7, or UDP
319/320 with the right messageType). A low rate flood of minimal PTP event
frames can therefore burn a lot of interrupts-off time in softirq context.

[ ... ]

> +void rtl8169_ptp_tx_tstamp(struct rtl8169_ptp *ptp, struct sk_buff *skb)
> +{
> +	if (ptp->config.tx_type != HWTSTAMP_TX_ON ||
> +	    test_and_set_bit_lock(RTL8169_PTP_TX_IN_PROGRESS, &ptp->state)) {
> +		ptp->tx_skipped++;
> +		return;
> +	}

[Severity: Low]
ptp->config.tx_type is read here with a plain load from ndo_start_xmit,
which holds only the TX queue lock. The writers run under RTNL and use
plain stores: rtl8169_ptp_hwtstamp_set() through
hwtstamp_config_from_kernel(), rtl8169_ptp_init() and rtl8169_ptp_stop().

rx_enabled uses READ_ONCE()/WRITE_ONCE() in the same situation. Should
tx_type be annotated the same way, so KCSAN does not report it?

> +
> +	skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> +	ptp->tx_skb = skb_get(skb);
> +	ptp->tx_start = jiffies;
> +	schedule_work(&ptp->tx_work);
> +}

[Severity: High]
Is the TX hardware timestamp actually tied to the skb that asked for it?

This function takes the single slot for any skb with SKBTX_HW_TSTAMP set.
It never calls ptp_classify_raw()/ptp_parse_header() and never records the
message type.

The hardware only has TX latches and ready bits for the four PTP event
types (PTP_TRX_TS_TX_{SYNC,DLYREQ,PDLYREQ,PDLYRSP}_RDY). An skb that is not
a PTP event never gets a timestamp, but it holds the slot for up to 15
seconds. Examples are NTP or any UDP sent with
SOF_TIMESTAMPING_TX_HARDWARE, PTP general messages, and PTPv1.

rtl8169_ptp_tx_work() treats any PTP_INSR.TX_TS as completion of the
tracked skb. So the next PTP event frame that goes out completes the wrong
request. That could be ptp4l's Sync, which was itself counted as
tx_skipped because the slot was busy. Its timestamp goes to the other
socket, and ptp4l never gets one.

rtl8169_ptp_egress_time() also picks the latch from whichever TX_RDY bits
are set, not from the requesting skb. Any value other than exactly one bit
falls into the default case and reads the SYNC latch. TX_RDY is never
cleared before a new request.

On a config change, rtl8169_ptp_hwtstamp_set() ->
rtl8169_ptp_hwtstamp_enable(true) writes 0xffff to PTP_INSR. That can wipe
the status of an in-flight request.

SO_TIMESTAMPING with SOF_TIMESTAMPING_TX_HARDWARE needs no privilege. Once
TX_ON is enabled, any local user can keep the slot busy, so ptp4l times
out waiting for TX timestamps and faults.

Should the request be classified, and matched against the message type the
hardware reports?

> +
> +void rtl8169_ptp_rx_tstamp(struct rtl8169_ptp *ptp, struct sk_buff *skb)
> +{

[ ... ]

> +	/* Only event messages are timestamped by the hardware */
> +	msgtype = ptp_get_msgtype(hdr, ptp_class);
> +	if (msgtype > PTP_MSGTYPE_PDELAY_RESP)
> +		return;

[Severity: Low]
Should this also check ptp_class & PTP_CLASS_V2?

For V1 classes, ptp_get_msgtype() returns hdr->control. V1 Follow_Up
(control = 2) and Delay_Resp (control = 3) are general messages. They
still pass this check and are fed to PTP_TRX_TS_MSGTYPE as PDELAY_REQ and
PDELAY_RESP.

They then get the PDELAY latch of some unrelated v2 frame. That
contradicts both the reported PTP_V2_EVENT filter and the comment above.

> +
> +	spin_lock_irqsave(&ptp->lock, flags);
> +	rtl8169_ptp_trx_ts_cmd(ptp, PTP_TRX_TS_SEL_RX |
> +				    FIELD_PREP(PTP_TRX_TS_MSGTYPE, msgtype) |
> +				    PTP_TRX_TS_EXEC);
> +	rtl8169_ptp_read_time(ptp, PTP_TRX_TS_TIME, &ts);
> +	spin_unlock_irqrestore(&ptp->lock, flags);
> +
> +	skb_hwtstamps(skb)->hwtstamp = timespec64_to_ktime(ts);
> +}

[Severity: Medium]
Can this attach a timestamp that belongs to a different frame?

The latch holds the last received event message of each type. It is read
when NAPI processes the skb, which can be well after the frame arrived.
Nothing checks or acknowledges PTP_INSR.RX_TS. Nothing matches the latch
to this packet either, for example by sequenceId or sourcePortIdentity.

If two event messages of the same type arrive before NAPI handles the
first, the earlier skb gets the later frame's ingress time. That is normal
on a master or boundary clock receiving Delay_Req from several slaves, and
with P2P and several peers.

Frames the classifier accepts but the hardware did not stamp get whatever
stale value is in the latch, or zero right after enable. A network peer
can also send a same-type event message right after a legitimate one and
shift the timestamp attached to the legitimate frame.

The commit message says the PHY "latches the timestamp of the last
received event message per message type", but does not mention this
misattribution.

[ ... ]

> +int rtl8169_ptp_hwtstamp_set(struct rtl8169_ptp *ptp,
> +			     struct kernel_hwtstamp_config *config,
> +			     struct netlink_ext_ack *extack)
> +{
> +	bool enable = false;
> +	bool rx_enabled;
> +
> +	if (!ptp->clock)
> +		return -ENETDOWN;

[Severity: Low]
The PHC only exists between ndo_open and ndo_stop. It is also missing if
ptp_clock_register() failed in rtl8169_ptp_init(). In those cases this
returns -ENETDOWN.

Meanwhile rtl8169_get_ts_info() picks the hardware path based only on
tp->ptp_support. rtl8169_ptp_get_ts_info() then still advertises
SOF_TIMESTAMPING_{TX,RX,RAW}_HARDWARE, tx_types and rx_filters.

If registration failed while the interface is up, would -EOPNOTSUPP be the
better errno here?

rtl8169_ptp_stop() also resets ptp->config to HWTSTAMP_TX_OFF and
HWTSTAMP_FILTER_NONE on every close. That silently drops the timestamping
config across a down/up cycle. Is that intended? The commit message does
not mention it.

[ ... ]

> +	if (ptp->config.tx_type != config->tx_type ||
> +	    ptp->config.rx_filter != config->rx_filter) {
> +		hwtstamp_config_from_kernel(&ptp->config, config);
> +		rtl8169_ptp_hwtstamp_enable(ptp, enable);
> +	}

[Severity: Low]
This makes rtl8169_ptp_hwtstamp_enable(ptp, true) reachable for the first
time. In the parent commit, which added PHC support, it was only ever
called with false.

The enable branch sets PTP_INT_RX_TS and PTP_INT_TX_TS in PTP_INER and
sets PTP_GEN_CFG_INT_CLR_MODE, but nothing services those interrupts. The
MAC interrupt handlers only handle LinkChg -> phy_mac_interrupt().
PTP_INSR.RX_TS is never acknowledged; only TX_TS is ever written back.

If the internal PHY routes PTP_INSR into its interrupt output toward the
MAC, could an RX_TS that is never cleared cause spurious link change
interrupts and phylib state machine runs, or keep the line asserted? Since
the driver only polls, are these interrupt sources needed at all?

> +	WRITE_ONCE(ptp->rx_enabled, rx_enabled);
> +
> +	return 0;
> +}
> +
> +int rtl8169_ptp_get_ts_info(struct rtl8169_ptp *ptp,
> +			    struct kernel_ethtool_ts_info *info)
> +{
> +	info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE |
> +				SOF_TIMESTAMPING_TX_HARDWARE |
> +				SOF_TIMESTAMPING_RX_HARDWARE |
> +				SOF_TIMESTAMPING_RAW_HARDWARE;
> +
> +	if (ptp->clock)
> +		info->phc_index = ptp_clock_index(ptp->clock);

[Severity: Medium]
Can this race with rtl8169_ptp_stop() and read a freed ptp_clock?

get_ts_info can be reached without RTNL:

setsockopt(SO_TIMESTAMPING with SOF_TIMESTAMPING_BIND_PHC)
  sock_timestamping_bind_phc()
    ethtool_get_phc_vclocks()
      __ethtool_get_ts_info()
        rtl8169_get_ts_info()
          rtl8169_ptp_get_ts_info()

That path holds only rcu_read_lock() and a device reference. At the same
time, rtl8169_close() under RTNL can run rtl8169_ptp_stop():

	ptp_clock_unregister(ptp->clock);
	ptp->clock = NULL;

ptp_clock_unregister() can drop the last reference and free the ptp_clock
before the pointer is cleared with a plain store. A reader that has
already seen the non-NULL pointer would then read freed memory in
ptp_clock_index().

Before this patch the driver used ethtool_op_get_ts_info(), which never
touched ptp->clock.

> +
> +	info->tx_types = BIT(HWTSTAMP_TX_OFF) | BIT(HWTSTAMP_TX_ON);
> +	info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) |
> +			   BIT(HWTSTAMP_FILTER_PTP_V2_EVENT) |
> +			   BIT(HWTSTAMP_FILTER_PTP_V2_L4_EVENT) |
> +			   BIT(HWTSTAMP_FILTER_PTP_V2_SYNC) |
> +			   BIT(HWTSTAMP_FILTER_PTP_V2_L4_SYNC) |
> +			   BIT(HWTSTAMP_FILTER_PTP_V2_DELAY_REQ) |
> +			   BIT(HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ);

[Severity: Low]
Should the L2 filters be in this mask?

rtl8169_ptp_hwtstamp_set() accepts HWTSTAMP_FILTER_PTP_V2_L2_EVENT,
HWTSTAMP_FILTER_PTP_V2_L2_SYNC and HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ and
upgrades them to PTP_V2_EVENT. None of the three are advertised here, but
PTP_V2_EVENT, which covers both L2 and L4, is.

The RX path does stamp L2 PTP frames, since ptp_parse_header() handles
PTP_CLASS_L2. If PTP_CTL_TYPE_3 is in fact L4 only, should neither the L2
filters nor PTP_V2_EVENT be accepted or advertised?

> +
> +	return 0;
> +}
> +
> +void rtl8169_ptp_get_ts_stats(struct rtl8169_ptp *ptp,
> +			      struct ethtool_ts_stats *stats)
> +{
> +	stats->pkts = ptp->tx_pkts;
> +	stats->lost = ptp->tx_timeouts;
> +	stats->err = ptp->tx_skipped;
> +}

[Severity: Low]
Do these mappings match the ethtool_ts_stats kernel-doc?

@err is documented as timestamp generation error events that the hardware
encountered. But rtl8169_ptp_tx_tstamp() also increments tx_skipped when
ptp->config.tx_type != HWTSTAMP_TX_ON. That is a socket asking for a
hardware timestamp while TX timestamping is off, not a hardware error.

rtl8169_ptp_stop() can discard a pending request through
rtl8169_ptp_tx_complete(ptp, NULL). That fits the @lost definition
(timestamping information from the hardware never arrived), but
tx_timeouts is not incremented.

[Severity: Low]
tx_pkts and tx_timeouts are u64 counters incremented in
rtl8169_ptp_tx_work(). tx_skipped is incremented in
rtl8169_ptp_tx_tstamp() from ndo_start_xmit. All three are read here with
plain loads, without u64_stats_sync or a lock shared with the writers. Can
a reader see torn values on 32-bit architectures?

[ ... ]

> @@ -275,7 +533,13 @@ void rtl8169_ptp_stop(struct rtl8169_ptp *ptp)
>  	if (!ptp->clock)
>  		return;
>  
> +	ptp->config.tx_type = HWTSTAMP_TX_OFF;
> +	ptp->config.rx_filter = HWTSTAMP_FILTER_NONE;
> +	WRITE_ONCE(ptp->rx_enabled, false);
>  	rtl8169_ptp_hwtstamp_enable(ptp, false);
> +	cancel_work_sync(&ptp->tx_work);
> +	if (ptp->tx_skb)
> +		rtl8169_ptp_tx_complete(ptp, NULL);
>  
>  	ptp_clock_unregister(ptp->clock);
>  	ptp->clock = NULL;

[Severity: High]
This is the only place where tx_work is cancelled and a pending tx_skb is
released, and it is only reached from ndo_stop. What happens across
suspend, runtime suspend, resume and shutdown?

rtl8169_suspend(), rtl8169_runtime_suspend() and rtl_shutdown() all go
through rtl8169_net_suspend() -> rtl8169_down(), which does:

	disable_work_sync(&tp->wk.work);
	...
	pci_clear_master(tp->pci_dev);
	...
	rtl_prepare_power_down(tp);

That path never cancels ptp->tx_work and never frees ptp->tx_skb. tx_work
requeues itself on system_percpu_wq for up to 15 seconds, so it keeps
doing GPHY_OCP accesses after the device has been powered down.

With the interface up, carrier loss calls pm_runtime_idle(). Then
rtl8169_runtime_idle() schedules suspend 10 seconds later, which is inside
RTL8169_PTP_TX_TIMEOUT:

	if (!netif_running(tp->dev) || !netif_carrier_ok(tp->dev))
		pm_schedule_suspend(device, 10000);

A request still pending at that point leaves tx_work polling a device in
D3hot. The reads return all-ones, so this sequence follows:

- PTP_INSR & PTP_INT_TX_TS looks ready
- the TX_RDY poll passes
- rtl8169_ptp_read_time() builds a KTIME_MAX timestamp
- skb_tstamp_tx() hands it to user space and tx_pkts counts it

On resume, rtl8169_up() goes through D3 -> D0, rtl8169_init_phy() (which
includes a PHY soft reset) and rtl8169_ptp_set_local_time(). It never
replays rtl8169_ptp_hwtstamp_enable() from ptp->config. If PTP_CTL and
PTP_INER do not survive that, ptp->config and rx_enabled still say
timestamping is on while the hardware has it off.

rtl8169_ptp_hwtstamp_set() only reprograms the hardware when the config
changes:

	if (ptp->config.tx_type != config->tx_type ||
	    ptp->config.rx_filter != config->rx_filter) {

So re-sending the same SIOCSHWTSTAMP config, as ptp4l does during fault
recovery, does nothing. Doesn't timestamping then stay broken until it is
switched off and back on, or the interface is cycled?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109023022.770-1-javen_xu%40realsil.com.cn