[PATCH 0/6] Support programmable pins for Ocelot PTP driver

[PATCH 0/6] Support programmable pins for Ocelot PTP driver

[Yangbo Lu]

The Ocelot PTP clock driver had been embedded into ocelot.c driver.
It had supported basic gettime64/settime64/adjtime/adjfine functions
by now which were used by both Ocelot switch and Felix switch.

This patch-set is to move current ptp clock code out of ocelot.c driver
maintaining as a single ptp_ocelot.c driver, and to implement 4
programmable pins (with only periodic signal function for now).

Yangbo Lu (6):
  ptp: move ocelot ptp clock code out of Ethernet driver
  MAINTAINERS: add entry for Microsemi Ocelot PTP driver
  net: mscc: ocelot: fix timestamp info if ptp clock does not work
  net: mscc: ocelot: redefine PTP pins
  net: mscc: ocelot: add wave programming registers definitions
  ptp_ocelot: support 4 programmable pins

 MAINTAINERS                                        |   9 +
 drivers/net/dsa/ocelot/felix.c                     |   3 +-
 drivers/net/dsa/ocelot/felix_vsc9959.c             |   2 +
 drivers/net/ethernet/mscc/ocelot.c                 | 207 +-------------
 drivers/net/ethernet/mscc/ocelot.h                 |   3 +-
 drivers/net/ethernet/mscc/ocelot_board.c           |   1 +
 drivers/net/ethernet/mscc/ocelot_regs.c            |   2 +
 drivers/ptp/Kconfig                                |  10 +
 drivers/ptp/Makefile                               |   1 +
 drivers/ptp/ptp_ocelot.c                           | 310 +++++++++++++++++++++
 include/soc/mscc/ocelot.h                          |  15 +-
 .../net/ethernet => include/soc}/mscc/ocelot_ptp.h |   3 +
 include/soc/mscc/ptp_ocelot.h                      |  34 +++
 13 files changed, 395 insertions(+), 205 deletions(-)
 create mode 100644 drivers/ptp/ptp_ocelot.c
 rename {drivers/net/ethernet => include/soc}/mscc/ocelot_ptp.h (88%)
 create mode 100644 include/soc/mscc/ptp_ocelot.h

-- 
2.7.4

[PATCH 1/6] ptp: move ocelot ptp clock code out of Ethernet driver

[Yangbo Lu]

The Ocelot PTP clock driver had been embedded into ocelot.c driver.
It had supported basic gettime64/settime64/adjtime/adjfine functions
by now which were used by both Ocelot switch and Felix switch.

This patch is to move current ptp clock code out of ocelot.c driver
maintaining as a single ptp_ocelot.c driver.
For futher new features implementation, the common code could be put
in ptp_ocelot.c driver and the switch specific code should be in
specific switch driver. The interrupt implementation in SoC is different
between Ocelot and Felix.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c                     |   3 +-
 drivers/net/ethernet/mscc/ocelot.c                 | 201 +------------------
 drivers/net/ethernet/mscc/ocelot.h                 |   3 +-
 drivers/net/ethernet/mscc/ocelot_board.c           |   1 +
 drivers/ptp/Kconfig                                |  10 +
 drivers/ptp/Makefile                               |   1 +
 drivers/ptp/ptp_ocelot.c                           | 217 +++++++++++++++++++++
 include/soc/mscc/ocelot.h                          |   1 -
 .../net/ethernet => include/soc}/mscc/ocelot_ptp.h |   1 +
 include/soc/mscc/ptp_ocelot.h                      |  34 ++++
 10 files changed, 271 insertions(+), 201 deletions(-)
 create mode 100644 drivers/ptp/ptp_ocelot.c
 rename {drivers/net/ethernet => include/soc}/mscc/ocelot_ptp.h (97%)
 create mode 100644 include/soc/mscc/ptp_ocelot.h

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 6954638..9f9efb9 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -8,6 +8,7 @@
 #include <soc/mscc/ocelot_dev.h>
 #include <soc/mscc/ocelot_ana.h>
 #include <soc/mscc/ocelot.h>
+#include <soc/mscc/ptp_ocelot.h>
 #include <linux/packing.h>
 #include <linux/module.h>
 #include <linux/of_net.h>
@@ -576,7 +577,7 @@ static bool felix_rxtstamp(struct dsa_switch *ds, int port,
 	struct ocelot *ocelot = ds->priv;
 	u8 *extraction = skb->data - ETH_HLEN - OCELOT_TAG_LEN;
 	u32 tstamp_lo, tstamp_hi;
-	struct timespec64 ts;
+	struct timespec64 ts = {0, 0};
 	u64 tstamp, val;
 
 	ocelot_ptp_gettime64(&ocelot->ptp_info, &ts);
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index dc0e273..b342bbd 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -21,6 +21,7 @@
 #include <net/netevent.h>
 #include <net/rtnetlink.h>
 #include <net/switchdev.h>
+#include <soc/mscc/ptp_ocelot.h>
 
 #include "ocelot.h"
 #include "ocelot_ace.h"
@@ -1989,200 +1990,6 @@ struct notifier_block ocelot_switchdev_blocking_nb __read_mostly = {
 };
 EXPORT_SYMBOL(ocelot_switchdev_blocking_nb);
 
-int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts)
-{
-	struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
-	unsigned long flags;
-	time64_t s;
-	u32 val;
-	s64 ns;
-
-	spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
-
-	val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
-	val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
-	val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_SAVE);
-	ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
-
-	s = ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN) & 0xffff;
-	s <<= 32;
-	s += ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
-	ns = ocelot_read_rix(ocelot, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
-
-	spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
-
-	/* Deal with negative values */
-	if (ns >= 0x3ffffff0 && ns <= 0x3fffffff) {
-		s--;
-		ns &= 0xf;
-		ns += 999999984;
-	}
-
-	set_normalized_timespec64(ts, s, ns);
-	return 0;
-}
-EXPORT_SYMBOL(ocelot_ptp_gettime64);
-
-static int ocelot_ptp_settime64(struct ptp_clock_info *ptp,
-				const struct timespec64 *ts)
-{
-	struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
-	unsigned long flags;
-	u32 val;
-
-	spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
-
-	val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
-	val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
-	val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
-
-	ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
-
-	ocelot_write_rix(ocelot, lower_32_bits(ts->tv_sec), PTP_PIN_TOD_SEC_LSB,
-			 TOD_ACC_PIN);
-	ocelot_write_rix(ocelot, upper_32_bits(ts->tv_sec), PTP_PIN_TOD_SEC_MSB,
-			 TOD_ACC_PIN);
-	ocelot_write_rix(ocelot, ts->tv_nsec, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
-
-	val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
-	val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
-	val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_LOAD);
-
-	ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
-
-	spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
-	return 0;
-}
-
-static int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
-{
-	if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
-		struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
-		unsigned long flags;
-		u32 val;
-
-		spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
-
-		val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
-		val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
-		val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
-
-		ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
-
-		ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
-		ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN);
-		ocelot_write_rix(ocelot, delta, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
-
-		val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
-		val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
-		val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_DELTA);
-
-		ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
-
-		spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
-	} else {
-		/* Fall back using ocelot_ptp_settime64 which is not exact. */
-		struct timespec64 ts;
-		u64 now;
-
-		ocelot_ptp_gettime64(ptp, &ts);
-
-		now = ktime_to_ns(timespec64_to_ktime(ts));
-		ts = ns_to_timespec64(now + delta);
-
-		ocelot_ptp_settime64(ptp, &ts);
-	}
-	return 0;
-}
-
-static int ocelot_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
-{
-	struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
-	u32 unit = 0, direction = 0;
-	unsigned long flags;
-	u64 adj = 0;
-
-	spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
-
-	if (!scaled_ppm)
-		goto disable_adj;
-
-	if (scaled_ppm < 0) {
-		direction = PTP_CFG_CLK_ADJ_CFG_DIR;
-		scaled_ppm = -scaled_ppm;
-	}
-
-	adj = PSEC_PER_SEC << 16;
-	do_div(adj, scaled_ppm);
-	do_div(adj, 1000);
-
-	/* If the adjustment value is too large, use ns instead */
-	if (adj >= (1L << 30)) {
-		unit = PTP_CFG_CLK_ADJ_FREQ_NS;
-		do_div(adj, 1000);
-	}
-
-	/* Still too big */
-	if (adj >= (1L << 30))
-		goto disable_adj;
-
-	ocelot_write(ocelot, unit | adj, PTP_CLK_CFG_ADJ_FREQ);
-	ocelot_write(ocelot, PTP_CFG_CLK_ADJ_CFG_ENA | direction,
-		     PTP_CLK_CFG_ADJ_CFG);
-
-	spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
-	return 0;
-
-disable_adj:
-	ocelot_write(ocelot, 0, PTP_CLK_CFG_ADJ_CFG);
-
-	spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
-	return 0;
-}
-
-static struct ptp_clock_info ocelot_ptp_clock_info = {
-	.owner		= THIS_MODULE,
-	.name		= "ocelot ptp",
-	.max_adj	= 0x7fffffff,
-	.n_alarm	= 0,
-	.n_ext_ts	= 0,
-	.n_per_out	= 0,
-	.n_pins		= 0,
-	.pps		= 0,
-	.gettime64	= ocelot_ptp_gettime64,
-	.settime64	= ocelot_ptp_settime64,
-	.adjtime	= ocelot_ptp_adjtime,
-	.adjfine	= ocelot_ptp_adjfine,
-};
-
-static int ocelot_init_timestamp(struct ocelot *ocelot)
-{
-	struct ptp_clock *ptp_clock;
-
-	ocelot->ptp_info = ocelot_ptp_clock_info;
-	ptp_clock = ptp_clock_register(&ocelot->ptp_info, ocelot->dev);
-	if (IS_ERR(ptp_clock))
-		return PTR_ERR(ptp_clock);
-	/* Check if PHC support is missing at the configuration level */
-	if (!ptp_clock)
-		return 0;
-
-	ocelot->ptp_clock = ptp_clock;
-
-	ocelot_write(ocelot, SYS_PTP_CFG_PTP_STAMP_WID(30), SYS_PTP_CFG);
-	ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_LOW);
-	ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_HIGH);
-
-	ocelot_write(ocelot, PTP_CFG_MISC_PTP_EN, PTP_CFG_MISC);
-
-	/* There is no device reconfiguration, PTP Rx stamping is always
-	 * enabled.
-	 */
-	ocelot->hwtstamp_config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
-
-	return 0;
-}
-
 /* Configure the maximum SDU (L2 payload) on RX to the value specified in @sdu.
  * The length of VLAN tags is accounted for automatically via DEV_MAC_TAGS_CFG.
  */
@@ -2507,7 +2314,8 @@ int ocelot_init(struct ocelot *ocelot)
 		ret = ocelot_init_timestamp(ocelot);
 		if (ret) {
 			dev_err(ocelot->dev,
-				"Timestamp initialization failed\n");
+				"Timestamp not enabled or initialization failed\n");
+			ocelot->ptp = 0;
 			return ret;
 		}
 	}
@@ -2524,8 +2332,7 @@ void ocelot_deinit(struct ocelot *ocelot)
 	cancel_delayed_work(&ocelot->stats_work);
 	destroy_workqueue(ocelot->stats_queue);
 	mutex_destroy(&ocelot->stats_lock);
