[PATCH net-next 0/3] Unlock new potential in SJA1105 with PTP system timestamping

[PATCH net-next 0/3] Unlock new potential in SJA1105 with PTP system timestamping

[Vladimir Oltean]

The SJA1105 being an automotive switch means it is designed to live in a
set-and-forget environment, far from the configure-at-runtime nature of
Linux. Frequently resetting the switch to change its static config means
it loses track of its PTP time, which is not good.

This patch series implements PTP system timestamping for this switch
(using the API introduced for SPI here:
https://www.mail-archive.com/netdev@vger.kernel.org/msg316725.html),
adding the following benefits to the driver:
- When under control of a user space PTP servo loop (ptp4l, phc2sys),
  the loss of sync during a switch reset is much more manageable, and
  the switch still remains in the s2 (locked servo) state.
- When synchronizing the switch using the software technique (based on
  reading clock A and writing the value to clock B, as opposed to
  relying on hardware timestamping), e.g. by using phc2sys, the sync
  accuracy is vastly improved due to the fact that the actual switch PTP
  time can now be more precisely correlated with something of better
  precision (CLOCK_REALTIME). The issue is that SPI transfers are
  inherently bad for measuring time with low jitter, but the newly
  introduced API aims to alleviate that issue somewhat.

This series is also a requirement for a future patch set that adds full
time-aware scheduling offload support for the switch.

Vladimir Oltean (3):
  net: dsa: sja1105: Implement the .gettimex64 system call for PTP
  net: dsa: sja1105: Restore PTP time after switch reset
  net: dsa: sja1105: Disallow management xmit during switch reset

 drivers/net/dsa/sja1105/sja1105.h      |   6 +-
 drivers/net/dsa/sja1105/sja1105_main.c |  42 +++++++++-
 drivers/net/dsa/sja1105/sja1105_ptp.c  | 103 ++++++++++++++++++-------
 drivers/net/dsa/sja1105/sja1105_ptp.h  |  24 +++++-
 drivers/net/dsa/sja1105/sja1105_spi.c  |  54 +++++++++----
 5 files changed, 177 insertions(+), 52 deletions(-)

-- 
2.17.1

[PATCH net-next 1/3] net: dsa: sja1105: Implement the .gettimex64 system call for PTP

[Vladimir Oltean]

Through the PTP_SYS_OFFSET_EXTENDED ioctl, it is possible for userspace
applications (i.e. phc2sys) to compensate for the delays incurred while
reading the PHC's time.

The task itself of taking the software timestamp is delegated to the SPI
subsystem, through the newly introduced API in struct spi_transfer. The
goal is to cross-timestamp I/O operations on the switch's PTP clock with
values in the local system clock (CLOCK_REALTIME). For that we need to
understand a bit of the hardware internals.

The 'read PTP time' message is a 12 byte structure, first 4 bytes of
which represent the SPI header, and the last 8 bytes represent the
64-bit PTP time. The switch itself starts processing the command
immediately after receiving the last bit of the address, i.e. at the
middle of byte 3 (last byte of header). The PTP time is shadowed to a
buffer register in the switch, and retrieved atomically during the
subsequent SPI frames.

A similar thing goes on for the 'write PTP time' message, although in
that case the switch waits until the 64-bit PTP time becomes fully
available before taking any action. So the byte that needs to be
software-timestamped is byte 11 (last) of the transfer.

The patch creates a common (and local) sja1105_xfer implementation for
the SPI I/O, and offers 3 front-ends:

- sja1105_xfer_u32 and sja1105_xfer_u64: these are capable of optionally
  requesting a PTP timestamp

- sja1105_xfer_buf: this is for large transfers (e.g. the static config
  buffer) and other misc data, and there is no point in giving
  timestamping capabilities to this.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/sja1105/sja1105.h      |  6 ++--
 drivers/net/dsa/sja1105/sja1105_main.c |  3 +-
 drivers/net/dsa/sja1105/sja1105_ptp.c  | 26 ++++++++------
 drivers/net/dsa/sja1105/sja1105_spi.c  | 48 +++++++++++++++++++++-----
 4 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index 91063ed3ef1b..64b3ee7b9771 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -122,9 +122,11 @@ int sja1105_xfer_buf(const struct sja1105_private *priv,
 		     sja1105_spi_rw_mode_t rw, u64 reg_addr,
 		     u8 *buf, size_t len);
 int sja1105_xfer_u32(const struct sja1105_private *priv,
-		     sja1105_spi_rw_mode_t rw, u64 reg_addr, u32 *value);
+		     sja1105_spi_rw_mode_t rw, u64 reg_addr, u32 *value,
+		     struct ptp_system_timestamp *ptp_sts);
 int sja1105_xfer_u64(const struct sja1105_private *priv,
-		     sja1105_spi_rw_mode_t rw, u64 reg_addr, u64 *value);
+		     sja1105_spi_rw_mode_t rw, u64 reg_addr, u64 *value,
+		     struct ptp_system_timestamp *ptp_sts);
 int sja1105_static_config_upload(struct sja1105_private *priv);
 int sja1105_inhibit_tx(const struct sja1105_private *priv,
 		       unsigned long port_bitmap, bool tx_inhibited);
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index d5dfda335aa1..d545edbbef9e 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1973,7 +1973,8 @@ static int sja1105_check_device_id(struct sja1105_private *priv)
 	u64 part_no;
 	int rc;
 
