2022-10-14 15:46:56

by Arun Ramadoss

[permalink] [raw]
Subject: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

The LAN937x switch has capable for supporting IEEE 1588 PTP protocol. This
patch series add gPTP profile 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. But the testing is
done only for lan937x switch.

Arun Ramadoss (6):
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

drivers/net/dsa/microchip/Kconfig | 10 +
drivers/net/dsa/microchip/Makefile | 1 +
drivers/net/dsa/microchip/ksz_common.c | 43 +-
drivers/net/dsa/microchip/ksz_common.h | 31 +
drivers/net/dsa/microchip/ksz_ptp.c | 755 ++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz_ptp.h | 84 +++
drivers/net/dsa/microchip/ksz_ptp_reg.h | 68 +++
include/linux/dsa/ksz_common.h | 53 ++
net/dsa/tag_ksz.c | 156 ++++-
9 files changed, 1192 insertions(+), 9 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


base-commit: 66ae04368efbe20eb8951c9a76158f99ce672f25
--
2.36.1


2022-10-14 15:47:06

by Arun Ramadoss

[permalink] [raw]
Subject: [RFC Patch net-next 1/6] net: dsa: microchip: adding the posix clock support

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: Arun Ramadoss <[email protected]>
---
drivers/net/dsa/microchip/Kconfig | 10 +
drivers/net/dsa/microchip/Makefile | 1 +
drivers/net/dsa/microchip/ksz_common.c | 14 +-
drivers/net/dsa/microchip/ksz_common.h | 17 ++
drivers/net/dsa/microchip/ksz_ptp.c | 269 ++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz_ptp.h | 43 ++++
drivers/net/dsa/microchip/ksz_ptp_reg.h | 52 +++++
7 files changed, 405 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..1e9712ff64e2 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -21,6 +21,16 @@ config NET_DSA_MICROCHIP_KSZ_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..29d2d00ea27f 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -6,4 +6,5 @@ ksz_switch-objs += ksz8795.o
ksz_switch-objs += lan937x_main.o
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_KSZ_PTP) += ksz_ptp.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 d612181b3226..084563e80660 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

@@ -1990,10 +1991,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 */
@@ -2002,6 +2009,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)
@@ -2018,6 +2027,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);
@@ -2831,6 +2842,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 9cfa179575ce..f936a4100423 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 */
@@ -442,6 +446,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..0ead0e097ed5
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -0,0 +1,269 @@
+// 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("Arun Ramadoss <[email protected]>");
+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

2022-10-14 15:47:28

by Arun Ramadoss

[permalink] [raw]
Subject: [RFC Patch net-next 3/6] net: dsa: microchip: Manipulating absolute time using ptp hw clock

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: Arun Ramadoss <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.h | 1 +
drivers/net/dsa/microchip/ksz_ptp.c | 55 +++++++++++++++++++++++++-
drivers/net/dsa/microchip/ksz_ptp.h | 3 ++
3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 0e5f02d3992e..b15bcb6251e9 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 5199840377aa..c4cef3884a4d 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);
}

@@ -223,6 +233,10 @@ 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);

+ 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);

@@ -276,6 +290,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;
@@ -309,14 +324,48 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)

ret = ksz_write16(dev, REG_PTP_CLK_CTRL, data16);

+ 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 = {
@@ -327,6 +376,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)
@@ -336,6 +386,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

2022-10-14 15:47:31

by Arun Ramadoss

[permalink] [raw]
Subject: [RFC Patch net-next 5/6] net: dsa: microchip: Adding the ptp packet reception logic

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: Arun Ramadoss <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 12 +++++++++++
drivers/net/dsa/microchip/ksz_ptp.c | 25 +++++++++++++++++++++
drivers/net/dsa/microchip/ksz_ptp.h | 6 ++++++
include/linux/dsa/ksz_common.h | 15 +++++++++++++
net/dsa/tag_ksz.c | 30 ++++++++++++++++++++++----
5 files changed, 84 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 0c0fdb7b7879..388731959b23 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2426,6 +2426,17 @@ 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;
+
+ return 0;
+}
+
static int ksz_port_vlan_filtering(struct dsa_switch *ds, int port,
bool flag, struct netlink_ext_ack *extack)
{
@@ -2819,6 +2830,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 2cae543f7e0b..5ae6eedb6b39 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -372,6 +372,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..9589909ab7d5 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -28,6 +28,7 @@ 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);

#else

@@ -64,6 +65,11 @@ 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;
+}
+
#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..82edd7228b3c 100644
--- a/include/linux/dsa/ksz_common.h
+++ b/include/linux/dsa/ksz_common.h
@@ -9,9 +9,24 @@

#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);
};

static inline struct ksz_tagger_data *
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index ca1261b04fe7..937a3e70b471 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -164,6 +164,25 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8795);
#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;
+ ktime_t tstamp;
+
+ 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);
+}
+
static struct sk_buff *ksz9477_xmit(struct sk_buff *skb,
struct net_device *dev)
{
@@ -197,8 +216,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);
}
@@ -257,10 +278,11 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893);
* 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)
*/
@@ -304,7 +326,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

2022-10-14 15:49:32

by Arun Ramadoss

[permalink] [raw]
Subject: [RFC Patch net-next 4/6] net: dsa: microchip: enable the ptp interrupt for timestamping

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 <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 15 +-
drivers/net/dsa/microchip/ksz_common.h | 10 ++
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, 249 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index d8ec5b641b89..0c0fdb7b7879 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1988,13 +1988,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);
@@ -2011,6 +2015,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)
@@ -2030,8 +2038,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 b15bcb6251e9..3a640ca4f7e1 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -85,6 +85,13 @@ struct ksz_irq {
struct ksz_device *dev;
};

+struct ksz_ptp_irq {
+ struct ksz_device *dev;
+ u16 ts_reg;
+ char name[16];
+ int irq_num;
+};
+
struct ksz_port {
bool remove_tag; /* Remove Tag flag set, for ksz8795 only */
bool learning;
@@ -106,6 +113,8 @@ struct ksz_port {
struct ksz_irq pirq;
u8 num;
#if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ_PTP)
+ struct ksz_irq ptpirq;
+ struct ksz_ptp_irq ptpmsg_irq[3];
bool hwts_tx_en;
#endif
};
@@ -605,6 +614,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 c4cef3884a4d..2cae543f7e0b 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;
@@ -422,6 +426,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->dev = port->ksz_dev;
+ 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("Arun Ramadoss <[email protected]>");
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
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

2022-10-14 16:17:38

by Arun Ramadoss

[permalink] [raw]
Subject: [RFC Patch net-next 2/6] net: dsa: microchip: Initial hardware time stamping support

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: Arun Ramadoss <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 2 +
drivers/net/dsa/microchip/ksz_common.h | 3 +
drivers/net/dsa/microchip/ksz_ptp.c | 111 ++++++++++++++++++++++++-
drivers/net/dsa/microchip/ksz_ptp.h | 14 ++++
include/linux/dsa/ksz_common.h | 23 +++++
net/dsa/tag_ksz.c | 59 +++++++++++++
6 files changed, 210 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 084563e80660..d8ec5b641b89 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2843,6 +2843,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 f936a4100423..0e5f02d3992e 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)
+ bool 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 0ead0e097ed5..5199840377aa 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,110 @@ 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_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;
+
+ if (dev->ports[port].hwts_tx_en)
+ config.tx_type = HWTSTAMP_TX_ON;
+ else
+ config.tx_type = HWTSTAMP_TX_OFF;
+
+ 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:
+ prt->hwts_tx_en = false;
+ break;
+ case HWTSTAMP_TX_ON:
+ prt->hwts_tx_en = true;
+ 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 38fa19c1e2d5..ca1261b04fe7 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>
@@ -13,6 +14,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)
@@ -245,6 +302,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

2022-10-14 16:20:02

by Arun Ramadoss

[permalink] [raw]
Subject: [RFC Patch net-next 6/6] net: dsa: microchip: add the transmission tstamp logic

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: Rakesh Sankaranarayanan <[email protected]>
Signed-off-by: Arun Ramadoss <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 2 +
drivers/net/dsa/microchip/ksz_ptp.c | 104 ++++++++++++++++++++++++-
drivers/net/dsa/microchip/ksz_ptp.h | 9 +++
include/linux/dsa/ksz_common.h | 15 ++++
net/dsa/tag_ksz.c | 67 +++++++++++++++-
5 files changed, 193 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 388731959b23..47232a08ed94 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2432,6 +2432,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;

return 0;
@@ -2868,6 +2869,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_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 5ae6eedb6b39..d11a490a6c87 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -163,6 +163,46 @@ 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_PDELAY_REQ:
+ case PTP_MSGTYPE_PDELAY_RESP:
+ case PTP_MSGTYPE_SYNC:
+ break;
+
+ default:
+ return;
+ }
+
+ clone = skb_clone_sk(skb);
+ if (!clone)
+ return;
+
+ KSZ_SKB_CB(skb)->clone = clone;
+}
+
/* These are function related to the ptp clock info */
static int _ksz_ptp_gettime(struct ksz_device *dev, struct timespec64 *ts)
{
@@ -397,6 +437,49 @@ 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 ksz_ptp_data *ptp_data = &dev->ptp_data;
+ 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(&ptp_data->tstamp_msg_comp,
+ msecs_to_jiffies(100));
+ if (!ret)
+ return;
+
+ hwtstamps.hwtstamp = ptp_data->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_ptp_data *ptp_data = &dev->ptp_data;
+
+ clone = KSZ_SKB_CB(skb)->clone;
+
+ reinit_completion(&ptp_data->tstamp_msg_comp);
+
+ /* Transfer skb to the host port. */
+ dsa_enqueue_skb(skb, skb->dev);
+
+ ksz_ptp_txtstamp_skb(dev, dev->ports, clone);
+
+ kfree(xmit_work);
+}
+
static const struct ptp_clock_info ksz_ptp_caps = {
.owner = THIS_MODULE,
.name = "Microchip Clock",
@@ -433,6 +516,8 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
if (ret)
goto error_unregister_clock;

+ init_completion(&ptp_data->tstamp_msg_comp);
+
return 0;

error_unregister_clock:
@@ -453,7 +538,24 @@ 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 = ptpmsg_irq->dev;
+ struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+ u32 tstamp_raw;
+ ktime_t tstamp;
+ int ret;
+
+ ret = ksz_read32(dev, ptpmsg_irq->ts_reg, &tstamp_raw);
+ if (ret)
+ return IRQ_NONE;
+
+ tstamp = ksz_decode_tstamp(tstamp_raw);
+
+ ptp_data->tstamp_msg = ksz_tstamp_reconstruct(dev->ds, tstamp);
+
+ complete(&ptp_data->tstamp_msg_comp);
+
+ return IRQ_HANDLED;
}

static irqreturn_t ksz_ptp_irq_thread_fn(int irq, void *dev_id)
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index 9589909ab7d5..b2035a0bcbb2 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -13,6 +13,8 @@ struct ksz_ptp_data {
struct ptp_clock *clock;
/* Serializes all operations on the PTP hardware clock */
struct mutex lock;
+ ktime_t tstamp_msg;
+ struct completion tstamp_msg_comp;
/* lock for accessing the clock_time */
spinlock_t clock_lock;
struct timespec64 clock_time;
@@ -26,8 +28,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);

#else
@@ -58,6 +62,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;
@@ -65,6 +72,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/include/linux/dsa/ksz_common.h b/include/linux/dsa/ksz_common.h
index 82edd7228b3c..15550fd88692 100644
--- a/include/linux/dsa/ksz_common.h
+++ b/include/linux/dsa/ksz_common.h
@@ -23,12 +23,27 @@ 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);
};

+struct ksz_skb_cb {
+ struct sk_buff *clone;
+ unsigned int ptp_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 937a3e70b471..582add3398d3 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -19,6 +19,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 *
@@ -48,6 +49,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;
}
@@ -55,12 +57,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;
@@ -271,10 +284,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);

-/* 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)
*
@@ -293,14 +307,61 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893);
#define LAN937X_TAIL_TAG_VALID BIT(13)
#define LAN937X_TAIL_TAG_PORT_MASK 7

+static void lan937x_xmit_timestamp(struct sk_buff *skb)
+{
+ put_unaligned_be32(0, skb_put(skb, KSZ9477_PTP_TAG_LEN));
+}
+
+static struct sk_buff *lan937x_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 *lan937x_xmit(struct sk_buff *skb,
struct net_device *dev)
{
struct dsa_port *dp = dsa_slave_to_port(dev);
const struct ethhdr *hdr = eth_hdr(skb);
+ struct ksz_tagger_private *priv;
__be16 *tag;
u16 val;

+ priv = ksz_tagger_private(dp->ds);
+
+ /* Tag encoding */
+ if (test_bit(KSZ_HWTS_EN, &priv->state))
+ lan937x_xmit_timestamp(skb);
+
if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
return NULL;

@@ -316,7 +377,7 @@ static struct sk_buff *lan937x_xmit(struct sk_buff *skb,

put_unaligned_be16(val, tag);

- return skb;
+ return lan937x_defer_xmit(dp, skb);
}