-	if (ocelot->ptp_clock)
-		ptp_clock_unregister(ocelot->ptp_clock);
+	ocelot_deinit_timestamp(ocelot);
 
 	for (i = 0; i < ocelot->num_phys_ports; i++) {
 		port = ocelot->ports[i];
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index e34ef83..5aa2e45 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -15,18 +15,17 @@
 #include <linux/phy.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
-#include <linux/ptp_clock_kernel.h>
 #include <linux/regmap.h>
 
 #include <soc/mscc/ocelot_qsys.h>
 #include <soc/mscc/ocelot_sys.h>
 #include <soc/mscc/ocelot_dev.h>
 #include <soc/mscc/ocelot_ana.h>
+#include <soc/mscc/ocelot_ptp.h>
 #include <soc/mscc/ocelot.h>
 #include "ocelot_rew.h"
 #include "ocelot_qs.h"
 #include "ocelot_tc.h"
-#include "ocelot_ptp.h"
 
 #define OCELOT_BUFFER_CELL_SZ 60
 
diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
index 0ac9fbf7..7e59cee 100644
--- a/drivers/net/ethernet/mscc/ocelot_board.c
+++ b/drivers/net/ethernet/mscc/ocelot_board.c
@@ -14,6 +14,7 @@
 #include <linux/skbuff.h>
 #include <net/switchdev.h>
 
+#include <soc/mscc/ptp_ocelot.h>
 #include <soc/mscc/ocelot_vcap.h>
 #include "ocelot.h"
 
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index 86400c7..ac08e9c 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -151,4 +151,14 @@ config PTP_1588_CLOCK_VMW
 	  To compile this driver as a module, choose M here: the module
 	  will be called ptp_vmw.
 
+config PTP_1588_CLOCK_OCELOT
+	bool "Microsemi Ocelot as PTP clock"
+	depends on MSCC_OCELOT_SWITCH || COMPILE_TEST
+	depends on PTP_1588_CLOCK
+	default y
+	help
+	  This driver adds support for using Microsemi Ocelot as a PTP
+	  clock. This clock is only useful if your PTP programs are
+	  getting hardware time stamps on the PTP Ethernet packets using
+	  the SO_TIMESTAMPING API.
 endmenu
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index 7aff75f..a0229b3 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -15,3 +15,4 @@ ptp-qoriq-$(CONFIG_DEBUG_FS)		+= ptp_qoriq_debugfs.o
 obj-$(CONFIG_PTP_1588_CLOCK_IDTCM)	+= ptp_clockmatrix.o
 obj-$(CONFIG_PTP_1588_CLOCK_IDT82P33)	+= ptp_idt82p33.o
 obj-$(CONFIG_PTP_1588_CLOCK_VMW)	+= ptp_vmw.o
+obj-$(CONFIG_PTP_1588_CLOCK_OCELOT)	+= ptp_ocelot.o
diff --git a/drivers/ptp/ptp_ocelot.c b/drivers/ptp/ptp_ocelot.c
new file mode 100644
index 0000000..59420a7
--- /dev/null
+++ b/drivers/ptp/ptp_ocelot.c
@@ -0,0 +1,217 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Microsemi Ocelot PTP clock driver
+ *
+ * Copyright (c) 2017 Microsemi Corporation
+ * Copyright 2020 NXP
+ */
+#include <soc/mscc/ocelot_ptp.h>
+#include <soc/mscc/ocelot_sys.h>
+#include <soc/mscc/ocelot.h>
+#include <soc/mscc/ptp_ocelot.h>
+
+int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts)
+{
+	struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
+	unsigned long flags;
+	time64_t s;
+	u32 val;
+	s64 ns;
+
+	spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
+
+	val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
+	val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
+	val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_SAVE);
+	ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
+
+	s = ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN) & 0xffff;
+	s <<= 32;
+	s += ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
+	ns = ocelot_read_rix(ocelot, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
+
+	spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
+
+	/* Deal with negative values */
+	if (ns >= 0x3ffffff0 && ns <= 0x3fffffff) {
+		s--;
+		ns &= 0xf;
+		ns += 999999984;
+	}
+
+	set_normalized_timespec64(ts, s, ns);
+	return 0;
+}
+EXPORT_SYMBOL(ocelot_ptp_gettime64);
+
+static int ocelot_ptp_settime64(struct ptp_clock_info *ptp,
+				const struct timespec64 *ts)
+{
+	struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
+
+	val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
+	val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
+	val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
+
+	ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
+
+	ocelot_write_rix(ocelot, lower_32_bits(ts->tv_sec), PTP_PIN_TOD_SEC_LSB,
+			 TOD_ACC_PIN);
+	ocelot_write_rix(ocelot, upper_32_bits(ts->tv_sec), PTP_PIN_TOD_SEC_MSB,
+			 TOD_ACC_PIN);
+	ocelot_write_rix(ocelot, ts->tv_nsec, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
+
+	val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
+	val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
+	val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_LOAD);
+
+	ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
+
+	spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
+	return 0;
+}
+
+static int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
+		struct ocelot *ocelot = container_of(ptp, struct ocelot,
+						     ptp_info);
+		unsigned long flags;
+		u32 val;
+
+		spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
+
+		val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
+		val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK |
+			 PTP_PIN_CFG_DOM);
+		val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
+
+		ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
+
+		ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
+		ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN);
+		ocelot_write_rix(ocelot, delta, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
+
+		val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
+		val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK |
+			 PTP_PIN_CFG_DOM);
+		val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_DELTA);
+
+		ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
+
+		spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
+	} else {
+		/* Fall back using ocelot_ptp_settime64 which is not exact. */
+		struct timespec64 ts;
+		u64 now;
+
+		ocelot_ptp_gettime64(ptp, &ts);
+
+		now = ktime_to_ns(timespec64_to_ktime(ts));
+		ts = ns_to_timespec64(now + delta);
+
+		ocelot_ptp_settime64(ptp, &ts);
+	}
+	return 0;
+}
+
+static int ocelot_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
+{
+	struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
+	u32 unit = 0, direction = 0;
+	unsigned long flags;
+	u64 adj = 0;
+
+	spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
+
+	if (!scaled_ppm)
+		goto disable_adj;
+
+	if (scaled_ppm < 0) {
+		direction = PTP_CFG_CLK_ADJ_CFG_DIR;
+		scaled_ppm = -scaled_ppm;
+	}
+
+	adj = PSEC_PER_SEC << 16;
+	do_div(adj, scaled_ppm);
+	do_div(adj, 1000);
+
+	/* If the adjustment value is too large, use ns instead */
+	if (adj >= (1L << 30)) {
+		unit = PTP_CFG_CLK_ADJ_FREQ_NS;
+		do_div(adj, 1000);
+	}
+
+	/* Still too big */
+	if (adj >= (1L << 30))
+		goto disable_adj;
+
+	ocelot_write(ocelot, unit | adj, PTP_CLK_CFG_ADJ_FREQ);
+	ocelot_write(ocelot, PTP_CFG_CLK_ADJ_CFG_ENA | direction,
+		     PTP_CLK_CFG_ADJ_CFG);
+
+	spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
+	return 0;
+
+disable_adj:
+	ocelot_write(ocelot, 0, PTP_CLK_CFG_ADJ_CFG);
+
+	spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
+	return 0;
+}
+
+static struct ptp_clock_info ocelot_ptp_clock_info = {
+	.owner		= THIS_MODULE,
+	.name		= "ocelot ptp",
+	.max_adj	= 0x7fffffff,
+	.n_alarm	= 0,
+	.n_ext_ts	= 0,
+	.n_per_out	= 0,
+	.n_pins		= 0,
+	.pps		= 0,
+	.gettime64	= ocelot_ptp_gettime64,
+	.settime64	= ocelot_ptp_settime64,
+	.adjtime	= ocelot_ptp_adjtime,
+	.adjfine	= ocelot_ptp_adjfine,
+};
+
+int ocelot_init_timestamp(struct ocelot *ocelot)
+{
+	struct ptp_clock *ptp_clock;
+
+	ocelot->ptp_info = ocelot_ptp_clock_info;
+	ptp_clock = ptp_clock_register(&ocelot->ptp_info, ocelot->dev);
+	if (IS_ERR(ptp_clock))
+		return PTR_ERR(ptp_clock);
+	/* Check if PHC support is missing at the configuration level */
+	if (!ptp_clock)
+		return 0;
+
+	ocelot->ptp_clock = ptp_clock;
+
+	ocelot_write(ocelot, SYS_PTP_CFG_PTP_STAMP_WID(30), SYS_PTP_CFG);
+	ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_LOW);
+	ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_HIGH);
+
+	ocelot_write(ocelot, PTP_CFG_MISC_PTP_EN, PTP_CFG_MISC);
+
+	/* There is no device reconfiguration, PTP Rx stamping is always
+	 * enabled.
+	 */
+	ocelot->hwtstamp_config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
+
+	return 0;
+}
+EXPORT_SYMBOL(ocelot_init_timestamp);
+
+int ocelot_deinit_timestamp(struct ocelot *ocelot)
+{
+	if (ocelot->ptp_clock)
+		ptp_clock_unregister(ocelot->ptp_clock);
+	return 0;
+}
+EXPORT_SYMBOL(ocelot_deinit_timestamp);
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 007b584..d9bad70 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -607,7 +607,6 @@ int ocelot_vlan_add(struct ocelot *ocelot, int port, u16 vid, bool pvid,
 int ocelot_vlan_del(struct ocelot *ocelot, int port, u16 vid);
 int ocelot_hwstamp_get(struct ocelot *ocelot, int port, struct ifreq *ifr);
 int ocelot_hwstamp_set(struct ocelot *ocelot, int port, struct ifreq *ifr);
-int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts);
 int ocelot_port_add_txtstamp_skb(struct ocelot_port *ocelot_port,
 				 struct sk_buff *skb);
 void ocelot_get_txtstamp(struct ocelot *ocelot);
diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.h b/include/soc/mscc/ocelot_ptp.h
similarity index 97%
rename from drivers/net/ethernet/mscc/ocelot_ptp.h
rename to include/soc/mscc/ocelot_ptp.h
index 9ede14a..2dd27f0 100644
--- a/drivers/net/ethernet/mscc/ocelot_ptp.h
+++ b/include/soc/mscc/ocelot_ptp.h
@@ -4,6 +4,7 @@
  *
  * License: Dual MIT/GPL
  * Copyright (c) 2017 Microsemi Corporation
+ * Copyright 2020 NXP
  */
 
 #ifndef _MSCC_OCELOT_PTP_H_
diff --git a/include/soc/mscc/ptp_ocelot.h b/include/soc/mscc/ptp_ocelot.h
new file mode 100644
index 0000000..b8d9c5b
--- /dev/null
+++ b/include/soc/mscc/ptp_ocelot.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Microsemi Ocelot PTP clock driver
+ *
+ * License: Dual MIT/GPL
+ * Copyright 2020 NXP
+ */
+
+#ifndef _PTP_OCELOT_H_
+#define _PTP_OCELOT_H_
+
+#include <soc/mscc/ocelot.h>
+#include <linux/ptp_clock_kernel.h>
+
+#ifdef CONFIG_PTP_1588_CLOCK_OCELOT
+int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts);
+int ocelot_init_timestamp(struct ocelot *ocelot);
+int ocelot_deinit_timestamp(struct ocelot *ocelot);
+#else
+static inline int ocelot_ptp_gettime64(struct ptp_clock_info *ptp,
+				       struct timespec64 *ts)
+{
+	return -EOPNOTSUPP;
+}
+static inline int ocelot_init_timestamp(struct ocelot *ocelot)
+{
+	return -EOPNOTSUPP;
+}
+static inline int ocelot_deinit_timestamp(struct ocelot *ocelot)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+#endif
-- 
2.7.4

Re: [PATCH 1/6] ptp: move ocelot ptp clock code out of Ethernet driver

[Vladimir Oltean]

I hate to be that guy who reports build error before the Kbuild robot
does, but here goes.

