[RFC Patch net-next v2 0/8] net: dsa: microchip: add PTP support for KSZ9x and LAN937x

[RFC Patch net-next v2 0/8] net: dsa: microchip: add PTP support for KSZ9x and LAN937x

[Arun Ramadoss]

The LAN937x switch has capable for supporting IEEE 1588 PTP protocol. This
patch series add PTP support and tested using the ptp4l application.
LAN937x has the same PTP register set similar to KSZ9563, hence the
implementation has been made common for the ksz switches.
KSZ9563 does not support two step timestamping but LAN937x supports both.
Tested the 1step & 2step p2p timestamping in LAN937x and p2p1step
timestamping in KSZ9563.

RFC v1 -> v2
- Added the p2p1step timestamping and conditional execution of 2 step for
  LAN937x only.
- Added the periodic output support

Arun Ramadoss (7):
  net: dsa: microchip: adding the posix clock support
  net: dsa: microchip: Initial hardware time stamping support
  net: dsa: microchip: Manipulating absolute time using ptp hw clock
  net: dsa: microchip: enable the ptp interrupt for timestamping
  net: dsa: microchip: Adding the ptp packet reception logic
  net: dsa: microchip: add the transmission tstamp logic
  net: dsa: microchip: ptp: add periodic output signal

Christian Eggers (1):
  net: ptp: add helper for one-step P2P clocks

 drivers/net/dsa/microchip/Kconfig       |   12 +
 drivers/net/dsa/microchip/Makefile      |    5 +
 drivers/net/dsa/microchip/ksz_common.c  |   44 +-
 drivers/net/dsa/microchip/ksz_common.h  |   48 +
 drivers/net/dsa/microchip/ksz_ptp.c     | 1117 +++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_ptp.h     |   96 ++
 drivers/net/dsa/microchip/ksz_ptp_reg.h |  136 +++
 include/linux/dsa/ksz_common.h          |   55 ++
 include/linux/ptp_classify.h            |   73 ++
 net/dsa/tag_ksz.c                       |  288 +++++-
 10 files changed, 1859 insertions(+), 15 deletions(-)
 create mode 100644 drivers/net/dsa/microchip/ksz_ptp.c
 create mode 100644 drivers/net/dsa/microchip/ksz_ptp.h
 create mode 100644 drivers/net/dsa/microchip/ksz_ptp_reg.h
 create mode 100644 include/linux/dsa/ksz_common.h

-- 
2.36.1

[RFC Patch net-next v2 1/8] net: ptp: add helper for one-step P2P clocks

[Arun Ramadoss]

From: Christian Eggers <ceggers@arri.de>

For P2P delay measurement, the ingress time stamp of the PDelay_Req is
required for the correction field of the PDelay_Resp. The application
echoes back the correction field of the PDelay_Req when sending the
PDelay_Resp.

Some hardware (like the ZHAW InES PTP time stamping IP core) subtracts
the ingress timestamp autonomously from the correction field, so that
the hardware only needs to add the egress timestamp on tx. Other
hardware (like the Microchip KSZ9563) reports the ingress time stamp via
an interrupt and requires that the software provides this time stamp via
tail-tag on tx.

In order to avoid introducing a further application interface for this,
the driver can simply emulate the behavior of the InES device and
subtract the ingress time stamp in software from the correction field.

On egress, the correction field can either be kept as it is (and the
time stamp field in the tail-tag is set to zero) or move the value from
the correction field back to the tail-tag.

Changing the correction field requires updating the UDP checksum (if UDP
is used as transport).

Signed-off-by: Christian Eggers <ceggers@arri.de>
---
 include/linux/ptp_classify.h | 73 ++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/include/linux/ptp_classify.h b/include/linux/ptp_classify.h
index 2b6ea36ad162..e32efe3c4d66 100644
--- a/include/linux/ptp_classify.h
+++ b/include/linux/ptp_classify.h
@@ -10,8 +10,12 @@
 #ifndef _PTP_CLASSIFY_H_
 #define _PTP_CLASSIFY_H_
 
+#include <asm/unaligned.h>
 #include <linux/ip.h>
+#include <linux/ktime.h>
 #include <linux/skbuff.h>
+#include <linux/udp.h>
+#include <net/checksum.h>
 
 #define PTP_CLASS_NONE  0x00 /* not a PTP event message */
 #define PTP_CLASS_V1    0x01 /* protocol version 1 */
@@ -129,6 +133,67 @@ static inline u8 ptp_get_msgtype(const struct ptp_header *hdr,
 	return msgtype;
 }
 
+/**
+ * ptp_check_diff8 - Computes new checksum (when altering a 64-bit field)
+ * @old: old field value
+ * @new: new field value
+ * @oldsum: previous checksum
+ *
+ * This function can be used to calculate a new checksum when only a single
+ * field is changed. Similar as ip_vs_check_diff*() in ip_vs.h.
+ *
+ * Return: Updated checksum
+ */
+static inline __wsum ptp_check_diff8(__be64 old, __be64 new, __wsum oldsum)
+{
+	__be64 diff[2] = { ~old, new };
+
+	return csum_partial(diff, sizeof(diff), oldsum);
+}
+
+/**
+ * ptp_header_update_correction - Update PTP header's correction field
+ * @skb: packet buffer
+ * @type: type of the packet (see ptp_classify_raw())
+ * @hdr: ptp header
+ * @correction: new correction value
+ *
+ * This updates the correction field of a PTP header and updates the UDP
+ * checksum (if UDP is used as transport). It is needed for hardware capable of
+ * one-step P2P that does not already modify the correction field of Pdelay_Req
+ * event messages on ingress.
+ */
+static inline
+void ptp_header_update_correction(struct sk_buff *skb, unsigned int type,
+				  struct ptp_header *hdr, s64 correction)
+{
+	__be64 correction_old;
+	struct udphdr *uhdr;
+
+	/* previous correction value is required for checksum update. */
+	memcpy(&correction_old,  &hdr->correction, sizeof(correction_old));
+
+	/* write new correction value */
+	put_unaligned_be64((u64)correction, &hdr->correction);
+
+	switch (type & PTP_CLASS_PMASK) {
+	case PTP_CLASS_IPV4:
+	case PTP_CLASS_IPV6:
+		/* locate udp header */
+		uhdr = (struct udphdr *)((char *)hdr - sizeof(struct udphdr));
+		break;
+	default:
+		return;
+	}
+
+	/* update checksum */
+	uhdr->check = csum_fold(ptp_check_diff8(correction_old,
+						hdr->correction,
+						~csum_unfold(uhdr->check)));
+	if (!uhdr->check)
+		uhdr->check = CSUM_MANGLED_0;
+}
+
 /**
  * ptp_msg_is_sync - Evaluates whether the given skb is a PTP Sync message
  * @skb: packet buffer
@@ -166,5 +231,13 @@ static inline bool ptp_msg_is_sync(struct sk_buff *skb, unsigned int type)
 {
 	return false;
 }
+
+static inline
+void ptp_onestep_p2p_move_t2_to_correction(struct sk_buff *skb,
+					   unsigned int type,
+					   struct ptp_header *hdr,
+					   ktime_t t2)
+{
+}
 #endif
 #endif /* _PTP_CLASSIFY_H_ */
-- 
2.36.1

Re: [RFC Patch net-next v2 1/8] net: ptp: add helper for one-step P2P clocks

[Richard Cochran]

On Mon, Nov 21, 2022 at 09:11:43PM +0530, Arun Ramadoss wrote:
> +/**
> + * ptp_header_update_correction - Update PTP header's correction field
> + * @skb: packet buffer
> + * @type: type of the packet (see ptp_classify_raw())
> + * @hdr: ptp header
> + * @correction: new correction value
> + *
> + * This updates the correction field of a PTP header and updates the UDP
> + * checksum (if UDP is used as transport). It is needed for hardware capable of
> + * one-step P2P that does not already modify the correction field of Pdelay_Req
> + * event messages on ingress.
> + */

Does this really belong in the common PTP header?

Seems more like a driver/hardware specific workaround to me.

Thanks,
Richard

Re: [RFC Patch net-next v2 1/8] net: ptp: add helper for one-step P2P clocks

[Arun.Ramadoss]

Hi Richard,
Thanks for the comment.

On Tue, 2022-11-22 at 06:34 -0800, Richard Cochran wrote:
> [Some people who received this message don't often get email from
> richardcochran@gmail.com. Learn why this is important at 
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Mon, Nov 21, 2022 at 09:11:43PM +0530, Arun Ramadoss wrote:
> > +/**
> > + * ptp_header_update_correction - Update PTP header's correction
> > field
> > + * @skb: packet buffer
> > + * @type: type of the packet (see ptp_classify_raw())
> > + * @hdr: ptp header
> > + * @correction: new correction value
> > + *
> > + * This updates the correction field of a PTP header and updates
> > the UDP
> > + * checksum (if UDP is used as transport). It is needed for
> > hardware capable of
> > + * one-step P2P that does not already modify the correction field
> > of Pdelay_Req
> > + * event messages on ingress.
> > + */
> 
> Does this really belong in the common PTP header?
> 
> Seems more like a driver/hardware specific workaround to me.

This patch series is extension of PTP support for KSZ9563 patch series
submitted two years back which is not mainlined.
In that patch review feedback, it was suggested to make this function
generic and so it was moved from ksz_common.h to ptp_classify.h

Link: 
https://lore.kernel.org/netdev/20201022113243.4shddtywgvpcqq6c@skbuf/


https://lore.kernel.org/netdev/20201022143429.GA9743@hoboy.vegasvil.org/

> 
> Thanks,
> Richard

Re: [RFC Patch net-next v2 1/8] net: ptp: add helper for one-step P2P clocks

[Richard Cochran]

On Wed, Nov 23, 2022 at 07:10:58AM +0000, Arun.Ramadoss@microchip.com wrote:
> This patch series is extension of PTP support for KSZ9563 patch series
> submitted two years back which is not mainlined.
> In that patch review feedback, it was suggested to make this function
> generic and so it was moved from ksz_common.h to ptp_classify.h
> 
> Link: 
> https://lore.kernel.org/netdev/20201022113243.4shddtywgvpcqq6c@skbuf/
> 
> 
> https://lore.kernel.org/netdev/20201022143429.GA9743@hoboy.vegasvil.org/

okay

Thanks,
Richard

[RFC Patch net-next v2 2/8] net: dsa: microchip: adding the posix clock support

[Arun Ramadoss]

This patch implement routines (adjfine, adjtime, gettime and settime)
for manipulating the chip's PTP clock. It registers the ptp caps
to posix clock register.

Signed-off-by: Christian Eggers <ceggers@arri.de>
Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/Kconfig       |  12 ++
 drivers/net/dsa/microchip/Makefile      |   5 +
 drivers/net/dsa/microchip/ksz_common.c  |  14 +-
 drivers/net/dsa/microchip/ksz_common.h  |  17 ++
 drivers/net/dsa/microchip/ksz_ptp.c     | 270 ++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_ptp.h     |  43 ++++
 drivers/net/dsa/microchip/ksz_ptp_reg.h |  52 +++++
 7 files changed, 412 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/dsa/microchip/ksz_ptp.c
 create mode 100644 drivers/net/dsa/microchip/ksz_ptp.h
 create mode 100644 drivers/net/dsa/microchip/ksz_ptp_reg.h

diff --git a/drivers/net/dsa/microchip/Kconfig b/drivers/net/dsa/microchip/Kconfig
index 06b1efdb5e7d..a1a3a04d0ea2 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -10,6 +10,7 @@ menuconfig NET_DSA_MICROCHIP_KSZ_COMMON
 config NET_DSA_MICROCHIP_KSZ9477_I2C
 	tristate "KSZ series I2C connected switch driver"
 	depends on NET_DSA_MICROCHIP_KSZ_COMMON && I2C
+	depends on PTP_1588_CLOCK_OPTIONAL
 	select REGMAP_I2C
 	help
 	  Select to enable support for registering switches configured through I2C.
@@ -17,10 +18,21 @@ config NET_DSA_MICROCHIP_KSZ9477_I2C
 config NET_DSA_MICROCHIP_KSZ_SPI
 	tristate "KSZ series SPI connected switch driver"
 	depends on NET_DSA_MICROCHIP_KSZ_COMMON && SPI
+	depends on PTP_1588_CLOCK_OPTIONAL
 	select REGMAP_SPI
 	help
 	  Select to enable support for registering switches configured through SPI.
 
+config NET_DSA_MICROCHIP_KSZ_PTP
+	bool "Support for the PTP clock on the KSZ9563/LAN937x Ethernet Switch"
+	depends on NET_DSA_MICROCHIP_KSZ_COMMON && PTP_1588_CLOCK
+	help
+	  This enables support for timestamping & PTP clock manipulation
+	  in the KSZ9563/LAN937x Ethernet switch
+
+	  Select to enable support for PTP feature for KSZ9563/lan937x series
+	  of switch.
+
 config NET_DSA_MICROCHIP_KSZ8863_SMI
 	tristate "KSZ series SMI connected switch driver"
 	depends on NET_DSA_MICROCHIP_KSZ_COMMON
diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile
index 28873559efc2..48360cc9fc68 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -4,6 +4,11 @@ ksz_switch-objs := ksz_common.o
 ksz_switch-objs += ksz9477.o
 ksz_switch-objs += ksz8795.o
 ksz_switch-objs += lan937x_main.o
+
+ifdef CONFIG_NET_DSA_MICROCHIP_KSZ_PTP
+ksz_switch-objs += ksz_ptp.o
+endif
+
 obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_I2C)	+= ksz9477_i2c.o
 obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ_SPI)		+= ksz_spi.o
 obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ8863_SMI)	+= ksz8863_smi.o
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 8c8db315317d..eb77eca0dcb2 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -27,6 +27,7 @@
 #include "ksz8.h"
 #include "ksz9477.h"
 #include "lan937x.h"
+#include "ksz_ptp.h"
 
 #define MIB_COUNTER_NUM 0x20
 
@@ -2016,10 +2017,16 @@ static int ksz_setup(struct dsa_switch *ds)
 		}
 	}
 
+	ret = ksz_ptp_clock_register(ds);
+	if (ret) {
+		dev_err(dev->dev, "Failed to register PTP clock: %d\n", ret);
+		goto out_pirq;
+	}
+
 	ret = ksz_mdio_register(dev);
 	if (ret < 0) {
 		dev_err(dev->dev, "failed to register the mdio");
-		goto out_pirq;
+		goto out_ptp_clock_unregister;
 	}
 
 	/* start switch */
@@ -2028,6 +2035,8 @@ static int ksz_setup(struct dsa_switch *ds)
 
 	return 0;
 
+out_ptp_clock_unregister:
+	ksz_ptp_clock_unregister(ds);
 out_pirq:
 	if (dev->irq > 0)
 		dsa_switch_for_each_user_port(dp, dev->ds)
@@ -2044,6 +2053,8 @@ static void ksz_teardown(struct dsa_switch *ds)
 	struct ksz_device *dev = ds->priv;
 	struct dsa_port *dp;
 
+	ksz_ptp_clock_unregister(ds);
+
 	if (dev->irq > 0) {
 		dsa_switch_for_each_user_port(dp, dev->ds)
 			ksz_irq_free(&dev->ports[dp->index].pirq);
@@ -2861,6 +2872,7 @@ static const struct dsa_switch_ops ksz_switch_ops = {
 	.get_pause_stats	= ksz_get_pause_stats,
 	.port_change_mtu	= ksz_change_mtu,
 	.port_max_mtu		= ksz_max_mtu,
+	.get_ts_info            = ksz_get_ts_info,
 };
 
 struct ksz_device *ksz_switch_alloc(struct device *base, void *priv)
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index c6726cbd5465..767f17d2c75d 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -14,6 +14,9 @@
 #include <linux/regmap.h>
 #include <net/dsa.h>
 #include <linux/irq.h>
+#include <linux/ptp_clock_kernel.h>
+
+#include "ksz_ptp.h"
 
 #define KSZ_MAX_NUM_PORTS 8
 
@@ -141,6 +144,7 @@ struct ksz_device {
 	u16 port_mask;
 	struct mutex lock_irq;		/* IRQ Access */
 	struct ksz_irq girq;
+	struct ksz_ptp_data ptp_data;
 };
 
 /* List of supported models */
@@ -444,6 +448,19 @@ static inline int ksz_write32(struct ksz_device *dev, u32 reg, u32 value)
 	return ret;
 }
 