static const struct dsa_device_ops lan937x_netdev_ops = {
--
2.36.1

2022-10-17 18:08:01

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

Hi Arun,

On Fri, Oct 14, 2022 at 08:58:51PM +0530, Arun Ramadoss wrote:
> The LAN937x switch has capable for supporting IEEE 1588 PTP protocol. This
> patch series add gPTP profile 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. But the testing is
> done only for lan937x switch.

Would it be possible to actually test these patches on KSZ9563?

Christian Eggers tried to add PTP support for this switch a while ago,
and he claims that two-step TX timestamping was de-featured for KSZ95xx
due to hardware errata.
https://patchwork.ozlabs.org/project/netdev/patch/[email protected]/

>
> Arun Ramadoss (6):
> 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
>
> drivers/net/dsa/microchip/Kconfig | 10 +
> drivers/net/dsa/microchip/Makefile | 1 +
> drivers/net/dsa/microchip/ksz_common.c | 43 +-
> drivers/net/dsa/microchip/ksz_common.h | 31 +
> drivers/net/dsa/microchip/ksz_ptp.c | 755 ++++++++++++++++++++++++
> drivers/net/dsa/microchip/ksz_ptp.h | 84 +++
> drivers/net/dsa/microchip/ksz_ptp_reg.h | 68 +++
> include/linux/dsa/ksz_common.h | 53 ++
> net/dsa/tag_ksz.c | 156 ++++-
> 9 files changed, 1192 insertions(+), 9 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
>
>
> base-commit: 66ae04368efbe20eb8951c9a76158f99ce672f25
> --
> 2.36.1
>

2022-10-17 19:01:03

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

On Fri, Oct 14, 2022 at 08:58:51PM +0530, Arun Ramadoss wrote:
> The LAN937x switch has capable for supporting IEEE 1588 PTP protocol. This
> patch series add gPTP profile 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. But the testing is
> done only for lan937x switch.

Unrelated to the proposed implementation. What user space stack do you
use for gPTP bridging?

2022-10-18 07:18:49

by Arun Ramadoss

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

On Mon, 2022-10-17 at 21:46 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
Hi Vladimir,

> On Fri, Oct 14, 2022 at 08:58:51PM +0530, Arun Ramadoss wrote:
> > The LAN937x switch has capable for supporting IEEE 1588 PTP
> > protocol. This
> > patch series add gPTP profile 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. But the
> > testing is
> > done only for lan937x switch.
>
> Unrelated to the proposed implementation. What user space stack do
> you
> use for gPTP bridging?
I had used LinuxPTP stack for testing this patch set and in specific
linuxptp/configs/gptp.cfg

Test Setup is of
LAN9370 DUT1 <LAN1> --- LAN9370 DUT2 <LAN1>

Ran the below command in both DUTS
#ptp4l -f ~/linuxptp/configs/gptp.cfg -i lan1

-
Arun

2022-10-18 07:24:02

by Arun Ramadoss

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

On Mon, 2022-10-17 at 20:19 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>

Hi Vladimir,
Thanks for the comment.

> Hi Arun,
>
> On Fri, Oct 14, 2022 at 08:58:51PM +0530, Arun Ramadoss wrote:
> > The LAN937x switch has capable for supporting IEEE 1588 PTP
> > protocol. This
> > patch series add gPTP profile 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. But the
> > testing is
> > done only for lan937x switch.
>
> Would it be possible to actually test these patches on KSZ9563?
>
> Christian Eggers tried to add PTP support for this switch a while
> ago,
> and he claims that two-step TX timestamping was de-featured for
> KSZ95xx
> due to hardware errata.
>
https://patchwork.ozlabs.org/project/netdev/patch/[email protected]/

I had developed this patch set to add gPTP support for LAN937x based on
the Christian eggers patch for KSZ9563. Initially I thought of keeping
implementation specific to LAN937x through lan937x_ptp.c files. Since
the register sets are same for LAN937x/KSZ9563, I developed using
ksz_ptp.c so that in future Christain eggers patch can be merged to it
to support the 1 step clock support.
I read the Hardware errata of KSZ95xx on 2 step clock and found that it
was fixed in LAN937x switches. If this is case, Do I need to move this
2 step timestamping specific to LAN937x as LAN937x_ptp.c & not claim
for ksz9563 or common implementation in ksz_ptp.c & export the
functionality based on chip-id in get_ts_info dsa hooks.

--
Arun

>
> >
> > Arun Ramadoss (6):
> > 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
> >
> > drivers/net/dsa/microchip/Kconfig | 10 +
> > drivers/net/dsa/microchip/Makefile | 1 +
> > drivers/net/dsa/microchip/ksz_common.c | 43 +-
> > drivers/net/dsa/microchip/ksz_common.h | 31 +
> > drivers/net/dsa/microchip/ksz_ptp.c | 755
> > ++++++++++++++++++++++++
> > drivers/net/dsa/microchip/ksz_ptp.h | 84 +++
> > drivers/net/dsa/microchip/ksz_ptp_reg.h | 68 +++
> > include/linux/dsa/ksz_common.h | 53 ++
> > net/dsa/tag_ksz.c | 156 ++++-
> > 9 files changed, 1192 insertions(+), 9 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
> >
> >
> > base-commit: 66ae04368efbe20eb8951c9a76158f99ce672f25
> > --
> > 2.36.1
> >

2022-10-18 10:05:41

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

On Tue, Oct 18, 2022 at 06:29:17AM +0000, [email protected] wrote:
> > On Fri, Oct 14, 2022 at 08:58:51PM +0530, Arun Ramadoss wrote:
> > > The LAN937x switch has capable for supporting IEEE 1588 PTP protocol. This
> > > patch series add gPTP profile 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. But the testing is
> > > done only for lan937x switch.
> >
> > Unrelated to the proposed implementation. What user space stack do you
> > use for gPTP bridging?
>
> I had used LinuxPTP stack for testing this patch set and in specific
> linuxptp/configs/gptp.cfg
>
> Test Setup is of
> LAN9370 DUT1 <LAN1> --- LAN9370 DUT2 <LAN1>
>
> Ran the below command in both DUTS
> #ptp4l -f ~/linuxptp/configs/gptp.cfg -i lan1

gPTP bridges are when you specify "-i" more than once, similar to how
1588 boundary clocks are created in ptp4l. Linuxptp does not support
gPTP bridges; the time information needs to be relayed differently
compared to both a BC and a TC, and there is also a Follow_Up information
TLV which needs to be updated.

So I guess the answer is, you don't test gPTP bridging, ok.

2022-10-18 10:39:27

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

On Tue, Oct 18, 2022 at 06:44:04AM +0000, [email protected] wrote:
> I had developed this patch set to add gPTP support for LAN937x based on
> the Christian eggers patch for KSZ9563. Initially I thought of keeping
> implementation specific to LAN937x through lan937x_ptp.c files. Since
> the register sets are same for LAN937x/KSZ9563, I developed using
> ksz_ptp.c so that in future Christain eggers patch can be merged to it
> to support the 1 step clock support.
> I read the Hardware errata of KSZ95xx on 2 step clock and found that it
> was fixed in LAN937x switches. If this is case, Do I need to move this
> 2 step timestamping specific to LAN937x as LAN937x_ptp.c & not claim
> for ksz9563 or common implementation in ksz_ptp.c & export the
> functionality based on chip-id in get_ts_info dsa hooks.

The high-level visible behavior needs to be that the kernel denies
hardware timestamping from being enabled on the platforms on which it
does not work (this includes platforms on which it is conveniently
"not tested" by Microchip engineers, despite there being published
errata stating it doesn't work). Then, the code organization needs to be
such that if anyone wants to add one step TX timestamping to KSZ9477/KSZ9563
as a workaround later, the code reuse is close to maximal without
further refactoring. And there should be plenty of reuse beyond the TX
timestamping procedure.

I expect that Christian will also be able to find some time to review
this RFC and propose some changes/ask some questions based on his prior
observations, at least so he said privately.

2022-10-18 14:36:02

by Arun Ramadoss

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

On Tue, 2022-10-18 at 13:29 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Tue, Oct 18, 2022 at 06:44:04AM +0000, [email protected]
> wrote:
> > I had developed this patch set to add gPTP support for LAN937x
> > based on
> > the Christian eggers patch for KSZ9563. Initially I thought of
> > keeping
> > implementation specific to LAN937x through lan937x_ptp.c files.
> > Since
> > the register sets are same for LAN937x/KSZ9563, I developed using
> > ksz_ptp.c so that in future Christain eggers patch can be merged to
> > it
> > to support the 1 step clock support.
> > I read the Hardware errata of KSZ95xx on 2 step clock and found
> > that it
> > was fixed in LAN937x switches. If this is case, Do I need to move
> > this
> > 2 step timestamping specific to LAN937x as LAN937x_ptp.c & not
> > claim
> > for ksz9563 or common implementation in ksz_ptp.c & export the
> > functionality based on chip-id in get_ts_info dsa hooks.
>
> The high-level visible behavior needs to be that the kernel denies
> hardware timestamping from being enabled on the platforms on which it
> does not work (this includes platforms on which it is conveniently
> "not tested" by Microchip engineers, despite there being published
> errata stating it doesn't work). Then, the code organization needs to
> be
> such that if anyone wants to add one step TX timestamping to
> KSZ9477/KSZ9563
> as a workaround later, the code reuse is close to maximal without
> further refactoring. And there should be plenty of reuse beyond the
> TX
> timestamping procedure.
>
> I expect that Christian will also be able to find some time to review
> this RFC and propose some changes/ask some questions based on his
> prior
> observations, at least so he said privately.

Thanks Vladimir. I will wait for Christian feedback.

Hi Christian,
To test this patch on KSZ9563, we need to increase the number of
interrupts port_nirqs in KSZ9893 from 2 to 3. Since the chip id of
KSZ9893 and KSZ9563 are same, I had reused the ksz_chip_data same for
both chips. But this chip differ with number of port interrupts. So we
need to update it. We are generating a new patch for adding the new
element in the ksz_chip_data for KSZ9563.
For now, you can update the code as below for testing the patch

-- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1266,7 +1266,7 @@ const struct ksz_chip_data ksz_switch_chips[] =
{
.num_statics = 16,
.cpu_ports = 0x07, /* can be configured as cpu
port */
.port_cnt = 3, /* total port count */
- .port_nirqs = 2,
+ .port_nirqs = 3,
.ops = &ksz9477_dev_ops,
.mib_names = ksz9477_mib_names,
.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),

--
Arun

2022-10-23 20:33:51

by Christian Eggers

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

Hi Arun, hi Vladimir,

On Tuesday, 18 October 2022, 15:42:41 CEST, [email protected] wrote:
> Thanks Vladimir. I will wait for Christian feedback.
>
> Hi Christian,
> To test this patch on KSZ9563, we need to increase the number of
> interrupts port_nirqs in KSZ9893 from 2 to 3. Since the chip id of
> KSZ9893 and KSZ9563 are same, I had reused the ksz_chip_data same for
> both chips. But this chip differ with number of port interrupts. So we
> need to update it. We are generating a new patch for adding the new
> element in the ksz_chip_data for KSZ9563.
> For now, you can update the code as below for testing the patch
>
> -- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1266,7 +1266,7 @@ const struct ksz_chip_data ksz_switch_chips[] =
> {
> .num_statics = 16,
> .cpu_ports = 0x07, /* can be configured as cpu
> port */
> .port_cnt = 3, /* total port count */
> - .port_nirqs = 2,
> + .port_nirqs = 3,
> .ops = &ksz9477_dev_ops,
> .mib_names = ksz9477_mib_names,
> .mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
>
> --

sorry for the delay. I'm currently quite busy with my own work and
my kids' school stuff. Additionally I had to update my internal kernel tree
from 5.15.y-stable-rt to the latest netdev which took longer than I
expected. (Preempt-RT patches tend to become smaller, my ones are only
getting larger).

Prior applying the patches, everything seems to build and run fine.

After applying the patch series, I had some trouble with linking. I had
configured nearly everything as a module:

CONFIG_PTP_1588_CLOCK=m
CONFIG_NET_DSA=m
CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON=m [ksz_switch.ko]
CONFIG_NET_DSA_MICROCHIP_KSZ9477_I2C=m
CONFIG_NET_DSA_MICROCHIP_KSZ_PTP=y [builtin]

With this configuration, kbuild doesn't even try to compile ksz_ptp.c:

ERROR: modpost: "ksz_hwtstamp_get" [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
ERROR: modpost: "ksz_hwtstamp_set" [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
ERROR: modpost: "ksz_port_txtstamp" [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
ERROR: modpost: "ksz_ptp_clock_register" [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
ERROR: modpost: "ksz_port_deferred_xmit" [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
ERROR: modpost: "ksz_ptp_clock_unregister" [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
ERROR: modpost: "ksz_ptp_irq_free" [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
ERROR: modpost: "ksz_tstamp_reconstruct" [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
ERROR: modpost: "ksz_get_ts_info" [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
ERROR: modpost: "ksz_ptp_irq_setup" [drivers/net/dsa/microchip/ksz_switch.ko] undefined!

After setting all of the above to 'y', the build process works (but I would prefer
being able to build as modules). At startup I get a NULL pointer dereference (see below),
but I haven't tried to track down the source yet (will continue tomorrow).

regards,
Christian

[ 17.749629] ksz9477-switch 0-005f: Port2: using phy mode rmii instead of rgmii
[ 17.785998] 8<--- cut here ---
[ 17.789732] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 17.798006] [00000000] *pgd=00000000
[ 17.801573] Internal error: Oops: 805 [#1] THUMB2
[ 17.806331] Modules linked in: st_magn_i2c st_sensors_i2c st_magn as73211 usb_storage st_sensors industrialio_triggered_buffer ksz9477_i2c(+) btusb rtc_rv3028 at24 kfifo_b
spidev leds_gpio leds_pwm led_class iio_trig_sysfs imx6sx_adc industrialio micrel fec imx_napi at25 spi_imx i2c_imx nfsv3 nfs lockd grace sunrpc usb_f_ncm u_ether libcomposi
[ 17.847335] CPU: 0 PID: 201 Comm: udevd Not tainted 6.1.0-rc1+ #198
[ 17.853768] Hardware name: Freescale i.MX6 Ultralite (Device Tree)
[ 17.860060] PC is at ksz_connect_tag_protocol+0x6/0x18
[ 17.865286] LR is at dsa_register_switch+0x5a9/0x780
[ 17.870336] pc : [<c02cc6be>] lr : [<c03a4f45>] psr: 60000033
[ 17.876774] sp : c22abc30 ip : ffffffff fp : 00000000
[ 17.882095] r10: c047660c r9 : c0476e70 r8 : c5dcb400
[ 17.887412] r7 : c4f31808 r6 : c1ba1f40 r5 : c4f31800 r4 : c1ba1f40
[ 17.894058] r3 : 00000000 r2 : c02d1325 r1 : 00000007 r0 : 00000000
[ 17.900766] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA Thumb Segment none
[ 17.908211] Control: 50c53c7d Table: 822b0059 DAC: 00000051
[ 17.914055] Register r0 information: NULL pointer
[ 17.918929] Register r1 information: non-paged memory
[ 17.924068] Register r2 information: non-slab/vmalloc memory
[ 17.929828] Register r3 information: NULL pointer
[ 17.934613] Register r4 information: slab kmalloc-192 start c1ba1f00 pointer offset 64 size 192
[ 17.943535] Register r5 information: slab kmalloc-64 start c4f31800 pointer offset 0 size 64
[ 17.952131] Register r6 information: slab kmalloc-192 start c1ba1f00 pointer offset 64 size 192
[ 17.961071] Register r7 information: slab kmalloc-64 start c4f31800 pointer offset 8 size 64
[ 17.969665] Register r8 information: slab kmalloc-512 start c5dcb400 pointer offset 0 size 512
[ 17.978434] Register r9 information: non-slab/vmalloc memory
[ 17.984254] Register r10 information: non-slab/vmalloc memory
[ 17.990103] Register r11 information: NULL pointer
[ 17.994977] Register r12 information: non-paged memory
[ 18.000206] Process udevd (pid: 201, stack limit = 0x4afbccb6)
[ 18.006223] Stack: (0xc22abc30 to 0xc22ac000)
[ 18.010658] bc20: 00000000 c22abc48 c0c01300 ff8064a4
[ 18.018992] bc40: ffffffff 00000002 c7eee0e4 00000000 00000168 bf981240 bf9810b8 c5dcbc00
[ 18.027527] bc60: 40000113 00000004 c22abc94 c0306423 40000113 c22f8cd0 c7ef70a8 c0306423
[ 18.035857] bc80: c22abc9c c030823d c7ef73cc c22f8cd0 00000007 c2249240 00000000 c7ef70a8
[ 18.044186] bca0: 000008e0 00000007 c051acdd c05414e4 c05414f9 c02cd2fb 00000000 00000000
[ 18.052718] bcc0: 00000000 00000002 ffffffff ffffffff c0fdf020 000000c4 c2249240 c0fdf000
[ 18.061047] bce0: 00000003 c0fdf020 c224928c bf981240 bf9810b8 bf9800ab c22abd24 bf9810b3
[ 18.069375] bd00: 00000010 00000001 00000000 00000000 00000000 00000020 00000000 00000000
[ 18.077893] bd20: 00000000 00000000 00000000 00000000 00000000 bf98002f bf98002b c2249258
[ 18.086222] bd40: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 18.094611] bd60: 0000ffff 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 18.102940] bd80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 18.111270] bda0: 00000001 00000001 00000000 00000000 00000000 00000000 00000000 00000000
[ 18.119684] bdc0: c32bffc0 c0fdf000 bf980033 bf98201c c0fdf020 c081f0c0 00000026 0000017b
[ 18.128019] bde0: 00000000 c02f8a8d c02f894b 00000000 c0fdf020 bf98201c 00000000 c02993f5
[ 18.136349] be00: 00000001 c0877380 c087748c bf98201c c0fdf020 c0299557 c0fdf064 00000000
[ 18.144739] be20: c0fdf020 bf98201c c080abd8 c081f0c0 c3035d34 c0299853 00000000 c0fdf020
[ 18.153068] be40: bf98201c c02997c7 c080abd8 c029880b c0d17dcc c1878ab0 bf98201c c3035d00
[ 18.153068] be40: bf98201c c02997c7 c080abd8 c029880b c0d17dcc c1878ab0 bf98201c c3035d00
[ 18.161477] be60: 00000000 c0298a93 bf98109f bf9810a0 0000006b bf98201c b6e6a290 c23dba00
[ 18.169808] be80: 00000000 c081f0c0 c23dba00 c0299c1b bf982074 bf982000 b6e6a290 c02f8875
[ 18.178137] bea0: 00000000 bf9b0001 b6e6a290 c0101907 c0c01100 00000cc0 ffffffff 00000008
[ 18.186525] bec0: 00000cc0 c01c1aa7 00006c65 00000000 00000000 c32bfc80 c0138a27 c32bfc80
[ 18.194853] bee0: 00000008 00000040 c32bfc80 c01ab021 00000cc0 ffffffff bf982080 b6e6a290
[ 18.203182] bf00: c32bfc80 0000017b c010027c c0138a41 bf982080 c7fb50e0 00000000 b6e6a290
[ 18.211592] bf20: 00000009 c0139cd1 c22abf38 7fffffff 00000000 00000002 c95aa000 c95ac958
[ 18.219922] bf40: c95aca40 c95aa000 001de8b0 c97764d0 c97762c8 c9733218 00003000 00003080
[ 18.228311] bf60: 00012340 000030e3 00000000 00000000 00000000 00000000 00000000 00012330
[ 18.236640] bf80: 00000749 0000074a 00000279 00000000 00000272 00000000 00000000 b6e7b670
[ 18.244971] bfa0: 0000017b c0100041 00000000 b6e7b670 00000009 b6e6a290 00000000 00000000
[ 18.253373] bfc0: 00000000 b6e7b670 0000017b 0000017b 00000000 b6e7b670 00020000 00000000
[ 18.261710] bfe0: b6e6a290 befaa918 b6e66a23 b6ed4c22 40000030 00000009 00000000 00000000
[ 18.270240] ksz_connect_tag_protocol from dsa_register_switch+0x5a9/0x780
[ 18.277243] dsa_register_switch from ksz_switch_register+0x417/0x48c
[ 18.283797] ksz_switch_register from ksz9477_i2c_probe+0x79/0xfce [ksz9477_i2c]
[ 18.291583] ksz9477_i2c_probe [ksz9477_i2c] from i2c_device_probe+0x143/0x15a
[ 18.299016] i2c_device_probe from really_probe+0xb1/0x188
[ 18.304597] really_probe from driver_probe_device+0x2b/0x88
[ 18.310354] driver_probe_device from __driver_attach+0x8d/0x9e
[ 18.316438] __driver_attach from bus_for_each_dev+0x2b/0x42
[ 18.322196] bus_for_each_dev from bus_add_driver+0x6f/0x128
[ 18.327955] bus_add_driver from driver_register+0x59/0x8e
[ 18.333537] driver_register from i2c_register_driver+0x35/0x54
[ 18.339642] i2c_register_driver from do_one_initcall+0x2b/0xbc
[ 18.345677] do_one_initcall from do_init_module+0x2d/0x1a0
[ 18.351347] do_init_module from sys_finit_module+0x65/0x6c
[ 18.357018] sys_finit_module from ret_fast_syscall+0x1/0x5c
[ 18.362838] Exception stack(0xc22abfa8 to 0xc22abff0)
[ 18.367982] bfa0: 00000000 b6e7b670 00000009 b6e6a290 00000000 00000000
[ 18.376311] bfc0: 00000000 b6e7b670 0000017b 0000017b 00000000 b6e7b670 00020000 00000000
[ 18.384717] bfe0: b6e6a290 befaa918 b6e66a23 b6ed4c22
[ 18.389862] Code: 0093 6a03 2000 4a02 (601a) 4a02
[ 18.394903] ---[ end trace 0000000000000000 ]---



2022-10-25 04:02:10

by Arun Ramadoss

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

Hi Christian,
Thanks for the feedback.

On Sun, 2022-10-23 at 22:15 +0200, Christian Eggers wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> Hi Arun, hi Vladimir,
>
> On Tuesday, 18 October 2022, 15:42:41 CEST,
> [email protected] wrote:
> > Thanks Vladimir. I will wait for Christian feedback.
> >
> > Hi Christian,
> > To test this patch on KSZ9563, we need to increase the number of
> > interrupts port_nirqs in KSZ9893 from 2 to 3. Since the chip id of
> > KSZ9893 and KSZ9563 are same, I had reused the ksz_chip_data same
> for
> > both chips. But this chip differ with number of port interrupts. So
> we
> > need to update it. We are generating a new patch for adding the new
> > element in the ksz_chip_data for KSZ9563.
> > For now, you can update the code as below for testing the patch
> >
> > -- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -1266,7 +1266,7 @@ const struct ksz_chip_data ksz_switch_chips[]
> =
> > {
> > .num_statics = 16,
> > .cpu_ports = 0x07, /* can be configured as cpu
> > port */
> > .port_cnt = 3, /* total port count */
> > - .port_nirqs = 2,
> > + .port_nirqs = 3,
> > .ops = &ksz9477_dev_ops,
> > .mib_names = ksz9477_mib_names,
> > .mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
> >
> > --
>
> sorry for the delay. I'm currently quite busy with my own work and
> my kids' school stuff. Additionally I had to update my internal
> kernel tree
> from 5.15.y-stable-rt to the latest netdev which took longer than I
> expected. (Preempt-RT patches tend to become smaller, my ones are
> only
> getting larger).
>
> Prior applying the patches, everything seems to build and run fine.
>
> After applying the patch series, I had some trouble with linking. I
> had
> configured nearly everything as a module:
>
> CONFIG_PTP_1588_CLOCK=m
> CONFIG_NET_DSA=m
> CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON=m [ksz_switch.ko]
> CONFIG_NET_DSA_MICROCHIP_KSZ9477_I2C=m
> CONFIG_NET_DSA_MICROCHIP_KSZ_PTP=y [builtin]
>
> With this configuration, kbuild doesn't even try to compile
> ksz_ptp.c:
>
> ERROR: modpost: "ksz_hwtstamp_get"
> [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> ERROR: modpost: "ksz_hwtstamp_set"
> [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> ERROR: modpost: "ksz_port_txtstamp"
> [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> ERROR: modpost: "ksz_ptp_clock_register"
> [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> ERROR: modpost: "ksz_port_deferred_xmit"
> [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> ERROR: modpost: "ksz_ptp_clock_unregister"
> [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> ERROR: modpost: "ksz_ptp_irq_free"
> [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> ERROR: modpost: "ksz_tstamp_reconstruct"
> [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> ERROR: modpost: "ksz_get_ts_info"
> [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> ERROR: modpost: "ksz_ptp_irq_setup"
> [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
>
> After setting all of the above to 'y', the build process works (but I
> would prefer
> being able to build as modules).

May be this is due to kconfig of config_ksz_ptp defined bool instead
of tristate. Do I need to change the config_ksz_ptp to tristate in
order to compile as modules?

> At startup I get a NULL pointer dereference (see below),
> but I haven't tried to track down the source yet (will continue
> tomorrow).

Not sure of Null pointer exception. Actually the
ksz_connect_tag_protocol is used only to assign the call back of
ksz_port_deferred_xmit and ksz_tstamp_reconstruct to the ds-
>tagger_data. The memory for ds->tagger_data is allocated using the
ksz_connect function in net/dsa/tag_ksz.c file. As per the flow, memory
is allocated in ksz_connect and then connect_tag dsa_hook in the
ksz_common.c is called in order to assign ptp call backs.

>
> regards,
> Christian
>
> [ 17.749629] ksz9477-switch 0-005f: Port2: using phy mode rmii
> instead of rgmii
> [ 17.785998] 8<--- cut here ---
> [ 17.789732] Unable to handle kernel NULL pointer dereference at
> virtual address 00000000
> [ 17.798006] [00000000] *pgd=00000000
> [ 17.801573] Internal error: Oops: 805 [#1] THUMB2
> [ 17.806331] Modules linked in: st_magn_i2c st_sensors_i2c st_magn
> as73211 usb_storage st_sensors industrialio_triggered_buffer
> ksz9477_i2c(+) btusb rtc_rv3028 at24 kfifo_b
> spidev leds_gpio leds_pwm led_class iio_trig_sysfs imx6sx_adc
> industrialio micrel fec imx_napi at25 spi_imx i2c_imx nfsv3 nfs lockd
> grace sunrpc usb_f_ncm u_ether libcomposi
> [ 17.847335] CPU: 0 PID: 201 Comm: udevd Not tainted 6.1.0-rc1+ #198
> [ 17.853768] Hardware name: Freescale i.MX6 Ultralite (Device Tree)
> [ 17.860060] PC is at ksz_connect_tag_protocol+0x6/0x18
> [ 17.865286] LR is at dsa_register_switch+0x5a9/0x780
> [ 17.870336] pc : [<c02cc6be>] lr : [<c03a4f45>] psr: 60000033
> [ 17.876774] sp : c22abc30 ip : ffffffff fp : 00000000
> [ 17.882095] r10: c047660c r9 : c0476e70 r8 : c5dcb400
> [ 17.887412] r7 : c4f31808 r6 : c1ba1f40 r5 : c4f31800 r4 : c1ba1f40
> [ 17.894058] r3 : 00000000 r2 : c02d1325 r1 : 00000007 r0 : 00000000
> [ 17.900766] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA Thumb
> Segment none
> [ 17.908211] Control: 50c53c7d Table: 822b0059 DAC: 00000051
> [ 17.914055] Register r0 information: NULL pointer
> [ 17.918929] Register r1 information: non-paged memory
> [ 17.924068] Register r2 information: non-slab/vmalloc memory
> [ 17.929828] Register r3 information: NULL pointer
> [ 17.934613] Register r4 information: slab kmalloc-192 start c1ba1f00
> pointer offset 64 size 192
> [ 17.943535] Register r5 information: slab kmalloc-64 start c4f31800
> pointer offset 0 size 64
> [ 17.952131] Register r6 information: slab kmalloc-192 start c1ba1f00
> pointer offset 64 size 192
> [ 17.961071] Register r7 information: slab kmalloc-64 start c4f31800
> pointer offset 8 size 64
> [ 17.969665] Register r8 information: slab kmalloc-512 start c5dcb400
> pointer offset 0 size 512
> [ 17.978434] Register r9 information: non-slab/vmalloc memory
> [ 17.984254] Register r10 information: non-slab/vmalloc memory
> [ 17.990103] Register r11 information: NULL pointer
> [ 17.994977] Register r12 information: non-paged memory
> [ 18.000206] Process udevd (pid: 201, stack limit = 0x4afbccb6)
> [ 18.006223] Stack: (0xc22abc30 to 0xc22ac000)
> [ 18.010658] bc20: 00000000 c22abc48 c0c01300 ff8064a4
> [ 18.018992] bc40: ffffffff 00000002 c7eee0e4 00000000 00000168
> bf981240 bf9810b8 c5dcbc00
> [ 18.027527] bc60: 40000113 00000004 c22abc94 c0306423 40000113
> c22f8cd0 c7ef70a8 c0306423
> [ 18.035857] bc80: c22abc9c c030823d c7ef73cc c22f8cd0 00000007
> c2249240 00000000 c7ef70a8
> [ 18.044186] bca0: 000008e0 00000007 c051acdd c05414e4 c05414f9
> c02cd2fb 00000000 00000000
> [ 18.052718] bcc0: 00000000 00000002 ffffffff ffffffff c0fdf020
> 000000c4 c2249240 c0fdf000
> [ 18.061047] bce0: 00000003 c0fdf020 c224928c bf981240 bf9810b8
> bf9800ab c22abd24 bf9810b3
> [ 18.069375] bd00: 00000010 00000001 00000000 00000000 00000000
> 00000020 00000000 00000000
> [ 18.077893] bd20: 00000000 00000000 00000000 00000000 00000000
> bf98002f bf98002b c2249258
> [ 18.086222] bd40: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [ 18.094611] bd60: 0000ffff 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [ 18.102940] bd80: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [ 18.111270] bda0: 00000001 00000001 00000000 00000000 00000000
> 00000000 00000000 00000000
> [ 18.119684] bdc0: c32bffc0 c0fdf000 bf980033 bf98201c c0fdf020
> c081f0c0 00000026 0000017b
> [ 18.128019] bde0: 00000000 c02f8a8d c02f894b 00000000 c0fdf020
> bf98201c 00000000 c02993f5
> [ 18.136349] be00: 00000001 c0877380 c087748c bf98201c c0fdf020
> c0299557 c0fdf064 00000000
> [ 18.144739] be20: c0fdf020 bf98201c c080abd8 c081f0c0 c3035d34
> c0299853 00000000 c0fdf020
> [ 18.153068] be40: bf98201c c02997c7 c080abd8 c029880b c0d17dcc
> c1878ab0 bf98201c c3035d00
> [ 18.153068] be40: bf98201c c02997c7 c080abd8 c029880b c0d17dcc
> c1878ab0 bf98201c c3035d00
> [ 18.161477] be60: 00000000 c0298a93 bf98109f bf9810a0 0000006b
> bf98201c b6e6a290 c23dba00
> [ 18.169808] be80: 00000000 c081f0c0 c23dba00 c0299c1b bf982074
> bf982000 b6e6a290 c02f8875
> [ 18.178137] bea0: 00000000 bf9b0001 b6e6a290 c0101907 c0c01100
> 00000cc0 ffffffff 00000008
> [ 18.186525] bec0: 00000cc0 c01c1aa7 00006c65 00000000 00000000
> c32bfc80 c0138a27 c32bfc80
> [ 18.194853] bee0: 00000008 00000040 c32bfc80 c01ab021 00000cc0
> ffffffff bf982080 b6e6a290
> [ 18.203182] bf00: c32bfc80 0000017b c010027c c0138a41 bf982080
> c7fb50e0 00000000 b6e6a290
> [ 18.211592] bf20: 00000009 c0139cd1 c22abf38 7fffffff 00000000
> 00000002 c95aa000 c95ac958
> [ 18.219922] bf40: c95aca40 c95aa000 001de8b0 c97764d0 c97762c8
> c9733218 00003000 00003080
> [ 18.228311] bf60: 00012340 000030e3 00000000 00000000 00000000
> 00000000 00000000 00012330
> [ 18.236640] bf80: 00000749 0000074a 00000279 00000000 00000272
> 00000000 00000000 b6e7b670
> [ 18.244971] bfa0: 0000017b c0100041 00000000 b6e7b670 00000009
> b6e6a290 00000000 00000000
> [ 18.253373] bfc0: 00000000 b6e7b670 0000017b 0000017b 00000000
> b6e7b670 00020000 00000000
> [ 18.261710] bfe0: b6e6a290 befaa918 b6e66a23 b6ed4c22 40000030
> 00000009 00000000 00000000
> [ 18.270240] ksz_connect_tag_protocol from
> dsa_register_switch+0x5a9/0x780
> [ 18.277243] dsa_register_switch from ksz_switch_register+0x417/0x48c
> [ 18.283797] ksz_switch_register from ksz9477_i2c_probe+0x79/0xfce
> [ksz9477_i2c]
> [ 18.291583] ksz9477_i2c_probe [ksz9477_i2c] from
> i2c_device_probe+0x143/0x15a
> [ 18.299016] i2c_device_probe from really_probe+0xb1/0x188
> [ 18.304597] really_probe from driver_probe_device+0x2b/0x88
> [ 18.310354] driver_probe_device from __driver_attach+0x8d/0x9e
> [ 18.316438] __driver_attach from bus_for_each_dev+0x2b/0x42
> [ 18.322196] bus_for_each_dev from bus_add_driver+0x6f/0x128
> [ 18.327955] bus_add_driver from driver_register+0x59/0x8e
> [ 18.333537] driver_register from i2c_register_driver+0x35/0x54
> [ 18.339642] i2c_register_driver from do_one_initcall+0x2b/0xbc
> [ 18.345677] do_one_initcall from do_init_module+0x2d/0x1a0
> [ 18.351347] do_init_module from sys_finit_module+0x65/0x6c
> [ 18.357018] sys_finit_module from ret_fast_syscall+0x1/0x5c
> [ 18.362838] Exception stack(0xc22abfa8 to 0xc22abff0)
> [ 18.367982] bfa0: 00000000 b6e7b670 00000009 b6e6a290 00000000
> 00000000
> [ 18.376311] bfc0: 00000000 b6e7b670 0000017b 0000017b 00000000
> b6e7b670 00020000 00000000
> [ 18.384717] bfe0: b6e6a290 befaa918 b6e66a23 b6ed4c22
> [ 18.389862] Code: 0093 6a03 2000 4a02 (601a) 4a02
> [ 18.394903] ---[ end trace 0000000000000000 ]---
>
>
>
> _______________________________________________________ Ch
> ristian
> Eggers Software Engineer ARRI Arnol
> d & Richter Cine Technik GmbH & Co. Betriebs KG
> Arriweg 17 ,
> 83071
> Stephanskirchen http://www.arri.com
>
> +49 8036 3009-3118
> [email protected]
> Get all the latest information from http://www.arri.com,, Fac
> ebook, Twitter, Instagram, LinkedIn and YouTube.
>
> Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
> Sitz: München ‑ Registergericht: Amtsgericht München ‑ Handelsregiste
> rnummer: HRA 57918
> Persönlich haftender Gesellschafter: Arnold & Richter Cine Technik Gm
> bH
> Sitz: München ‑ Registergericht: Amtsgericht München ‑ Handelsregiste
> rnummer: HRB 54477
> Geschäftsführer: Dr. Matthias Erb (Chairman); Dr. Michael Neuhäuser;
> Stephan Schenk; Walter Trauninger
>
>
>

2022-10-25 06:39:17

by Christian Eggers

[permalink] [raw]
Subject: Re: [RFC Patch net-next 5/6] net: dsa: microchip: Adding the ptp packet reception logic

Hi Arun,

On Friday, 14 October 2022, 17:28:56 CEST, Arun Ramadoss wrote:
> 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: Arun Ramadoss <[email protected]>
> ---
> drivers/net/dsa/microchip/ksz_common.c | 12 +++++++++++
> drivers/net/dsa/microchip/ksz_ptp.c | 25 +++++++++++++++++++++
> drivers/net/dsa/microchip/ksz_ptp.h | 6 ++++++
> include/linux/dsa/ksz_common.h | 15 +++++++++++++
> net/dsa/tag_ksz.c | 30 ++++++++++++++++++++++----
> 5 files changed, 84 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 0c0fdb7b7879..388731959b23 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -2426,6 +2426,17 @@ 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);

NULL pointer dereference is here:

ksz_connect() is only called for "lan937x", not for the other KSZ switches.
If ksz_connect() shall only be called for PTP switches, the result of
ksz_tagger_data() may be NULL.

> + tagger_data->meta_tstamp_handler = ksz_tstamp_reconstruct;
> +
> + return 0;
> +}
> +
> static int ksz_port_vlan_filtering(struct dsa_switch *ds, int port,
> bool flag, struct netlink_ext_ack *extack)
> {
> @@ -2819,6 +2830,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 2cae543f7e0b..5ae6eedb6b39 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -372,6 +372,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..9589909ab7d5 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.h
> +++ b/drivers/net/dsa/microchip/ksz_ptp.h
> @@ -28,6 +28,7 @@ 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);
>
> #else
>
> @@ -64,6 +65,11 @@ 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;
> +}
> +
> #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..82edd7228b3c 100644
> --- a/include/linux/dsa/ksz_common.h
> +++ b/include/linux/dsa/ksz_common.h
> @@ -9,9 +9,24 @@
>
> #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);
> };
>
> static inline struct ksz_tagger_data *
> diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
> index ca1261b04fe7..937a3e70b471 100644
> --- a/net/dsa/tag_ksz.c
> +++ b/net/dsa/tag_ksz.c
> @@ -164,6 +164,25 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8795);
> #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;
> + ktime_t tstamp;
> +
> + tagger_data = ksz_tagger_data(ds);

another potential NULL pointer dereference here:
> + 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);
> +}
> +
> static struct sk_buff *ksz9477_xmit(struct sk_buff *skb,
> struct net_device *dev)
> {
> @@ -197,8 +216,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);
> }
> @@ -257,10 +278,11 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893);
> * 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)
> */
> @@ -304,7 +326,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);
>




2022-10-26 16:56:59

by Christian Eggers

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

Hi Arun, hi Vladimir,

On Tuesday, 18 October 2022, 15:42:41 CEST, [email protected] wrote:
> ...
> Thanks Vladimir. I will wait for Christian feedback.
>
> Hi Christian,
> To test this patch on KSZ9563, we need to increase the number of
> interrupts port_nirqs in KSZ9893 from 2 to 3. Since the chip id of
> KSZ9893 and KSZ9563 are same, I had reused the ksz_chip_data same for
> both chips. But this chip differ with number of port interrupts. So we
> need to update it. We are generating a new patch for adding the new
> element in the ksz_chip_data for KSZ9563.
> For now, you can update the code as below for testing the patch

today I hard first success with your patch series on KSZ9563! ptp4l reported
delay measurements between switch port 1 and the connected Meinberg clock:

> ptp4l[75.590]: port 2: new foreign master ec4670.fffe.0a9dcc-1
> ptp4l[79.590]: selected best master clock ec4670.fffe.0a9dcc
> ptp4l[79.590]: updating UTC offset to 37
> ptp4l[79.591]: port 2: LISTENING to UNCALIBRATED on RS_SLAVE
> ptp4l[81.114]: port 2: delay timeout
> ptp4l[81.117]: delay filtered 338 raw 338
> ptp4l[81.118]: port 2: minimum delay request interval 2^1
> ptp4l[81.434]: port 1: announce timeout
> ptp4l[81.434]: config item lan0.udp_ttl is 1
> ptp4l[81.436]: config item (null).dscp_event is 0
> ptp4l[81.437]: config item (null).dscp_general is 0
> ptp4l[81.437]: selected best master clock ec4670.fffe.0a9dcc
> ptp4l[81.438]: updating UTC offset to 37
> ptp4l[81.843]: master offset 33 s0 freq +6937 path delay 338
> ptp4l[82.844]: master offset 26 s2 freq +6930 path delay 338
> ptp4l[82.844]: port 2: UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
> ptp4l[83.844]: master offset 32 s2 freq +6962 path delay 338
> ptp4l[84.844]: master offset 3 s2 freq +6943 path delay 338
> ptp4l[85.844]: master offset -14 s2 freq +6927 path delay 338
> ptp4l[86.042]: port 2: delay timeout
> ptp4l[86.045]: delay filtered 336 raw 335
> ptp4l[86.211]: port 2: delay timeout
> ptp4l[86.213]: delay filtered 335 raw 331
> ptp4l[86.844]: master offset 3 s2 freq +6939 path delay 335
> ptp4l[87.847]: master offset -7 s2 freq +6930 path delay 335

As a next step I'll try to configure the external output for 1PPS. Is this
already implemented in your patches? The file /sys/class/ptp/ptp2/n_periodic_outputs
shows '0' on my system.

BTW: Which is the preferred delay measurement which I shall test (E2E/P2P)? I
started with E2E is this was configured in the hardware and needs no 1-step
time stamping, but I had to add PTP_MSGTYPE_DELAY_REQ in ksz_port_txtstamp().

On Tuesday, 25 October 2022, 05:41:26 CEST, [email protected] wrote:
> On Sun, 2022-10-23 at 22:15 +0200, Christian Eggers wrote:
> > ...
> > After applying the patch series, I had some trouble with linking. I
> > had
> > configured nearly everything as a module:
> >
> > CONFIG_PTP_1588_CLOCK=m
> > CONFIG_NET_DSA=m
> > CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON=m [ksz_switch.ko]
> > CONFIG_NET_DSA_MICROCHIP_KSZ9477_I2C=m
> > CONFIG_NET_DSA_MICROCHIP_KSZ_PTP=y [builtin]
> >
> > With this configuration, kbuild doesn't even try to compile
> > ksz_ptp.c:
> >
> > ERROR: modpost: "ksz_hwtstamp_get"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_hwtstamp_set"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_port_txtstamp"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_ptp_clock_register"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_port_deferred_xmit"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_ptp_clock_unregister"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_ptp_irq_free"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_tstamp_reconstruct"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_get_ts_info"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_ptp_irq_setup"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> >
> > After setting all of the above to 'y', the build process works (but I
> > would prefer
> > being able to build as modules).
>
> May be this is due to kconfig of config_ksz_ptp defined bool instead
> of tristate. Do I need to change the config_ksz_ptp to tristate in
> order to compile as modules?
I'm not an expert for kbuild and cannot tell whether it's allowed to use
bool options which depend on tristate options. At least ksz_ptp.c is compiled
by kbuild if tristate is used. But I needed to add additional EXPORT_SYMBOL()
statements for all non-static functions (see below) for successful linking.

I'm unsure whether it makes sense to build ksz_ptp as a separate module.
Perhaps it should be (conditionally) added to ksz_switch.ko.

On Tuesday, 18 October 2022, 08:44:04 CEST, [email protected] wrote:
> I had developed this patch set to add gPTP support for LAN937x based on
> the Christian eggers patch for KSZ9563.
Maybe this could be mentioned somewhere (e.g. extra line in file header of
ksz_ptp.c). It took a lot of effort (for me) to get this initially running
(e.g. due to limited documentation / support by Microchip). But I'm quite happy
that this is continued now as it is likely that I'll need PTP support for the
KSZ9563 soon.

For KSZ9563, we will need support for 1-step time stamping as two-step
is not possible.

I've stashed all my local changes into an additional patch (see below).
Please feel free to integrate this into your series. As soon I get 1PPS
running, I'll continue testing. Note that I'll be unavailable between Friday
and next Tuesday.

regards,
Christian
---
drivers/net/dsa/microchip/Kconfig | 2 +-
drivers/net/dsa/microchip/ksz9477_i2c.c | 2 +
drivers/net/dsa/microchip/ksz_common.c | 2 +-
drivers/net/dsa/microchip/ksz_ptp.c | 13 +++++-
net/dsa/tag_ksz.c | 60 +++++++++++++++++++++++--
5 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/Kconfig b/drivers/net/dsa/microchip/Kconfig
index 1e9712ff64e2..ac34c01f39a6 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -22,7 +22,7 @@ config NET_DSA_MICROCHIP_KSZ_SPI
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"
+ tristate "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
diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c
index 3763930dc6fc..7eb7d887bf43 100644
--- a/drivers/net/dsa/microchip/ksz9477_i2c.c
+++ b/drivers/net/dsa/microchip/ksz9477_i2c.c
@@ -41,6 +41,8 @@ static int ksz9477_i2c_probe(struct i2c_client *i2c,
if (i2c->dev.platform_data)
dev->pdata = i2c->dev.platform_data;

+ dev->irq = i2c->irq;
+
ret = ksz_switch_register(dev);

/* Main DSA driver may not be started yet. */
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 889b3d398def..679c66f1e420 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1266,7 +1266,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.num_statics = 16,
.cpu_ports = 0x07, /* can be configured as cpu port */
.port_cnt = 3, /* total port count */
- .port_nirqs = 2,
+ .port_nirqs = 3,
.ops = &ksz9477_dev_ops,
.mib_names = ksz9477_mib_names,
.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index d11a490a6c87..6e6814286dec 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -68,6 +68,7 @@ int ksz_get_ts_info(struct dsa_switch *ds, int port, struct ethtool_ts_info *ts)

return 0;
}
+EXPORT_SYMBOL(ksz_get_ts_info);

int ksz_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr)
{
@@ -90,6 +91,7 @@ int ksz_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr)
return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
-EFAULT : 0;
}
+EXPORT_SYMBOL(ksz_hwtstamp_get);

static int ksz_set_hwtstamp_config(struct ksz_device *dev, int port,
struct hwtstamp_config *config)
@@ -106,7 +108,7 @@ static int ksz_set_hwtstamp_config(struct ksz_device *dev, int port,
case HWTSTAMP_TX_OFF:
prt->hwts_tx_en = false;
break;
- case HWTSTAMP_TX_ON:
+ case HWTSTAMP_TX_ONESTEP_P2P:
prt->hwts_tx_en = true;
break;
default:
@@ -162,6 +164,7 @@ int ksz_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr)
mutex_unlock(&ptp_data->lock);
return ret;
}
+EXPORT_SYMBOL(ksz_hwtstamp_set);

void ksz_port_txtstamp(struct dsa_switch *ds, int port,
struct sk_buff *skb)
@@ -187,6 +190,7 @@ void ksz_port_txtstamp(struct dsa_switch *ds, int port,
ptp_msg_type = ptp_get_msgtype(hdr, type);

switch (ptp_msg_type) {
+ case PTP_MSGTYPE_DELAY_REQ:
case PTP_MSGTYPE_PDELAY_REQ:
case PTP_MSGTYPE_PDELAY_RESP:
case PTP_MSGTYPE_SYNC:
@@ -202,6 +206,7 @@ void ksz_port_txtstamp(struct dsa_switch *ds, int port,

KSZ_SKB_CB(skb)->clone = clone;
}
+EXPORT_SYMBOL(ksz_port_txtstamp);

/* These are function related to the ptp clock info */
static int _ksz_ptp_gettime(struct ksz_device *dev, struct timespec64 *ts)
@@ -436,6 +441,7 @@ ktime_t ksz_tstamp_reconstruct(struct dsa_switch *ds, ktime_t tstamp)

return timespec64_to_ktime(ts);
}
+EXPORT_SYMBOL(ksz_tstamp_reconstruct);

static void ksz_ptp_txtstamp_skb(struct ksz_device *dev,
struct ksz_port *prt, struct sk_buff *skb)
@@ -479,6 +485,7 @@ void ksz_port_deferred_xmit(struct kthread_work *work)

kfree(xmit_work);
}
+EXPORT_SYMBOL(ksz_port_deferred_xmit);

static const struct ptp_clock_info ksz_ptp_caps = {
.owner = THIS_MODULE,
@@ -524,6 +531,7 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
ptp_clock_unregister(ptp_data->clock);
return ret;
}
+EXPORT_SYMBOL(ksz_ptp_clock_register);

void ksz_ptp_clock_unregister(struct dsa_switch *ds)
{
@@ -535,6 +543,7 @@ void ksz_ptp_clock_unregister(struct dsa_switch *ds)

ptp_clock_unregister(ptp_data->clock);
}
+EXPORT_SYMBOL(ksz_ptp_clock_unregister);

static irqreturn_t ksz_ptp_msg_thread_fn(int irq, void *dev_id)
{
@@ -734,6 +743,7 @@ int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)

return ret;
}
+EXPORT_SYMBOL(ksz_ptp_irq_setup);

void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p)
{
@@ -749,6 +759,7 @@ void ksz_ptp_irq_free(struct dsa_switch *ds, u8 p)

irq_domain_remove(ptpirq->domain);
}
+EXPORT_SYMBOL(ksz_ptp_irq_free);

MODULE_AUTHOR("Arun Ramadoss <[email protected]>");
MODULE_DESCRIPTION("PTP support for KSZ switch");
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 582add3398d3..e7680718b478 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -251,17 +251,69 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9477);
#define KSZ9893_TAIL_TAG_OVERRIDE BIT(5)
#define KSZ9893_TAIL_TAG_LOOKUP BIT(6)

+/* Time stamp tag is only inserted if PTP is enabled in hardware. */
+static void ksz9893_xmit_timestamp(struct sk_buff *skb)
+{
+// struct sk_buff *clone = KSZ9477_SKB_CB(skb)->clone;
+// struct ptp_header *ptp_hdr;
+// unsigned int ptp_type;
+ u32 tstamp_raw = 0;
+ 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 *ksz9893_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 *ksz9893_xmit(struct sk_buff *skb,
struct net_device *dev)
{
struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct ksz_tagger_private *priv;
u8 *addr;
u8 *tag;

+ priv = ksz_tagger_private(dp->ds);
+
+ /* Tag encoding */
+ if (test_bit(KSZ_HWTS_EN, &priv->state))
+ ksz9893_xmit_timestamp(skb);
+
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);

@@ -270,7 +322,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 ksz9893_defer_xmit(dp, skb);
}

static const struct dsa_device_ops ksz9893_netdev_ops = {
@@ -278,7 +330,9 @@ static const struct dsa_device_ops ksz9893_netdev_ops = {
.proto = DSA_TAG_PROTO_KSZ9893,
.xmit = ksz9893_xmit,
.rcv = ksz9477_rcv,
- .needed_tailroom = KSZ_INGRESS_TAG_LEN,
+ .connect = ksz_connect,
+ .disconnect = ksz_disconnect,
+ .needed_tailroom = KSZ_INGRESS_TAG_LEN + KSZ9477_PTP_TAG_LEN,
};

DSA_TAG_DRIVER(ksz9893_netdev_ops);
--
2.35.3


2022-10-26 20:09:42

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

Hi Arun,

On Tue, Oct 25, 2022 at 03:41:26AM +0000, [email protected] wrote:
> > CONFIG_PTP_1588_CLOCK=m
> > CONFIG_NET_DSA=m
> > CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON=m [ksz_switch.ko]
> > CONFIG_NET_DSA_MICROCHIP_KSZ9477_I2C=m
> > CONFIG_NET_DSA_MICROCHIP_KSZ_PTP=y [builtin]
> >
> > With this configuration, kbuild doesn't even try to compile
> > ksz_ptp.c:
> >
> > ERROR: modpost: "ksz_hwtstamp_get"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_hwtstamp_set"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_port_txtstamp"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_ptp_clock_register"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_port_deferred_xmit"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_ptp_clock_unregister"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_ptp_irq_free"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_tstamp_reconstruct"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_get_ts_info"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> > ERROR: modpost: "ksz_ptp_irq_setup"
> > [drivers/net/dsa/microchip/ksz_switch.ko] undefined!
> >
> > After setting all of the above to 'y', the build process works (but
> > I would prefer being able to build as modules).
>
> May be this is due to kconfig of config_ksz_ptp defined bool instead
> of tristate. Do I need to change the config_ksz_ptp to tristate in
> order to compile as modules?

You don't want a separate kernel module for PTP support, so no, you
don't want to make CONFIG_NET_DSA_MICROCHIP_KSZ_PTP tristate.

But what you want is for the ksz_ptp.o object file to be included into
ksz_switch-objs when CONFIG_NET_DSA_MICROCHIP_KSZ_PTP is enabled.

See how sja1105 does it:

ifdef CONFIG_NET_DSA_SJA1105_PTP
sja1105-objs += sja1105_ptp.o
endif

You'll also want to make NET_DSA_MICROCHIP_KSZ9477_I2C and
NET_DSA_MICROCHIP_KSZ_SPI to depend on PTP_1588_CLOCK_OPTIONAL, because
CONFIG_PTP_1588_CLOCK can be compiled as module (as Christian shows),
and in that case, you'll want the KSZ drivers to also be built as
modules (otherwise they're built-in, and built-in code cannot depend on
symbols exported from modules, because the modules may never be inserted).

It's best to actually experiment with this, you cannot get it right if
you don't experiment.

2022-10-26 20:38:57

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC Patch net-next 5/6] net: dsa: microchip: Adding the ptp packet reception logic

On Tue, Oct 25, 2022 at 08:17:37AM +0200, Christian Eggers wrote:
> > +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);
>
> NULL pointer dereference is here:
>
> ksz_connect() is only called for "lan937x", not for the other KSZ switches.
> If ksz_connect() shall only be called for PTP switches, the result of
> ksz_tagger_data() may be NULL.
>
> > + tagger_data->meta_tstamp_handler = ksz_tstamp_reconstruct;
> > +
> > + return 0;
> > +}
> > +

The observation is correct. All other drivers which use this API check
the protocol given as argument, and for good reason. Only "lan937x"
allocates tagger-owned storage in Arun's implementation, but the common
ksz library supports more tagging protocols.

2022-10-26 21:53:34

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

On Wed, Oct 26, 2022 at 06:47:53PM +0200, Christian Eggers wrote:
> Hi Arun, hi Vladimir,
>
> On Tuesday, 18 October 2022, 15:42:41 CEST, [email protected] wrote:
> > ...
> > Thanks Vladimir. I will wait for Christian feedback.
> >
> > Hi Christian,
> > To test this patch on KSZ9563, we need to increase the number of
> > interrupts port_nirqs in KSZ9893 from 2 to 3. Since the chip id of
> > KSZ9893 and KSZ9563 are same, I had reused the ksz_chip_data same for
> > both chips. But this chip differ with number of port interrupts. So we
> > need to update it. We are generating a new patch for adding the new
> > element in the ksz_chip_data for KSZ9563.
> > For now, you can update the code as below for testing the patch
>
> today I hard first success with your patch series on KSZ9563! ptp4l reported
> delay measurements between switch port 1 and the connected Meinberg clock:
>
> > ptp4l[75.590]: port 2: new foreign master ec4670.fffe.0a9dcc-1
> > ptp4l[79.590]: selected best master clock ec4670.fffe.0a9dcc
> > ptp4l[79.590]: updating UTC offset to 37
> > ptp4l[79.591]: port 2: LISTENING to UNCALIBRATED on RS_SLAVE
> > ptp4l[81.114]: port 2: delay timeout
> > ptp4l[81.117]: delay filtered 338 raw 338
> > ptp4l[81.118]: port 2: minimum delay request interval 2^1
> > ptp4l[81.434]: port 1: announce timeout
> > ptp4l[81.434]: config item lan0.udp_ttl is 1
> > ptp4l[81.436]: config item (null).dscp_event is 0
> > ptp4l[81.437]: config item (null).dscp_general is 0
> > ptp4l[81.437]: selected best master clock ec4670.fffe.0a9dcc
> > ptp4l[81.438]: updating UTC offset to 37
> > ptp4l[81.843]: master offset 33 s0 freq +6937 path delay 338
> > ptp4l[82.844]: master offset 26 s2 freq +6930 path delay 338
> > ptp4l[82.844]: port 2: UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
> > ptp4l[83.844]: master offset 32 s2 freq +6962 path delay 338
> > ptp4l[84.844]: master offset 3 s2 freq +6943 path delay 338
> > ptp4l[85.844]: master offset -14 s2 freq +6927 path delay 338
> > ptp4l[86.042]: port 2: delay timeout
> > ptp4l[86.045]: delay filtered 336 raw 335
> > ptp4l[86.211]: port 2: delay timeout
> > ptp4l[86.213]: delay filtered 335 raw 331
> > ptp4l[86.844]: master offset 3 s2 freq +6939 path delay 335
> > ptp4l[87.847]: master offset -7 s2 freq +6930 path delay 335
>
> As a next step I'll try to configure the external output for 1PPS. Is this
> already implemented in your patches? The file /sys/class/ptp/ptp2/n_periodic_outputs
> shows '0' on my system.

Arun didn't share the PPS output patch publicly, so I don't know why
we're discussing this here. Anyway, in it, Arun (incorrectly)
implemented support for PTP_CLK_REQ_PPS rather than PTP_CLK_REQ_PEROUT,
so there will not be any n_periodic_outputs visible in sysfs. For now,
try via pps_available and pps_enable.

>
> BTW: Which is the preferred delay measurement which I shall test (E2E/P2P)?

As this time around there is somebody from Microchip finally on the
line, I will not interfere in this part. I tried once, and failed to
understand the KSZ PTP philosophy. I hope you get some answers from
Arun. Just one question below.

> I started with E2E is this was configured in the hardware and needs no 1-step
> time stamping, but I had to add PTP_MSGTYPE_DELAY_REQ in ksz_port_txtstamp().

Hm? So if E2E "doesn't need" 1-step TX timestamping and KSZ9563 doesn't
support 2-step TX timestamping, then what kind of TX timestamping is
used here for Delay_Req messages?

Perhaps you mean that E2E doesn't need moving the RX timestamp of the
Pdelay_Req (t2) into the KSZ TX timestamp trailer of the Pdelay_Resp (t3)?

> > May be this is due to kconfig of config_ksz_ptp defined bool instead
> > of tristate. Do I need to change the config_ksz_ptp to tristate in
> > order to compile as modules?
>
> I'm not an expert for kbuild and cannot tell whether it's allowed to use
> bool options which depend on tristate options. At least ksz_ptp.c is compiled
> by kbuild if tristate is used. But I needed to add additional EXPORT_SYMBOL()
> statements for all non-static functions (see below) for successful linking.

If ksz_ptp.o gets linked into ksz_ptp.ko, then yes. But this probably
doesn't make sense, as you point out. So EXPORT_SYMBOL() should not be
needed.

> I'm unsure whether it makes sense to build ksz_ptp as a separate module.
> Perhaps it should be (conditionally) added to ksz_switch.ko.
>
> On Tuesday, 18 October 2022, 08:44:04 CEST, [email protected] wrote:
> > I had developed this patch set to add gPTP support for LAN937x based on
> > the Christian eggers patch for KSZ9563.
>
> Maybe this could be mentioned somewhere (e.g. extra line in file header of
> ksz_ptp.c). It took a lot of effort (for me) to get this initially running
> (e.g. due to limited documentation / support by Microchip). But I'm quite happy
> that this is continued now as it is likely that I'll need PTP support for the
> KSZ9563 soon.
>
> For KSZ9563, we will need support for 1-step time stamping as two-step
> is not possible.
>
> I've stashed all my local changes into an additional patch (see below).
> Please feel free to integrate this into your series. As soon I get 1PPS
> running, I'll continue testing. Note that I'll be unavailable between Friday
> and next Tuesday.
>
> regards,
> Christian
> static int ksz_set_hwtstamp_config(struct ksz_device *dev, int port,
> struct hwtstamp_config *config)
> @@ -106,7 +108,7 @@ static int ksz_set_hwtstamp_config(struct ksz_device *dev, int port,
> case HWTSTAMP_TX_OFF:
> prt->hwts_tx_en = false;
> break;
> - case HWTSTAMP_TX_ON:
> + case HWTSTAMP_TX_ONESTEP_P2P:

One shouldn't replace the other; this implementation is simplistic, of course.

Also, why did you choose HWTSTAMP_TX_ONESTEP_P2P and not HWTSTAMP_TX_ONESTEP_SYNC?

> prt->hwts_tx_en = true;
> break;
> default:
> @@ -162,6 +164,7 @@ int ksz_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr)
> mutex_unlock(&ptp_data->lock);
> return ret;
> }
> +EXPORT_SYMBOL(ksz_hwtstamp_set);
> diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
> index 582add3398d3..e7680718b478 100644
> --- a/net/dsa/tag_ksz.c
> +++ b/net/dsa/tag_ksz.c
> @@ -251,17 +251,69 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9477);
> #define KSZ9893_TAIL_TAG_OVERRIDE BIT(5)
> #define KSZ9893_TAIL_TAG_LOOKUP BIT(6)
>
> +/* Time stamp tag is only inserted if PTP is enabled in hardware. */
> +static void ksz9893_xmit_timestamp(struct sk_buff *skb)
> +{
> +// struct sk_buff *clone = KSZ9477_SKB_CB(skb)->clone;
> +// struct ptp_header *ptp_hdr;
> +// unsigned int ptp_type;
> + u32 tstamp_raw = 0;
> + put_unaligned_be32(tstamp_raw, skb_put(skb, KSZ9477_PTP_TAG_LEN));
> +}

