All of lore.kernel.org
 help / color / mirror / Atom feed
* [net-next 0/6] ptp: support virtual clocks for multiple domains
@ 2021-05-07  8:57 Yangbo Lu
  2021-05-07  8:57 ` [net-next 1/6] ptp: add ptp virtual clock driver framework Yangbo Lu
                   ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: Yangbo Lu @ 2021-05-07  8:57 UTC (permalink / raw)
  To: netdev
  Cc: Yangbo Lu, David S . Miller, Richard Cochran, Claudiu Manoil,
	Jakub Kicinski

PTP hardware clock user space API, together with SO_TIMESTAMPING socket
options presents a standardized method for developing PTP user space
programs.

Current ptp driver exposes only one /dev/ptpN device which is the time
source for hardware timestamping. No matter the /dev/ptpN device is
registered by a physical clock or a virtual clock utilizing timecounter,
there is only one.

This patch-set provides a way to support PTP virtual clocks for multiple
domains:

- Register PTP virtual clock with specified domain via sysfs. No number
  limitation. Or remove them via sysfs if not needs them.

- The domain that ptp clock serves could be changed via sysfs too.

- Regarding network timestamp, the additional work to support domain is
  parsing PTP packet domain number, and convert hardware timestamp to
  domain time before transmitting it to upper stack. (If no ptp device
  serves the domain, just use the hardware timestamp. No conversion.)

So actually, the ptp virtual clock driver exposes more /dev/ptpN
devices for using. The timestamp conversion to domain time is handled in
driver. No user space API changing to support multiple domains.
And single domain PTP user space application could be ran multiple times
in background for multiple domains synchronization verification.

For example, we have ptp virtual clocks ptp1, ptp2, and ptp3 registered
for domain 1, 2 and 3 on board1. We could run ptp4l for 3 domains
synchronization on board1.

  ptp4l -i eno0 -p/dev/ptp1 -m --domainNumber=1 --priority1=127 > domain1-master.log 2>&1 &
  ptp4l -i eno0 -p/dev/ptp2 -m --domainNumber=2 --priority1=127 > domain2-master.log 2>&1 &
  ptp4l -i eno0 -p/dev/ptp3 -m --domainNumber=3 --priority1=128 > domain3-slave.log 2>&1 &

On board2 which is connected to board1 in back-to-back way, we have ptp
virtual clocks ptp1, ptp2, and ptp3 registered for domain 1, 2 and 3.
We could run ptp4l for 3 domains synchronization with board1 on board2.

  ptp4l -i eno0 -p/dev/ptp1 -m --domainNumber=1 --priority1=128 > domain1-slave.log 2>&1 &
  ptp4l -i eno0 -p/dev/ptp2 -m --domainNumber=2 --priority1=128 > domain2-slave.log 2>&1 &
  ptp4l -i eno0 -p/dev/ptp3 -m --domainNumber=3 --priority1=127 > domain3-master.log 2>&1 &

But this patch-set is not perfect. And there is still much work to do,
such as,

- Make changing on physical clock transparent to virtual clocks.
  The virtual clock is based on free running physical clock. If physical
  clock has to be changed, how to make the change transparent to all
  virtual clocks? Actually the frequency adjustmend on physical clock
  may be hidden by updating virtual clocks in opposite direction at same
  time. Considering the ppb values adjusted, the code execution delay
  will be little enough to ignore. But it's hard to hide clock stepping,
  by now I think the workaround may be inhibiting physical clock stepping
  when run user space ptp application.

- User space API for PTP virtual clock registering/removing, for user
  space PTP application.

- Make common function for ptp domain parsing so that any driver could
  use it.

- We may consider some reworking on this ptp virtual clock driver, so that
  current ptp device drivers with timecounter used may convert to use it.

Yangbo Lu (6):
  ptp: add ptp virtual clock driver framework
  ptp: support virtual clock and domain via sysfs
  ptp_qoriq: export ptp clock reading function for cyclecounter
  enetc_ptp: support ptp virtual clock
  enetc: store ptp device pointer
  enetc: support PTP domain timestamp conversion

 Documentation/ABI/testing/sysfs-ptp           |  25 +++
 MAINTAINERS                                   |   6 +
 drivers/net/ethernet/freescale/enetc/enetc.c  |  37 +++-
 drivers/net/ethernet/freescale/enetc/enetc.h  |   1 +
 .../net/ethernet/freescale/enetc/enetc_pf.c   |   5 +
 .../net/ethernet/freescale/enetc/enetc_ptp.c  |  11 ++
 .../net/ethernet/freescale/enetc/enetc_vf.c   |   5 +
 drivers/ptp/Makefile                          |   2 +-
 drivers/ptp/ptp_private.h                     |  25 +++
 drivers/ptp/ptp_qoriq.c                       |  15 ++
 drivers/ptp/ptp_sysfs.c                       | 122 ++++++++++++
 drivers/ptp/ptp_vclock.c                      | 176 ++++++++++++++++++
 include/linux/fsl/ptp_qoriq.h                 |   1 +
 include/linux/ptp_clock_kernel.h              |  51 +++++
 14 files changed, 478 insertions(+), 4 deletions(-)
 create mode 100644 drivers/ptp/ptp_vclock.c


base-commit: 9d31d2338950293ec19d9b095fbaa9030899dcb4
-- 
2.25.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [net-next 1/6] ptp: add ptp virtual clock driver framework
  2021-05-07  8:57 [net-next 0/6] ptp: support virtual clocks for multiple domains Yangbo Lu
@ 2021-05-07  8:57 ` Yangbo Lu
  2021-05-07 10:42     ` kernel test robot
  2021-05-07 11:26     ` kernel test robot
  2021-05-07  8:57 ` [net-next 2/6] ptp: support virtual clock and domain via sysfs Yangbo Lu
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 22+ messages in thread
From: Yangbo Lu @ 2021-05-07  8:57 UTC (permalink / raw)
  To: netdev
  Cc: Yangbo Lu, David S . Miller, Richard Cochran, Claudiu Manoil,
	Jakub Kicinski

This patch is to add ptp virtual clock driver framework. The main purpose
is to support PTP multiple domains over multiple PTP virtual clocks with
only single physical clock.

What has the patch exported,

- ptp_vclock_cc structure for specifying cyclecounter information that ptp
  virtual clocks will use. When registering ptp clock for physical clock,
  the ptp virtual clock will be valid to register if the pointer of
  ptp_vclock_cc structure is provided in ptp_clock_info of physical clock.

- ptp_vclock_register/ptp_vclock_unregister APIs for ptp virtual clock
  register/unregister for specified domain number. They are private for ptp
  driver.

- ptp_clock_domain_tstamp API to convert hardware time stamp to domain time
  stamp.

- ptp_get_pclock_info API for device driver to get ptp_clock_info pointer
  of physical clock from cyclecounter pointer of ptp virtual clock.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 MAINTAINERS                      |   6 ++
 drivers/ptp/Makefile             |   2 +-
 drivers/ptp/ptp_private.h        |  25 +++++
 drivers/ptp/ptp_vclock.c         | 176 +++++++++++++++++++++++++++++++
 include/linux/ptp_clock_kernel.h |  51 +++++++++
 5 files changed, 259 insertions(+), 1 deletion(-)
 create mode 100644 drivers/ptp/ptp_vclock.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4796ccf9f871..9f9280b29e47 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14733,6 +14733,12 @@ F:	drivers/net/phy/dp83640*
 F:	drivers/ptp/*
 F:	include/linux/ptp_cl*
 
+PTP VIRTUAL CLOCK SUPPORT
+M:	Yangbo Lu <yangbo.lu@nxp.com>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	drivers/ptp/ptp_vclock.c
+
 PTRACE SUPPORT
 M:	Oleg Nesterov <oleg@redhat.com>
 S:	Maintained
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index db5aef3bddc6..3c75d7de7793 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -3,7 +3,7 @@
 # Makefile for PTP 1588 clock support.
 #
 
-ptp-y					:= ptp_clock.o ptp_chardev.o ptp_sysfs.o
+ptp-y					:= ptp_clock.o ptp_chardev.o ptp_sysfs.o ptp_vclock.o
 obj-$(CONFIG_PTP_1588_CLOCK)		+= ptp.o
 obj-$(CONFIG_PTP_1588_CLOCK_DTE)	+= ptp_dte.o
 obj-$(CONFIG_PTP_1588_CLOCK_INES)	+= ptp_ines.o
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 6b97155148f1..9ff0afc57a7f 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -48,6 +48,29 @@ struct ptp_clock {
 	struct kthread_delayed_work aux_work;
 };
 
+#define cc_to_vclock(d) container_of((d), struct ptp_vclock, cc)
+#define dw_to_vclock(d) container_of((d), struct ptp_vclock, refresh_work)
+#define info_to_vclock(d) container_of((d), struct ptp_vclock, info)
+
+struct ptp_vclock {
+	struct ptp_clock *pclock;
+	struct ptp_clock_info info;
+	struct ptp_clock *clock;
+	struct cyclecounter cc;
+	struct timecounter tc;
+	spinlock_t lock;	/* protects tc/cc */
+	struct delayed_work refresh_work;
+	unsigned long refresh_interval;
+	u32 mult;
+	u32 mult_num;
+	u32 mult_dem;
+};
+
+struct domain_tstamp {
+	u64 tstamp;
+	u8 domain;
+};
+
 /*
  * The function queue_cnt() is safe for readers to call without
  * holding q->lock. Readers use this function to verify that the queue
@@ -89,4 +112,6 @@ extern const struct attribute_group *ptp_groups[];
 int ptp_populate_pin_groups(struct ptp_clock *ptp);
 void ptp_cleanup_pin_groups(struct ptp_clock *ptp);
 
+struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock, u8 domain);
+void ptp_vclock_unregister(struct ptp_vclock *vclock);
 #endif
diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
new file mode 100644
index 000000000000..765acc0f7576
--- /dev/null
+++ b/drivers/ptp/ptp_vclock.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * PTP virtual clock driver
+ *
+ * Copyright 2021 NXP
+ */
+#include <linux/slab.h>
+#include "ptp_private.h"
+
+static int ptp_vclock_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
+{
+	struct ptp_vclock *vclock = info_to_vclock(ptp);
+	unsigned long flags;
+	s64 mult_adj;
+
+	mult_adj = (s64)scaled_ppm * vclock->mult_num;
+	mult_adj = div_s64(mult_adj, vclock->mult_dem);
+
+	spin_lock_irqsave(&vclock->lock, flags);
+	timecounter_read(&vclock->tc);
+	vclock->cc.mult = vclock->mult + mult_adj;
+	spin_unlock_irqrestore(&vclock->lock, flags);
+
+	return 0;
+}
+
+static int ptp_vclock_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct ptp_vclock *vclock = info_to_vclock(ptp);
+	unsigned long flags;
+
+	spin_lock_irqsave(&vclock->lock, flags);
+	timecounter_adjtime(&vclock->tc, delta);
+	spin_unlock_irqrestore(&vclock->lock, flags);
+
+	return 0;
+}
+
+static int ptp_vclock_gettime(struct ptp_clock_info *ptp,
+			      struct timespec64 *ts)
+{
+	struct ptp_vclock *vclock = info_to_vclock(ptp);
+	unsigned long flags;
+	u64 ns;
+
+	spin_lock_irqsave(&vclock->lock, flags);
+	ns = timecounter_read(&vclock->tc);
+	spin_unlock_irqrestore(&vclock->lock, flags);
+	*ts = ns_to_timespec64(ns);
+
+	return 0;
+}
+
+static int ptp_vclock_settime(struct ptp_clock_info *ptp,
+			      const struct timespec64 *ts)
+{
+	struct ptp_vclock *vclock = info_to_vclock(ptp);
+	u64 ns = timespec64_to_ns(ts);
+	unsigned long flags;
+
+	spin_lock_irqsave(&vclock->lock, flags);
+	timecounter_init(&vclock->tc, &vclock->cc, ns);
+	spin_unlock_irqrestore(&vclock->lock, flags);
+
+	return 0;
+}
+
+static const struct ptp_clock_info ptp_vclock_info = {
+	.owner		= THIS_MODULE,
+	.name		= "ptp vclock",
+	.adjfine	= ptp_vclock_adjfine,
+	.adjtime	= ptp_vclock_adjtime,
+	.gettime64	= ptp_vclock_gettime,
+	.settime64	= ptp_vclock_settime,
+	.max_adj	= 32000000,
+};
+
+static void ptp_vclock_refresh(struct work_struct *work)
+{
+	struct delayed_work *dw = to_delayed_work(work);
+	struct ptp_vclock *vclock = dw_to_vclock(dw);
+	struct timespec64 ts;
+
+	ptp_vclock_gettime(&vclock->info, &ts);
+	schedule_delayed_work(&vclock->refresh_work, vclock->refresh_interval);
+}
+
+static int ptp_clock_find_domain_tstamp(struct device *dev, void *data)
+{
+	struct ptp_clock *ptp = dev_get_drvdata(dev);
+	struct ptp_clock_info *info = ptp->info;
+	struct domain_tstamp *domain_ts = data;
+	struct ptp_vclock *vclock;
+	unsigned long flags;
+
+	if (!info->is_vclock)
+		return 0;
+
+	/* Convert to domain tstamp if there is a domain matched */
+	if (info->domain == domain_ts->domain) {
+		vclock = info_to_vclock(info);
+		spin_lock_irqsave(&vclock->lock, flags);
+		domain_ts->tstamp = timecounter_cyc2time(&vclock->tc, domain_ts->tstamp);
+		spin_unlock_irqrestore(&vclock->lock, flags);
+		/* For break. Not error. */
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+void ptp_clock_domain_tstamp(struct device *dev, u64 *tstamp, u8 domain)
+{
+	struct domain_tstamp domain_ts;
+
+	domain_ts.tstamp = *tstamp;
+	domain_ts.domain = domain;
+
+	device_for_each_child(dev, &domain_ts, ptp_clock_find_domain_tstamp);
+	*tstamp = domain_ts.tstamp;
+}
+EXPORT_SYMBOL(ptp_clock_domain_tstamp);
+
+struct ptp_clock_info *ptp_get_pclock_info(const struct cyclecounter *cc)
+{
+	struct ptp_vclock *vclock = cc_to_vclock(cc);
+
+	return vclock->pclock->info;
+}
+EXPORT_SYMBOL(ptp_get_pclock_info);
+
+struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock, u8 domain)
+{
+	struct ptp_vclock_cc *vclock_cc = pclock->info->vclock_cc;
+	struct ptp_vclock *vclock;
+
+	vclock = kzalloc(sizeof(*vclock), GFP_KERNEL);
+	if (!vclock)
+		return NULL;
+
+	vclock->pclock = pclock;
+
+	vclock->info = ptp_vclock_info;
+	vclock->info.vclock_cc = vclock_cc;
+	vclock->info.is_vclock = true;
+	vclock->info.domain = domain;
+
+	vclock->cc = vclock_cc->cc;
+	vclock->mult = vclock_cc->cc.mult;
+	vclock->refresh_interval = vclock_cc->refresh_interval;
+	vclock->mult_num = vclock_cc->mult_num;
+	vclock->mult_dem = vclock_cc->mult_dem;
+
+	spin_lock_init(&vclock->lock);
+
+	vclock->clock = ptp_clock_register(&vclock->info, pclock->dev.parent);
+	if (IS_ERR_OR_NULL(vclock->clock)) {
+		kfree(vclock);
+		return NULL;
+	}
+
+	timecounter_init(&vclock->tc, &vclock->cc,
+			 ktime_to_ns(ktime_get_real()));
+
+	INIT_DELAYED_WORK(&vclock->refresh_work, ptp_vclock_refresh);
+	schedule_delayed_work(&vclock->refresh_work, vclock->refresh_interval);
+
+	return vclock;
+}
+
+void ptp_vclock_unregister(struct ptp_vclock *vclock)
+{
+	cancel_delayed_work_sync(&vclock->refresh_work);
+	ptp_clock_unregister(vclock->clock);
+	kfree(vclock);
+}
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 0d47fd33b228..b2823af9b150 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -48,6 +48,32 @@ struct ptp_system_timestamp {
 	struct timespec64 post_ts;
 };
 
+/**
+ * struct ptp_vclock_cc - ptp virtual clock cycle counter info
+ *
+ * @cc:               cyclecounter structure
+ * @refresh_interval: time interval to refresh time counter, to avoid 64-bit
+ *                    overflow during delta conversion. For example, with
+ *                    cc.mult value 2^28,  there are 36 bits left of cycle
+ *                    counter. With 1 ns counter resolution, the overflow time
+ *                    is 2^36 ns which is 68.7 s. The refresh_interval may be
+ *                    (60 * HZ) less than 68.7 s.
+ * @mult_num:         parameter for cc.mult adjustment calculation, see below
+ * @mult_dem:         parameter for cc.mult adjustment calculation, see below
+ *
+ * scaled_ppm to adjustment(mult_adj) of cc.mult
+ *
+ * mult_adj = mult * (ppb / 10^9)
+ *          = mult * (scaled_ppm * 1000 / 2^16) / 10^9
+ *          = scaled_ppm * mult_num / mult_dem
+ */
+struct ptp_vclock_cc {
+	struct cyclecounter cc;
+	unsigned long refresh_interval;
+	u32 mult_num;
+	u32 mult_dem;
+};
+
 /**
  * struct ptp_clock_info - describes a PTP hardware clock
  *
@@ -157,6 +183,11 @@ struct ptp_clock_info {
 	int (*verify)(struct ptp_clock_info *ptp, unsigned int pin,
 		      enum ptp_pin_function func, unsigned int chan);
 	long (*do_aux_work)(struct ptp_clock_info *ptp);
+
+	/* For virtual clock */
+	struct ptp_vclock_cc *vclock_cc;
+	u8 domain;
+	bool is_vclock;
 };
 
 struct ptp_clock;
@@ -286,6 +317,21 @@ int ptp_schedule_worker(struct ptp_clock *ptp, unsigned long delay);
  */
 void ptp_cancel_worker_sync(struct ptp_clock *ptp);
 
+/**
+ * ptp_get_pclock_info() - get ptp_clock_info pointer of physical clock
+ *
+ * @cc:     cyclecounter pointer of ptp virtual clock.
+ */
+struct ptp_clock_info *ptp_get_pclock_info(const struct cyclecounter *cc);
+
+/**
+ * ptp_clock_domain_tstamp() - convert to domain time stamp
+ *
+ * @dev:     device pointer of current ptp clock.
+ * @tstamp:  time stamp pointer to hardware time stamp
+ * @domain:  domain number to convert
+ */
+void ptp_clock_domain_tstamp(struct device *dev, u64 *tstamp, u8 domain);
 #else
 static inline struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
 						   struct device *parent)
@@ -306,6 +352,11 @@ static inline int ptp_schedule_worker(struct ptp_clock *ptp,
 static inline void ptp_cancel_worker_sync(struct ptp_clock *ptp)
 { }
 
+static inline struct ptp_clock_info *ptp_get_pclock_info(const struct cyclecounter *cc);
+{ return NULL; }
+
+void ptp_clock_domain_tstamp(struct device *dev, u64 *tstamp, u8 domain)
+{ }
 #endif
 
 static inline void ptp_read_system_prets(struct ptp_system_timestamp *sts)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [net-next 2/6] ptp: support virtual clock and domain via sysfs
  2021-05-07  8:57 [net-next 0/6] ptp: support virtual clocks for multiple domains Yangbo Lu
  2021-05-07  8:57 ` [net-next 1/6] ptp: add ptp virtual clock driver framework Yangbo Lu