+static inline int ksz_rmw16(struct ksz_device *dev, u32 reg, u16 mask,
+			    u16 value)
+{
+	int ret;
+
+	ret = regmap_update_bits(dev->regmap[1], reg, mask, value);
+	if (ret)
+		dev_err(dev->dev, "can't rmw 16bit reg: 0x%x %pe\n", reg,
+			ERR_PTR(ret));
+
+	return ret;
+}
+
 static inline int ksz_write64(struct ksz_device *dev, u32 reg, u64 value)
 {
 	u32 val[2];
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
new file mode 100644
index 000000000000..cad0c6087419
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -0,0 +1,270 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Microchip LAN937X PTP Implementation
+ * Copyright (C) 2021-2022 Microchip Technology Inc.
+ */
+
+#include <linux/kernel.h>
+#include <linux/ptp_classify.h>
+#include <linux/ptp_clock_kernel.h>
+
+#include "ksz_common.h"
+#include "ksz_ptp.h"
+#include "ksz_ptp_reg.h"
+
+#define ptp_caps_to_data(d) \
+		container_of((d), struct ksz_ptp_data, caps)
+#define ptp_data_to_ksz_dev(d) \
+		container_of((d), struct ksz_device, ptp_data)
+
+#define MAX_DRIFT_CORR 6250000
+
+#define KSZ_PTP_INC_NS 40  /* HW clock is incremented every 40 ns (by 40) */
+#define KSZ_PTP_SUBNS_BITS 32  /* Number of bits in sub-nanoseconds counter */
+
+/* The function is return back the capability of timestamping feature when
+ * requested through ethtool -T <interface> utility
+ */
+int ksz_get_ts_info(struct dsa_switch *ds, int port, struct ethtool_ts_info *ts)
+{
+	struct ksz_device *dev	= ds->priv;
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+
+	ts->so_timestamping = SOF_TIMESTAMPING_TX_HARDWARE |
+			      SOF_TIMESTAMPING_RX_HARDWARE |
+			      SOF_TIMESTAMPING_RAW_HARDWARE;
+
+	ts->tx_types = (1 << HWTSTAMP_TX_OFF);
+
+	ts->rx_filters = (1 << HWTSTAMP_FILTER_NONE);
+
+	ts->phc_index = ptp_clock_index(ptp_data->clock);
+
+	return 0;
+}
+
+/* These are function related to the ptp clock info */
+static int _ksz_ptp_gettime(struct ksz_device *dev, struct timespec64 *ts)
+{
+	u32 nanoseconds;
+	u32 seconds;
+	u8 phase;
+	int ret;
+
+	/* Copy current PTP clock into shadow registers */
+	ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_READ_TIME, PTP_READ_TIME);
+	if (ret)
+		return ret;
+
+	/* Read from shadow registers */
+	ret = ksz_read8(dev, REG_PTP_RTC_SUB_NANOSEC__2, &phase);
+	if (ret)
+		return ret;
+
+	ret = ksz_read32(dev, REG_PTP_RTC_NANOSEC, &nanoseconds);
+	if (ret)
+		return ret;
+
+	ret = ksz_read32(dev, REG_PTP_RTC_SEC, &seconds);
+	if (ret)
+		return ret;
+
+	ts->tv_sec = seconds;
+	ts->tv_nsec = nanoseconds + phase * 8;
+
+	return 0;
+}
+
+static int ksz_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+	ret = _ksz_ptp_gettime(dev, ts);
+	mutex_unlock(&ptp_data->lock);
+
+	return ret;
+}
+
+static int ksz_ptp_settime(struct ptp_clock_info *ptp,
+			   const struct timespec64 *ts)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	/* Write to shadow registers */
+
+	/* Write 0 to clock phase */
+	ret = ksz_write16(dev, REG_PTP_RTC_SUB_NANOSEC__2, PTP_RTC_0NS);
+	if (ret)
+		goto error_return;
+
+	/* nanoseconds */
+	ret = ksz_write32(dev, REG_PTP_RTC_NANOSEC, ts->tv_nsec);
+	if (ret)
+		goto error_return;
+
+	/* seconds */
+	ret = ksz_write32(dev, REG_PTP_RTC_SEC, ts->tv_sec);
+	if (ret)
+		goto error_return;
+
+	/* Load PTP clock from shadow registers */
+	ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_LOAD_TIME, PTP_LOAD_TIME);
+
+error_return:
+	mutex_unlock(&ptp_data->lock);
+
+	return ret;
+}
+
+static int ksz_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	if (scaled_ppm) {
+		s64 ppb, adj;
+		u32 data32;
+
+		/* see scaled_ppm_to_ppb() in ptp_clock.c for details */
+		ppb = 1 + scaled_ppm;
+		ppb *= 125;
+		ppb *= KSZ_PTP_INC_NS;
+		ppb <<= KSZ_PTP_SUBNS_BITS - 13;
+		adj = div_s64(ppb, NSEC_PER_SEC);
+
+		data32 = abs(adj);
+		data32 &= PTP_SUBNANOSEC_M;
+		if (adj >= 0)
+			data32 |= PTP_RATE_DIR;
+
+		ret = ksz_write32(dev, REG_PTP_SUBNANOSEC_RATE, data32);
+		if (ret)
+			goto error_return;
+
+		ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ADJ_ENABLE,
+				PTP_CLK_ADJ_ENABLE);
+		if (ret)
+			goto error_return;
+	} else {
+		ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ADJ_ENABLE, 0);
+		if (ret)
+			goto error_return;
+	}
+
+error_return:
+	mutex_unlock(&ptp_data->lock);
+	return ret;
+}
+
+static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	s32 sec, nsec;
+	u16 data16;
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	/* do not use ns_to_timespec64(),
+	 * both sec and nsec are subtracted by hw
+	 */
+	sec = div_s64_rem(delta, NSEC_PER_SEC, &nsec);
+
+	ret = ksz_write32(dev, REG_PTP_RTC_NANOSEC, abs(nsec));
+	if (ret)
+		goto error_return;
+
+	ret = ksz_write32(dev, REG_PTP_RTC_SEC, abs(sec));
+	if (ret)
+		goto error_return;
+
+	ret = ksz_read16(dev, REG_PTP_CLK_CTRL, &data16);
+	if (ret)
+		goto error_return;
+
+	data16 |= PTP_STEP_ADJ;
+
+	/*PTP_STEP_DIR -- 0: subtract, 1: add */
+	if (delta < 0)
+		data16 &= ~PTP_STEP_DIR;
+	else
+		data16 |= PTP_STEP_DIR;
+
+	ret = ksz_write16(dev, REG_PTP_CLK_CTRL, data16);
+
+error_return:
+	mutex_unlock(&ptp_data->lock);
+	return ret;
+}
+
+static int ksz_ptp_start_clock(struct ksz_device *dev)
+{
+	return ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE, PTP_CLK_ENABLE);
+}
+
+static const struct ptp_clock_info ksz_ptp_caps = {
+	.owner		= THIS_MODULE,
+	.name		= "Microchip Clock",
+	.max_adj	= MAX_DRIFT_CORR,
+	.gettime64	= ksz_ptp_gettime,
+	.settime64	= ksz_ptp_settime,
+	.adjfine	= ksz_ptp_adjfine,
+	.adjtime	= ksz_ptp_adjtime,
+};
+
+int ksz_ptp_clock_register(struct dsa_switch *ds)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	int ret;
+
+	mutex_init(&ptp_data->lock);
+
+	ptp_data->caps = ksz_ptp_caps;
+
+	/* Start hardware counter */
+	ret = ksz_ptp_start_clock(dev);
+	if (ret)
+		return ret;
+
+	/* Register the PTP Clock */
+	ptp_data->clock = ptp_clock_register(&ptp_data->caps, dev->dev);
+	if (IS_ERR_OR_NULL(ptp_data->clock))
+		return PTR_ERR(ptp_data->clock);
+
+	ret = ksz_rmw16(dev, REG_PTP_MSG_CONF1, PTP_802_1AS, PTP_802_1AS);
+	if (ret)
+		goto error_unregister_clock;
+
+	return 0;
+
+error_unregister_clock:
+	ptp_clock_unregister(ptp_data->clock);
+	return ret;
+}
+
+void ksz_ptp_clock_unregister(struct dsa_switch *ds)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+
+	if (IS_ERR_OR_NULL(ptp_data->clock))
+		return;
+
+	ptp_clock_unregister(ptp_data->clock);
+}
+
+MODULE_AUTHOR("Christian Eggers <ceggers@arri.de>");
+MODULE_AUTHOR("Arun Ramadoss <arun.ramadoss@microchip.com>");
+MODULE_DESCRIPTION("PTP support for KSZ switch");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
new file mode 100644
index 000000000000..ac53b0df2733
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Microchip LAN937X PTP Implementation
+ * Copyright (C) 2020-2021 Microchip Technology Inc.
+ */
+
+#ifndef _NET_DSA_DRIVERS_KSZ_PTP_H
+#define _NET_DSA_DRIVERS_KSZ_PTP_H
+
+#if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ_PTP)
+
+struct ksz_ptp_data {
+	struct ptp_clock_info caps;
+	struct ptp_clock *clock;
+	/* Serializes all operations on the PTP hardware clock */
+	struct mutex lock;
+};
+
+int ksz_ptp_clock_register(struct dsa_switch *ds);
+
+void ksz_ptp_clock_unregister(struct dsa_switch *ds);
+
+int ksz_get_ts_info(struct dsa_switch *ds, int port,
+		    struct ethtool_ts_info *ts);
+
+#else
+
+struct ksz_ptp_data {
+	/* Serializes all operations on the PTP hardware clock */
+	struct mutex lock;
+};
+
+static inline int ksz_ptp_clock_register(struct dsa_switch *ds)
+{
+	return 0;
+}
+
+static inline void ksz_ptp_clock_unregister(struct dsa_switch *ds) { }
+
+#define ksz_get_ts_info NULL
+
+#endif	/* End of CONFIG_NET_DSA_MICROCHIOP_KSZ_PTP */
+
+#endif
diff --git a/drivers/net/dsa/microchip/ksz_ptp_reg.h b/drivers/net/dsa/microchip/ksz_ptp_reg.h
new file mode 100644
index 000000000000..2bf8395475b9
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz_ptp_reg.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Microchip KSZ PTP register definitions
+ * Copyright (C) 2019-2021 Microchip Technology Inc.
+ */
+
+/* 5 - PTP Clock */
+#define REG_PTP_CLK_CTRL		0x0500
+
+#define PTP_STEP_ADJ			BIT(6)
+#define PTP_STEP_DIR			BIT(5)
+#define PTP_READ_TIME			BIT(4)
+#define PTP_LOAD_TIME			BIT(3)
+#define PTP_CLK_ADJ_ENABLE		BIT(2)
+#define PTP_CLK_ENABLE			BIT(1)
+#define PTP_CLK_RESET			BIT(0)
+
+#define REG_PTP_RTC_SUB_NANOSEC__2	0x0502
+
+#define PTP_RTC_SUB_NANOSEC_M		0x0007
+#define PTP_RTC_0NS			0x00
+
+#define REG_PTP_RTC_NANOSEC		0x0504
+#define REG_PTP_RTC_NANOSEC_H		0x0504
+#define REG_PTP_RTC_NANOSEC_L		0x0506
+
+#define REG_PTP_RTC_SEC			0x0508
+#define REG_PTP_RTC_SEC_H		0x0508
+#define REG_PTP_RTC_SEC_L		0x050A
+
+#define REG_PTP_SUBNANOSEC_RATE		0x050C
+#define REG_PTP_SUBNANOSEC_RATE_H	0x050C
+#define PTP_SUBNANOSEC_M		0x3FFFFFFF
+
+#define PTP_RATE_DIR			BIT(31)
+#define PTP_TMP_RATE_ENABLE		BIT(30)
+
+#define REG_PTP_SUBNANOSEC_RATE_L	0x050E
+
+#define REG_PTP_RATE_DURATION		0x0510
+#define REG_PTP_RATE_DURATION_H		0x0510
+#define REG_PTP_RATE_DURATION_L		0x0512
+
+#define REG_PTP_MSG_CONF1		0x0514
+
+#define PTP_802_1AS			BIT(7)
+#define PTP_ENABLE			BIT(6)
+#define PTP_ETH_ENABLE			BIT(5)
+#define PTP_IPV4_UDP_ENABLE		BIT(4)
+#define PTP_IPV6_UDP_ENABLE		BIT(3)
+#define PTP_TC_P2P			BIT(2)
+#define PTP_MASTER			BIT(1)
+#define PTP_1STEP			BIT(0)
-- 
2.36.1

Re: [RFC Patch net-next v2 2/8] net: dsa: microchip: adding the posix clock support

[Vladimir Oltean]

On Mon, Nov 21, 2022 at 09:11:44PM +0530, Arun Ramadoss wrote:
> +int ksz_ptp_clock_register(struct dsa_switch *ds)
> +{
> +	/* Register the PTP Clock */
> +	ptp_data->clock = ptp_clock_register(&ptp_data->caps, dev->dev);
> +	if (IS_ERR_OR_NULL(ptp_data->clock))
> +		return PTR_ERR(ptp_data->clock);
> +}
> +
> +void ksz_ptp_clock_unregister(struct dsa_switch *ds)
> +{
> +	struct ksz_device *dev = ds->priv;
> +	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +
> +	if (IS_ERR_OR_NULL(ptp_data->clock))
> +		return;
> +
> +	ptp_clock_unregister(ptp_data->clock);
> +}

API usage seems to be incorrect here (probably copied from sja1105 which
is written by me and also incorrect, yay).

The intention with IS_ERR_OR_NULL() is for the caller to return 0
(success) when ptp_clock_register() returns NULL (when PTP support
is compiled out), and this will not make the driver fail to probe.

There isn't a reason to use IS_ERR_OR_NULL() in the normal unregister
code path, because the code won't get there in the IS_ERR() case.
So a simple "if (ptp_data->clock) ptp_clock_unregister(ptp_data->clock)"
would do.

Re: [RFC Patch net-next v2 2/8] net: dsa: microchip: adding the posix clock support

[Vladimir Oltean]

On Mon, Nov 21, 2022 at 09:11:44PM +0530, Arun Ramadoss wrote:
> @@ -17,10 +18,21 @@ config NET_DSA_MICROCHIP_KSZ9477_I2C
>  config NET_DSA_MICROCHIP_KSZ_SPI
>  	tristate "KSZ series SPI connected switch driver"
>  	depends on NET_DSA_MICROCHIP_KSZ_COMMON && SPI
> +	depends on PTP_1588_CLOCK_OPTIONAL
>  	select REGMAP_SPI
>  	help
>  	  Select to enable support for registering switches configured through SPI.
>  
> +config NET_DSA_MICROCHIP_KSZ_PTP
> +	bool "Support for the PTP clock on the KSZ9563/LAN937x Ethernet Switch"
> +	depends on NET_DSA_MICROCHIP_KSZ_COMMON && PTP_1588_CLOCK
> +	help
> +	  This enables support for timestamping & PTP clock manipulation

Please use "and" instead of "&".

> +	  in the KSZ9563/LAN937x Ethernet switch
> +
> +	  Select to enable support for PTP feature for KSZ9563/lan937x series

Please capitalize both KSZ9563 and LAN937X. This help text is the
business card of the feature, you need it to look nice and shiny.

Also, "for PTP feature for ..."? How about "enable PTP hardware
timestamping and clock manipulation support for ..."?

> +	  of switch.

switches

> +
>  config NET_DSA_MICROCHIP_KSZ8863_SMI
>  	tristate "KSZ series SMI connected switch driver"
>  	depends on NET_DSA_MICROCHIP_KSZ_COMMON
> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> new file mode 100644
> index 000000000000..cad0c6087419
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -0,0 +1,270 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Microchip LAN937X PTP Implementation
> + * Copyright (C) 2021-2022 Microchip Technology Inc.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/ptp_classify.h>
> +#include <linux/ptp_clock_kernel.h>
> +
> +#include "ksz_common.h"
> +#include "ksz_ptp.h"
> +#include "ksz_ptp_reg.h"
> +
> +#define ptp_caps_to_data(d) \
> +		container_of((d), struct ksz_ptp_data, caps)
> +#define ptp_data_to_ksz_dev(d) \
> +		container_of((d), struct ksz_device, ptp_data)
> +
> +#define MAX_DRIFT_CORR 6250000

KSZ_MAX_DRIFT_CORR maybe? Also maybe a small comment about the
assumptions that were made when it was calculated?

> +
> +#define KSZ_PTP_INC_NS 40  /* HW clock is incremented every 40 ns (by 40) */
> +#define KSZ_PTP_SUBNS_BITS 32  /* Number of bits in sub-nanoseconds counter */
> +
> +/* The function is return back the capability of timestamping feature when
> + * requested through ethtool -T <interface> utility
> + */
> +int ksz_get_ts_info(struct dsa_switch *ds, int port, struct ethtool_ts_info *ts)
> +{
> +	struct ksz_device *dev	= ds->priv;
> +	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +
> +	ts->so_timestamping = SOF_TIMESTAMPING_TX_HARDWARE |
> +			      SOF_TIMESTAMPING_RX_HARDWARE |
> +			      SOF_TIMESTAMPING_RAW_HARDWARE;
> +
> +	ts->tx_types = (1 << HWTSTAMP_TX_OFF);
> +
> +	ts->rx_filters = (1 << HWTSTAMP_FILTER_NONE);
> +
> +	ts->phc_index = ptp_clock_index(ptp_data->clock);

Ah, but I don't think the optionality of ptp_data->clock is dealt with
very well here. ptp_data->clock can be NULL, and ethtool -T can still be
run on the interface. That will dereference a NULL pointer in ptp_clock_index().

int ptp_clock_index(struct ptp_clock *ptp)
{
	return ptp->index;
}
EXPORT_SYMBOL(ptp_clock_index);

> +
> +	return 0;
> +}
> +
> +int ksz_ptp_clock_register(struct dsa_switch *ds)
> +{
> +	struct ksz_device *dev = ds->priv;
> +	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +	int ret;
> +
> +	mutex_init(&ptp_data->lock);
> +
> +	ptp_data->caps = ksz_ptp_caps;
> +
> +	/* Start hardware counter */
> +	ret = ksz_ptp_start_clock(dev);
> +	if (ret)
> +		return ret;
> +
> +	/* Register the PTP Clock */
> +	ptp_data->clock = ptp_clock_register(&ptp_data->caps, dev->dev);
> +	if (IS_ERR_OR_NULL(ptp_data->clock))
> +		return PTR_ERR(ptp_data->clock);
> +
> +	ret = ksz_rmw16(dev, REG_PTP_MSG_CONF1, PTP_802_1AS, PTP_802_1AS);

A small comment as to what this does? I see in other places you're
generous with comments, like "Register the PTP clock" above the
ptp_clock_register() call.

> +	if (ret)
> +		goto error_unregister_clock;
> +
> +	return 0;
> +
> +error_unregister_clock:
> +	ptp_clock_unregister(ptp_data->clock);
> +	return ret;
> +}
> +
> +MODULE_AUTHOR("Christian Eggers <ceggers@arri.de>");
> +MODULE_AUTHOR("Arun Ramadoss <arun.ramadoss@microchip.com>");
> +MODULE_DESCRIPTION("PTP support for KSZ switch");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
> new file mode 100644
> index 000000000000..ac53b0df2733
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/ksz_ptp.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Microchip LAN937X PTP Implementation
> + * Copyright (C) 2020-2021 Microchip Technology Inc.
> + */
> +
> +#ifndef _NET_DSA_DRIVERS_KSZ_PTP_H
> +#define _NET_DSA_DRIVERS_KSZ_PTP_H
> +
> +#if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ_PTP)
> +
> +#endif	/* End of CONFIG_NET_DSA_MICROCHIOP_KSZ_PTP */

MICROCHIP not MICROCHIOP

> +
> +#endif

[RFC Patch net-next v2 3/8] net: dsa: microchip: Initial hardware time stamping support

[Arun Ramadoss]

This patch adds the routine for get_ts_info, hwstamp_get, set. This enables
the PTP support towards userspace applications such as linuxptp.
Tx timestamping can be enabled per port and Rx timestamping enabled
globally.

Signed-off-by: Christian Eggers <ceggers@arri.de>
Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c |   2 +
 drivers/net/dsa/microchip/ksz_common.h |   3 +
 drivers/net/dsa/microchip/ksz_ptp.c    | 115 ++++++++++++++++++++++++-
 drivers/net/dsa/microchip/ksz_ptp.h    |  14 +++
 include/linux/dsa/ksz_common.h         |  23 +++++
 net/dsa/tag_ksz.c                      |  63 ++++++++++++++
 6 files changed, 218 insertions(+), 2 deletions(-)
 create mode 100644 include/linux/dsa/ksz_common.h

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index eb77eca0dcb2..0abbb2ebcd00 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2873,6 +2873,8 @@ static const struct dsa_switch_ops ksz_switch_ops = {
 	.port_change_mtu	= ksz_change_mtu,
 	.port_max_mtu		= ksz_max_mtu,
 	.get_ts_info            = ksz_get_ts_info,
+	.port_hwtstamp_get      = ksz_hwtstamp_get,
+	.port_hwtstamp_set      = ksz_hwtstamp_set,
 };
 
 struct ksz_device *ksz_switch_alloc(struct device *base, void *priv)
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 767f17d2c75d..605d0a295288 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -104,6 +104,9 @@ struct ksz_port {
 	struct ksz_device *ksz_dev;
 	struct ksz_irq pirq;
 	u8 num;
+#if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ_PTP)
+	u8 hwts_tx_en;
+#endif
 };
 
 struct ksz_device {
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index cad0c6087419..1b2880d013ed 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2021-2022 Microchip Technology Inc.
  */
 
+#include <linux/dsa/ksz_common.h>
 #include <linux/kernel.h>
 #include <linux/ptp_classify.h>
 #include <linux/ptp_clock_kernel.h>
@@ -21,6 +22,17 @@
 #define KSZ_PTP_INC_NS 40  /* HW clock is incremented every 40 ns (by 40) */
 #define KSZ_PTP_SUBNS_BITS 32  /* Number of bits in sub-nanoseconds counter */
 
+static int ksz_ptp_enable_mode(struct ksz_device *dev, bool enable)
+{
+	u16 data = 0;
+
+	/* Enable PTP mode */
+	if (enable)
+		data = PTP_ENABLE;
+
+	return ksz_rmw16(dev, REG_PTP_MSG_CONF1, PTP_ENABLE, data);
+}
+
 /* The function is return back the capability of timestamping feature when
  * requested through ethtool -T <interface> utility
  */
@@ -33,15 +45,114 @@ int ksz_get_ts_info(struct dsa_switch *ds, int port, struct ethtool_ts_info *ts)
 			      SOF_TIMESTAMPING_RX_HARDWARE |
 			      SOF_TIMESTAMPING_RAW_HARDWARE;
 
-	ts->tx_types = (1 << HWTSTAMP_TX_OFF);
+	ts->tx_types = (1 << HWTSTAMP_TX_OFF) | (1 << HWTSTAMP_TX_ONESTEP_P2P);
+
+	if (is_lan937x(dev))
+		ts->tx_types |= (1 << HWTSTAMP_TX_ON);
 
-	ts->rx_filters = (1 << HWTSTAMP_FILTER_NONE);
+	ts->rx_filters =
+		(1 << HWTSTAMP_FILTER_NONE) | (1 << HWTSTAMP_FILTER_ALL);
 
 	ts->phc_index = ptp_clock_index(ptp_data->clock);
 
 	return 0;
 }
 