This is needed for one-step TX timestamping, ok.

> +
> +/* Defer transmit if waiting for egress time stamp is required. */
> +static struct sk_buff *ksz9893_defer_xmit(struct dsa_port *dp,
> + struct sk_buff *skb)

No need to duplicate, can rename lan937x_defer_xmit() and call that.

Although I'm not exactly clear *which* packets will need deferred
transmission on ksz9xxx. To my knowledge, such a procedure is only
necessary for 2-step TX timestamping, when the TX timestamp must be
propagated back to the socket error queue via skb_complete_tx_timestamp().
For one-step, AFAIK*, this isn't needed.

This is not used, right? Because the function call is shortcircuited by
the "if (test_bit(KSZ_HWTS_EN, &priv->state))" test earlier.

*Or is this intended to be used for the "Software Two-Step Simulation
Mode in hardware 1-Step Mode" that was suggested in the errata sheet,
where one-step Sync messages still get their TX timestamp reported to
user space as if they were two-step?
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Errata-80000786B.pdf

2022-10-27 17:21:10

by Arun Ramadoss

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

Hi Christian,
On Thu, 2022-10-27 at 00:44 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Wed, Oct 26, 2022 at 06:47:53PM +0200, Christian Eggers wrote:
> > Hi Arun, hi Vladimir,
> >
> > On Tuesday, 18 October 2022, 15:42:41 CEST,
> > [email protected] wrote:
> > > ...
> > > Thanks Vladimir. I will wait for Christian feedback.
> > >
> > > Hi Christian,
> > > To test this patch on KSZ9563, we need to increase the number of
> > > interrupts port_nirqs in KSZ9893 from 2 to 3. Since the chip id
> > > of
> > > KSZ9893 and KSZ9563 are same, I had reused the ksz_chip_data same
> > > for
> > > both chips. But this chip differ with number of port interrupts.
> > > So we
> > > need to update it. We are generating a new patch for adding the
> > > new
> > > element in the ksz_chip_data for KSZ9563.
> > > For now, you can update the code as below for testing the patch
> >
> > today I hard first success with your patch series on KSZ9563! ptp4l
> > reported
> > delay measurements between switch port 1 and the connected Meinberg
> > clock:

