[RFC net-next 0/7] Add basic SyncE interfaces

[RFC net-next 0/7] Add basic SyncE interfaces

[Arkadiusz Kubalewski]

SyncE - Synchronous Ethernet is defined in ITU-T Rec. G.8264
(https://www.itu.int/rec/T-REC-G.8264)

SyncE allows synchronizing the frequency of ethernet PHY clock signal
(the frequency used to send the data onto wire), to some reference
clock signal.

Multiple reference clock sources can be available. PHY ports recover
the frequency at which the transmitter sent the data on the RX side.
Alternatively, we can use external sources like 1PPS GPS, etc.

This patch series introduces basic interfaces for communication
with a SyncE capable device.

The first part of the interface allows acquiring the synchronization
state of DPLL (Digital Phase Locked Loop). DPLL LOCKED state means
that the frequency generated by it is locked to the input frequency.
As a result, PHYs connected to it are synchronized to the chosen input
frequency signal.

The second part can be used to select the port from which the clock
gets recovered. Each PHY chip can have multiple pins on which the
recovered clock can be propagated. For example, a SyncE-capable PHY
can recover the carrier frequency of the first port, divide it
internally, and output it as a reference clock on PIN 0.
When such a signal is enabled, the DPLL can LOCK to the frequency
recovered on PIN 0.

Next steps:
 - Add CONFIG_SYNCE definition into Kconfig
 - Add more configuration interfaces. Aiming at devlink, since this
   would be device-wide configuration

Arkadiusz Kubalewski (7):
  ptp: Add interface for acquiring DPLL state
  selftests/ptp: Add usage of PTP_DPLL_GETSTATE ioctl in testptp
  ice: add get_dpll_state ptp interface usage
  net: add ioctl interface for recover reference clock on netdev
  selftests/net: Add test app for SIOC{S|G}SYNCE
  ice: add SIOC{S|G}SYNCE interface usage to recover reference signal
  ice: add sysfs interface to configure PHY recovered reference signal

 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  62 +++++
 drivers/net/ethernet/intel/ice/ice_common.c   | 101 ++++++++
 drivers/net/ethernet/intel/ice/ice_common.h   |   9 +
 drivers/net/ethernet/intel/ice/ice_main.c     |   4 +
 drivers/net/ethernet/intel/ice/ice_ptp.c      | 234 +++++++++++++++++-
 drivers/net/ethernet/intel/ice/ice_ptp.h      |   9 +
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h   |   6 +
 drivers/ptp/ptp_chardev.c                     |  15 ++
 drivers/ptp/ptp_clockmatrix.h                 |  12 -
 drivers/ptp/ptp_private.h                     |   2 +
 drivers/ptp/ptp_sysfs.c                       |  48 ++++
 include/linux/ptp_clock_kernel.h              |   9 +
 include/uapi/linux/net_synce.h                |  21 ++
 include/uapi/linux/ptp_clock.h                |  27 ++
 include/uapi/linux/sockios.h                  |   4 +
 net/core/dev_ioctl.c                          |   6 +-
 tools/testing/selftests/net/Makefile          |   1 +
 tools/testing/selftests/net/phy_ref_clk.c     | 138 +++++++++++
 tools/testing/selftests/ptp/testptp.c         |  27 +-
 19 files changed, 720 insertions(+), 15 deletions(-)
 create mode 100644 include/uapi/linux/net_synce.h
 create mode 100644 tools/testing/selftests/net/phy_ref_clk.c


base-commit: aba1e4adb54e020d3ca85a4df3ef0f8febe87548
-- 
2.24.0

[RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Arkadiusz Kubalewski]

Previously there was no common interface for monitoring
synchronization state of Digital Phase Locked Loop.

Add interface through PTP ioctl subsystem for tools,
as well as sysfs human-friendly part of the interface.

enum dpll_state moved to uapi definition, it is required to
have common definition of DPLL states in uapi.

Add new callback function, must be implemented by ptp
enabled driver in order to get the state of dpll.

Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 drivers/ptp/ptp_chardev.c        | 15 ++++++++++
 drivers/ptp/ptp_clockmatrix.h    | 12 --------
 drivers/ptp/ptp_private.h        |  2 ++
 drivers/ptp/ptp_sysfs.c          | 48 ++++++++++++++++++++++++++++++++
 include/linux/ptp_clock_kernel.h |  9 ++++++
 include/uapi/linux/ptp_clock.h   | 27 ++++++++++++++++++
 6 files changed, 101 insertions(+), 12 deletions(-)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index af3bc65c4595..32b2713f18a5 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -106,6 +106,14 @@ int ptp_open(struct posix_clock *pc, fmode_t fmode)
 	return 0;
 }
 