+int ksz_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr)
+{
+	struct ksz_tagger_data *tagger_data = ksz_tagger_data(ds);
+	struct ksz_device *dev = ds->priv;
+	struct hwtstamp_config config;
+
+	config.flags = 0;
+
+	config.tx_type = dev->ports[port].hwts_tx_en;
+
+	if (tagger_data->hwtstamp_get_state(ds))
+		config.rx_filter = HWTSTAMP_FILTER_ALL;
+	else
+		config.rx_filter = HWTSTAMP_FILTER_NONE;
+
+	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
+		-EFAULT : 0;
+}
+
+static int ksz_set_hwtstamp_config(struct ksz_device *dev, int port,
+				   struct hwtstamp_config *config)
+{
+	struct ksz_tagger_data *tagger_data = ksz_tagger_data(dev->ds);
+	struct ksz_port *prt = &dev->ports[port];
+	bool rx_on;
+
+	/* reserved for future extensions */
+	if (config->flags)
+		return -EINVAL;
+
+	switch (config->tx_type) {
+	case HWTSTAMP_TX_OFF:
+	case HWTSTAMP_TX_ONESTEP_P2P:
+		prt->hwts_tx_en = config->tx_type;
+		break;
+	case HWTSTAMP_TX_ON:
+		if (!is_lan937x(dev))
+			return -ERANGE;
+
+		prt->hwts_tx_en = config->tx_type;
+		break;
+	default:
+		return -ERANGE;
+	}
+
+	switch (config->rx_filter) {
+	case HWTSTAMP_FILTER_NONE:
+		rx_on = false;
+		break;
+	default:
+		rx_on = true;
+		break;
+	}
+
+	if (rx_on != tagger_data->hwtstamp_get_state(dev->ds)) {
+		int ret;
+
+		tagger_data->hwtstamp_set_state(dev->ds, false);
+
+		ret = ksz_ptp_enable_mode(dev, rx_on);
+		if (ret)
+			return ret;
+
+		if (rx_on)
+			tagger_data->hwtstamp_set_state(dev->ds, true);
+	}
+
+	return 0;
+}
+
+int ksz_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	struct hwtstamp_config config;
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	ret = copy_from_user(&config, ifr->ifr_data, sizeof(config));
+	if (ret)
+		goto error_return;
+
+	ret = ksz_set_hwtstamp_config(dev, port, &config);
+	if (ret)
+		goto error_return;
+
+	/* Save the chosen configuration to be returned later. */
+	ret = copy_to_user(ifr->ifr_data, &config, sizeof(config));
+
+error_return:
+	mutex_unlock(&ptp_data->lock);
+	return ret;
+}
+
 /* These are function related to the ptp clock info */
 static int _ksz_ptp_gettime(struct ksz_device *dev, struct timespec64 *ts)
 {
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index ac53b0df2733..4c024cc9d935 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -21,6 +21,8 @@ void ksz_ptp_clock_unregister(struct dsa_switch *ds);
 
 int ksz_get_ts_info(struct dsa_switch *ds, int port,
 		    struct ethtool_ts_info *ts);
+int ksz_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr);
+int ksz_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr);
 
 #else
 
@@ -38,6 +40,18 @@ static inline void ksz_ptp_clock_unregister(struct dsa_switch *ds) { }
 
 #define ksz_get_ts_info NULL
 
+static inline int ksz_hwtstamp_get(struct dsa_switch *ds, int port,
+				   struct ifreq *ifr)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int ksz_hwtstamp_set(struct dsa_switch *ds, int port,
+				   struct ifreq *ifr)
+{
+	return -EOPNOTSUPP;
+}
+
 #endif	/* End of CONFIG_NET_DSA_MICROCHIOP_KSZ_PTP */
 
 #endif
diff --git a/include/linux/dsa/ksz_common.h b/include/linux/dsa/ksz_common.h
new file mode 100644
index 000000000000..8903bce4753b
--- /dev/null
+++ b/include/linux/dsa/ksz_common.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Microchip switch tag common header
+ *
+ * Copyright (C) 2021-2022 Microchip Technology Inc.
+ */
+
+#ifndef _NET_DSA_KSZ_COMMON_H_
+#define _NET_DSA_KSZ_COMMON_H_
+
+#include <net/dsa.h>
+
+struct ksz_tagger_data {
+	bool (*hwtstamp_get_state)(struct dsa_switch *ds);
+	void (*hwtstamp_set_state)(struct dsa_switch *ds, bool on);
+};
+
+static inline struct ksz_tagger_data *
+ksz_tagger_data(struct dsa_switch *ds)
+{
+	return ds->tagger_data;
+}
+
+#endif /* _NET_DSA_KSZ_COMMON_H_ */
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 37db5156f9a3..6a909a300c13 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2017 Microchip Technology
  */
 
+#include <linux/dsa/ksz_common.h>
 #include <linux/etherdevice.h>
 #include <linux/list.h>
 #include <net/dsa.h>
@@ -18,6 +19,62 @@
 #define KSZ_EGRESS_TAG_LEN		1
 #define KSZ_INGRESS_TAG_LEN		1
 
+#define KSZ_HWTS_EN  0
+
+struct ksz_tagger_private {
+	struct ksz_tagger_data data; /* Must be first */
+	unsigned long state;
+};
+
+static struct ksz_tagger_private *
+ksz_tagger_private(struct dsa_switch *ds)
+{
+	return ds->tagger_data;
+}
+
+static bool ksz_hwtstamp_get_state(struct dsa_switch *ds)
+{
+	struct ksz_tagger_private *priv = ksz_tagger_private(ds);
+
+	return test_bit(KSZ_HWTS_EN, &priv->state);
+}
+
+static void ksz_hwtstamp_set_state(struct dsa_switch *ds, bool on)
+{
+	struct ksz_tagger_private *priv = ksz_tagger_private(ds);
+
+	if (on)
+		set_bit(KSZ_HWTS_EN, &priv->state);
+	else
+		clear_bit(KSZ_HWTS_EN, &priv->state);
+}
+
+static void ksz_disconnect(struct dsa_switch *ds)
+{
+	struct ksz_tagger_private *priv = ds->tagger_data;
+
+	kfree(priv);
+	ds->tagger_data = NULL;
+}
+
+static int ksz_connect(struct dsa_switch *ds)
+{
+	struct ksz_tagger_data *tagger_data;
+	struct ksz_tagger_private *priv;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	/* Export functions for switch driver use */
+	tagger_data = &priv->data;
+	tagger_data->hwtstamp_get_state = ksz_hwtstamp_get_state;
+	tagger_data->hwtstamp_set_state = ksz_hwtstamp_set_state;
+	ds->tagger_data = priv;
+
+	return 0;
+}
+
 static struct sk_buff *ksz_common_rcv(struct sk_buff *skb,
 				      struct net_device *dev,
 				      unsigned int port, unsigned int len)
@@ -156,6 +213,8 @@ static const struct dsa_device_ops ksz9477_netdev_ops = {
 	.proto	= DSA_TAG_PROTO_KSZ9477,
 	.xmit	= ksz9477_xmit,
 	.rcv	= ksz9477_rcv,
+	.connect = ksz_connect,
+	.disconnect = ksz_disconnect,
 	.needed_tailroom = KSZ9477_INGRESS_TAG_LEN,
 };
 
@@ -192,6 +251,8 @@ static const struct dsa_device_ops ksz9893_netdev_ops = {
 	.proto	= DSA_TAG_PROTO_KSZ9893,
 	.xmit	= ksz9893_xmit,
 	.rcv	= ksz9477_rcv,
+	.connect = ksz_connect,
+	.disconnect = ksz_disconnect,
 	.needed_tailroom = KSZ_INGRESS_TAG_LEN,
 };
 
@@ -250,6 +311,8 @@ static const struct dsa_device_ops lan937x_netdev_ops = {
 	.proto	= DSA_TAG_PROTO_LAN937X,
 	.xmit	= lan937x_xmit,
 	.rcv	= ksz9477_rcv,
+	.connect = ksz_connect,
+	.disconnect = ksz_disconnect,
 	.needed_tailroom = LAN937X_EGRESS_TAG_LEN,
 };
 
-- 
2.36.1

Re: [RFC Patch net-next v2 3/8] net: dsa: microchip: Initial hardware time stamping support

[Vladimir Oltean]

On Mon, Nov 21, 2022 at 09:11:45PM +0530, Arun Ramadoss wrote:
> +static int ksz_ptp_enable_mode(struct ksz_device *dev, bool enable)
> +{
> +	u16 data = 0;
> +
> +	/* Enable PTP mode */
> +	if (enable)
> +		data = PTP_ENABLE;
> +
> +	return ksz_rmw16(dev, REG_PTP_MSG_CONF1, PTP_ENABLE, data);
> +}
> +
> +static int ksz_set_hwtstamp_config(struct ksz_device *dev, int port,
> +				   struct hwtstamp_config *config)
> +{
> +	struct ksz_tagger_data *tagger_data = ksz_tagger_data(dev->ds);
> +	struct ksz_port *prt = &dev->ports[port];
> +	bool rx_on;
> +
> +	/* reserved for future extensions */
> +	if (config->flags)
> +		return -EINVAL;
> +
> +	switch (config->tx_type) {
> +	case HWTSTAMP_TX_OFF:
> +	case HWTSTAMP_TX_ONESTEP_P2P:
> +		prt->hwts_tx_en = config->tx_type;
> +		break;
> +	case HWTSTAMP_TX_ON:
> +		if (!is_lan937x(dev))
> +			return -ERANGE;
> +
> +		prt->hwts_tx_en = config->tx_type;
> +		break;
> +	default:
> +		return -ERANGE;
> +	}
> +
> +	switch (config->rx_filter) {
> +	case HWTSTAMP_FILTER_NONE:
> +		rx_on = false;
> +		break;
> +	default:
> +		rx_on = true;
> +		break;
> +	}
> +
> +	if (rx_on != tagger_data->hwtstamp_get_state(dev->ds)) {
> +		int ret;
> +
> +		tagger_data->hwtstamp_set_state(dev->ds, false);
> +
> +		ret = ksz_ptp_enable_mode(dev, rx_on);
> +		if (ret)
> +			return ret;
> +
> +		if (rx_on)
> +			tagger_data->hwtstamp_set_state(dev->ds, true);
> +	}

What's your excuse which such a horrible code pattern? What will happen
so bad with the packet if it's flagged with a TX timestamp request in
KSZ_SKB_CB(skb) at the same time as REG_PTP_MSG_CONF1 is written to?

Also, doesn't dev->ports[port].hwts_tx_en serve as a guard against
flagging packets for TX timestamps when you shouldn't?

> +
> +	return 0;
> +}
> diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
> index 37db5156f9a3..6a909a300c13 100644
> --- a/net/dsa/tag_ksz.c
> +++ b/net/dsa/tag_ksz.c
> @@ -4,6 +4,7 @@
>   * Copyright (c) 2017 Microchip Technology
>   */
>  
> +#include <linux/dsa/ksz_common.h>
>  #include <linux/etherdevice.h>
>  #include <linux/list.h>
>  #include <net/dsa.h>
> @@ -18,6 +19,62 @@
>  #define KSZ_EGRESS_TAG_LEN		1
>  #define KSZ_INGRESS_TAG_LEN		1
>  
> +#define KSZ_HWTS_EN  0
> +
> +struct ksz_tagger_private {
> +	struct ksz_tagger_data data; /* Must be first */
> +	unsigned long state;
> +};
> +
> +static struct ksz_tagger_private *
> +ksz_tagger_private(struct dsa_switch *ds)
> +{
> +	return ds->tagger_data;
> +}
> +
> +static bool ksz_hwtstamp_get_state(struct dsa_switch *ds)
> +{
> +	struct ksz_tagger_private *priv = ksz_tagger_private(ds);
> +
> +	return test_bit(KSZ_HWTS_EN, &priv->state);
> +}
> +
> +static void ksz_hwtstamp_set_state(struct dsa_switch *ds, bool on)
> +{
> +	struct ksz_tagger_private *priv = ksz_tagger_private(ds);
> +
> +	if (on)
> +		set_bit(KSZ_HWTS_EN, &priv->state);
> +	else
> +		clear_bit(KSZ_HWTS_EN, &priv->state);
> +}

Re: [RFC Patch net-next v2 3/8] net: dsa: microchip: Initial hardware time stamping support

[Arun.Ramadoss]

Hi Vladimir,
Thanks for the comment.