@ 2021-05-07  8:57 ` Yangbo Lu
  2021-05-07  8:57 ` [net-next 3/6] ptp_qoriq: export ptp clock reading function for cyclecounter Yangbo Lu
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Yangbo Lu @ 2021-05-07  8:57 UTC (permalink / raw)
  To: netdev
  Cc: Yangbo Lu, David S . Miller, Richard Cochran, Claudiu Manoil,
	Jakub Kicinski

Add support for virtual clock and domain via sysfs. Attributes
new_vclock_domain/delete_vclock_domain are to create/remove ptp
virtual clock. Attribute domain is to change domain value of the
ptp clock.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 Documentation/ABI/testing/sysfs-ptp |  25 ++++++
 drivers/ptp/ptp_sysfs.c             | 122 ++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
index 2363ad810ddb..9c419c5554b5 100644
--- a/Documentation/ABI/testing/sysfs-ptp
+++ b/Documentation/ABI/testing/sysfs-ptp
@@ -25,6 +25,23 @@ Description:
 		MAC based ones. The string does not necessarily have
 		to be any kind of unique id.
 
+What:		/sys/class/ptp/ptpN/delete_vclock_domain
+Date:		May 2021
+Contact:	Yangbo Lu <yangbo.lu@nxp.com>
+Description:
+		This write-only file removes PTP virtual clock for the
+		specified domain. Write the u8 domain value into this
+		file to remove the PTP virtual clock.
+
+What:		/sys/class/ptp/ptpN/domain
+Date:		May 2021
+Contact:	Yangbo Lu <yangbo.lu@nxp.com>
+Description:
+		This file contains the domain value that the PTP clock
+		serves. Time stamps of PTP messages of this domain are
+		provided by this PTP clock. Write a new u8 value into
+		this file to change the domain.
+
 What:		/sys/class/ptp/ptpN/max_adjustment
 Date:		September 2010
 Contact:	Richard Cochran <richardcochran@gmail.com>
@@ -101,6 +118,14 @@ Description:
 		the form of three integers: channel index, seconds,
 		and nanoseconds.
 
+What:		/sys/class/ptp/ptpN/new_vclock_domain
+Date:		May 2021
+Contact:	Yangbo Lu <yangbo.lu@nxp.com>
+Description:
+		This write-only file creates PTP virtual clock for a
+		specified domain. Write the u8 domain value into this
+		file to create the PTP virtual clock.
+
 What:		/sys/class/ptp/ptpN/period
 Date:		September 2010
 Contact:	Richard Cochran <richardcochran@gmail.com>
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index be076a91e20e..d8e7e05bd52d 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -121,6 +121,119 @@ static ssize_t period_store(struct device *dev,
 }
 static DEVICE_ATTR(period, 0220, NULL, period_store);
 