+int ptp_get_dpll_state(struct ptp_clock *ptp, struct ptp_dpll_state *ds)
+{
+	if (!ptp->info->get_dpll_state)
+		return -EOPNOTSUPP;
+
+	return ptp->info->get_dpll_state(ptp->info, ds);
+}
+
 long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 {
 	struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
@@ -119,6 +127,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 	struct ptp_clock_caps caps;
 	struct ptp_clock_time *pct;
 	unsigned int i, pin_index;
+	struct ptp_dpll_state ds;
 	struct ptp_pin_desc pd;
 	struct timespec64 ts;
 	int enable, err = 0;
@@ -418,6 +427,12 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 		mutex_unlock(&ptp->pincfg_mux);
 		break;
 
+	case PTP_DPLL_GETSTATE:
+		err = ptp_get_dpll_state(ptp, &ds);
+		if (!err && copy_to_user((void __user *)arg, &ds, sizeof(ds)))
+			err = -EFAULT;
+		break;
+
 	default:
 		err = -ENOTTY;
 		break;
diff --git a/drivers/ptp/ptp_clockmatrix.h b/drivers/ptp/ptp_clockmatrix.h
index fb323271063e..0ce2f280c6d3 100644
--- a/drivers/ptp/ptp_clockmatrix.h
+++ b/drivers/ptp/ptp_clockmatrix.h
@@ -107,18 +107,6 @@ enum scsr_tod_write_type_sel {
 	SCSR_TOD_WR_TYPE_SEL_MAX = SCSR_TOD_WR_TYPE_SEL_DELTA_MINUS,
 };
 
-/* Values STATUS.DPLL_SYS_STATUS.DPLL_SYS_STATE */
-enum dpll_state {
-	DPLL_STATE_MIN = 0,
-	DPLL_STATE_FREERUN = DPLL_STATE_MIN,
-	DPLL_STATE_LOCKACQ = 1,
-	DPLL_STATE_LOCKREC = 2,
-	DPLL_STATE_LOCKED = 3,
-	DPLL_STATE_HOLDOVER = 4,
-	DPLL_STATE_OPEN_LOOP = 5,
-	DPLL_STATE_MAX = DPLL_STATE_OPEN_LOOP,
-};
-
 struct idtcm;
 
 struct idtcm_channel {
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index dba6be477067..c57fb54e2b57 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -117,6 +117,8 @@ ssize_t ptp_read(struct posix_clock *pc,
 __poll_t ptp_poll(struct posix_clock *pc,
 	      struct file *fp, poll_table *wait);
 
+int ptp_get_dpll_state(struct ptp_clock *ptp, struct ptp_dpll_state *ds);
+
 /*
  * see ptp_sysfs.c
  */
diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index b3d96b747292..fb0890fab266 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -302,6 +302,52 @@ static ssize_t max_vclocks_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(max_vclocks);
 
+static inline int dpll_state_to_str(enum dpll_state ds, const char **ds_str)
+{
+	const char * const dpll_state_string[] = {
+		"FREERUN",
+		"LOCKACQ",
+		"LOCKREC",
+		"LOCKED",
+		"HOLDOVER",
+		"OPEN_LOOP",
+		"INVALID",
+	};
+	size_t max = sizeof(dpll_state_string) /
+		     sizeof(dpll_state_string[0]);
+
+	if (ds < 0 || ds >= max)
+		return -EINVAL;
+	*ds_str = dpll_state_string[ds];
+
+	return 0;
+}
+
+static ssize_t dpll_state_show(struct device *dev,
+			       struct device_attribute *attr, char *page)
+{
+	struct ptp_clock *ptp = dev_get_drvdata(dev);
+	struct ptp_dpll_state ds;
+	const char *ds_str;
+	ssize_t size = 0;
+	int i, err;
+
+	err = ptp_get_dpll_state(ptp, &ds);
+	if (err)
+		return err;
+
+	for (i = 0; i < ds.dpll_num; i++) {
+		err = dpll_state_to_str(ds.state[i], &ds_str);
+		if (err)
+			return err;
+		size += snprintf(page + size, PAGE_SIZE - 1, "%d %s\n",
+				 i, ds_str);
+	}
+
+	return size;
+}
+static DEVICE_ATTR_RO(dpll_state);
+
 static struct attribute *ptp_attrs[] = {
 	&dev_attr_clock_name.attr,
 
@@ -318,6 +364,8 @@ static struct attribute *ptp_attrs[] = {
 	&dev_attr_pps_enable.attr,
 	&dev_attr_n_vclocks.attr,
 	&dev_attr_max_vclocks.attr,
+
+	&dev_attr_dpll_state.attr,
 	NULL
 };
 
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 71fac9237725..d56cd02d778e 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -129,6 +129,13 @@ struct ptp_system_timestamp {
  *                scheduling time (>=0) or negative value in case further
  *                scheduling is not required.
  *
+ * @get_dpll_state:  Request driver to check and update state of its DPLLs
+ *                   (Digital Phase Locked Loop).
+ *                   Driver returns structure filled with number of
+ *                   available DPLLs and their states.
+ *                   On success function returns 0, or negative on failed
+ *                   attempt.
+ *
  * Drivers should embed their ptp_clock_info within a private
  * structure, obtaining a reference to it using container_of().
  *
@@ -160,6 +167,8 @@ 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);
+	int (*get_dpll_state)(struct ptp_clock_info *ptp,
+			      struct ptp_dpll_state *ds);
 };
 
 struct ptp_clock;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 1d108d597f66..773505ad59e1 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -198,6 +198,32 @@ struct ptp_pin_desc {
 	unsigned int rsv[5];
 };
 
+enum dpll_state {
+	DPLL_STATE_MIN = 0,
+	DPLL_STATE_FREERUN = DPLL_STATE_MIN,
+	DPLL_STATE_LOCKACQ = 1,
+	DPLL_STATE_LOCKREC = 2,
+	DPLL_STATE_LOCKED = 3,
+	DPLL_STATE_HOLDOVER = 4,
+	DPLL_STATE_OPEN_LOOP = 5,
+	DPLL_STATE_MAX = DPLL_STATE_OPEN_LOOP,
+};
+
+#define PTP_MAX_DPLL_NUM_PER_DEVICE	8
+
+struct ptp_dpll_state {
+	/*
+	 * Number of available dplls on the device.
+	 */
+	int dpll_num;
+	/*
+	 * State of DPLLs. Values defined in enum dpll_states.
+	 * Indexed by DPLL index on the device.
+	 * Valid indicies < dpll_num
+	 */
+	__u8 state[PTP_MAX_DPLL_NUM_PER_DEVICE];
+};
+
 #define PTP_CLK_MAGIC '='
 
 #define PTP_CLOCK_GETCAPS  _IOR(PTP_CLK_MAGIC, 1, struct ptp_clock_caps)
@@ -223,6 +249,7 @@ struct ptp_pin_desc {
 	_IOWR(PTP_CLK_MAGIC, 17, struct ptp_sys_offset_precise)
 #define PTP_SYS_OFFSET_EXTENDED2 \
 	_IOWR(PTP_CLK_MAGIC, 18, struct ptp_sys_offset_extended)
+#define PTP_DPLL_GETSTATE   _IOR(PTP_CLK_MAGIC, 19, struct ptp_dpll_state)
 
 struct ptp_extts_event {
 	struct ptp_clock_time t; /* Time event occured. */
-- 
2.24.0

Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Richard Cochran]

On Mon, Aug 16, 2021 at 06:07:11PM +0200, Arkadiusz Kubalewski wrote:
> Previously there was no common interface for monitoring
> synchronization state of Digital Phase Locked Loop.
> 
> Add interface through PTP ioctl subsystem for tools,
> as well as sysfs human-friendly part of the interface.
> 
> enum dpll_state moved to uapi definition, it is required to
> have common definition of DPLL states in uapi.

This has nothing to do with PTP, right?

So why isn't it a RTNL feature instead?

Thanks,
Richard

RE: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Machnikowski, Maciej]

> > Previously there was no common interface for monitoring
> > synchronization state of Digital Phase Locked Loop.
> >
> > Add interface through PTP ioctl subsystem for tools, as well as sysfs
> > human-friendly part of the interface.
> >
> > enum dpll_state moved to uapi definition, it is required to have
> > common definition of DPLL states in uapi.
> 
> This has nothing to do with PTP, right?
> 
> So why isn't it a RTNL feature instead?
> 
> Thanks,
> Richard


The logic behind adding the DPLL state to the PTP subsystem is that SyncE DPLL on Network adapters, in most cases, drive the PTP timer.
Having access to it in the PTP subsystem is beneficial, as Telco standards, like G.8275.1/2, require a different behavior depending on the SyncE availability and state.
Multiport adapters use a single PLL to drive all ports. If we add the PLL interface to the PTP device - a tool that would implement support for Telco standards would have a single source of information about the presence and state of physical sync.

Moreover, it'll open the path for implementing PLL state-aware ext_ts that would generate events only when the PLL device is locked to the incoming signals improving the quality of external TS events. I.e. an external PLL can be used to monitor incoming 1PPS signal and disable event generation when its frequency drifts off 1Hz by preset margins.
 
If we bind it to the Network port, a tool would need to find the port that owns the PLL and read the state from a different place, which sounds suboptimal. Additionally we'll lose ability to rely on external HW to monitor external TS events.
 
Regards
Maciek

Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Richard Cochran]

On Tue, Aug 17, 2021 at 09:41:49AM +0000, Machnikowski, Maciej wrote:

> The logic behind adding the DPLL state to the PTP subsystem is that SyncE DPLL on Network adapters, in most cases, drive the PTP timer.

So what?  The logic in the HW has nothing to do with the proper user
space interfaces.  For example, we have SO_TIMESTAMPING and PHC as
separate APIs, even though HW devices often implement both.

> Having access to it in the PTP subsystem is beneficial, as Telco
> standards, like G.8275.1/2, require a different behavior depending
> on the SyncE availability and state.

Right, but this does say anything about the API.

> Multiport adapters use a single PLL to drive all ports. If we add
> the PLL interface to the PTP device - a tool that would implement
> support for Telco standards would have a single source of
> information about the presence and state of physical sync.

Not really.  Nothing guarantees a sane mapping from MAC to PHC.  In
many systems, there a many of each.

> Moreover, it'll open the path for implementing PLL state-aware
> ext_ts that would generate events only when the PLL device is locked
> to the incoming signals improving the quality of external TS
> events. I.e. an external PLL can be used to monitor incoming 1PPS
> signal and disable event generation when its frequency drifts off
> 1Hz by preset margins.

I don't see how this prevents using RTNL (or something else) as the
API for the SyncE configuration.

> If we bind it to the Network port, a tool would need to find the
> port that owns the PLL and read the state from a different place,
> which sounds suboptimal.

This is exactly how it works in ptpl4 today.  Some information comes
from the PHC, some from RTNL (link state), some from ethtool
(phc-interface mapping and time stamping abilities).

> Additionally we'll lose ability to rely on external HW to monitor
> external TS events.

Sorry, I can't see that at all.

Please do NOT tack this stuff onto the PHC subsystem just because you
can.

Thanks,
Richard

RE: [Intel-wired-lan] [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Keller, Jacob E]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Richard Cochran
> Sent: Wednesday, August 18, 2021 10:03 AM
> To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> Cc: cong.wang@bytedance.com; arnd@arndb.de; gustavoars@kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> colin.king@canonical.com; intel-wired-lan@lists.osuosl.org; nikolay@nvidia.com;
> linux-kselftest@vger.kernel.org; kuba@kernel.org; shuah@kernel.org;
> davem@davemloft.net
> Subject: Re: [Intel-wired-lan] [RFC net-next 1/7] ptp: Add interface for acquiring
> DPLL state
> 
> On Tue, Aug 17, 2021 at 09:41:49AM +0000, Machnikowski, Maciej wrote:
> 
> > The logic behind adding the DPLL state to the PTP subsystem is that SyncE DPLL
> on Network adapters, in most cases, drive the PTP timer.
> 
> So what?  The logic in the HW has nothing to do with the proper user
> space interfaces.  For example, we have SO_TIMESTAMPING and PHC as
> separate APIs, even though HW devices often implement both.
> 
> > Having access to it in the PTP subsystem is beneficial, as Telco
> > standards, like G.8275.1/2, require a different behavior depending
> > on the SyncE availability and state.
> 
> Right, but this does say anything about the API.
> 
> > Multiport adapters use a single PLL to drive all ports. If we add
> > the PLL interface to the PTP device - a tool that would implement
> > support for Telco standards would have a single source of
> > information about the presence and state of physical sync.
> 
> Not really.  Nothing guarantees a sane mapping from MAC to PHC.  In
> many systems, there a many of each.
> 

Well, I think the point of placing it in the PTP subsystem is that there is a sane mapping between PHC <-> DPLL. There's only one DPLL for the PHC.

Thanks,
Jake

RE: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Machnikowski, Maciej]

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Wednesday, August 18, 2021 7:03 PM
> To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> Cc: Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; linux-
> kernel@vger.kernel.org; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; linux-kselftest@vger.kernel.org; Brandeburg,
> Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; davem@davemloft.net; kuba@kernel.org;
> shuah@kernel.org; arnd@arndb.de; nikolay@nvidia.com;
> cong.wang@bytedance.com; colin.king@canonical.com;
> gustavoars@kernel.org
> Subject: Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state
> 
> > Additionally we'll lose ability to rely on external HW to monitor
> > external TS events.
> 
> Sorry, I can't see that at all.
> 
> Please do NOT tack this stuff onto the PHC subsystem just because you can.
> 
> Thanks,
> Richard

OK, Let's take a step back and forget about SyncE. 
A PTP clock is a device that has a phase and a frequency, and its frequency can be adjusted using API calls.

On the other hand, there's the physical side of the PTP clock. The PTP clock can run on cheap quartz, on the CSAC, or the PLL. 
The first two of them will get a clock signal from a passive device, but in the PLL case, it'll get it from an active one.
If it runs on an active PLL device, you get another place that adjusts the frequency of your PTP clock. 
No matter what source you use as a reference for it - CSAC, SyncE, or GNSS receiver.

Adding the PLL state to the PTP subsystem is just another indicator of the state of the PTP clock. 
The upper layer can use it, or ignored it, but it fits into the timer subsystem, as the time generated by the PTP on top will be derived from the frequency generated by the PLL.
And it is applicable to both a PHC and a completely separate implementation of timer, like the one that's present in the Time Card .

Regards
Maciek

Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Richard Cochran]