I tried to bring up the KSZ9563 setup following are my observation
- With this patch series, I am getting the Null pointer exception.
- After applying the patch provided by you, switch probe is successful.

Usually I test the gPTP using the following command
# ptp4l -f ~/ptp4l/gPTP.cfg -i lan1

How did you test this PTP in your setup, so that I can also get the
same result as below.

> >
> > > ptp4l[75.590]: port 2: new foreign master ec4670.fffe.0a9dcc-1
> > > ptp4l[79.590]: selected best master clock ec4670.fffe.0a9dcc
> > > ptp4l[79.590]: updating UTC offset to 37
> > > ptp4l[79.591]: port 2: LISTENING to UNCALIBRATED on RS_SLAVE
> > > ptp4l[81.114]: port 2: delay timeout
> > > ptp4l[81.117]: delay filtered 338 raw 338
> > > ptp4l[81.118]: port 2: minimum delay request interval 2^1
> > > ptp4l[81.434]: port 1: announce timeout
> > > ptp4l[81.434]: config item lan0.udp_ttl is 1
> > > ptp4l[81.436]: config item (null).dscp_event is 0
> > > ptp4l[81.437]: config item (null).dscp_general is 0
> > > ptp4l[81.437]: selected best master clock ec4670.fffe.0a9dcc
> > > ptp4l[81.438]: updating UTC offset to 37
> > > ptp4l[81.843]: master offset 33 s0 freq +6937 path
> > > delay 338
> > > ptp4l[82.844]: master offset 26 s2 freq +6930 path
> > > delay 338
> > > ptp4l[82.844]: port 2: UNCALIBRATED to SLAVE on
> > > MASTER_CLOCK_SELECTED
> > > ptp4l[83.844]: master offset 32 s2 freq +6962 path
> > > delay 338
> > > ptp4l[84.844]: master offset 3 s2 freq +6943 path
> > > delay 338
> > > ptp4l[85.844]: master offset -14 s2 freq +6927 path
> > > delay 338
> > > ptp4l[86.042]: port 2: delay timeout
> > > ptp4l[86.045]: delay filtered 336 raw 335
> > > ptp4l[86.211]: port 2: delay timeout
> > > ptp4l[86.213]: delay filtered 335 raw 331
> > > ptp4l[86.844]: master offset 3 s2 freq +6939 path
> > > delay 335
> > > ptp4l[87.847]: master offset -7 s2 freq +6930 path
> > > delay 335
> >
> > As a next step I'll try to configure the external output for 1PPS.
> > Is this
> > already implemented in your patches? The file
> > /sys/class/ptp/ptp2/n_periodic_outputs
> > shows '0' on my system.
>
> Arun didn't share the PPS output patch publicly, so I don't know why
> we're discussing this here. Anyway, in it, Arun (incorrectly)
> implemented support for PTP_CLK_REQ_PPS rather than
> PTP_CLK_REQ_PEROUT,
> so there will not be any n_periodic_outputs visible in sysfs. For
> now,
> try via pps_available and pps_enable.
>
> >
> > BTW: Which is the preferred delay measurement which I shall test
> > (E2E/P2P)?
>
> As this time around there is somebody from Microchip finally on the
> line, I will not interfere in this part. I tried once, and failed to
> understand the KSZ PTP philosophy. I hope you get some answers from
> Arun. Just one question below.
>
> > I started with E2E is this was configured in the hardware and needs
> > no 1-step
> > time stamping, but I had to add PTP_MSGTYPE_DELAY_REQ in
> > ksz_port_txtstamp().
>
> Hm? So if E2E "doesn't need" 1-step TX timestamping and KSZ9563
> doesn't
> support 2-step TX timestamping, then what kind of TX timestamping is
> used here for Delay_Req messages?
>
> Perhaps you mean that E2E doesn't need moving the RX timestamp of the
> Pdelay_Req (t2) into the KSZ TX timestamp trailer of the Pdelay_Resp
> (t3)?
>
> > > May be this is due to kconfig of config_ksz_ptp defined bool
> > > instead
> > > of tristate. Do I need to change the config_ksz_ptp to tristate
> > > in
> > > order to compile as modules?
> >
> > I'm not an expert for kbuild and cannot tell whether it's allowed
> > to use
> > bool options which depend on tristate options. At least ksz_ptp.c
> > is compiled
> > by kbuild if tristate is used. But I needed to add additional
> > EXPORT_SYMBOL()
> > statements for all non-static functions (see below) for successful
> > linking.
>
> If ksz_ptp.o gets linked into ksz_ptp.ko, then yes. But this probably
> doesn't make sense, as you point out. So EXPORT_SYMBOL() should not
> be
> needed.
>
> > I'm unsure whether it makes sense to build ksz_ptp as a separate
> > module.
> > Perhaps it should be (conditionally) added to ksz_switch.ko.
> >
> > On Tuesday, 18 October 2022, 08:44:04 CEST,
> > [email protected] wrote:
> > > I had developed this patch set to add gPTP support for LAN937x
> > > based on
> > > the Christian eggers patch for KSZ9563.
> >
> > Maybe this could be mentioned somewhere (e.g. extra line in file
> > header of
> > ksz_ptp.c).