-	rc = sja1105_xfer_u32(priv, SPI_READ, regs->device_id, &device_id);
+	rc = sja1105_xfer_u32(priv, SPI_READ, regs->device_id, &device_id,
+			      NULL);
 	if (rc < 0)
 		return rc;
 
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index 783100397f8a..fac72af24baf 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2019, Vladimir Oltean <olteanv@gmail.com>
  */
+#include <linux/spi/spi.h>
 #include "sja1105.h"
 
 /* The adjfine API clamps ppb between [-32,768,000, 32,768,000], and
@@ -335,11 +336,13 @@ static int sja1105_ptpegr_ts_poll(struct dsa_switch *ds, int port, u64 *ts)
 }
 
 /* Caller must hold ptp_data->lock */
-static int sja1105_ptpclkval_read(struct sja1105_private *priv, u64 *ticks)
+static int sja1105_ptpclkval_read(struct sja1105_private *priv, u64 *ticks,
+				  struct ptp_system_timestamp *ptp_sts)
 {
 	const struct sja1105_regs *regs = priv->info->regs;
 
-	return sja1105_xfer_u64(priv, SPI_READ, regs->ptpclkval, ticks);
+	return sja1105_xfer_u64(priv, SPI_READ, regs->ptpclkval, ticks,
+				ptp_sts);
 }
 
 /* Caller must hold ptp_data->lock */
@@ -347,7 +350,8 @@ static int sja1105_ptpclkval_write(struct sja1105_private *priv, u64 ticks)
 {
 	const struct sja1105_regs *regs = priv->info->regs;
 
-	return sja1105_xfer_u64(priv, SPI_WRITE, regs->ptpclkval, &ticks);
+	return sja1105_xfer_u64(priv, SPI_WRITE, regs->ptpclkval, &ticks,
+				NULL);
 }
 
 #define rxtstamp_to_tagger(d) \
@@ -370,7 +374,7 @@ static void sja1105_rxtstamp_work(struct work_struct *work)
 		u64 ticks, ts;
 		int rc;
 
-		rc = sja1105_ptpclkval_read(priv, &ticks);
+		rc = sja1105_ptpclkval_read(priv, &ticks, NULL);
 		if (rc < 0) {
 			dev_err(ds->dev, "Failed to read PTP clock: %d\n", rc);
 			kfree_skb(skb);
@@ -441,8 +445,9 @@ int sja1105_ptp_reset(struct dsa_switch *ds)
 	return rc;
 }
 