On Wed, Aug 18, 2021 at 10:36:03PM +0000, Machnikowski, Maciej wrote:

> OK, Let's take a step back and forget about SyncE. 

Ahem, the title of this series is:

    [RFC net-next 0/7] Add basic SyncE interfaces

I'd be happy to see support for configuring SyncE.

But I guess this series is about something totally different.


Thanks,
Richard

RE: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Machnikowski, Maciej]

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Thursday, August 19, 2021 5:34 PM
> To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> Cc: Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; linux-
> kernel@vger.kernel.org; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; linux-kselftest@vger.kernel.org; Brandeburg,
> Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; davem@davemloft.net; kuba@kernel.org;
> shuah@kernel.org; arnd@arndb.de; nikolay@nvidia.com;
> cong.wang@bytedance.com; colin.king@canonical.com;
> gustavoars@kernel.org; Bross, Kevin <kevin.bross@intel.com>; Stanton,
> Kevin B <kevin.b.stanton@intel.com>; Ahmad Byagowi <abyagowi@fb.com>
> Subject: Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state
> 
> On Wed, Aug 18, 2021 at 10:36:03PM +0000, Machnikowski, Maciej wrote:
> 
> > OK, Let's take a step back and forget about SyncE.
> 
> Ahem, the title of this series is:
> 
>     [RFC net-next 0/7] Add basic SyncE interfaces
> 
> I'd be happy to see support for configuring SyncE.
> 
> But I guess this series is about something totally different.
> 
> 
> Thanks,
> Richard

If it helps we'd be happy to separate that in 2 separate RFCs.
This was squashed together under SyncE support umbrella to show one of the use cases, but PTP changes are more generic and cover all PTP clocks that use DPLL for the physical clock generation.

Regards
Maciek

Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Richard Cochran]

On Thu, Aug 19, 2021 at 03:40:22PM +0000, Machnikowski, Maciej wrote:

> If it helps we'd be happy to separate that in 2 separate RFCs.

It would help me if you could explain the connection.  I have a
totally different understanding of SyncE which I explained here:

   https://lore.kernel.org/netdev/20150317161128.GA8793@localhost.localdomain/

So you need to implement two things, one in kernel and one in user
space.

1. Control bits according to IEEE 802.3 Section 40.5.2 as Ethtool or RTNL.

2. User space Ethernet Synchronization Messaging Channel (ESMC)
   service according to IEEE 802.3ay

The PHY should be automatically controlled by #1.

As I said before, none of this belongs in the PHC subsystem.

Thanks,
Richard

RE: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Machnikowski, Maciej]

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Friday, August 20, 2021 5:56 PM
> To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> Cc: Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; linux-
> kernel@vger.kernel.org; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; linux-kselftest@vger.kernel.org; Brandeburg,
> Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; davem@davemloft.net; kuba@kernel.org;
> shuah@kernel.org; arnd@arndb.de; nikolay@nvidia.com;
> cong.wang@bytedance.com; colin.king@canonical.com;
> gustavoars@kernel.org; Bross, Kevin <kevin.bross@intel.com>; Stanton,
> Kevin B <kevin.b.stanton@intel.com>; Ahmad Byagowi <abyagowi@fb.com>
> Subject: Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state
> 
> 1. Control bits according to IEEE 802.3 Section 40.5.2 as Ethtool or RTNL.
> 
> 2. User space Ethernet Synchronization Messaging Channel (ESMC)
>    service according to IEEE 802.3ay
> 
> The PHY should be automatically controlled by #1.
> 
> As I said before, none of this belongs in the PHC subsystem.
> 
> Thanks,
> Richard

Sure!

I did a talk at netDev 0x15 covering SyncE - you can refer to the slides for more detailed info, and hopefully the recording will be available soon as well:
https://netdevconf.info/0x15/session.html?Introduction-to-time-synchronization-over-Ethernet

At its core - SyncE requires 2 parts (see slide 22/23)
 - SyncE capable PHY
 - the external DPLL

The SyncE capable PHY is a PHY that can recover the physical clock, at which the data symbols are transferred, (usually) divide it and output it to the external PLL. It can also redirect the recovered and divided clock to more than one pin.
Since the 40.5.2 is not applicable to higher-speed ethernet which don't use auto-negotiation, but rather the link training sequence where the RX side always syncs its clock to the TX side.

The external DPLL tunes the frequency generated by a crystal to the frequency recovered by the PHY, and drives the outputs.

On the other end - the SyncE PHY uses the clock generated by the DPLL to transmit the data to the next element.

So to be able to control SyncE we need 2 interfaces:
- Interface to enable the recovered clock output at the given pin
- interface to monitor the DPLL to see if the clock that we got is valid, or not.

If it comes to ESMC (G.8264) messages, SyncE itself can run in 2 modes (slides 29/30 will give you more details):
- QL-Disabled - with no ESMC messages - it base on the local information from the PLL to make all decisions
- QL-Enabled - that adds ESMC and quality message transfer between the nodes.

Additionally, the SyncE DPLL can be synchronized to the external sources, like a 1PPS or a 10M signal from the GNSS.

That's why the RFC proposes 2 interfaces:
- one for enabling redirected clock on a selected pin of the PHY
- one for the physical frequency lock of the DPLL

The connection with the PTP subsystem is that in most use cases I heard about SyncE is used as a physical frequency syntonization for PTP clocks.
Hence adding a DPLL monitoring there would solve 2 issues at the same time - monitoring of a GNSS-syntonized PTP clock and the SyncE syntonized one and would make a single point to monitor by the upper layer applications.

Let me know if that makes more sense now. We could add a separate SyncE and separate PTP DPLL monitoring interfaces, but in most cases they will point to the same device.

Regards
Maciek

Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Richard Cochran]

On Fri, Aug 20, 2021 at 06:30:02PM +0000, Machnikowski, Maciej wrote:

> I did a talk at netDev 0x15 covering SyncE - you can refer to the slides for more detailed info, and hopefully the recording will be available soon as well:
> https://netdevconf.info/0x15/session.html?Introduction-to-time-synchronization-over-Ethernet

These slides are very clear and nicely done!

( And they also confirm that (ab)using the PHC chardev ioctl for the DPLL
stuff is the wrong interface ;^)

> The SyncE capable PHY is a PHY that can recover the physical clock,
> at which the data symbols are transferred, (usually) divide it and
> output it to the external PLL. It can also redirect the recovered
> and divided clock to more than one pin.

Right, and as your slides show so clearly, the DPLL is connected to
the PHY, not to the time stamping unit with the PTP clock.

> Since the 40.5.2 is not applicable to higher-speed ethernet which
> don't use auto-negotiation, but rather the link training sequence
> where the RX side always syncs its clock to the TX side.

I really want an interface that will also work with Gigabit and even
100 Megabit like the PHYTER (which does support SyncE).
 
> The external DPLL tunes the frequency generated by a crystal to the frequency recovered by the PHY, and drives the outputs.
> 
> On the other end - the SyncE PHY uses the clock generated by the DPLL to transmit the data to the next element.

So I guess that this is an implementation detail of the higher speed PHYs.

> That's why the RFC proposes 2 interfaces:
> - one for enabling redirected clock on a selected pin of the PHY
> - one for the physical frequency lock of the DPLL
> 
> The connection with the PTP subsystem is that in most use cases I
> heard about SyncE is used as a physical frequency syntonization for
> PTP clocks.

As your slides correctly show, SyncE is about distributing frequency
and not about Phase/ToD.  Of course you can combine SyncE with PTP to
get both, provided that you disable frequency adjustment in the PTP
software stack (in linuxptp, this is the "nullf" servo).  But SyncE in
fact predates PTP, and it can and should be configurable even on
interfaces that lack PHC altogther (or on kernels without PHC
enabled).

> Let me know if that makes more sense now. We could add a separate
> SyncE and separate PTP DPLL monitoring interfaces, but in most cases
> they will point to the same device.

This is just a coincidence of the device you are working with.  The
kernel really needs an interface that works with all kind of hardware
setups.  Imagine a computer with discrete MACs with HW time
stamping/PHC and discrete PHYs with SyncE support.  The PHC driver
won't have any connection to the PHY+DPLL.

Your API must be on the interface/MAC, with the possibility being
handled directly by the MAC driver (for integrated devices) or calling
into the PHY layers (phylib, phylink, and drivers/phy).

If you need a DPLL monitoring interface for your card, it ought to go
through the network interface to the MAC/PHY driver and then to the
DPLL itself.  That way, it will work with different types of hardware.

Thanks,
Richard

Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Richard Cochran]

On Fri, Aug 20, 2021 at 06:30:02PM +0000, Machnikowski, Maciej wrote:

> Since the 40.5.2 is not applicable to higher-speed ethernet which
> don't use auto-negotiation, but rather the link training sequence
> where the RX side always syncs its clock to the TX side.

By "the RX side always syncs its clock to the TX side" do you mean the
RX channel synchronizes to the link partner's TX channel?

Wow, that brings back the 100 megabit scheme I guess.  That's cool,
because the same basic idea applies to the PHYTER then.

Still we are doing to need a way for user space to query the HW
topology to discover whether a given ports may be syntonized from a
second port.  I don't think your pin selection thing works unless user
space can tell what the pins are connected to.

Thanks,
Richard

RE: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Machnikowski, Maciej]

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Sunday, August 22, 2021 4:31 AM
> Subject: Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state
> 
> On Fri, Aug 20, 2021 at 06:30:02PM +0000, Machnikowski, Maciej wrote:
> 
> > Since the 40.5.2 is not applicable to higher-speed ethernet which
> > don't use auto-negotiation, but rather the link training sequence
> > where the RX side always syncs its clock to the TX side.
> 
> By "the RX side always syncs its clock to the TX side" do you mean the RX
> channel synchronizes to the link partner's TX channel?
> 
> Wow, that brings back the 100 megabit scheme I guess.  That's cool, because
> the same basic idea applies to the PHYTER then.
> 