Sure, I will add it in the File Header in the next version.

> > It took a lot of effort (for me) to get this initially running
> > (e.g. due to limited documentation / support by Microchip). But
> > I'm quite happy
> > that this is continued now as it is likely that I'll need PTP
> > support for the
> > KSZ9563 soon.
> >
> > For KSZ9563, we will need support for 1-step time stamping as two-
> > step
> > is not possible.
> >
> > I've stashed all my local changes into an additional patch (see
> > below).
> > Please feel free to integrate this into your series. As soon I get
> > 1PPS
> > running, I'll continue testing.

I thought 1PPS and periodic output are same, So I sent the 1PPS patch.
I need to look into periodic output.

> > Note that I'll be unavailable between Friday
> > and next Tuesday.

If you can elaborate the test need to be done in KSZ9563, I can try to
do during your Vacation.

> >
> > regards,
> > Christian
> > static int ksz_set_hwtstamp_config(struct ksz_device *dev, int
> > port,
> > struct hwtstamp_config *config)
> > @@ -106,7 +108,7 @@ static int ksz_set_hwtstamp_config(struct
> > ksz_device *dev, int port,
> > case HWTSTAMP_TX_OFF:
> > prt->hwts_tx_en = false;
> > break;
> > - case HWTSTAMP_TX_ON:
> > + case HWTSTAMP_TX_ONESTEP_P2P:
>
> One shouldn't replace the other; this implementation is simplistic,
> of course.
>
> Also, why did you choose HWTSTAMP_TX_ONESTEP_P2P and not
> HWTSTAMP_TX_ONESTEP_SYNC?
>
> > prt->hwts_tx_en = true;
> > break;
> > default:
> > @@ -162,6 +164,7 @@ int ksz_hwtstamp_set(struct dsa_switch *ds, int
> > port, struct ifreq *ifr)
> > mutex_unlock(&ptp_data->lock);
> > return ret;
> > }
> > +EXPORT_SYMBOL(ksz_hwtstamp_set);
> > diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
> > index 582add3398d3..e7680718b478 100644
> > --- a/net/dsa/tag_ksz.c
> > +++ b/net/dsa/tag_ksz.c
> > @@ -251,17 +251,69 @@
> > MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9477);
> > #define KSZ9893_TAIL_TAG_OVERRIDE BIT(5)
> > #define KSZ9893_TAIL_TAG_LOOKUP BIT(6)
> >
> > +/* Time stamp tag is only inserted if PTP is enabled in hardware.
> > */
> > +static void ksz9893_xmit_timestamp(struct sk_buff *skb)
> > +{
> > +// struct sk_buff *clone = KSZ9477_SKB_CB(skb)->clone;
> > +// struct ptp_header *ptp_hdr;
> > +// unsigned int ptp_type;
> > + u32 tstamp_raw = 0;
> > + put_unaligned_be32(tstamp_raw, skb_put(skb,
> > KSZ9477_PTP_TAG_LEN));
> > +}
>
> This is needed for one-step TX timestamping, ok.
>
> > +
> > +/* Defer transmit if waiting for egress time stamp is
> > required. */
> > +static struct sk_buff *ksz9893_defer_xmit(struct dsa_port *dp,
> > + struct sk_buff *skb)
>
> No need to duplicate, can rename lan937x_defer_xmit() and call that.
>
> Although I'm not exactly clear *which* packets will need deferred
> transmission on ksz9xxx. To my knowledge, such a procedure is only
> necessary for 2-step TX timestamping, when the TX timestamp must be
> propagated back to the socket error queue via
> skb_complete_tx_timestamp().
> For one-step, AFAIK*, this isn't needed.
>
> This is not used, right? Because the function call is shortcircuited
> by
> the "if (test_bit(KSZ_HWTS_EN, &priv->state))" test earlier.
>
> *Or is this intended to be used for the "Software Two-Step Simulation
> Mode in hardware 1-Step Mode" that was suggested in the errata sheet,
> where one-step Sync messages still get their TX timestamp reported to
> user space as if they were two-step?
>
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Errata-80000786B.pdf