On Tue, 2022-11-22 at 01:13 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Mon, Nov 21, 2022 at 09:11:45PM +0530, Arun Ramadoss wrote:
> > +static int ksz_set_hwtstamp_config(struct ksz_device *dev, int
> > port,
> > +                                struct hwtstamp_config *config)
> > +{
> > +     struct ksz_tagger_data *tagger_data = ksz_tagger_data(dev-
> > >ds);
> > +     struct ksz_port *prt = &dev->ports[port];
> > +     bool rx_on;
> > +
> > +     /* reserved for future extensions */
> > +     if (config->flags)
> > +             return -EINVAL;
> > +
> > +     switch (config->tx_type) {
> > +     case HWTSTAMP_TX_OFF:
> > +     case HWTSTAMP_TX_ONESTEP_P2P:
> > +             prt->hwts_tx_en = config->tx_type;
> > +             break;
> > +     case HWTSTAMP_TX_ON:
> > +             if (!is_lan937x(dev))
> > +                     return -ERANGE;
> > +
> > +             prt->hwts_tx_en = config->tx_type;
> > +             break;
> > +     default:
> > +             return -ERANGE;
> > +     }
> > +
> > +     switch (config->rx_filter) {
> > +     case HWTSTAMP_FILTER_NONE:
> > +             rx_on = false;
> > +             break;
> > +     default:
> > +             rx_on = true;
> > +             break;
> > +     }
> > +
> > +     if (rx_on != tagger_data->hwtstamp_get_state(dev->ds)) {
> > +             int ret;
> > +
> > +             tagger_data->hwtstamp_set_state(dev->ds, false);
> > +
> > +             ret = ksz_ptp_enable_mode(dev, rx_on);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             if (rx_on)
> > +                     tagger_data->hwtstamp_set_state(dev->ds,
> > true);
> > +     }
> 
> What's your excuse which such a horrible code pattern? What will
> happen
> so bad with the packet if it's flagged with a TX timestamp request in
> KSZ_SKB_CB(skb) at the same time as REG_PTP_MSG_CONF1 is written to?
> 
> Also, doesn't dev->ports[port].hwts_tx_en serve as a guard against
> flagging packets for TX timestamps when you shouldn't?
> 

I took this configuration template routine from other driver. Can I
replace above snippet with

tagger_data->hwtstamp_set_state(dev->ds, rx_on);
ret = ksz_ptp_enable_mode(dev, rx_on);
if (ret)
    return ret;

> diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
> > index 37db5156f9a3..6a909a300c13 100644
> > --- a/net/dsa/tag_ksz.c
> > +++ b/net/dsa/tag_ksz.c
> > @@ -4,6 +4,7 @@
> >   * Copyright (c) 2017 Microchip Technology
> >   */
> > 
> > +#include <linux/dsa/ksz_common.h>
> >  #include <linux/etherdevice.h>
> >  #include <linux/list.h>
> >  #include <net/dsa.h>
> > @@ -18,6 +19,62 @@
> >  #define KSZ_EGRESS_TAG_LEN           1
> >  #define KSZ_INGRESS_TAG_LEN          1
> > 
> > +#define KSZ_HWTS_EN  0
> > +
> > +struct ksz_tagger_private {
> > +     struct ksz_tagger_data data; /* Must be first */
> > +     unsigned long state;
> > +};
> > +
> > 

Re: [RFC Patch net-next v2 3/8] net: dsa: microchip: Initial hardware time stamping support

[Vladimir Oltean]

On Wed, Nov 23, 2022 at 01:57:47PM +0000, Arun.Ramadoss@microchip.com wrote:
> > What's your excuse which such a horrible code pattern? What will happen
> > so bad with the packet if it's flagged with a TX timestamp request in
> > KSZ_SKB_CB(skb) at the same time as REG_PTP_MSG_CONF1 is written to?
> > 
> > Also, doesn't dev->ports[port].hwts_tx_en serve as a guard against
> > flagging packets for TX timestamps when you shouldn't?
> > 
> 
> I took this configuration template routine from other driver.

Not really a good excuse. The sja1105 driver has more hardware-specific
issues to deal with, not necessarily the same as ksz.

> Can I replace above snippet with
> 
> tagger_data->hwtstamp_set_state(dev->ds, rx_on);
> ret = ksz_ptp_enable_mode(dev, rx_on);
> if (ret)
>     return ret;

Why do you need to call hwtstamp_set_state anyway?

Re: [RFC Patch net-next v2 3/8] net: dsa: microchip: Initial hardware time stamping support

[Arun.Ramadoss]

On Thu, 2022-11-24 at 12:22 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Wed, Nov 23, 2022 at 01:57:47PM +0000, Arun.Ramadoss@microchip.com
>  wrote:
> > > What's your excuse which such a horrible code pattern? What will
> > > happen
> > > so bad with the packet if it's flagged with a TX timestamp
> > > request in
> > > KSZ_SKB_CB(skb) at the same time as REG_PTP_MSG_CONF1 is written
> > > to?
> > > 
> > > Also, doesn't dev->ports[port].hwts_tx_en serve as a guard
> > > against
> > > flagging packets for TX timestamps when you shouldn't?
> > > 
> > 
> > I took this configuration template routine from other driver.

Mistake here. It is carried forwarded from Christian Eggers patch.

> 
> Not really a good excuse. The sja1105 driver has more hardware-
> specific
> issues to deal with, not necessarily the same as ksz.
> 
> > Can I replace above snippet with
> > 
> > tagger_data->hwtstamp_set_state(dev->ds, rx_on);
> > ret = ksz_ptp_enable_mode(dev, rx_on);
> > if (ret)
> >     return ret;
> 
> Why do you need to call hwtstamp_set_state anyway?

In tag_ksz.c, xmit function query this state, to determine whether to
allocate the 4 PTP timestamp bytes in the skb_buffer or not. Using this
tagger_data set state, ptp enable and disable is communicated between
ksz_ptp.c and tag_ksz.c

Re: [RFC Patch net-next v2 3/8] net: dsa: microchip: Initial hardware time stamping support

[Vladimir Oltean]

On Thu, Nov 24, 2022 at 10:52:46AM +0000, Arun.Ramadoss@microchip.com wrote:
> Mistake here. It is carried forwarded from Christian Eggers patch.

Still taken from sja1105_hwtstamp_set(). Anyway, doesn't matter where
it's taken from, as long as it has a justification for being there.

> > Why do you need to call hwtstamp_set_state anyway?
> 
> In tag_ksz.c, xmit function query this state, to determine whether to
> allocate the 4 PTP timestamp bytes in the skb_buffer or not. Using this
> tagger_data set state, ptp enable and disable is communicated between
> ksz_ptp.c and tag_ksz.c

Why do you need to query this state in particular, considering that the
skb goes first through the port_txtstamp() dsa_switch_ops function?
Can't you just check there if TX timestamping is enabled, and leave a
mark in KSZ_SKB_CB()?

Re: [RFC Patch net-next v2 3/8] net: dsa: microchip: Initial hardware time stamping support

[Arun.Ramadoss]

Hi Vladimir,

On Thu, 2022-11-24 at 16:14 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Thu, Nov 24, 2022 at 10:52:46AM +0000, Arun.Ramadoss@microchip.com
>  wrote:
> > Mistake here. It is carried forwarded from Christian Eggers patch.
> 
> Still taken from sja1105_hwtstamp_set(). Anyway, doesn't matter where
> it's taken from, as long as it has a justification for being there.
> 
> > > Why do you need to call hwtstamp_set_state anyway?
> > 
> > In tag_ksz.c, xmit function query this state, to determine whether
> > to
> > allocate the 4 PTP timestamp bytes in the skb_buffer or not. Using
> > this
> > tagger_data set state, ptp enable and disable is communicated
> > between
> > ksz_ptp.c and tag_ksz.c
> 
> Why do you need to query this state in particular, considering that
> the
> skb goes first through the port_txtstamp() dsa_switch_ops function?
> Can't you just check there if TX timestamping is enabled, and leave a
> mark in KSZ_SKB_CB()?
KSZ switches need a additional 4 bytes in tail tag if the PTP is
enabled in hardware. If the PTP is enabled and if we didn't add 4
additional bytes in the tail tag then packets are corrupted.

Tristram explained this in the patch conversation

https://lore.kernel.org/netdev/20201118203013.5077-1-ceggers@arri.de/T/#mb3eba4918bda351a405168e7a2140d29262f4c63

I did the follwing experiment today, 
* Removed the ptp time stamp check in tag_ksz.c. In the ksz_xmit
function, 4 additional bytes are added only if KSZ_SKB_CB->ts_en bit is
set.
* Setup the board, ping two boards. Ping is successful.
* Run the ptpl in the background
* Now if I run the ping, ping is not successful. And also in the ptp4l
log message it shows as bad message received.

We need a mechanism to inform tag_ksz.c to add 4 additional bytes in
tail_tag for all the packets if the ptp is enabled in the hardware.

Re: [RFC Patch net-next v2 3/8] net: dsa: microchip: Initial hardware time stamping support

[Vladimir Oltean]

On Fri, Nov 25, 2022 at 07:06:07AM +0000, Arun.Ramadoss@microchip.com wrote:
> KSZ switches need a additional 4 bytes in tail tag if the PTP is
> enabled in hardware. If the PTP is enabled and if we didn't add 4
> additional bytes in the tail tag then packets are corrupted.
> 
> Tristram explained this in the patch conversation
> 
> https://lore.kernel.org/netdev/20201118203013.5077-1-ceggers@arri.de/T/#mb3eba4918bda351a405168e7a2140d29262f4c63
> 
> I did the follwing experiment today, 
> * Removed the ptp time stamp check in tag_ksz.c. In the ksz_xmit
> function, 4 additional bytes are added only if KSZ_SKB_CB->ts_en bit is
> set.
> * Setup the board, ping two boards. Ping is successful.
> * Run the ptpl in the background
> * Now if I run the ping, ping is not successful. And also in the ptp4l
> log message it shows as bad message received.
> 
> We need a mechanism to inform tag_ksz.c to add 4 additional bytes in
> tail_tag for all the packets if the ptp is enabled in the hardware.

Ok. The code + comments need to be sufficiently self-explanatory that
this question does not get asked again. It will not be trivial to do a
proper job documenting the hardware oddities as a justification for the
software workarounds, but it should be possible.

[RFC Patch net-next v2 4/8] net: dsa: microchip: Manipulating absolute time using ptp hw clock

[Arun Ramadoss]

This patch is used for reconstructing the absolute time from the 32bit
hardware time stamping value. The do_aux ioctl is used for reading the
ptp hardware clock and store it to global variable.
The timestamped value in tail tag during rx and register during tx are
32 bit value (2 bit seconds and 30 bit nanoseconds). The time taken to
read entire ptp clock will be time consuming. In order to speed up, the
software clock is maintained. This clock time will be added to 32 bit
timestamp to get the absolute time stamp.

Signed-off-by: Christian Eggers <ceggers@arri.de>
Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.h |  1 +
 drivers/net/dsa/microchip/ksz_ptp.c    | 59 +++++++++++++++++++++++++-
 drivers/net/dsa/microchip/ksz_ptp.h    |  3 ++
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 605d0a295288..4c092376fc89 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -7,6 +7,7 @@
 #ifndef __KSZ_COMMON_H
 #define __KSZ_COMMON_H
 
+#include <linux/dsa/ksz_common.h>
 #include <linux/etherdevice.h>
 #include <linux/kernel.h>
 #include <linux/mutex.h>
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 1b2880d013ed..5db67dc6a565 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -24,12 +24,22 @@
 
 static int ksz_ptp_enable_mode(struct ksz_device *dev, bool enable)
 {
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
 	u16 data = 0;
+	int ret;
 
 	/* Enable PTP mode */
-	if (enable)
+	if (enable) {
 		data = PTP_ENABLE;
 
+		/* Schedule cyclic call of ksz_ptp_do_aux_work() */
+		ret = ptp_schedule_worker(ptp_data->clock, 0);
+		if (ret)
+			return ret;
+	} else {
+		ptp_cancel_worker_sync(ptp_data->clock);
+	}
+
 	return ksz_rmw16(dev, REG_PTP_MSG_CONF1, PTP_ENABLE, data);
 }
 
@@ -226,6 +236,12 @@ static int ksz_ptp_settime(struct ptp_clock_info *ptp,
 
 	/* Load PTP clock from shadow registers */
 	ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_LOAD_TIME, PTP_LOAD_TIME);
+	if (ret)
+		goto error_return;
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time = *ts;
+	spin_unlock_bh(&ptp_data->clock_lock);
 
 error_return:
 	mutex_unlock(&ptp_data->lock);
@@ -280,6 +296,7 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
 	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	struct timespec64 delta64 = ns_to_timespec64(delta);
 	s32 sec, nsec;
 	u16 data16;
 	int ret;
@@ -312,15 +329,51 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 		data16 |= PTP_STEP_DIR;
 
 	ret = ksz_write16(dev, REG_PTP_CLK_CTRL, data16);
+	if (ret)
+		goto error_return;
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time = timespec64_add(ptp_data->clock_time, delta64);
+	spin_unlock_bh(&ptp_data->clock_lock);
 
 error_return:
 	mutex_unlock(&ptp_data->lock);
 	return ret;
 }
 
+/*  Function is pointer to the do_aux_work in the ptp_clock capability */
+static long ksz_ptp_do_aux_work(struct ptp_clock_info *ptp)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	struct timespec64 ts;
+
+	mutex_lock(&ptp_data->lock);
+	_ksz_ptp_gettime(dev, &ts);
+	mutex_unlock(&ptp_data->lock);
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time = ts;
+	spin_unlock_bh(&ptp_data->clock_lock);
+
+	return HZ;  /* reschedule in 1 second */
+}
+
 static int ksz_ptp_start_clock(struct ksz_device *dev)
 {
-	return ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE, PTP_CLK_ENABLE);
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	int ret;
+
+	ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_CLK_ENABLE, PTP_CLK_ENABLE);
+	if (ret)
+		return ret;
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time.tv_sec = 0;
+	ptp_data->clock_time.tv_nsec = 0;
+	spin_unlock_bh(&ptp_data->clock_lock);
+
+	return 0;
 }
 
 static const struct ptp_clock_info ksz_ptp_caps = {
@@ -331,6 +384,7 @@ static const struct ptp_clock_info ksz_ptp_caps = {
 	.settime64	= ksz_ptp_settime,
 	.adjfine	= ksz_ptp_adjfine,
 	.adjtime	= ksz_ptp_adjtime,
+	.do_aux_work	= ksz_ptp_do_aux_work,
 };
 
 int ksz_ptp_clock_register(struct dsa_switch *ds)
@@ -340,6 +394,7 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
 	int ret;
 
 	mutex_init(&ptp_data->lock);
+	spin_lock_init(&ptp_data->clock_lock);
 
 	ptp_data->caps = ksz_ptp_caps;
 
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index 4c024cc9d935..09c0e58c365e 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -13,6 +13,9 @@ struct ksz_ptp_data {
 	struct ptp_clock *clock;
 	/* Serializes all operations on the PTP hardware clock */
 	struct mutex lock;
+	/* lock for accessing the clock_time */
+	spinlock_t clock_lock;
+	struct timespec64 clock_time;
 };
 
 int ksz_ptp_clock_register(struct dsa_switch *ds);
-- 
2.36.1

[RFC Patch net-next v2 5/8] net: dsa: microchip: enable the ptp interrupt for timestamping

[Arun Ramadoss]

PTP Interrupt mask and status register differ from the global and port
interrupt mechanism by two methods. One is that for global/port
interrupt enabling we have to clear the bit but for ptp interrupt we
have to set the bit. And other is bit12:0 is reserver in ptp interrupt
registers. This forced to not use the generic implementation of
global/port interrupt method routine. This patch implement the ptp
interrupt mechanism to read the timestamp register for sync, pdelay_req
and pdelay_resp.

Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c  |  15 +-
 drivers/net/dsa/microchip/ksz_common.h  |  11 ++
 drivers/net/dsa/microchip/ksz_ptp.c     | 201 ++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_ptp.h     |   9 ++
 drivers/net/dsa/microchip/ksz_ptp_reg.h |  16 ++
 5 files changed, 250 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 0abbb2ebcd00..c0b9b406fca8 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2014,13 +2014,17 @@ static int ksz_setup(struct dsa_switch *ds)
 			ret = ksz_pirq_setup(dev, dp->index);
 			if (ret)
 				goto out_girq;
+
+			ret = ksz_ptp_irq_setup(ds, dp->index);
+			if (ret)
+				goto out_pirq;
 		}
 	}
 
 	ret = ksz_ptp_clock_register(ds);
 	if (ret) {
 		dev_err(dev->dev, "Failed to register PTP clock: %d\n", ret);
-		goto out_pirq;
+		goto out_ptpirq;
 	}
 
 	ret = ksz_mdio_register(dev);
@@ -2037,6 +2041,10 @@ static int ksz_setup(struct dsa_switch *ds)
 
 out_ptp_clock_unregister:
 	ksz_ptp_clock_unregister(ds);
+out_ptpirq:
+	if (dev->irq > 0)
+		dsa_switch_for_each_user_port(dp, dev->ds)
+			ksz_ptp_irq_free(ds, dp->index);
 out_pirq:
 	if (dev->irq > 0)
 		dsa_switch_for_each_user_port(dp, dev->ds)
@@ -2056,8 +2064,11 @@ static void ksz_teardown(struct dsa_switch *ds)
 	ksz_ptp_clock_unregister(ds);
 
 	if (dev->irq > 0) {
-		dsa_switch_for_each_user_port(dp, dev->ds)
+		dsa_switch_for_each_user_port(dp, dev->ds) {
+			ksz_ptp_irq_free(ds, dp->index);
+
 			ksz_irq_free(&dev->ports[dp->index].pirq);
+		}
 
 		ksz_irq_free(&dev->girq);
 	}
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 4c092376fc89..510aff116f3e 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -22,6 +22,7 @@
 #define KSZ_MAX_NUM_PORTS 8
 
 struct ksz_device;
+struct ksz_port;
 
 struct vlan_table {
 	u32 table[3];
@@ -85,6 +86,13 @@ struct ksz_irq {
 	struct ksz_device *dev;
 };
 
+struct ksz_ptp_irq {
+	struct ksz_port *port;
+	u16 ts_reg;
+	char name[16];
+	int irq_num;
+};
+
 struct ksz_port {
 	bool remove_tag;		/* Remove Tag flag set, for ksz8795 only */
 	bool learning;
@@ -107,6 +115,8 @@ struct ksz_port {
 	u8 num;
 #if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ_PTP)
 	u8 hwts_tx_en;
+	struct ksz_irq ptpirq;
+	struct ksz_ptp_irq ptpmsg_irq[3];
 #endif
 };
 
@@ -608,6 +618,7 @@ static inline int is_lan937x(struct ksz_device *dev)
 #define REG_PORT_INT_MASK		0x001F
 
 #define PORT_SRC_PHY_INT		1
+#define PORT_SRC_PTP_INT		2
 
 /* Regmap tables generation */
 #define KSZ_SPI_OP_RD		3
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 5db67dc6a565..4b46cdc1c3bb 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -4,6 +4,8 @@
  */
 
 #include <linux/dsa/ksz_common.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/ptp_classify.h>
 #include <linux/ptp_clock_kernel.h>
@@ -22,6 +24,8 @@
 #define KSZ_PTP_INC_NS 40  /* HW clock is incremented every 40 ns (by 40) */
 #define KSZ_PTP_SUBNS_BITS 32  /* Number of bits in sub-nanoseconds counter */
 
+#define KSZ_PTP_INT_START 13
+
 static int ksz_ptp_enable_mode(struct ksz_device *dev, bool enable)
 {
 	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
@@ -430,6 +434,203 @@ void ksz_ptp_clock_unregister(struct dsa_switch *ds)
 	ptp_clock_unregister(ptp_data->clock);
 }
 
+static irqreturn_t ksz_ptp_msg_thread_fn(int irq, void *dev_id)
+{
+	return IRQ_NONE;
+}
+
+static irqreturn_t ksz_ptp_irq_thread_fn(int irq, void *dev_id)
+{
+	struct ksz_irq *ptpirq = dev_id;
+	unsigned int nhandled = 0;
+	struct ksz_device *dev;
+	unsigned int sub_irq;
+	u16 data;
+	int ret;
+	u8 n;
+
+	dev = ptpirq->dev;
+
+	/* Read interrupt status register */
+	ret = ksz_read16(dev, ptpirq->reg_status, &data);
+	if (ret)
+		goto out;
+
+	for (n = 0; n < ptpirq->nirqs; ++n) {
+		if (data & BIT(n + KSZ_PTP_INT_START)) {
+			sub_irq = irq_find_mapping(ptpirq->domain, n);
+			handle_nested_irq(sub_irq);
+			++nhandled;
+		}
+	}
+
+	//Clear the interrupts W1C
+	ret = ksz_write16(dev, ptpirq->reg_status, data);
+	if (ret)
+		return IRQ_NONE;
+
+out:
+	return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE);
+}
+
+static void ksz_ptp_irq_mask(struct irq_data *d)
+{
+	struct ksz_irq *kirq = irq_data_get_irq_chip_data(d);
+
+	kirq->masked &= ~BIT(d->hwirq + KSZ_PTP_INT_START);
+}
+
+static void ksz_ptp_irq_unmask(struct irq_data *d)
+{
+	struct ksz_irq *kirq = irq_data_get_irq_chip_data(d);
+
+	kirq->masked |= BIT(d->hwirq + KSZ_PTP_INT_START);
+}
+
+static void ksz_ptp_irq_bus_lock(struct irq_data *d)
+{
+	struct ksz_irq *kirq  = irq_data_get_irq_chip_data(d);
+
+	mutex_lock(&kirq->dev->lock_irq);
+}
+
+static void ksz_ptp_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct ksz_irq *kirq  = irq_data_get_irq_chip_data(d);
+	struct ksz_device *dev = kirq->dev;
+	int ret;
+
+	ret = ksz_write16(dev, kirq->reg_mask, kirq->masked);
+	if (ret)
+		dev_err(dev->dev, "failed to change IRQ mask\n");
+
+	mutex_unlock(&dev->lock_irq);
+}
+
+static const struct irq_chip ksz_ptp_irq_chip = {
+	.name			= "ksz-irq",
+	.irq_mask		= ksz_ptp_irq_mask,
+	.irq_unmask		= ksz_ptp_irq_unmask,
+	.irq_bus_lock		= ksz_ptp_irq_bus_lock,
+	.irq_bus_sync_unlock	= ksz_ptp_irq_bus_sync_unlock,
+};
+
+static int ksz_ptp_irq_domain_map(struct irq_domain *d,
+				  unsigned int irq, irq_hw_number_t hwirq)
+{
+	irq_set_chip_data(irq, d->host_data);
+	irq_set_chip_and_handler(irq, &ksz_ptp_irq_chip, handle_level_irq);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops ksz_ptp_irq_domain_ops = {
+	.map	= ksz_ptp_irq_domain_map,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+static int ksz_ptp_msg_irq_setup(struct ksz_port *port)
+{
+	u16 ts_reg[] = {REG_PTP_PORT_PDRESP_TS, REG_PTP_PORT_XDELAY_TS,
+			REG_PTP_PORT_SYNC_TS};
+	struct ksz_device *dev = port->ksz_dev;
+	struct ksz_irq *ptpirq = &port->ptpirq;
+	struct ksz_ptp_irq *ptpmsg_irq;
+	int ret;
+	u8 n;
+
+	for (n = 0; n < ptpirq->nirqs; n++) {
+		ptpmsg_irq = &port->ptpmsg_irq[n];
+
+		ptpmsg_irq->port = port;
+		ptpmsg_irq->ts_reg = dev->dev_ops->get_port_addr(port->num,
+								 ts_reg[n]);
+		ptpmsg_irq->irq_num = irq_create_mapping(ptpirq->domain, n);
+		if (ptpmsg_irq->irq_num < 0) {
+			ret = ptpmsg_irq->irq_num;
+			goto out;
+		}
+
+		snprintf(ptpmsg_irq->name, sizeof(ptpmsg_irq->name),
+			 "PTP-MSG-%d", n);
+
+		ret = request_threaded_irq(ptpmsg_irq->irq_num, NULL,
+					   ksz_ptp_msg_thread_fn,
+					   IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
+					   ptpmsg_irq->name, ptpmsg_irq);
+		if (ret)
+			goto out;
+	}
+
+	return 0;
+
+out:
+	while (n--)
+		irq_dispose_mapping(port->ptpmsg_irq[n].irq_num);
+
+	return ret;
+}
+
+int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
+{
+	struct ksz_device *dev = ds->priv;
+	const struct ksz_dev_ops *ops = dev->dev_ops;
+	struct ksz_port *port = &dev->ports[p];
+	struct ksz_irq *ptpirq = &port->ptpirq;
+	int ret;
+
+	ptpirq->dev = dev;
+	ptpirq->masked = 0;
+	ptpirq->nirqs = 3;
+	ptpirq->reg_mask = ops->get_port_addr(p, REG_PTP_PORT_TX_INT_ENABLE__2);
+	ptpirq->reg_status = ops->get_port_addr(p,
+						REG_PTP_PORT_TX_INT_STATUS__2);
+	snprintf(ptpirq->name, sizeof(ptpirq->name), "ptp_irq-%d", p);
+
+	ptpirq->irq_num = irq_find_mapping(port->pirq.domain, PORT_SRC_PTP_INT);
+	if (ptpirq->irq_num < 0)
+		return ptpirq->irq_num;
+
+	ptpirq->domain = irq_domain_add_simple(dev->dev->of_node, ptpirq->nirqs,
+					       0, &ksz_ptp_irq_domain_ops,
+					       ptpirq);
+	if (!ptpirq->domain)
+		return -ENOMEM;
+
+	ret = request_threaded_irq(ptpirq->irq_num, NULL, ksz_ptp_irq_thread_fn,
+				   IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
+				   ptpirq->name, ptpirq);
+	if (ret)
+		goto out;
+
+	ret = ksz_ptp_msg_irq_setup(port);
+	if (ret)
+		goto out;
+
+	return 0;
+
+out:
+	irq_dispose_mapping(ptpirq->irq_num);
+
+	return ret;
+}
+
+void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_port *port = &dev->ports[p];
+	struct ksz_irq *ptpirq = &port->ptpirq;
+	u8 n;
+
+	free_irq(ptpirq->irq_num, ptpirq);
+
+	for (n = 0; n < ptpirq->nirqs; n++)
+		irq_dispose_mapping(port->ptpmsg_irq[n].irq_num);
+
+	irq_domain_remove(ptpirq->domain);
+}
+
 MODULE_AUTHOR("Christian Eggers <ceggers@arri.de>");
 MODULE_AUTHOR("Arun Ramadoss <arun.ramadoss@microchip.com>");
 MODULE_DESCRIPTION("PTP support for KSZ switch");
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index 09c0e58c365e..7e5d374d2acf 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -26,6 +26,8 @@ int ksz_get_ts_info(struct dsa_switch *ds, int port,
 		    struct ethtool_ts_info *ts);
 int ksz_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr);
 int ksz_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr);