Yes! Sounds very similar! :)

> Still we are doing to need a way for user space to query the HW topology to
> discover whether a given ports may be syntonized from a second port.  I
> don't think your pin selection thing works unless user space can tell what the
> pins are connected to.
> 
> Thanks,
> Richard

And a good catch! I'll update the RFC to add the query functionality and move the SyncE logic/pins to the netdev subsystem.

Thanks 

Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Richard Cochran]

On Fri, Aug 20, 2021 at 06:30:02PM +0000, Machnikowski, Maciej wrote:

> So to be able to control SyncE we need 2 interfaces:
> - Interface to enable the recovered clock output at the given pin
> - interface to monitor the DPLL to see if the clock that we got is valid, or not.
> 
> If it comes to ESMC (G.8264) messages, SyncE itself can run in 2 modes (slides 29/30 will give you more details):
> - QL-Disabled - with no ESMC messages - it base on the local information from the PLL to make all decisions
> - QL-Enabled - that adds ESMC and quality message transfer between the nodes.

How do you get the QL codes from this?

+enum if_eec_state {
+       IF_EEC_STATE_INVALID = 0,
+       IF_EEC_STATE_FREERUN,
+       IF_EEC_STATE_LOCKACQ,
+       IF_EEC_STATE_LOCKREC,
+       IF_EEC_STATE_LOCKED,
+       IF_EEC_STATE_HOLDOVER,
+       IF_EEC_STATE_OPEN_LOOP,
+       __IF_EEC_STATE_MAX,
+};

Thanks,
Richard

RE: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state

[Machnikowski, Maciej]

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Monday, August 30, 2021 11:06 PM
> To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> Subject: Re: [RFC net-next 1/7] ptp: Add interface for acquiring DPLL state
> 
> On Fri, Aug 20, 2021 at 06:30:02PM +0000, Machnikowski, Maciej wrote:
> 
> > So to be able to control SyncE we need 2 interfaces:
> > - Interface to enable the recovered clock output at the given pin
> > - interface to monitor the DPLL to see if the clock that we got is valid, or
> not.
> >
> > If it comes to ESMC (G.8264) messages, SyncE itself can run in 2 modes
> (slides 29/30 will give you more details):
> > - QL-Disabled - with no ESMC messages - it base on the local information
> from the PLL to make all decisions
> > - QL-Enabled - that adds ESMC and quality message transfer between the
> nodes.
> 
> How do you get the QL codes from this?
> 
> +enum if_eec_state {
> +       IF_EEC_STATE_INVALID = 0,
> +       IF_EEC_STATE_FREERUN,
> +       IF_EEC_STATE_LOCKACQ,
> +       IF_EEC_STATE_LOCKREC,
> +       IF_EEC_STATE_LOCKED,
> +       IF_EEC_STATE_HOLDOVER,
> +       IF_EEC_STATE_OPEN_LOOP,
> +       __IF_EEC_STATE_MAX,
> +};

This structure is for monitoring the lock state - or in other words - quality 
of incoming sync signal. 

The Locked state here means that the frequency used for transmitting the 
data is syntonized with the input one. If something goes wrong, like the 
frequency you recover from the link goes beyond the specified range or 
the external signal is lost, the QL level will change accordingly.

The application layer running on top of this API needs to get the proper
QL level from the config file (just like the clockClass in PTP) and broadcast
it when the state is locked and switch to QL-DNU when you get out of 
the lock state and expire preset hw-dependent holdover clock.

Also, if you are syntonizing to the SyncE clock you need to wait with
passing along QL-levels until the state reported by the EEC changes
to LOCKED.

Regards
Maciek

[RFC net-next 2/7] selftests/ptp: Add usage of PTP_DPLL_GETSTATE ioctl in testptp

[Arkadiusz Kubalewski]

Allow get Digital Phase Locked Loop state of ptp enabled device
through ptp related ioctl PTP_DPLL_GETSTATE.

Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 tools/testing/selftests/ptp/testptp.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ptp/testptp.c b/tools/testing/selftests/ptp/testptp.c
index f7911aaeb007..67de96cf0962 100644
--- a/tools/testing/selftests/ptp/testptp.c
+++ b/tools/testing/selftests/ptp/testptp.c
@@ -141,6 +141,7 @@ static void usage(char *progname)
 		" -S         set the system time from the ptp clock time\n"
 		" -t val     shift the ptp clock time by 'val' seconds\n"
 		" -T val     set the ptp clock time to 'val' seconds\n"
+		" -u         get list of available DPLLs and their state values"
 		" -z         test combinations of rising/falling external time stamp flags\n",
 		progname);
 }
@@ -156,6 +157,7 @@ int main(int argc, char *argv[])
 	struct timex tx;
 	struct ptp_clock_time *pct;
 	struct ptp_sys_offset *sysoff;
+	struct ptp_dpll_state *ds;
 
 	char *progname;
 	unsigned int i;
@@ -177,6 +179,7 @@ int main(int argc, char *argv[])
 	int pps = -1;
 	int seconds = 0;
 	int settime = 0;
+	int dpll_state = 0;
 
 	int64_t t1, t2, tp;
 	int64_t interval, offset;
@@ -186,7 +189,7 @@ int main(int argc, char *argv[])
 
 	progname = strrchr(argv[0], '/');
 	progname = progname ? 1+progname : argv[0];
-	while (EOF != (c = getopt(argc, argv, "cd:e:f:ghH:i:k:lL:p:P:sSt:T:w:z"))) {
+	while (EOF != (c = getopt(argc, argv, "cd:e:f:ghH:i:k:lL:p:P:sSt:T:uw:z"))) {
 		switch (c) {
 		case 'c':
 			capabilities = 1;
@@ -242,6 +245,9 @@ int main(int argc, char *argv[])
 			settime = 3;
 			seconds = atoi(optarg);
 			break;
+		case 'u':
+			dpll_state = 1;
+			break;
 		case 'w':
 			pulsewidth = atoi(optarg);
 			break;
@@ -506,6 +512,25 @@ int main(int argc, char *argv[])
 		free(sysoff);
 	}
 
+	if (dpll_state) {
+		ds = calloc(1, sizeof(*ds));
+		if (!ds) {
+			perror("calloc");
+			return -1;
+		}
+
+		if (ioctl(fd, PTP_DPLL_GETSTATE, ds))
+			perror("PTP_DPLL_GETSTATE");
+		else
+			puts("get dpll state request okay");
+
+		printf("dpll state:\n");
+		for (i = 0; i < ds->dpll_num; i++)
+			printf("dpll id:%i state:%u\n", i, ds->state[i]);
+
+		free(ds);
+	}
+
 	close(fd);
 	return 0;
 }
-- 
2.24.0

Re: [RFC net-next 2/7] selftests/ptp: Add usage of PTP_DPLL_GETSTATE ioctl in testptp

[Richard Cochran]

On Mon, Aug 16, 2021 at 06:07:12PM +0200, Arkadiusz Kubalewski wrote:
> Allow get Digital Phase Locked Loop state of ptp enabled device
> through ptp related ioctl PTP_DPLL_GETSTATE.

So I think this should go into ethtool instead.

Thanks,
Richard

[RFC net-next 3/7] ice: add get_dpll_state ptp interface usage

[Arkadiusz Kubalewski]

Add new Admin Queue Command definitions for
getting status of Digital Phase Locked Loop.

Implement new part of ptp interface for getting
state of Digital Phase Locked Loop.

Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   | 33 +++++++++++++++
 drivers/net/ethernet/intel/ice/ice_common.c   | 37 +++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_common.h   |  3 ++
 drivers/net/ethernet/intel/ice/ice_ptp.c      | 40 +++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h   |  6 +++
 5 files changed, 119 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 6c727745bb29..f0c5a1f4910b 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1916,6 +1916,36 @@ struct ice_aqc_get_pkg_info_resp {
 	struct ice_aqc_get_pkg_info pkg_info[];
 };
 
+/* Get CGU DPLL status (direct 0x0C66) */
+struct ice_aqc_get_cgu_dpll_status {
+	u8 dpll_num;
+	u8 ref_state;
+#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_LOS		BIT(0)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_SCM		BIT(1)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_CFM		BIT(2)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_GST		BIT(3)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_PFM		BIT(4)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_REF_SW_ESYNC	BIT(6)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_FAST_LOCK_EN	BIT(7)
+	__le16 dpll_state;
+#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_LOCK		BIT(0)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_HO		BIT(1)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_HO_READY	BIT(2)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_FLHIT		BIT(5)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_PSLHIT	BIT(7)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_CLK_REF_SHIFT 8
+#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_CLK_REF_SEL	\
+	MAKEMASK(0x1F, ICE_AQC_GET_CGU_DPLL_STATUS_STATE_CLK_REF_SHIFT)
+#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_MODE_SHIFT	13
+#define ICE_AQC_GET_CGU_DPLL_STATUS_STATE_MODE \
+	MAKEMASK(0x7, ICE_AQC_GET_CGU_DPLL_STATUS_STATE_MODE_SHIFT)
+	__le32 phase_offset_h;
+	__le32 phase_offset_l;
+	u8 eec_mode;
+	u8 rsvd[1];
+	__le16 node_handle;
+};
+
 /* Driver Shared Parameters (direct, 0x0C90) */
 struct ice_aqc_driver_shared_params {
 	u8 set_or_get_op;
@@ -2020,6 +2050,7 @@ struct ice_aq_desc {
 		struct ice_aqc_fw_logging fw_logging;
 		struct ice_aqc_get_clear_fw_log get_clear_fw_log;
 		struct ice_aqc_download_pkg download_pkg;
+		struct ice_aqc_get_cgu_dpll_status get_cgu_dpll_status;
 		struct ice_aqc_driver_shared_params drv_shared_params;
 		struct ice_aqc_set_mac_lb set_mac_lb;
 		struct ice_aqc_alloc_free_res_cmd sw_res_ctrl;
@@ -2184,6 +2215,8 @@ enum ice_adminq_opc {
 	ice_aqc_opc_update_pkg				= 0x0C42,
 	ice_aqc_opc_get_pkg_info_list			= 0x0C43,
 
+	ice_aqc_opc_get_cgu_dpll_status                 = 0x0C66,
+
 	ice_aqc_opc_driver_shared_params		= 0x0C90,
 
 	/* Standalone Commands/Events */
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 1a3c6b60fdca..1935412941ef 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -5012,3 +5012,40 @@ bool ice_fw_supports_report_dflt_cfg(struct ice_hw *hw)
 	}
 	return false;
 }
+
+/**
+ * ice_aq_get_cgu_dpll_status - get DPLL status from Firmware
+ * @hw: pointer to the HW struct
+ * @dpll_num: DPLL index
+ * @ref_state: Reference clock state
+ * @dpll_state: DPLL state
+ * @phase_offset: Phase offset in ps
+ * @eec_mode: EEC_mode
+ *
+ * Get CGU DPLL status from Firmware (0x0C66)
+ * Returns 0 on success, negative on failure.
+ */
+enum ice_status
+ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8 dpll_num, u8 *ref_state,
+			   u16 *dpll_state, u64 *phase_offset, u8 *eec_mode)
+{
+	struct ice_aqc_get_cgu_dpll_status *cmd;
+	struct ice_aq_desc desc;
+	enum ice_status status;
+
+	ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_get_cgu_dpll_status);
+	cmd = &desc.params.get_cgu_dpll_status;
+	cmd->dpll_num = dpll_num;
+
+	status = ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
+	if (!status) {
+		*ref_state = cmd->ref_state;
+		*dpll_state = le16_to_cpu(cmd->dpll_state);
+		*phase_offset = le32_to_cpu(cmd->phase_offset_h);
+		*phase_offset <<= 32;
+		*phase_offset += le32_to_cpu(cmd->phase_offset_l);
+		*eec_mode = cmd->eec_mode;
+	}
+
+	return status;
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index fb16070f02e2..eb2e082c43cb 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -199,4 +199,7 @@ bool ice_fw_supports_lldp_fltr_ctrl(struct ice_hw *hw);
 enum ice_status
 ice_lldp_fltr_add_remove(struct ice_hw *hw, u16 vsi_num, bool add);
 bool ice_fw_supports_report_dflt_cfg(struct ice_hw *hw);
+enum ice_status
+ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8 dpll_num, u8 *ref_state,
+			   u16 *dpll_state, u64 *phase_offset, u8 *eec_mode);
 #endif /* _ICE_COMMON_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 031d044ffe7d..d48200a838e1 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1255,6 +1255,45 @@ ice_ptp_settime64(struct ptp_clock_info *info, const struct timespec64 *ts)
 	return 0;
 }
 