-static int sja1105_ptp_gettime(struct ptp_clock_info *ptp,
-			       struct timespec64 *ts)
+static int sja1105_ptp_gettimex(struct ptp_clock_info *ptp,
+				struct timespec64 *ts,
+				struct ptp_system_timestamp *ptp_sts)
 {
 	struct sja1105_ptp_data *ptp_data = ptp_caps_to_data(ptp);
 	struct sja1105_private *priv = ptp_data_to_sja1105(ptp_data);
@@ -451,7 +456,7 @@ static int sja1105_ptp_gettime(struct ptp_clock_info *ptp,
 
 	mutex_lock(&ptp_data->lock);
 
-	rc = sja1105_ptpclkval_read(priv, &ticks);
+	rc = sja1105_ptpclkval_read(priv, &ticks, ptp_sts);
 	*ts = ns_to_timespec64(sja1105_ticks_to_ns(ticks));
 
 	mutex_unlock(&ptp_data->lock);
@@ -516,7 +521,8 @@ static int sja1105_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 
 	mutex_lock(&ptp_data->lock);
 
-	rc = sja1105_xfer_u32(priv, SPI_WRITE, regs->ptpclkrate, &clkrate32);
+	rc = sja1105_xfer_u32(priv, SPI_WRITE, regs->ptpclkrate, &clkrate32,
+			      NULL);
 
 	mutex_unlock(&ptp_data->lock);
 
@@ -558,7 +564,7 @@ int sja1105_ptp_clock_register(struct dsa_switch *ds)
 		.name		= "SJA1105 PHC",
 		.adjfine	= sja1105_ptp_adjfine,
 		.adjtime	= sja1105_ptp_adjtime,
-		.gettime64	= sja1105_ptp_gettime,
+		.gettimex64	= sja1105_ptp_gettimex,
 		.settime64	= sja1105_ptp_settime,
 		.max_adj	= SJA1105_MAX_ADJ_PPB,
 	};
@@ -604,7 +610,7 @@ void sja1105_ptp_txtstamp_skb(struct dsa_switch *ds, int slot,
 
 	mutex_lock(&ptp_data->lock);
 
-	rc = sja1105_ptpclkval_read(priv, &ticks);
+	rc = sja1105_ptpclkval_read(priv, &ticks, NULL);
 	if (rc < 0) {
 		dev_err(ds->dev, "Failed to read PTP clock: %d\n", rc);
 		kfree_skb(skb);
diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
index ed02410a9366..691cd250e50a 100644
--- a/drivers/net/dsa/sja1105/sja1105_spi.c
+++ b/drivers/net/dsa/sja1105/sja1105_spi.c
@@ -42,9 +42,9 @@ sja1105_spi_message_pack(void *buf, const struct sja1105_spi_message *msg)
  * - SPI_READ:  creates and sends an SPI read message from absolute
  *		address reg_addr, writing @len bytes into *buf
  */
-int sja1105_xfer_buf(const struct sja1105_private *priv,
-		     sja1105_spi_rw_mode_t rw, u64 reg_addr,
-		     u8 *buf, size_t len)
+static int sja1105_xfer(const struct sja1105_private *priv,
+			sja1105_spi_rw_mode_t rw, u64 reg_addr, u8 *buf,
+			size_t len, struct ptp_system_timestamp *ptp_sts)
 {
 	struct sja1105_chunk chunk = {
 		.len = min_t(size_t, len, SJA1105_SIZE_SPI_MSG_MAXLEN),
@@ -81,6 +81,7 @@ int sja1105_xfer_buf(const struct sja1105_private *priv,
 		struct spi_transfer *chunk_xfer = sja1105_chunk_xfer(xfers, i);
 		struct spi_transfer *hdr_xfer = sja1105_hdr_xfer(xfers, i);
 		u8 *hdr_buf = sja1105_hdr_buf(hdr_bufs, i);
+		struct spi_transfer *ptp_sts_xfer;
 		struct sja1105_spi_message msg;
 
 		/* Populate the transfer's header buffer */
@@ -102,6 +103,26 @@ int sja1105_xfer_buf(const struct sja1105_private *priv,
 			chunk_xfer->tx_buf = chunk.buf;
 		chunk_xfer->len = chunk.len;
 
+		/* Request timestamping for the transfer. Instead of letting
+		 * callers specify which byte they want to timestamp, we can
+		 * make certain assumptions:
+		 * - A read operation will request a software timestamp when
+		 *   what's being read is the PTP time. That is snapshotted by
+		 *   the switch hardware at the end of the command portion
+		 *   (hdr_xfer).
+		 * - A write operation will request a software timestamp on
+		 *   actions that modify the PTP time. Taking clock stepping as
+		 *   an example, the switch writes the PTP time at the end of
+		 *   the data portion (chunk_xfer).
+		 */
+		if (rw == SPI_READ)
+			ptp_sts_xfer = hdr_xfer;
+		else
+			ptp_sts_xfer = chunk_xfer;
+		ptp_sts_xfer->ptp_sts_word_pre = ptp_sts_xfer->len - 1;
+		ptp_sts_xfer->ptp_sts_word_post = ptp_sts_xfer->len - 1;
+		ptp_sts_xfer->ptp_sts = ptp_sts;
+
 		/* Calculate next chunk */
 		chunk.buf += chunk.len;
 		chunk.reg_addr += chunk.len / 4;
@@ -123,6 +144,13 @@ int sja1105_xfer_buf(const struct sja1105_private *priv,
 	return rc;
 }
 
+int sja1105_xfer_buf(const struct sja1105_private *priv,
+		     sja1105_spi_rw_mode_t rw, u64 reg_addr,
+		     u8 *buf, size_t len)
+{
+	return sja1105_xfer(priv, rw, reg_addr, buf, len, NULL);
+}
+
 /* If @rw is:
  * - SPI_WRITE: creates and sends an SPI write message at absolute
  *		address reg_addr
@@ -133,7 +161,8 @@ int sja1105_xfer_buf(const struct sja1105_private *priv,
  * CPU endianness and directly usable by software running on the core.
  */
 int sja1105_xfer_u64(const struct sja1105_private *priv,
-		     sja1105_spi_rw_mode_t rw, u64 reg_addr, u64 *value)
+		     sja1105_spi_rw_mode_t rw, u64 reg_addr, u64 *value,
+		     struct ptp_system_timestamp *ptp_sts)
 {
 	u8 packed_buf[8];
 	int rc;
@@ -141,7 +170,7 @@ int sja1105_xfer_u64(const struct sja1105_private *priv,
 	if (rw == SPI_WRITE)
 		sja1105_pack(packed_buf, value, 63, 0, 8);
 
-	rc = sja1105_xfer_buf(priv, rw, reg_addr, packed_buf, 8);
+	rc = sja1105_xfer(priv, rw, reg_addr, packed_buf, 8, ptp_sts);
 
 	if (rw == SPI_READ)
 		sja1105_unpack(packed_buf, value, 63, 0, 8);
@@ -151,7 +180,8 @@ int sja1105_xfer_u64(const struct sja1105_private *priv,
 
 /* Same as above, but transfers only a 4 byte word */
 int sja1105_xfer_u32(const struct sja1105_private *priv,
-		     sja1105_spi_rw_mode_t rw, u64 reg_addr, u32 *value)
+		     sja1105_spi_rw_mode_t rw, u64 reg_addr, u32 *value,
+		     struct ptp_system_timestamp *ptp_sts)
 {
 	u8 packed_buf[4];
 	u64 tmp;
@@ -165,7 +195,7 @@ int sja1105_xfer_u32(const struct sja1105_private *priv,
 		sja1105_pack(packed_buf, &tmp, 31, 0, 4);
 	}
 
-	rc = sja1105_xfer_buf(priv, rw, reg_addr, packed_buf, 4);
+	rc = sja1105_xfer(priv, rw, reg_addr, packed_buf, 4, ptp_sts);
 
 	if (rw == SPI_READ) {
 		sja1105_unpack(packed_buf, &tmp, 31, 0, 4);
@@ -293,7 +323,7 @@ int sja1105_inhibit_tx(const struct sja1105_private *priv,
 	int rc;
 
 	rc = sja1105_xfer_u32(priv, SPI_READ, regs->port_control,
-			      &inhibit_cmd);
+			      &inhibit_cmd, NULL);
 	if (rc < 0)
 		return rc;
 
@@ -303,7 +333,7 @@ int sja1105_inhibit_tx(const struct sja1105_private *priv,
 		inhibit_cmd &= ~port_bitmap;
 
 	return sja1105_xfer_u32(priv, SPI_WRITE, regs->port_control,
-				&inhibit_cmd);
+				&inhibit_cmd, NULL);
 }
 
 struct sja1105_status {
-- 
2.17.1

[PATCH net-next 2/3] net: dsa: sja1105: Restore PTP time after switch reset

[Vladimir Oltean]

The PTP time of the switch is not preserved when uploading a new static
configuration. Work around this hardware oddity by reading its PTP time
before a static config upload, and restoring it afterwards.

Static config changes are expected to occur at runtime even in scenarios
directly related to PTP, i.e. the Time-Aware Scheduler of the switch is
programmed in this way.

Perhaps the larger implication of this patch is that the PTP .gettimex64
and .settime functions need to be exposed to sja1105_main.c, where the
PTP lock needs to be held during this entire process. So their core
implementation needs to move to some common functions which get exposed
in sja1105_ptp.h.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 35 ++++++++++-
 drivers/net/dsa/sja1105/sja1105_ptp.c  | 81 +++++++++++++++++++-------
 drivers/net/dsa/sja1105/sja1105_ptp.h  | 24 +++++++-
 drivers/net/dsa/sja1105/sja1105_spi.c  |  6 --
 4 files changed, 114 insertions(+), 32 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index d545edbbef9e..2b8919a25392 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1349,9 +1349,15 @@ static void sja1105_bridge_leave(struct dsa_switch *ds, int port,
  */
 int sja1105_static_config_reload(struct sja1105_private *priv)
 {
+	struct ptp_system_timestamp ptp_sts_before;
+	struct ptp_system_timestamp ptp_sts_after;
 	struct sja1105_mac_config_entry *mac;
 	int speed_mbps[SJA1105_NUM_PORTS];
+	struct dsa_switch *ds = priv->ds;
+	s64 t1, t2, t3, t4;
+	s64 t12, t34;
 	int rc, i;
+	s64 now;
 
 	mac = priv->static_config.tables[BLK_IDX_MAC_CONFIG].entries;
 
@@ -1365,10 +1371,37 @@ int sja1105_static_config_reload(struct sja1105_private *priv)
 		mac[i].speed = SJA1105_SPEED_AUTO;
 	}
 
+	/* No PTP operations can run right now */
+	mutex_lock(&priv->ptp_data.lock);
+
+	rc = __sja1105_ptp_gettimex(ds, &now, &ptp_sts_before);
+	if (rc < 0)
+		goto out_unlock_ptp;
+
 	/* Reset switch and send updated static configuration */
 	rc = sja1105_static_config_upload(priv);
 	if (rc < 0)
-		goto out;
+		goto out_unlock_ptp;
+
+	rc = __sja1105_ptp_settime(ds, 0, &ptp_sts_after);
+	if (rc < 0)
+		goto out_unlock_ptp;
+
+	t1 = timespec64_to_ns(&ptp_sts_before.pre_ts);
+	t2 = timespec64_to_ns(&ptp_sts_before.post_ts);
+	t3 = timespec64_to_ns(&ptp_sts_after.pre_ts);
+	t4 = timespec64_to_ns(&ptp_sts_after.post_ts);
+	/* Mid point, corresponds to pre-reset PTPCLKVAL */
+	t12 = t1 + (t2 - t1) / 2;
+	/* Mid point, corresponds to post-reset PTPCLKVAL, aka 0 */
+	t34 = t3 + (t4 - t3) / 2;
+	/* Advance PTPCLKVAL by the time it took since its readout */
+	now += (t34 - t12);
+
+	__sja1105_ptp_adjtime(ds, now);
+
+out_unlock_ptp:
+	mutex_unlock(&priv->ptp_data.lock);
 
 	/* Configure the CGU (PLLs) for MII and RMII PHYs.
 	 * For these interfaces there is no dynamic configuration
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index fac72af24baf..0a35813f9328 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -346,12 +346,13 @@ static int sja1105_ptpclkval_read(struct sja1105_private *priv, u64 *ticks,
 }
 
 /* Caller must hold ptp_data->lock */
-static int sja1105_ptpclkval_write(struct sja1105_private *priv, u64 ticks)
+static int sja1105_ptpclkval_write(struct sja1105_private *priv, u64 ticks,
+				   struct ptp_system_timestamp *ptp_sts)
 {
 	const struct sja1105_regs *regs = priv->info->regs;
 
 	return sja1105_xfer_u64(priv, SPI_WRITE, regs->ptpclkval, &ticks,
-				NULL);
+				ptp_sts);
 }
 
 #define rxtstamp_to_tagger(d) \
@@ -427,7 +428,7 @@ bool sja1105_port_txtstamp(struct dsa_switch *ds, int port,
 	return true;
 }
 
-int sja1105_ptp_reset(struct dsa_switch *ds)
+static int sja1105_ptp_reset(struct dsa_switch *ds)
 {
 	struct sja1105_private *priv = ds->priv;
 	struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
@@ -445,19 +446,38 @@ int sja1105_ptp_reset(struct dsa_switch *ds)
 	return rc;
 }
 
+/* Caller must hold ptp_data->lock */
+int __sja1105_ptp_gettimex(struct dsa_switch *ds, u64 *ns,
+			   struct ptp_system_timestamp *ptp_sts)
+{
+	struct sja1105_private *priv = ds->priv;
+	u64 ticks;
+	int rc;
+
+	rc = sja1105_ptpclkval_read(priv, &ticks, ptp_sts);
+	if (rc < 0) {
+		dev_err(ds->dev, "Failed to read PTP clock: %d\n", rc);
+		return rc;
+	}
+
+	*ns = sja1105_ticks_to_ns(ticks);
+
+	return 0;
+}
+
 static int sja1105_ptp_gettimex(struct ptp_clock_info *ptp,
 				struct timespec64 *ts,
 				struct ptp_system_timestamp *ptp_sts)
 {
 	struct sja1105_ptp_data *ptp_data = ptp_caps_to_data(ptp);
 	struct sja1105_private *priv = ptp_data_to_sja1105(ptp_data);
-	u64 ticks = 0;
+	u64 now = 0;
 	int rc;
 
 	mutex_lock(&ptp_data->lock);
 
-	rc = sja1105_ptpclkval_read(priv, &ticks, ptp_sts);
-	*ts = ns_to_timespec64(sja1105_ticks_to_ns(ticks));
+	rc = __sja1105_ptp_gettimex(priv->ds, &now, ptp_sts);
+	*ts = ns_to_timespec64(now);
 
 	mutex_unlock(&ptp_data->lock);
 
@@ -479,24 +499,34 @@ static int sja1105_ptp_mode_set(struct sja1105_private *priv,
 }
 
 /* Write to PTPCLKVAL while PTPCLKADD is 0 */
+int __sja1105_ptp_settime(struct dsa_switch *ds, u64 ns,
+			  struct ptp_system_timestamp *ptp_sts)
+{
+	struct sja1105_private *priv = ds->priv;
+	u64 ticks = ns_to_sja1105_ticks(ns);
+	int rc;
+
+	rc = sja1105_ptp_mode_set(priv, PTP_SET_MODE);
+	if (rc < 0) {
+		dev_err(priv->ds->dev, "Failed to put PTPCLK in set mode\n");
+		return rc;
+	}
+
+	return sja1105_ptpclkval_write(priv, ticks, ptp_sts);
+}
+
 static int sja1105_ptp_settime(struct ptp_clock_info *ptp,
 			       const struct timespec64 *ts)
 {
 	struct sja1105_ptp_data *ptp_data = ptp_caps_to_data(ptp);
 	struct sja1105_private *priv = ptp_data_to_sja1105(ptp_data);
-	u64 ticks = ns_to_sja1105_ticks(timespec64_to_ns(ts));
+	u64 ns = timespec64_to_ns(ts);
 	int rc;
 
 	mutex_lock(&ptp_data->lock);
 
-	rc = sja1105_ptp_mode_set(priv, PTP_SET_MODE);
-	if (rc < 0) {
-		dev_err(priv->ds->dev, "Failed to put PTPCLK in set mode\n");
-		goto out;
-	}
+	rc = __sja1105_ptp_settime(priv->ds, ns, NULL);
 
-	rc = sja1105_ptpclkval_write(priv, ticks);
-out:
 	mutex_unlock(&ptp_data->lock);
 
 	return rc;
@@ -530,24 +560,31 @@ static int sja1105_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 }
 
 /* Write to PTPCLKVAL while PTPCLKADD is 1 */
-static int sja1105_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+int __sja1105_ptp_adjtime(struct dsa_switch *ds, s64 delta)
 {
-	struct sja1105_ptp_data *ptp_data = ptp_caps_to_data(ptp);
-	struct sja1105_private *priv = ptp_data_to_sja1105(ptp_data);
+	struct sja1105_private *priv = ds->priv;
 	s64 ticks = ns_to_sja1105_ticks(delta);
 	int rc;
 
-	mutex_lock(&ptp_data->lock);
-
 	rc = sja1105_ptp_mode_set(priv, PTP_ADD_MODE);
 	if (rc < 0) {
 		dev_err(priv->ds->dev, "Failed to put PTPCLK in add mode\n");
-		goto out;
+		return rc;
 	}
 
-	rc = sja1105_ptpclkval_write(priv, ticks);
+	return sja1105_ptpclkval_write(priv, ticks, NULL);
+}
+
+static int sja1105_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct sja1105_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct sja1105_private *priv = ptp_data_to_sja1105(ptp_data);
+	int rc;
+
+	mutex_lock(&ptp_data->lock);
+
+	rc = __sja1105_ptp_adjtime(priv->ds, delta);
 
-out:
 	mutex_unlock(&ptp_data->lock);
 
 	return rc;
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.h b/drivers/net/dsa/sja1105/sja1105_ptp.h
index 243f130374d2..19e707db7e8c 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.h
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.h
@@ -51,8 +51,6 @@ int sja1105_get_ts_info(struct dsa_switch *ds, int port,
 void sja1105_ptp_txtstamp_skb(struct dsa_switch *ds, int slot,
 			      struct sk_buff *clone);
 
-int sja1105_ptp_reset(struct dsa_switch *ds);
-
 bool sja1105_port_rxtstamp(struct dsa_switch *ds, int port,
 			   struct sk_buff *skb, unsigned int type);
 
@@ -63,6 +61,14 @@ int sja1105_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr);
 
 int sja1105_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr);
 
+int __sja1105_ptp_gettimex(struct dsa_switch *ds, u64 *ns,
+			   struct ptp_system_timestamp *sts);
+
+int __sja1105_ptp_settime(struct dsa_switch *ds, u64 ns,
+			  struct ptp_system_timestamp *ptp_sts);
+
+int __sja1105_ptp_adjtime(struct dsa_switch *ds, s64 delta);
+
 #else
 
 struct sja1105_ptp_cmd;
@@ -87,7 +93,19 @@ static inline void sja1105_ptp_txtstamp_skb(struct dsa_switch *ds, int slot,
 {
 }
 
-static inline int sja1105_ptp_reset(struct dsa_switch *ds)
+static inline int __sja1105_ptp_gettimex(struct dsa_switch *ds, u64 *ns,
+					 struct ptp_system_timestamp *sts)
+{
+	return 0;
+}
+
+static inline int __sja1105_ptp_settime(struct dsa_switch *ds, u64 ns,
+					struct ptp_system_timestamp *ptp_sts)
+{
+	return 0;
+}
+
+static inline int __sja1105_ptp_adjtime(struct dsa_switch *ds, s64 delta)
 {
 	return 0;
 }
diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
index 691cd250e50a..cb48e77f63fd 100644
--- a/drivers/net/dsa/sja1105/sja1105_spi.c
+++ b/drivers/net/dsa/sja1105/sja1105_spi.c
@@ -511,12 +511,6 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
 		dev_info(dev, "Succeeded after %d tried\n", RETRIES - retries);
 	}
 
-	rc = sja1105_ptp_reset(priv->ds);
-	if (rc < 0)
-		dev_err(dev, "Failed to reset PTP clock: %d\n", rc);
-
-	dev_info(dev, "Reset switch and programmed static config\n");
-
 out:
 	kfree(config_buf);
 	return rc;
-- 
2.17.1

[PATCH net-next 3/3] net: dsa: sja1105: Disallow management xmit during switch reset

[Vladimir Oltean]

The purpose here is to avoid ptp4l fail due to this condition:

  timed out while polling for tx timestamp
  increasing tx_timestamp_timeout may correct this issue, but it is likely caused by a driver bug
  port 1: send peer delay request failed

So either reset the switch before the management frame was sent, or
after it was timestamped as well, but not in the middle.

The condition may arise either due to a true timeout (i.e. because
re-uploading the static config takes time), or due to the TX timestamp
actually getting lost due to reset. For the former we can increase
tx_timestamp_timeout in userspace, for the latter we need this patch.

Locking all traffic during switch reset does not make sense at all,
though. Forcing all CPU-originated traffic to potentially block waiting
for a sleepable context to send > 800 bytes over SPI is not a good idea.
Flows that are autonomously forwarded by the switch will get dropped
anyway during switch reset no matter what. So just let all other
CPU-originated traffic be dropped as well.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 2b8919a25392..475cc2d8b0e8 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1359,6 +1359,8 @@ int sja1105_static_config_reload(struct sja1105_private *priv)
 	int rc, i;
 	s64 now;
 
+	mutex_lock(&priv->mgmt_lock);
+
 	mac = priv->static_config.tables[BLK_IDX_MAC_CONFIG].entries;
 
 	/* Back up the dynamic link speed changed by sja1105_adjust_port_config
@@ -1417,6 +1419,8 @@ int sja1105_static_config_reload(struct sja1105_private *priv)
 			goto out;
 	}
 out:
+	mutex_unlock(&priv->mgmt_lock);
+
 	return rc;
 }
 
-- 
2.17.1

Re: [PATCH net-next 0/3] Unlock new potential in SJA1105 with PTP system timestamping

[Richard Cochran]

On Sat, Nov 09, 2019 at 01:32:21PM +0200, Vladimir Oltean wrote:
> The SJA1105 being an automotive switch means it is designed to live in a
> set-and-forget environment, far from the configure-at-runtime nature of
> Linux. Frequently resetting the switch to change its static config means
> it loses track of its PTP time, which is not good.
> 
> This patch series implements PTP system timestamping for this switch
> (using the API introduced for SPI here:
> https://www.mail-archive.com/netdev@vger.kernel.org/msg316725.html),
> adding the following benefits to the driver:
> - When under control of a user space PTP servo loop (ptp4l, phc2sys),
>   the loss of sync during a switch reset is much more manageable, and
>   the switch still remains in the s2 (locked servo) state.
> - When synchronizing the switch using the software technique (based on
>   reading clock A and writing the value to clock B, as opposed to
>   relying on hardware timestamping), e.g. by using phc2sys, the sync
>   accuracy is vastly improved due to the fact that the actual switch PTP
>   time can now be more precisely correlated with something of better
>   precision (CLOCK_REALTIME). The issue is that SPI transfers are
>   inherently bad for measuring time with low jitter, but the newly
>   introduced API aims to alleviate that issue somewhat.
> 
> This series is also a requirement for a future patch set that adds full
> time-aware scheduling offload support for the switch.
> 
> Vladimir Oltean (3):
>   net: dsa: sja1105: Implement the .gettimex64 system call for PTP
>   net: dsa: sja1105: Restore PTP time after switch reset
>   net: dsa: sja1105: Disallow management xmit during switch reset

For the series:

Acked-by: Richard Cochran <richardcochran@gmail.com>

Re: [PATCH net-next 0/3] Unlock new potential in SJA1105 with PTP system timestamping

[Vladimir Oltean]

On Sat, 9 Nov 2019 at 17:19, Richard Cochran <richardcochran@gmail.com> wrote:
>
> On Sat, Nov 09, 2019 at 01:32:21PM +0200, Vladimir Oltean wrote:
> > The SJA1105 being an automotive switch means it is designed to live in a
> > set-and-forget environment, far from the configure-at-runtime nature of
> > Linux. Frequently resetting the switch to change its static config means
> > it loses track of its PTP time, which is not good.
> >
> > This patch series implements PTP system timestamping for this switch
> > (using the API introduced for SPI here:
> > https://www.mail-archive.com/netdev@vger.kernel.org/msg316725.html),
> > adding the following benefits to the driver:
> > - When under control of a user space PTP servo loop (ptp4l, phc2sys),
> >   the loss of sync during a switch reset is much more manageable, and
> >   the switch still remains in the s2 (locked servo) state.
> > - When synchronizing the switch using the software technique (based on
> >   reading clock A and writing the value to clock B, as opposed to
> >   relying on hardware timestamping), e.g. by using phc2sys, the sync
> >   accuracy is vastly improved due to the fact that the actual switch PTP
> >   time can now be more precisely correlated with something of better
> >   precision (CLOCK_REALTIME). The issue is that SPI transfers are
> >   inherently bad for measuring time with low jitter, but the newly
> >   introduced API aims to alleviate that issue somewhat.
> >
> > This series is also a requirement for a future patch set that adds full
> > time-aware scheduling offload support for the switch.
> >
> > Vladimir Oltean (3):
> >   net: dsa: sja1105: Implement the .gettimex64 system call for PTP
> >   net: dsa: sja1105: Restore PTP time after switch reset
> >   net: dsa: sja1105: Disallow management xmit during switch reset
>
> For the series:
>
> Acked-by: Richard Cochran <richardcochran@gmail.com>

Thanks Richard.

David, I noticed you put these patches in Patchwork in the 'Needs
Review / ACK' state. Do you need more in-depth review, or did you just
miss Richard's tag, since Patchwork ignores them if they're posted on
the cover letter?

[ in fact, I have a similar question for the "Accomodate DSA front-end
into Ocelot" series, which also has 1 R and 1 A on the cover letter ]

Regards,
-Vladimir

Re: [PATCH net-next 0/3] Unlock new potential in SJA1105 with PTP system timestamping

[David Miller]

From: Vladimir Oltean <olteanv@gmail.com>
Date: Mon, 11 Nov 2019 15:19:26 +0200

> David, I noticed you put these patches in Patchwork in the 'Needs
> Review / ACK' state. Do you need more in-depth review, or did you just
> miss Richard's tag, since Patchwork ignores them if they're posted on
> the cover letter?

Nothing needs to happen, I will take care of this series and apply it.

Thanks.

Re: [PATCH net-next 0/3] Unlock new potential in SJA1105 with PTP system timestamping

[David Miller]

From: Vladimir Oltean <olteanv@gmail.com>
Date: Sat,  9 Nov 2019 13:32:21 +0200

> The SJA1105 being an automotive switch means it is designed to live in a
> set-and-forget environment, far from the configure-at-runtime nature of
> Linux. Frequently resetting the switch to change its static config means
> it loses track of its PTP time, which is not good.
> 
> This patch series implements PTP system timestamping for this switch
> (using the API introduced for SPI here:
> https://www.mail-archive.com/netdev@vger.kernel.org/msg316725.html),
> adding the following benefits to the driver:
> - When under control of a user space PTP servo loop (ptp4l, phc2sys),
>   the loss of sync during a switch reset is much more manageable, and
>   the switch still remains in the s2 (locked servo) state.
> - When synchronizing the switch using the software technique (based on
>   reading clock A and writing the value to clock B, as opposed to
>   relying on hardware timestamping), e.g. by using phc2sys, the sync
>   accuracy is vastly improved due to the fact that the actual switch PTP
>   time can now be more precisely correlated with something of better
>   precision (CLOCK_REALTIME). The issue is that SPI transfers are
>   inherently bad for measuring time with low jitter, but the newly
>   introduced API aims to alleviate that issue somewhat.
> 
> This series is also a requirement for a future patch set that adds full
> time-aware scheduling offload support for the switch.

Series applied, thank you.