+static int check_domain_avail(struct device *dev, void *data)
+{
+	struct ptp_clock *ptp = dev_get_drvdata(dev);
+	struct ptp_clock_info *info = ptp->info;
+	u8 *domain = data;
+
+	if (info->domain == *domain)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int delete_vclock_domain(struct device *dev, void *data)
+{
+	struct ptp_clock *ptp = dev_get_drvdata(dev);
+	struct ptp_clock_info *info = ptp->info;
+	struct ptp_vclock *vclock = info_to_vclock(info);
+	u8 *domain = data;
+
+	if (!info->is_vclock)
+		return 0;
+
+	if (info->domain == *domain) {
+		ptp_vclock_unregister(vclock);
+		/* For break. Not error. */
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static ssize_t domain_show(struct device *dev,
+			   struct device_attribute *attr, char *page)
+{
+	struct ptp_clock *ptp = dev_get_drvdata(dev);
+
+	return snprintf(page, PAGE_SIZE-1, "%d\n", ptp->info->domain);
+}
+
+static ssize_t domain_store(struct device *dev,
+			    struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	struct ptp_clock *ptp = dev_get_drvdata(dev);
+	struct ptp_clock_info *info = ptp->info;
+	int err = -EINVAL;
+	u8 domain;
+
+	if (kstrtou8(buf, 0, &domain))
+		goto out;
+
+	if (device_for_each_child(dev->parent, &domain, check_domain_avail)) {
+		dev_err(dev, "the domain value already in used\n");
+		goto out;
+	}
+
+	info->domain = domain;
+
+	return count;
+out:
+	return err;
+}
+static DEVICE_ATTR_RW(domain);
+
+static ssize_t new_vclock_domain_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	struct ptp_clock *ptp = dev_get_drvdata(dev);
+	struct ptp_vclock *vclock;
+	int err = -EINVAL;
+	u8 domain;
+
+	if (kstrtou8(buf, 0, &domain))
+		goto out;
+
+	if (device_for_each_child(dev->parent, &domain, check_domain_avail)) {
+		dev_err(dev, "the domain value already in used\n");
+		goto out;
+	}
+
+	vclock = ptp_vclock_register(ptp, domain);
+	if (!vclock)
+		goto out;
+
+	return count;
+out:
+	return err;
+}
+static DEVICE_ATTR_WO(new_vclock_domain);
+
+static ssize_t delete_vclock_domain_store(struct device *dev,
+					  struct device_attribute *attr,
+					  const char *buf, size_t count)
+{
+	int err = -EINVAL;
+	u8 domain;
+
+	if (kstrtou8(buf, 0, &domain))
+		goto out;
+
+	if (!device_for_each_child(dev->parent, &domain,
+				   delete_vclock_domain)) {
+		dev_err(dev, "no such vclock domain in used\n");
+		goto out;
+	}
+
+	return count;
+out:
+	return err;
+}
+static DEVICE_ATTR_WO(delete_vclock_domain);
+
 static ssize_t pps_enable_store(struct device *dev,
 				struct device_attribute *attr,
 				const char *buf, size_t count)
@@ -161,6 +274,9 @@ static struct attribute *ptp_attrs[] = {
 	&dev_attr_extts_enable.attr,
 	&dev_attr_fifo.attr,
 	&dev_attr_period.attr,
+	&dev_attr_domain.attr,
+	&dev_attr_new_vclock_domain.attr,
+	&dev_attr_delete_vclock_domain.attr,
 	&dev_attr_pps_enable.attr,
 	NULL
 };
@@ -183,6 +299,12 @@ static umode_t ptp_is_attribute_visible(struct kobject *kobj,
 	} else if (attr == &dev_attr_pps_enable.attr) {
 		if (!info->pps)
 			mode = 0;
+	} else if (attr == &dev_attr_new_vclock_domain.attr) {
+		if (info->is_vclock || !info->vclock_cc)
+			mode = 0;
+	} else if (attr == &dev_attr_delete_vclock_domain.attr) {
+		if (info->is_vclock || !info->vclock_cc)
+			mode = 0;
 	}
 
 	return mode;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [net-next 3/6] ptp_qoriq: export ptp clock reading function for cyclecounter
  2021-05-07  8:57 [net-next 0/6] ptp: support virtual clocks for multiple domains Yangbo Lu
  2021-05-07  8:57 ` [net-next 1/6] ptp: add ptp virtual clock driver framework Yangbo Lu
  2021-05-07  8:57 ` [net-next 2/6] ptp: support virtual clock and domain via sysfs Yangbo Lu
@ 2021-05-07  8:57 ` Yangbo Lu
  2021-05-07  8:57 ` [net-next 4/6] enetc_ptp: support ptp virtual clock Yangbo Lu
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Yangbo Lu @ 2021-05-07  8:57 UTC (permalink / raw)
  To: netdev
  Cc: Yangbo Lu, David S . Miller, Richard Cochran, Claudiu Manoil,
	Jakub Kicinski

Export ptp clock reading function for cyclecounter to read cycles.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 drivers/ptp/ptp_qoriq.c       | 15 +++++++++++++++
 include/linux/fsl/ptp_qoriq.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/ptp/ptp_qoriq.c b/drivers/ptp/ptp_qoriq.c
index 08f4cf0ad9e3..4617055a3307 100644
--- a/drivers/ptp/ptp_qoriq.c
+++ b/drivers/ptp/ptp_qoriq.c
@@ -311,6 +311,21 @@ int ptp_qoriq_enable(struct ptp_clock_info *ptp,
 }
 EXPORT_SYMBOL_GPL(ptp_qoriq_enable);
 
+u64 ptp_qoriq_clock_read(const struct cyclecounter *cc)
+{
+	struct ptp_clock_info *ptp = ptp_get_pclock_info(cc);
+	struct ptp_qoriq *ptp_qoriq = container_of(ptp, struct ptp_qoriq, caps);
+	unsigned long flags;
+	u64 ns;
+
+	spin_lock_irqsave(&ptp_qoriq->lock, flags);
+	ns = tmr_cnt_read(ptp_qoriq);
+	spin_unlock_irqrestore(&ptp_qoriq->lock, flags);
+
+	return ns;
+}
+EXPORT_SYMBOL_GPL(ptp_qoriq_clock_read);
+
 static const struct ptp_clock_info ptp_qoriq_caps = {
 	.owner		= THIS_MODULE,
 	.name		= "qoriq ptp clock",
diff --git a/include/linux/fsl/ptp_qoriq.h b/include/linux/fsl/ptp_qoriq.h
index 01acebe37fab..9a2ecd696c7e 100644
--- a/include/linux/fsl/ptp_qoriq.h
+++ b/include/linux/fsl/ptp_qoriq.h
@@ -193,6 +193,7 @@ int ptp_qoriq_settime(struct ptp_clock_info *ptp,
 		      const struct timespec64 *ts);
 int ptp_qoriq_enable(struct ptp_clock_info *ptp,
 		     struct ptp_clock_request *rq, int on);
+u64 ptp_qoriq_clock_read(const struct cyclecounter *cc);
 int extts_clean_up(struct ptp_qoriq *ptp_qoriq, int index, bool update_event);
 #ifdef CONFIG_DEBUG_FS
 void ptp_qoriq_create_debugfs(struct ptp_qoriq *ptp_qoriq);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [net-next 4/6] enetc_ptp: support ptp virtual clock
  2021-05-07  8:57 [net-next 0/6] ptp: support virtual clocks for multiple domains Yangbo Lu
                   ` (2 preceding siblings ...)
  2021-05-07  8:57 ` [net-next 3/6] ptp_qoriq: export ptp clock reading function for cyclecounter Yangbo Lu
@ 2021-05-07  8:57 ` Yangbo Lu
  2021-05-07 19:58   ` Claudiu Manoil
  2021-05-07 20:50     ` kernel test robot
  2021-05-07  8:57 ` [net-next 5/6] enetc: store ptp device pointer Yangbo Lu
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 22+ messages in thread
From: Yangbo Lu @ 2021-05-07  8:57 UTC (permalink / raw)
  To: netdev
  Cc: Yangbo Lu, David S . Miller, Richard Cochran, Claudiu Manoil,
	Jakub Kicinski

Add support for ptp virtual clock.

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

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ptp.c b/drivers/net/ethernet/freescale/enetc/enetc_ptp.c
index bc594892507a..52de736df800 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_ptp.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_ptp.c
@@ -10,6 +10,16 @@
 int enetc_phc_index = -1;
 EXPORT_SYMBOL(enetc_phc_index);
 
+struct ptp_vclock_cc ptp_qoriq_vclock_cc = {
+	.cc.read		= ptp_qoriq_clock_read,
+	.cc.mask		= CYCLECOUNTER_MASK(64),
+	.cc.shift		= 28,
+	.cc.mult		= (1 << 28),
+	.refresh_interval	= (HZ * 60),
+	.mult_num		= (1 << 6),
+	.mult_dem		= 15625,
+};
+
 static struct ptp_clock_info enetc_ptp_caps = {
 	.owner		= THIS_MODULE,
 	.name		= "ENETC PTP clock",
@@ -24,6 +34,7 @@ static struct ptp_clock_info enetc_ptp_caps = {
 	.gettime64	= ptp_qoriq_gettime,
 	.settime64	= ptp_qoriq_settime,
 	.enable		= ptp_qoriq_enable,
+	.vclock_cc	= &ptp_qoriq_vclock_cc,
 };
 
 static int enetc_ptp_probe(struct pci_dev *pdev,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [net-next 5/6] enetc: store ptp device pointer
  2021-05-07  8:57 [net-next 0/6] ptp: support virtual clocks for multiple domains Yangbo Lu
                   ` (3 preceding siblings ...)
  2021-05-07  8:57 ` [net-next 4/6] enetc_ptp: support ptp virtual clock Yangbo Lu
@ 2021-05-07  8:57 ` Yangbo Lu
  2021-05-07  8:57 ` [net-next 6/6] enetc: support PTP domain timestamp conversion Yangbo Lu
  2021-05-08 19:17 ` [net-next 0/6] ptp: support virtual clocks for multiple domains Richard Cochran
  6 siblings, 0 replies; 22+ messages in thread
From: Yangbo Lu @ 2021-05-07  8:57 UTC (permalink / raw)
  To: netdev
  Cc: Yangbo Lu, David S . Miller, Richard Cochran, Claudiu Manoil,
	Jakub Kicinski

Store ptp device pointer which will be used for ptp domain
timestamp conversion.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.h    | 1 +
 drivers/net/ethernet/freescale/enetc/enetc_pf.c | 5 +++++
 drivers/net/ethernet/freescale/enetc/enetc_vf.c | 5 +++++
 3 files changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index 08b283347d9c..03e1ee1f6615 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -351,6 +351,7 @@ struct enetc_ndev_priv {
 
 	struct work_struct	tx_onestep_tstamp;
 	struct sk_buff_head	tx_skbs;
+	struct device *ptp_dev;
 };
 
 /* Messaging */
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 31274325159a..71029b26e92e 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -1201,6 +1201,7 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 {
 	struct device_node *node = pdev->dev.of_node;
 	struct enetc_ndev_priv *priv;
+	struct pci_dev *ptp_pdev;
 	struct net_device *ndev;
 	struct enetc_si *si;
 	struct enetc_pf *pf;
@@ -1293,6 +1294,10 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 		goto err_alloc_msix;
 	}
 
+	ptp_pdev = pci_get_device(PCI_VENDOR_ID_FREESCALE, ENETC_DEV_ID_PTP, NULL);
+	if (ptp_pdev)
+		priv->ptp_dev = &ptp_pdev->dev;
+
 	if (!of_get_phy_mode(node, &pf->if_mode)) {
 		err = enetc_mdiobus_create(pf, node);
 		if (err)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_vf.c b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
index 03090ba7e226..17fea364b091 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
@@ -138,6 +138,7 @@ static int enetc_vf_probe(struct pci_dev *pdev,
 			  const struct pci_device_id *ent)
 {
 	struct enetc_ndev_priv *priv;
+	struct pci_dev *ptp_pdev;
 	struct net_device *ndev;
 	struct enetc_si *si;
 	int err;
@@ -188,6 +189,10 @@ static int enetc_vf_probe(struct pci_dev *pdev,
 		goto err_alloc_msix;
 	}
 
+	ptp_pdev = pci_get_device(PCI_VENDOR_ID_FREESCALE, ENETC_DEV_ID_PTP, NULL);
+	if (ptp_pdev)
+		priv->ptp_dev = &ptp_pdev->dev;
+
 	err = register_netdev(ndev);
 	if (err)
 		goto err_reg_netdev;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [net-next 6/6] enetc: support PTP domain timestamp conversion
  2021-05-07  8:57 [net-next 0/6] ptp: support virtual clocks for multiple domains Yangbo Lu
                   ` (4 preceding siblings ...)
  2021-05-07  8:57 ` [net-next 5/6] enetc: store ptp device pointer Yangbo Lu
@ 2021-05-07  8:57 ` Yangbo Lu
  2021-05-08 19:17 ` [net-next 0/6] ptp: support virtual clocks for multiple domains Richard Cochran
  6 siblings, 0 replies; 22+ messages in thread
From: Yangbo Lu @ 2021-05-07  8:57 UTC (permalink / raw)
  To: netdev
  Cc: Yangbo Lu, David S . Miller, Richard Cochran, Claudiu Manoil,
	Jakub Kicinski

Support timestamp conversion to specified PTP domain in PTP packet.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 37 ++++++++++++++++++--
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 3ca93adb9662..e7fb2fae98e0 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -7,6 +7,7 @@
 #include <linux/udp.h>
 #include <linux/vmalloc.h>
 #include <linux/ptp_classify.h>
+#include <linux/ptp_clock_kernel.h>
 #include <net/pkt_sched.h>
 
 static int enetc_num_stack_tx_queues(struct enetc_ndev_priv *priv)
@@ -472,13 +473,36 @@ static void enetc_get_tx_tstamp(struct enetc_hw *hw, union enetc_tx_bd *txbd,
 	*tstamp = (u64)hi << 32 | tstamp_lo;
 }
 
-static void enetc_tstamp_tx(struct sk_buff *skb, u64 tstamp)
+static int enetc_ptp_parse_domain(struct sk_buff *skb, u8 *domain)
+{
+	unsigned int ptp_class;
+	struct ptp_header *hdr;
+
+	ptp_class = ptp_classify_raw(skb);
+	if (ptp_class == PTP_CLASS_NONE)
+		return -EINVAL;
+
+	hdr = ptp_parse_header(skb, ptp_class);
+	if (!hdr)
+		return -EINVAL;
+
+	*domain = hdr->domain_number;
+	return 0;
+}
+
+static void enetc_tstamp_tx(struct enetc_ndev_priv *priv, struct sk_buff *skb,
+			    u64 tstamp)
 {
 	struct skb_shared_hwtstamps shhwtstamps;
+	u64 ts = tstamp;
+	u8 domain;
 
 	if (skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) {
+		if (!enetc_ptp_parse_domain(skb, &domain))
+			ptp_clock_domain_tstamp(priv->ptp_dev, &ts, domain);
+
 		memset(&shhwtstamps, 0, sizeof(shhwtstamps));
-		shhwtstamps.hwtstamp = ns_to_ktime(tstamp);
+		shhwtstamps.hwtstamp = ns_to_ktime(ts);
 		skb_txtime_consumed(skb);
 		skb_tstamp_tx(skb, &shhwtstamps);
 	}
@@ -575,7 +599,7 @@ static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget)
 				 */
 				schedule_work(&priv->tx_onestep_tstamp);
 			} else if (unlikely(do_twostep_tstamp)) {
-				enetc_tstamp_tx(skb, tstamp);
+				enetc_tstamp_tx(priv, skb, tstamp);
 				do_twostep_tstamp = false;
 			}
 			napi_consume_skb(skb, napi_budget);
@@ -698,6 +722,7 @@ static void enetc_get_rx_tstamp(struct net_device *ndev,
 	struct enetc_hw *hw = &priv->si->hw;
 	u32 lo, hi, tstamp_lo;
 	u64 tstamp;
+	u8 domain;
 
 	if (le16_to_cpu(rxbd->r.flags) & ENETC_RXBD_FLAG_TSTMP) {
 		lo = enetc_rd_reg_hot(hw->reg + ENETC_SICTR0);
@@ -708,6 +733,12 @@ static void enetc_get_rx_tstamp(struct net_device *ndev,
 			hi -= 1;
 
 		tstamp = (u64)hi << 32 | tstamp_lo;
+
+		skb_reset_mac_header(skb);
+
+		if (!enetc_ptp_parse_domain(skb, &domain))
+			ptp_clock_domain_tstamp(priv->ptp_dev, &tstamp, domain);
+
 		memset(shhwtstamps, 0, sizeof(*shhwtstamps));
 		shhwtstamps->hwtstamp = ns_to_ktime(tstamp);
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [net-next 1/6] ptp: add ptp virtual clock driver framework
  2021-05-07  8:57 ` [net-next 1/6] ptp: add ptp virtual clock driver framework Yangbo Lu
@ 2021-05-07 10:42     ` kernel test robot
  2021-05-07 11:26     ` kernel test robot
  1 sibling, 0 replies; 22+ messages in thread
From: kernel test robot @ 2021-05-07 10:42 UTC (permalink / raw)
  To: Yangbo Lu, netdev
  Cc: kbuild-all, Yangbo Lu, David S . Miller, Richard Cochran,
	Claudiu Manoil, Jakub Kicinski

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

Hi Yangbo,

I love your patch! Yet something to improve:

[auto build test ERROR on 9d31d2338950293ec19d9b095fbaa9030899dcb4]

url:    https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
base:   9d31d2338950293ec19d9b095fbaa9030899dcb4
config: microblaze-randconfig-s032-20210507 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/1f46e22fa0f24ac9acde10ca897266e0bac0f367
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
        git checkout 1f46e22fa0f24ac9acde10ca897266e0bac0f367
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=microblaze 

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

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/spi/spi.h:17,
                    from include/linux/iio/common/st_sensors.h:14,
                    from include/linux/iio/common/st_sensors_i2c.h:14,
                    from drivers/iio/common/st_sensors/st_sensors_i2c.c:16:
>> include/linux/ptp_clock_kernel.h:71:22: error: field 'cc' has incomplete type
      71 |  struct cyclecounter cc;
         |                      ^~
>> include/linux/ptp_clock_kernel.h:356:1: error: expected identifier or '(' before '{' token
     356 | { return NULL; }
         | ^
>> include/linux/ptp_clock_kernel.h:358:6: warning: no previous prototype for 'ptp_clock_domain_tstamp' [-Wmissing-prototypes]
     358 | void ptp_clock_domain_tstamp(struct device *dev, u64 *tstamp, u8 domain)
         |      ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/ptp_clock_kernel.h:355:38: warning: 'ptp_get_pclock_info' declared 'static' but never defined [-Wunused-function]
     355 | static inline struct ptp_clock_info *ptp_get_pclock_info(const struct cyclecounter *cc);
         |                                      ^~~~~~~~~~~~~~~~~~~


vim +/cc +71 include/linux/ptp_clock_kernel.h

    50	
    51	/**
    52	 * struct ptp_vclock_cc - ptp virtual clock cycle counter info
    53	 *
    54	 * @cc:               cyclecounter structure
    55	 * @refresh_interval: time interval to refresh time counter, to avoid 64-bit
    56	 *                    overflow during delta conversion. For example, with
    57	 *                    cc.mult value 2^28,  there are 36 bits left of cycle
    58	 *                    counter. With 1 ns counter resolution, the overflow time
    59	 *                    is 2^36 ns which is 68.7 s. The refresh_interval may be
    60	 *                    (60 * HZ) less than 68.7 s.
    61	 * @mult_num:         parameter for cc.mult adjustment calculation, see below
    62	 * @mult_dem:         parameter for cc.mult adjustment calculation, see below
    63	 *
    64	 * scaled_ppm to adjustment(mult_adj) of cc.mult
    65	 *
    66	 * mult_adj = mult * (ppb / 10^9)
    67	 *          = mult * (scaled_ppm * 1000 / 2^16) / 10^9
    68	 *          = scaled_ppm * mult_num / mult_dem
    69	 */
    70	struct ptp_vclock_cc {
  > 71		struct cyclecounter cc;
    72		unsigned long refresh_interval;
    73		u32 mult_num;
    74		u32 mult_dem;
    75	};
    76	

---
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: 23499 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [net-next 1/6] ptp: add ptp virtual clock driver framework
@ 2021-05-07 10:42     ` kernel test robot
  0 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2021-05-07 10:42 UTC (permalink / raw)
  To: kbuild-all

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

Hi Yangbo,

I love your patch! Yet something to improve:

[auto build test ERROR on 9d31d2338950293ec19d9b095fbaa9030899dcb4]

url:    https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
base:   9d31d2338950293ec19d9b095fbaa9030899dcb4
config: microblaze-randconfig-s032-20210507 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/1f46e22fa0f24ac9acde10ca897266e0bac0f367
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
        git checkout 1f46e22fa0f24ac9acde10ca897266e0bac0f367
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=microblaze 

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

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/spi/spi.h:17,
                    from include/linux/iio/common/st_sensors.h:14,
                    from include/linux/iio/common/st_sensors_i2c.h:14,
                    from drivers/iio/common/st_sensors/st_sensors_i2c.c:16:
>> include/linux/ptp_clock_kernel.h:71:22: error: field 'cc' has incomplete type
      71 |  struct cyclecounter cc;
         |                      ^~
>> include/linux/ptp_clock_kernel.h:356:1: error: expected identifier or '(' before '{' token
     356 | { return NULL; }
         | ^
>> include/linux/ptp_clock_kernel.h:358:6: warning: no previous prototype for 'ptp_clock_domain_tstamp' [-Wmissing-prototypes]
     358 | void ptp_clock_domain_tstamp(struct device *dev, u64 *tstamp, u8 domain)
         |      ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/ptp_clock_kernel.h:355:38: warning: 'ptp_get_pclock_info' declared 'static' but never defined [-Wunused-function]
     355 | static inline struct ptp_clock_info *ptp_get_pclock_info(const struct cyclecounter *cc);
         |                                      ^~~~~~~~~~~~~~~~~~~


vim +/cc +71 include/linux/ptp_clock_kernel.h

    50	
    51	/**
    52	 * struct ptp_vclock_cc - ptp virtual clock cycle counter info
    53	 *
    54	 * @cc:               cyclecounter structure
    55	 * @refresh_interval: time interval to refresh time counter, to avoid 64-bit
    56	 *                    overflow during delta conversion. For example, with
    57	 *                    cc.mult value 2^28,  there are 36 bits left of cycle
    58	 *                    counter. With 1 ns counter resolution, the overflow time
    59	 *                    is 2^36 ns which is 68.7 s. The refresh_interval may be
    60	 *                    (60 * HZ) less than 68.7 s.
    61	 * @mult_num:         parameter for cc.mult adjustment calculation, see below
    62	 * @mult_dem:         parameter for cc.mult adjustment calculation, see below
    63	 *
    64	 * scaled_ppm to adjustment(mult_adj) of cc.mult
    65	 *
    66	 * mult_adj = mult * (ppb / 10^9)
    67	 *          = mult * (scaled_ppm * 1000 / 2^16) / 10^9
    68	 *          = scaled_ppm * mult_num / mult_dem
    69	 */
    70	struct ptp_vclock_cc {
  > 71		struct cyclecounter cc;
    72		unsigned long refresh_interval;
    73		u32 mult_num;
    74		u32 mult_dem;
    75	};
    76	

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

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [net-next 1/6] ptp: add ptp virtual clock driver framework
  2021-05-07  8:57 ` [net-next 1/6] ptp: add ptp virtual clock driver framework Yangbo Lu
@ 2021-05-07 11:26     ` kernel test robot
  2021-05-07 11:26     ` kernel test robot
  1 sibling, 0 replies; 22+ messages in thread
From: kernel test robot @ 2021-05-07 11:26 UTC (permalink / raw)
  To: Yangbo Lu, netdev
  Cc: kbuild-all, Yangbo Lu, David S . Miller, Richard Cochran,
	Claudiu Manoil, Jakub Kicinski

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

Hi Yangbo,

I love your patch! Yet something to improve:

[auto build test ERROR on 9d31d2338950293ec19d9b095fbaa9030899dcb4]

url:    https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
base:   9d31d2338950293ec19d9b095fbaa9030899dcb4
config: nds32-randconfig-r012-20210507 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1f46e22fa0f24ac9acde10ca897266e0bac0f367
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
        git checkout 1f46e22fa0f24ac9acde10ca897266e0bac0f367
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=nds32 

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

All errors (new ones prefixed by >>):

   In file included from drivers/ptp/ptp_private.h:16,
                    from drivers/ptp/ptp_clock.c:20:
>> include/linux/ptp_clock_kernel.h:71:22: error: field 'cc' has incomplete type
      71 |  struct cyclecounter cc;
         |                      ^~
   In file included from drivers/ptp/ptp_clock.c:20:
>> drivers/ptp/ptp_private.h:59:22: error: field 'cc' has incomplete type
      59 |  struct cyclecounter cc;
         |                      ^~
>> drivers/ptp/ptp_private.h:60:21: error: field 'tc' has incomplete type
      60 |  struct timecounter tc;
         |                     ^~
--
   In file included from drivers/ptp/ptp_private.h:16,
                    from drivers/ptp/ptp_vclock.c:8:
>> include/linux/ptp_clock_kernel.h:71:22: error: field 'cc' has incomplete type
      71 |  struct cyclecounter cc;
         |                      ^~
   In file included from drivers/ptp/ptp_vclock.c:8:
>> drivers/ptp/ptp_private.h:59:22: error: field 'cc' has incomplete type
      59 |  struct cyclecounter cc;
         |                      ^~
>> drivers/ptp/ptp_private.h:60:21: error: field 'tc' has incomplete type
      60 |  struct timecounter tc;
         |                     ^~
   drivers/ptp/ptp_vclock.c: In function 'ptp_vclock_adjfine':
>> drivers/ptp/ptp_vclock.c:20:2: error: implicit declaration of function 'timecounter_read'; did you mean 'refcount_read'? [-Werror=implicit-function-declaration]
      20 |  timecounter_read(&vclock->tc);
         |  ^~~~~~~~~~~~~~~~
         |  refcount_read
   drivers/ptp/ptp_vclock.c: In function 'ptp_vclock_adjtime':
>> drivers/ptp/ptp_vclock.c:33:2: error: implicit declaration of function 'timecounter_adjtime' [-Werror=implicit-function-declaration]
      33 |  timecounter_adjtime(&vclock->tc, delta);
         |  ^~~~~~~~~~~~~~~~~~~
   drivers/ptp/ptp_vclock.c: In function 'ptp_vclock_settime':
>> drivers/ptp/ptp_vclock.c:62:2: error: implicit declaration of function 'timecounter_init'; did you mean 'timerqueue_init'? [-Werror=implicit-function-declaration]
      62 |  timecounter_init(&vclock->tc, &vclock->cc, ns);
         |  ^~~~~~~~~~~~~~~~
         |  timerqueue_init
   drivers/ptp/ptp_vclock.c: In function 'ptp_clock_find_domain_tstamp':
>> drivers/ptp/ptp_vclock.c:103:23: error: implicit declaration of function 'timecounter_cyc2time' [-Werror=implicit-function-declaration]
     103 |   domain_ts->tstamp = timecounter_cyc2time(&vclock->tc, domain_ts->tstamp);
         |                       ^~~~~~~~~~~~~~~~~~~~
   In file included from <command-line>:
   drivers/ptp/ptp_vclock.c: In function 'ptp_get_pclock_info':
>> include/linux/kernel.h:709:32: error: dereferencing pointer to incomplete type 'const struct cyclecounter'
     709 |  BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
         |                                ^~~~~~
   include/linux/compiler_types.h:308:9: note: in definition of macro '__compiletime_assert'
     308 |   if (!(condition))     \
         |         ^~~~~~~~~
   include/linux/compiler_types.h:328:2: note: in expansion of macro '_compiletime_assert'
     328 |  _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |  ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:709:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     709 |  BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
         |  ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:709:20: note: in expansion of macro '__same_type'
     709 |  BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
         |                    ^~~~~~~~~~~
   drivers/ptp/ptp_private.h:51:25: note: in expansion of macro 'container_of'
      51 | #define cc_to_vclock(d) container_of((d), struct ptp_vclock, cc)
         |                         ^~~~~~~~~~~~
   drivers/ptp/ptp_vclock.c:126:30: note: in expansion of macro 'cc_to_vclock'
     126 |  struct ptp_vclock *vclock = cc_to_vclock(cc);
         |                              ^~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   In file included from include/linux/spi/spi.h:17,
                    from drivers/iio/accel/adxl372.c:14:
>> include/linux/ptp_clock_kernel.h:71:22: error: field 'cc' has incomplete type
      71 |  struct cyclecounter cc;
         |                      ^~


vim +/cc +71 include/linux/ptp_clock_kernel.h

    50	
    51	/**
    52	 * struct ptp_vclock_cc - ptp virtual clock cycle counter info
    53	 *
    54	 * @cc:               cyclecounter structure
    55	 * @refresh_interval: time interval to refresh time counter, to avoid 64-bit
    56	 *                    overflow during delta conversion. For example, with
    57	 *                    cc.mult value 2^28,  there are 36 bits left of cycle
    58	 *                    counter. With 1 ns counter resolution, the overflow time
    59	 *                    is 2^36 ns which is 68.7 s. The refresh_interval may be
    60	 *                    (60 * HZ) less than 68.7 s.
    61	 * @mult_num:         parameter for cc.mult adjustment calculation, see below
    62	 * @mult_dem:         parameter for cc.mult adjustment calculation, see below
    63	 *
    64	 * scaled_ppm to adjustment(mult_adj) of cc.mult
    65	 *
    66	 * mult_adj = mult * (ppb / 10^9)
    67	 *          = mult * (scaled_ppm * 1000 / 2^16) / 10^9
    68	 *          = scaled_ppm * mult_num / mult_dem
    69	 */
    70	struct ptp_vclock_cc {
  > 71		struct cyclecounter cc;
    72		unsigned long refresh_interval;
    73		u32 mult_num;
    74		u32 mult_dem;
    75	};
    76	

---
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: 25430 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [net-next 1/6] ptp: add ptp virtual clock driver framework
@ 2021-05-07 11:26     ` kernel test robot
  0 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2021-05-07 11:26 UTC (permalink / raw)
  To: kbuild-all

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

Hi Yangbo,

I love your patch! Yet something to improve:

[auto build test ERROR on 9d31d2338950293ec19d9b095fbaa9030899dcb4]

url:    https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
base:   9d31d2338950293ec19d9b095fbaa9030899dcb4
config: nds32-randconfig-r012-20210507 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1f46e22fa0f24ac9acde10ca897266e0bac0f367
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
        git checkout 1f46e22fa0f24ac9acde10ca897266e0bac0f367
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=nds32 

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

All errors (new ones prefixed by >>):

   In file included from drivers/ptp/ptp_private.h:16,
                    from drivers/ptp/ptp_clock.c:20:
>> include/linux/ptp_clock_kernel.h:71:22: error: field 'cc' has incomplete type
      71 |  struct cyclecounter cc;
         |                      ^~
   In file included from drivers/ptp/ptp_clock.c:20:
>> drivers/ptp/ptp_private.h:59:22: error: field 'cc' has incomplete type
      59 |  struct cyclecounter cc;
         |                      ^~
>> drivers/ptp/ptp_private.h:60:21: error: field 'tc' has incomplete type
      60 |  struct timecounter tc;
         |                     ^~
--
   In file included from drivers/ptp/ptp_private.h:16,
                    from drivers/ptp/ptp_vclock.c:8:
>> include/linux/ptp_clock_kernel.h:71:22: error: field 'cc' has incomplete type
      71 |  struct cyclecounter cc;
         |                      ^~
   In file included from drivers/ptp/ptp_vclock.c:8:
>> drivers/ptp/ptp_private.h:59:22: error: field 'cc' has incomplete type
      59 |  struct cyclecounter cc;
         |                      ^~
>> drivers/ptp/ptp_private.h:60:21: error: field 'tc' has incomplete type
      60 |  struct timecounter tc;
         |                     ^~
   drivers/ptp/ptp_vclock.c: In function 'ptp_vclock_adjfine':
>> drivers/ptp/ptp_vclock.c:20:2: error: implicit declaration of function 'timecounter_read'; did you mean 'refcount_read'? [-Werror=implicit-function-declaration]
      20 |  timecounter_read(&vclock->tc);
         |  ^~~~~~~~~~~~~~~~
         |  refcount_read
   drivers/ptp/ptp_vclock.c: In function 'ptp_vclock_adjtime':
>> drivers/ptp/ptp_vclock.c:33:2: error: implicit declaration of function 'timecounter_adjtime' [-Werror=implicit-function-declaration]
      33 |  timecounter_adjtime(&vclock->tc, delta);
         |  ^~~~~~~~~~~~~~~~~~~
   drivers/ptp/ptp_vclock.c: In function 'ptp_vclock_settime':
>> drivers/ptp/ptp_vclock.c:62:2: error: implicit declaration of function 'timecounter_init'; did you mean 'timerqueue_init'? [-Werror=implicit-function-declaration]
      62 |  timecounter_init(&vclock->tc, &vclock->cc, ns);
         |  ^~~~~~~~~~~~~~~~
         |  timerqueue_init
   drivers/ptp/ptp_vclock.c: In function 'ptp_clock_find_domain_tstamp':
>> drivers/ptp/ptp_vclock.c:103:23: error: implicit declaration of function 'timecounter_cyc2time' [-Werror=implicit-function-declaration]
     103 |   domain_ts->tstamp = timecounter_cyc2time(&vclock->tc, domain_ts->tstamp);
         |                       ^~~~~~~~~~~~~~~~~~~~
   In file included from <command-line>:
   drivers/ptp/ptp_vclock.c: In function 'ptp_get_pclock_info':
>> include/linux/kernel.h:709:32: error: dereferencing pointer to incomplete type 'const struct cyclecounter'
     709 |  BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
         |                                ^~~~~~
   include/linux/compiler_types.h:308:9: note: in definition of macro '__compiletime_assert'
     308 |   if (!(condition))     \
         |         ^~~~~~~~~
   include/linux/compiler_types.h:328:2: note: in expansion of macro '_compiletime_assert'
     328 |  _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |  ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:709:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     709 |  BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
         |  ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:709:20: note: in expansion of macro '__same_type'
     709 |  BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
         |                    ^~~~~~~~~~~
   drivers/ptp/ptp_private.h:51:25: note: in expansion of macro 'container_of'
      51 | #define cc_to_vclock(d) container_of((d), struct ptp_vclock, cc)
         |                         ^~~~~~~~~~~~
   drivers/ptp/ptp_vclock.c:126:30: note: in expansion of macro 'cc_to_vclock'
     126 |  struct ptp_vclock *vclock = cc_to_vclock(cc);
         |                              ^~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   In file included from include/linux/spi/spi.h:17,
                    from drivers/iio/accel/adxl372.c:14:
>> include/linux/ptp_clock_kernel.h:71:22: error: field 'cc' has incomplete type
      71 |  struct cyclecounter cc;
         |                      ^~


vim +/cc +71 include/linux/ptp_clock_kernel.h

    50	
    51	/**
    52	 * struct ptp_vclock_cc - ptp virtual clock cycle counter info
    53	 *
    54	 * @cc:               cyclecounter structure
    55	 * @refresh_interval: time interval to refresh time counter, to avoid 64-bit
    56	 *                    overflow during delta conversion. For example, with
    57	 *                    cc.mult value 2^28,  there are 36 bits left of cycle
    58	 *                    counter. With 1 ns counter resolution, the overflow time
    59	 *                    is 2^36 ns which is 68.7 s. The refresh_interval may be
    60	 *                    (60 * HZ) less than 68.7 s.
    61	 * @mult_num:         parameter for cc.mult adjustment calculation, see below
    62	 * @mult_dem:         parameter for cc.mult adjustment calculation, see below
    63	 *
    64	 * scaled_ppm to adjustment(mult_adj) of cc.mult
    65	 *
    66	 * mult_adj = mult * (ppb / 10^9)
    67	 *          = mult * (scaled_ppm * 1000 / 2^16) / 10^9
    68	 *          = scaled_ppm * mult_num / mult_dem
    69	 */
    70	struct ptp_vclock_cc {
  > 71		struct cyclecounter cc;
    72		unsigned long refresh_interval;
    73		u32 mult_num;
    74		u32 mult_dem;
    75	};
    76	

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

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [net-next 4/6] enetc_ptp: support ptp virtual clock
  2021-05-07  8:57 ` [net-next 4/6] enetc_ptp: support ptp virtual clock Yangbo Lu
@ 2021-05-07 19:58   ` Claudiu Manoil
  2021-05-07 20:50     ` kernel test robot
  1 sibling, 0 replies; 22+ messages in thread
From: Claudiu Manoil @ 2021-05-07 19:58 UTC (permalink / raw)
  To: Yangbo Lu, netdev
  Cc: David S . Miller, Richard Cochran, Claudiu Manoil, Jakub Kicinski

On 07.05.2021 11:57, Yangbo Lu wrote:
> Add support for ptp virtual clock.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
>   drivers/net/ethernet/freescale/enetc/enetc_ptp.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ptp.c b/drivers/net/ethernet/freescale/enetc/enetc_ptp.c
> index bc594892507a..52de736df800 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_ptp.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_ptp.c
> @@ -10,6 +10,16 @@
>   int enetc_phc_index = -1;
>   EXPORT_SYMBOL(enetc_phc_index);
>   
> +struct ptp_vclock_cc ptp_qoriq_vclock_cc = {
^ static

> +	.cc.read		= ptp_qoriq_clock_read,
> +	.cc.mask		= CYCLECOUNTER_MASK(64),
> +	.cc.shift		= 28,
> +	.cc.mult		= (1 << 28),
> +	.refresh_interval	= (HZ * 60),
> +	.mult_num		= (1 << 6),
> +	.mult_dem		= 15625,
> +};
> +

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [net-next 4/6] enetc_ptp: support ptp virtual clock
  2021-05-07  8:57 ` [net-next 4/6] enetc_ptp: support ptp virtual clock Yangbo Lu
@ 2021-05-07 20:50     ` kernel test robot
  2021-05-07 20:50     ` kernel test robot
  1 sibling, 0 replies; 22+ messages in thread
From: kernel test robot @ 2021-05-07 20:50 UTC (permalink / raw)
  To: Yangbo Lu, netdev
  Cc: kbuild-all, Yangbo Lu, David S . Miller, Richard Cochran,
	Claudiu Manoil, Jakub Kicinski

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

Hi Yangbo,

I love your patch! Yet something to improve:

[auto build test ERROR on 9d31d2338950293ec19d9b095fbaa9030899dcb4]

url:    https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
base:   9d31d2338950293ec19d9b095fbaa9030899dcb4
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/2df0961abd74b90986d36beeaf4116ed699f232e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
        git checkout 2df0961abd74b90986d36beeaf4116ed699f232e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=mips 

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

All errors (new ones prefixed by >>):

   In file included from include/linux/fsl/ptp_qoriq.h:11,
                    from drivers/net/ethernet/freescale/enetc/enetc_ptp.c:6:
   include/linux/ptp_clock_kernel.h:71:22: error: field 'cc' has incomplete type
      71 |  struct cyclecounter cc;
         |                      ^~
>> drivers/net/ethernet/freescale/enetc/enetc_ptp.c:14:2: error: field name not in record or union initializer
      14 |  .cc.read  = ptp_qoriq_clock_read,
         |  ^
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:14:2: note: (near initialization for 'ptp_qoriq_vclock_cc')
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:15:2: error: field name not in record or union initializer
      15 |  .cc.mask  = CYCLECOUNTER_MASK(64),
         |  ^
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:15:2: note: (near initialization for 'ptp_qoriq_vclock_cc')
>> drivers/net/ethernet/freescale/enetc/enetc_ptp.c:15:14: error: implicit declaration of function 'CYCLECOUNTER_MASK' [-Werror=implicit-function-declaration]
      15 |  .cc.mask  = CYCLECOUNTER_MASK(64),
         |              ^~~~~~~~~~~~~~~~~
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:16:2: error: field name not in record or union initializer
      16 |  .cc.shift  = 28,
         |  ^
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:16:2: note: (near initialization for 'ptp_qoriq_vclock_cc')
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:17:2: error: field name not in record or union initializer
      17 |  .cc.mult  = (1 << 28),
         |  ^
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:17:2: note: (near initialization for 'ptp_qoriq_vclock_cc')
   cc1: some warnings being treated as errors


vim +14 drivers/net/ethernet/freescale/enetc/enetc_ptp.c

    12	
    13	struct ptp_vclock_cc ptp_qoriq_vclock_cc = {
  > 14		.cc.read		= ptp_qoriq_clock_read,
  > 15		.cc.mask		= CYCLECOUNTER_MASK(64),
    16		.cc.shift		= 28,
    17		.cc.mult		= (1 << 28),
    18		.refresh_interval	= (HZ * 60),
    19		.mult_num		= (1 << 6),
    20		.mult_dem		= 15625,
    21	};
    22	

---
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: 70760 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [net-next 4/6] enetc_ptp: support ptp virtual clock
@ 2021-05-07 20:50     ` kernel test robot
  0 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2021-05-07 20:50 UTC (permalink / raw)
  To: kbuild-all

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

Hi Yangbo,

I love your patch! Yet something to improve:

[auto build test ERROR on 9d31d2338950293ec19d9b095fbaa9030899dcb4]

url:    https://github.com/0day-ci/linux/commits/Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
base:   9d31d2338950293ec19d9b095fbaa9030899dcb4
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/2df0961abd74b90986d36beeaf4116ed699f232e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yangbo-Lu/ptp-support-virtual-clocks-for-multiple-domains/20210507-164927
        git checkout 2df0961abd74b90986d36beeaf4116ed699f232e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=mips 

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

All errors (new ones prefixed by >>):

   In file included from include/linux/fsl/ptp_qoriq.h:11,
                    from drivers/net/ethernet/freescale/enetc/enetc_ptp.c:6:
   include/linux/ptp_clock_kernel.h:71:22: error: field 'cc' has incomplete type
      71 |  struct cyclecounter cc;
         |                      ^~
>> drivers/net/ethernet/freescale/enetc/enetc_ptp.c:14:2: error: field name not in record or union initializer
      14 |  .cc.read  = ptp_qoriq_clock_read,
         |  ^
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:14:2: note: (near initialization for 'ptp_qoriq_vclock_cc')
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:15:2: error: field name not in record or union initializer
      15 |  .cc.mask  = CYCLECOUNTER_MASK(64),
         |  ^
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:15:2: note: (near initialization for 'ptp_qoriq_vclock_cc')
>> drivers/net/ethernet/freescale/enetc/enetc_ptp.c:15:14: error: implicit declaration of function 'CYCLECOUNTER_MASK' [-Werror=implicit-function-declaration]
      15 |  .cc.mask  = CYCLECOUNTER_MASK(64),
         |              ^~~~~~~~~~~~~~~~~
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:16:2: error: field name not in record or union initializer
      16 |  .cc.shift  = 28,
         |  ^
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:16:2: note: (near initialization for 'ptp_qoriq_vclock_cc')
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:17:2: error: field name not in record or union initializer
      17 |  .cc.mult  = (1 << 28),
         |  ^
   drivers/net/ethernet/freescale/enetc/enetc_ptp.c:17:2: note: (near initialization for 'ptp_qoriq_vclock_cc')
   cc1: some warnings being treated as errors


vim +14 drivers/net/ethernet/freescale/enetc/enetc_ptp.c

    12	
    13	struct ptp_vclock_cc ptp_qoriq_vclock_cc = {
  > 14		.cc.read		= ptp_qoriq_clock_read,
  > 15		.cc.mask		= CYCLECOUNTER_MASK(64),
    16		.cc.shift		= 28,
    17		.cc.mult		= (1 << 28),
    18		.refresh_interval	= (HZ * 60),
    19		.mult_num		= (1 << 6),
    20		.mult_dem		= 15625,
    21	};
    22	

---
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: 70760 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [net-next 0/6] ptp: support virtual clocks for multiple domains
  2021-05-07  8:57 [net-next 0/6] ptp: support virtual clocks for multiple domains Yangbo Lu
                   ` (5 preceding siblings ...)
  2021-05-07  8:57 ` [net-next 6/6] enetc: support PTP domain timestamp conversion Yangbo Lu
@ 2021-05-08 19:17 ` Richard Cochran
  2021-05-10  3:04   ` Y.b. Lu
  6 siblings, 1 reply; 22+ messages in thread
From: Richard Cochran @ 2021-05-08 19:17 UTC (permalink / raw)
  To: Yangbo Lu; +Cc: netdev, David S . Miller, Claudiu Manoil, Jakub Kicinski

On Fri, May 07, 2021 at 04:57:50PM +0800, Yangbo Lu wrote:
>   ptp4l -i eno0 -p/dev/ptp1 -m --domainNumber=1 --priority1=128 > domain1-slave.log 2>&1 &
>   ptp4l -i eno0 -p/dev/ptp2 -m --domainNumber=2 --priority1=128 > domain2-slave.log 2>&1 &
>   ptp4l -i eno0 -p/dev/ptp3 -m --domainNumber=3 --priority1=127 > domain3-master.log 2>&1 &

> - Make changing on physical clock transparent to virtual clocks.
>   The virtual clock is based on free running physical clock. If physical
>   clock has to be changed, how to make the change transparent to all
>   virtual clocks?

Yes, this is a serious defect of this patch series, and there is no
way to fix it.  In the above example, suppose that domainNumber 1
needs +11 ppm and domainNumber 2 needs -12 ppm.  You can't adjust one
clock in two different ways.

>   Actually the frequency adjustmend on physical clock
>   may be hidden by updating virtual clocks in opposite direction at same
>   time. Considering the ppb values adjusted, the code execution delay
>   will be little enough to ignore.

Assuming that the frequency offset is exactly the same on all domains,
which will very often be false.

>   But it's hard to hide clock stepping,
>   by now I think the workaround may be inhibiting physical clock stepping
>   when run user space ptp application.

That won't work either, because a phase offset on one domain will
result in a large slew at the maximum rate, but that rate would spoil
the other domains.

The best way to support multiple PTP domains simultaneously
is in the application.  It is really the only way, because the kernel
does not handle any details of the PTP, like domainNumber.  The kernel
only provides clock control and packet time stamping.

ptp4l does not handle multiple domains today, but it definitely could
be added with some effort.  It would have to synchronize the clock to
one chosen domain, and track the phase and frequency offsets of each
of the other domains with respect to the chosen domain.  Having done
this, the software can convert time stamps between the domains
perfectly.  Using the tracked phase and frequency offsets, it can also
switch domains seamlessly without hacks or guesswork.

So I have to say NAK to this series because it can't do any of that,
and it cannot be made to work either.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [net-next 0/6] ptp: support virtual clocks for multiple domains
  2021-05-08 19:17 ` [net-next 0/6] ptp: support virtual clocks for multiple domains Richard Cochran
@ 2021-05-10  3:04   ` Y.b. Lu
  2021-05-10 23:18     ` Richard Cochran
  0 siblings, 1 reply; 22+ messages in thread
From: Y.b. Lu @ 2021-05-10  3:04 UTC (permalink / raw)
  To: Richard Cochran; +Cc: netdev, David S . Miller, Claudiu Manoil, Jakub Kicinski

Hi Richard,

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: 2021年5月9日 3:17
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; Claudiu
> Manoil <claudiu.manoil@nxp.com>; Jakub Kicinski <kuba@kernel.org>
> Subject: Re: [net-next 0/6] ptp: support virtual clocks for multiple domains
> 
> On Fri, May 07, 2021 at 04:57:50PM +0800, Yangbo Lu wrote:
> >   ptp4l -i eno0 -p/dev/ptp1 -m --domainNumber=1 --priority1=128 >
> domain1-slave.log 2>&1 &
> >   ptp4l -i eno0 -p/dev/ptp2 -m --domainNumber=2 --priority1=128 >
> domain2-slave.log 2>&1 &
> >   ptp4l -i eno0 -p/dev/ptp3 -m --domainNumber=3 --priority1=127 >
> > domain3-master.log 2>&1 &
> 
> > - Make changing on physical clock transparent to virtual clocks.
> >   The virtual clock is based on free running physical clock. If physical
> >   clock has to be changed, how to make the change transparent to all
> >   virtual clocks?
> 
> Yes, this is a serious defect of this patch series, and there is no way to fix it.  In
> the above example, suppose that domainNumber 1 needs +11 ppm and
> domainNumber 2 needs -12 ppm.  You can't adjust one clock in two different
> ways.

There may be some misunderstanding. In the example, domain 1, 2, 3 are based on PTP virtual clocks ptp1, ptp2 and ptp3 which are utilizing their own timecounter.
The clock adjustment on them won't affect each other. The example worked fine in my verification.

> 
> >   Actually the frequency adjustmend on physical clock
> >   may be hidden by updating virtual clocks in opposite direction at same
> >   time. Considering the ppb values adjusted, the code execution delay
> >   will be little enough to ignore.
> 
> Assuming that the frequency offset is exactly the same on all domains, which
> will very often be false.

I mean if the physical clock keeps free running, all virtual clocks utilizing their own timercounter can work fine independently without affecting on each other.
If the physical clock has change on frequency, there is also way to make the change not affect virtual clocks.
For example, when there is +12 ppm change on physical clock, just give -12 ppm change on virtual clocks.
And the code execution delay just has very little affecting on the time error considering only 12 ppm frequency adjusted.
This is a TODO work.

> 
> >   But it's hard to hide clock stepping,
> >   by now I think the workaround may be inhibiting physical clock stepping
> >   when run user space ptp application.
> 
> That won't work either, because a phase offset on one domain will result in a
> large slew at the maximum rate, but that rate would spoil the other domains.

Phase offset changing on physical clock affects virtual clocks. This is not able to address.
But there is workaround, like we inhibit physical clock stepping during PTP application running.

> 
> The best way to support multiple PTP domains simultaneously is in the
> application.  It is really the only way, because the kernel does not handle any
> details of the PTP, like domainNumber.  The kernel only provides clock control
> and packet time stamping.

I understand. I explain my idea above to make it clear avoid misunderstanding.
This patch-set focuses on clock control and timestamping. It provides support for virtual clocks and domain timestamp related.
Put these in kernel, PTP applications don't have to do such thing by each of them.

> 
> ptp4l does not handle multiple domains today, but it definitely could be added
> with some effort.  It would have to synchronize the clock to one chosen
> domain, and track the phase and frequency offsets of each of the other
> domains with respect to the chosen domain.  Having done this, the software
> can convert time stamps between the domains perfectly.  Using the tracked
> phase and frequency offsets, it can also switch domains seamlessly without
> hacks or guesswork.

Sure. This can be done in application. I'm not sure whether my explaining address the "defect" you mentioned :)
I hope yes. Actually this patch-set can provide ptp1, ptp2, ... , which are standard ptp devices with domains bound.
ptp4l can directly use them for domain clock controls. The timestamp is already converted to domain in kernel.
This benefits all PTP applications.

> 
> So I have to say NAK to this series because it can't do any of that, and it cannot
> be made to work either.

There may be misunderstanding.
Hope for comments after seeing my explaining:)

Thank you.

> 
> Thanks,
> Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [net-next 0/6] ptp: support virtual clocks for multiple domains
  2021-05-10  3:04   ` Y.b. Lu
@ 2021-05-10 23:18     ` Richard Cochran
  2021-05-11 10:40       ` Y.b. Lu
  0 siblings, 1 reply; 22+ messages in thread
From: Richard Cochran @ 2021-05-10 23:18 UTC (permalink / raw)
  To: Y.b. Lu; +Cc: netdev, David S . Miller, Claudiu Manoil, Jakub Kicinski

On Mon, May 10, 2021 at 03:04:39AM +0000, Y.b. Lu wrote:

> There may be some misunderstanding. In the example, domain 1, 2, 3 are based on PTP virtual clocks ptp1, ptp2 and ptp3 which are utilizing their own timecounter.
> The clock adjustment on them won't affect each other. The example worked fine in my verification.

Okay, now I think I understand what you are suggesting.  Still, there
are issues you haven't considered.

> I mean if the physical clock keeps free running, all virtual clocks utilizing their own timercounter can work fine independently without affecting on each other.

Right.  This is critical!

> If the physical clock has change on frequency, there is also way to make the change not affect virtual clocks.
> For example, when there is +12 ppm change on physical clock, just give -12 ppm change on virtual clocks.

That will cause issues in very many cases.

For example, what happens when the "real" clock sees a large offset,
and it doesn't step, but rather applies the maximum frequency offset
to slew the clock?  That maximum might be larger that the max possible
in the virtual clocks.  Even with smaller frequency offsets, the
un-synchronized, quasi random changes in the "real" clock will spoil
the virtual clocks.  I won't support such an approach.

However, if the "real" clock is guaranteed to stay free running, and
the virtual clocks give up any hope of producing periodic outputs,
then your idea might work.

Thinking out loud: You could make a sysfs knob that converts a "real"
clock into two or more virtual clocks.  For example:

    cat /sys/class/ptp/ptp0/number_vclocks
    0 # ptp0 is a "real" clock

    echo 3 > /sys/class/ptp/ptp0/number_vclocks
    # This resets the frequency offset to zero and creates three
    # new clocks using timecounter numeric adjustment, ptp0, 1, and 2.
    # ptp0 loses its periodic output abilities.
    # ptp0 is now a virtual clock, just like ptp1 and 2.

    echo 0 > /sys/class/ptp/ptp0/number_vclocks
    # back to normal again.

In addition to that, you will need a way to make the relationships
between the clocks and the network interfaces discoverable.

It needs more thought and careful design, but I think having
clock_gettime() available for the different clocks would be nice to
have for the applications.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [net-next 0/6] ptp: support virtual clocks for multiple domains
  2021-05-10 23:18     ` Richard Cochran
@ 2021-05-11 10:40       ` Y.b. Lu
  2021-05-11 15:49         ` Richard Cochran
  0 siblings, 1 reply; 22+ messages in thread
From: Y.b. Lu @ 2021-05-11 10:40 UTC (permalink / raw)
  To: Richard Cochran; +Cc: netdev, David S . Miller, Claudiu Manoil, Jakub Kicinski

Hi Richard,

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: 2021年5月11日 7:19
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; Claudiu
> Manoil <claudiu.manoil@nxp.com>; Jakub Kicinski <kuba@kernel.org>
> Subject: Re: [net-next 0/6] ptp: support virtual clocks for multiple domains
> 
> On Mon, May 10, 2021 at 03:04:39AM +0000, Y.b. Lu wrote:
> 
> > There may be some misunderstanding. In the example, domain 1, 2, 3 are
> based on PTP virtual clocks ptp1, ptp2 and ptp3 which are utilizing their own
> timecounter.
> > The clock adjustment on them won't affect each other. The example worked
> fine in my verification.
> 
> Okay, now I think I understand what you are suggesting.  Still, there are issues
> you haven't considered.
> 
> > I mean if the physical clock keeps free running, all virtual clocks utilizing their
> own timercounter can work fine independently without affecting on each
> other.
> 
> Right.  This is critical!
> 
> > If the physical clock has change on frequency, there is also way to make the
> change not affect virtual clocks.
> > For example, when there is +12 ppm change on physical clock, just give -12
> ppm change on virtual clocks.
> 
> That will cause issues in very many cases.
> 
> For example, what happens when the "real" clock sees a large offset, and it
> doesn't step, but rather applies the maximum frequency offset to slew the
> clock?  That maximum might be larger that the max possible in the virtual
> clocks.  Even with smaller frequency offsets, the un-synchronized, quasi
> random changes in the "real" clock will spoil the virtual clocks.  I won't
> support such an approach.
> 

What I thought was in code writing registers to adjust physical clock frequency, and immediately adjusting virtual clocks in opposite direction.
Make the operations atomic by locking. Assume the code execution has a DELAY, and the frequency adjusted is PPM.
Then the time error affecting on virtual clock will be DELAY * PPM. I'm not sure what the DELAY value will be on other platforms.
Just for example, for 1us delay, 1000ppm adjustment will have 1ns time error.

But indeed, this approach may be not feasible as you said. Especially it is adjusting clock in max frequency, and there are many virtual clocks.
The time error may be large enough to cause issues. (I'm not sure whether I understand you correctly, sorry.)

So, a question is, for hardware which supports only one PTP clock, can multiple domains be supported where physical clock also participates in synchronization of a domain?
(Because sometime the physical clock is required to be synchronized for TSN using, or other usages.)
Do you think it's possible?

> However, if the "real" clock is guaranteed to stay free running, and the virtual
> clocks give up any hope of producing periodic outputs, then your idea might
> work.
> 
> Thinking out loud: You could make a sysfs knob that converts a "real"
> clock into two or more virtual clocks.  For example:
> 
>     cat /sys/class/ptp/ptp0/number_vclocks
>     0 # ptp0 is a "real" clock
> 
>     echo 3 > /sys/class/ptp/ptp0/number_vclocks
>     # This resets the frequency offset to zero and creates three
>     # new clocks using timecounter numeric adjustment, ptp0, 1, and 2.
>     # ptp0 loses its periodic output abilities.
>     # ptp0 is now a virtual clock, just like ptp1 and 2.
> 
>     echo 0 > /sys/class/ptp/ptp0/number_vclocks
>     # back to normal again.
> 
> In addition to that, you will need a way to make the relationships between the
> clocks and the network interfaces discoverable.

Agree. This should be done carefully and everything should be considered.
Will converting physical clock ptp0 to virtual clock ptp0 introduce more effort to implement,
comparing to keep physical clock ptp0 but limit to use it?

> 
> It needs more thought and careful design, but I think having
> clock_gettime() available for the different clocks would be nice to have for the
> applications.

Thank you. Then regarding the domain timestamp, do you think it's proper to do the conversion in kernel as I implemented.

> 
> Thanks,
> Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [net-next 0/6] ptp: support virtual clocks for multiple domains
  2021-05-11 10:40       ` Y.b. Lu
@ 2021-05-11 15:49         ` Richard Cochran
  2021-05-14  6:41           ` Y.b. Lu
  0 siblings, 1 reply; 22+ messages in thread
From: Richard Cochran @ 2021-05-11 15:49 UTC (permalink / raw)
  To: Y.b. Lu; +Cc: netdev, David S . Miller, Claudiu Manoil, Jakub Kicinski

On Tue, May 11, 2021 at 10:40:15AM +0000, Y.b. Lu wrote:
> What I thought was in code writing registers to adjust physical clock frequency, and immediately adjusting virtual clocks in opposite direction.
> Make the operations atomic by locking. Assume the code execution has a DELAY, and the frequency adjusted is PPM.
> Then the time error affecting on virtual clock will be DELAY * PPM. I'm not sure what the DELAY value will be on other platforms.
> Just for example, for 1us delay, 1000ppm adjustment will have 1ns time error.
> 
> But indeed, this approach may be not feasible as you said. Especially it is adjusting clock in max frequency, and there are many virtual clocks.
> The time error may be large enough to cause issues. (I'm not sure whether I understand you correctly, sorry.)

You understand correctly.

> So, a question is, for hardware which supports only one PTP clock, can multiple domains be supported where physical clock also participates in synchronization of a domain?
> (Because sometime the physical clock is required to be synchronized for TSN using, or other usages.)
> Do you think it's possible?

No, it won't work.  You can't adjust both the physical clock and the
timecounters at the same time.  The code would be an awful hack, and
it would not work in all real world circumstances.  If the kernel
offers a new time service, then it has to work always.

So, getting back to my user space idea, it _would_ work to let the
application stack control the HW clock as before, but to track the
other domains numerically.  Then, the other applications could use the
TIME_STATUS_NP management message (designed for use with gPTP and
free_running) to get the current time in the other domains.

So take your pick.  You can't have it both ways, I'm afraid.

> > In addition to that, you will need a way to make the relationships between the
> > clocks and the network interfaces discoverable.
> 
> Agree. This should be done carefully and everything should be considered.
> Will converting physical clock ptp0 to virtual clock ptp0 introduce more effort to implement,
> comparing to keep physical clock ptp0 but limit to use it?

I think either way, it would be a substantial change in the kernel
code.

> > It needs more thought and careful design, but I think having
> > clock_gettime() available for the different clocks would be nice to have for the
> > applications.
> 
> Thank you. Then regarding the domain timestamp, do you think it's proper to do the conversion in kernel as I implemented.

Yes, it would be very nice for the applications, because they wouldn't
have to use a different API for gettime().

The drawback is that you loose the ability to generate synchronized
signals in HW from the physical clock.

(Time stamping inputs would still work, because the timecounter code
cleverly allows that.)

Thanks,
Richard

^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [net-next 0/6] ptp: support virtual clocks for multiple domains
  2021-05-11 15:49         ` Richard Cochran
@ 2021-05-14  6:41           ` Y.b. Lu
  2021-05-14 17:44             ` Richard Cochran
  0 siblings, 1 reply; 22+ messages in thread
From: Y.b. Lu @ 2021-05-14  6:41 UTC (permalink / raw)
  To: Richard Cochran; +Cc: netdev, David S . Miller, Claudiu Manoil, Jakub Kicinski

Hi Richard,

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: 2021年5月11日 23:50
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; Claudiu
> Manoil <claudiu.manoil@nxp.com>; Jakub Kicinski <kuba@kernel.org>
> Subject: Re: [net-next 0/6] ptp: support virtual clocks for multiple domains
> 
> On Tue, May 11, 2021 at 10:40:15AM +0000, Y.b. Lu wrote:
> > What I thought was in code writing registers to adjust physical clock
> frequency, and immediately adjusting virtual clocks in opposite direction.
> > Make the operations atomic by locking. Assume the code execution has a
> DELAY, and the frequency adjusted is PPM.
> > Then the time error affecting on virtual clock will be DELAY * PPM. I'm not
> sure what the DELAY value will be on other platforms.
> > Just for example, for 1us delay, 1000ppm adjustment will have 1ns time
> error.
> >
> > But indeed, this approach may be not feasible as you said. Especially it is
> adjusting clock in max frequency, and there are many virtual clocks.
> > The time error may be large enough to cause issues. (I'm not sure
> > whether I understand you correctly, sorry.)
> 
> You understand correctly.
> 
> > So, a question is, for hardware which supports only one PTP clock, can
> multiple domains be supported where physical clock also participates in
> synchronization of a domain?
> > (Because sometime the physical clock is required to be synchronized
> > for TSN using, or other usages.) Do you think it's possible?
> 
> No, it won't work.  You can't adjust both the physical clock and the
> timecounters at the same time.  The code would be an awful hack, and it
> would not work in all real world circumstances.  If the kernel offers a new
> time service, then it has to work always.
> 
> So, getting back to my user space idea, it _would_ work to let the application
> stack control the HW clock as before, but to track the other domains
> numerically.  Then, the other applications could use the TIME_STATUS_NP
> management message (designed for use with gPTP and
> free_running) to get the current time in the other domains.
> 
> So take your pick.  You can't have it both ways, I'm afraid.
> 

I give up supporting physical clock and the timecounters adjusting at the same time, but I may continue to support virtual clock per your suggestion.
 
Getting back to your user space idea, I'd like to understand further to see if I can make some contribution.
Actually I can't think out how to track (there is not timecouner like in kernel) in a easy way, and I have some concerns too.

I assume we have a way for physical clock domain to track phase and frequency offset for each of other domains.
Are the phase and frequency offset enough to convert timestamp?

Still the key problem is hiding physical clock changes for the virtual.
There is another reason that I initially gave workaround of no stepping on physical clock.
Because timestamping is asynchronous with physical clock phase changing.
When a timestamp is captured and physical clock time has a small change, we have no idea if timestamping happened before or after clock changing during conversion.

Finally gPTP standard does use management messages. I think it doesn’t matter only if it implements the function we need.
Maybe I haven't got your point...

Thanks a lot.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [net-next 0/6] ptp: support virtual clocks for multiple domains
  2021-05-14  6:41           ` Y.b. Lu
@ 2021-05-14 17:44             ` Richard Cochran
  2021-05-17  2:46               ` Y.b. Lu
  0 siblings, 1 reply; 22+ messages in thread
From: Richard Cochran @ 2021-05-14 17:44 UTC (permalink / raw)
  To: Y.b. Lu; +Cc: netdev, David S . Miller, Claudiu Manoil, Jakub Kicinski

On Fri, May 14, 2021 at 06:41:28AM +0000, Y.b. Lu wrote:
> I give up supporting physical clock and the timecounters adjusting at the same time, but I may continue to support virtual clock per your suggestion.

Okay, so the physical clock stays free running when virtual clocks are
active.

> Getting back to your user space idea, I'd like to understand further to see if I can make some contribution.
> Actually I can't think out how to track (there is not timecouner like in kernel) in a easy way, and I have some concerns too.

Maybe I was not clear before.  You can implement the virtual clocks in
the kernel.  User space will not need to be involved.

It is easy for the kernel to hide the physical clock when virtual
clocks are created from it.

Thanks,
Richard


^ permalink raw reply	[flat|nested] 22+ messages in thread

* RE: [net-next 0/6] ptp: support virtual clocks for multiple domains
  2021-05-14 17:44             ` Richard Cochran
@ 2021-05-17  2:46               ` Y.b. Lu
  0 siblings, 0 replies; 22+ messages in thread
From: Y.b. Lu @ 2021-05-17  2:46 UTC (permalink / raw)
  To: Richard Cochran; +Cc: netdev, David S . Miller, Claudiu Manoil, Jakub Kicinski

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: 2021年5月15日 1:45
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; Claudiu
> Manoil <claudiu.manoil@nxp.com>; Jakub Kicinski <kuba@kernel.org>
> Subject: Re: [net-next 0/6] ptp: support virtual clocks for multiple domains
> 
> On Fri, May 14, 2021 at 06:41:28AM +0000, Y.b. Lu wrote:
> > I give up supporting physical clock and the timecounters adjusting at the
> same time, but I may continue to support virtual clock per your suggestion.
> 
> Okay, so the physical clock stays free running when virtual clocks are active.

Yes.

> 
> > Getting back to your user space idea, I'd like to understand further to see if I
> can make some contribution.
> > Actually I can't think out how to track (there is not timecouner like in kernel)
> in a easy way, and I have some concerns too.
> 
> Maybe I was not clear before.  You can implement the virtual clocks in the
> kernel.  User space will not need to be involved.
> 

Thank you very much to make it clear for me.

> It is easy for the kernel to hide the physical clock when virtual clocks are
> created from it.
> 
> Thanks,
> Richard


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-05-17  2:46 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07  8:57 [net-next 0/6] ptp: support virtual clocks for multiple domains Yangbo Lu
2021-05-07  8:57 ` [net-next 1/6] ptp: add ptp virtual clock driver framework Yangbo Lu
2021-05-07 10:42   ` kernel test robot
2021-05-07 10:42     ` kernel test robot
2021-05-07 11:26   ` kernel test robot
2021-05-07 11:26     ` kernel test robot
2021-05-07  8:57 ` [net-next 2/6] ptp: support virtual clock and domain via sysfs Yangbo Lu
2021-05-07  8:57 ` [net-next 3/6] ptp_qoriq: export ptp clock reading function for cyclecounter Yangbo Lu
2021-05-07  8:57 ` [net-next 4/6] enetc_ptp: support ptp virtual clock Yangbo Lu
2021-05-07 19:58   ` Claudiu Manoil
2021-05-07 20:50   ` kernel test robot
2021-05-07 20:50     ` kernel test robot
2021-05-07  8:57 ` [net-next 5/6] enetc: store ptp device pointer Yangbo Lu
2021-05-07  8:57 ` [net-next 6/6] enetc: support PTP domain timestamp conversion Yangbo Lu
2021-05-08 19:17 ` [net-next 0/6] ptp: support virtual clocks for multiple domains Richard Cochran
2021-05-10  3:04   ` Y.b. Lu
2021-05-10 23:18     ` Richard Cochran
2021-05-11 10:40       ` Y.b. Lu
2021-05-11 15:49         ` Richard Cochran
2021-05-14  6:41           ` Y.b. Lu
2021-05-14 17:44             ` Richard Cochran
2021-05-17  2:46               ` Y.b. Lu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.