+/**
+ * ice_ptp_get_dpll_state - Get state of the DPLL
+ * @info: the driver's PTP info structure
+ * @ds: structure with states that is going to be filled by driver
+ *
+ * Get the synchronization state of Digital Phase Locked Loop from
+ * hardware.
+ * Returns 0 on success, negative otherwise.
+ */
+static int
+ice_ptp_get_dpll_state(struct ptp_clock_info *info, struct ptp_dpll_state *ds)
+{
+	struct ice_pf *pf = ptp_info_to_pf(info);
+	u8 ref_state, eec_mode, state;
+	struct ice_hw *hw = &pf->hw;
+	u64 phase_offset;
+	u16 dpll_state;
+	int ret, i;
+
+	for (i = 0; i < ICE_CGU_DPLL_MAX; i++) {
+		ret = ice_aq_get_cgu_dpll_status(hw, i, &ref_state,
+						 &dpll_state, &phase_offset,
+						 &eec_mode);
+		if (ret)
+			return ret;
+		state = dpll_state >>
+			ICE_AQC_GET_CGU_DPLL_STATUS_STATE_MODE_SHIFT;
+		if (state &  ICE_AQC_GET_CGU_DPLL_STATUS_STATE_LOCK)
+			ds->state[i] = DPLL_STATE_LOCKED;
+		else if (state & ICE_AQC_GET_CGU_DPLL_STATUS_STATE_HO)
+			ds->state[i] = DPLL_STATE_HOLDOVER;
+		else
+			ds->state[i] = DPLL_STATE_FREERUN;
+	}
+	ds->dpll_num = ICE_CGU_DPLL_MAX;
+
+	return ret;
+}
+
 /**
  * ice_ptp_adjtime_nonatomic - Do a non-atomic clock adjustment
  * @info: the driver's PTP info structure
@@ -1613,6 +1652,7 @@ static void ice_ptp_set_caps(struct ice_pf *pf)
 	info->adjfine = ice_ptp_adjfine;
 	info->gettimex64 = ice_ptp_gettimex64;
 	info->settime64 = ice_ptp_settime64;
+	info->get_dpll_state = ice_ptp_get_dpll_state;
 
 	if (ice_is_e810(&pf->hw))
 		ice_ptp_set_funcs_e810(pf, info);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 5fd3c673480c..c68376f864f7 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -112,6 +112,12 @@ struct ice_cgu_pll_params_e822 {
 extern const struct
 ice_cgu_pll_params_e822 e822_cgu_params[NUM_ICE_TIME_REF_FREQ];
 
+enum ice_e810t_cgu_dpll {
+	ICE_CGU_DPLL_SYNCE,
+	ICE_CGU_DPLL_PTP,
+	ICE_CGU_DPLL_MAX
+};
+
 /* Table of constants related to possible TIME_REF sources */
 extern const struct ice_time_ref_info_e822 e822_time_ref[NUM_ICE_TIME_REF_FREQ];
 
-- 
2.24.0

[RFC net-next 4/7] net: add ioctl interface for recover reference clock on netdev

[Arkadiusz Kubalewski]

Previously there was no similar interface. It is required for
configuration of Synchronous Ethernet (SyncE).

User must be able to enable or disable propagation of its
PHY recovered clock signal onto available output pin.
This allows the signal to be used as frequency reference signal
for different hardware chips.

Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 include/uapi/linux/net_synce.h | 21 +++++++++++++++++++++
 include/uapi/linux/sockios.h   |  4 ++++
 net/core/dev_ioctl.c           |  6 +++++-
 3 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/net_synce.h

diff --git a/include/uapi/linux/net_synce.h b/include/uapi/linux/net_synce.h
new file mode 100644
index 000000000000..a482a7e43151
--- /dev/null
+++ b/include/uapi/linux/net_synce.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Userspace API for synchronous ethernet configuration
+ *
+ * Copyright (C) 2021 Intel Corporation
+ * Author: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
+ *
+ */
+#ifndef _NET_SYNCE_H
+#define _NET_SYNCE_H
+#include <linux/types.h>
+
+/*
+ * Structure used for passing data with SIOCSSYNCE and SIOCGSYNCE ioctls
+ */
+struct synce_ref_clk_cfg {
+	__u8 pin_id;
+	_Bool enable;
+};
+
+#endif /* _NET_SYNCE_H */
diff --git a/include/uapi/linux/sockios.h b/include/uapi/linux/sockios.h
index 7d1bccbbef78..32c7d4909c31 100644
--- a/include/uapi/linux/sockios.h
+++ b/include/uapi/linux/sockios.h
@@ -153,6 +153,10 @@
 #define SIOCSHWTSTAMP	0x89b0		/* set and get config		*/
 #define SIOCGHWTSTAMP	0x89b1		/* get config			*/
 
+/* synchronous ethernet config per physical function */
+#define SIOCSSYNCE	0x89c0		/* set and get config           */
+#define SIOCGSYNCE	0x89c1		/* get config                   */
+
 /* Device private ioctl calls */
 
 /*
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 0e87237fd871..fe21365c9492 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -406,7 +406,9 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
 		    cmd == SIOCGMIIREG ||
 		    cmd == SIOCSMIIREG ||
 		    cmd == SIOCSHWTSTAMP ||
-		    cmd == SIOCGHWTSTAMP) {
+		    cmd == SIOCGHWTSTAMP ||
+		    cmd == SIOCSSYNCE ||
+		    cmd == SIOCGSYNCE) {
 			err = dev_eth_ioctl(dev, ifr, cmd);
 		} else if (cmd == SIOCBONDENSLAVE ||
 		    cmd == SIOCBONDRELEASE ||
@@ -577,6 +579,7 @@ int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr,
 	case SIOCBRADDIF:
 	case SIOCBRDELIF:
 	case SIOCSHWTSTAMP:
+	case SIOCSSYNCE:
 		if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
 			return -EPERM;
 		fallthrough;
@@ -605,6 +608,7 @@ int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr,
 	default:
 		if (cmd == SIOCWANDEV ||
 		    cmd == SIOCGHWTSTAMP ||
+		    cmd == SIOCGSYNCE ||
 		    (cmd >= SIOCDEVPRIVATE &&
 		     cmd <= SIOCDEVPRIVATE + 15)) {
 			dev_load(net, ifr->ifr_name);
-- 
2.24.0

Re: [RFC net-next 4/7] net: add ioctl interface for recover reference clock on netdev

[Arnd Bergmann]

On Mon, Aug 16, 2021 at 6:18 PM Arkadiusz Kubalewski
<arkadiusz.kubalewski@intel.com> wrote:

> +/*
> + * Structure used for passing data with SIOCSSYNCE and SIOCGSYNCE ioctls
> + */
> +struct synce_ref_clk_cfg {
> +       __u8 pin_id;
> +       _Bool enable;
> +};