+int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p);
+void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p);
 
 #else
 
@@ -55,6 +57,13 @@ static inline int ksz_hwtstamp_set(struct dsa_switch *ds, int port,
 	return -EOPNOTSUPP;
 }
 
+static inline int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
+{
+	return 0;
+}
+
+static inline void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p) {}
+
 #endif	/* End of CONFIG_NET_DSA_MICROCHIOP_KSZ_PTP */
 
 #endif
diff --git a/drivers/net/dsa/microchip/ksz_ptp_reg.h b/drivers/net/dsa/microchip/ksz_ptp_reg.h
index 2bf8395475b9..2ae6c8b01b00 100644
--- a/drivers/net/dsa/microchip/ksz_ptp_reg.h
+++ b/drivers/net/dsa/microchip/ksz_ptp_reg.h
@@ -50,3 +50,19 @@
 #define PTP_TC_P2P			BIT(2)
 #define PTP_MASTER			BIT(1)
 #define PTP_1STEP			BIT(0)
+
+/* Port PTP Register */
+#define REG_PTP_PORT_RX_DELAY__2	0x0C00
+#define REG_PTP_PORT_TX_DELAY__2	0x0C02
+#define REG_PTP_PORT_ASYM_DELAY__2	0x0C04
+
+#define REG_PTP_PORT_XDELAY_TS		0x0C08
+#define REG_PTP_PORT_SYNC_TS		0x0C0C
+#define REG_PTP_PORT_PDRESP_TS		0x0C10
+
+#define REG_PTP_PORT_TX_INT_STATUS__2	0x0C14
+#define REG_PTP_PORT_TX_INT_ENABLE__2	0x0C16
+
+#define PTP_PORT_SYNC_INT		BIT(15)
+#define PTP_PORT_XDELAY_REQ_INT		BIT(14)
+#define PTP_PORT_PDELAY_RESP_INT	BIT(13)
-- 
2.36.1

[RFC Patch net-next v2 6/8] net: dsa: microchip: Adding the ptp packet reception logic

[Arun Ramadoss]

This patch adds the routines for timestamping received ptp packets.
Whenever the ptp packet is received, the 4 byte hardware time stamped
value is append to its packet. This 4 byte value is extracted from the
tail tag and reconstructed to absolute time and assigned to skb
hwtstamp.

Signed-off-by: Christian Eggers <ceggers@arri.de>
Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 13 +++++
 drivers/net/dsa/microchip/ksz_ptp.c    | 78 ++++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_ptp.h    | 12 ++++
 include/linux/dsa/ksz_common.h         | 16 ++++++
 net/dsa/tag_ksz.c                      | 73 ++++++++++++++++++++++--
 5 files changed, 186 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index c0b9b406fca8..6430241fce46 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2453,6 +2453,18 @@ static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds,
 	return proto;
 }
 
+static int ksz_connect_tag_protocol(struct dsa_switch *ds,
+				    enum dsa_tag_protocol proto)
+{
+	struct ksz_tagger_data *tagger_data;
+
+	tagger_data = ksz_tagger_data(ds);
+	tagger_data->meta_tstamp_handler = ksz_tstamp_reconstruct;
+	tagger_data->is_ptp_twostep = ksz_is_ptp_twostep;
+
+	return 0;
+}
+
 static int ksz_port_vlan_filtering(struct dsa_switch *ds, int port,
 				   bool flag, struct netlink_ext_ack *extack)
 {
@@ -2849,6 +2861,7 @@ static int ksz_switch_detect(struct ksz_device *dev)
 
 static const struct dsa_switch_ops ksz_switch_ops = {
 	.get_tag_protocol	= ksz_get_tag_protocol,
+	.connect_tag_protocol   = ksz_connect_tag_protocol,
 	.get_phy_flags		= ksz_get_phy_flags,
 	.setup			= ksz_setup,
 	.teardown		= ksz_teardown,
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 4b46cdc1c3bb..c9c43a98427b 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -167,6 +167,59 @@ int ksz_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr)
 	return ret;
 }
 
+void ksz_port_txtstamp(struct dsa_switch *ds, int port,
+		       struct sk_buff *skb)
+{
+	struct ksz_device *dev	= ds->priv;
+	struct ksz_port *prt = &dev->ports[port];
+	struct ptp_header *hdr;
+	struct sk_buff *clone;
+	unsigned int type;
+	u8 ptp_msg_type;
+
+	if (!prt->hwts_tx_en)
+		return;
+
+	type = ptp_classify_raw(skb);
+	if (type == PTP_CLASS_NONE)
+		return;
+
+	hdr = ptp_parse_header(skb, type);
+	if (!hdr)
+		return;
+
+	ptp_msg_type = ptp_get_msgtype(hdr, type);
+
+	switch (ptp_msg_type) {
+	case PTP_MSGTYPE_SYNC:
+		if (prt->hwts_tx_en == HWTSTAMP_TX_ONESTEP_P2P)
+			return;
+	case PTP_MSGTYPE_PDELAY_REQ:
+	case PTP_MSGTYPE_PDELAY_RESP:
+		break;
+
+	default:
+		return;
+	}
+
+	clone = skb_clone_sk(skb);
+	if (!clone)
+		return;
+
+	/* caching the value to be used in later */
+	KSZ_SKB_CB(skb)->clone = clone;
+	KSZ_SKB_CB(clone)->ptp_type = type;
+	KSZ_SKB_CB(clone)->ptp_msg_type = ptp_msg_type;
+}
+
+bool ksz_is_ptp_twostep(struct dsa_switch *ds, unsigned int port)
+{
+	struct ksz_device *dev	= ds->priv;
+	struct ksz_port *prt = &dev->ports[port];
+
+	return (prt->hwts_tx_en == HWTSTAMP_TX_ON);
+}
+
 /* These are function related to the ptp clock info */
 static int _ksz_ptp_gettime(struct ksz_device *dev, struct timespec64 *ts)
 {
@@ -380,6 +433,31 @@ static int ksz_ptp_start_clock(struct ksz_device *dev)
 	return 0;
 }
 
+ktime_t ksz_tstamp_reconstruct(struct dsa_switch *ds, ktime_t tstamp)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	struct timespec64 ts = ktime_to_timespec64(tstamp);
+	struct timespec64 ptp_clock_time;
+	struct timespec64 diff;
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_clock_time = ptp_data->clock_time;
+	spin_unlock_bh(&ptp_data->clock_lock);
+
+	/* calculate full time from partial time stamp */
+	ts.tv_sec = (ptp_clock_time.tv_sec & ~3) | ts.tv_sec;
+
+	/* find nearest possible point in time */
+	diff = timespec64_sub(ts, ptp_clock_time);
+	if (diff.tv_sec > 2)
+		ts.tv_sec -= 4;
+	else if (diff.tv_sec < -2)
+		ts.tv_sec += 4;
+
+	return timespec64_to_ktime(ts);
+}
+
 static const struct ptp_clock_info ksz_ptp_caps = {
 	.owner		= THIS_MODULE,
 	.name		= "Microchip Clock",
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index 7e5d374d2acf..46b1ccbace81 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -28,6 +28,8 @@ int ksz_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr);
 int ksz_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr);
 int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p);
 void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p);
+ktime_t ksz_tstamp_reconstruct(struct dsa_switch *ds, ktime_t tstamp);
+bool ksz_is_ptp_twostep(struct dsa_switch *ds, unsigned int port);
 
 #else
 
@@ -64,6 +66,16 @@ static inline int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 
 static inline void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p) {}
 
+static inline ktime_t ksz_tstamp_reconstruct(struct dsa_switch *ds, ktime_t tstamp)
+{
+	return 0;
+}
+
+bool ksz_is_ptp_twostep(struct dsa_switch *ds, unsigned int port)
+{
+	return 0;
+}
+
 #endif	/* End of CONFIG_NET_DSA_MICROCHIOP_KSZ_PTP */
 
 #endif
diff --git a/include/linux/dsa/ksz_common.h b/include/linux/dsa/ksz_common.h
index 8903bce4753b..d71851dbeb4d 100644
--- a/include/linux/dsa/ksz_common.h
+++ b/include/linux/dsa/ksz_common.h
@@ -9,9 +9,25 @@
 
 #include <net/dsa.h>
 
+/* All time stamps from the KSZ consist of 2 bits for seconds and 30 bits for
+ * nanoseconds. This is NOT the same as 32 bits for nanoseconds.
+ */
+#define KSZ_TSTAMP_SEC_MASK  GENMASK(31, 30)
+#define KSZ_TSTAMP_NSEC_MASK GENMASK(29, 0)
+
+static inline ktime_t ksz_decode_tstamp(u32 tstamp)
+{
+	u64 ns = FIELD_GET(KSZ_TSTAMP_SEC_MASK, tstamp) * NSEC_PER_SEC +
+		 FIELD_GET(KSZ_TSTAMP_NSEC_MASK, tstamp);
+
+	return ns_to_ktime(ns);
+}
+
 struct ksz_tagger_data {
 	bool (*hwtstamp_get_state)(struct dsa_switch *ds);
 	void (*hwtstamp_set_state)(struct dsa_switch *ds, bool on);
+	ktime_t (*meta_tstamp_handler)(struct dsa_switch *ds, ktime_t tstamp);
+	bool (*is_ptp_twostep)(struct dsa_switch *ds, unsigned int port);
 };
 
 static inline struct ksz_tagger_data *
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 6a909a300c13..39b27f6e26be 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -7,6 +7,7 @@
 #include <linux/dsa/ksz_common.h>
 #include <linux/etherdevice.h>
 #include <linux/list.h>
+#include <linux/ptp_classify.h>
 #include <net/dsa.h>
 #include "dsa_priv.h"
 
@@ -169,6 +170,63 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8795, KSZ8795_NAME);
 #define KSZ9477_TAIL_TAG_OVERRIDE	BIT(9)
 #define KSZ9477_TAIL_TAG_LOOKUP		BIT(10)
 
+static void ksz_rcv_timestamp(struct sk_buff *skb, u8 *tag,
+			      struct net_device *dev, unsigned int port)
+{
+	struct skb_shared_hwtstamps *hwtstamps = skb_hwtstamps(skb);
+	u8 *tstamp_raw = tag - KSZ9477_PTP_TAG_LEN;
+	struct dsa_switch *ds = dev->dsa_ptr->ds;
+	struct ksz_tagger_data *tagger_data;
+	struct ptp_header *ptp_hdr;
+	unsigned int ptp_type;
+	u8 ptp_msg_type;
+	ktime_t tstamp;
+	s64 correction;
+
+	tagger_data = ksz_tagger_data(ds);
+	if (!tagger_data->meta_tstamp_handler)
+		return;
+
+	/* convert time stamp and write to skb */
+	tstamp = ksz_decode_tstamp(get_unaligned_be32(tstamp_raw));
+	memset(hwtstamps, 0, sizeof(*hwtstamps));
+	hwtstamps->hwtstamp = tagger_data->meta_tstamp_handler(ds, tstamp);
+
+	if (skb_headroom(skb) < ETH_HLEN)
+		return;
+
+	__skb_push(skb, ETH_HLEN);
+	ptp_type = ptp_classify_raw(skb);
+	__skb_pull(skb, ETH_HLEN);
+
+	if (ptp_type == PTP_CLASS_NONE)
+		return;
+
+	ptp_hdr = ptp_parse_header(skb, ptp_type);
+	if (!ptp_hdr)
+		return;
+
+	if (!tagger_data->is_ptp_twostep)
+		return;
+
+	if (tagger_data->is_ptp_twostep(ds, port))
+		return;
+
+	ptp_msg_type = ptp_get_msgtype(ptp_hdr, ptp_type);
+	if (ptp_msg_type != PTP_MSGTYPE_PDELAY_REQ)
+		return;
+
+	/* Only subtract the partial time stamp from the correction field.  When
+	 * the hardware adds the egress time stamp to the correction field of
+	 * the PDelay_Resp message on tx, also only the partial time stamp will
+	 * be added.
+	 */
+	correction = (s64)get_unaligned_be64(&ptp_hdr->correction);
+	correction -= ktime_to_ns(tstamp) << 16;
+
+	ptp_header_update_correction(skb, ptp_type, ptp_hdr, correction);
+}
+
 static struct sk_buff *ksz9477_xmit(struct sk_buff *skb,
 				    struct net_device *dev)
 {
@@ -202,8 +260,10 @@ static struct sk_buff *ksz9477_rcv(struct sk_buff *skb, struct net_device *dev)
 	unsigned int len = KSZ_EGRESS_TAG_LEN;
 
 	/* Extra 4-bytes PTP timestamp */
-	if (tag[0] & KSZ9477_PTP_TAG_INDICATION)
+	if (tag[0] & KSZ9477_PTP_TAG_INDICATION) {
+		ksz_rcv_timestamp(skb, tag, dev, port);
 		len += KSZ9477_PTP_TAG_LEN;
+	}
 
 	return ksz_common_rcv(skb, dev, port, len);
 }
@@ -215,7 +275,7 @@ static const struct dsa_device_ops ksz9477_netdev_ops = {
 	.rcv	= ksz9477_rcv,
 	.connect = ksz_connect,
 	.disconnect = ksz_disconnect,
-	.needed_tailroom = KSZ9477_INGRESS_TAG_LEN,
+	.needed_tailroom = KSZ9477_INGRESS_TAG_LEN + KSZ9477_PTP_TAG_LEN,
 };
 
 DSA_TAG_DRIVER(ksz9477_netdev_ops);
@@ -253,7 +313,7 @@ static const struct dsa_device_ops ksz9893_netdev_ops = {
 	.rcv	= ksz9477_rcv,
 	.connect = ksz_connect,
 	.disconnect = ksz_disconnect,
-	.needed_tailroom = KSZ_INGRESS_TAG_LEN,
+	.needed_tailroom = KSZ_INGRESS_TAG_LEN + KSZ9477_PTP_TAG_LEN,
 };
 
 DSA_TAG_DRIVER(ksz9893_netdev_ops);
@@ -266,10 +326,11 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893, KSZ9893_NAME);
  * tag0 : represents tag override, lookup and valid
  * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x80=port8)
  *
- * For rcv, 1 byte is added before FCS.
+ * For rcv, 1/5 bytes is added before FCS.
  * ---------------------------------------------------------------------------
- * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|FCS(4bytes)
  * ---------------------------------------------------------------------------
+ * ts   : time stamp (Present only if bit 7 of tag0 is set)
  * tag0 : zero-based value represents port
  *	  (eg, 0x00=port1, 0x02=port3, 0x07=port8)
  */
@@ -313,7 +374,7 @@ static const struct dsa_device_ops lan937x_netdev_ops = {
 	.rcv	= ksz9477_rcv,
 	.connect = ksz_connect,
 	.disconnect = ksz_disconnect,
-	.needed_tailroom = LAN937X_EGRESS_TAG_LEN,
+	.needed_tailroom = LAN937X_EGRESS_TAG_LEN + KSZ9477_PTP_TAG_LEN,
 };
 
 DSA_TAG_DRIVER(lan937x_netdev_ops);
-- 
2.36.1

[RFC Patch net-next v2 7/8] net: dsa: microchip: add the transmission tstamp logic

[Arun Ramadoss]

This patch adds the routines for transmission of ptp packets. When the
ptp packets(sync, pdelay_req, pdelay_rsp) to be transmitted, the skb is
copied to global skb through port_txtstamp ioctl.
After the packet is transmitted, ISR is triggered. The time at which
packet transmitted is recorded to separate register available for each
message. This value is reconstructed to absolute time and posted to the
user application through skb complete.