2022-11-02 13:28:29

by Christian Eggers

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

Hi Vladimir,

On Wednesday, 26 October 2022, 23:44:55 CEST, Vladimir Oltean wrote:
> Arun didn't share the PPS output patch publicly, so I don't know why
> we're discussing this here. Anyway, in it, Arun (incorrectly)
> implemented support for PTP_CLK_REQ_PPS rather than PTP_CLK_REQ_PEROUT,
> so there will not be any n_periodic_outputs visible in sysfs. For now,
> try via pps_available and pps_enable.
I can continue testing this week. I can either try with the (incorrect)
PTP_CLK_REQ_PPS or I can try to forward port my earlier patches.

> > BTW: Which is the preferred delay measurement which I shall test (E2E/P2P)?
>
> As this time around there is somebody from Microchip finally on the
> line, I will not interfere in this part. I tried once, and failed to
> understand the KSZ PTP philosophy. I hope you get some answers from
> Arun. Just one question below.
>
> > I started with E2E is this was configured in the hardware and needs no 1-step
> > time stamping, but I had to add PTP_MSGTYPE_DELAY_REQ in ksz_port_txtstamp().
>
> Hm? So if E2E "doesn't need" 1-step TX timestamping and KSZ9563 doesn't
> support 2-step TX timestamping, then what kind of TX timestamping is
> used here for Delay_Req messages?
> Perhaps you mean that E2E doesn't need moving the RX timestamp of the
> Pdelay_Req (t2) into the KSZ TX timestamp trailer of the Pdelay_Resp (t3)?
I think that Delay_Req is not related to 1-step / 2-step time stamping. As far
as I understand, this is only relevant for SYNC and PDelay_Resp.