I'm not sure if there are any guarantees about the size and alignment of _Bool,
maybe better use __u8 here as well, if only for clarity.

> +#endif /* _NET_SYNCE_H */
> diff --git a/include/uapi/linux/sockios.h b/include/uapi/linux/sockios.h
> index 7d1bccbbef78..32c7d4909c31 100644
> --- a/include/uapi/linux/sockios.h
> +++ b/include/uapi/linux/sockios.h
> @@ -153,6 +153,10 @@
>  #define SIOCSHWTSTAMP  0x89b0          /* set and get config           */
>  #define SIOCGHWTSTAMP  0x89b1          /* get config                   */
>
> +/* synchronous ethernet config per physical function */
> +#define SIOCSSYNCE     0x89c0          /* set and get config           */
> +#define SIOCGSYNCE     0x89c1          /* get config                   */

I understand that these are traditionally using the old-style 16-bit
numbers, but is there any reason to keep doing that rather than
making them modern like this?

#define SIOCSSYNCE     _IOWR(0x89, 0xc0, struct  synce_ref_clk_cfg)
/* set and get config   */
#define SIOCGSYNCE     _IOR(0x89, 0xc1, struct  synce_ref_clk_cfg)
/* get config   */

        Arnd

RE: [RFC net-next 4/7] net: add ioctl interface for recover reference clock on netdev

[Kubalewski, Arkadiusz]

>On Mon, Aug 16, 2021 at 6:18 PM Arkadiusz Kubalewski
><arkadiusz.kubalewski@intel.com> wrote:
>
>> +/*
>> + * Structure used for passing data with SIOCSSYNCE and SIOCGSYNCE ioctls
>> + */
>> +struct synce_ref_clk_cfg {
>> +       __u8 pin_id;
>> +       _Bool enable;
>> +};
>
>I'm not sure if there are any guarantees about the size and alignment of _Bool,
>maybe better use __u8 here as well, if only for clarity.
>

Sure, will fix that in next patch, seems reasonable

>> +#endif /* _NET_SYNCE_H */
>> diff --git a/include/uapi/linux/sockios.h b/include/uapi/linux/sockios.h
>> index 7d1bccbbef78..32c7d4909c31 100644
>> --- a/include/uapi/linux/sockios.h
>> +++ b/include/uapi/linux/sockios.h
>> @@ -153,6 +153,10 @@
>>  #define SIOCSHWTSTAMP  0x89b0          /* set and get config           */
>>  #define SIOCGHWTSTAMP  0x89b1          /* get config                   */
>>
>> +/* synchronous ethernet config per physical function */
>> +#define SIOCSSYNCE     0x89c0          /* set and get config           */
>> +#define SIOCGSYNCE     0x89c1          /* get config                   */
>
>I understand that these are traditionally using the old-style 16-bit
>numbers, but is there any reason to keep doing that rather than
>making them modern like this?

Personally I would try to keep it one way, just for consistency, 
but you might be right - making it modern way is better option.
If no other objections to this comment I am going to change it according to
Arnd's suggestion in next patch.

>
>#define SIOCSSYNCE     _IOWR(0x89, 0xc0, struct  synce_ref_clk_cfg)
>/* set and get config   */
>#define SIOCGSYNCE     _IOR(0x89, 0xc1, struct  synce_ref_clk_cfg)
>/* get config   */
>
>        Arnd
>

Thank you,
Arkadiusz

Re: [RFC net-next 4/7] net: add ioctl interface for recover reference clock on netdev

[Richard Cochran]

On Mon, Aug 16, 2021 at 06:07:14PM +0200, Arkadiusz Kubalewski wrote:

> +/*
> + * Structure used for passing data with SIOCSSYNCE and SIOCGSYNCE ioctls
> + */
> +struct synce_ref_clk_cfg {
> +	__u8 pin_id;

How can the user know what values of 'pin_id' are valid and useful?

> +	_Bool enable;
> +};

Thanks,
Richard

[RFC net-next 5/7] selftests/net: Add test app for SIOC{S|G}SYNCE

[Arkadiusz Kubalewski]

Test usage of new netdev IOCTLs: SIOCSSYNCE and SIOCGSYNCE.

Allow set or get the netdev driver configuration
of PHY reference clock on output pins.

Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 tools/testing/selftests/net/Makefile      |   1 +
 tools/testing/selftests/net/phy_ref_clk.c | 138 ++++++++++++++++++++++
 2 files changed, 139 insertions(+)
 create mode 100644 tools/testing/selftests/net/phy_ref_clk.c

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 4f9f73e7a299..ace9a5130478 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -39,6 +39,7 @@ TEST_GEN_FILES += hwtstamp_config rxtimestamp timestamping txtimestamp
 TEST_GEN_FILES += ipsec
 TEST_GEN_FILES += ioam6_parser
 TEST_GEN_FILES += gro
+TEST_GEN_FILES += phy_ref_clk
 TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
 TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict tls
 TEST_GEN_FILES += toeplitz
diff --git a/tools/testing/selftests/net/phy_ref_clk.c b/tools/testing/selftests/net/phy_ref_clk.c
new file mode 100644
index 000000000000..dc07cf3d4569
--- /dev/null
+++ b/tools/testing/selftests/net/phy_ref_clk.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * This program allows to test behavior of netdev after sending
+ * SyncE related ioctl: SIOCGSYNCE and SIOCSSYNCE.
+ * SIOCGSYNCE - was designed to check how output pin on PHY port
+ * was configured.
+ * SIOCSSYNCE - was designed to configure (enable or disable)
+ * one of the pins, that PHY can propagate its recovered clock
+ * signal onto.
+ *
+ * Copyright (C) 2021 Intel Corporation.
+ * Author: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <arpa/inet.h>
+#include <net/if.h>
+
+#include <asm/types.h>
+#include <linux/sockios.h>
+#include <linux/net_synce.h>
+
+static void usage(const char *error)
+{
+	if (error)
+		printf("invalid: %s\n\n", error);
+	printf("phy_ref_clk <interface> <pin_id> [enable]\n\n"
+		"Enable or disable phy-recovered reference clock signal on given output pin.\n"
+		"Depending on HW configuration, phy recovered clock may be enabled\n"
+		"or disabled on one of output pins which are at hardware's disposal\n\n"
+		"Params:\n"
+		" <interface> - name of netdev implementing SIOCGSYNCE and SIOCSSYNCE\n"
+		" <pin_id> - pin on which clock recovered from PHY shall be propagated\n"
+		"    (0-X), X - number of output pins at HW disposal\n"
+		" In case no other arguments are given, ask the driver\n"
+		" for the current config of recovered clock on the interface.\n\n"
+		" [enable] - if pin shal be enabled or disabled (0/1)\n\n");
+	exit(1);
+}
+
+static int get_ref_clk(const char *ifname, __u8 pin)
+{
+	struct synce_ref_clk_cfg ref_clk;
+	struct ifreq ifdata;
+	int sd, rc;
+
+	if (!ifname || *ifname == '\0')
+		return -1;
+
+	memset(&ifdata, 0, sizeof(ifdata));
+
+	strncpy(ifdata.ifr_name, ifname, IFNAMSIZ);
+
+	sd = socket(PF_INET, SOCK_DGRAM, IPPROTO_IP);
+	if (sd < 0) {
+		printf("socket failed\n");
+		return -1;
+	}
+
+	ref_clk.pin_id = pin;
+	ifdata.ifr_data = (void *)&ref_clk;
+
+	rc = ioctl(sd, SIOCGSYNCE, (char *)&ifdata);
+	close(sd);
+	if (rc != 0) {
+		printf("ioctl(SIOCGSYNCE) failed\n");
+		return rc;
+	}
+	printf("GET: pin %u is %s\n",
+		ref_clk.pin_id, ref_clk.enable ? "enabled" : "disabled");
+
+	return 0;
+}
+
+static int set_ref_clk(const char *ifname, __u8 pin, _Bool enable)
+{
+	struct synce_ref_clk_cfg ref_clk;
+	struct ifreq ifdata;
+	int sd, rc;
+
+	if (!ifname || *ifname == '\0')
+		return -1;
+
+	memset(&ifdata, 0, sizeof(ifdata));
+
+	strcpy(ifdata.ifr_name, ifname);
+
+	sd = socket(PF_INET, SOCK_DGRAM, IPPROTO_IP);
+	if (sd < 0) {
+		printf("socket failed\n");
+		return -1;
+	}
+
+	ref_clk.pin_id = pin;
+	ref_clk.enable = enable;
+	ifdata.ifr_data = (void *)&ref_clk;
+
+	rc = ioctl(sd, SIOCSSYNCE, (char *)&ifdata);
+	close(sd);
+	if (rc != 0) {
+		printf("ioctl(SIOCSSYNCE) failed\n");
+		return rc;
+	}
+	printf("SET: pin %u is %s",
+	       ref_clk.pin_id, ref_clk.enable ? "enabled" : "disabled");
+
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	_Bool enable;
+	__u8 pin;
+	int ret;
+
+	if (argc > 4 || argc < 3)
+		usage("argument count");
+
+	ret = sscanf(argv[2], "%u", &pin);
+	if (ret != 1)
+		usage(argv[2]);
+
+	if (argc == 3) {
+		ret = get_ref_clk(argv[1], pin);
+	} else if (argc == 4) {
+		ret = sscanf(argv[3], "%u", &enable);
+		if (ret != 1)
+			usage(argv[3]);
+		ret = set_ref_clk(argv[1], pin, enable);
+	}
+	return ret;
+}
-- 
2.24.0