Signed-off-by: Christian Eggers <ceggers@arri.de>
Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c  |   2 +
 drivers/net/dsa/microchip/ksz_common.h  |   3 +
 drivers/net/dsa/microchip/ksz_ptp.c     |  86 +++++++++++++-
 drivers/net/dsa/microchip/ksz_ptp.h     |   7 ++
 drivers/net/dsa/microchip/ksz_ptp_reg.h |   3 +
 include/linux/dsa/ksz_common.h          |  16 +++
 net/dsa/tag_ksz.c                       | 152 ++++++++++++++++++++++--
 7 files changed, 258 insertions(+), 11 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 6430241fce46..411a71710161 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2459,6 +2459,7 @@ static int ksz_connect_tag_protocol(struct dsa_switch *ds,
 	struct ksz_tagger_data *tagger_data;
 
 	tagger_data = ksz_tagger_data(ds);
+	tagger_data->xmit_work_fn = ksz_port_deferred_xmit;
 	tagger_data->meta_tstamp_handler = ksz_tstamp_reconstruct;
 	tagger_data->is_ptp_twostep = ksz_is_ptp_twostep;
 
@@ -2899,6 +2900,7 @@ static const struct dsa_switch_ops ksz_switch_ops = {
 	.get_ts_info            = ksz_get_ts_info,
 	.port_hwtstamp_get      = ksz_hwtstamp_get,
 	.port_hwtstamp_set      = ksz_hwtstamp_set,
+	.port_txtstamp		= ksz_port_txtstamp,
 };
 
 struct ksz_device *ksz_switch_alloc(struct device *base, void *priv)
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 510aff116f3e..764b6a3e5187 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -89,6 +89,7 @@ struct ksz_irq {
 struct ksz_ptp_irq {
 	struct ksz_port *port;
 	u16 ts_reg;
+	bool ts_en;
 	char name[16];
 	int irq_num;
 };
@@ -117,6 +118,8 @@ struct ksz_port {
 	u8 hwts_tx_en;
 	struct ksz_irq ptpirq;
 	struct ksz_ptp_irq ptpmsg_irq[3];
+	ktime_t tstamp_msg;
+	struct completion tstamp_msg_comp;
 #endif
 };
 
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index c9c43a98427b..5506adaac488 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -104,14 +104,25 @@ static int ksz_set_hwtstamp_config(struct ksz_device *dev, int port,
 
 	switch (config->tx_type) {
 	case HWTSTAMP_TX_OFF:
+		prt->ptpmsg_irq[KSZ_SYNC_MSG].ts_en  = 0;
+		prt->ptpmsg_irq[KSZ_XDREQ_MSG].ts_en = 0;
+		prt->ptpmsg_irq[KSZ_PDRES_MSG].ts_en = 0;
+		prt->hwts_tx_en = HWTSTAMP_TX_OFF;
+		break;
 	case HWTSTAMP_TX_ONESTEP_P2P:
-		prt->hwts_tx_en = config->tx_type;
+		prt->ptpmsg_irq[KSZ_SYNC_MSG].ts_en  = 0;
+		prt->ptpmsg_irq[KSZ_XDREQ_MSG].ts_en = 1;
+		prt->ptpmsg_irq[KSZ_PDRES_MSG].ts_en = 0;
+		prt->hwts_tx_en = HWTSTAMP_TX_ONESTEP_P2P;
 		break;
 	case HWTSTAMP_TX_ON:
 		if (!is_lan937x(dev))
 			return -ERANGE;
 
-		prt->hwts_tx_en = config->tx_type;
+		prt->ptpmsg_irq[KSZ_SYNC_MSG].ts_en  = 1;
+		prt->ptpmsg_irq[KSZ_XDREQ_MSG].ts_en = 1;
+		prt->ptpmsg_irq[KSZ_PDRES_MSG].ts_en = 1;
+		prt->hwts_tx_en = HWTSTAMP_TX_ON;
 		break;
 	default:
 		return -ERANGE;
@@ -220,7 +231,6 @@ bool ksz_is_ptp_twostep(struct dsa_switch *ds, unsigned int port)
 	return (prt->hwts_tx_en == HWTSTAMP_TX_ON);
 }
 
-/* These are function related to the ptp clock info */
 static int _ksz_ptp_gettime(struct ksz_device *dev, struct timespec64 *ts)
 {
 	u32 nanoseconds;
@@ -458,6 +468,50 @@ ktime_t ksz_tstamp_reconstruct(struct dsa_switch *ds, ktime_t tstamp)
 	return timespec64_to_ktime(ts);
 }
 
+static void ksz_ptp_txtstamp_skb(struct ksz_device *dev,
+				 struct ksz_port *prt, struct sk_buff *skb)
+{
+	struct skb_shared_hwtstamps hwtstamps = {};
+	int ret;
+
+	skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+
+	/* timeout must include tstamp latency, IRQ latency and time for
+	 * reading the time stamp.
+	 */
+	ret = wait_for_completion_timeout(&prt->tstamp_msg_comp,
+					  msecs_to_jiffies(100));
+	if (!ret)
+		return;
+
+	hwtstamps.hwtstamp = prt->tstamp_msg;
+	skb_complete_tx_timestamp(skb, &hwtstamps);
+}
+
+#define work_to_xmit_work(w) \
+		container_of((w), struct ksz_deferred_xmit_work, work)
+void ksz_port_deferred_xmit(struct kthread_work *work)
+{
+	struct ksz_deferred_xmit_work *xmit_work = work_to_xmit_work(work);
+	struct sk_buff *clone, *skb = xmit_work->skb;
+	struct dsa_switch *ds = xmit_work->dp->ds;
+	struct ksz_device *dev = ds->priv;
+	struct ksz_port *prt;
+
+	prt = &dev->ports[xmit_work->dp->index];
+
+	clone = KSZ_SKB_CB(skb)->clone;
+
+	reinit_completion(&prt->tstamp_msg_comp);
+
+	/* Transfer skb to the host port. */
+	dsa_enqueue_skb(skb, skb->dev);
+
+	ksz_ptp_txtstamp_skb(dev, prt, clone);
+
+	kfree(xmit_work);
+}
+
 static const struct ptp_clock_info ksz_ptp_caps = {
 	.owner		= THIS_MODULE,
 	.name		= "Microchip Clock",
@@ -514,7 +568,29 @@ void ksz_ptp_clock_unregister(struct dsa_switch *ds)
 
 static irqreturn_t ksz_ptp_msg_thread_fn(int irq, void *dev_id)
 {
-	return IRQ_NONE;
+	struct ksz_ptp_irq *ptpmsg_irq = dev_id;
+	struct ksz_device *dev;
+	struct ksz_port *port;
+	u32 tstamp_raw;
+	ktime_t tstamp;
+	int ret;
+
+	port = ptpmsg_irq->port;
+	dev = port->ksz_dev;
+
+	if (ptpmsg_irq->ts_en) {
+		ret = ksz_read32(dev, ptpmsg_irq->ts_reg, &tstamp_raw);
+		if (ret)
+			return IRQ_NONE;
+
+		tstamp = ksz_decode_tstamp(tstamp_raw);
+
+		port->tstamp_msg = ksz_tstamp_reconstruct(dev->ds, tstamp);
+
+		complete(&port->tstamp_msg_comp);
+	}
+
+	return IRQ_HANDLED;
 }
 
 static irqreturn_t ksz_ptp_irq_thread_fn(int irq, void *dev_id)
@@ -666,6 +742,8 @@ int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 						REG_PTP_PORT_TX_INT_STATUS__2);
 	snprintf(ptpirq->name, sizeof(ptpirq->name), "ptp_irq-%d", p);
 
+	init_completion(&port->tstamp_msg_comp);
+
 	ptpirq->irq_num = irq_find_mapping(port->pirq.domain, PORT_SRC_PTP_INT);
 	if (ptpirq->irq_num < 0)
 		return ptpirq->irq_num;
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index 46b1ccbace81..7d6786caa633 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -26,8 +26,10 @@ int ksz_get_ts_info(struct dsa_switch *ds, int port,
 		    struct ethtool_ts_info *ts);
 int ksz_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr);
 int ksz_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr);
+void ksz_port_txtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb);
 int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p);
 void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p);
+void ksz_port_deferred_xmit(struct kthread_work *work);
 ktime_t ksz_tstamp_reconstruct(struct dsa_switch *ds, ktime_t tstamp);
 bool ksz_is_ptp_twostep(struct dsa_switch *ds, unsigned int port);
 
@@ -59,6 +61,9 @@ static inline int ksz_hwtstamp_set(struct dsa_switch *ds, int port,
 	return -EOPNOTSUPP;
 }
 
+static inline void ksz_port_txtstamp(struct dsa_switch *ds, int port,
+				     struct sk_buff *skb) {}
+
 static inline int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 {
 	return 0;
@@ -66,6 +71,8 @@ static inline int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 
 static inline void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p) {}
 
+static inline void ksz_port_deferred_xmit(struct kthread_work *work) {}
+
 static inline ktime_t ksz_tstamp_reconstruct(struct dsa_switch *ds, ktime_t tstamp)
 {
 	return 0;
diff --git a/drivers/net/dsa/microchip/ksz_ptp_reg.h b/drivers/net/dsa/microchip/ksz_ptp_reg.h
index 2ae6c8b01b00..ccb87bbdfbcd 100644
--- a/drivers/net/dsa/microchip/ksz_ptp_reg.h
+++ b/drivers/net/dsa/microchip/ksz_ptp_reg.h
@@ -66,3 +66,6 @@
 #define PTP_PORT_SYNC_INT		BIT(15)
 #define PTP_PORT_XDELAY_REQ_INT		BIT(14)
 #define PTP_PORT_PDELAY_RESP_INT	BIT(13)
+#define KSZ_SYNC_MSG			2
+#define KSZ_XDREQ_MSG			1
+#define KSZ_PDRES_MSG			0
diff --git a/include/linux/dsa/ksz_common.h b/include/linux/dsa/ksz_common.h
index d71851dbeb4d..4dd4ccaa06ab 100644
--- a/include/linux/dsa/ksz_common.h
+++ b/include/linux/dsa/ksz_common.h
@@ -23,13 +23,29 @@ static inline ktime_t ksz_decode_tstamp(u32 tstamp)
 	return ns_to_ktime(ns);
 }
 
+struct ksz_deferred_xmit_work {
+	struct dsa_port *dp;
+	struct sk_buff *skb;
+	struct kthread_work work;
+};
+
 struct ksz_tagger_data {
+	void (*xmit_work_fn)(struct kthread_work *work);
 	bool (*hwtstamp_get_state)(struct dsa_switch *ds);
 	void (*hwtstamp_set_state)(struct dsa_switch *ds, bool on);
 	ktime_t (*meta_tstamp_handler)(struct dsa_switch *ds, ktime_t tstamp);
 	bool (*is_ptp_twostep)(struct dsa_switch *ds, unsigned int port);
 };
 