On Fri, 20 Mar 2020 at 12:41, Yangbo Lu <yangbo.lu@nxp.com> wrote:
>
> The Ocelot PTP clock driver had been embedded into ocelot.c driver.
> It had supported basic gettime64/settime64/adjtime/adjfine functions
> by now which were used by both Ocelot switch and Felix switch.
>
> This patch is to move current ptp clock code out of ocelot.c driver
> maintaining as a single ptp_ocelot.c driver.
> For futher new features implementation, the common code could be put
> in ptp_ocelot.c driver and the switch specific code should be in
> specific switch driver. The interrupt implementation in SoC is different
> between Ocelot and Felix.
>
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
>  drivers/net/dsa/ocelot/felix.c                     |   3 +-
>  drivers/net/ethernet/mscc/ocelot.c                 | 201 +------------------
>  drivers/net/ethernet/mscc/ocelot.h                 |   3 +-
>  drivers/net/ethernet/mscc/ocelot_board.c           |   1 +
>  drivers/ptp/Kconfig                                |  10 +
>  drivers/ptp/Makefile                               |   1 +
>  drivers/ptp/ptp_ocelot.c                           | 217 +++++++++++++++++++++
>  include/soc/mscc/ocelot.h                          |   1 -
>  .../net/ethernet => include/soc}/mscc/ocelot_ptp.h |   1 +
>  include/soc/mscc/ptp_ocelot.h                      |  34 ++++
>  10 files changed, 271 insertions(+), 201 deletions(-)
>  create mode 100644 drivers/ptp/ptp_ocelot.c
>  rename {drivers/net/ethernet => include/soc}/mscc/ocelot_ptp.h (97%)
>  create mode 100644 include/soc/mscc/ptp_ocelot.h
>
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index 6954638..9f9efb9 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -8,6 +8,7 @@
>  #include <soc/mscc/ocelot_dev.h>
>  #include <soc/mscc/ocelot_ana.h>
>  #include <soc/mscc/ocelot.h>
> +#include <soc/mscc/ptp_ocelot.h>
>  #include <linux/packing.h>
>  #include <linux/module.h>
>  #include <linux/of_net.h>
> @@ -576,7 +577,7 @@ static bool felix_rxtstamp(struct dsa_switch *ds, int port,
>         struct ocelot *ocelot = ds->priv;
>         u8 *extraction = skb->data - ETH_HLEN - OCELOT_TAG_LEN;
>         u32 tstamp_lo, tstamp_hi;
> -       struct timespec64 ts;
> +       struct timespec64 ts = {0, 0};
>         u64 tstamp, val;
>
>         ocelot_ptp_gettime64(&ocelot->ptp_info, &ts);
> diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
> index dc0e273..b342bbd 100644
> --- a/drivers/net/ethernet/mscc/ocelot.c
> +++ b/drivers/net/ethernet/mscc/ocelot.c
> @@ -21,6 +21,7 @@
>  #include <net/netevent.h>
>  #include <net/rtnetlink.h>
>  #include <net/switchdev.h>
> +#include <soc/mscc/ptp_ocelot.h>
>
>  #include "ocelot.h"
>  #include "ocelot_ace.h"
> @@ -1989,200 +1990,6 @@ struct notifier_block ocelot_switchdev_blocking_nb __read_mostly = {
>  };
>  EXPORT_SYMBOL(ocelot_switchdev_blocking_nb);
>
> -int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts)
> -{
> -       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> -       unsigned long flags;
> -       time64_t s;
> -       u32 val;
> -       s64 ns;
> -
> -       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> -
> -       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> -       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> -       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_SAVE);
> -       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> -
> -       s = ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN) & 0xffff;
> -       s <<= 32;
> -       s += ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
> -       ns = ocelot_read_rix(ocelot, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> -
> -       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> -
> -       /* Deal with negative values */
> -       if (ns >= 0x3ffffff0 && ns <= 0x3fffffff) {
> -               s--;
> -               ns &= 0xf;
> -               ns += 999999984;
> -       }
> -
> -       set_normalized_timespec64(ts, s, ns);
> -       return 0;
> -}
> -EXPORT_SYMBOL(ocelot_ptp_gettime64);
> -
> -static int ocelot_ptp_settime64(struct ptp_clock_info *ptp,
> -                               const struct timespec64 *ts)
> -{
> -       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> -       unsigned long flags;
> -       u32 val;
> -
> -       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> -
> -       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> -       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> -       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
> -
> -       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> -
> -       ocelot_write_rix(ocelot, lower_32_bits(ts->tv_sec), PTP_PIN_TOD_SEC_LSB,
> -                        TOD_ACC_PIN);
> -       ocelot_write_rix(ocelot, upper_32_bits(ts->tv_sec), PTP_PIN_TOD_SEC_MSB,
> -                        TOD_ACC_PIN);
> -       ocelot_write_rix(ocelot, ts->tv_nsec, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> -
> -       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> -       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> -       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_LOAD);
> -
> -       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> -
> -       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> -       return 0;
> -}
> -
> -static int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> -{
> -       if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
> -               struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> -               unsigned long flags;
> -               u32 val;
> -
> -               spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> -
> -               val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> -               val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> -               val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
> -
> -               ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> -
> -               ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
> -               ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN);
> -               ocelot_write_rix(ocelot, delta, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> -
> -               val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> -               val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> -               val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_DELTA);
> -
> -               ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> -
> -               spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> -       } else {
> -               /* Fall back using ocelot_ptp_settime64 which is not exact. */
> -               struct timespec64 ts;
> -               u64 now;
> -
> -               ocelot_ptp_gettime64(ptp, &ts);
> -
> -               now = ktime_to_ns(timespec64_to_ktime(ts));
> -               ts = ns_to_timespec64(now + delta);
> -
> -               ocelot_ptp_settime64(ptp, &ts);
> -       }
> -       return 0;
> -}
> -
> -static int ocelot_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> -{
> -       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> -       u32 unit = 0, direction = 0;
> -       unsigned long flags;
> -       u64 adj = 0;
> -
> -       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> -
> -       if (!scaled_ppm)
> -               goto disable_adj;
> -
> -       if (scaled_ppm < 0) {
> -               direction = PTP_CFG_CLK_ADJ_CFG_DIR;
> -               scaled_ppm = -scaled_ppm;
> -       }
> -
> -       adj = PSEC_PER_SEC << 16;
> -       do_div(adj, scaled_ppm);
> -       do_div(adj, 1000);
> -
> -       /* If the adjustment value is too large, use ns instead */
> -       if (adj >= (1L << 30)) {
> -               unit = PTP_CFG_CLK_ADJ_FREQ_NS;
> -               do_div(adj, 1000);
> -       }
> -
> -       /* Still too big */
> -       if (adj >= (1L << 30))
> -               goto disable_adj;
> -
> -       ocelot_write(ocelot, unit | adj, PTP_CLK_CFG_ADJ_FREQ);
> -       ocelot_write(ocelot, PTP_CFG_CLK_ADJ_CFG_ENA | direction,
> -                    PTP_CLK_CFG_ADJ_CFG);
> -
> -       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> -       return 0;
> -
> -disable_adj:
> -       ocelot_write(ocelot, 0, PTP_CLK_CFG_ADJ_CFG);
> -
> -       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> -       return 0;
> -}
> -
> -static struct ptp_clock_info ocelot_ptp_clock_info = {
> -       .owner          = THIS_MODULE,
> -       .name           = "ocelot ptp",
> -       .max_adj        = 0x7fffffff,
> -       .n_alarm        = 0,
> -       .n_ext_ts       = 0,
> -       .n_per_out      = 0,
> -       .n_pins         = 0,
> -       .pps            = 0,
> -       .gettime64      = ocelot_ptp_gettime64,
> -       .settime64      = ocelot_ptp_settime64,
> -       .adjtime        = ocelot_ptp_adjtime,
> -       .adjfine        = ocelot_ptp_adjfine,
> -};
> -
> -static int ocelot_init_timestamp(struct ocelot *ocelot)
> -{
> -       struct ptp_clock *ptp_clock;
> -
> -       ocelot->ptp_info = ocelot_ptp_clock_info;
> -       ptp_clock = ptp_clock_register(&ocelot->ptp_info, ocelot->dev);
> -       if (IS_ERR(ptp_clock))
> -               return PTR_ERR(ptp_clock);
> -       /* Check if PHC support is missing at the configuration level */
> -       if (!ptp_clock)
> -               return 0;
> -
> -       ocelot->ptp_clock = ptp_clock;
> -
> -       ocelot_write(ocelot, SYS_PTP_CFG_PTP_STAMP_WID(30), SYS_PTP_CFG);
> -       ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_LOW);
> -       ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_HIGH);
> -
> -       ocelot_write(ocelot, PTP_CFG_MISC_PTP_EN, PTP_CFG_MISC);
> -
> -       /* There is no device reconfiguration, PTP Rx stamping is always
> -        * enabled.
> -        */
> -       ocelot->hwtstamp_config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
> -
> -       return 0;
> -}
> -
>  /* Configure the maximum SDU (L2 payload) on RX to the value specified in @sdu.
>   * The length of VLAN tags is accounted for automatically via DEV_MAC_TAGS_CFG.
>   */
> @@ -2507,7 +2314,8 @@ int ocelot_init(struct ocelot *ocelot)
>                 ret = ocelot_init_timestamp(ocelot);
>                 if (ret) {
>                         dev_err(ocelot->dev,
> -                               "Timestamp initialization failed\n");
> +                               "Timestamp not enabled or initialization failed\n");
> +                       ocelot->ptp = 0;
>                         return ret;
>                 }
>         }
> @@ -2524,8 +2332,7 @@ void ocelot_deinit(struct ocelot *ocelot)
>         cancel_delayed_work(&ocelot->stats_work);
>         destroy_workqueue(ocelot->stats_queue);
>         mutex_destroy(&ocelot->stats_lock);
> -       if (ocelot->ptp_clock)
> -               ptp_clock_unregister(ocelot->ptp_clock);
> +       ocelot_deinit_timestamp(ocelot);
>
>         for (i = 0; i < ocelot->num_phys_ports; i++) {
>                 port = ocelot->ports[i];
> diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
> index e34ef83..5aa2e45 100644
> --- a/drivers/net/ethernet/mscc/ocelot.h
> +++ b/drivers/net/ethernet/mscc/ocelot.h
> @@ -15,18 +15,17 @@
>  #include <linux/phy.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> -#include <linux/ptp_clock_kernel.h>
>  #include <linux/regmap.h>
>
>  #include <soc/mscc/ocelot_qsys.h>
>  #include <soc/mscc/ocelot_sys.h>
>  #include <soc/mscc/ocelot_dev.h>
>  #include <soc/mscc/ocelot_ana.h>
> +#include <soc/mscc/ocelot_ptp.h>
>  #include <soc/mscc/ocelot.h>
>  #include "ocelot_rew.h"
>  #include "ocelot_qs.h"
>  #include "ocelot_tc.h"
> -#include "ocelot_ptp.h"
>
>  #define OCELOT_BUFFER_CELL_SZ 60
>
> diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
> index 0ac9fbf7..7e59cee 100644
> --- a/drivers/net/ethernet/mscc/ocelot_board.c
> +++ b/drivers/net/ethernet/mscc/ocelot_board.c
> @@ -14,6 +14,7 @@
>  #include <linux/skbuff.h>
>  #include <net/switchdev.h>
>
> +#include <soc/mscc/ptp_ocelot.h>
>  #include <soc/mscc/ocelot_vcap.h>
>  #include "ocelot.h"
>
> diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
> index 86400c7..ac08e9c 100644
> --- a/drivers/ptp/Kconfig
> +++ b/drivers/ptp/Kconfig
> @@ -151,4 +151,14 @@ config PTP_1588_CLOCK_VMW
>           To compile this driver as a module, choose M here: the module
>           will be called ptp_vmw.
>
> +config PTP_1588_CLOCK_OCELOT
> +       bool "Microsemi Ocelot as PTP clock"

Why bool and not tristate? Compilation breaks when
MSCC_OCELOT_SWITCH=m because it forces PTP_1588_CLOCK_OCELOT=y.

drivers/ptp/ptp_ocelot.o: In function `ocelot_ptp_settime64':
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:56: undefined reference
to `__ocelot_read_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:60: undefined reference
to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:62: undefined reference
to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:64: undefined reference
to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:66: undefined reference
to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:68: undefined reference
to `__ocelot_read_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:72: undefined reference
to `__ocelot_write_ix'
drivers/ptp/ptp_ocelot.o: In function `ocelot_ptp_adjfine':
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:161: undefined
reference to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:153: undefined
reference to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:154: undefined
reference to `__ocelot_write_ix'
drivers/ptp/ptp_ocelot.o: In function `ocelot_ptp_gettime64':
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:23: undefined reference
to `__ocelot_read_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:26: undefined reference
to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:28: undefined reference
to `__ocelot_read_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:30: undefined reference
to `__ocelot_read_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:31: undefined reference
to `__ocelot_read_ix'
drivers/ptp/ptp_ocelot.o: In function `ocelot_ptp_enable':
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:231: undefined
reference to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:232: undefined
reference to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:235: undefined
reference to `__ocelot_write_ix'
drivers/ptp/ptp_ocelot.o: In function `ocelot_init_timestamp':
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:289: undefined
reference to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:290: undefined
reference to `__ocelot_write_ix'
drivers/ptp/ptp_ocelot.o:/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:291:
more undefined references to `__ocelot_write_ix' follow
drivers/ptp/ptp_ocelot.o: In function `ocelot_ptp_adjtime':
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:88: undefined reference
to `__ocelot_read_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:93: undefined reference
to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:95: undefined reference
to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:96: undefined reference
to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:97: undefined reference
to `__ocelot_write_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:99: undefined reference
to `__ocelot_read_ix'
/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:104: undefined
reference to `__ocelot_write_ix'

> +       depends on MSCC_OCELOT_SWITCH || COMPILE_TEST
> +       depends on PTP_1588_CLOCK
> +       default y
> +       help
> +         This driver adds support for using Microsemi Ocelot as a PTP
> +         clock. This clock is only useful if your PTP programs are
> +         getting hardware time stamps on the PTP Ethernet packets using
> +         the SO_TIMESTAMPING API.
>  endmenu
> diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
> index 7aff75f..a0229b3 100644
> --- a/drivers/ptp/Makefile
> +++ b/drivers/ptp/Makefile
> @@ -15,3 +15,4 @@ ptp-qoriq-$(CONFIG_DEBUG_FS)          += ptp_qoriq_debugfs.o
>  obj-$(CONFIG_PTP_1588_CLOCK_IDTCM)     += ptp_clockmatrix.o
>  obj-$(CONFIG_PTP_1588_CLOCK_IDT82P33)  += ptp_idt82p33.o
>  obj-$(CONFIG_PTP_1588_CLOCK_VMW)       += ptp_vmw.o
> +obj-$(CONFIG_PTP_1588_CLOCK_OCELOT)    += ptp_ocelot.o
> diff --git a/drivers/ptp/ptp_ocelot.c b/drivers/ptp/ptp_ocelot.c
> new file mode 100644
> index 0000000..59420a7
> --- /dev/null
> +++ b/drivers/ptp/ptp_ocelot.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Microsemi Ocelot PTP clock driver
> + *
> + * Copyright (c) 2017 Microsemi Corporation
> + * Copyright 2020 NXP
> + */
> +#include <soc/mscc/ocelot_ptp.h>
> +#include <soc/mscc/ocelot_sys.h>
> +#include <soc/mscc/ocelot.h>
> +#include <soc/mscc/ptp_ocelot.h>
> +
> +int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts)
> +{
> +       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> +       unsigned long flags;
> +       time64_t s;
> +       u32 val;
> +       s64 ns;
> +
> +       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> +
> +       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> +       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> +       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_SAVE);
> +       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> +
> +       s = ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN) & 0xffff;
> +       s <<= 32;
> +       s += ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
> +       ns = ocelot_read_rix(ocelot, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> +
> +       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> +
> +       /* Deal with negative values */
> +       if (ns >= 0x3ffffff0 && ns <= 0x3fffffff) {
> +               s--;
> +               ns &= 0xf;
> +               ns += 999999984;
> +       }
> +
> +       set_normalized_timespec64(ts, s, ns);
> +       return 0;
> +}
> +EXPORT_SYMBOL(ocelot_ptp_gettime64);
> +
> +static int ocelot_ptp_settime64(struct ptp_clock_info *ptp,
> +                               const struct timespec64 *ts)
> +{
> +       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> +       unsigned long flags;
> +       u32 val;
> +
> +       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> +
> +       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> +       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> +       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
> +
> +       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> +
> +       ocelot_write_rix(ocelot, lower_32_bits(ts->tv_sec), PTP_PIN_TOD_SEC_LSB,
> +                        TOD_ACC_PIN);
> +       ocelot_write_rix(ocelot, upper_32_bits(ts->tv_sec), PTP_PIN_TOD_SEC_MSB,
> +                        TOD_ACC_PIN);
> +       ocelot_write_rix(ocelot, ts->tv_nsec, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> +
> +       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> +       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> +       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_LOAD);
> +
> +       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> +
> +       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> +       return 0;
> +}
> +
> +static int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> +{
> +       if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
> +               struct ocelot *ocelot = container_of(ptp, struct ocelot,
> +                                                    ptp_info);
> +               unsigned long flags;
> +               u32 val;
> +
> +               spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> +
> +               val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> +               val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK |
> +                        PTP_PIN_CFG_DOM);
> +               val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
> +
> +               ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> +
> +               ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
> +               ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN);
> +               ocelot_write_rix(ocelot, delta, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> +
> +               val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> +               val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK |
> +                        PTP_PIN_CFG_DOM);
> +               val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_DELTA);
> +
> +               ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> +
> +               spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> +       } else {
> +               /* Fall back using ocelot_ptp_settime64 which is not exact. */
> +               struct timespec64 ts;
> +               u64 now;
> +
> +               ocelot_ptp_gettime64(ptp, &ts);
> +
> +               now = ktime_to_ns(timespec64_to_ktime(ts));
> +               ts = ns_to_timespec64(now + delta);
> +
> +               ocelot_ptp_settime64(ptp, &ts);
> +       }
> +       return 0;
> +}
> +
> +static int ocelot_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> +{
> +       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> +       u32 unit = 0, direction = 0;
> +       unsigned long flags;
> +       u64 adj = 0;
> +
> +       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> +
> +       if (!scaled_ppm)
> +               goto disable_adj;
> +
> +       if (scaled_ppm < 0) {
> +               direction = PTP_CFG_CLK_ADJ_CFG_DIR;
> +               scaled_ppm = -scaled_ppm;
> +       }
> +
> +       adj = PSEC_PER_SEC << 16;
> +       do_div(adj, scaled_ppm);
> +       do_div(adj, 1000);
> +
> +       /* If the adjustment value is too large, use ns instead */
> +       if (adj >= (1L << 30)) {
> +               unit = PTP_CFG_CLK_ADJ_FREQ_NS;
> +               do_div(adj, 1000);
> +       }
> +
> +       /* Still too big */
> +       if (adj >= (1L << 30))
> +               goto disable_adj;
> +
> +       ocelot_write(ocelot, unit | adj, PTP_CLK_CFG_ADJ_FREQ);
> +       ocelot_write(ocelot, PTP_CFG_CLK_ADJ_CFG_ENA | direction,
> +                    PTP_CLK_CFG_ADJ_CFG);
> +
> +       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> +       return 0;
> +
> +disable_adj:
> +       ocelot_write(ocelot, 0, PTP_CLK_CFG_ADJ_CFG);
> +
> +       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> +       return 0;
> +}
> +
> +static struct ptp_clock_info ocelot_ptp_clock_info = {
> +       .owner          = THIS_MODULE,
> +       .name           = "ocelot ptp",
> +       .max_adj        = 0x7fffffff,
> +       .n_alarm        = 0,
> +       .n_ext_ts       = 0,
> +       .n_per_out      = 0,
> +       .n_pins         = 0,
> +       .pps            = 0,
> +       .gettime64      = ocelot_ptp_gettime64,
> +       .settime64      = ocelot_ptp_settime64,
> +       .adjtime        = ocelot_ptp_adjtime,
> +       .adjfine        = ocelot_ptp_adjfine,
> +};
> +
> +int ocelot_init_timestamp(struct ocelot *ocelot)
> +{
> +       struct ptp_clock *ptp_clock;
> +
> +       ocelot->ptp_info = ocelot_ptp_clock_info;
> +       ptp_clock = ptp_clock_register(&ocelot->ptp_info, ocelot->dev);
> +       if (IS_ERR(ptp_clock))
> +               return PTR_ERR(ptp_clock);
> +       /* Check if PHC support is missing at the configuration level */
> +       if (!ptp_clock)
> +               return 0;
> +
> +       ocelot->ptp_clock = ptp_clock;
> +
> +       ocelot_write(ocelot, SYS_PTP_CFG_PTP_STAMP_WID(30), SYS_PTP_CFG);
> +       ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_LOW);
> +       ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_HIGH);
> +
> +       ocelot_write(ocelot, PTP_CFG_MISC_PTP_EN, PTP_CFG_MISC);
> +
> +       /* There is no device reconfiguration, PTP Rx stamping is always
> +        * enabled.
> +        */
> +       ocelot->hwtstamp_config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(ocelot_init_timestamp);
> +
> +int ocelot_deinit_timestamp(struct ocelot *ocelot)
> +{
> +       if (ocelot->ptp_clock)
> +               ptp_clock_unregister(ocelot->ptp_clock);
> +       return 0;
> +}
> +EXPORT_SYMBOL(ocelot_deinit_timestamp);
> diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
> index 007b584..d9bad70 100644
> --- a/include/soc/mscc/ocelot.h
> +++ b/include/soc/mscc/ocelot.h
> @@ -607,7 +607,6 @@ int ocelot_vlan_add(struct ocelot *ocelot, int port, u16 vid, bool pvid,
>  int ocelot_vlan_del(struct ocelot *ocelot, int port, u16 vid);
>  int ocelot_hwstamp_get(struct ocelot *ocelot, int port, struct ifreq *ifr);
>  int ocelot_hwstamp_set(struct ocelot *ocelot, int port, struct ifreq *ifr);
> -int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts);
>  int ocelot_port_add_txtstamp_skb(struct ocelot_port *ocelot_port,
>                                  struct sk_buff *skb);
>  void ocelot_get_txtstamp(struct ocelot *ocelot);
> diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.h b/include/soc/mscc/ocelot_ptp.h
> similarity index 97%
> rename from drivers/net/ethernet/mscc/ocelot_ptp.h
> rename to include/soc/mscc/ocelot_ptp.h
> index 9ede14a..2dd27f0 100644
> --- a/drivers/net/ethernet/mscc/ocelot_ptp.h
> +++ b/include/soc/mscc/ocelot_ptp.h
> @@ -4,6 +4,7 @@
>   *
>   * License: Dual MIT/GPL
>   * Copyright (c) 2017 Microsemi Corporation
> + * Copyright 2020 NXP
>   */
>
>  #ifndef _MSCC_OCELOT_PTP_H_
> diff --git a/include/soc/mscc/ptp_ocelot.h b/include/soc/mscc/ptp_ocelot.h
> new file mode 100644
> index 0000000..b8d9c5b
> --- /dev/null
> +++ b/include/soc/mscc/ptp_ocelot.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Microsemi Ocelot PTP clock driver
> + *
> + * License: Dual MIT/GPL
> + * Copyright 2020 NXP
> + */
> +
> +#ifndef _PTP_OCELOT_H_
> +#define _PTP_OCELOT_H_
> +
> +#include <soc/mscc/ocelot.h>
> +#include <linux/ptp_clock_kernel.h>
> +
> +#ifdef CONFIG_PTP_1588_CLOCK_OCELOT

And if you decide to allow building it as a module, you should change
this to "#if IS_REACHABLE(CONFIG_PTP_1588_CLOCK_OCELOT)" to cover that
case too.

> +int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts);
> +int ocelot_init_timestamp(struct ocelot *ocelot);
> +int ocelot_deinit_timestamp(struct ocelot *ocelot);
> +#else
> +static inline int ocelot_ptp_gettime64(struct ptp_clock_info *ptp,
> +                                      struct timespec64 *ts)
> +{
> +       return -EOPNOTSUPP;
> +}
> +static inline int ocelot_init_timestamp(struct ocelot *ocelot)
> +{
> +       return -EOPNOTSUPP;
> +}
> +static inline int ocelot_deinit_timestamp(struct ocelot *ocelot)
> +{
> +       return -EOPNOTSUPP;
> +}
> +#endif
> +#endif
> --
> 2.7.4
>

Thanks,
-Vladimir

RE: [PATCH 1/6] ptp: move ocelot ptp clock code out of Ethernet driver

[Y.b. Lu]

Hi Vladimir,

> -----Original Message-----
> From: Vladimir Oltean <olteanv@gmail.com>
> Sent: Saturday, March 21, 2020 12:37 AM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: lkml <linux-kernel@vger.kernel.org>; netdev <netdev@vger.kernel.org>;
> David S . Miller <davem@davemloft.net>; Richard Cochran
> <richardcochran@gmail.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Claudiu Manoil <claudiu.manoil@nxp.com>; Andrew Lunn <andrew@lunn.ch>;
> Vivien Didelot <vivien.didelot@gmail.com>; Florian Fainelli
> <f.fainelli@gmail.com>; Alexandre Belloni <alexandre.belloni@bootlin.com>;
> Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH 1/6] ptp: move ocelot ptp clock code out of Ethernet
> driver
> 
> I hate to be that guy who reports build error before the Kbuild robot
> does, but here goes.
> 
> On Fri, 20 Mar 2020 at 12:41, Yangbo Lu <yangbo.lu@nxp.com> wrote:
> >
> > The Ocelot PTP clock driver had been embedded into ocelot.c driver.
> > It had supported basic gettime64/settime64/adjtime/adjfine functions
> > by now which were used by both Ocelot switch and Felix switch.
> >
> > This patch is to move current ptp clock code out of ocelot.c driver
> > maintaining as a single ptp_ocelot.c driver.
> > For futher new features implementation, the common code could be put
> > in ptp_ocelot.c driver and the switch specific code should be in
> > specific switch driver. The interrupt implementation in SoC is different
> > between Ocelot and Felix.
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> >  drivers/net/dsa/ocelot/felix.c                     |   3 +-
> >  drivers/net/ethernet/mscc/ocelot.c                 | 201 +------------------
> >  drivers/net/ethernet/mscc/ocelot.h                 |   3 +-
> >  drivers/net/ethernet/mscc/ocelot_board.c           |   1 +
> >  drivers/ptp/Kconfig                                |  10 +
> >  drivers/ptp/Makefile                               |   1 +
> >  drivers/ptp/ptp_ocelot.c                           | 217
> +++++++++++++++++++++
> >  include/soc/mscc/ocelot.h                          |   1 -
> >  .../net/ethernet => include/soc}/mscc/ocelot_ptp.h |   1 +
> >  include/soc/mscc/ptp_ocelot.h                      |  34 ++++
> >  10 files changed, 271 insertions(+), 201 deletions(-)
> >  create mode 100644 drivers/ptp/ptp_ocelot.c
> >  rename {drivers/net/ethernet => include/soc}/mscc/ocelot_ptp.h (97%)
> >  create mode 100644 include/soc/mscc/ptp_ocelot.h
> >
> > diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> > index 6954638..9f9efb9 100644
> > --- a/drivers/net/dsa/ocelot/felix.c
> > +++ b/drivers/net/dsa/ocelot/felix.c
> > @@ -8,6 +8,7 @@
> >  #include <soc/mscc/ocelot_dev.h>
> >  #include <soc/mscc/ocelot_ana.h>
> >  #include <soc/mscc/ocelot.h>
> > +#include <soc/mscc/ptp_ocelot.h>
> >  #include <linux/packing.h>
> >  #include <linux/module.h>
> >  #include <linux/of_net.h>
> > @@ -576,7 +577,7 @@ static bool felix_rxtstamp(struct dsa_switch *ds, int
> port,
> >         struct ocelot *ocelot = ds->priv;
> >         u8 *extraction = skb->data - ETH_HLEN - OCELOT_TAG_LEN;
> >         u32 tstamp_lo, tstamp_hi;
> > -       struct timespec64 ts;
> > +       struct timespec64 ts = {0, 0};
> >         u64 tstamp, val;
> >
> >         ocelot_ptp_gettime64(&ocelot->ptp_info, &ts);
> > diff --git a/drivers/net/ethernet/mscc/ocelot.c
> b/drivers/net/ethernet/mscc/ocelot.c
> > index dc0e273..b342bbd 100644
> > --- a/drivers/net/ethernet/mscc/ocelot.c
> > +++ b/drivers/net/ethernet/mscc/ocelot.c
> > @@ -21,6 +21,7 @@
> >  #include <net/netevent.h>
> >  #include <net/rtnetlink.h>
> >  #include <net/switchdev.h>
> > +#include <soc/mscc/ptp_ocelot.h>
> >
> >  #include "ocelot.h"
> >  #include "ocelot_ace.h"
> > @@ -1989,200 +1990,6 @@ struct notifier_block
> ocelot_switchdev_blocking_nb __read_mostly = {
> >  };
> >  EXPORT_SYMBOL(ocelot_switchdev_blocking_nb);
> >
> > -int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts)
> > -{
> > -       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> > -       unsigned long flags;
> > -       time64_t s;
> > -       u32 val;
> > -       s64 ns;
> > -
> > -       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> > -
> > -       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> > -       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK |
> PTP_PIN_CFG_DOM);
> > -       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_SAVE);
> > -       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> > -
> > -       s = ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN)
> & 0xffff;
> > -       s <<= 32;
> > -       s += ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
> > -       ns = ocelot_read_rix(ocelot, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> > -
> > -       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > -
> > -       /* Deal with negative values */
> > -       if (ns >= 0x3ffffff0 && ns <= 0x3fffffff) {
> > -               s--;
> > -               ns &= 0xf;
> > -               ns += 999999984;
> > -       }
> > -
> > -       set_normalized_timespec64(ts, s, ns);
> > -       return 0;
> > -}
> > -EXPORT_SYMBOL(ocelot_ptp_gettime64);
> > -
> > -static int ocelot_ptp_settime64(struct ptp_clock_info *ptp,
> > -                               const struct timespec64 *ts)
> > -{
> > -       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> > -       unsigned long flags;
> > -       u32 val;
> > -
> > -       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> > -
> > -       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> > -       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK |
> PTP_PIN_CFG_DOM);
> > -       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
> > -
> > -       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> > -
> > -       ocelot_write_rix(ocelot, lower_32_bits(ts->tv_sec),
> PTP_PIN_TOD_SEC_LSB,
> > -                        TOD_ACC_PIN);
> > -       ocelot_write_rix(ocelot, upper_32_bits(ts->tv_sec),
> PTP_PIN_TOD_SEC_MSB,
> > -                        TOD_ACC_PIN);
> > -       ocelot_write_rix(ocelot, ts->tv_nsec, PTP_PIN_TOD_NSEC,
> TOD_ACC_PIN);
> > -
> > -       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> > -       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK |
> PTP_PIN_CFG_DOM);
> > -       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_LOAD);
> > -
> > -       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> > -
> > -       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > -       return 0;
> > -}
> > -
> > -static int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> > -{
> > -       if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
> > -               struct ocelot *ocelot = container_of(ptp, struct ocelot,
> ptp_info);
> > -               unsigned long flags;
> > -               u32 val;
> > -
> > -               spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> > -
> > -               val = ocelot_read_rix(ocelot, PTP_PIN_CFG,
> TOD_ACC_PIN);
> > -               val &= ~(PTP_PIN_CFG_SYNC |
> PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> > -               val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
> > -
> > -               ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> > -
> > -               ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_LSB,
> TOD_ACC_PIN);
> > -               ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_MSB,
> TOD_ACC_PIN);
> > -               ocelot_write_rix(ocelot, delta, PTP_PIN_TOD_NSEC,
> TOD_ACC_PIN);
> > -
> > -               val = ocelot_read_rix(ocelot, PTP_PIN_CFG,
> TOD_ACC_PIN);
> > -               val &= ~(PTP_PIN_CFG_SYNC |
> PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> > -               val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_DELTA);
> > -
> > -               ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> > -
> > -               spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > -       } else {
> > -               /* Fall back using ocelot_ptp_settime64 which is not exact.
> */
> > -               struct timespec64 ts;
> > -               u64 now;
> > -
> > -               ocelot_ptp_gettime64(ptp, &ts);
> > -
> > -               now = ktime_to_ns(timespec64_to_ktime(ts));
> > -               ts = ns_to_timespec64(now + delta);
> > -
> > -               ocelot_ptp_settime64(ptp, &ts);
> > -       }
> > -       return 0;
> > -}
> > -
> > -static int ocelot_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> > -{
> > -       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> > -       u32 unit = 0, direction = 0;
> > -       unsigned long flags;
> > -       u64 adj = 0;
> > -
> > -       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> > -
> > -       if (!scaled_ppm)
> > -               goto disable_adj;
> > -
> > -       if (scaled_ppm < 0) {
> > -               direction = PTP_CFG_CLK_ADJ_CFG_DIR;
> > -               scaled_ppm = -scaled_ppm;
> > -       }
> > -
> > -       adj = PSEC_PER_SEC << 16;
> > -       do_div(adj, scaled_ppm);
> > -       do_div(adj, 1000);
> > -
> > -       /* If the adjustment value is too large, use ns instead */
> > -       if (adj >= (1L << 30)) {
> > -               unit = PTP_CFG_CLK_ADJ_FREQ_NS;
> > -               do_div(adj, 1000);
> > -       }
> > -
> > -       /* Still too big */
> > -       if (adj >= (1L << 30))
> > -               goto disable_adj;
> > -
> > -       ocelot_write(ocelot, unit | adj, PTP_CLK_CFG_ADJ_FREQ);
> > -       ocelot_write(ocelot, PTP_CFG_CLK_ADJ_CFG_ENA | direction,
> > -                    PTP_CLK_CFG_ADJ_CFG);
> > -
> > -       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > -       return 0;
> > -
> > -disable_adj:
> > -       ocelot_write(ocelot, 0, PTP_CLK_CFG_ADJ_CFG);
> > -
> > -       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > -       return 0;
> > -}
> > -
> > -static struct ptp_clock_info ocelot_ptp_clock_info = {
> > -       .owner          = THIS_MODULE,
> > -       .name           = "ocelot ptp",
> > -       .max_adj        = 0x7fffffff,
> > -       .n_alarm        = 0,
> > -       .n_ext_ts       = 0,
> > -       .n_per_out      = 0,
> > -       .n_pins         = 0,
> > -       .pps            = 0,
> > -       .gettime64      = ocelot_ptp_gettime64,
> > -       .settime64      = ocelot_ptp_settime64,
> > -       .adjtime        = ocelot_ptp_adjtime,
> > -       .adjfine        = ocelot_ptp_adjfine,
> > -};
> > -
> > -static int ocelot_init_timestamp(struct ocelot *ocelot)
> > -{
> > -       struct ptp_clock *ptp_clock;
> > -
> > -       ocelot->ptp_info = ocelot_ptp_clock_info;
> > -       ptp_clock = ptp_clock_register(&ocelot->ptp_info, ocelot->dev);
> > -       if (IS_ERR(ptp_clock))
> > -               return PTR_ERR(ptp_clock);
> > -       /* Check if PHC support is missing at the configuration level */
> > -       if (!ptp_clock)
> > -               return 0;
> > -
> > -       ocelot->ptp_clock = ptp_clock;
> > -
> > -       ocelot_write(ocelot, SYS_PTP_CFG_PTP_STAMP_WID(30),
> SYS_PTP_CFG);
> > -       ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_LOW);
> > -       ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_HIGH);
> > -
> > -       ocelot_write(ocelot, PTP_CFG_MISC_PTP_EN, PTP_CFG_MISC);
> > -
> > -       /* There is no device reconfiguration, PTP Rx stamping is always
> > -        * enabled.
> > -        */
> > -       ocelot->hwtstamp_config.rx_filter =
> HWTSTAMP_FILTER_PTP_V2_EVENT;
> > -
> > -       return 0;
> > -}
> > -
> >  /* Configure the maximum SDU (L2 payload) on RX to the value specified in
> @sdu.
> >   * The length of VLAN tags is accounted for automatically via
> DEV_MAC_TAGS_CFG.
> >   */
> > @@ -2507,7 +2314,8 @@ int ocelot_init(struct ocelot *ocelot)
> >                 ret = ocelot_init_timestamp(ocelot);
> >                 if (ret) {
> >                         dev_err(ocelot->dev,
> > -                               "Timestamp initialization failed\n");
> > +                               "Timestamp not enabled or
> initialization failed\n");
> > +                       ocelot->ptp = 0;
> >                         return ret;
> >                 }
> >         }
> > @@ -2524,8 +2332,7 @@ void ocelot_deinit(struct ocelot *ocelot)
> >         cancel_delayed_work(&ocelot->stats_work);
> >         destroy_workqueue(ocelot->stats_queue);
> >         mutex_destroy(&ocelot->stats_lock);
> > -       if (ocelot->ptp_clock)
> > -               ptp_clock_unregister(ocelot->ptp_clock);
> > +       ocelot_deinit_timestamp(ocelot);
> >
> >         for (i = 0; i < ocelot->num_phys_ports; i++) {
> >                 port = ocelot->ports[i];
> > diff --git a/drivers/net/ethernet/mscc/ocelot.h
> b/drivers/net/ethernet/mscc/ocelot.h
> > index e34ef83..5aa2e45 100644
> > --- a/drivers/net/ethernet/mscc/ocelot.h
> > +++ b/drivers/net/ethernet/mscc/ocelot.h
> > @@ -15,18 +15,17 @@
> >  #include <linux/phy.h>
> >  #include <linux/phy/phy.h>
> >  #include <linux/platform_device.h>
> > -#include <linux/ptp_clock_kernel.h>
> >  #include <linux/regmap.h>
> >
> >  #include <soc/mscc/ocelot_qsys.h>
> >  #include <soc/mscc/ocelot_sys.h>
> >  #include <soc/mscc/ocelot_dev.h>
> >  #include <soc/mscc/ocelot_ana.h>
> > +#include <soc/mscc/ocelot_ptp.h>
> >  #include <soc/mscc/ocelot.h>
> >  #include "ocelot_rew.h"
> >  #include "ocelot_qs.h"
> >  #include "ocelot_tc.h"
> > -#include "ocelot_ptp.h"
> >
> >  #define OCELOT_BUFFER_CELL_SZ 60
> >
> > diff --git a/drivers/net/ethernet/mscc/ocelot_board.c
> b/drivers/net/ethernet/mscc/ocelot_board.c
> > index 0ac9fbf7..7e59cee 100644
> > --- a/drivers/net/ethernet/mscc/ocelot_board.c
> > +++ b/drivers/net/ethernet/mscc/ocelot_board.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/skbuff.h>
> >  #include <net/switchdev.h>
> >
> > +#include <soc/mscc/ptp_ocelot.h>
> >  #include <soc/mscc/ocelot_vcap.h>
> >  #include "ocelot.h"
> >
> > diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
> > index 86400c7..ac08e9c 100644
> > --- a/drivers/ptp/Kconfig
> > +++ b/drivers/ptp/Kconfig
> > @@ -151,4 +151,14 @@ config PTP_1588_CLOCK_VMW
> >           To compile this driver as a module, choose M here: the module
> >           will be called ptp_vmw.
> >
> > +config PTP_1588_CLOCK_OCELOT
> > +       bool "Microsemi Ocelot as PTP clock"
> 
> Why bool and not tristate? Compilation breaks when
> MSCC_OCELOT_SWITCH=m because it forces PTP_1588_CLOCK_OCELOT=y.

The ocelot driver (MSCC_OCELOT_SWITCH) was designed as driver module providing just functions.
Now I am aware that drivers/net/ethernet/mscc/ should still be the better place to put the ptp driver, if we just move common ptp functions out.
The ptp driver should be bool for building into module of MSCC_OCELOT_SWITCH, providing functions to ocelot platform or felix platform.

And I'd like to do another changes to move ocelot_init_timestamp/ocelot_deinit_timestamp calling to ocelot_board.c and felix.c.
Because the ptp capabilities will be different between them in development stage since they have different interrupt implementation in hardware.

Thanks.

> 
> drivers/ptp/ptp_ocelot.o: In function `ocelot_ptp_settime64':
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:56: undefined reference
> to `__ocelot_read_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:60: undefined reference
> to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:62: undefined reference
> to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:64: undefined reference
> to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:66: undefined reference
> to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:68: undefined reference
> to `__ocelot_read_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:72: undefined reference
> to `__ocelot_write_ix'
> drivers/ptp/ptp_ocelot.o: In function `ocelot_ptp_adjfine':
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:161: undefined
> reference to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:153: undefined
> reference to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:154: undefined
> reference to `__ocelot_write_ix'
> drivers/ptp/ptp_ocelot.o: In function `ocelot_ptp_gettime64':
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:23: undefined reference
> to `__ocelot_read_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:26: undefined reference
> to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:28: undefined reference
> to `__ocelot_read_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:30: undefined reference
> to `__ocelot_read_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:31: undefined reference
> to `__ocelot_read_ix'
> drivers/ptp/ptp_ocelot.o: In function `ocelot_ptp_enable':
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:231: undefined
> reference to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:232: undefined
> reference to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:235: undefined
> reference to `__ocelot_write_ix'
> drivers/ptp/ptp_ocelot.o: In function `ocelot_init_timestamp':
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:289: undefined
> reference to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:290: undefined
> reference to `__ocelot_write_ix'
> drivers/ptp/ptp_ocelot.o:/opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:291:
> more undefined references to `__ocelot_write_ix' follow
> drivers/ptp/ptp_ocelot.o: In function `ocelot_ptp_adjtime':
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:88: undefined reference
> to `__ocelot_read_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:93: undefined reference
> to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:95: undefined reference
> to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:96: undefined reference
> to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:97: undefined reference
> to `__ocelot_write_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:99: undefined reference
> to `__ocelot_read_ix'
> /opt/ls1028ardb-linux/drivers/ptp/ptp_ocelot.c:104: undefined
> reference to `__ocelot_write_ix'
> 
> > +       depends on MSCC_OCELOT_SWITCH || COMPILE_TEST
> > +       depends on PTP_1588_CLOCK
> > +       default y
> > +       help
> > +         This driver adds support for using Microsemi Ocelot as a PTP
> > +         clock. This clock is only useful if your PTP programs are
> > +         getting hardware time stamps on the PTP Ethernet packets using
> > +         the SO_TIMESTAMPING API.
> >  endmenu
> > diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
> > index 7aff75f..a0229b3 100644
> > --- a/drivers/ptp/Makefile
> > +++ b/drivers/ptp/Makefile
> > @@ -15,3 +15,4 @@ ptp-qoriq-$(CONFIG_DEBUG_FS)          +=
> ptp_qoriq_debugfs.o
> >  obj-$(CONFIG_PTP_1588_CLOCK_IDTCM)     += ptp_clockmatrix.o
> >  obj-$(CONFIG_PTP_1588_CLOCK_IDT82P33)  += ptp_idt82p33.o
> >  obj-$(CONFIG_PTP_1588_CLOCK_VMW)       += ptp_vmw.o
> > +obj-$(CONFIG_PTP_1588_CLOCK_OCELOT)    += ptp_ocelot.o
> > diff --git a/drivers/ptp/ptp_ocelot.c b/drivers/ptp/ptp_ocelot.c
> > new file mode 100644
> > index 0000000..59420a7
> > --- /dev/null
> > +++ b/drivers/ptp/ptp_ocelot.c
> > @@ -0,0 +1,217 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Microsemi Ocelot PTP clock driver
> > + *
> > + * Copyright (c) 2017 Microsemi Corporation
> > + * Copyright 2020 NXP
> > + */
> > +#include <soc/mscc/ocelot_ptp.h>
> > +#include <soc/mscc/ocelot_sys.h>
> > +#include <soc/mscc/ocelot.h>
> > +#include <soc/mscc/ptp_ocelot.h>
> > +
> > +int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts)
> > +{
> > +       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> > +       unsigned long flags;
> > +       time64_t s;
> > +       u32 val;
> > +       s64 ns;
> > +
> > +       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> > +
> > +       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> > +       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK |
> PTP_PIN_CFG_DOM);
> > +       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_SAVE);
> > +       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> > +
> > +       s = ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN)
> & 0xffff;
> > +       s <<= 32;
> > +       s += ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_LSB,
> TOD_ACC_PIN);
> > +       ns = ocelot_read_rix(ocelot, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> > +
> > +       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > +
> > +       /* Deal with negative values */
> > +       if (ns >= 0x3ffffff0 && ns <= 0x3fffffff) {
> > +               s--;
> > +               ns &= 0xf;
> > +               ns += 999999984;
> > +       }
> > +
> > +       set_normalized_timespec64(ts, s, ns);
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL(ocelot_ptp_gettime64);
> > +
> > +static int ocelot_ptp_settime64(struct ptp_clock_info *ptp,
> > +                               const struct timespec64 *ts)
> > +{
> > +       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> > +       unsigned long flags;
> > +       u32 val;
> > +
> > +       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> > +
> > +       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> > +       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK |
> PTP_PIN_CFG_DOM);
> > +       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
> > +
> > +       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> > +
> > +       ocelot_write_rix(ocelot, lower_32_bits(ts->tv_sec),
> PTP_PIN_TOD_SEC_LSB,
> > +                        TOD_ACC_PIN);
> > +       ocelot_write_rix(ocelot, upper_32_bits(ts->tv_sec),
> PTP_PIN_TOD_SEC_MSB,
> > +                        TOD_ACC_PIN);
> > +       ocelot_write_rix(ocelot, ts->tv_nsec, PTP_PIN_TOD_NSEC,
> TOD_ACC_PIN);
> > +
> > +       val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> > +       val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK |
> PTP_PIN_CFG_DOM);
> > +       val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_LOAD);
> > +
> > +       ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> > +
> > +       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > +       return 0;
> > +}
> > +
> > +static int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> > +{
> > +       if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
> > +               struct ocelot *ocelot = container_of(ptp, struct ocelot,
> > +                                                    ptp_info);
> > +               unsigned long flags;
> > +               u32 val;
> > +
> > +               spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> > +
> > +               val = ocelot_read_rix(ocelot, PTP_PIN_CFG,
> TOD_ACC_PIN);
> > +               val &= ~(PTP_PIN_CFG_SYNC |
> PTP_PIN_CFG_ACTION_MASK |
> > +                        PTP_PIN_CFG_DOM);
> > +               val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
> > +
> > +               ocelot_write_rix(ocelot, val, PTP_PIN_CFG,
> TOD_ACC_PIN);
> > +
> > +               ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_LSB,
> TOD_ACC_PIN);
> > +               ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_MSB,
> TOD_ACC_PIN);
> > +               ocelot_write_rix(ocelot, delta, PTP_PIN_TOD_NSEC,
> TOD_ACC_PIN);
> > +
> > +               val = ocelot_read_rix(ocelot, PTP_PIN_CFG,
> TOD_ACC_PIN);
> > +               val &= ~(PTP_PIN_CFG_SYNC |
> PTP_PIN_CFG_ACTION_MASK |
> > +                        PTP_PIN_CFG_DOM);
> > +               val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_DELTA);
> > +
> > +               ocelot_write_rix(ocelot, val, PTP_PIN_CFG,
> TOD_ACC_PIN);
> > +
> > +               spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > +       } else {
> > +               /* Fall back using ocelot_ptp_settime64 which is not
> exact. */
> > +               struct timespec64 ts;
> > +               u64 now;
> > +
> > +               ocelot_ptp_gettime64(ptp, &ts);
> > +
> > +               now = ktime_to_ns(timespec64_to_ktime(ts));
> > +               ts = ns_to_timespec64(now + delta);
> > +
> > +               ocelot_ptp_settime64(ptp, &ts);
> > +       }
> > +       return 0;
> > +}
> > +
> > +static int ocelot_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> > +{
> > +       struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> > +       u32 unit = 0, direction = 0;
> > +       unsigned long flags;
> > +       u64 adj = 0;
> > +
> > +       spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
> > +
> > +       if (!scaled_ppm)
> > +               goto disable_adj;
> > +
> > +       if (scaled_ppm < 0) {
> > +               direction = PTP_CFG_CLK_ADJ_CFG_DIR;
> > +               scaled_ppm = -scaled_ppm;
> > +       }
> > +
> > +       adj = PSEC_PER_SEC << 16;
> > +       do_div(adj, scaled_ppm);
> > +       do_div(adj, 1000);
> > +
> > +       /* If the adjustment value is too large, use ns instead */
> > +       if (adj >= (1L << 30)) {
> > +               unit = PTP_CFG_CLK_ADJ_FREQ_NS;
> > +               do_div(adj, 1000);
> > +       }
> > +
> > +       /* Still too big */
> > +       if (adj >= (1L << 30))
> > +               goto disable_adj;
> > +
> > +       ocelot_write(ocelot, unit | adj, PTP_CLK_CFG_ADJ_FREQ);
> > +       ocelot_write(ocelot, PTP_CFG_CLK_ADJ_CFG_ENA | direction,
> > +                    PTP_CLK_CFG_ADJ_CFG);
> > +
> > +       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > +       return 0;
> > +
> > +disable_adj:
> > +       ocelot_write(ocelot, 0, PTP_CLK_CFG_ADJ_CFG);
> > +
> > +       spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
> > +       return 0;
> > +}
> > +
> > +static struct ptp_clock_info ocelot_ptp_clock_info = {
> > +       .owner          = THIS_MODULE,
> > +       .name           = "ocelot ptp",
> > +       .max_adj        = 0x7fffffff,
> > +       .n_alarm        = 0,
> > +       .n_ext_ts       = 0,
> > +       .n_per_out      = 0,
> > +       .n_pins         = 0,
> > +       .pps            = 0,
> > +       .gettime64      = ocelot_ptp_gettime64,
> > +       .settime64      = ocelot_ptp_settime64,
> > +       .adjtime        = ocelot_ptp_adjtime,
> > +       .adjfine        = ocelot_ptp_adjfine,
> > +};
> > +
> > +int ocelot_init_timestamp(struct ocelot *ocelot)
> > +{
> > +       struct ptp_clock *ptp_clock;
> > +
> > +       ocelot->ptp_info = ocelot_ptp_clock_info;
> > +       ptp_clock = ptp_clock_register(&ocelot->ptp_info, ocelot->dev);
> > +       if (IS_ERR(ptp_clock))
> > +               return PTR_ERR(ptp_clock);
> > +       /* Check if PHC support is missing at the configuration level */
> > +       if (!ptp_clock)
> > +               return 0;
> > +
> > +       ocelot->ptp_clock = ptp_clock;
> > +
> > +       ocelot_write(ocelot, SYS_PTP_CFG_PTP_STAMP_WID(30),
> SYS_PTP_CFG);
> > +       ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_LOW);
> > +       ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_HIGH);
> > +
> > +       ocelot_write(ocelot, PTP_CFG_MISC_PTP_EN, PTP_CFG_MISC);
> > +
> > +       /* There is no device reconfiguration, PTP Rx stamping is always
> > +        * enabled.
> > +        */
> > +       ocelot->hwtstamp_config.rx_filter =
> HWTSTAMP_FILTER_PTP_V2_EVENT;
> > +
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL(ocelot_init_timestamp);
> > +
> > +int ocelot_deinit_timestamp(struct ocelot *ocelot)
> > +{
> > +       if (ocelot->ptp_clock)
> > +               ptp_clock_unregister(ocelot->ptp_clock);
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL(ocelot_deinit_timestamp);
> > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
> > index 007b584..d9bad70 100644
> > --- a/include/soc/mscc/ocelot.h
> > +++ b/include/soc/mscc/ocelot.h
> > @@ -607,7 +607,6 @@ int ocelot_vlan_add(struct ocelot *ocelot, int port,
> u16 vid, bool pvid,
> >  int ocelot_vlan_del(struct ocelot *ocelot, int port, u16 vid);
> >  int ocelot_hwstamp_get(struct ocelot *ocelot, int port, struct ifreq *ifr);
> >  int ocelot_hwstamp_set(struct ocelot *ocelot, int port, struct ifreq *ifr);
> > -int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64
> *ts);
> >  int ocelot_port_add_txtstamp_skb(struct ocelot_port *ocelot_port,
> >                                  struct sk_buff *skb);
> >  void ocelot_get_txtstamp(struct ocelot *ocelot);
> > diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.h
> b/include/soc/mscc/ocelot_ptp.h
> > similarity index 97%
> > rename from drivers/net/ethernet/mscc/ocelot_ptp.h
> > rename to include/soc/mscc/ocelot_ptp.h
> > index 9ede14a..2dd27f0 100644
> > --- a/drivers/net/ethernet/mscc/ocelot_ptp.h
> > +++ b/include/soc/mscc/ocelot_ptp.h
> > @@ -4,6 +4,7 @@
> >   *
> >   * License: Dual MIT/GPL
> >   * Copyright (c) 2017 Microsemi Corporation
> > + * Copyright 2020 NXP
> >   */
> >
> >  #ifndef _MSCC_OCELOT_PTP_H_
> > diff --git a/include/soc/mscc/ptp_ocelot.h b/include/soc/mscc/ptp_ocelot.h
> > new file mode 100644
> > index 0000000..b8d9c5b
> > --- /dev/null
> > +++ b/include/soc/mscc/ptp_ocelot.h
> > @@ -0,0 +1,34 @@
> > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> > +/*
> > + * Microsemi Ocelot PTP clock driver
> > + *
> > + * License: Dual MIT/GPL
> > + * Copyright 2020 NXP
> > + */
> > +
> > +#ifndef _PTP_OCELOT_H_
> > +#define _PTP_OCELOT_H_
> > +
> > +#include <soc/mscc/ocelot.h>
> > +#include <linux/ptp_clock_kernel.h>
> > +
> > +#ifdef CONFIG_PTP_1588_CLOCK_OCELOT
> 
> And if you decide to allow building it as a module, you should change
> this to "#if IS_REACHABLE(CONFIG_PTP_1588_CLOCK_OCELOT)" to cover that
> case too.
> 
> > +int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64
> *ts);
> > +int ocelot_init_timestamp(struct ocelot *ocelot);
> > +int ocelot_deinit_timestamp(struct ocelot *ocelot);
> > +#else
> > +static inline int ocelot_ptp_gettime64(struct ptp_clock_info *ptp,
> > +                                      struct timespec64 *ts)
> > +{
> > +       return -EOPNOTSUPP;
> > +}
> > +static inline int ocelot_init_timestamp(struct ocelot *ocelot)
> > +{
> > +       return -EOPNOTSUPP;
> > +}
> > +static inline int ocelot_deinit_timestamp(struct ocelot *ocelot)
> > +{
> > +       return -EOPNOTSUPP;
> > +}
> > +#endif
> > +#endif
> > --
> > 2.7.4
> >
> 
> Thanks,
> -Vladimir