[RFC net-next 6/7] ice: add SIOC{S|G}SYNCE interface usage to recover reference signal

[Arkadiusz Kubalewski]

Add new Admin Queue Command definitions for getting and setting
configuration of PHY recovered clock signal from Firmware.

Allow user to enable or disable propagation of PHY recovered clock
signal onto requested output pin with new IOCTLs: SIOCGSYNCE,
SIOCSSYNCE.

Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   | 31 ++++++-
 drivers/net/ethernet/intel/ice/ice_common.c   | 64 ++++++++++++++
 drivers/net/ethernet/intel/ice/ice_common.h   |  6 ++
 drivers/net/ethernet/intel/ice/ice_main.c     |  4 +
 drivers/net/ethernet/intel/ice/ice_ptp.c      | 83 +++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_ptp.h      |  8 ++
 6 files changed, 195 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index f0c5a1f4910b..103b036c3c3f 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1946,6 +1946,31 @@ struct ice_aqc_get_cgu_dpll_status {
 	__le16 node_handle;
 };
 
+/* Set PHY recovered clock output (direct 0x0630) */
+struct ice_aqc_set_phy_rec_clk_out {
+	u8 phy_output;
+	u8 port_num;
+	u8 flags;
+#define ICE_AQC_SET_PHY_REC_CLK_OUT_OUT_EN      BIT(0)
+#define ICE_AQC_SET_PHY_REC_CLK_OUT_CURR_PORT   0xFF
+	u8 rsvd;
+	__le32 freq;
+	u8 rsvd2[6];
+	__le16 node_handle;
+};
+
+/* Get PHY recovered clock output (direct 0x0631) */
+struct ice_aqc_get_phy_rec_clk_out {
+	u8 phy_output;
+	u8 port_num;
+	u8 flags;
+#define ICE_AQC_GET_PHY_REC_CLK_OUT_OUT_EN      BIT(0)
+	u8 rsvd;
+	__le32 freq;
+	u8 rsvd2[6];
+	__le16 node_handle;
+};
+
 /* Driver Shared Parameters (direct, 0x0C90) */
 struct ice_aqc_driver_shared_params {
 	u8 set_or_get_op;
@@ -2051,6 +2076,8 @@ struct ice_aq_desc {
 		struct ice_aqc_get_clear_fw_log get_clear_fw_log;
 		struct ice_aqc_download_pkg download_pkg;
 		struct ice_aqc_get_cgu_dpll_status get_cgu_dpll_status;
+		struct ice_aqc_set_phy_rec_clk_out set_phy_rec_clk_out;
+		struct ice_aqc_get_phy_rec_clk_out get_phy_rec_clk_out;
 		struct ice_aqc_driver_shared_params drv_shared_params;
 		struct ice_aqc_set_mac_lb set_mac_lb;
 		struct ice_aqc_alloc_free_res_cmd sw_res_ctrl;
@@ -2215,7 +2242,9 @@ enum ice_adminq_opc {
 	ice_aqc_opc_update_pkg				= 0x0C42,
 	ice_aqc_opc_get_pkg_info_list			= 0x0C43,
 
-	ice_aqc_opc_get_cgu_dpll_status                 = 0x0C66,
+	ice_aqc_opc_get_cgu_dpll_status			= 0x0C66,
+	ice_aqc_opc_set_phy_rec_clk_out			= 0x0630,
+	ice_aqc_opc_get_phy_rec_clk_out			= 0x0631,
 
 	ice_aqc_opc_driver_shared_params		= 0x0C90,
 
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 1935412941ef..985a4aabf55a 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -4993,6 +4993,70 @@ ice_lldp_fltr_add_remove(struct ice_hw *hw, u16 vsi_num, bool add)
 	return ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
 }
 
+/**
+ * ice_aq_set_phy_rec_clk_out - set RCLK phy out
+ * @hw: pointer to the HW struct
+ * @phy_output: PHY reference clock output pin
+ * @enable: GPIO state to be applied
+ * @freq: PHY output frequency
+ *
+ * Set CGU reference priority (0x0630)
+ * Return 0 on success or negative value on failure.
+ */
+enum ice_status
+ice_aq_set_phy_rec_clk_out(struct ice_hw *hw, u8 phy_output, bool enable,
+			   u32 *freq)
+{
+	struct ice_aqc_set_phy_rec_clk_out *cmd;
+	struct ice_aq_desc desc;
+	enum ice_status status;
+
+	ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_set_phy_rec_clk_out);
+	cmd = &desc.params.set_phy_rec_clk_out;
+	cmd->phy_output = phy_output;
+	cmd->port_num = ICE_AQC_SET_PHY_REC_CLK_OUT_CURR_PORT;
+	cmd->flags = enable & ICE_AQC_SET_PHY_REC_CLK_OUT_OUT_EN;
+	cmd->freq = cpu_to_le32(*freq);
+
+	status = ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
+	if (!status)
+		*freq = le32_to_cpu(cmd->freq);
+
+	return status;
+}
+
+/**
+ * ice_aq_get_phy_rec_clk_out
+ * @hw: pointer to the HW struct
+ * @phy_output: PHY reference clock output pin
+ * @port_num: Port number
+ * @flags: PHY flags
+ * @freq: PHY output frequency
+ *
+ * Get PHY recovered clock output (0x0631)
+ */
+enum ice_status
+ice_aq_get_phy_rec_clk_out(struct ice_hw *hw, u8 phy_output, u8 *port_num,
+			   u8 *flags, u32 *freq)
+{
+	struct ice_aqc_get_phy_rec_clk_out *cmd;
+	struct ice_aq_desc desc;
+	enum ice_status status;
+
+	ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_get_phy_rec_clk_out);
+	cmd = &desc.params.get_phy_rec_clk_out;
+	cmd->phy_output = phy_output;
+
+	status = ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
+	if (!status) {
+		*port_num = cmd->port_num;
+		*flags = cmd->flags;
+		*freq = le32_to_cpu(cmd->freq);
+	}
+
+	return status;
+}
+
 /**
  * ice_fw_supports_report_dflt_cfg
  * @hw: pointer to the hardware structure
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index eb2e082c43cb..1c2e08377224 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -202,4 +202,10 @@ bool ice_fw_supports_report_dflt_cfg(struct ice_hw *hw);
 enum ice_status
 ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8 dpll_num, u8 *ref_state,
 			   u16 *dpll_state, u64 *phase_offset, u8 *eec_mode);
+enum ice_status
+ice_aq_set_phy_rec_clk_out(struct ice_hw *hw, u8 phy_output, bool enable,
+			   u32 *freq);
+enum ice_status
+ice_aq_get_phy_rec_clk_out(struct ice_hw *hw, u8 phy_output, u8 *port_num,
+			   u8 *flags, u32 *freq);
 #endif /* _ICE_COMMON_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 802a59345bfa..60ab4b80d919 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -6675,6 +6675,10 @@ static int ice_eth_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 		return ice_ptp_get_ts_config(pf, ifr);
 	case SIOCSHWTSTAMP:
 		return ice_ptp_set_ts_config(pf, ifr);
+	case SIOCGSYNCE:
+		return ice_ptp_get_ref_clk(pf, ifr);
+	case SIOCSSYNCE:
+		return ice_ptp_set_ref_clk(pf, ifr);
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index d48200a838e1..23ab85dbbfc8 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -3,6 +3,7 @@
 
 #include "ice.h"
 #include "ice_lib.h"
+#include <linux/net_synce.h>
 
 #define E810_OUT_PROP_DELAY_NS 1
 
@@ -2205,3 +2206,85 @@ void ice_ptp_release(struct ice_pf *pf)
 
 	dev_info(ice_pf_to_dev(pf), "Removed PTP clock\n");
 }
+
+/**
+ * ice_ptp_get_ref_clk - get state of PHY recovered clock pin
+ * @pf:  pointer to pf structure
+ * @ifr: pointer to ioctl data
+ *
+ * Get state of the pin from Firmware and pass it to the user.
+ */
+int ice_ptp_get_ref_clk(struct ice_pf *pf, struct ifreq *ifr)
+{
+	u8 flags = 0, port_num = ICE_AQC_SET_PHY_REC_CLK_OUT_CURR_PORT;
+	struct synce_ref_clk_cfg ref_clk;
+	u32 freq = 0;
+	int ret;
+
+	if (copy_from_user(&ref_clk, ifr->ifr_data, sizeof(ref_clk)))
+		return -EFAULT;
+
+	if (ref_clk.pin_id > ICE_C827_RCLKB_PIN) {
+		ret = -EINVAL;
+		goto out;
+	}
+	ret = ice_aq_get_phy_rec_clk_out(&pf->hw, ref_clk.pin_id,
+					 &port_num, &flags, &freq);
+
+	if (ret) {
+		dev_warn(ice_pf_to_dev(pf),
+			 "Failed to read recover reference clock config on pin %u err %d aq_err %s\n",
+			 ref_clk.pin_id,
+			 ret, ice_aq_str(pf->hw.adminq.sq_last_status));
+		goto out;
+	}
+	ref_clk.enable = !!(flags & ICE_AQC_SET_PHY_REC_CLK_OUT_OUT_EN);
+	dev_dbg(ice_pf_to_dev(pf),
+		"recover reference clock on pin: %u is %s\n",
+		ref_clk.pin_id,
+		ref_clk.enable ? "enabled" : "disabled");
+	ret = copy_to_user(ifr->ifr_data, &ref_clk, sizeof(ref_clk));
+out:
+	return ret;
+}
+
+/**
+ * ice_ptp_set_ref_clk - set state of PHY recovered clock pin
+ * @pf:  pointer to pf structure
+ * @ifr: pointer to ioctl data
+ *
+ * Set state of the pin in the Firmware according to the user input.
+ */
+int ice_ptp_set_ref_clk(struct ice_pf *pf, struct ifreq *ifr)
+{
+	struct synce_ref_clk_cfg ref_clk;
+	u32 freq = 0;
+	int ret;
+
+	if (copy_from_user(&ref_clk, ifr->ifr_data, sizeof(ref_clk)))
+		return -EFAULT;
+
+	if (ref_clk.pin_id > ICE_C827_RCLKB_PIN) {
+		ret = -EINVAL;
+		goto out;
+	}
+	ret = ice_aq_set_phy_rec_clk_out(&pf->hw, ref_clk.pin_id,
+					 ref_clk.enable, &freq);
+
+	if (ret) {
+		dev_warn(ice_pf_to_dev(pf),
+			 "Failed to %s recover reference clock on pin %u err %d aq_err %s\n",
+			 ref_clk.enable ? "enable" : "disable",
+			 ref_clk.pin_id,
+			 ret, ice_aq_str(pf->hw.adminq.sq_last_status));
+		goto out;
+	}
+
+	dev_dbg(ice_pf_to_dev(pf),
+		"%s recover reference clock on pin: %u\n",
+		ref_clk.enable ? "Enabled" : " Disabled",
+		ref_clk.pin_id);
+	ret = copy_to_user(ifr->ifr_data, &ref_clk, sizeof(ref_clk));
+out:
+	return ret;
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 49d7154e627c..75656eb3084a 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -24,6 +24,12 @@ struct ice_perout_channel {
 	u64 start_time;
 };
 
+enum ice_phy_rclk_pins {
+	ICE_C827_RCLKA_PIN,		/* SCL pin */
+	ICE_C827_RCLKB_PIN,		/* SDA pin */
+	ICE_C827_RCLK_PINS_NUM		/* number of pins */
+};
+
 /* The ice hardware captures Tx hardware timestamps in the PHY. The timestamp
  * is stored in a buffer of registers. Depending on the specific hardware,
  * this buffer might be shared across multiple PHY ports.
@@ -223,4 +229,6 @@ static inline void ice_ptp_release(struct ice_pf *pf) { }
 static inline int ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup)
 { return 0; }
 #endif /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
+int ice_ptp_get_ref_clk(struct ice_pf *pf, struct ifreq *ifr);
+int ice_ptp_set_ref_clk(struct ice_pf *pf, struct ifreq *ifr);
 #endif /* _ICE_PTP_H_ */
-- 
2.24.0

[RFC net-next 7/7] ice: add sysfs interface to configure PHY recovered reference signal

[Arkadiusz Kubalewski]

Allow user to enable or disable propagation of PHY recovered clock
signal onto requested output pin with new human friendly device private
sysfs interface.

Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ptp.c | 111 ++++++++++++++++++++++-
 drivers/net/ethernet/intel/ice/ice_ptp.h |   1 +
 2 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 23ab85dbbfc8..054346a8fdbd 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -9,6 +9,114 @@
 
 #define UNKNOWN_INCVAL_E822 0x100000000ULL
 
+static ssize_t ice_sysfs_phy_write(struct kobject *kobj,
+				   struct kobj_attribute *attr,
+				   const char *buf, size_t count);
+
+static struct kobj_attribute phy_attribute = __ATTR(synce, 0220,
+	NULL, ice_sysfs_phy_write);
+
+/**
+ * __get_pf_pdev - helper function to get the pdev
+ * @kobj:       kobject passed
+ * @pdev:       PCI device information struct
+ *
+ * Raturns 0 on success, negative on failure
+ */
+static int __get_pf_pdev(struct kobject *kobj, struct pci_dev **pdev)
+{
+	struct device *dev;
+
+	if (!kobj->parent)
+		return -EINVAL;
+
+	/* get pdev */
+	dev = kobj_to_dev(kobj->parent);
+	*pdev = to_pci_dev(dev);
+
+	return 0;
+}
+
+#define ICE_C827_RCLKB_PIN      1 /* SDA pin */
+
+/**
+ * ice_sysfs_phy_write - sysfs interface for setting PHY recovered clock pins
+ * @kobj:  sysfs node
+ * @attr:  sysfs node attributes
+ * @buf:   string representing enable and pin number
+ * @count: length of the 'buf' string
+ *
+ * Return number of bytes written on success or negative value on failure.
+ **/
+static ssize_t
+ice_sysfs_phy_write(struct kobject *kobj, struct kobj_attribute *attr,
+		    const char *buf, size_t count)
+{
+	enum ice_status ret = 0;
+	unsigned int ena, pin;
+	struct pci_dev *pdev;
+	struct ice_pf *pf;
+	u32 freq = 0;
+	int cnt;
+
+	if (__get_pf_pdev(kobj, &pdev))
+		return -EPERM;
+
+	pf = pci_get_drvdata(pdev);
+
+	cnt = sscanf(buf, "%u %u", &ena, &pin);
+	if (cnt != 2 || pin > ICE_C827_RCLKB_PIN)
+		return -EINVAL;
+
+	ret = ice_aq_set_phy_rec_clk_out(&pf->hw, pin, !!ena, &freq);
+	if (ret)
+		return -EIO;
+
+	return count;
+}
+
+/**
+ * ice_phy_sysfs_init - initialize sysfs for DPLL
+ * @pf: pointer to pf structure
+ *
+ * Initialize sysfs for handling DPLL in HW.
+ **/
+static void ice_phy_sysfs_init(struct ice_pf *pf)
+{
+	struct kobject *phy_kobj;
+
+	phy_kobj = kobject_create_and_add("phy", &pf->pdev->dev.kobj);
+	if (!phy_kobj) {
+		dev_info(&pf->pdev->dev, "Failed to create PHY kobject\n");
+		return;
+	}
+
+	if (sysfs_create_file(phy_kobj, &phy_attribute.attr)) {
+		dev_info(&pf->pdev->dev, "Failed to create synce kobject\n");
+		kobject_put(phy_kobj);
+		return;
+	}
+
+	pf->ptp.phy_kobj = phy_kobj;
+}
+
+/**
+ * ice_ptp_sysfs_release - release sysfs resources of ptp and synce features
+ * @pf: pointer to pf structure
+ *
+ * Release sysfs interface resources for handling configuration of
+ * ptp and synce features.
+ */
+static void ice_ptp_sysfs_release(struct ice_pf *pf)
+{
+	if (pf->ptp.phy_kobj) {
+		sysfs_remove_file(pf->ptp.phy_kobj, &phy_attribute.attr);
+		kobject_del(pf->ptp.phy_kobj);
+		kobject_put(pf->ptp.phy_kobj);
+		pf->ptp.phy_kobj = 0;
+	}
+}
+
 /**
  * ice_set_tx_tstamp - Enable or disable Tx timestamping
  * @pf: The PF pointer to search in
@@ -2121,6 +2229,7 @@ void ice_ptp_init(struct ice_pf *pf)
 			return;
 	}
 
+	ice_phy_sysfs_init(pf);
 	/* Disable timestamping for both Tx and Rx */
 	ice_ptp_cfg_timestamp(pf, false);
 
@@ -2180,7 +2289,7 @@ void ice_ptp_release(struct ice_pf *pf)
 {
 	/* Disable timestamping for both Tx and Rx */
 	ice_ptp_cfg_timestamp(pf, false);
-
+	ice_ptp_sysfs_release(pf);
 	ice_ptp_release_tx_tracker(pf, &pf->ptp.port.tx);
 
 	clear_bit(ICE_FLAG_PTP, pf->flags);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 75656eb3084a..9b526782a977 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -143,6 +143,7 @@ struct ice_ptp {
 	struct ptp_clock_info info;
 	struct ptp_clock *clock;
 	struct hwtstamp_config tstamp_config;
+	struct kobject *phy_kobj;
 };
 
 #define __ptp_port_to_ptp(p) \
-- 
2.24.0

Re: [RFC net-next 0/7] Add basic SyncE interfaces

[Richard Cochran]

On Mon, Aug 16, 2021 at 06:07:10PM +0200, Arkadiusz Kubalewski wrote:

> The second part can be used to select the port from which the clock
> gets recovered. Each PHY chip can have multiple pins on which the
> recovered clock can be propagated. For example, a SyncE-capable PHY
> can recover the carrier frequency of the first port, divide it
> internally, and output it as a reference clock on PIN 0.

This really sounds like its own thing, and not a PHC at all.

> Next steps:
>  - Add CONFIG_SYNCE definition into Kconfig
>  - Add more configuration interfaces. Aiming at devlink, since this
>    would be device-wide configuration

As a first step, finding an appropriate kernel/user space API would be
needed.

Thanks,
Richard

Re: [RFC net-next 0/7] Add basic SyncE interfaces

[Richard Cochran]

On Mon, Aug 16, 2021 at 06:07:10PM +0200, Arkadiusz Kubalewski wrote:

> Multiple reference clock sources can be available. PHY ports recover
> the frequency at which the transmitter sent the data on the RX side.
> Alternatively, we can use external sources like 1PPS GPS, etc.

There is a generic PHY subsystem (drivers/phy) used by USB, PCIe, CAN,
and so on.  Why not use that?

Thanks,
Richard