+struct ksz_skb_cb {
+	struct sk_buff *clone;
+	unsigned int ptp_type;
+	u8 ptp_msg_type;
+};
+
+#define KSZ_SKB_CB(skb) \
+	((struct ksz_skb_cb *)((skb)->cb))
+
 static inline struct ksz_tagger_data *
 ksz_tagger_data(struct dsa_switch *ds)
 {
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 39b27f6e26be..fa6bb6df6984 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -25,6 +25,7 @@
 struct ksz_tagger_private {
 	struct ksz_tagger_data data; /* Must be first */
 	unsigned long state;
+	struct kthread_worker *xmit_worker;
 };
 
 static struct ksz_tagger_private *
@@ -54,6 +55,7 @@ static void ksz_disconnect(struct dsa_switch *ds)
 {
 	struct ksz_tagger_private *priv = ds->tagger_data;
 
+	kthread_destroy_worker(priv->xmit_worker);
 	kfree(priv);
 	ds->tagger_data = NULL;
 }
@@ -61,12 +63,23 @@ static void ksz_disconnect(struct dsa_switch *ds)
 static int ksz_connect(struct dsa_switch *ds)
 {
 	struct ksz_tagger_data *tagger_data;
+	struct kthread_worker *xmit_worker;
 	struct ksz_tagger_private *priv;
+	int ret;
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
+	xmit_worker = kthread_create_worker(0, "dsa%d:%d_xmit",
+					    ds->dst->index, ds->index);
+	if (IS_ERR(xmit_worker)) {
+		ret = PTR_ERR(xmit_worker);
+		kfree(priv);
+		return ret;
+	}
+
+	priv->xmit_worker = xmit_worker;
 	/* Export functions for switch driver use */
 	tagger_data = &priv->data;
 	tagger_data->hwtstamp_get_state = ksz_hwtstamp_get_state;
@@ -227,18 +240,127 @@ static void ksz_rcv_timestamp(struct sk_buff *skb, u8 *tag,
 	ptp_header_update_correction(skb, ptp_type, ptp_hdr, correction);
 }
 
+/* Time stamp tag is only inserted if PTP is enabled in hardware. */
+static void ksz_xmit_timestamp(struct dsa_switch *ds, struct sk_buff *skb,
+			       unsigned int port)
+{
+	struct sk_buff *clone = KSZ_SKB_CB(skb)->clone;
+	struct ksz_tagger_data *tagger_data;
+	struct ptp_header *ptp_hdr;
+	unsigned int ptp_type;
+	u32 tstamp_raw = 0;
+	u8 ptp_msg_type;
+	s64 correction;
+
+	if (!clone)
+		goto out_put_tag;
+
+	/* Use cached PTP type from ksz_ptp_port_txtstamp().  */
+	ptp_type = KSZ_SKB_CB(clone)->ptp_type;
+	if (ptp_type == PTP_CLASS_NONE)
+		goto out_put_tag;
+
+	ptp_hdr = ptp_parse_header(skb, ptp_type);
+	if (!ptp_hdr)
+		goto out_put_tag;
+
+	tagger_data = ksz_tagger_data(ds);
+	if (!tagger_data->is_ptp_twostep)
+		goto out_put_tag;
+
+	if (tagger_data->is_ptp_twostep(ds, port))
+		goto out_put_tag;
+
+	ptp_msg_type = KSZ_SKB_CB(clone)->ptp_msg_type;
+	if (ptp_msg_type != PTP_MSGTYPE_PDELAY_RESP)
+		goto out_put_tag;
+
+	correction = (s64)get_unaligned_be64(&ptp_hdr->correction);
+
+	/* For PDelay_Resp messages we will likely have a negative value in the
+	 * correction field (see ksz9477_rcv()). The switch hardware cannot
+	 * correctly update such values (produces an off by one error in the UDP
+	 * checksum), so it must be moved to the time stamp field in the tail
+	 * tag.
+	 */
+	if (correction < 0) {
+		struct timespec64 ts;
+
+		/* Move ingress time stamp from PTP header's correction field to
+		 * tail tag. Format of the correction filed is 48 bit ns + 16
+		 * bit fractional ns.
+		 */
+		ts = ns_to_timespec64(-correction >> 16);
+		tstamp_raw = ((ts.tv_sec & 3) << 30) | ts.tv_nsec;
+
+		/* Set correction field to 0 and update UDP checksum.  */
+		ptp_header_update_correction(skb, ptp_type, ptp_hdr, 0);
+	}
+
+	/* For PDelay_Resp messages, the clone is not required in
+	 * skb_complete_tx_timestamp() and should be freed here.
+	 */
+	kfree_skb(clone);
+	KSZ_SKB_CB(skb)->clone = NULL;
+
+out_put_tag:
+	put_unaligned_be32(tstamp_raw, skb_put(skb, KSZ9477_PTP_TAG_LEN));
+}
+
+/* Defer transmit if waiting for egress time stamp is required.  */
+static struct sk_buff *ksz_defer_xmit(struct dsa_port *dp, struct sk_buff *skb)
+{
+	struct ksz_tagger_data *tagger_data = ksz_tagger_data(dp->ds);
+	struct ksz_tagger_private *priv = ksz_tagger_private(dp->ds);
+	void (*xmit_work_fn)(struct kthread_work *work);
+	struct sk_buff *clone = KSZ_SKB_CB(skb)->clone;
+	struct ksz_deferred_xmit_work *xmit_work;
+	struct kthread_worker *xmit_worker;
+
+	if (!clone)
+		return skb;  /* no deferred xmit for this packet */
+
+	xmit_work_fn = tagger_data->xmit_work_fn;
+	xmit_worker = priv->xmit_worker;
+
+	if (!xmit_work_fn || !xmit_worker)
+		return NULL;
+
+	xmit_work = kzalloc(sizeof(*xmit_work), GFP_ATOMIC);
+	if (!xmit_work)
+		return NULL;
+
+	kthread_init_work(&xmit_work->work, xmit_work_fn);
+	/* Increase refcount so the kfree_skb in dsa_slave_xmit
+	 * won't really free the packet.
+	 */
+	xmit_work->dp = dp;
+	xmit_work->skb = skb_get(skb);
+
+	kthread_queue_work(xmit_worker, &xmit_work->work);
+
+	return NULL;
+}
+
 static struct sk_buff *ksz9477_xmit(struct sk_buff *skb,
 				    struct net_device *dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct ksz_tagger_private *priv;
+	struct dsa_switch *ds = dp->ds;
 	__be16 *tag;
 	u8 *addr;
 	u16 val;
 
+	priv = ksz_tagger_private(ds);
+
+	/* Tag encoding */
+	if (test_bit(KSZ_HWTS_EN, &priv->state))
+		ksz_xmit_timestamp(ds, skb, dp->index);
+
 	if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
 		return NULL;
 
-	/* Tag encoding */
 	tag = skb_put(skb, KSZ9477_INGRESS_TAG_LEN);
 	addr = skb_mac_header(skb);
 
@@ -249,7 +371,7 @@ static struct sk_buff *ksz9477_xmit(struct sk_buff *skb,
 
 	*tag = cpu_to_be16(val);
 
-	return skb;
+	return ksz_defer_xmit(dp, skb);
 }
 
 static struct sk_buff *ksz9477_rcv(struct sk_buff *skb, struct net_device *dev)
@@ -288,13 +410,20 @@ static struct sk_buff *ksz9893_xmit(struct sk_buff *skb,
 				    struct net_device *dev)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct ksz_tagger_private *priv;
+	struct dsa_switch *ds = dp->ds;
 	u8 *addr;
 	u8 *tag;
 
+	priv = ksz_tagger_private(ds);
+
+	/* Tag encoding */
+	if (test_bit(KSZ_HWTS_EN, &priv->state))
+		ksz_xmit_timestamp(ds, skb, dp->index);
+
 	if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
 		return NULL;
 
-	/* Tag encoding */
 	tag = skb_put(skb, KSZ_INGRESS_TAG_LEN);
 	addr = skb_mac_header(skb);
 
@@ -303,7 +432,7 @@ static struct sk_buff *ksz9893_xmit(struct sk_buff *skb,
 	if (is_link_local_ether_addr(addr))
 		*tag |= KSZ9893_TAIL_TAG_OVERRIDE;
 
-	return skb;
+	return ksz_defer_xmit(dp, skb);
 }
 
 static const struct dsa_device_ops ksz9893_netdev_ops = {
@@ -319,10 +448,11 @@ static const struct dsa_device_ops ksz9893_netdev_ops = {
 DSA_TAG_DRIVER(ksz9893_netdev_ops);
 MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893, KSZ9893_NAME);
 
-/* For xmit, 2 bytes are added before FCS.
+/* For xmit, 2/6 bytes are added before FCS.
  * ---------------------------------------------------------------------------
- * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
  * ---------------------------------------------------------------------------
+ * ts   : time stamp (Present only if PTP is enabled in the Hardware)
  * tag0 : represents tag override, lookup and valid
  * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x80=port8)
  *
@@ -346,9 +476,17 @@ static struct sk_buff *lan937x_xmit(struct sk_buff *skb,
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	const struct ethhdr *hdr = eth_hdr(skb);
+	struct ksz_tagger_private *priv;
+	struct dsa_switch *ds = dp->ds;
 	__be16 *tag;
 	u16 val;
 
+	priv = ksz_tagger_private(ds);
+
+	/* Tag encoding */
+	if (test_bit(KSZ_HWTS_EN, &priv->state))
+		ksz_xmit_timestamp(ds, skb, dp->index);
+
 	if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
 		return NULL;
 
@@ -364,7 +502,7 @@ static struct sk_buff *lan937x_xmit(struct sk_buff *skb,
 
 	put_unaligned_be16(val, tag);
 
-	return skb;
+	return ksz_defer_xmit(dp, skb);
 }
 
 static const struct dsa_device_ops lan937x_netdev_ops = {
-- 
2.36.1

Re: [RFC Patch net-next v2 7/8] net: dsa: microchip: add the transmission tstamp logic

[Vladimir Oltean]

Hi Arun,

On Mon, Nov 21, 2022 at 09:11:49PM +0530, Arun Ramadoss wrote:
> This patch adds the routines for transmission of ptp packets. When the
> ptp packets(sync, pdelay_req, pdelay_rsp) to be transmitted, the skb is
> copied to global skb through port_txtstamp ioctl.
> After the packet is transmitted, ISR is triggered. The time at which
> packet transmitted is recorded to separate register available for each
> message. This value is reconstructed to absolute time and posted to the
> user application through skb complete.

"skb complete" is not a thing. "socket error queue" is.

> 
> Signed-off-by: Christian Eggers <ceggers@arri.de>
> Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
> Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
> ---
> +static void ksz_ptp_txtstamp_skb(struct ksz_device *dev,
> +				 struct ksz_port *prt, struct sk_buff *skb)
> +{
> +	struct skb_shared_hwtstamps hwtstamps = {};
> +	int ret;
> +
> +	skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> +
> +	/* timeout must include tstamp latency, IRQ latency and time for
> +	 * reading the time stamp.
> +	 */
> +	ret = wait_for_completion_timeout(&prt->tstamp_msg_comp,
> +					  msecs_to_jiffies(100));
> +	if (!ret)
> +		return;
> +
> +	hwtstamps.hwtstamp = prt->tstamp_msg;
> +	skb_complete_tx_timestamp(skb, &hwtstamps);
> +}
> +
> +#define work_to_xmit_work(w) \
> +		container_of((w), struct ksz_deferred_xmit_work, work)
> +void ksz_port_deferred_xmit(struct kthread_work *work)
> +{
> +	struct ksz_deferred_xmit_work *xmit_work = work_to_xmit_work(work);
> +	struct sk_buff *clone, *skb = xmit_work->skb;
> +	struct dsa_switch *ds = xmit_work->dp->ds;
> +	struct ksz_device *dev = ds->priv;
> +	struct ksz_port *prt;
> +
> +	prt = &dev->ports[xmit_work->dp->index];
> +
> +	clone = KSZ_SKB_CB(skb)->clone;
> +
> +	reinit_completion(&prt->tstamp_msg_comp);
> +
> +	/* Transfer skb to the host port. */
> +	dsa_enqueue_skb(skb, skb->dev);
> +
> +	ksz_ptp_txtstamp_skb(dev, prt, clone);
> +
> +	kfree(xmit_work);
> +}
> +
>  static const struct ptp_clock_info ksz_ptp_caps = {
>  	.owner		= THIS_MODULE,
>  	.name		= "Microchip Clock",
> @@ -514,7 +568,29 @@ void ksz_ptp_clock_unregister(struct dsa_switch *ds)
>  
>  static irqreturn_t ksz_ptp_msg_thread_fn(int irq, void *dev_id)
>  {
> -	return IRQ_NONE;
> +	struct ksz_ptp_irq *ptpmsg_irq = dev_id;
> +	struct ksz_device *dev;
> +	struct ksz_port *port;
> +	u32 tstamp_raw;
> +	ktime_t tstamp;
> +	int ret;
> +
> +	port = ptpmsg_irq->port;
> +	dev = port->ksz_dev;
> +
> +	if (ptpmsg_irq->ts_en) {
> +		ret = ksz_read32(dev, ptpmsg_irq->ts_reg, &tstamp_raw);
> +		if (ret)
> +			return IRQ_NONE;
> +
> +		tstamp = ksz_decode_tstamp(tstamp_raw);
> +
> +		port->tstamp_msg = ksz_tstamp_reconstruct(dev->ds, tstamp);
> +
> +		complete(&port->tstamp_msg_comp);
> +	}
> +
> +	return IRQ_HANDLED;
>  }

I dug out some notes I had taken while reviewing a previous patch set
from Christian. There was a race condition which caused rare TX timestamping
timeouts, and the issue seems very much still present here. See below my
notes, luckily they resulted in a patch which solved the problem at the time.

From 97ad5ac1541349584fc63f1d28ce12a6675dcff0 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Wed, 20 Oct 2021 00:39:37 +0300
Subject: [PATCH] net: dsa: ksz9477_ptp: fix race condition between IRQ thread
 and deferred xmit kthread

Two-step PTP TX timestamping for the ksz9477 driver works as follows:

1. ksz9477_port_deferred_xmit() initializes a completion structure and
   queues the PTP skb to the DSA master

2. DSA master sends the packet to the switch, which forwards it to the
   egress port and the TX timestamp is taken.

3. Switch raises its PTP IRQ and the ksz9477_ptp_port_interrupt()
   handler is run.

4. The PTP timestamp is read, and the completion structure is signaled.

5. PTP interrupts are rearmed for the next timestampable skb.

6. The deferred xmit kthread is woken up by the completion. It collects
   the TX timestamp from the irq kthread, it annotates the skb clone
   with that timestamp, delivers it to the socket error queue, and
   exits.

7. The deferred xmit kthread gets rescheduled with the next
   timestampable PTP packet and the steps from 1 are executed again,
   identically.

There is an issue in the fact that steps 5 and 6 might not actually run
in this exact order. Step 6, the deferred xmit kthread getting woken up
by the completion, might happen as soon as the completion is signaled at
step 4. In that case, the deferred xmit kthread might run to completion
and we might reach step 7, while step 5 (write-1-to-clear to the IRQ
status register, to rearm the interrupt, has _not_ yet run).

If the deferred xmit kthread makes enough progress with the _next_ PTP
skb, such that it actually manages to enqueue it to the DSA master, and
that makes it all the way to the hardware, which takes another TX
timestamp, we have a problem if the IRQ kthread has not cleared the PTP
TX timestamp status yet.

If it clears the PTP status register now, it has effectively eaten a TX
timestamp.

The implication is that the completion for this second PTP skb will time
out, but otherwise, the system will keep chugging on, it will not be
forever stuck. The IRQ kthread does not get rearmed because it has no
reason to (the PTP IRQ is cleared), and the deferred xmit kthread will
free the skb for the completion that timed out, and carry on with its
life. The next skb can go through the cycle 1-6 just fine.

The problem which makes the above scenario possible is that we clear the
interrupt status after we signal the completion. Do it before, and the
interrupt handler is free to do whatever it wishes until it returns.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/microchip/ksz9477_ptp.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477_ptp.c b/drivers/net/dsa/microchip/ksz9477_ptp.c
index c646689cb71e..bc3f0283859a 100644
--- a/drivers/net/dsa/microchip/ksz9477_ptp.c
+++ b/drivers/net/dsa/microchip/ksz9477_ptp.c
@@ -1106,6 +1106,11 @@ irqreturn_t ksz9477_ptp_port_interrupt(struct ksz_device *dev, int port)
 	if (ret)
 		return IRQ_NONE;
 
+	/* Clear interrupt(s) (W1C) */
+	ret = ksz_write16(dev, addr, data);
+	if (ret)
+		return IRQ_NONE;
+
 	if (data & PTP_PORT_XDELAY_REQ_INT) {
 		/* Timestamp for Pdelay_Req / Delay_Req */
 		struct ksz_device_ptp_shared *ptp_shared = &dev->ptp_shared;
@@ -1128,11 +1133,6 @@ irqreturn_t ksz9477_ptp_port_interrupt(struct ksz_device *dev, int port)
 		complete(&prt->tstamp_completion);
 	}
 
-	/* Clear interrupt(s) (W1C) */
-	ret = ksz_write16(dev, addr, data);
-	if (ret)
-		return IRQ_NONE;
-
 	return IRQ_HANDLED;
 }
 

About the only difference seems to be that ACK-ing the interrupt is done
at the end of ksz_ptp_irq_thread_fn(), while complete(&port->tstamp_msg_comp)
is called from ksz_ptp_msg_thread_fn() - which is called by handle_nested_irq()
IIUC.

>  
> +/* Time stamp tag is only inserted if PTP is enabled in hardware. */
> +static void ksz_xmit_timestamp(struct dsa_switch *ds, struct sk_buff *skb,
> +			       unsigned int port)
> +{
> +	struct sk_buff *clone = KSZ_SKB_CB(skb)->clone;
> +	struct ksz_tagger_data *tagger_data;
> +	struct ptp_header *ptp_hdr;
> +	unsigned int ptp_type;
> +	u32 tstamp_raw = 0;
> +	u8 ptp_msg_type;
> +	s64 correction;
> +
> +	if (!clone)
> +		goto out_put_tag;
> +
> +	/* Use cached PTP type from ksz_ptp_port_txtstamp().  */
> +	ptp_type = KSZ_SKB_CB(clone)->ptp_type;
> +	if (ptp_type == PTP_CLASS_NONE)
> +		goto out_put_tag;
> +
> +	ptp_hdr = ptp_parse_header(skb, ptp_type);
> +	if (!ptp_hdr)
> +		goto out_put_tag;
> +
> +	tagger_data = ksz_tagger_data(ds);
> +	if (!tagger_data->is_ptp_twostep)
> +		goto out_put_tag;
> +
> +	if (tagger_data->is_ptp_twostep(ds, port))
> +		goto out_put_tag;
> +
> +	ptp_msg_type = KSZ_SKB_CB(clone)->ptp_msg_type;
> +	if (ptp_msg_type != PTP_MSGTYPE_PDELAY_RESP)
> +		goto out_put_tag;
> +
> +	correction = (s64)get_unaligned_be64(&ptp_hdr->correction);
> +
> +	/* For PDelay_Resp messages we will likely have a negative value in the
> +	 * correction field (see ksz9477_rcv()). The switch hardware cannot
> +	 * correctly update such values (produces an off by one error in the UDP
> +	 * checksum), so it must be moved to the time stamp field in the tail
> +	 * tag.
> +	 */
> +	if (correction < 0) {
> +		struct timespec64 ts;
> +
> +		/* Move ingress time stamp from PTP header's correction field to
> +		 * tail tag. Format of the correction filed is 48 bit ns + 16
> +		 * bit fractional ns.
> +		 */
> +		ts = ns_to_timespec64(-correction >> 16);
> +		tstamp_raw = ((ts.tv_sec & 3) << 30) | ts.tv_nsec;
> +
> +		/* Set correction field to 0 and update UDP checksum.  */
> +		ptp_header_update_correction(skb, ptp_type, ptp_hdr, 0);
> +	}
> +
> +	/* For PDelay_Resp messages, the clone is not required in
> +	 * skb_complete_tx_timestamp() and should be freed here.
> +	 */
> +	kfree_skb(clone);
> +	KSZ_SKB_CB(skb)->clone = NULL;
> +
> +out_put_tag:
> +	put_unaligned_be32(tstamp_raw, skb_put(skb, KSZ9477_PTP_TAG_LEN));
> +}

Re: [RFC Patch net-next v2 7/8] net: dsa: microchip: add the transmission tstamp logic

[Arun.Ramadoss]

Hi Vladimir,
Thanks for the detailed explanation of race condition.

On Tue, 2022-11-22 at 00:51 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> Hi Arun,
> 
> On Mon, Nov 21, 2022 at 09:11:49PM +0530, Arun Ramadoss wrote:
> > This patch adds the routines for transmission of ptp packets. When
> > the
> > ptp packets(sync, pdelay_req, pdelay_rsp) to be transmitted, the
> > skb is
> > copied to global skb through port_txtstamp ioctl.
> > After the packet is transmitted, ISR is triggered. The time at
> > which
> > packet transmitted is recorded to separate register available for
> > each
> > message. This value is reconstructed to absolute time and posted to
> > the
> > user application through skb complete.
> 
> "skb complete" is not a thing. "socket error queue" is.

I will update as socket error queue.

> 
> > 
> > 
> diff --git a/drivers/net/dsa/microchip/ksz9477_ptp.c
> b/drivers/net/dsa/microchip/ksz9477_ptp.c
> index c646689cb71e..bc3f0283859a 100644
> --- a/drivers/net/dsa/microchip/ksz9477_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz9477_ptp.c
> @@ -1106,6 +1106,11 @@ irqreturn_t ksz9477_ptp_port_interrupt(struct
> ksz_device *dev, int port)
>         if (ret)
>                 return IRQ_NONE;
> 
> +       /* Clear interrupt(s) (W1C) */
> +       ret = ksz_write16(dev, addr, data);
> +       if (ret)
> +               return IRQ_NONE;
> +
>         if (data & PTP_PORT_XDELAY_REQ_INT) {
>                 /* Timestamp for Pdelay_Req / Delay_Req */
>                 struct ksz_device_ptp_shared *ptp_shared = &dev-
> >ptp_shared;
> @@ -1128,11 +1133,6 @@ irqreturn_t ksz9477_ptp_port_interrupt(struct
> ksz_device *dev, int port)
>                 complete(&prt->tstamp_completion);
>         }
> 
> -       /* Clear interrupt(s) (W1C) */
> -       ret = ksz_write16(dev, addr, data);
> -       if (ret)
> -               return IRQ_NONE;
> -
>         return IRQ_HANDLED;
>  }
> 
> 
> About the only difference seems to be that ACK-ing the interrupt is
> done
> at the end of ksz_ptp_irq_thread_fn(), while complete(&port-
> >tstamp_msg_comp)
> is called from ksz_ptp_msg_thread_fn() - which is called by
> handle_nested_irq()
> IIUC.

I will clear the interrupts before calling the handle_nested_irq().

> 
> > 
> > +/*

[RFC Patch net-next v2 8/8] net: dsa: microchip: ptp: add periodic output signal

[Arun Ramadoss]

LAN937x and KSZ PTP supported switches has Three Trigger output unit.
This TOU can used to generate the periodic signal for PTP. TOU has the
cycle width register of 32 bit in size and period width register of 24
bit, each value is of 8ns so the pulse width can be maximum 125ms.

Tested using ./testptp -d /dev/ptp0 -p 1000000000 -w 100000000 for
generating the 10ms pulse width

Signed-off-by: Christian Eggers <ceggers@arri.de>
Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.h  |  13 +
 drivers/net/dsa/microchip/ksz_ptp.c     | 324 ++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_ptp.h     |   8 +
 drivers/net/dsa/microchip/ksz_ptp_reg.h |  65 +++++
 4 files changed, 410 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 764b6a3e5187..e572fa4c79ff 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -478,6 +478,19 @@ static inline int ksz_rmw16(struct ksz_device *dev, u32 reg, u16 mask,
 	return ret;
 }
 
+static inline int ksz_rmw32(struct ksz_device *dev, u32 reg, u32 mask,
+			    u32 value)
+{
+	int ret;
+
+	ret = regmap_update_bits(dev->regmap[2], reg, mask, value);
+	if (ret)
+		dev_err(dev->dev, "can't rmw 32bit reg: 0x%x %pe\n", reg,
+			ERR_PTR(ret));
+
+	return ret;
+}
+
 static inline int ksz_write64(struct ksz_device *dev, u32 reg, u64 value)
 {
 	u32 val[2];
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 5506adaac488..ce5134cc917f 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -26,6 +26,84 @@
 
 #define KSZ_PTP_INT_START 13
 
+#define KSZ_PER_OUT_TOU 0   /* trigger output unit 0 */
+
+static int ksz_ptp_restart_perout(struct ksz_device *dev);
+
+static int ksz_ptp_tou_gpio(struct ksz_device *dev)
+{
+	int ret;
+
+	/* Set the Led Override register */
+	ret = ksz_rmw32(dev, REG_SW_GLOBAL_LED_OVR__4, LED_OVR_1, LED_OVR_1);
+	if (ret)
+		return ret;
+
+	/* Set the Led Source register */
+	return ksz_rmw32(dev, REG_SW_GLOBAL_LED_SRC__4, LED_SRC_PTP_GPIO_1,
+			 LED_SRC_PTP_GPIO_1);
+}
+
+/* Shared register access routines (Trigger Output Unit) */
+static int ksz_ptp_tou_reset(struct ksz_device *dev, u8 unit)
+{
+	u32 data;
+	int ret;
+
+	/* Reset trigger unit (clears TRIGGER_EN, but not GPIOSTATx) */
+	ret = ksz_rmw32(dev, REG_PTP_CTRL_STAT__4, TRIG_RESET, TRIG_RESET);
+
+	/* Clear DONE */
+	data = FIELD_PREP(TRIG_DONE_M, (1 << unit));
+	ret = ksz_write32(dev, REG_PTP_TRIG_STATUS__4, data);
+	if (ret)
+		return ret;
+
+	/* Clear IRQ */
+	data = FIELD_PREP(TRIG_INT_M, (1 << unit));
+	ret = ksz_write32(dev, REG_PTP_INT_STATUS__4, data);
+	if (ret)
+		return ret;
+
+	/* Clear reset and set GPIO direction */
+	return ksz_rmw32(dev, REG_PTP_CTRL_STAT__4, (TRIG_RESET | TRIG_ENABLE),
+			 0);
+}
+
+static int ksz_ptp_tou_pulse_verify(u64 pulse_ns)
+{
+	u32 data;
+
+	if (pulse_ns & 0x3)
+		return -EINVAL;
+
+	data = (pulse_ns / 8);
+	if (!FIELD_FIT(TRIG_PULSE_WIDTH_M, data))
+		return -ERANGE;
+
+	return 0;
+}
+
+static int ksz_ptp_tou_target_time_set(struct ksz_device *dev,
+				       struct timespec64 const *ts)
+{
+	int ret;
+
+	/* Hardware has only 32 bit */
+	if ((ts->tv_sec & 0xffffffff) != ts->tv_sec)
+		return -EINVAL;
+
+	ret = ksz_write32(dev, REG_TRIG_TARGET_NANOSEC, ts->tv_nsec);
+	if (ret)
+		return ret;
+
+	ret = ksz_write32(dev, REG_TRIG_TARGET_SEC, ts->tv_sec);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int ksz_ptp_enable_mode(struct ksz_device *dev, bool enable)
 {
 	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
@@ -306,6 +384,20 @@ static int ksz_ptp_settime(struct ptp_clock_info *ptp,
 	if (ret)
 		goto error_return;
 
+	switch (ptp_data->tou_mode) {
+	case KSZ_PTP_TOU_IDLE:
+		break;
+
+	case KSZ_PTP_TOU_PEROUT:
+		dev_info(dev->dev, "Restarting periodic output signal\n");
+
+		ret = ksz_ptp_restart_perout(dev);
+		if (ret)
+			goto error_return;
+
+		break;
+	}
+
 	spin_lock_bh(&ptp_data->clock_lock);
 	ptp_data->clock_time = *ts;
 	spin_unlock_bh(&ptp_data->clock_lock);
@@ -399,6 +491,20 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	if (ret)
 		goto error_return;
 
+	switch (ptp_data->tou_mode) {
+	case KSZ_PTP_TOU_IDLE:
+		break;
+
+	case KSZ_PTP_TOU_PEROUT:
+		dev_info(dev->dev, "Restarting periodic output signal\n");
+
+		ret = ksz_ptp_restart_perout(dev);
+		if (ret)
+			goto error_return;
+
+		break;
+	}
+
 	spin_lock_bh(&ptp_data->clock_lock);
 	ptp_data->clock_time = timespec64_add(ptp_data->clock_time, delta64);
 	spin_unlock_bh(&ptp_data->clock_lock);
@@ -408,6 +514,222 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	return ret;
 }
 
+static int ksz9477_ptp_tou_start(struct ksz_device *dev)
+{
+	u32 data;
+	int ret;
+
+	ret = ksz_rmw32(dev, REG_PTP_CTRL_STAT__4, (GPIO_OUT | TRIG_ENABLE),
+			(GPIO_OUT | TRIG_ENABLE));
+	if (ret)
+		return ret;
+
+	/* Check error flag:
+	 * - the ACTIVE flag is NOT cleared an error!
+	 */
+	ret = ksz_read32(dev, REG_PTP_TRIG_STATUS__4, &data);
+	if (ret)
+		return ret;
+
+	if (FIELD_GET(TRIG_ERROR_M, data) & (1 << KSZ_PER_OUT_TOU)) {
+		dev_err(dev->dev, "%s: Trigger unit0 error!\n", __func__);
+		ret = -EIO;
+		/* Unit will be reset on next access */
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ksz_ptp_configure_perout(struct ksz_device *dev,
+				    u32 cycle_width_ns,
+				    u32 pulse_width_ns,
+				    struct timespec64 const *target_time)
+{
+	u32 data;
+	int ret;
+
+	/* Enable notify, set rising edge, set periodic pattern */
+	data = FIELD_PREP(TRIG_NOTIFY, 1) |
+	       FIELD_PREP(TRIG_PATTERN_M, TRIG_POS_PERIOD);
+	ret = ksz_write32(dev, REG_TRIG_CTRL__4, data);
+	if (ret)
+		return ret;
+
+	/* Set Cycle width */
+	ret = ksz_write32(dev, REG_TRIG_CYCLE_WIDTH, cycle_width_ns);
+	if (ret)
+		return ret;
+
+	/* Set cycle count 0 - Infinite */
+	ret = ksz_rmw32(dev, REG_TRIG_CYCLE_CNT, TRIG_CYCLE_CNT_M, 0);
+	if (ret)
+		return ret;
+
+	/* Set Pulse width units of 8ns */
+	data = (pulse_width_ns / 8);
+	ret = ksz_write32(dev, REG_TRIG_PULSE_WIDTH__4, data);
+	if (ret)
+		return ret;
+
+	ret = ksz_ptp_tou_target_time_set(dev, target_time);
+	if (ret)
+		return ret;
+
+	/* Configure GPIO pins */
+	ret = ksz_ptp_tou_gpio(dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+#define KSZ_PEROUT_VALID_FLAGS ( \
+				 PTP_PEROUT_DUTY_CYCLE \
+				 )
+
+static int ksz_ptp_enable_perout(struct ksz_device *dev,
+				 struct ptp_perout_request const *perout_request,
+				 int on)
+{
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	u64 cycle_width_ns;
+	u64 pulse_width_ns;
+	int ret;
+
+	if (perout_request->flags & ~KSZ_PEROUT_VALID_FLAGS)
+		return -EINVAL;
+
+	if (ptp_data->tou_mode != KSZ_PTP_TOU_PEROUT &&
+	    ptp_data->tou_mode != KSZ_PTP_TOU_IDLE)
+		return -EBUSY;
+
+	ret = ksz_ptp_tou_reset(dev, KSZ_PER_OUT_TOU);
+	if (ret)
+		return ret;
+
+	if (!on) {
+		ptp_data->tou_mode = KSZ_PTP_TOU_IDLE;
+		return 0;  /* success */
+	}
+
+	ptp_data->perout_target_time_first.tv_sec  = perout_request->start.sec;
+	ptp_data->perout_target_time_first.tv_nsec = perout_request->start.nsec;
+
+	ptp_data->perout_period.tv_sec = perout_request->period.sec;
+	ptp_data->perout_period.tv_nsec = perout_request->period.nsec;
+
+	cycle_width_ns = timespec64_to_ns(&ptp_data->perout_period);
+	if ((cycle_width_ns & TRIG_CYCLE_WIDTH_M) != cycle_width_ns)
+		return -EINVAL;
+
+	if (perout_request->flags & PTP_PEROUT_DUTY_CYCLE)
+		pulse_width_ns = perout_request->on.sec * NSEC_PER_SEC +
+			perout_request->on.nsec;
+
+	else
+		/* Use a duty cycle of 50%. Maximum pulse width supported by the
+		 * hardware is a little bit more than 125 ms.
+		 */
+		pulse_width_ns = min_t(u64,
+				       (perout_request->period.sec * NSEC_PER_SEC
+					+ perout_request->period.nsec) / 2
+				       / 8 * 8,
+				       125000000LL);
+
+	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns);
+	if (ret)
+		return ret;
+
+	ret = ksz_ptp_configure_perout(dev, cycle_width_ns,
+				       pulse_width_ns,
+				       &ptp_data->perout_target_time_first);
+	if (ret)
+		return ret;
+
+	/* Activate trigger unit */
+	ret = ksz9477_ptp_tou_start(dev);
+	if (ret)
+		return ret;
+
+	ptp_data->tou_mode = KSZ_PTP_TOU_PEROUT;
+
+	return 0;
+}
+
+static int ksz_ptp_restart_perout(struct ksz_device *dev)
+{
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	s64 now_ns, first_ns, period_ns, next_ns;
+	struct timespec64 now;
+	unsigned int count;
+	int ret;
+
+	ret = _ksz_ptp_gettime(dev, &now);
+	if (ret)
+		return ret;
+
+	now_ns = timespec64_to_ns(&now);
+	first_ns = timespec64_to_ns(&ptp_data->perout_target_time_first);
+
+	/* Calculate next perout event based on start time and period */
+	period_ns = timespec64_to_ns(&ptp_data->perout_period);
+
+	if (first_ns < now_ns) {
+		count = div_u64(now_ns - first_ns, period_ns);
+		next_ns = first_ns + count * period_ns;
+	} else {
+		next_ns = first_ns;
+	}
+
+	/* Ensure 100 ms guard time prior next event */
+	while (next_ns < now_ns + 100000000)
+		next_ns += period_ns;
+
+	/* Restart periodic output signal */
+	{
+		struct timespec64 next = ns_to_timespec64(next_ns);
+		struct ptp_perout_request perout_request = {
+			.start = {
+				.sec  = next.tv_sec,
+				.nsec = next.tv_nsec
+			},
+			.period = {
+				.sec  = ptp_data->perout_period.tv_sec,
+				.nsec = ptp_data->perout_period.tv_nsec
+			},
+			.index = 0,
+			.flags = 0,  /* keep current values */
+		};
+		ret = ksz_ptp_enable_perout(dev, &perout_request, 1);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ksz_ptp_enable(struct ptp_clock_info *ptp,
+			  struct ptp_clock_request *req, int on)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	struct ptp_perout_request *perout_request = &req->perout;
+	int ret;
+
+	switch (req->type) {
+	case PTP_CLK_REQ_PEROUT:
+		mutex_lock(&ptp_data->lock);
+		ret = ksz_ptp_enable_perout(dev, perout_request, on);
+		mutex_unlock(&ptp_data->lock);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
 /*  Function is pointer to the do_aux_work in the ptp_clock capability */
 static long ksz_ptp_do_aux_work(struct ptp_clock_info *ptp)
 {
@@ -521,6 +843,8 @@ static const struct ptp_clock_info ksz_ptp_caps = {
 	.adjfine	= ksz_ptp_adjfine,
 	.adjtime	= ksz_ptp_adjtime,
 	.do_aux_work	= ksz_ptp_do_aux_work,
+	.enable		= ksz_ptp_enable,
+	.n_per_out	= 1,
 };
 
 int ksz_ptp_clock_register(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index 7d6786caa633..5f13fbc56820 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -8,6 +8,11 @@
 
 #if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ_PTP)
 
+enum ksz_ptp_tou_mode {
+	KSZ_PTP_TOU_IDLE,
+	KSZ_PTP_TOU_PEROUT,
+};
+
 struct ksz_ptp_data {
 	struct ptp_clock_info caps;
 	struct ptp_clock *clock;
@@ -16,6 +21,9 @@ struct ksz_ptp_data {
 	/* lock for accessing the clock_time */
 	spinlock_t clock_lock;
 	struct timespec64 clock_time;
+	enum ksz_ptp_tou_mode tou_mode;
+	struct timespec64 perout_target_time_first;  /* start of first perout pulse */
+	struct timespec64 perout_period;
 };
 
 int ksz_ptp_clock_register(struct dsa_switch *ds);
diff --git a/drivers/net/dsa/microchip/ksz_ptp_reg.h b/drivers/net/dsa/microchip/ksz_ptp_reg.h
index ccb87bbdfbcd..df3b4371c49b 100644
--- a/drivers/net/dsa/microchip/ksz_ptp_reg.h
+++ b/drivers/net/dsa/microchip/ksz_ptp_reg.h
@@ -3,6 +3,14 @@
  * Copyright (C) 2019-2021 Microchip Technology Inc.
  */
 
+#define REG_SW_GLOBAL_LED_OVR__4	0x0120
+#define LED_OVR_2			BIT(1)
+#define LED_OVR_1			BIT(0)
+
+#define REG_SW_GLOBAL_LED_SRC__4	0x0128
+#define LED_SRC_PTP_GPIO_1		BIT(3)
+#define LED_SRC_PTP_GPIO_2		BIT(2)
+
 /* 5 - PTP Clock */
 #define REG_PTP_CLK_CTRL		0x0500
 
@@ -51,6 +59,63 @@
 #define PTP_MASTER			BIT(1)
 #define PTP_1STEP			BIT(0)
 
+#define REG_PTP_TRIG_STATUS__4		0x0524
+
+#define TRIG_ERROR_M			GENMASK(18, 16)
+#define TRIG_DONE_M			GENMASK(2, 0)
+
+#define REG_PTP_INT_STATUS__4		0x0528
+
+#define TRIG_INT_M			GENMASK(18, 16)
+#define TS_INT_M			GENMASK(1, 0)
+
+#define REG_PTP_CTRL_STAT__4           0x052C
+
+#define GPIO_IN                        BIT(7)
+#define GPIO_OUT                       BIT(6)
+#define TS_INT_ENABLE                  BIT(5)
+#define TRIG_ACTIVE                    BIT(4)
+#define TRIG_ENABLE                    BIT(3)
+#define TRIG_RESET                     BIT(2)
+#define TS_ENABLE                      BIT(1)
+#define TS_RESET                       BIT(0)
+
+#define REG_TRIG_TARGET_NANOSEC        0x0530
+#define REG_TRIG_TARGET_SEC            0x0534
+
+#define REG_TRIG_CTRL__4               0x0538
+
+#define TRIG_CASCADE_ENABLE            BIT(31)
+#define TRIG_CASCADE_TAIL              BIT(30)
+#define TRIG_CASCADE_UPS_M             GENMASK(29, 26)
+#define TRIG_NOW                       BIT(25)
+#define TRIG_NOTIFY                    BIT(24)
+#define TRIG_EDGE                      BIT(23)
+#define TRIG_PATTERN_M		       GENMASK(22, 20)
+#define TRIG_NEG_EDGE                  0
+#define TRIG_POS_EDGE                  1
+#define TRIG_NEG_PULSE                 2
+#define TRIG_POS_PULSE                 3
+#define TRIG_NEG_PERIOD                4
+#define TRIG_POS_PERIOD                5
+#define TRIG_REG_OUTPUT                6
+#define TRIG_GPO_M		       GENMASK(19, 16)
+#define TRIG_CASCADE_ITERATE_CNT_M     GENMASK(15, 0)
+
+#define REG_TRIG_CYCLE_WIDTH           0x053C
+#define TRIG_CYCLE_WIDTH_M	       GENMASK(31, 0)
+
+#define REG_TRIG_CYCLE_CNT             0x0540
+
+#define TRIG_CYCLE_CNT_M	       GENMASK(31, 16)
+#define TRIG_BIT_PATTERN_M             GENMASK(15, 0)
+
+#define REG_TRIG_ITERATE_TIME          0x0544
+
+#define REG_TRIG_PULSE_WIDTH__4        0x0548
+
+#define TRIG_PULSE_WIDTH_M             GENMASK(23, 0)
+
 /* Port PTP Register */
 #define REG_PTP_PORT_RX_DELAY__2	0x0C00
 #define REG_PTP_PORT_TX_DELAY__2	0x0C02
-- 
2.36.1

Re: [RFC Patch net-next v2 8/8] net: dsa: microchip: ptp: add periodic output signal

[Richard Cochran]

On Mon, Nov 21, 2022 at 09:11:50PM +0530, Arun Ramadoss wrote:

> +static int ksz_ptp_enable_perout(struct ksz_device *dev,
> +				 struct ptp_perout_request const *perout_request,
> +				 int on)
> +{
> +	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +	u64 cycle_width_ns;
> +	u64 pulse_width_ns;
> +	int ret;
> +
> +	if (perout_request->flags & ~KSZ_PEROUT_VALID_FLAGS)
> +		return -EINVAL;
> +
> +	if (ptp_data->tou_mode != KSZ_PTP_TOU_PEROUT &&
> +	    ptp_data->tou_mode != KSZ_PTP_TOU_IDLE)
> +		return -EBUSY;
> +
> +	ret = ksz_ptp_tou_reset(dev, KSZ_PER_OUT_TOU);
> +	if (ret)
> +		return ret;
> +
> +	if (!on) {
> +		ptp_data->tou_mode = KSZ_PTP_TOU_IDLE;
> +		return 0;  /* success */
> +	}
> +
> +	ptp_data->perout_target_time_first.tv_sec  = perout_request->start.sec;
> +	ptp_data->perout_target_time_first.tv_nsec = perout_request->start.nsec;
> +
> +	ptp_data->perout_period.tv_sec = perout_request->period.sec;
> +	ptp_data->perout_period.tv_nsec = perout_request->period.nsec;
> +
> +	cycle_width_ns = timespec64_to_ns(&ptp_data->perout_period);
> +	if ((cycle_width_ns & TRIG_CYCLE_WIDTH_M) != cycle_width_ns)
> +		return -EINVAL;
> +
> +	if (perout_request->flags & PTP_PEROUT_DUTY_CYCLE)
> +		pulse_width_ns = perout_request->on.sec * NSEC_PER_SEC +
> +			perout_request->on.nsec;
> +
> +	else
> +		/* Use a duty cycle of 50%. Maximum pulse width supported by the
> +		 * hardware is a little bit more than 125 ms.
> +		 */
> +		pulse_width_ns = min_t(u64,
> +				       (perout_request->period.sec * NSEC_PER_SEC
> +					+ perout_request->period.nsec) / 2
> +				       / 8 * 8,
> +				       125000000LL);

CodyStyle nit: if/else bodies need {} because of two lines in 'else'.

Thanks,
Richard


> +
> +	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns);
> +	if (ret)
> +		return ret;
> +
> +	ret = ksz_ptp_configure_perout(dev, cycle_width_ns,
> +				       pulse_width_ns,
> +				       &ptp_data->perout_target_time_first);
> +	if (ret)
> +		return ret;
> +
> +	/* Activate trigger unit */
> +	ret = ksz9477_ptp_tou_start(dev);
> +	if (ret)
> +		return ret;
> +
> +	ptp_data->tou_mode = KSZ_PTP_TOU_PEROUT;
> +
> +	return 0;
> +}

Re: [RFC Patch net-next v2 8/8] net: dsa: microchip: ptp: add periodic output signal

[Richard Cochran]

On Mon, Nov 21, 2022 at 09:11:50PM +0530, Arun Ramadoss wrote:

> +static int ksz_ptp_restart_perout(struct ksz_device *dev)
> +{
> +	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +	s64 now_ns, first_ns, period_ns, next_ns;
> +	struct timespec64 now;
> +	unsigned int count;
> +	int ret;
> +
> +	ret = _ksz_ptp_gettime(dev, &now);
> +	if (ret)
> +		return ret;
> +
> +	now_ns = timespec64_to_ns(&now);
> +	first_ns = timespec64_to_ns(&ptp_data->perout_target_time_first);
> +
> +	/* Calculate next perout event based on start time and period */
> +	period_ns = timespec64_to_ns(&ptp_data->perout_period);
> +
> +	if (first_ns < now_ns) {
> +		count = div_u64(now_ns - first_ns, period_ns);
> +		next_ns = first_ns + count * period_ns;
> +	} else {
> +		next_ns = first_ns;
> +	}
> +
> +	/* Ensure 100 ms guard time prior next event */
> +	while (next_ns < now_ns + 100000000)
> +		next_ns += period_ns;
> +
> +	/* Restart periodic output signal */
> +	{

CodingStyle: avoid anonymous blocks.  Move to helper function instead?

Thanks,
Richard


> +		struct timespec64 next = ns_to_timespec64(next_ns);
> +		struct ptp_perout_request perout_request = {
> +			.start = {
> +				.sec  = next.tv_sec,
> +				.nsec = next.tv_nsec
> +			},
> +			.period = {
> +				.sec  = ptp_data->perout_period.tv_sec,
> +				.nsec = ptp_data->perout_period.tv_nsec
> +			},
> +			.index = 0,
> +			.flags = 0,  /* keep current values */
> +		};
> +		ret = ksz_ptp_enable_perout(dev, &perout_request, 1);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}

Re: [RFC Patch net-next v2 0/8] net: dsa: microchip: add PTP support for KSZ9x and LAN937x

[Vladimir Oltean]

Hi Arun,

On Mon, Nov 21, 2022 at 09:11:42PM +0530, Arun Ramadoss wrote:
> The LAN937x switch has capable for supporting IEEE 1588 PTP protocol. This
> patch series add PTP support and tested using the ptp4l application.
> LAN937x has the same PTP register set similar to KSZ9563, hence the
> implementation has been made common for the ksz switches.
> KSZ9563 does not support two step timestamping but LAN937x supports both.
> Tested the 1step & 2step p2p timestamping in LAN937x and p2p1step
> timestamping in KSZ9563.

A process-related pattern I noticed in your patches. The Author: is in
general the same as the first Signed-off-by:. I don't know of cases
where that's not true.

There can be more subsequent Signed-off-by: tags, and those are people
through the hands of whom those patches have passed, and who might have
made changes to them.

When you use Christian's patches (verbatim or with non-radical rework,
like fixes here and there, styling rework, commit message rewrite),
you need Christian to appear in the Author: and first Signed-off-by:
field, and you in the second. When patches are more or less a complete
rework (such that it no longer resembles Christian's original intentions
and it would be misleading to put his sign off on something which he did
not write), you can put yourself as author and first sign off, and use
Co-developed-by: + Signed-off-by for Christian's work (the sign off
still seems to be required for some reason). You need to use your
judgement here, you can't always put your name on others' work.
You can also say "based on a previous patch posted on the mailing lists
which was heavily reworked" and provide a Link: tag with a
lore.kernel.org or patchwork.kernel.org link. Under the "---" sign in
the patch you can also clarify the changes you've made, if you decide to
keep Christian's authorship but make significant but not radical changes.
These annotations will always be visible in patchwork even if not in
git. At least that's what I would do.