Re: [PATCH 1/6] ptp: move ocelot ptp clock code out of Ethernet driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2250 bytes --]

Hi Yangbo,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on next-20200320]
[cannot apply to linux/master net/master linus/master v5.6-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Yangbo-Lu/Support-programmable-pins-for-Ocelot-PTP-driver/20200321-004535
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 3fd177cb2b47b29d36431229e4737e7286eefa29
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.2.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   sh4-linux-ld: drivers/ptp/ptp_ocelot.o: in function `ocelot_ptp_settime64':
>> ptp_ocelot.c:(.text+0xc0): undefined reference to `__ocelot_read_ix'
>> sh4-linux-ld: ptp_ocelot.c:(.text+0xc8): undefined reference to `__ocelot_write_ix'
   sh4-linux-ld: drivers/ptp/ptp_ocelot.o: in function `ocelot_ptp_gettime64':
>> (.text+0x1c8): undefined reference to `__ocelot_read_ix'
>> sh4-linux-ld: (.text+0x1d0): undefined reference to `__ocelot_write_ix'
   sh4-linux-ld: drivers/ptp/ptp_ocelot.o: in function `ocelot_ptp_adjtime':
>> ptp_ocelot.c:(.text+0x34c): undefined reference to `__ocelot_write_ix'
>> sh4-linux-ld: ptp_ocelot.c:(.text+0x350): undefined reference to `__ocelot_read_ix'
   sh4-linux-ld: drivers/ptp/ptp_ocelot.o: in function `ocelot_init_timestamp':
>> (.text+0x47c): undefined reference to `__ocelot_write_ix'
   sh4-linux-ld: drivers/ptp/ptp_ocelot.o: in function `ocelot_ptp_adjfine':
   ptp_ocelot.c:(.text+0x690): undefined reference to `__ocelot_write_ix'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53790 bytes --]

Re: [PATCH 1/6] ptp: move ocelot ptp clock code out of Ethernet driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2391 bytes --]

Hi Yangbo,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on next-20200320]
[cannot apply to linux/master net/master linus/master v5.6-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Yangbo-Lu/Support-programmable-pins-for-Ocelot-PTP-driver/20200321-004535
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 3fd177cb2b47b29d36431229e4737e7286eefa29
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.2.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   m68k-linux-ld: drivers/ptp/ptp_ocelot.o: in function `ocelot_ptp_settime64':
   ptp_ocelot.c:(.text+0x38): undefined reference to `__ocelot_read_ix'
>> m68k-linux-ld: ptp_ocelot.c:(.text+0x52): undefined reference to `__ocelot_write_ix'
   m68k-linux-ld: drivers/ptp/ptp_ocelot.o: in function `ocelot_ptp_adjfine':
   ptp_ocelot.c:(.text+0x196): undefined reference to `__ocelot_write_ix'
   m68k-linux-ld: ptp_ocelot.c:(.text+0x1ce): undefined reference to `__ocelot_write_ix'
   m68k-linux-ld: drivers/ptp/ptp_ocelot.o: in function `ocelot_ptp_gettime64':
   ptp_ocelot.c:(.text+0x222): undefined reference to `__ocelot_read_ix'
   m68k-linux-ld: ptp_ocelot.c:(.text+0x240): undefined reference to `__ocelot_write_ix'
   m68k-linux-ld: drivers/ptp/ptp_ocelot.o: in function `ocelot_ptp_adjtime':
   ptp_ocelot.c:(.text+0x34c): undefined reference to `__ocelot_read_ix'
   m68k-linux-ld: ptp_ocelot.c:(.text+0x366): undefined reference to `__ocelot_write_ix'
   m68k-linux-ld: drivers/ptp/ptp_ocelot.o: in function `ocelot_init_timestamp':
   ptp_ocelot.c:(.text+0x4be): undefined reference to `__ocelot_write_ix'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52834 bytes --]

[PATCH 2/6] MAINTAINERS: add entry for Microsemi Ocelot PTP driver

[Yangbo Lu]

Add entry for Microsemi Ocelot PTP driver.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5dbee41..8da6fc1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11115,6 +11115,15 @@ S:	Supported
 F:	drivers/net/ethernet/mscc/
 F:	include/soc/mscc/ocelot*
 
+MICROSEMI OCELOT PTP CLOCK DRIVER
+M:	Alexandre Belloni <alexandre.belloni@bootlin.com>
+M:	Yangbo Lu <yangbo.lu@nxp.com>
+M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
+L:	netdev@vger.kernel.org
+S:	Supported
+F:	drivers/ptp/ptp_ocelot.c
+F:	include/soc/mscc/ptp_ocelot.h
+
 MICROSOFT SURFACE PRO 3 BUTTON DRIVER
 M:	Chen Yu <yu.c.chen@intel.com>
 L:	platform-driver-x86@vger.kernel.org
-- 
2.7.4

Re: [PATCH 2/6] MAINTAINERS: add entry for Microsemi Ocelot PTP driver

[Alexandre Belloni]

Hi,

On 20/03/2020 18:37:22+0800, Yangbo Lu wrote:
> Add entry for Microsemi Ocelot PTP driver.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
>  MAINTAINERS | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5dbee41..8da6fc1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11115,6 +11115,15 @@ S:	Supported
>  F:	drivers/net/ethernet/mscc/
>  F:	include/soc/mscc/ocelot*
>  
> +MICROSEMI OCELOT PTP CLOCK DRIVER
> +M:	Alexandre Belloni <alexandre.belloni@bootlin.com>

I'm open to not be listed here as I'm not the main author of the code
and I'm not actively working on ptp for ocelot...

> +M:	Yangbo Lu <yangbo.lu@nxp.com>
> +M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>

...as long as you keep that address.

> +L:	netdev@vger.kernel.org
> +S:	Supported
> +F:	drivers/ptp/ptp_ocelot.c
> +F:	include/soc/mscc/ptp_ocelot.h
> +
>  MICROSOFT SURFACE PRO 3 BUTTON DRIVER
>  M:	Chen Yu <yu.c.chen@intel.com>
>  L:	platform-driver-x86@vger.kernel.org
> -- 
> 2.7.4
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [PATCH 2/6] MAINTAINERS: add entry for Microsemi Ocelot PTP driver

[Y.b. Lu]

Hi Alexandre,

> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Sent: Saturday, March 21, 2020 1:29 AM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; David S . Miller
> <davem@davemloft.net>; Richard Cochran <richardcochran@gmail.com>;
> Vladimir Oltean <vladimir.oltean@nxp.com>; Claudiu Manoil
> <claudiu.manoil@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Vivien Didelot
> <vivien.didelot@gmail.com>; Florian Fainelli <f.fainelli@gmail.com>;
> Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH 2/6] MAINTAINERS: add entry for Microsemi Ocelot PTP
> driver
> 
> Hi,
> 
> On 20/03/2020 18:37:22+0800, Yangbo Lu wrote:
> > Add entry for Microsemi Ocelot PTP driver.
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> >  MAINTAINERS | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5dbee41..8da6fc1 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11115,6 +11115,15 @@ S:	Supported
> >  F:	drivers/net/ethernet/mscc/
> >  F:	include/soc/mscc/ocelot*
> >
> > +MICROSEMI OCELOT PTP CLOCK DRIVER
> > +M:	Alexandre Belloni <alexandre.belloni@bootlin.com>
> 
> I'm open to not be listed here as I'm not the main author of the code
> and I'm not actively working on ptp for ocelot...
> 
> > +M:	Yangbo Lu <yangbo.lu@nxp.com>
> > +M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> 
> ...as long as you keep that address.

Get it. And thanks a lot.

> 
> > +L:	netdev@vger.kernel.org
> > +S:	Supported
> > +F:	drivers/ptp/ptp_ocelot.c
> > +F:	include/soc/mscc/ptp_ocelot.h
> > +
> >  MICROSOFT SURFACE PRO 3 BUTTON DRIVER
> >  M:	Chen Yu <yu.c.chen@intel.com>
> >  L:	platform-driver-x86@vger.kernel.org
> > --
> > 2.7.4
> >
> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin.
> com&amp;data=02%7C01%7Cyangbo.lu%40nxp.com%7Ca8238c4d91e74bb0
> 29b708d7ccf41ed6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
> 637203221179059159&amp;sdata=zEpGkU97BJryTf9NpcHj1%2BgHnxQhV%2
> BXoC9iewMmzjrw%3D&amp;reserved=0

[PATCH 3/6] net: mscc: ocelot: fix timestamp info if ptp clock does not work

[Yangbo Lu]

The timestamp info should be only software timestamp capabilities
if ptp clock does not work.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index b342bbd..114d605 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1348,6 +1348,12 @@ int ocelot_get_ts_info(struct ocelot *ocelot, int port,
 {
 	info->phc_index = ocelot->ptp_clock ?
 			  ptp_clock_index(ocelot->ptp_clock) : -1;
+	if (info->phc_index == -1) {
+		info->so_timestamping |= SOF_TIMESTAMPING_TX_SOFTWARE |
+					 SOF_TIMESTAMPING_RX_SOFTWARE |
+					 SOF_TIMESTAMPING_SOFTWARE;
+		return 0;
+	}
 	info->so_timestamping |= SOF_TIMESTAMPING_TX_SOFTWARE |
 				 SOF_TIMESTAMPING_RX_SOFTWARE |
 				 SOF_TIMESTAMPING_SOFTWARE |
-- 
2.7.4

[PATCH 4/6] net: mscc: ocelot: redefine PTP pins

[Yangbo Lu]

There are 5 PTP_PINS register groups on Ocelot switch.
Except the one used for TOD operations, there are still
4 register groups for programmable pins. So redefine the
4 programmable pins.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 include/soc/mscc/ocelot.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index d9bad70..0ad61e3 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -440,10 +440,11 @@ enum ocelot_regfield {
 	REGFIELD_MAX
 };
 
-enum ocelot_clk_pins {
-	ALT_PPS_PIN	= 1,
-	EXT_CLK_PIN,
-	ALT_LDST_PIN,
+enum ocelot_ptp_pins {
+	PTP_PIN_0,
+	PTP_PIN_1,
+	PTP_PIN_2,
+	PTP_PIN_3,
 	TOD_ACC_PIN
 };
 
-- 
2.7.4

[PATCH 5/6] net: mscc: ocelot: add wave programming registers definitions

[Yangbo Lu]

Add wave programming registers definitions for Ocelot platforms.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 drivers/net/dsa/ocelot/felix_vsc9959.c  | 2 ++
 drivers/net/ethernet/mscc/ocelot_regs.c | 2 ++
 include/soc/mscc/ocelot.h               | 2 ++
 include/soc/mscc/ocelot_ptp.h           | 2 ++
 4 files changed, 8 insertions(+)

diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index b4078f3..4fe707e 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -313,6 +313,8 @@ static const u32 vsc9959_ptp_regmap[] = {
 	REG(PTP_PIN_TOD_SEC_MSB,           0x000004),
 	REG(PTP_PIN_TOD_SEC_LSB,           0x000008),
 	REG(PTP_PIN_TOD_NSEC,              0x00000c),
+	REG(PTP_PIN_WF_HIGH_PERIOD,        0x000014),
+	REG(PTP_PIN_WF_LOW_PERIOD,         0x000018),
 	REG(PTP_CFG_MISC,                  0x0000a0),
 	REG(PTP_CLK_CFG_ADJ_CFG,           0x0000a4),
 	REG(PTP_CLK_CFG_ADJ_FREQ,          0x0000a8),
diff --git a/drivers/net/ethernet/mscc/ocelot_regs.c b/drivers/net/ethernet/mscc/ocelot_regs.c
index b88b589..ed4dd01 100644
--- a/drivers/net/ethernet/mscc/ocelot_regs.c
+++ b/drivers/net/ethernet/mscc/ocelot_regs.c
@@ -239,6 +239,8 @@ static const u32 ocelot_ptp_regmap[] = {
 	REG(PTP_PIN_TOD_SEC_MSB,           0x000004),
 	REG(PTP_PIN_TOD_SEC_LSB,           0x000008),
 	REG(PTP_PIN_TOD_NSEC,              0x00000c),
+	REG(PTP_PIN_WF_HIGH_PERIOD,        0x000014),
+	REG(PTP_PIN_WF_LOW_PERIOD,         0x000018),
 	REG(PTP_CFG_MISC,                  0x0000a0),
 	REG(PTP_CLK_CFG_ADJ_CFG,           0x0000a4),
 	REG(PTP_CLK_CFG_ADJ_FREQ,          0x0000a8),
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 0ad61e3..bcce278 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -385,6 +385,8 @@ enum ocelot_reg {
 	PTP_PIN_TOD_SEC_MSB,
 	PTP_PIN_TOD_SEC_LSB,
 	PTP_PIN_TOD_NSEC,
+	PTP_PIN_WF_HIGH_PERIOD,
+	PTP_PIN_WF_LOW_PERIOD,
 	PTP_CFG_MISC,
 	PTP_CLK_CFG_ADJ_CFG,
 	PTP_CLK_CFG_ADJ_FREQ,
diff --git a/include/soc/mscc/ocelot_ptp.h b/include/soc/mscc/ocelot_ptp.h
index 2dd27f0..3dd63d2 100644
--- a/include/soc/mscc/ocelot_ptp.h
+++ b/include/soc/mscc/ocelot_ptp.h
@@ -14,6 +14,8 @@
 #define PTP_PIN_TOD_SEC_MSB_RSZ		PTP_PIN_CFG_RSZ
 #define PTP_PIN_TOD_SEC_LSB_RSZ		PTP_PIN_CFG_RSZ
 #define PTP_PIN_TOD_NSEC_RSZ		PTP_PIN_CFG_RSZ
+#define PTP_PIN_WF_HIGH_PERIOD_RSZ	PTP_PIN_CFG_RSZ
+#define PTP_PIN_WF_LOW_PERIOD_RSZ	PTP_PIN_CFG_RSZ
 
 #define PTP_PIN_CFG_DOM			BIT(0)
 #define PTP_PIN_CFG_SYNC		BIT(2)
-- 
2.7.4

[PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Yangbo Lu]

Support 4 programmable pins for only one function periodic
signal for now. Since the hardware is not able to support
absolute start time, driver starts periodic signal immediately.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 drivers/ptp/ptp_ocelot.c  | 97 ++++++++++++++++++++++++++++++++++++++++++++++-
 include/soc/mscc/ocelot.h |  3 ++
 2 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/drivers/ptp/ptp_ocelot.c b/drivers/ptp/ptp_ocelot.c
index 59420a7..299928e 100644
--- a/drivers/ptp/ptp_ocelot.c
+++ b/drivers/ptp/ptp_ocelot.c
@@ -164,26 +164,119 @@ static int ocelot_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 	return 0;
 }
 
+static int ocelot_ptp_verify(struct ptp_clock_info *ptp, unsigned int pin,
+			     enum ptp_pin_function func, unsigned int chan)
+{
+	switch (func) {
+	case PTP_PF_NONE:
+	case PTP_PF_PEROUT:
+		break;
+	case PTP_PF_EXTTS:
+	case PTP_PF_PHYSYNC:
+		return -1;
+	}
+	return 0;
+}
+
+static int ocelot_ptp_enable(struct ptp_clock_info *ptp,
+			     struct ptp_clock_request *rq, int on)
+{
+	struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
+	enum ocelot_ptp_pins ptp_pin;
+	struct timespec64 ts;
+	unsigned long flags;
+	int pin = -1;
+	u32 val;
+	s64 ns;
+
+	switch (rq->type) {
+	case PTP_CLK_REQ_PEROUT:
+		/* Reject requests with unsupported flags */
+		if (rq->perout.flags)
+			return -EOPNOTSUPP;
+
+		/*
+		 * TODO: support disabling function
+		 * When ptp_disable_pinfunc() is to disable function,
+		 * it has already held pincfg_mux.
+		 * However ptp_find_pin() in .enable() called also needs
+		 * to hold pincfg_mux.
+		 * This causes dead lock. So, just return for function
+		 * disabling, and this needs fix-up.
+		 */
+		if (!on)
+			break;
+
+		pin = ptp_find_pin(ocelot->ptp_clock, PTP_PF_PEROUT,
+				   rq->perout.index);
+		if (pin == 0)
+			ptp_pin = PTP_PIN_0;
+		else if (pin == 1)
+			ptp_pin = PTP_PIN_1;
+		else if (pin == 2)
+			ptp_pin = PTP_PIN_2;
+		else if (pin == 3)
+			ptp_pin = PTP_PIN_3;
+		else
+			return -EINVAL;
+
+		ts.tv_sec = rq->perout.period.sec;
+		ts.tv_nsec = rq->perout.period.nsec;
+		ns = timespec64_to_ns(&ts);
+		ns = ns >> 1;
+		if (ns > 0x3fffffff || ns <= 0x6)
+			return -EINVAL;
+
+		spin_lock_irqsave(&ocelot->ptp_clock_lock, flags);
+		ocelot_write_rix(ocelot, ns, PTP_PIN_WF_LOW_PERIOD, ptp_pin);
+		ocelot_write_rix(ocelot, ns, PTP_PIN_WF_HIGH_PERIOD, ptp_pin);
+
+		val = PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_CLOCK);
+		ocelot_write_rix(ocelot, val, PTP_PIN_CFG, ptp_pin);
+		spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
+		dev_warn(ocelot->dev,
+			 "Starting periodic signal now! (absolute start time not supported)\n");
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+	return 0;
+}
+
 static struct ptp_clock_info ocelot_ptp_clock_info = {
 	.owner		= THIS_MODULE,
 	.name		= "ocelot ptp",
 	.max_adj	= 0x7fffffff,
 	.n_alarm	= 0,
 	.n_ext_ts	= 0,
-	.n_per_out	= 0,
-	.n_pins		= 0,
+	.n_per_out	= OCELOT_PTP_PINS_NUM,
+	.n_pins		= OCELOT_PTP_PINS_NUM,
 	.pps		= 0,
 	.gettime64	= ocelot_ptp_gettime64,
 	.settime64	= ocelot_ptp_settime64,
 	.adjtime	= ocelot_ptp_adjtime,
 	.adjfine	= ocelot_ptp_adjfine,
+	.verify		= ocelot_ptp_verify,
+	.enable		= ocelot_ptp_enable,
 };
 
 int ocelot_init_timestamp(struct ocelot *ocelot)
 {
 	struct ptp_clock *ptp_clock;
+	int i;
 
 	ocelot->ptp_info = ocelot_ptp_clock_info;
+
+	for (i = 0; i < OCELOT_PTP_PINS_NUM; i++) {
+		struct ptp_pin_desc *p = &ocelot->ptp_pins[i];
+
+		snprintf(p->name, sizeof(p->name), "switch_1588_dat%d", i);
+		p->index = i;
+		p->func = PTP_PF_NONE;
+	}
+
+	ocelot->ptp_info.pin_config = &ocelot->ptp_pins[0];
+
 	ptp_clock = ptp_clock_register(&ocelot->ptp_info, ocelot->dev);
 	if (IS_ERR(ptp_clock))
 		return PTR_ERR(ptp_clock);
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index bcce278..db2fb14 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -92,6 +92,8 @@
 #define OCELOT_SPEED_100		2
 #define OCELOT_SPEED_10			3
 
+#define OCELOT_PTP_PINS_NUM		4
+
 #define TARGET_OFFSET			24
 #define REG_MASK			GENMASK(TARGET_OFFSET - 1, 0)
 #define REG(reg, offset)		[reg & REG_MASK] = offset
@@ -544,6 +546,7 @@ struct ocelot {
 	struct mutex			ptp_lock;
 	/* Protects the PTP clock */
 	spinlock_t			ptp_clock_lock;
+	struct ptp_pin_desc		ptp_pins[OCELOT_PTP_PINS_NUM];
 };
 
 #define ocelot_read_ix(ocelot, reg, gi, ri) __ocelot_read_ix(ocelot, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
-- 
2.7.4

Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Vladimir Oltean]

Hi Yangbo,

On Fri, 20 Mar 2020 at 12:42, Yangbo Lu <yangbo.lu@nxp.com> wrote:
>
> Support 4 programmable pins for only one function periodic
> signal for now. Since the hardware is not able to support
> absolute start time, driver starts periodic signal immediately.
>

Are you absolutely sure it doesn't support absolute start time?
Because that would mean it's pretty useless if the phase of the PTP
clock signal is out of control.

I tested your patch on the LS1028A-RDB board using the following commands:

# Select PEROUT function and assign a channel to each of pins
SWITCH_1588_DAT0 and SWITCH_1588_DAT1
echo '2 0' > /sys/class/ptp/ptp1/pins/switch_1588_dat0
echo '2 1' > /sys/class/ptp/ptp1/pins/switch_1588_dat1
# Generate pulses with 1 second period on channel 0
echo '0 0 0 1 0' > /sys/class/ptp/ptp1/period
# Generate pulses with 1 second period on channel 1
echo '1 0 0 1 0' > /sys/class/ptp/ptp1/period

And here is what I get:
https://drive.google.com/open?id=1ErWufJL0TWv6hKDQdF1pRL5gn4hn4X-r

So the periodic output really starts 'now' just like the print says,
so the output from DAT0 is not even in sync with DAT1.

> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---

Thanks,
-Vladimir

RE: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Y.b. Lu]

Hi Vladimir and Richard,

> -----Original Message-----
> From: Vladimir Oltean <olteanv@gmail.com>
> Sent: Friday, March 20, 2020 9:21 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: lkml <linux-kernel@vger.kernel.org>; netdev <netdev@vger.kernel.org>;
> David S . Miller <davem@davemloft.net>; Richard Cochran
> <richardcochran@gmail.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Claudiu Manoil <claudiu.manoil@nxp.com>; Andrew Lunn <andrew@lunn.ch>;
> Vivien Didelot <vivien.didelot@gmail.com>; Florian Fainelli
> <f.fainelli@gmail.com>; Alexandre Belloni <alexandre.belloni@bootlin.com>;
> Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins
> 
> Hi Yangbo,
> 
> On Fri, 20 Mar 2020 at 12:42, Yangbo Lu <yangbo.lu@nxp.com> wrote:
> >
> > Support 4 programmable pins for only one function periodic
> > signal for now. Since the hardware is not able to support
> > absolute start time, driver starts periodic signal immediately.
> >
> 
> Are you absolutely sure it doesn't support absolute start time?
> Because that would mean it's pretty useless if the phase of the PTP
> clock signal is out of control.

I'm absolutely sure that absolute start time is not supported for periodic clock unless reference manual is wrong.
And I don’t think we need to consider phase for periodic clock which is with a specified period.

But PPS is different. Pulse should be generated must after seconds increased.
The waveform_high/low should be configurable for phase and pulse width if supported.
This is supported by hardware but was not implemented by this patch. I was considering to add later.

In my one previous patch, I was suggested to implement PPS with programmable pin periodic clock function.
But I didn’t find how should PPS be implemented with periodic clock function after checking ptp driver.
https://patchwork.ozlabs.org/patch/1215464/

Vladimir talked with me, for the special PPS case, we may consider,
if (req.perout.period.sec ==1 && req.perout.period.nsec == 0) and configure WAVEFORM_LOW to be equal to req_perout.start.nsec.

Richard, do you think is it ok?

And another problem I am facing is, in .enable() callback (PTP_CLK_REQ_PEROUT request) I defined.
                /*
                 * TODO: support disabling function
                 * When ptp_disable_pinfunc() is to disable function,
                 * it has already held pincfg_mux.
                 * However ptp_find_pin() in .enable() called also needs
                 * to hold pincfg_mux.
                 * This causes dead lock. So, just return for function
                 * disabling, and this needs fix-up.
                 */
Hope some suggestions here.
Thanks a lot.

> 
> I tested your patch on the LS1028A-RDB board using the following commands:
> 
> # Select PEROUT function and assign a channel to each of pins
> SWITCH_1588_DAT0 and SWITCH_1588_DAT1
> echo '2 0' > /sys/class/ptp/ptp1/pins/switch_1588_dat0
> echo '2 1' > /sys/class/ptp/ptp1/pins/switch_1588_dat1
> # Generate pulses with 1 second period on channel 0
> echo '0 0 0 1 0' > /sys/class/ptp/ptp1/period
> # Generate pulses with 1 second period on channel 1
> echo '1 0 0 1 0' > /sys/class/ptp/ptp1/period
> 
> And here is what I get:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdrive.g
> oogle.com%2Fopen%3Fid%3D1ErWufJL0TWv6hKDQdF1pRL5gn4hn4X-r&amp;
> data=02%7C01%7Cyangbo.lu%40nxp.com%7Cbd3e65bdaabb4999737d08d7c
> cd17eee%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63720307
> 2457124468&amp;sdata=4D97D9ZoA%2FDJeSAN%2Fha4zNuZL6GwRLNxpNY
> QiLsOsyM%3D&amp;reserved=0
> 
> So the periodic output really starts 'now' just like the print says,
> so the output from DAT0 is not even in sync with DAT1.
> 
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> 
> Thanks,
> -Vladimir

Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Richard Cochran]

On Tue, Mar 24, 2020 at 05:21:27AM +0000, Y.b. Lu wrote:
> In my one previous patch, I was suggested to implement PPS with programmable pin periodic clock function.
> But I didn’t find how should PPS be implemented with periodic clock function after checking ptp driver.
> https://patchwork.ozlabs.org/patch/1215464/

Yes, for generating a 1-PPS output waveform, users call ioctl
PTP_CLK_REQ_PEROUT with ptp_perout_request.period={1,0}.

If your device can't control the start time, then it can accept an
unspecified time of ptp_perout_request.start={0,0}.
 
> Vladimir talked with me, for the special PPS case, we may consider,
> if (req.perout.period.sec ==1 && req.perout.period.nsec == 0) and configure WAVEFORM_LOW to be equal to req_perout.start.nsec.
> 
> Richard, do you think is it ok?

Sound okay to me (but I don't know about WAVEFORM_LOW).

> And another problem I am facing is, in .enable() callback (PTP_CLK_REQ_PEROUT request) I defined.
>                 /*
>                  * TODO: support disabling function
>                  * When ptp_disable_pinfunc() is to disable function,
>                  * it has already held pincfg_mux.
>                  * However ptp_find_pin() in .enable() called also needs
>                  * to hold pincfg_mux.
>                  * This causes dead lock. So, just return for function
>                  * disabling, and this needs fix-up.
>                  */
> Hope some suggestions here.

See my reply to the patch.

Thanks,
Richard

RE: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Y.b. Lu]

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Tuesday, March 24, 2020 9:20 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: Vladimir Oltean <olteanv@gmail.com>; lkml
> <linux-kernel@vger.kernel.org>; netdev <netdev@vger.kernel.org>; David S .
> Miller <davem@davemloft.net>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Claudiu Manoil <claudiu.manoil@nxp.com>; Andrew Lunn <andrew@lunn.ch>;
> Vivien Didelot <vivien.didelot@gmail.com>; Florian Fainelli
> <f.fainelli@gmail.com>; Alexandre Belloni <alexandre.belloni@bootlin.com>;
> Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins
> 
> On Tue, Mar 24, 2020 at 05:21:27AM +0000, Y.b. Lu wrote:
> > In my one previous patch, I was suggested to implement PPS with
> programmable pin periodic clock function.
> > But I didn’t find how should PPS be implemented with periodic clock
> function after checking ptp driver.
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> ork.ozlabs.org%2Fpatch%2F1215464%2F&amp;data=02%7C01%7Cyangbo.lu
> %40nxp.com%7Cbfdbd209ae014cd8484b08d7cff60c13%7C686ea1d3bc2b4c6
> fa92cd99c5c301635%7C0%7C0%7C637206527981191161&amp;sdata=oy9m
> T%2Bl69H%2BmpzM9T2kPXQNSMm5w%2FowLhzziUJX2gZc%3D&amp;reserv
> ed=0
> 
> Yes, for generating a 1-PPS output waveform, users call ioctl
> PTP_CLK_REQ_PEROUT with ptp_perout_request.period={1,0}.
> 
> If your device can't control the start time, then it can accept an
> unspecified time of ptp_perout_request.start={0,0}.

Get it. Thanks a lot.

> 
> > Vladimir talked with me, for the special PPS case, we may consider,
> > if (req.perout.period.sec ==1 && req.perout.period.nsec == 0) and configure
> WAVEFORM_LOW to be equal to req_perout.start.nsec.
> >
> > Richard, do you think is it ok?
> 
> Sound okay to me (but I don't know about WAVEFORM_LOW).

Sorry. I should have explain more. There is a SYNC bit in Ocelot PTP hardware for PPS generation.
WAFEFORM_LOW register could be used to adjust phase.

RM says,
"For the CLOCK action, the sync option makes the pin generate a single pulse, <WAFEFORM_LOW>
nanoseconds after the time of day has increased the seconds. The pulse will get a width of
<WAVEFORM_HIGH> nanoseconds."

Then I will add PPS case in next version patch.
Thanks.

> 
> > And another problem I am facing is, in .enable() callback
> (PTP_CLK_REQ_PEROUT request) I defined.
> >                 /*
> >                  * TODO: support disabling function
> >                  * When ptp_disable_pinfunc() is to disable function,
> >                  * it has already held pincfg_mux.
> >                  * However ptp_find_pin() in .enable() called also needs
> >                  * to hold pincfg_mux.
> >                  * This causes dead lock. So, just return for function
> >                  * disabling, and this needs fix-up.
> >                  */
> > Hope some suggestions here.
> 
> See my reply to the patch.
> 
> Thanks,
> Richard

Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Horatiu Vultur]

Hi Vladimir,

The 03/20/2020 15:20, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Yangbo,
> 
> On Fri, 20 Mar 2020 at 12:42, Yangbo Lu <yangbo.lu@nxp.com> wrote:
> >
> > Support 4 programmable pins for only one function periodic
> > signal for now. Since the hardware is not able to support
> > absolute start time, driver starts periodic signal immediately.
> >
> 
> Are you absolutely sure it doesn't support absolute start time?
> Because that would mean it's pretty useless if the phase of the PTP
> clock signal is out of control.

It looks like there is no support for absolute start time. But you
should be able to control the phase using the register
PIN_WF_LOW_PERIOD.

> 
> I tested your patch on the LS1028A-RDB board using the following commands:
> 
> # Select PEROUT function and assign a channel to each of pins
> SWITCH_1588_DAT0 and SWITCH_1588_DAT1
> echo '2 0' > /sys/class/ptp/ptp1/pins/switch_1588_dat0
> echo '2 1' > /sys/class/ptp/ptp1/pins/switch_1588_dat1
> # Generate pulses with 1 second period on channel 0
> echo '0 0 0 1 0' > /sys/class/ptp/ptp1/period
> # Generate pulses with 1 second period on channel 1
> echo '1 0 0 1 0' > /sys/class/ptp/ptp1/period
> 
> And here is what I get:
> https://drive.google.com/open?id=1ErWufJL0TWv6hKDQdF1pRL5gn4hn4X-r
> 
> So the periodic output really starts 'now' just like the print says,
> so the output from DAT0 is not even in sync with DAT1.
> 
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> 
> Thanks,
> -Vladimir

-- 
/Horatiu

Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Richard Cochran]

On Fri, Mar 20, 2020 at 06:37:26PM +0800, Yangbo Lu wrote:
> +static int ocelot_ptp_enable(struct ptp_clock_info *ptp,
> +			     struct ptp_clock_request *rq, int on)
> +{
> +	struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> +	enum ocelot_ptp_pins ptp_pin;
> +	struct timespec64 ts;
> +	unsigned long flags;
> +	int pin = -1;
> +	u32 val;
> +	s64 ns;
> +
> +	switch (rq->type) {
> +	case PTP_CLK_REQ_PEROUT:
> +		/* Reject requests with unsupported flags */
> +		if (rq->perout.flags)
> +			return -EOPNOTSUPP;
> +
> +		/*
> +		 * TODO: support disabling function
> +		 * When ptp_disable_pinfunc() is to disable function,
> +		 * it has already held pincfg_mux.
> +		 * However ptp_find_pin() in .enable() called also needs
> +		 * to hold pincfg_mux.
> +		 * This causes dead lock. So, just return for function
> +		 * disabling, and this needs fix-up.

What dead lock?

When enable(PTP_CLK_REQ_PEROUT, on=0) is called, you don't need to
call ptp_disable_pinfunc().  Just stop the periodic waveform
generator.  The assignment of function to pin remains unchanged.

> +		 */
> +		if (!on)
> +			break;
> +
> +		pin = ptp_find_pin(ocelot->ptp_clock, PTP_PF_PEROUT,
> +				   rq->perout.index);
> +		if (pin == 0)
> +			ptp_pin = PTP_PIN_0;
> +		else if (pin == 1)
> +			ptp_pin = PTP_PIN_1;
> +		else if (pin == 2)
> +			ptp_pin = PTP_PIN_2;
> +		else if (pin == 3)
> +			ptp_pin = PTP_PIN_3;
> +		else
> +			return -EINVAL;

Return -EBUSY here instead.

Thanks,
Richard

RE: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Y.b. Lu]

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Tuesday, March 24, 2020 9:08 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; David S . Miller
> <davem@davemloft.net>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Claudiu Manoil <claudiu.manoil@nxp.com>; Andrew Lunn <andrew@lunn.ch>;
> Vivien Didelot <vivien.didelot@gmail.com>; Florian Fainelli
> <f.fainelli@gmail.com>; Alexandre Belloni <alexandre.belloni@bootlin.com>;
> Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins
> 
> On Fri, Mar 20, 2020 at 06:37:26PM +0800, Yangbo Lu wrote:
> > +static int ocelot_ptp_enable(struct ptp_clock_info *ptp,
> > +			     struct ptp_clock_request *rq, int on)
> > +{
> > +	struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> > +	enum ocelot_ptp_pins ptp_pin;
> > +	struct timespec64 ts;
> > +	unsigned long flags;
> > +	int pin = -1;
> > +	u32 val;
> > +	s64 ns;
> > +
> > +	switch (rq->type) {
> > +	case PTP_CLK_REQ_PEROUT:
> > +		/* Reject requests with unsupported flags */
> > +		if (rq->perout.flags)
> > +			return -EOPNOTSUPP;
> > +
> > +		/*
> > +		 * TODO: support disabling function
> > +		 * When ptp_disable_pinfunc() is to disable function,
> > +		 * it has already held pincfg_mux.
> > +		 * However ptp_find_pin() in .enable() called also needs
> > +		 * to hold pincfg_mux.
> > +		 * This causes dead lock. So, just return for function
> > +		 * disabling, and this needs fix-up.
> 
> What dead lock?
> 
> When enable(PTP_CLK_REQ_PEROUT, on=0) is called, you don't need to
> call ptp_disable_pinfunc().  Just stop the periodic waveform
> generator.  The assignment of function to pin remains unchanged.

This happens when we try to change pin function through ptp_ioctl PTP_PIN_SETFUNC.
When software holds pincfg_mux and calls ptp_set_pinfunc, it will disable the function previous assigned and the current function of current pin calling ptp_disable_pinfunc.
The problem is the enable callback in ptp_disable_pinfunc may have to hold pincfg_mux for ptp_find_pin.

The calling should be like this,
ptp_set_pinfunc (hold pincfg_mux)
---> ptp_disable_pinfunc
   ---> .enable
      ---> ptp_find_pin (hold pincfg_mux)

Thanks.

> 
> > +		 */
> > +		if (!on)
> > +			break;
> > +
> > +		pin = ptp_find_pin(ocelot->ptp_clock, PTP_PF_PEROUT,
> > +				   rq->perout.index);
> > +		if (pin == 0)
> > +			ptp_pin = PTP_PIN_0;
> > +		else if (pin == 1)
> > +			ptp_pin = PTP_PIN_1;
> > +		else if (pin == 2)
> > +			ptp_pin = PTP_PIN_2;
> > +		else if (pin == 3)
> > +			ptp_pin = PTP_PIN_3;
> > +		else
> > +			return -EINVAL;
> 
> Return -EBUSY here instead.

Thanks. Will modify it in next version.

> 
> Thanks,
> Richard

Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Richard Cochran]

On Wed, Mar 25, 2020 at 03:08:46AM +0000, Y.b. Lu wrote:

> The calling should be like this,
> ptp_set_pinfunc (hold pincfg_mux)
> ---> ptp_disable_pinfunc
>    ---> .enable
>       ---> ptp_find_pin (hold pincfg_mux)

I see.  The call

    ptp_disable_pinfunc() --> .enable()

is really

    ptp_disable_pinfunc() --> .enable(on=0)

or disable.

All of the other drivers (except mv88e6xxx which has a bug) avoid the
deadlock by only calling ptp_find_pin() when invoked by .enable(on=1);

Of course, that is horrible, and I am going to find a way to fix it.

For now, maybe you can drop the "programmable pins" feature for your
driver?  After all, the pins are not programmable.

Thanks,
Richard

RE: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Y.b. Lu]

Hi Richard,

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Wednesday, March 25, 2020 9:42 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; David S . Miller
> <davem@davemloft.net>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Claudiu Manoil <claudiu.manoil@nxp.com>; Andrew Lunn <andrew@lunn.ch>;
> Vivien Didelot <vivien.didelot@gmail.com>; Florian Fainelli
> <f.fainelli@gmail.com>; Alexandre Belloni <alexandre.belloni@bootlin.com>;
> Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins
> 
> On Wed, Mar 25, 2020 at 03:08:46AM +0000, Y.b. Lu wrote:
> 
> > The calling should be like this,
> > ptp_set_pinfunc (hold pincfg_mux)
> > ---> ptp_disable_pinfunc
> >    ---> .enable
> >       ---> ptp_find_pin (hold pincfg_mux)
> 
> I see.  The call
> 
>     ptp_disable_pinfunc() --> .enable()
> 
> is really
> 
>     ptp_disable_pinfunc() --> .enable(on=0)
> 
> or disable.
> 
> All of the other drivers (except mv88e6xxx which has a bug) avoid the
> deadlock by only calling ptp_find_pin() when invoked by .enable(on=1);
> 
> Of course, that is horrible, and I am going to find a way to fix it.

Thanks a lot.
Do you think it is ok to move protection into ptp_set_pinfunc() to protect just pin_config accessing?
ptp_disable_pinfunc() not touching pin_config could be out of protection.
But it seems indeed total ptp_set_pinfunc() should be under protection...

> 
> For now, maybe you can drop the "programmable pins" feature for your
> driver?  After all, the pins are not programmable.

I still want to confirm, did you mean the deadlock issue? Or you thought the pin supports only PTP_PF_PEROUT in hardware?
I could modify commit messages to indicate the pin supports both PTP_PF_PEROUT and PTP_PF_EXTTS, and PTP_PF_EXTTS support will be added in the future.
Thanks a lot.

> 
> Thanks,
> Richard

Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Richard Cochran]

On Thu, Mar 26, 2020 at 09:34:52AM +0000, Y.b. Lu wrote:
> > Of course, that is horrible, and I am going to find a way to fix it.
> 
> Thanks a lot.
> Do you think it is ok to move protection into ptp_set_pinfunc() to protect just pin_config accessing?
> ptp_disable_pinfunc() not touching pin_config could be out of protection.
> But it seems indeed total ptp_set_pinfunc() should be under protection...

Yes, and I have way to fix that.  I will post a patch soon...

> I could modify commit messages to indicate the pin supports both PTP_PF_PEROUT and PTP_PF_EXTTS, and PTP_PF_EXTTS support will be added in the future.

Thanks for explaining.  Since you do have programmable pin, please
wait for my patch to fix the deadlock.

Thanks,
Richard

RE: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Y.b. Lu]

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Thursday, March 26, 2020 10:00 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; David S . Miller
> <davem@davemloft.net>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Claudiu Manoil <claudiu.manoil@nxp.com>; Andrew Lunn <andrew@lunn.ch>;
> Vivien Didelot <vivien.didelot@gmail.com>; Florian Fainelli
> <f.fainelli@gmail.com>; Alexandre Belloni <alexandre.belloni@bootlin.com>;
> Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins
> 
> On Thu, Mar 26, 2020 at 09:34:52AM +0000, Y.b. Lu wrote:
> > > Of course, that is horrible, and I am going to find a way to fix it.
> >
> > Thanks a lot.
> > Do you think it is ok to move protection into ptp_set_pinfunc() to protect
> just pin_config accessing?
> > ptp_disable_pinfunc() not touching pin_config could be out of protection.
> > But it seems indeed total ptp_set_pinfunc() should be under protection...
> 
> Yes, and I have way to fix that.  I will post a patch soon...
> 
> > I could modify commit messages to indicate the pin supports both
> PTP_PF_PEROUT and PTP_PF_EXTTS, and PTP_PF_EXTTS support will be added
> in the future.
> 
> Thanks for explaining.  Since you do have programmable pin, please
> wait for my patch to fix the deadlock.

Thanks a lot. Will wait your fix-up.

Best regards,
Yangbo Lu

> 
> Thanks,
> Richard

RE: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Y.b. Lu]

> -----Original Message-----
> From: Y.b. Lu
> Sent: Friday, March 27, 2020 1:48 PM
> To: Richard Cochran <richardcochran@gmail.com>
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; David S . Miller
> <davem@davemloft.net>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Claudiu Manoil <claudiu.manoil@nxp.com>; Andrew Lunn <andrew@lunn.ch>;
> Vivien Didelot <vivien.didelot@gmail.com>; Florian Fainelli
> <f.fainelli@gmail.com>; Alexandre Belloni <alexandre.belloni@bootlin.com>;
> Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Subject: RE: [PATCH 6/6] ptp_ocelot: support 4 programmable pins
> 
> > -----Original Message-----
> > From: Richard Cochran <richardcochran@gmail.com>
> > Sent: Thursday, March 26, 2020 10:00 PM
> > To: Y.b. Lu <yangbo.lu@nxp.com>
> > Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; David S . Miller
> > <davem@davemloft.net>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> > Claudiu Manoil <claudiu.manoil@nxp.com>; Andrew Lunn
> <andrew@lunn.ch>;
> > Vivien Didelot <vivien.didelot@gmail.com>; Florian Fainelli
> > <f.fainelli@gmail.com>; Alexandre Belloni <alexandre.belloni@bootlin.com>;
> > Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> > Subject: Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins
> >
> > On Thu, Mar 26, 2020 at 09:34:52AM +0000, Y.b. Lu wrote:
> > > > Of course, that is horrible, and I am going to find a way to fix it.
> > >
> > > Thanks a lot.
> > > Do you think it is ok to move protection into ptp_set_pinfunc() to protect
> > just pin_config accessing?
> > > ptp_disable_pinfunc() not touching pin_config could be out of protection.
> > > But it seems indeed total ptp_set_pinfunc() should be under protection...
> >
> > Yes, and I have way to fix that.  I will post a patch soon...
> >
> > > I could modify commit messages to indicate the pin supports both
> > PTP_PF_PEROUT and PTP_PF_EXTTS, and PTP_PF_EXTTS support will be
> added
> > in the future.
> >
> > Thanks for explaining.  Since you do have programmable pin, please
> > wait for my patch to fix the deadlock.
> 
> Thanks a lot. Will wait your fix-up.

I see the fix-up was merged. Thanks Richard.
62582a7 ptp: Avoid deadlocks in the programmable pin code.

I just sent out v2 patch-set based on that:)

> 
> Best regards,
> Yangbo Lu
> 
> >
> > Thanks,
> > Richard

Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Richard Cochran]

On Fri, Mar 20, 2020 at 06:37:26PM +0800, Yangbo Lu wrote:
> Support 4 programmable pins for only one function periodic
> signal for now.

For now?

> +static int ocelot_ptp_verify(struct ptp_clock_info *ptp, unsigned int pin,
> +			     enum ptp_pin_function func, unsigned int chan)
> +{
> +	switch (func) {
> +	case PTP_PF_NONE:
> +	case PTP_PF_PEROUT:
> +		break;

If the functions cannot be changed, then supporting the
PTP_PIN_SETFUNC ioctl does not make sense!

> +	case PTP_PF_EXTTS:
> +	case PTP_PF_PHYSYNC:
> +		return -1;
> +	}
> +	return 0;
> +}

Thanks,
Richard

RE: [PATCH 6/6] ptp_ocelot: support 4 programmable pins

[Y.b. Lu]

Hi Richard,

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Wednesday, March 25, 2020 9:16 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; David S . Miller
> <davem@davemloft.net>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Claudiu Manoil <claudiu.manoil@nxp.com>; Andrew Lunn <andrew@lunn.ch>;
> Vivien Didelot <vivien.didelot@gmail.com>; Florian Fainelli
> <f.fainelli@gmail.com>; Alexandre Belloni <alexandre.belloni@bootlin.com>;
> Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH 6/6] ptp_ocelot: support 4 programmable pins
> 
> On Fri, Mar 20, 2020 at 06:37:26PM +0800, Yangbo Lu wrote:
> > Support 4 programmable pins for only one function periodic
> > signal for now.
> 
> For now?

Yes. The pin on Ocelot/Felix supports both PTP_PF_PEROUT and PTP_PF_EXTTS functions.
But the PTP_PF_EXTTS function should be implemented separately in Ocelot and Felix since hardware interrupt implementation is different on them.
I am responsible for Felix. However I am facing some issue on PTP_PF_EXTTS function on hardware. It may take a long time to discuss internally.

Thanks.

> 
> > +static int ocelot_ptp_verify(struct ptp_clock_info *ptp, unsigned int pin,
> > +			     enum ptp_pin_function func, unsigned int chan)
> > +{
> > +	switch (func) {
> > +	case PTP_PF_NONE:
> > +	case PTP_PF_PEROUT:
> > +		break;
> 
> If the functions cannot be changed, then supporting the
> PTP_PIN_SETFUNC ioctl does not make sense!

Did you mean the dead lock issue? Or you thought the pin supported only PTP_PF_PEROUT function in hardware?

> 
> > +	case PTP_PF_EXTTS:
> > +	case PTP_PF_PHYSYNC:
> > +		return -1;
> > +	}
> > +	return 0;
> > +}
> 
> Thanks,
> Richard