> > > May be this is due to kconfig of config_ksz_ptp defined bool instead
> > > of tristate. Do I need to change the config_ksz_ptp to tristate in
> > > order to compile as modules?
> >
> > I'm not an expert for kbuild and cannot tell whether it's allowed to use
> > bool options which depend on tristate options. At least ksz_ptp.c is compiled
> > by kbuild if tristate is used. But I needed to add additional EXPORT_SYMBOL()
> > statements for all non-static functions (see below) for successful linking.
>
> If ksz_ptp.o gets linked into ksz_ptp.ko, then yes. But this probably
> doesn't make sense, as you point out. So EXPORT_SYMBOL() should not be
> needed.
>
> > I'm unsure whether it makes sense to build ksz_ptp as a separate module.
> > Perhaps it should be (conditionally) added to ksz_switch.ko.

So let's conditionally add ksz_ptp.o to ksz_switch.ko.

> > static int ksz_set_hwtstamp_config(struct ksz_device *dev, int port,
> > struct hwtstamp_config *config)
> > @@ -106,7 +108,7 @@ static int ksz_set_hwtstamp_config(struct ksz_device *dev, int port,
> > case HWTSTAMP_TX_OFF:
> > prt->hwts_tx_en = false;
> > break;
> > - case HWTSTAMP_TX_ON:
> > + case HWTSTAMP_TX_ONESTEP_P2P:
>
> One shouldn't replace the other; this implementation is simplistic, of course.
>
> Also, why did you choose HWTSTAMP_TX_ONESTEP_P2P and not HWTSTAMP_TX_ONESTEP_SYNC?
Because my (old) ptp4l.conf was configured for P2P+p2p1step. When I started working on
KSZ9563 PTP two years ago, you suggested doing P2P first, because E2E is affected by nasty
packet filters on the switch hardware... But for the first tests now I switched to E2E
for the reasons you mentioned above.

Probably HWTSTAMP_TX_ON needs to be rejected for KSZ9563 and only HWTSTAMP_TX_ONESTEP_SYNC
(for E2E) and HWTSTAMP_TX_ONESTEP_P2P (for P2P) should be accepted.

>
> > prt->hwts_tx_en = true;
> > break;
> > default:
> > @@ -162,6 +164,7 @@ int ksz_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr)
> > mutex_unlock(&ptp_data->lock);
> > return ret;
> > }
> > +EXPORT_SYMBOL(ksz_hwtstamp_set);
> > diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
> > index 582add3398d3..e7680718b478 100644
> > --- a/net/dsa/tag_ksz.c
> > +++ b/net/dsa/tag_ksz.c
> > @@ -251,17 +251,69 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9477);
> > #define KSZ9893_TAIL_TAG_OVERRIDE BIT(5)
> > #define KSZ9893_TAIL_TAG_LOOKUP BIT(6)
> >
> > +/* Time stamp tag is only inserted if PTP is enabled in hardware. */
> > +static void ksz9893_xmit_timestamp(struct sk_buff *skb)
> > +{
> > +// struct sk_buff *clone = KSZ9477_SKB_CB(skb)->clone;
> > +// struct ptp_header *ptp_hdr;
> > +// unsigned int ptp_type;
> > + u32 tstamp_raw = 0;
> > + put_unaligned_be32(tstamp_raw, skb_put(skb, KSZ9477_PTP_TAG_LEN));
> > +}
>
> This is needed for one-step TX timestamping, ok.
Yes, here is some work left for 1-step PDelay_Resp.

>
> > +
> > +/* Defer transmit if waiting for egress time stamp is required. */
> > +static struct sk_buff *ksz9893_defer_xmit(struct dsa_port *dp,
> > + struct sk_buff *skb)
>
> No need to duplicate, can rename lan937x_defer_xmit() and call that.
I wanted only "to make it run", I let the details for Arun.

>
> Although I'm not exactly clear *which* packets will need deferred
> transmission on ksz9xxx. To my knowledge, such a procedure is only
> necessary for 2-step TX timestamping, when the TX timestamp must be
> propagated back to the socket error queue via skb_complete_tx_timestamp().
> For one-step, AFAIK*, this isn't needed.
As far as I remember, deferred xmit is needed for outgoing time stamps,
which are reported back to the application via the socket's error queue.
Because the KSZ time stamp unit only reports the time stamp itself (but
no sequence number or similar), only a single outgoing packet is allowed
to pass the TS unit at once. Otherwise no mapping between the TS and the
skb would be impossible.

>
> This is not used, right? Because the function call is shortcircuited by
> the "if (test_bit(KSZ_HWTS_EN, &priv->state))" test earlier.
I am, quite sure that ksz9893_defer_xmit() is actually required.

