Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1741648 > unrolled thread

[PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx

Back to article view | Back to linux.kernel

Contents

  [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx Brandon Streiff <brandon.streiff@ni.com> - 2017-09-28 17:40 +0200
    [PATCH net-next RFC 4/9] net: dsa: mv88e6xxx: add support for event capture Brandon Streiff <brandon.streiff@ni.com> - 2017-09-28 17:40 +0200
    [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock Brandon Streiff <brandon.streiff@ni.com> - 2017-09-28 17:40 +0200
      Re: [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time  as a PTP hardware clock Andrew Lunn <andrew@lunn.ch> - 2017-09-28 19:00 +0200
        RE: [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time  as a PTP hardware clock Brandon Streiff <brandon.streiff@ni.com> - 2017-09-29 17:50 +0200
      Re: [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time  as a PTP hardware clock Andrew Lunn <andrew@lunn.ch> - 2017-09-28 19:10 +0200
        RE: [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time  as a PTP hardware clock Brandon Streiff <brandon.streiff@ni.com> - 2017-09-29 17:20 +0200
    Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx Andrew Lunn <andrew@lunn.ch> - 2017-09-28 19:40 +0200
      Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx Florian Fainelli <f.fainelli@gmail.com> - 2017-09-28 20:00 +0200
      RE: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx Brandon Streiff <brandon.streiff@ni.com> - 2017-09-29 17:40 +0200
    Re: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx Richard Cochran <richardcochran@gmail.com> - 2017-09-29 11:50 +0200

#1741648 — [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx

This patch series adds support for PTP timestamping through the DSA
framework, as well as an implementation for mv8e6xxx switches.

This implementation was targeted at a National Instruments platform
that uses the Marvell 88E6341 (Topaz). I've tried to enable support on
other Marvell switches where the register interfaces seemed compatible,
but I don't have the hardware to verify their operation myself.

This series probably ties in well with Richard's comment last week
("Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for
traffic shapers") about figuring out proper interfaces for managing
switch-level PTP timestamps.

A couple patches that I expect may need further polishing:

- Patch #2: We expose the switch time as a PTP clock but don't support
  adjustment (max_adj=0). Our platform adjusted a systemwide oscillator
  from userspace, so we didn't need adjustment at this layer, but other
  PTP clock drivers support this and we probably should too.

- Patch #3: The GPIO config support is handled in a very simple manner.
  I suspect a longer term goal would be to use pinctrl here.

- Patch #6: the dsa_switch pointer and port index is plumbed from
  dsa_device_ops::rcv so that we can call the correct port_rxtstamp
  method. This involved instrumenting all of the *_tag_rcv functions in
  a way that's kind of a kludge and that I'm not terribly happy with.

This applies to net-next as of 14a0d032f4ec.

Feedback is appreciated.

-- brandon


Brandon Streiff (9):
  net: dsa: mv88e6xxx: add accessors for PTP/TAI registers
  net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock
  net: dsa: mv88e6xxx: add support for GPIO configuration
  net: dsa: mv88e6xxx: add support for event capture
  net: dsa: forward hardware timestamping ioctls to switch driver
  net: dsa: forward timestamping callbacks to switch drivers
  ptp: add offset for reserved field to header
  net: dsa: mv88e6xxx: add rx/tx timestamping support
  net: dsa: mv88e6xxx: add workaround for 6341 timestamping

 drivers/net/dsa/mv88e6xxx/Kconfig    |  10 +
 drivers/net/dsa/mv88e6xxx/Makefile   |   2 +
 drivers/net/dsa/mv88e6xxx/chip.c     |  65 +++++
 drivers/net/dsa/mv88e6xxx/chip.h     |  71 +++++
 drivers/net/dsa/mv88e6xxx/global2.c  | 244 ++++++++++++++++
 drivers/net/dsa/mv88e6xxx/global2.h  |  59 +++-
 drivers/net/dsa/mv88e6xxx/hwtstamp.c | 548 +++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/hwtstamp.h | 171 +++++++++++
 drivers/net/dsa/mv88e6xxx/ptp.c      | 493 +++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/ptp.h      |  99 +++++++
 include/linux/ptp_classify.h         |   1 +
 include/net/dsa.h                    |  28 +-
 net/dsa/dsa.c                        |  39 ++-
 net/dsa/slave.c                      |  67 ++++-
 net/dsa/tag_brcm.c                   |   6 +-
 net/dsa/tag_dsa.c                    |   6 +-
 net/dsa/tag_edsa.c                   |   6 +-
 net/dsa/tag_ksz.c                    |   6 +-
 net/dsa/tag_lan9303.c                |   6 +-
 net/dsa/tag_mtk.c                    |   6 +-
 net/dsa/tag_qca.c                    |   6 +-
 net/dsa/tag_trailer.c                |   6 +-
 22 files changed, 1929 insertions(+), 16 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/hwtstamp.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/hwtstamp.h
 create mode 100644 drivers/net/dsa/mv88e6xxx/ptp.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/ptp.h

-- 
2.1.4

[toc] | [next] | [standalone]

#1741649 — [PATCH net-next RFC 4/9] net: dsa: mv88e6xxx: add support for event capture

This patch adds support for configuring mv88e6xxx GPIO lines as PTP
pins, so that they may be used for time stamping external events or for
periodic output.

Signed-off-by: Brandon Streiff <brandon.streiff@ni.com>
---
 drivers/net/dsa/mv88e6xxx/chip.h |   4 +
 drivers/net/dsa/mv88e6xxx/ptp.c  | 317 ++++++++++++++++++++++++++++++++++++++-
 drivers/net/dsa/mv88e6xxx/ptp.h  |  16 ++
 3 files changed, 335 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 5f132e2..b763778 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -227,6 +227,10 @@ struct mv88e6xxx_chip {
 
 	struct ptp_clock	*ptp_clock;
 	struct ptp_clock_info	ptp_clock_info;
+	struct delayed_work	tai_event_work;
+	struct ptp_pin_desc	pin_config[MV88E6XXX_MAX_GPIO];
+	u16 trig_config;
+	u16 evcap_config;
 };
 
 struct mv88e6xxx_bus_ops {
diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
index 155f8e9..1422d85 100644
--- a/drivers/net/dsa/mv88e6xxx/ptp.c
+++ b/drivers/net/dsa/mv88e6xxx/ptp.c
@@ -18,6 +18,8 @@
 #include "global2.h"
 #include "ptp.h"
 
+#define TAI_EVENT_WORK_INTERVAL msecs_to_jiffies(100)
+
 static int mv88e6xxx_tai_read(struct mv88e6xxx_chip *chip, int addr,
 			      u16 *data, int len)
 {
@@ -27,6 +29,14 @@ static int mv88e6xxx_tai_read(struct mv88e6xxx_chip *chip, int addr,
 	return chip->info->ops->avb_ops->tai_read(chip, addr, data, len);
 }
 
+static int mv88e6xxx_tai_write(struct mv88e6xxx_chip *chip, int addr, u16 data)
+{
+	if (!chip->info->ops->avb_ops->tai_write)
+		return -EOPNOTSUPP;
+
+	return chip->info->ops->avb_ops->tai_write(chip, addr, data);
+}
+
 static u64 mv88e6xxx_ptp_clock_read(const struct cyclecounter *cc)
 {
 	struct mv88e6xxx_chip *chip =
@@ -42,6 +52,144 @@ static u64 mv88e6xxx_ptp_clock_read(const struct cyclecounter *cc)
 		return ((u32)phc_time[1] << 16) | phc_time[0];
 }
 
+static int mv88e6xxx_disable_trig(struct mv88e6xxx_chip *chip)
+{
+	int err;
+	u16 global_config;
+
+	chip->trig_config = 0;
+	global_config = (chip->evcap_config | chip->trig_config);
+	err = mv88e6xxx_tai_write(chip, MV88E6XXX_TAI_CFG, global_config);
+
+	return err;
+}
+
+static int mv88e6xxx_config_periodic_trig(struct mv88e6xxx_chip *chip,
+					  u32 ns, u16 picos)
+{
+	int err;
+	u16 global_config;
+
+	if (picos >= 1000)
+		return -ERANGE;
+
+	/* TRIG generation is in units of 8 ns clock periods. Convert ns
+	 * and ps into 8 ns clock periods and up to 8000 additional ps
+	 */
+	picos += (ns & 0x7) * 1000;
+	ns = ns >> 3;
+
+	err = mv88e6xxx_tai_write(chip, MV88E6XXX_TAI_TRIG_GEN_AMOUNT_LO,
+				  ns & 0xffff);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_tai_write(chip, MV88E6XXX_TAI_TRIG_GEN_AMOUNT_HI,
+				  ns >> 16);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_tai_write(chip, MV88E6XXX_TAI_TRIG_CLOCK_COMP,
+				  picos);
+	if (err)
+		return err;
+
+	chip->trig_config = MV88E6XXX_TAI_CFG_TRIG_ENABLE;
+	global_config = (chip->evcap_config | chip->trig_config);
+	err = mv88e6xxx_tai_write(chip, MV88E6XXX_TAI_CFG, global_config);
+
+	return err;
+}
+
+/* mv88e6xxx_config_eventcap - configure TAI event capture
+ * @event: PTP_CLOCK_PPS (internal) or PTP_CLOCK_EXTTS (external)
+ * @rising: zero for falling-edge trigger, else rising-edge trigger
+ *
+ * This will also reset the capture sequence counter.
+ */
+static int mv88e6xxx_config_eventcap(struct mv88e6xxx_chip *chip, int event,
+				     int rising)
+{
+	u16 global_config;
+	u16 cap_config;
+	int err;
+
+	chip->evcap_config = MV88E6XXX_TAI_CFG_CAP_OVERWRITE |
+			     MV88E6XXX_TAI_CFG_CAP_CTR_START;
+	if (!rising)
+		chip->evcap_config |= MV88E6XXX_TAI_CFG_EVREQ_FALLING;
+
+	global_config = (chip->evcap_config | chip->trig_config);
+	err = mv88e6xxx_tai_write(chip, MV88E6XXX_TAI_CFG, global_config);
+	if (err)
+		return err;
+
+	if (event == PTP_CLOCK_PPS) {
+		cap_config = MV88E6XXX_TAI_EVENT_STATUS_CAP_TRIG;
+	} else if (event == PTP_CLOCK_EXTTS) {
+		/* if STATUS_CAP_TRIG is unset we capture PTP_EVREQ events */
+		cap_config = 0;
+	} else {
+		return -EINVAL;
+	}
+
+	/* Write the capture config; this also clears the capture counter */
+	err = mv88e6xxx_tai_write(chip, MV88E6XXX_TAI_EVENT_STATUS,
+				  cap_config);
+
+	return err;
+}
+
+static void mv88e6xxx_tai_event_work(struct work_struct *ugly)
+{
+	struct delayed_work *dw = to_delayed_work(ugly);
+	struct mv88e6xxx_chip *chip =
+		container_of(dw, struct mv88e6xxx_chip, tai_event_work);
+	u16 ev_status[4];
+	int err;
+
+	mutex_lock(&chip->reg_lock);
+
+	err = mv88e6xxx_tai_read(chip, MV88E6XXX_TAI_EVENT_STATUS,
+				 ev_status, ARRAY_SIZE(ev_status));
+	if (err) {
+		mutex_unlock(&chip->reg_lock);
+		return;
+	}
+
+	if (ev_status[0] & MV88E6XXX_TAI_EVENT_STATUS_ERROR)
+		dev_warn(chip->dev, "missed event capture\n");
+
+	if (ev_status[0] & MV88E6XXX_TAI_EVENT_STATUS_VALID) {
+		struct ptp_clock_event ev;
+		u32 raw_ts = ((u32)ev_status[2] << 16) | ev_status[1];
+
+		/* Clear the valid bit so the next timestamp can come in */
+		ev_status[0] &= ~MV88E6XXX_TAI_EVENT_STATUS_VALID;
+		err = mv88e6xxx_tai_write(chip, MV88E6XXX_TAI_EVENT_STATUS,
+					  ev_status[0]);
+
+		if (ev_status[0] & MV88E6XXX_TAI_EVENT_STATUS_CAP_TRIG) {
+			/* TAI is configured to timestamp internal events.
+			 * This will be a PPS event.
+			 */
+			ev.type = PTP_CLOCK_PPS;
+		} else {
+			/* Otherwise this is an external timestamp */
+			ev.type = PTP_CLOCK_EXTTS;
+		}
+		/* We only have one timestamping channel. */
+		ev.index = 0;
+		ev.timestamp = timecounter_cyc2time(&chip->tstamp_tc, raw_ts);
+
+		ptp_clock_event(chip->ptp_clock, &ev);
+	}
+
+	mutex_unlock(&chip->reg_lock);
+
+	schedule_delayed_work(&chip->tai_event_work, TAI_EVENT_WORK_INTERVAL);
+}
+
 static int mv88e6xxx_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 {
 	if (scaled_ppm == 0)
@@ -95,16 +243,163 @@ static int mv88e6xxx_ptp_settime(struct ptp_clock_info *ptp,
 	return 0;
 }
 
+static int mv88e6xxx_ptp_enable_extts(struct mv88e6xxx_chip *chip,
+				      struct ptp_clock_request *rq, int on)
+{
+	int rising = (rq->extts.flags & PTP_RISING_EDGE);
+	int pin;
+	int err;
+
+	pin = ptp_find_pin(chip->ptp_clock, PTP_PF_EXTTS, rq->extts.index);
+
+	if (pin < 0)
+		return -EBUSY;
+
+	mutex_lock(&chip->reg_lock);
+
+	if (on) {
+		err = mv88e6xxx_g2_set_gpio_config(
+			chip, pin, MV88E6XXX_G2_SCRATCH_GPIO_MODE_EVREQ,
+			MV88E6XXX_G2_SCRATCH_GPIO_DIR_IN);
+		if (err)
+			goto out;
+
+		schedule_delayed_work(&chip->tai_event_work,
+				      TAI_EVENT_WORK_INTERVAL);
+
+		err = mv88e6xxx_config_eventcap(chip, PTP_CLOCK_EXTTS,
+						rising);
+	} else {
+		err = mv88e6xxx_g2_set_gpio_config(
+			chip, pin, MV88E6XXX_G2_SCRATCH_GPIO_MODE_GPIO,
+			MV88E6XXX_G2_SCRATCH_GPIO_DIR_IN);
+
+		cancel_delayed_work_sync(&chip->tai_event_work);
+	}
+
+out:
+	mutex_unlock(&chip->reg_lock);
+
+	return err;
+}
+
+static int mv88e6xxx_ptp_enable_perout(struct mv88e6xxx_chip *chip,
+				       struct ptp_clock_request *rq, int on)
+{
+	struct timespec ts;
+	u64 ns;
+	int pin;
+	int err;
+
+	pin = ptp_find_pin(chip->ptp_clock, PTP_PF_PEROUT, rq->extts.index);
+
+	if (pin < 0)
+		return -EBUSY;
+
+	ts.tv_sec = rq->perout.period.sec;
+	ts.tv_nsec = rq->perout.period.nsec;
+	ns = timespec_to_ns(&ts);
+
+	if (ns > U32_MAX)
+		return -ERANGE;
+
+	mutex_lock(&chip->reg_lock);
+
+	err = mv88e6xxx_config_periodic_trig(chip, (u32)ns, 0);
+	if (err)
+		goto out;
+
+	if (on) {
+		err = mv88e6xxx_g2_set_gpio_config(
+			chip, pin, MV88E6XXX_G2_SCRATCH_GPIO_MODE_TRIG,
+			MV88E6XXX_G2_SCRATCH_GPIO_DIR_OUT);
+	} else {
+		err = mv88e6xxx_g2_set_gpio_config(
+			chip, pin, MV88E6XXX_G2_SCRATCH_GPIO_MODE_GPIO,
+			MV88E6XXX_G2_SCRATCH_GPIO_DIR_IN);
+	}
+
+out:
+	mutex_unlock(&chip->reg_lock);
+
+	return err;
+}
+
+static int mv88e6xxx_ptp_enable_pps(struct mv88e6xxx_chip *chip,
+				    struct ptp_clock_request *rq, int on)
+{
+	int pin;
+	int err;
+
+	pin = ptp_find_pin(chip->ptp_clock, PTP_PF_PEROUT, rq->extts.index);
+
+	if (pin < 0)
+		return -EBUSY;
+
+	mutex_lock(&chip->reg_lock);
+
+	if (on) {
+		err = mv88e6xxx_g2_set_gpio_config(
+			chip, pin, MV88E6XXX_G2_SCRATCH_GPIO_MODE_TRIG,
+			MV88E6XXX_G2_SCRATCH_GPIO_DIR_OUT);
+		if (err)
+			goto out;
+		err = mv88e6xxx_config_periodic_trig(chip,
+						     NSEC_PER_SEC, 0);
+		if (err)
+			goto out;
+
+		schedule_delayed_work(&chip->tai_event_work, 0);
+
+		err = mv88e6xxx_config_eventcap(chip, PTP_CLOCK_PPS, 1);
+	} else {
+		err = mv88e6xxx_g2_set_gpio_config(
+			chip, pin, MV88E6XXX_G2_SCRATCH_GPIO_MODE_GPIO,
+			MV88E6XXX_G2_SCRATCH_GPIO_DIR_IN);
+		if (err)
+			goto out;
+
+		err = mv88e6xxx_disable_trig(chip);
+
+		cancel_delayed_work_sync(&chip->tai_event_work);
+	}
+
+out:
+	mutex_unlock(&chip->reg_lock);
+
+	return err;
+}
+
 static int mv88e6xxx_ptp_enable(struct ptp_clock_info *ptp,
 				struct ptp_clock_request *rq, int on)
 {
-	return -EOPNOTSUPP;
+	struct mv88e6xxx_chip *chip =
+		container_of(ptp, struct mv88e6xxx_chip, ptp_clock_info);
+
+	switch (rq->type) {
+	case PTP_CLK_REQ_EXTTS:
+		return mv88e6xxx_ptp_enable_extts(chip, rq, on);
+	case PTP_CLK_REQ_PEROUT:
+		return mv88e6xxx_ptp_enable_perout(chip, rq, on);
+	case PTP_CLK_REQ_PPS:
+		return mv88e6xxx_ptp_enable_pps(chip, rq, on);
+	default:
+		return -EOPNOTSUPP;
+	}
 }
 
 static int mv88e6xxx_ptp_verify(struct ptp_clock_info *ptp, unsigned int pin,
 				enum ptp_pin_function func, unsigned int chan)
 {
-	return -EOPNOTSUPP;
+	switch (func) {
+	case PTP_PF_NONE:
+	case PTP_PF_EXTTS:
+	case PTP_PF_PEROUT:
+		break;
+	case PTP_PF_PHYSYNC:
+		return -EOPNOTSUPP;
+	}
+	return 0;
 }
 
 /* The 32-bit timestamp counter overflows every ~34.3 seconds; this task
@@ -132,6 +427,8 @@ static void mv88e6xxx_ptp_overflow_check(struct work_struct *work)
 
 int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip)
 {
+	int i;
+
 	/* Set up the cycle counter */
 	memset(&chip->tstamp_cc, 0, sizeof(chip->tstamp_cc));
 	chip->tstamp_cc.read	= mv88e6xxx_ptp_clock_read;
@@ -146,12 +443,27 @@ int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip)
 	chip->last_overflow_check = jiffies;
 
 	INIT_DELAYED_WORK(&chip->overflow_work, mv88e6xxx_ptp_overflow_check);
+	INIT_DELAYED_WORK(&chip->tai_event_work, mv88e6xxx_tai_event_work);
 
 	chip->ptp_clock_info.owner = THIS_MODULE;
 	snprintf(chip->ptp_clock_info.name, sizeof(chip->ptp_clock_info.name),
 		 dev_name(chip->dev));
 	chip->ptp_clock_info.max_adj	= 0;
 
+	chip->ptp_clock_info.n_ext_ts	= 1;
+	chip->ptp_clock_info.n_per_out	= 1;
+	chip->ptp_clock_info.n_pins	= mv88e6xxx_num_gpio(chip);
+	chip->ptp_clock_info.pps	= 1;
+
+	for (i = 0; i < chip->ptp_clock_info.n_pins; ++i) {
+		struct ptp_pin_desc *ppd = &chip->pin_config[i];
+
+		snprintf(ppd->name, sizeof(ppd->name), "mv88e6xxx_gpio%d", i);
+		ppd->index = i;
+		ppd->func = PTP_PF_NONE;
+	}
+	chip->ptp_clock_info.pin_config = chip->pin_config;
+
 	chip->ptp_clock_info.adjfine	= mv88e6xxx_ptp_adjfine;
 	chip->ptp_clock_info.adjtime	= mv88e6xxx_ptp_adjtime;
 	chip->ptp_clock_info.gettime64	= mv88e6xxx_ptp_gettime;
@@ -173,6 +485,7 @@ void mv88e6xxx_ptp_free(struct mv88e6xxx_chip *chip)
 {
 	if (chip->ptp_clock) {
 		cancel_delayed_work_sync(&chip->overflow_work);
+		cancel_delayed_work_sync(&chip->tai_event_work);
 
 		ptp_clock_unregister(chip->ptp_clock);
 		chip->ptp_clock = NULL;
diff --git a/drivers/net/dsa/mv88e6xxx/ptp.h b/drivers/net/dsa/mv88e6xxx/ptp.h
index c662953..33b1842 100644
--- a/drivers/net/dsa/mv88e6xxx/ptp.h
+++ b/drivers/net/dsa/mv88e6xxx/ptp.h
@@ -21,6 +21,18 @@
 
 /* Offset 0x00: TAI Global Config */
 #define MV88E6XXX_TAI_CFG			0x00
+#define MV88E6XXX_TAI_CFG_CAP_OVERWRITE		0x8000
+#define MV88E6XXX_TAI_CFG_CAP_CTR_START		0x4000
+#define MV88E6XXX_TAI_CFG_EVREQ_FALLING		0x2000
+#define MV88E6XXX_TAI_CFG_TRIG_ACTIVE_LO	0x1000
+#define MV88E6XXX_TAI_CFG_IRL_ENABLE		0x0400
+#define MV88E6XXX_TAI_CFG_TRIG_IRQ_EN		0x0200
+#define MV88E6XXX_TAI_CFG_EVREQ_IRQ_EN		0x0100
+#define MV88E6XXX_TAI_CFG_TRIG_LOCK		0x0080
+#define MV88E6XXX_TAI_CFG_BLOCK_UPDATE		0x0008
+#define MV88E6XXX_TAI_CFG_MULTI_PTP		0x0004
+#define MV88E6XXX_TAI_CFG_TRIG_MODE_ONESHOT	0x0002
+#define MV88E6XXX_TAI_CFG_TRIG_ENABLE		0x0001
 
 /* Offset 0x01: Timestamp Clock Period (ps) */
 #define MV88E6XXX_TAI_CLOCK_PERIOD		0x01
@@ -46,6 +58,10 @@
 
 /* Offset 0x09: Event Status */
 #define MV88E6XXX_TAI_EVENT_STATUS		0x09
+#define MV88E6XXX_TAI_EVENT_STATUS_CAP_TRIG	0x4000
+#define MV88E6XXX_TAI_EVENT_STATUS_ERROR	0x0200
+#define MV88E6XXX_TAI_EVENT_STATUS_VALID	0x0100
+#define MV88E6XXX_TAI_EVENT_STATUS_CTR_MASK	0x00ff
 
 /* Offset 0x0A/0x0B: Event Time */
 #define MV88E6XXX_TAI_EVENT_TIME_LO		0x0a
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1741650 — [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock

This patch adds basic support for exposing the 32-bit timestamp counter
inside the mv88e6xxx switch code as a ptp_clock.

Signed-off-by: Brandon Streiff <brandon.streiff@ni.com>
---
 drivers/net/dsa/mv88e6xxx/Kconfig  |  10 +++
 drivers/net/dsa/mv88e6xxx/Makefile |   1 +
 drivers/net/dsa/mv88e6xxx/chip.c   |  20 +++++
 drivers/net/dsa/mv88e6xxx/chip.h   |  16 ++++
 drivers/net/dsa/mv88e6xxx/ptp.c    | 180 +++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/ptp.h    |  83 +++++++++++++++++
 6 files changed, 310 insertions(+)
 create mode 100644 drivers/net/dsa/mv88e6xxx/ptp.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/ptp.h

diff --git a/drivers/net/dsa/mv88e6xxx/Kconfig b/drivers/net/dsa/mv88e6xxx/Kconfig
index 1aaa7a9..ae9e7f7 100644
--- a/drivers/net/dsa/mv88e6xxx/Kconfig
+++ b/drivers/net/dsa/mv88e6xxx/Kconfig
@@ -18,3 +18,13 @@ config NET_DSA_MV88E6XXX_GLOBAL2
 
 	  It is required on most chips. If the chip you compile the support for
 	  doesn't have such registers set, say N here. In doubt, say Y.
+
+config NET_DSA_MV88E6XXX_PTP
+	bool "PTP support for Marvell 88E6xxx"
+	default n
+	depends on NET_DSA_MV88E6XXX_GLOBAL2
+	imply NETWORK_PHY_TIMESTAMPING
+	imply PTP_1588_CLOCK
+	help
+	  Say Y to enable PTP hardware timestamping on Marvell 88E6xxx switch
+	  chips that support it.
diff --git a/drivers/net/dsa/mv88e6xxx/Makefile b/drivers/net/dsa/mv88e6xxx/Makefile
index 5cd5551..47eda30 100644
--- a/drivers/net/dsa/mv88e6xxx/Makefile
+++ b/drivers/net/dsa/mv88e6xxx/Makefile
@@ -6,4 +6,5 @@ mv88e6xxx-objs += global1_vtu.o
 mv88e6xxx-$(CONFIG_NET_DSA_MV88E6XXX_GLOBAL2) += global2.o
 mv88e6xxx-objs += phy.o
 mv88e6xxx-objs += port.o
+mv88e6xxx-$(CONFIG_NET_DSA_MV88E6XXX_PTP) += ptp.o
 mv88e6xxx-objs += serdes.o
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 67efa7b..2ed37d8 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -38,6 +38,7 @@
 #include "global2.h"
 #include "phy.h"
 #include "port.h"
+#include "ptp.h"
 #include "serdes.h"
 
 static void assert_reg_lock(struct mv88e6xxx_chip *chip)
@@ -2033,6 +2034,13 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	if (err)
 		goto unlock;
 
+	/* Setup PTP Hardware Clock */
+	if (chip->info->ptp_support) {
+		err = mv88e6xxx_ptp_setup(chip);
+		if (err)
+			goto unlock;
+	}
+
 unlock:
 	mutex_unlock(&chip->reg_lock);
 
@@ -3418,6 +3426,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.pvt = true,
 		.multi_chip = true,
 		.tag_protocol = DSA_TAG_PROTO_DSA,
+		.ptp_support = true,
 		.ops = &mv88e6191_ops,
 	},
 
@@ -3438,6 +3447,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.pvt = true,
 		.multi_chip = true,
 		.tag_protocol = DSA_TAG_PROTO_EDSA,
+		.ptp_support = true,
 		.ops = &mv88e6240_ops,
 	},
 
@@ -3458,6 +3468,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.pvt = true,
 		.multi_chip = true,
 		.tag_protocol = DSA_TAG_PROTO_DSA,
+		.ptp_support = true,
 		.ops = &mv88e6290_ops,
 	},
 
@@ -3477,6 +3488,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.pvt = true,
 		.multi_chip = true,
 		.tag_protocol = DSA_TAG_PROTO_EDSA,
+		.ptp_support = true,
 		.ops = &mv88e6320_ops,
 	},
 
@@ -3495,6 +3507,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.atu_move_port_mask = 0xf,
 		.multi_chip = true,
 		.tag_protocol = DSA_TAG_PROTO_EDSA,
+		.ptp_support = true,
 		.ops = &mv88e6321_ops,
 	},
 
@@ -3514,6 +3527,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.pvt = true,
 		.multi_chip = true,
 		.tag_protocol = DSA_TAG_PROTO_EDSA,
+		.ptp_support = true,
 		.ops = &mv88e6341_ops,
 	},
 
@@ -3574,6 +3588,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.pvt = true,
 		.multi_chip = true,
 		.tag_protocol = DSA_TAG_PROTO_EDSA,
+		.ptp_support = true,
 		.ops = &mv88e6352_ops,
 	},
 	[MV88E6390] = {
@@ -3593,6 +3608,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.pvt = true,
 		.multi_chip = true,
 		.tag_protocol = DSA_TAG_PROTO_DSA,
+		.ptp_support = true,
 		.ops = &mv88e6390_ops,
 	},
 	[MV88E6390X] = {
@@ -3612,6 +3628,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.pvt = true,
 		.multi_chip = true,
 		.tag_protocol = DSA_TAG_PROTO_DSA,
+		.ptp_support = true,
 		.ops = &mv88e6390x_ops,
 	},
 };
@@ -3949,6 +3966,9 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
 	struct dsa_switch *ds = dev_get_drvdata(&mdiodev->dev);
 	struct mv88e6xxx_chip *chip = ds->priv;
 
+	if (chip->info->ptp_support)
+		mv88e6xxx_ptp_free(chip);
+
 	mv88e6xxx_phy_destroy(chip);
 	mv88e6xxx_unregister_switch(chip);
 	mv88e6xxx_mdios_unregister(chip);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index d99d592..648bd50 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -16,6 +16,8 @@
 #include <linux/irq.h>
 #include <linux/gpio/consumer.h>
 #include <linux/phy.h>
+#include <linux/ptp_clock_kernel.h>
+#include <linux/timecounter.h>
 #include <net/dsa.h>
 
 #ifndef UINT64_MAX
@@ -126,6 +128,9 @@ struct mv88e6xxx_info {
 	 */
 	u8 atu_move_port_mask;
 	const struct mv88e6xxx_ops *ops;
+
+	/* Supports PTP */
+	bool ptp_support;
 };
 
 struct mv88e6xxx_atu_entry {
@@ -208,6 +213,17 @@ struct mv88e6xxx_chip {
 	int irq;
 	int device_irq;
 	int watchdog_irq;
+
+	/* This cyclecounter abstracts the switch PTP time.
+	 * reg_lock must be held for any operation that read()s.
+	 */
+	struct cyclecounter	tstamp_cc;
+	struct timecounter	tstamp_tc;
+	struct delayed_work	overflow_work;
+	unsigned long		last_overflow_check;
+
+	struct ptp_clock	*ptp_clock;
+	struct ptp_clock_info	ptp_clock_info;
 };
 
 struct mv88e6xxx_bus_ops {
diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
new file mode 100644
index 0000000..155f8e9
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/ptp.c
@@ -0,0 +1,180 @@
+/*
+ * Marvell 88E6xxx Switch PTP support
+ *
+ * Copyright (c) 2008 Marvell Semiconductor
+ *
+ * Copyright (c) 2017 National Instruments
+ *      Erik Hons <erik.hons@ni.com>
+ *      Brandon Streiff <brandon.streiff@ni.com>
+ *      Dane Wagner <dane.wagner@ni.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include "chip.h"
+#include "global2.h"
+#include "ptp.h"
+
+static int mv88e6xxx_tai_read(struct mv88e6xxx_chip *chip, int addr,
+			      u16 *data, int len)
+{
+	if (!chip->info->ops->avb_ops->tai_read)
+		return -EOPNOTSUPP;
+
+	return chip->info->ops->avb_ops->tai_read(chip, addr, data, len);
+}
+
+static u64 mv88e6xxx_ptp_clock_read(const struct cyclecounter *cc)
+{
+	struct mv88e6xxx_chip *chip =
+		container_of(cc, struct mv88e6xxx_chip, tstamp_cc);
+	int err;
+	u16 phc_time[2];
+
+	err = mv88e6xxx_tai_read(chip, MV88E6XXX_TAI_TIME_LO, phc_time,
+				 ARRAY_SIZE(phc_time));
+	if (err)
+		return 0;
+	else
+		return ((u32)phc_time[1] << 16) | phc_time[0];
+}
+
+static int mv88e6xxx_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
+{
+	if (scaled_ppm == 0)
+		return 0;
+
+	return -EOPNOTSUPP;
+}
+
+static int mv88e6xxx_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct mv88e6xxx_chip *chip =
+		container_of(ptp, struct mv88e6xxx_chip, ptp_clock_info);
+
+	mutex_lock(&chip->reg_lock);
+	timecounter_adjtime(&chip->tstamp_tc, delta);
+	mutex_unlock(&chip->reg_lock);
+
+	return 0;
+}
+
+static int mv88e6xxx_ptp_gettime(struct ptp_clock_info *ptp,
+				 struct timespec64 *ts)
+{
+	struct mv88e6xxx_chip *chip =
+		container_of(ptp, struct mv88e6xxx_chip, ptp_clock_info);
+	u64 ns;
+
+	mutex_lock(&chip->reg_lock);
+	ns = timecounter_read(&chip->tstamp_tc);
+	chip->last_overflow_check = jiffies;
+	mutex_unlock(&chip->reg_lock);
+
+	*ts = ns_to_timespec64(ns);
+
+	return 0;
+}
+
+static int mv88e6xxx_ptp_settime(struct ptp_clock_info *ptp,
+				 const struct timespec64 *ts)
+{
+	struct mv88e6xxx_chip *chip =
+		container_of(ptp, struct mv88e6xxx_chip, ptp_clock_info);
+	u64 ns;
+
+	ns = timespec64_to_ns(ts);
+
+	mutex_lock(&chip->reg_lock);
+	timecounter_init(&chip->tstamp_tc, &chip->tstamp_cc, ns);
+	mutex_unlock(&chip->reg_lock);
+
+	return 0;
+}
+
+static int mv88e6xxx_ptp_enable(struct ptp_clock_info *ptp,
+				struct ptp_clock_request *rq, int on)
+{
+	return -EOPNOTSUPP;
+}
+
+static int mv88e6xxx_ptp_verify(struct ptp_clock_info *ptp, unsigned int pin,
+				enum ptp_pin_function func, unsigned int chan)
+{
+	return -EOPNOTSUPP;
+}
+
+/* The 32-bit timestamp counter overflows every ~34.3 seconds; this task
+ * forces periodic reads so that we don't miss any wraparounds.
+ */
+#define MV88E6XXX_TAI_OVERFLOW_PERIOD (34 * HZ / 2)
+static void mv88e6xxx_ptp_overflow_check(struct work_struct *work)
+{
+	struct delayed_work *dw = to_delayed_work(work);
+	struct mv88e6xxx_chip *chip =
+		container_of(dw, struct mv88e6xxx_chip, overflow_work);
+	bool timeout = time_is_before_jiffies(chip->last_overflow_check +
+					      MV88E6XXX_TAI_OVERFLOW_PERIOD);
+
+	if (timeout) {
+		mutex_lock(&chip->reg_lock);
+		timecounter_read(&chip->tstamp_tc);
+		chip->last_overflow_check = jiffies;
+		mutex_unlock(&chip->reg_lock);
+	}
+
+	schedule_delayed_work(&chip->overflow_work,
+			      MV88E6XXX_TAI_OVERFLOW_PERIOD);
+}
+
+int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip)
+{
+	/* Set up the cycle counter */
+	memset(&chip->tstamp_cc, 0, sizeof(chip->tstamp_cc));
+	chip->tstamp_cc.read	= mv88e6xxx_ptp_clock_read;
+	chip->tstamp_cc.mask	= CYCLECOUNTER_MASK(32);
+	/* Raw timestamps are in units of 8-ns clock periods. */
+	chip->tstamp_cc.mult	= 8;
+	chip->tstamp_cc.shift	= 0;
+
+	timecounter_init(&chip->tstamp_tc, &chip->tstamp_cc,
+			 ktime_to_ns(ktime_get_real()));
+
+	chip->last_overflow_check = jiffies;
+
+	INIT_DELAYED_WORK(&chip->overflow_work, mv88e6xxx_ptp_overflow_check);
+
+	chip->ptp_clock_info.owner = THIS_MODULE;
+	snprintf(chip->ptp_clock_info.name, sizeof(chip->ptp_clock_info.name),
+		 dev_name(chip->dev));
+	chip->ptp_clock_info.max_adj	= 0;
+
+	chip->ptp_clock_info.adjfine	= mv88e6xxx_ptp_adjfine;
+	chip->ptp_clock_info.adjtime	= mv88e6xxx_ptp_adjtime;
+	chip->ptp_clock_info.gettime64	= mv88e6xxx_ptp_gettime;
+	chip->ptp_clock_info.settime64	= mv88e6xxx_ptp_settime;
+	chip->ptp_clock_info.enable	= mv88e6xxx_ptp_enable;
+	chip->ptp_clock_info.verify	= mv88e6xxx_ptp_verify;
+
+	chip->ptp_clock = ptp_clock_register(&chip->ptp_clock_info, chip->dev);
+	if (IS_ERR(chip->ptp_clock))
+		return PTR_ERR(chip->ptp_clock);
+
+	schedule_delayed_work(&chip->overflow_work,
+			      MV88E6XXX_TAI_OVERFLOW_PERIOD);
+
+	return 0;
+}
+
+void mv88e6xxx_ptp_free(struct mv88e6xxx_chip *chip)
+{
+	if (chip->ptp_clock) {
+		cancel_delayed_work_sync(&chip->overflow_work);
+
+		ptp_clock_unregister(chip->ptp_clock);
+		chip->ptp_clock = NULL;
+	}
+}
diff --git a/drivers/net/dsa/mv88e6xxx/ptp.h b/drivers/net/dsa/mv88e6xxx/ptp.h
new file mode 100644
index 0000000..c662953
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/ptp.h
@@ -0,0 +1,83 @@
+/*
+ * Marvell 88E6xxx Switch PTP support
+ *
+ * Copyright (c) 2008 Marvell Semiconductor
+ *
+ * Copyright (c) 2017 National Instruments
+ *      Erik Hons <erik.hons@ni.com>
+ *      Brandon Streiff <brandon.streiff@ni.com>
+ *      Dane Wagner <dane.wagner@ni.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _MV88E6XXX_PTP_H
+#define _MV88E6XXX_PTP_H
+
+#include "chip.h"
+
+/* Offset 0x00: TAI Global Config */
+#define MV88E6XXX_TAI_CFG			0x00
+
+/* Offset 0x01: Timestamp Clock Period (ps) */
+#define MV88E6XXX_TAI_CLOCK_PERIOD		0x01
+
+/* Offset 0x02/0x03: Trigger Generation Amount */
+#define MV88E6XXX_TAI_TRIG_GEN_AMOUNT_LO	0x02
+#define MV88E6XXX_TAI_TRIG_GEN_AMOUNT_HI	0x03
+
+/* Offset 0x04: Clock Compensation */
+#define MV88E6XXX_TAI_TRIG_CLOCK_COMP		0x04
+
+/* Offset 0x05: Trigger Configuration */
+#define MV88E6XXX_TAI_TRIG_CFG			0x05
+
+/* Offset 0x06: Ingress Rate Limiter Clock Generation Amount */
+#define MV88E6XXX_TAI_IRL_AMOUNT		0x06
+
+/* Offset 0x07: Ingress Rate Limiter Compensation */
+#define MV88E6XXX_TAI_IRL_COMP			0x07
+
+/* Offset 0x08: Ingress Rate Limiter Compensation */
+#define MV88E6XXX_TAI_IRL_COMP_PS		0x08
+
+/* Offset 0x09: Event Status */
+#define MV88E6XXX_TAI_EVENT_STATUS		0x09
+
+/* Offset 0x0A/0x0B: Event Time */
+#define MV88E6XXX_TAI_EVENT_TIME_LO		0x0a
+#define MV88E6XXX_TAI_EVENT_TYPE_HI		0x0b
+
+/* Offset 0x0E/0x0F: PTP Global Time */
+#define MV88E6XXX_TAI_TIME_LO			0x0e
+#define MV88E6XXX_TAI_TIME_HI			0x0f
+
+/* Offset 0x10/0x11: Trig Generation Time */
+#define MV88E6XXX_TAI_TRIG_TIME_LO		0x10
+#define MV88E6XXX_TAI_TRIG_TIME_HI		0x11
+
+/* Offset 0x12: Lock Status */
+#define MV88E6XXX_TAI_LOCK_STATUS		0x12
+
+#ifdef CONFIG_NET_DSA_MV88E6XXX_PTP
+
+int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip);
+void mv88e6xxx_ptp_free(struct mv88e6xxx_chip *chip);
+
+#else /* !CONFIG_NET_DSA_MV88E6XXX_PTP */
+
+static inline int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip)
+{
+	return -EOPNOTSUPP;
+}
+
+static void mv88e6xxx_ptp_free(struct mv88e6xxx_chip *chip)
+{
+}
+
+#endif /* CONFIG_NET_DSA_MV88E6XXX_PTP */
+
+#endif /* _MV88E6XXX_PTP_H */
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1741710 — Re: [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock

On Thu, Sep 28, 2017 at 10:25:34AM -0500, Brandon Streiff wrote:
> This patch adds basic support for exposing the 32-bit timestamp counter
> inside the mv88e6xxx switch code as a ptp_clock.
> 
> Signed-off-by: Brandon Streiff <brandon.streiff@ni.com>
> ---
>  drivers/net/dsa/mv88e6xxx/Kconfig  |  10 +++
>  drivers/net/dsa/mv88e6xxx/Makefile |   1 +
>  drivers/net/dsa/mv88e6xxx/chip.c   |  20 +++++
>  drivers/net/dsa/mv88e6xxx/chip.h   |  16 ++++
>  drivers/net/dsa/mv88e6xxx/ptp.c    | 180 +++++++++++++++++++++++++++++++++++++
>  drivers/net/dsa/mv88e6xxx/ptp.h    |  83 +++++++++++++++++
>  6 files changed, 310 insertions(+)
>  create mode 100644 drivers/net/dsa/mv88e6xxx/ptp.c
>  create mode 100644 drivers/net/dsa/mv88e6xxx/ptp.h
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/Kconfig b/drivers/net/dsa/mv88e6xxx/Kconfig
> index 1aaa7a9..ae9e7f7 100644
> --- a/drivers/net/dsa/mv88e6xxx/Kconfig
> +++ b/drivers/net/dsa/mv88e6xxx/Kconfig
> @@ -18,3 +18,13 @@ config NET_DSA_MV88E6XXX_GLOBAL2
>  
>  	  It is required on most chips. If the chip you compile the support for
>  	  doesn't have such registers set, say N here. In doubt, say Y.
> +
> +config NET_DSA_MV88E6XXX_PTP
> +	bool "PTP support for Marvell 88E6xxx"
> +	default n
> +	depends on NET_DSA_MV88E6XXX_GLOBAL2
> +	imply NETWORK_PHY_TIMESTAMPING

Hi Brandon

Cool to see this code.

One probably dumb question so far..

It is the MAC which is doing the time stamping, not they PHY?
So why NETWORK_PHY_TIMESTAMPING?

   Andrew

[toc] | [prev] | [next] | [standalone]

#1742213 — RE: [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock

> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 28, 2017 11:57 AM
> 
> It is the MAC which is doing the time stamping, not they PHY?
> So why NETWORK_PHY_TIMESTAMPING?

NETWORK_PHY_TIMESTAMPING implies NET_PTP_CLASSIFY (which I do use) and net/core/timestamping.c (which I didn't). It probably makes more sense to just depend on NET_PTP_CLASSIFY directly.

-- brandon

[toc] | [prev] | [next] | [standalone]

#1741713 — Re: [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock

> +/* The 32-bit timestamp counter overflows every ~34.3 seconds; this task
> + * forces periodic reads so that we don't miss any wraparounds.
> + */
> +#define MV88E6XXX_TAI_OVERFLOW_PERIOD (34 * HZ / 2)
> +static void mv88e6xxx_ptp_overflow_check(struct work_struct *work)
> +{
> +	struct delayed_work *dw = to_delayed_work(work);
> +	struct mv88e6xxx_chip *chip =
> +		container_of(dw, struct mv88e6xxx_chip, overflow_work);
> +	bool timeout = time_is_before_jiffies(chip->last_overflow_check +
> +					      MV88E6XXX_TAI_OVERFLOW_PERIOD);
> +
> +	if (timeout) {

Why do you need this timeout? Do you think the kernel will call this
more often than required?

Also, if it did call this function early, you skip the read, and
reschedule. There is then a danger the next read is after the
wraparound.....

> +		mutex_lock(&chip->reg_lock);
> +		timecounter_read(&chip->tstamp_tc);
> +		chip->last_overflow_check = jiffies;
> +		mutex_unlock(&chip->reg_lock);
> +	}
> +
> +	schedule_delayed_work(&chip->overflow_work,
> +			      MV88E6XXX_TAI_OVERFLOW_PERIOD);
> +}

Thanks
	Andrew

[toc] | [prev] | [next] | [standalone]

#1742190 — RE: [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock

> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 28, 2017 12:03 PM
> 
> > +	bool timeout = time_is_before_jiffies(chip->last_overflow_check +
> > +					      MV88E6XXX_TAI_OVERFLOW_PERIOD);
> > +
> > +	if (timeout) {
> 
> Why do you need this timeout? Do you think the kernel will call this
> more often than required?
> 
> Also, if it did call this function early, you skip the read, and
> reschedule. There is then a danger the next read is after the
> wraparound.....

That was, conceptually, a copy-paste from ixgbe_ptp.c as I was looking for how to implement the overflow accounting; that driver has a similar time_is_before_jiffies check in ixgbe_ptp_overflow_check.
 
Although now that I'm looking it over again, I'm also not certain of the need. Even if we're called more frequently than we expect, that doesn't seem to be harmful with regard to timekeeping. Hmm.

-- brandon

[toc] | [prev] | [next] | [standalone]

#1741728

> - Patch #3: The GPIO config support is handled in a very simple manner.
>   I suspect a longer term goal would be to use pinctrl here.

I assume ptp already has the core code to use pinctrl and Linux
standard GPIOs? What does the device tree binding look like? How do
you specify the GPIOs to use?

What we want to avoid is defining an ABI now, otherwise it is going to
be hard to swap to pinctrl later.

> - Patch #6: the dsa_switch pointer and port index is plumbed from
>   dsa_device_ops::rcv so that we can call the correct port_rxtstamp
>   method. This involved instrumenting all of the *_tag_rcv functions in
>   a way that's kind of a kludge and that I'm not terribly happy with.

Yes, this is ugly. I will see if i can find a better way to do
this. 

      Andrew

[toc] | [prev] | [next] | [standalone]

#1741748

On 09/28/2017 10:36 AM, Andrew Lunn wrote:
>> - Patch #3: The GPIO config support is handled in a very simple manner.
>>   I suspect a longer term goal would be to use pinctrl here.
> 
> I assume ptp already has the core code to use pinctrl and Linux
> standard GPIOs? What does the device tree binding look like? How do
> you specify the GPIOs to use?
> 
> What we want to avoid is defining an ABI now, otherwise it is going to
> be hard to swap to pinctrl later.
> 
>> - Patch #6: the dsa_switch pointer and port index is plumbed from
>>   dsa_device_ops::rcv so that we can call the correct port_rxtstamp
>>   method. This involved instrumenting all of the *_tag_rcv functions in
>>   a way that's kind of a kludge and that I'm not terribly happy with.
> 
> Yes, this is ugly. I will see if i can find a better way to do
> this. 

See my reply in patch 6, I may be missing something, but once
dst->rdcv() has been called, skb->dev points to the slave network device
which already contains the switch port and switch information in
dsa_slave_priv, so that should lift the need for asking the individual
taggers' rcv() callback to tell us about it.
-- 
Florian

[toc] | [prev] | [next] | [standalone]

#1742207

> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 28, 2017 12:36 PM
>
> I assume ptp already has the core code to use pinctrl and Linux
> standard GPIOs? What does the device tree binding look like? How do
> you specify the GPIOs to use?
> 
> What we want to avoid is defining an ABI now, otherwise it is going to
> be hard to swap to pinctrl later.

A ptp_clock_info has an array of struct ptp_pin_desc which defines "pins" with a name ("Hardware specific human readable pin name"), an index, and a bitmask of valid functions. The ptp_pin_desc structure is shared with usermode for the PTP_PIN_GETFUNC and PTP_PIN_SETFUNC ioctls. The pins are also exposed in sysfs (see Documentation/ABI/testing/sysfs-ptp). The underlying implementation for configuring the hardware is left up to the PHC driver. I don't see any drivers today that use the PHC pin API as a layer over pinctrl/gpiochip, but there's no reason that that couldn't be the case.

For mv88e6xxx, we name the pins using the pattern "mv88e6xxx_gpio%d"; this appears to be in line with current practice (igb_ptp.c uses "SDP%d", mlx5 driver uses "mlx5_pps%d"). Usermode code appears to be expected to determine which pin it needs to use. (Our current userspace code, for instance, knows that it needs to find "mv88e6xxx_gpio8".)

For mv88e6xxx, Device Tree does feel like a better option here for declaring names, functions, and pin usages. Not all platforms that use the PTP API use Device Tree though.

-- brandon

[toc] | [prev] | [next] | [standalone]

#1742015

Brandon,

On Thu, Sep 28, 2017 at 10:25:32AM -0500, Brandon Streiff wrote:
> - Patch #2: We expose the switch time as a PTP clock but don't support
>   adjustment (max_adj=0).

The driver should implement a cyclecounter/timecounter.

> Our platform adjusted a systemwide oscillator
>   from userspace, so we didn't need adjustment at this layer, but other
>   PTP clock drivers support this and we probably should too.

We don't currently have any way to support this kind of HW or anything
like an external VCO.  I would like to find a way to do this, but that
is a different kettle of fish as it might require changes in the PHC
subsystem.  For this driver, I think we should get it merged using the
cyclecounter/timecounter (as that will benefit lots of users) and
worry about the external oscillator later.
 
> Feedback is appreciated.

I happy to see this series.  I just finished porting an out-of-tree
PHC driver for the Marvell mv88e635x, and I want to mainline it, but I
also have a few uglies.

Unfortunately I am in the middle of a move right now, and so my review
of this series might have to wait a bit.  However, I am looking
forward to comparing notes, and then getting this support in.

Thanks,
Richard

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web