>
> *Or is this intended to be used for the "Software Two-Step Simulation
> Mode in hardware 1-Step Mode" that was suggested in the errata sheet,
> where one-step Sync messages still get their TX timestamp reported to
> user space as if they were two-step?
> http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Errata-80000786B.pdf
No, I didn't try to got that way.

regards,
Christian




2022-11-02 14:11:03

by Christian Eggers

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

Hi Arun,

On Thursday, 27 October 2022, 17:51:54 CEST, [email protected] wrote:
> I tried to bring up the KSZ9563 setup following are my observation
> - With this patch series, I am getting the Null pointer exception.
> - After applying the patch provided by you, switch probe is successful.
>
> Usually I test the gPTP using the following command
> # ptp4l -f ~/ptp4l/gPTP.cfg -i lan1
>
> How did you test this PTP in your setup, so that I can also get the
> same result as below.

as the KSZ9563 supports no 2-step time stamping, I use 1-step with E2E.
The problem with E2E is, that the "master/slave" filter
on the KSZ9563 filters out some message types. In order to circumvent
this, I use the KSZ as slave_only for the first tests. Later I would like
to use P2P, but this requires additional implementation in the driver.

> > > Maybe this could be mentioned somewhere (e.g. extra line in file
> > > header of
> > > ksz_ptp.c).
>
> Sure, I will add it in the File Header in the next version.
thanks.

>
> I thought 1PPS and periodic output are same, So I sent the 1PPS patch.
> I need to look into periodic output.
I fell the same when I first started with PTP... But maybe you can use
same code for periodic output from my original patches.

regards,
Christian





2022-11-04 10:48:19

by Arun Ramadoss

[permalink] [raw]
Subject: Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch

Hi Christian,
I updated the n_per_out code and tested with lan9370 board, it is
working in LED_0 pin.

Is there any testing pending from your side. If no can I generate next
version of patch with your changes which you shared in this thread.

Attached the code for reference

diff --git a/drivers/net/dsa/microchip/ksz_common.h
b/drivers/net/dsa/microchip/ksz_common.h
index 3a640ca4f7e1..f6a1a7acda07 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -472,6 +472,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 d11a490a6c87..54527754a50f 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -26,6 +26,147 @@

#define KSZ_PTP_INT_START 13

+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, unsigned int
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 = 1 << (unit + TRIG_DONE_S);
+ ret = ksz_write32(dev, REG_PTP_TRIG_STATUS__4, data);
+ if (ret)
+ return ret;
+
+ /* Clear IRQ */
+ data = 1 << (unit + TRIG_INT_S);
+ 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_cycle_width_set(struct ksz_device *dev, u32
width_ns)
+{
+ int ret;
+
+ ret = ksz_write32(dev, REG_TRIG_CYCLE_WIDTH, width_ns);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int ksz_ptp_tou_cycle_count_set(struct ksz_device *dev, u16
count)
+{
+ u32 data;
+ int ret;
+
+ ret = ksz_read32(dev, REG_TRIG_CYCLE_CNT, &data);
+ if (ret)
+ return ret;
+
+ data &= ~(TRIG_CYCLE_CNT_M << TRIG_CYCLE_CNT_S);
+ data |= (count & TRIG_CYCLE_CNT_M) << TRIG_CYCLE_CNT_S;
+
+ ret = ksz_write32(dev, REG_TRIG_CYCLE_CNT, data);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int ksz_ptp_tou_pulse_verify(u64 pulse_ns)
+{
+ u32 data;
+
+ if (pulse_ns & 0x3)
+ return -EINVAL;
+
+ data = (pulse_ns / 8);
+ if (data != (data & TRIG_PULSE_WIDTH_M))
+ return -ERANGE;
+
+ return 0;
+}
+
+static int ksz_ptp_tou_pulse_set(struct ksz_device *dev, u32 pulse_ns)
+{
+ u32 data;
+
+ data = (pulse_ns / 8);
+
+ return ksz_write32(dev, REG_TRIG_PULSE_WIDTH__4, data);
+}
+
+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_tou_start(struct ksz_device *dev, u32 *ctrl_stat_)
+{
+ u32 ctrl_stat;
+ int ret;
+
+ /* Configure GPIO pins */
+ ret = ksz_ptp_tou_gpio(dev);
+ if (ret)
+ return ret;
+
+ ret = ksz_read32(dev, REG_PTP_CTRL_STAT__4, &ctrl_stat);
+ if (ret)
+ return ret;
+
+ ctrl_stat |= (TRIG_ENABLE | GPIO_OUT);
+
+ ret = ksz_write32(dev, REG_PTP_CTRL_STAT__4, ctrl_stat);
+ if (ret)
+ return ret;
+
+ if (ctrl_stat_)
+ *ctrl_stat_ = ctrl_stat;
+
+ return 0;
+}
+
static int ksz_ptp_enable_mode(struct ksz_device *dev, bool enable)
{
struct ksz_ptp_data *ptp_data = &dev->ptp_data;
@@ -277,6 +418,20 @@ 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);

+ 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);
@@ -368,6 +523,20 @@ static int ksz_ptp_adjtime(struct ptp_clock_info
*ptp, s64 delta)

ret = ksz_write16(dev, REG_PTP_CLK_CTRL, data16);

+ 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);
@@ -377,6 +546,201 @@ static int ksz_ptp_adjtime(struct ptp_clock_info
*ptp, s64 delta)
return ret;
}

+static int ksz_ptp_configure_perout(struct ksz_device *dev,
+ u32 cycle_width_ns, u16
cycle_count,
+ u32 pulse_width_ns,
+ struct timespec64 const
*target_time)
+{
+ u32 trig_ctrl;
+ int ret;
+
+ /* Enable notify, set rising edge, set periodic pattern */
+ trig_ctrl = TRIG_NOTIFY | (TRIG_POS_PERIOD << TRIG_PATTERN_S);
+ ret = ksz_write32(dev, REG_TRIG_CTRL__4, trig_ctrl);
+ if (ret)
+ return ret;
+
+ ret = ksz_ptp_tou_cycle_width_set(dev, cycle_width_ns);
+ if (ret)
+ return ret;
+
+ ksz_ptp_tou_cycle_count_set(dev, cycle_count);
+ if (ret)
+ return ret;
+
+ ret = ksz_ptp_tou_pulse_set(dev, pulse_width_ns);
+ if (ret)
+ return ret;
+
+ ret = ksz_ptp_tou_target_time_set(dev, target_time);
+ 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;
+ u32 gpio_stat0;
+ 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, 0);
+ 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 & GENMASK(31, 0)) != 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,
+ 0,
+ pulse_width_ns,
+ &ptp_data-
>perout_target_time_first);
+ if (ret)
+ return ret;
+
+ /* Activate trigger unit */
+ ret = ksz_ptp_tou_start(dev, NULL);
+ if (ret)
+ return ret;
+
+ /* Check error flag:
+ * - the ACTIVE flag is NOT cleared an error!
+ */
+ ret = ksz_read32(dev, REG_PTP_TRIG_STATUS__4, &gpio_stat0);
+ if (ret)
+ return ret;
+
+ if (gpio_stat0 & (1 << (0 + TRIG_ERROR_S))) {
+ dev_err(dev->dev, "%s: Trigger unit0 error!\n",
__func__);
+ ret = -EIO;
+ /* Unit will be reset on next access */
+ 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)
{
@@ -489,6 +853,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 b2035a0bcbb2..160c05e573bd 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;
@@ -18,6 +23,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 2ae6c8b01b00..0f391a01a7f7 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,77 @@
#define PTP_MASTER BIT(1)
#define PTP_1STEP BIT(0)

+#define REG_PTP_UNIT_INDEX__4 0x0520
+
+#define PTP_UNIT_M 0xF
+
+#define PTP_GPIO_INDEX_S 16
+#define PTP_TSI_INDEX_S 8
+#define PTP_TOU_INDEX_S 0
+
+#define REG_PTP_TRIG_STATUS__4 0x0524
+
+#define TRIG_ERROR_S 16
+#define TRIG_DONE_S 0
+
+#define REG_PTP_INT_STATUS__4 0x0528
+
+#define TRIG_INT_S 16
+#define TS_INT_S 0
+
+#define TRIG_UNIT_M 0x7
+#define TS_UNIT_M 0x3
+
+#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 0xF
+#define TRIG_CASCADE_UPS_S 26
+#define TRIG_NOW BIT(25)
+#define TRIG_NOTIFY BIT(24)
+#define TRIG_EDGE BIT(23)
+#define TRIG_PATTERN_S 20
+#define TRIG_PATTERN_M 0x7
+#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_S 16
+#define TRIG_GPO_M 0xF
+#define TRIG_CASCADE_ITERATE_CNT_M 0xFFFF
+
+#define REG_TRIG_CYCLE_WIDTH 0x053C
+
+#define REG_TRIG_CYCLE_CNT 0x0540
+
+#define TRIG_CYCLE_CNT_M 0xFFFF
+#define TRIG_CYCLE_CNT_S 16
+#define TRIG_BIT_PATTERN_M 0xFFFF
+
+#define REG_TRIG_ITERATE_TIME 0x0544
+
+#define REG_TRIG_PULSE_WIDTH__4 0x0548
+
+#define TRIG_PULSE_WIDTH_M 0x00FFFFFF
+
/* Port PTP Register */
#define REG_PTP_PORT_RX_DELAY__2 0x0C00
#define REG_PTP_PORT_TX_DELAY__2 0x0C02

On Wed, 2022-11-02 at 14:11 +0100, Christian Eggers wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> Hi Vladimir,
>
> On Wednesday, 26 October 2022, 23:44:55 CEST, Vladimir Oltean wrote:
> > Arun didn't share the PPS output patch publicly, so I don't know
> why
> > we're discussing this here. Anyway, in it, Arun (incorrectly)
> > implemented support for PTP_CLK_REQ_PPS rather than
> PTP_CLK_REQ_PEROUT,
> > so there will not be any n_periodic_outputs visible in sysfs. For
> now,
> > try via pps_available and pps_enable.
> I can continue testing this week. I can either try with the
> (incorrect)
> PTP_CLK_REQ_PPS or I can try to forward port my earlier patches.
>
> > > BTW: Which is the preferred delay measurement which I shall test
> (E2E/P2P)?
> >
> > As this time around there is somebody from Microchip finally on the
> > line, I will not interfere in this part. I tried once, and failed
> to
> > understand the KSZ PTP philosophy. I hope you get some answers from
> > Arun. Just one question below.
> >
> > > I started with E2E is this was configured in the hardware and
> needs no 1-step
> > > time stamping, but I had to add PTP_MSGTYPE_DELAY_REQ in
> ksz_port_txtstamp().
> >
> > Hm? So if E2E "doesn't need" 1-step TX timestamping and KSZ9563
> doesn't
> > support 2-step TX timestamping, then what kind of TX timestamping
> is
> > used here for Delay_Req messages?
> > Perhaps you mean that E2E doesn't need moving the RX timestamp of
> the
> > Pdelay_Req (t2) into the KSZ TX timestamp trailer of the
> Pdelay_Resp (t3)?
> I think that Delay_Req is not related to 1-step / 2-step time
> stamping. As far
> as I understand, this is only relevant for SYNC and PDelay_Resp.
>
> > > > May be this is due to kconfig of config_ksz_ptp defined bool
> instead
> > > > of tristate. Do I need to change the config_ksz_ptp to tristate
> in
> > > > order to compile as modules?
> > >
> > > I'm not an expert for kbuild and cannot tell whether it's allowed
> to use
> > > bool options which depend on tristate options. At least ksz_ptp.c
> is compiled
> > > by kbuild if tristate is used. But I needed to add additional
> EXPORT_SYMBOL()
> > > statements for all non-static functions (see below) for
> successful linking.
> >
> > If ksz_ptp.o gets linked into ksz_ptp.ko, then yes. But this
> probably
> > doesn't make sense, as you point out. So EXPORT_SYMBOL() should not
> be
> > needed.
> >
> > > I'm unsure whether it makes sense to build ksz_ptp as a separate
> module.
> > > Perhaps it should be (conditionally) added to ksz_switch.ko.
>
> So let's conditionally add ksz_ptp.o to ksz_switch.ko.
>
> > > static int ksz_set_hwtstamp_config(struct ksz_device *dev, int
> port,
> > > struct hwtstamp_config *config)
> > > @@ -106,7 +108,7 @@ static int ksz_set_hwtstamp_config(struct
> ksz_device *dev, int port,
> > > case HWTSTAMP_TX_OFF:
> > > prt->hwts_tx_en = false;
> > > break;
> > > - case HWTSTAMP_TX_ON:
> > > + case HWTSTAMP_TX_ONESTEP_P2P:
> >
> > One shouldn't replace the other; this implementation is simplistic,
> of course.
> >
> > Also, why did you choose HWTSTAMP_TX_ONESTEP_P2P and not
> HWTSTAMP_TX_ONESTEP_SYNC?
> Because my (old) ptp4l.conf was configured for P2P+p2p1step. When I
> started working on
> KSZ9563 PTP two years ago, you suggested doing P2P first, because E2E
> is affected by nasty
> packet filters on the switch hardware... But for the first tests now
> I switched to E2E
> for the reasons you mentioned above.
>
> Probably HWTSTAMP_TX_ON needs to be rejected for KSZ9563 and only
> HWTSTAMP_TX_ONESTEP_SYNC
>