Skip to contentSkip to conversation

Peer to Peer One-Step time stamping

PATCH Superseded Richard Cochran 5 participants 32 messages Dec 16, 2019 – Dec 20, 2019

Subsystems (9)
Networking Network drivers Device tree bindings Ethernet PHY +5 more subsystems

v6 of 9 Superseded Latest is v9 →

lore
Share & tools
lore

Richard Cochran

author maintainer
#0 lore raw

v6 cover Peer to Peer One-Step time stamping

This series adds support for PTP (IEEE 1588) P2P one-step time stamping along with a driver for a hardware device that supports this.

If the hardware supports p2p one-step, it subtracts the ingress time stamp value from the Pdelay_Request correction field. The user space software stack then simply copies the correction field into the Pdelay_Response, and on transmission the hardware adds the egress time stamp into the correction field.

This new functionality extends CONFIG_NETWORK_PHY_TIMESTAMPING to cover MII snooping devices, but it still depends on phylib, just as that option does. Expanding beyond phylib is not within the scope of the this series.

User space support is available in the current linuxptp master branch.

- Patch 1 adds phy_device methods for existing time stamping fields. (NEW in v6)
- Patches 2-5 convert the stack and drivers to the new methods. (NEW in v6)
- Patches 6-9 add support for MII time stamping in non-PHY devices.
- Patch 10 adds the new P2P 1-step option.
- Patch 11 adds a driver implementing the new option.

Thanks,
Richard

Changed in v6:
~~~~~~~~~~~~~~

- Added methods for accessing the phy_device time stamping fields.
- Adjust the device tree documentation per Rob's v5 review.
- Fixed the build failures due to missing exports.

Changed in v5:
~~~~~~~~~~~~~~

- Fixed build failure in macvlan.
- Fixed latent bug with its gcc warning in the driver.

Changed in v4:
~~~~~~~~~~~~~~

- Correct error paths and PTR_ERR return values in the framework.
- Expanded KernelDoc comments WRT PHY locking.
- Pick up Andrew's review tag.

Changed in v3:
~~~~~~~~~~~~~~

- Simplify the device tree binding and document the time stamping phandle by itself.

Changed in v2:
~~~~~~~~~~~~~~

- Per the v1 review, changed the modeling of MII time stamping devices. They are no longer a kind of mdio device.

Richard Cochran (11):
net: phy: Introduce helper functions for time stamping support.
net: macvlan: Use the PHY time stamping interface.
net: vlan: Use the PHY time stamping interface.
net: ethtool: Use the PHY time stamping interface.
net: netcp_ethss: Use the PHY time stamping interface.
net: Introduce a new MII time stamping interface.
net: Add a layer for non-PHY MII time stamping drivers.
dt-bindings: ptp: Introduce MII time stamping devices.
net: mdio: of: Register discovered MII time stampers.
net: Introduce peer to peer one step PTP time stamping.
ptp: Add a driver for InES time stamping IP core.

 .../devicetree/bindings/ptp/ptp-ines.txt      |  35 +
 .../devicetree/bindings/ptp/timestamper.txt   |  41 +
 .../net/ethernet/broadcom/bnx2x/bnx2x_main.c  |   1 +
 drivers/net/ethernet/ti/netcp_ethss.c         |   8 +-
 drivers/net/macvlan.c                         |   4 +-
 drivers/net/phy/Makefile                      |   2 +
 drivers/net/phy/dp83640.c                     |  47 +-
 drivers/net/phy/mii_timestamper.c             | 125 +++
 drivers/net/phy/phy.c                         |   4 +-
 drivers/net/phy/phy_device.c                  |   5 +
 drivers/of/of_mdio.c                          |  30 +-
 drivers/ptp/Kconfig                           |  10 +
 drivers/ptp/Makefile                          |   1 +
 drivers/ptp/ptp_ines.c                        | 859 ++++++++++++++++++
 include/linux/mii_timestamper.h               | 121 +++
 include/linux/phy.h                           |  85 +-
 include/uapi/linux/net_tstamp.h               |   8 +
 net/8021q/vlan_dev.c                          |   4 +-
 net/Kconfig                                   |   7 +-
 net/core/dev_ioctl.c                          |   1 +
 net/core/timestamping.c                       |  20 +-
 net/ethtool/ioctl.c                           |   4 +-
 22 files changed, 1357 insertions(+), 65 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ptp/ptp-ines.txt
 create mode 100644 Documentation/devicetree/bindings/ptp/timestamper.txt
 create mode 100644 drivers/net/phy/mii_timestamper.c
 create mode 100644 drivers/ptp/ptp_ines.c
 create mode 100644 include/linux/mii_timestamper.h
Signature · 1 line
2.20.1
author maintainer
#1 lore raw

PATCH v6 1/11 net: phy: Introduce helper functions for time stamping support.

Some parts of the networking stack and at least one driver test fields within the 'struct phy_device' in order to query time stamping capabilities and to invoke time stamping methods. This patch adds a functional interface around the time stamping fields. This will allow insulating the callers from future changes to the details of the time stamping implemenation.

Signed-off-by Richard Cochran <[email protected]>
---
 include/linux/phy.h | 60 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)
include/linux/phy.h +60 -0
index 5032d453ac66..fc51aacb03a7 100644
@@ -936,6 +936,66 @@ static inline bool phy_polling_mode(struct phy_device *phydev)
 	return phydev->irq == PHY_POLL;
 }
 
+/**
+ * phy_has_hwtstamp - Tests whether a PHY time stamp configuration.
+ * @phydev: the phy_device struct
+ */
+static inline bool phy_has_hwtstamp(struct phy_device *phydev)
+{
+	return phydev && phydev->drv && phydev->drv->hwtstamp;
+}
+
+/**
+ * phy_has_rxtstamp - Tests whether a PHY supports receive time stamping.
+ * @phydev: the phy_device struct
+ */
+static inline bool phy_has_rxtstamp(struct phy_device *phydev)
+{
+	return phydev && phydev->drv && phydev->drv->rxtstamp;
+}
+
+/**
+ * phy_has_tsinfo - Tests whether a PHY reports time stamping and/or
+ * PTP hardware clock capabilities.
+ * @phydev: the phy_device struct
+ */
+static inline bool phy_has_tsinfo(struct phy_device *phydev)
+{
+	return phydev && phydev->drv && phydev->drv->ts_info;
+}
+
+/**
+ * phy_has_txtstamp - Tests whether a PHY supports transmit time stamping.
+ * @phydev: the phy_device struct
+ */
+static inline bool phy_has_txtstamp(struct phy_device *phydev)
+{
+	return phydev && phydev->drv && phydev->drv->txtstamp;
+}
+
+static inline int phy_hwtstamp(struct phy_device *phydev, struct ifreq *ifr)
+{
+	return phydev->drv->hwtstamp(phydev, ifr);
+}
+
+static inline bool phy_rxtstamp(struct phy_device *phydev, struct sk_buff *skb,
+				int type)
+{
+	return phydev->drv->rxtstamp(phydev, skb, type);
+}
+
+static inline int phy_ts_info(struct phy_device *phydev,
+			      struct ethtool_ts_info *tsinfo)
+{
+	return phydev->drv->ts_info(phydev, tsinfo);
+}
+
+static inline void phy_txtstamp(struct phy_device *phydev, struct sk_buff *skb,
+				int type)
+{
+	phydev->drv->txtstamp(phydev, skb, type);
+}
+
 /**
  * phy_is_internal - Convenience function for testing if a PHY is internal
  * @phydev: the phy_device struct
Signature · 1 line
2.20.1
maintainer Reviewed-by on v6 1/11
#2 lore raw

Re: [PATCH V6 net-next 01/11] net: phy: Introduce helper functions for time stamping support.

Quoted 8 lines · Richard Cochran

Some parts of the networking stack and at least one driver test fields within the 'struct phy_device' in order to query time stamping capabilities and to invoke time stamping methods. This patch adds a functional interface around the time stamping fields. This will allow insulating the callers from future changes to the details of the time stamping implemenation.

Signed-off-by Richard Cochran <[email protected]>
Reviewed-by Andrew Lunn <[email protected]>

Andrew

author maintainer
#3 lore raw

PATCH v6 2/11 net: macvlan: Use the PHY time stamping interface.

In reply to Richard Cochran

The macvlan layer tests fields of the phy_device in order to determine whether to invoke the PHY's tsinfo ethtool callback. This patch replaces the open coded logic with an invocation of the proper methods.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/macvlan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/macvlan.c +2 -2
index 05631d97eeb4..d066cf58c926 100644
@@ -1036,8 +1036,8 @@ static int macvlan_ethtool_get_ts_info(struct net_device *dev,
 	const struct ethtool_ops *ops = real_dev->ethtool_ops;
 	struct phy_device *phydev = real_dev->phydev;
 
-	if (phydev && phydev->drv && phydev->drv->ts_info) {
-		 return phydev->drv->ts_info(phydev, info);
+	if (phy_has_tsinfo(phydev)) {
+		return phy_ts_info(phydev, info);
 	} else if (ops->get_ts_info) {
 		return ops->get_ts_info(real_dev, info);
 	} else {
Signature · 1 line
2.20.1
maintainer Reviewed-by on v6 2/11
#4 lore raw

Re: [PATCH V6 net-next 02/11] net: macvlan: Use the PHY time stamping interface.

Quoted 6 lines · Richard Cochran

The macvlan layer tests fields of the phy_device in order to determine whether to invoke the PHY's tsinfo ethtool callback. This patch replaces the open coded logic with an invocation of the proper methods.

Signed-off-by Richard Cochran <[email protected]>
Reviewed-by Andrew Lunn <[email protected]>

Andrew

author maintainer
#5 lore raw

PATCH v6 3/11 net: vlan: Use the PHY time stamping interface.

In reply to Richard Cochran

The vlan layer tests fields of the phy_device in order to determine whether to invoke the PHY's tsinfo ethtool callback. This patch replaces the open coded logic with an invocation of the proper methods.

Signed-off-by Richard Cochran <[email protected]>
---
 net/8021q/vlan_dev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
net/8021q/vlan_dev.c +2 -2
index e5bff5cc6f97..5ff8059837b4 100644
@@ -646,8 +646,8 @@ static int vlan_ethtool_get_ts_info(struct net_device *dev,
 	const struct ethtool_ops *ops = vlan->real_dev->ethtool_ops;
 	struct phy_device *phydev = vlan->real_dev->phydev;
 
-	if (phydev && phydev->drv && phydev->drv->ts_info) {
-		 return phydev->drv->ts_info(phydev, info);
+	if (phy_has_tsinfo(phydev)) {
+		return phy_ts_info(phydev, info);
 	} else if (ops->get_ts_info) {
 		return ops->get_ts_info(vlan->real_dev, info);
 	} else {
Signature · 1 line
2.20.1
maintainer Reviewed-by on v6 3/11
#6 lore raw

Re: [PATCH V6 net-next 03/11] net: vlan: Use the PHY time stamping interface.

Quoted 6 lines · Richard Cochran

The vlan layer tests fields of the phy_device in order to determine whether to invoke the PHY's tsinfo ethtool callback. This patch replaces the open coded logic with an invocation of the proper methods.

Signed-off-by Richard Cochran <[email protected]>
Reviewed-by Andrew Lunn <[email protected]>

Andrew

author maintainer
#7 lore raw

PATCH v6 4/11 net: ethtool: Use the PHY time stamping interface.

In reply to Richard Cochran

The ethtool layer tests fields of the phy_device in order to determine whether to invoke the PHY's tsinfo ethtool callback. This patch replaces the open coded logic with an invocation of the proper methods.

Signed-off-by Richard Cochran <[email protected]>
---
 net/ethtool/ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
net/ethtool/ioctl.c +2 -2
index aed2c2cf1623..88f7cddf5a6f 100644
@@ -2096,8 +2096,8 @@ static int ethtool_get_ts_info(struct net_device *dev, void __user *useraddr)
 	memset(&info, 0, sizeof(info));
 	info.cmd = ETHTOOL_GET_TS_INFO;
 
-	if (phydev && phydev->drv && phydev->drv->ts_info) {
-		err = phydev->drv->ts_info(phydev, &info);
+	if (phy_has_tsinfo(phydev)) {
+		err = phy_ts_info(phydev, &info);
 	} else if (ops->get_ts_info) {
 		err = ops->get_ts_info(dev, &info);
 	} else {
Signature · 1 line
2.20.1
maintainer Reviewed-by on v6 4/11
#8 lore raw

Re: [PATCH V6 net-next 04/11] net: ethtool: Use the PHY time stamping interface.

Quoted 6 lines · Richard Cochran

The ethtool layer tests fields of the phy_device in order to determine whether to invoke the PHY's tsinfo ethtool callback. This patch replaces the open coded logic with an invocation of the proper methods.

Signed-off-by Richard Cochran <[email protected]>
Reviewed-by Andrew Lunn <[email protected]>

Andrew

author maintainer
#9 lore raw

PATCH v6 5/11 net: netcp_ethss: Use the PHY time stamping interface.

In reply to Richard Cochran

Thie netcp_ethss driver tests fields of the phy_device in order to determine whether to defer to the PHY's time stamping functionality. This patch replaces the open coded logic with an invocation of the proper methods.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/ethernet/ti/netcp_ethss.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/net/ethernet/ti/netcp_ethss.c +3 -5
index 86a3f42a3dcc..1280ccd581d4 100644
@@ -2533,8 +2533,6 @@ static int gbe_del_vid(void *intf_priv, int vid)
 }
 
 #if IS_ENABLED(CONFIG_TI_CPTS)
-#define HAS_PHY_TXTSTAMP(p) ((p)->drv && (p)->drv->txtstamp)
-#define HAS_PHY_RXTSTAMP(p) ((p)->drv && (p)->drv->rxtstamp)
 
 static void gbe_txtstamp(void *context, struct sk_buff *skb)
 {

@@ -2566,7 +2564,7 @@ static int gbe_txtstamp_mark_pkt(struct gbe_intf *gbe_intf,
 	 * We mark it here because skb_tx_timestamp() is called
 	 * after all the txhooks are called.
 	 */
-	if (phydev && HAS_PHY_TXTSTAMP(phydev)) {
+	if (phy_has_txtstamp(phydev)) {
 		skb_shinfo(p_info->skb)->tx_flags |= SKBTX_IN_PROGRESS;
 		return 0;
 	}

@@ -2588,7 +2586,7 @@ static int gbe_rxtstamp(struct gbe_intf *gbe_intf, struct netcp_packet *p_info)
 	if (p_info->rxtstamp_complete)
 		return 0;
 
-	if (phydev && HAS_PHY_RXTSTAMP(phydev)) {
+	if (phy_has_rxtstamp(phydev)) {
 		p_info->rxtstamp_complete = true;
 		return 0;
 	}

@@ -2830,7 +2828,7 @@ static int gbe_ioctl(void *intf_priv, struct ifreq *req, int cmd)
 	struct gbe_intf *gbe_intf = intf_priv;
 	struct phy_device *phy = gbe_intf->slave->phy;
 
-	if (!phy || !phy->drv->hwtstamp) {
+	if (!phy_has_hwtstamp(phy)) {
 		switch (cmd) {
 		case SIOCGHWTSTAMP:
 			return gbe_hwtstamp_get(gbe_intf, req);
Signature · 1 line
2.20.1
maintainer Reviewed-by on v6 5/11
#10 lore raw

Re: [PATCH V6 net-next 05/11] net: netcp_ethss: Use the PHY time stamping interface.

Quoted 1 line · Richard Cochran

Thie netcp_ethss driver tests fields of the phy_device in order to

The

Quoted 5 lines

determine whether to defer to the PHY's time stamping functionality. This patch replaces the open coded logic with an invocation of the proper methods.

Signed-off-by Richard Cochran <[email protected]>
Reviewed-by Andrew Lunn <[email protected]>

Andrew

author maintainer
#11 lore raw

PATCH v6 6/11 net: Introduce a new MII time stamping interface.

In reply to Richard Cochran

Currently the stack supports time stamping in PHY devices. However, there are newer, non-PHY devices that can snoop an MII bus and provide time stamps. In order to support such devices, this patch introduces a new interface to be used by both PHY and non-PHY devices.

In addition, the one and only user of the old PHY time stamping API is converted to the new interface.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/phy/dp83640.c       | 47 +++++++++++++++++---------
 drivers/net/phy/phy.c           |  4 +--
 drivers/net/phy/phy_device.c    |  2 ++
 include/linux/mii_timestamper.h | 58 +++++++++++++++++++++++++++++++++
 include/linux/phy.h             | 41 ++++++-----------------
 net/core/timestamping.c         | 20 ++++++------
 6 files changed, 114 insertions(+), 58 deletions(-)
 create mode 100644 include/linux/mii_timestamper.h
drivers/net/phy/dp83640.c +32 -15
index 8f241b57fcf6..42624c91466d 100644
@@ -98,6 +98,7 @@ struct dp83640_private {
 	struct list_head list;
 	struct dp83640_clock *clock;
 	struct phy_device *phydev;
+	struct mii_timestamper mii_ts;
 	struct delayed_work ts_work;
 	int hwts_tx_en;
 	int hwts_rx_en;

@@ -201,6 +202,14 @@ static void dp83640_gpio_defaults(struct ptp_pin_desc *pd)
 static LIST_HEAD(phyter_clocks);
 static DEFINE_MUTEX(phyter_clocks_lock);
 
+static int dp83640_hwtstamp(struct mii_timestamper *mii_ts,
+			    struct ifreq *ifr);
+static int dp83640_ts_info(struct mii_timestamper *mii_ts,
+			   struct ethtool_ts_info *info);
+static bool dp83640_rxtstamp(struct mii_timestamper *mii_ts,
+			     struct sk_buff *skb, int type);
+static void dp83640_txtstamp(struct mii_timestamper *mii_ts,
+			     struct sk_buff *skb, int type);
 static void rx_timestamp_work(struct work_struct *work);
 
 /* extended register access functions */

@@ -1149,13 +1158,18 @@ static int dp83640_probe(struct phy_device *phydev)
 		goto no_memory;
 
 	dp83640->phydev = phydev;
-	INIT_DELAYED_WORK(&dp83640->ts_work, rx_timestamp_work);
+	dp83640->mii_ts.rxtstamp = dp83640_rxtstamp;
+	dp83640->mii_ts.txtstamp = dp83640_txtstamp;
+	dp83640->mii_ts.hwtstamp = dp83640_hwtstamp;
+	dp83640->mii_ts.ts_info  = dp83640_ts_info;
 
+	INIT_DELAYED_WORK(&dp83640->ts_work, rx_timestamp_work);
 	INIT_LIST_HEAD(&dp83640->rxts);
 	INIT_LIST_HEAD(&dp83640->rxpool);
 	for (i = 0; i < MAX_RXTS; i++)
 		list_add(&dp83640->rx_pool_data[i].list, &dp83640->rxpool);
 
+	phydev->mii_ts = &dp83640->mii_ts;
 	phydev->priv = dp83640;
 
 	spin_lock_init(&dp83640->rx_lock);

@@ -1196,6 +1210,8 @@ static void dp83640_remove(struct phy_device *phydev)
 	if (phydev->mdio.addr == BROADCAST_ADDR)
 		return;
 
+	phydev->mii_ts = NULL;
+
 	enable_status_frames(phydev, false);
 	cancel_delayed_work_sync(&dp83640->ts_work);
 

@@ -1319,9 +1335,10 @@ static int dp83640_config_intr(struct phy_device *phydev)
 	}
 }
 
-static int dp83640_hwtstamp(struct phy_device *phydev, struct ifreq *ifr)
+static int dp83640_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
 {
-	struct dp83640_private *dp83640 = phydev->priv;
+	struct dp83640_private *dp83640 =
+		container_of(mii_ts, struct dp83640_private, mii_ts);
 	struct hwtstamp_config cfg;
 	u16 txcfg0, rxcfg0;
 

@@ -1397,8 +1414,8 @@ static int dp83640_hwtstamp(struct phy_device *phydev, struct ifreq *ifr)
 
 	mutex_lock(&dp83640->clock->extreg_lock);
 
-	ext_write(0, phydev, PAGE5, PTP_TXCFG0, txcfg0);
-	ext_write(0, phydev, PAGE5, PTP_RXCFG0, rxcfg0);
+	ext_write(0, dp83640->phydev, PAGE5, PTP_TXCFG0, txcfg0);
+	ext_write(0, dp83640->phydev, PAGE5, PTP_RXCFG0, rxcfg0);
 
 	mutex_unlock(&dp83640->clock->extreg_lock);
 

@@ -1428,10 +1445,11 @@ static void rx_timestamp_work(struct work_struct *work)
 		schedule_delayed_work(&dp83640->ts_work, SKB_TIMESTAMP_TIMEOUT);
 }
 
-static bool dp83640_rxtstamp(struct phy_device *phydev,
+static bool dp83640_rxtstamp(struct mii_timestamper *mii_ts,
 			     struct sk_buff *skb, int type)
 {
-	struct dp83640_private *dp83640 = phydev->priv;
+	struct dp83640_private *dp83640 =
+		container_of(mii_ts, struct dp83640_private, mii_ts);
 	struct dp83640_skb_info *skb_info = (struct dp83640_skb_info *)skb->cb;
 	struct list_head *this, *next;
 	struct rxts *rxts;

@@ -1477,11 +1495,12 @@ static bool dp83640_rxtstamp(struct phy_device *phydev,
 	return true;
 }
 
-static void dp83640_txtstamp(struct phy_device *phydev,
+static void dp83640_txtstamp(struct mii_timestamper *mii_ts,
 			     struct sk_buff *skb, int type)
 {
 	struct dp83640_skb_info *skb_info = (struct dp83640_skb_info *)skb->cb;
-	struct dp83640_private *dp83640 = phydev->priv;
+	struct dp83640_private *dp83640 =
+		container_of(mii_ts, struct dp83640_private, mii_ts);
 
 	switch (dp83640->hwts_tx_en) {
 

@@ -1504,9 +1523,11 @@ static void dp83640_txtstamp(struct phy_device *phydev,
 	}
 }
 
-static int dp83640_ts_info(struct phy_device *dev, struct ethtool_ts_info *info)
+static int dp83640_ts_info(struct mii_timestamper *mii_ts,
+			   struct ethtool_ts_info *info)
 {
-	struct dp83640_private *dp83640 = dev->priv;
+	struct dp83640_private *dp83640 =
+		container_of(mii_ts, struct dp83640_private, mii_ts);
 
 	info->so_timestamping =
 		SOF_TIMESTAMPING_TX_HARDWARE |

@@ -1537,10 +1558,6 @@ static struct phy_driver dp83640_driver = {
 	.config_init	= dp83640_config_init,
 	.ack_interrupt  = dp83640_ack_interrupt,
 	.config_intr    = dp83640_config_intr,
-	.ts_info	= dp83640_ts_info,
-	.hwtstamp	= dp83640_hwtstamp,
-	.rxtstamp	= dp83640_rxtstamp,
-	.txtstamp	= dp83640_txtstamp,
 };
 
 static int __init dp83640_init(void)
drivers/net/phy/phy.c +2 -2
index 80be4d691e5b..541ed01496bf 100644
@@ -422,8 +422,8 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
 		return 0;
 
 	case SIOCSHWTSTAMP:
-		if (phydev->drv && phydev->drv->hwtstamp)
-			return phydev->drv->hwtstamp(phydev, ifr);
+		if (phydev->mii_ts && phydev->mii_ts->hwtstamp)
+			return phydev->mii_ts->hwtstamp(phydev->mii_ts, ifr);
 		/* fall through */
 
 	default:
drivers/net/phy/phy_device.c +2 -0
index 0887ed2bb050..ee45838f90c9 100644
@@ -919,6 +919,8 @@ static void phy_link_change(struct phy_device *phydev, bool up, bool do_carrier)
 			netif_carrier_off(netdev);
 	}
 	phydev->adjust_link(netdev);
+	if (phydev->mii_ts && phydev->mii_ts->link_state)
+		phydev->mii_ts->link_state(phydev->mii_ts, phydev);
 }
 
 /**
include/linux/mii_timestamper.h new file +58 -0

58 lines not on this page · raw message

include/linux/phy.h +10 -31

102 lines not on this page · raw message

net/core/timestamping.c +10 -10
index 7911235706a9..04840697fe79 100644
@@ -13,7 +13,7 @@
 static unsigned int classify(const struct sk_buff *skb)
 {
 	if (likely(skb->dev && skb->dev->phydev &&
-		   skb->dev->phydev->drv))
+		   skb->dev->phydev->mii_ts))
 		return ptp_classify_raw(skb);
 	else
 		return PTP_CLASS_NONE;

@@ -21,7 +21,7 @@ static unsigned int classify(const struct sk_buff *skb)
 
 void skb_clone_tx_timestamp(struct sk_buff *skb)
 {
-	struct phy_device *phydev;
+	struct mii_timestamper *mii_ts;
 	struct sk_buff *clone;
 	unsigned int type;
 

@@ -32,22 +32,22 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
 	if (type == PTP_CLASS_NONE)
 		return;
 
-	phydev = skb->dev->phydev;
-	if (likely(phydev->drv->txtstamp)) {
+	mii_ts = skb->dev->phydev->mii_ts;
+	if (likely(mii_ts->txtstamp)) {
 		clone = skb_clone_sk(skb);
 		if (!clone)
 			return;
-		phydev->drv->txtstamp(phydev, clone, type);
+		mii_ts->txtstamp(mii_ts, clone, type);
 	}
 }
 EXPORT_SYMBOL_GPL(skb_clone_tx_timestamp);
 
 bool skb_defer_rx_timestamp(struct sk_buff *skb)
 {
-	struct phy_device *phydev;
+	struct mii_timestamper *mii_ts;
 	unsigned int type;
 
-	if (!skb->dev || !skb->dev->phydev || !skb->dev->phydev->drv)
+	if (!skb->dev || !skb->dev->phydev || !skb->dev->phydev->mii_ts)
 		return false;
 
 	if (skb_headroom(skb) < ETH_HLEN)

@@ -62,9 +62,9 @@ bool skb_defer_rx_timestamp(struct sk_buff *skb)
 	if (type == PTP_CLASS_NONE)
 		return false;
 
-	phydev = skb->dev->phydev;
-	if (likely(phydev->drv->rxtstamp))
-		return phydev->drv->rxtstamp(phydev, skb, type);
+	mii_ts = skb->dev->phydev->mii_ts;
+	if (likely(mii_ts->rxtstamp))
+		return mii_ts->rxtstamp(mii_ts, skb, type);
 
 	return false;
 }
Signature · 1 line
2.20.1
maintainer
#12 lore raw

Re: [PATCH V6 net-next 06/11] net: Introduce a new MII time stamping interface.

drivers/net/phy/dp83640.c static void dp83640_gpio_defaults(struct ptp_pin_desc *pd) · line 213

Quoted 9 lines
drivers/net/phy/dp83640.c +8 -0
+static int dp83640_hwtstamp(struct mii_timestamper *mii_ts,
+			    struct ifreq *ifr);
+static int dp83640_ts_info(struct mii_timestamper *mii_ts,
+			   struct ethtool_ts_info *info);
+static bool dp83640_rxtstamp(struct mii_timestamper *mii_ts,
+			     struct sk_buff *skb, int type);
+static void dp83640_txtstamp(struct mii_timestamper *mii_ts,
+			     struct sk_buff *skb, int type);
 static void rx_timestamp_work(struct work_struct *work);

Hi Richard

Forward declarations are considered bad. Please add a new patch to the series which moves code around first. You can probably move dp83640_probe() further down.

drivers/net/phy/dp83640.c static void rx_timestamp_work(struct work_struct *work) · line 1455

Quoted 10 lines
drivers/net/phy/dp83640.c +3 -2
-static bool dp83640_rxtstamp(struct phy_device *phydev,
+static bool dp83640_rxtstamp(struct mii_timestamper *mii_ts,
 			     struct sk_buff *skb, int type)
 {
-	struct dp83640_private *dp83640 = phydev->priv;
+	struct dp83640_private *dp83640 =
+		container_of(mii_ts, struct dp83640_private, mii_ts);
 	struct dp83640_skb_info *skb_info = (struct dp83640_skb_info *)skb->cb;
 	struct list_head *this, *next;
 	struct rxts *rxts;

David will probably complain about reverse christmas tree. Having to fold dp83640_private is unfortunate here. Maybe consider adding a macro for the container_of() so you can avoid the fold?

drivers/net/phy/phy_device.c static void phy_link_change(struct phy_device *phydev, bool up, bool do_carrier) · line 924

Quoted 11 lines
drivers/net/phy/phy_device.c +2 -0
index 0887ed2bb050..ee45838f90c9 100644
@@ -919,6 +919,8 @@ static void phy_link_change(struct phy_device *phydev, bool up, bool do_carrier)
 			netif_carrier_off(netdev);
 	}
 	phydev->adjust_link(netdev);
+	if (phydev->mii_ts && phydev->mii_ts->link_state)
+		phydev->mii_ts->link_state(phydev->mii_ts, phydev);
 }

FYI:

When using phylink, not phylib, this call will not happen. You need to add a similar bit of code in phylink_mac_config().

For the moment what you have is sufficient. I doubt anybody is using the dp83640 with phylink, and the new hardware you are targeting seems to be RGMII based, not SERDES, which is the main use case for PHYLINK.

Andrew

author maintainer
#13 lore raw

Re: [PATCH V6 net-next 06/11] net: Introduce a new MII time stamping interface.

Quoted 1 line · Andrew Lunn

Forward declarations are considered bad.

Not by me!

Quoted 2 lines

Please add a new patch to the
series which moves code around first.

Sorry, I disagree. For new drivers, sure, but for testing, production drivers, moving code blocks around "just because" is only asking for new bugs due to copy-pastos.

Quoted 2 lines

When using phylink, not phylib, this call will not happen. You need to add a similar bit of code in phylink_mac_config().

Good to know.

Quoted 3 lines

For the moment what you have is sufficient. I doubt anybody is using the dp83640 with phylink, and the new hardware you are targeting seems to be RGMII based, not SERDES, which is the main use case for PHYLINK.

Yeah, my impression is that the phyter will be the first and last phy time stamping device ever created. Designers reject this part because it is 100 mbit only. And there are no gigabit+ phys with time stamping at all.

So I don't anticipate the phylink layer needing any of this time stamping stuff in the foreseeable future.

Thanks,
Richard

maintainer
#14 lore raw

Re: [PATCH V6 net-next 06/11] net: Introduce a new MII time stamping interface.

Quoted 4 lines · Richard Cochran
Quoted 1 line · Andrew Lunn

Forward declarations are considered bad.

Not by me!

Lets see what David says.

Quoted 8 lines
Quoted 3 lines

For the moment what you have is sufficient. I doubt anybody is using the dp83640 with phylink, and the new hardware you are targeting seems to be RGMII based, not SERDES, which is the main use case for PHYLINK.

Yeah, my impression is that the phyter will be the first and last phy time stamping device ever created. Designers reject this part because it is 100 mbit only. And there are no gigabit+ phys with time stamping at all.

The Marvell PHY datasheets indicate they support PTP. I've not looked at how they implement it, and if the current model will work.

Andrew

author maintainer
#15 lore raw

Re: [PATCH V6 net-next 06/11] net: Introduce a new MII time stamping interface.

Quoted 2 lines · Andrew Lunn

The Marvell PHY datasheets indicate they support PTP. I've not looked at how they implement it, and if the current model will work.

IIRC, those parts time stamp on the MII bus input, and not in the PHY itself, and so they offer no advantage over MAC time stamping. I suspect that is why there has been so little interest.

Thanks,
Richard

maintainer
#16 lore raw

Re: [PATCH V6 net-next 06/11] net: Introduce a new MII time stamping interface.

In reply to Andrew Lunn

From: Andrew Lunn <[email protected]>
Date: Fri, 20 Dec 2019 16:33:59 +0100

Quoted 7 lines · Richard Cochran
Quoted 4 lines · Andrew Lunn
Quoted 1 line

Forward declarations are considered bad.

Not by me!

Lets see what David says.

I strongly dislike them too, it's indicative of poorly organized code.

It's pointless and avoidable clutter, and always when they exist people never remember to remove them as code gets re-organized in the future.

Yeah, avoid forward declarations for sure.

author maintainer
#17 lore raw

PATCH v6 7/11 net: Add a layer for non-PHY MII time stamping drivers.

In reply to Richard Cochran

While PHY time stamping drivers can simply attach their interface directly to the PHY instance, stand alone drivers require support in order to manage their services. Non-PHY MII time stamping drivers have a control interface over another bus like I2C, SPI, UART, or via a memory mapped peripheral. The controller device will be associated with one or more time stamping channels, each of which sits snoops in on a MII bus.

This patch provides a glue layer that will enable time stamping channels to find their controlling device.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/phy/Makefile          |   2 +
 drivers/net/phy/mii_timestamper.c | 125 ++++++++++++++++++++++++++++++
 include/linux/mii_timestamper.h   |  63 +++++++++++++++
 net/Kconfig                       |   7 +-
 4 files changed, 194 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/phy/mii_timestamper.c
drivers/net/phy/Makefile +2 -0
index d846b4dc1c68..fe5badf13b65 100644
@@ -43,6 +43,8 @@ obj-$(CONFIG_MDIO_SUN4I)	+= mdio-sun4i.o
 obj-$(CONFIG_MDIO_THUNDER)	+= mdio-thunder.o
 obj-$(CONFIG_MDIO_XGENE)	+= mdio-xgene.o
 
+obj-$(CONFIG_NETWORK_PHY_TIMESTAMPING) += mii_timestamper.o
+
 obj-$(CONFIG_SFP)		+= sfp.o
 sfp-obj-$(CONFIG_SFP)		+= sfp-bus.o
 obj-y				+= $(sfp-obj-y) $(sfp-obj-m)
drivers/net/phy/mii_timestamper.c new file +125 -0

125 lines not on this page · raw message

include/linux/mii_timestamper.h +63 -0

79 lines not on this page · raw message

net/Kconfig +4 -3
index bd191f978a23..52af65e5d28c 100644
@@ -108,9 +108,10 @@ config NETWORK_PHY_TIMESTAMPING
 	bool "Timestamping in PHY devices"
 	select NET_PTP_CLASSIFY
 	help
-	  This allows timestamping of network packets by PHYs with
-	  hardware timestamping capabilities. This option adds some
-	  overhead in the transmit and receive paths.
+	  This allows timestamping of network packets by PHYs (or
+	  other MII bus snooping devices) with hardware timestamping
+	  capabilities. This option adds some overhead in the transmit
+	  and receive paths.
 
 	  If you are unsure how to answer this question, answer N.
Signature · 1 line
2.20.1
author maintainer
#18 lore raw

PATCH v6 8/11 dt-bindings: ptp: Introduce MII time stamping devices.

In reply to Richard Cochran

This patch add a new binding that allows non-PHY MII time stamping devices to find their buses. The new documentation covers both the generic binding and one upcoming user.

Signed-off-by Richard Cochran <[email protected]>
Reviewed-by Andrew Lunn <[email protected]>
---
 .../devicetree/bindings/ptp/ptp-ines.txt      | 35 ++++++++++++++++
 .../devicetree/bindings/ptp/timestamper.txt   | 41 +++++++++++++++++++
 2 files changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ptp/ptp-ines.txt
 create mode 100644 Documentation/devicetree/bindings/ptp/timestamper.txt
Documentation/devicetree/bindings/ptp/ptp-ines.txt new file +35 -0
new file mode 100644
index 000000000000..4c242bd1ce9c
@@ -0,0 +1,35 @@
+ZHAW InES PTP time stamping IP core
+
+The IP core needs two different kinds of nodes.  The control node
+lives somewhere in the memory map and specifies the address of the
+control registers.  There can be up to three port handles placed as
+attributes of PHY nodes.  These associate a particular MII bus with a
+port index within the IP core.
+
+Required properties of the control node:
+
+- compatible:		"ines,ptp-ctrl"
+- reg:			physical address and size of the register bank
+
+Required format of the port handle within the PHY node:
+
+- timestamper:		provides control node reference and
+			the port channel within the IP core
+
+Example:
+
+	tstamper: timestamper@60000000 {
+		compatible = "ines,ptp-ctrl";
+		reg = <0x60000000 0x80>;
+	};
+
+	ethernet@80000000 {
+		...
+		mdio {
+			...
+			ethernet-phy@3 {
+				...
+				timestamper = <&tstamper 0>;
+			};
+		};
+	};
Documentation/devicetree/bindings/ptp/timestamper.txt new file +41 -0

41 lines not on this page · raw message

Signature · 1 line
2.20.1
maintainer
#19 lore raw

Re: [PATCH V6 net-next 08/11] dt-bindings: ptp: Introduce MII time stamping devices.

Quoted 6 lines · Richard Cochran

This patch add a new binding that allows non-PHY MII time stamping devices to find their buses. The new documentation covers both the generic binding and one upcoming user.

Signed-off-by Richard Cochran <[email protected]>
Reviewed-by Andrew Lunn <[email protected]>

Rob might want YAML?

Quoted 35 lines
---
 .../devicetree/bindings/ptp/ptp-ines.txt      | 35 ++++++++++++++++
 .../devicetree/bindings/ptp/timestamper.txt   | 41 +++++++++++++++++++
 2 files changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ptp/ptp-ines.txt
 create mode 100644 Documentation/devicetree/bindings/ptp/timestamper.txt
Documentation/devicetree/bindings/ptp/ptp-ines.txt new file +22 -0
new file mode 100644
index 000000000000..4c242bd1ce9c
@@ -0,0 +1,35 @@
+ZHAW InES PTP time stamping IP core
+
+The IP core needs two different kinds of nodes.  The control node
+lives somewhere in the memory map and specifies the address of the
+control registers.  There can be up to three port handles placed as
+attributes of PHY nodes.  These associate a particular MII bus with a
+port index within the IP core.
+
+Required properties of the control node:
+
+- compatible:		"ines,ptp-ctrl"
+- reg:			physical address and size of the register bank
+
+Required format of the port handle within the PHY node:
+
+- timestamper:		provides control node reference and
+			the port channel within the IP core
+
+Example:
+
+	tstamper: timestamper@60000000 {
+		compatible = "ines,ptp-ctrl";

Documentation/devicetree/bindings/ptp/ptp-ines.txt line 32

Quoted 10 lines
Documentation/devicetree/bindings/ptp/ptp-ines.txt new file +10 -0
+		reg = <0x60000000 0x80>;
+	};
+
+	ethernet@80000000 {
+		...
+		mdio {
+			...
+			ethernet-phy@3 {
+				...
+				timestamper = <&tstamper 0>;

You should also add this to Documentation/devicetree/bindings/net/ethernet-phy.yaml

Andrew

maintainer
#20 lore raw

Re: [PATCH V6 net-next 08/11] dt-bindings: ptp: Introduce MII time stamping devices.

Quoted 9 lines · Andrew Lunn
Quoted 6 lines · Richard Cochran

This patch add a new binding that allows non-PHY MII time stamping devices to find their buses. The new documentation covers both the generic binding and one upcoming user.

Signed-off-by Richard Cochran <[email protected]>
Reviewed-by Andrew Lunn <[email protected]>

Rob might want YAML?

Yes, of course I would, but not if this is close to merging as it's on v6.

Rob

author maintainer
#21 lore raw

Re: [PATCH V6 net-next 08/11] dt-bindings: ptp: Introduce MII time stamping devices.

Quoted 5 lines · Rob Herring
Quoted 1 line · Andrew Lunn

Rob might want YAML?

Yes, of course I would, but not if this is close to merging as it's on v6.

How nice of you. I appreciate your leniency!

Thanks,
Richard

maintainer
#22 lore raw

Re: [PATCH V6 net-next 08/11] dt-bindings: ptp: Introduce MII time stamping devices.

In reply to Richard Cochran
Quoted 69 lines · Richard Cochran

This patch add a new binding that allows non-PHY MII time stamping devices to find their buses. The new documentation covers both the generic binding and one upcoming user.

Signed-off-by Richard Cochran <[email protected]>
Reviewed-by Andrew Lunn <[email protected]>
---
 .../devicetree/bindings/ptp/ptp-ines.txt      | 35 ++++++++++++++++
 .../devicetree/bindings/ptp/timestamper.txt   | 41 +++++++++++++++++++
 2 files changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ptp/ptp-ines.txt
 create mode 100644 Documentation/devicetree/bindings/ptp/timestamper.txt
Documentation/devicetree/bindings/ptp/ptp-ines.txt new file +35 -0
new file mode 100644
index 000000000000..4c242bd1ce9c
@@ -0,0 +1,35 @@
+ZHAW InES PTP time stamping IP core
+
+The IP core needs two different kinds of nodes.  The control node
+lives somewhere in the memory map and specifies the address of the
+control registers.  There can be up to three port handles placed as
+attributes of PHY nodes.  These associate a particular MII bus with a
+port index within the IP core.
+
+Required properties of the control node:
+
+- compatible:		"ines,ptp-ctrl"
+- reg:			physical address and size of the register bank
+
+Required format of the port handle within the PHY node:
+
+- timestamper:		provides control node reference and
+			the port channel within the IP core
+
+Example:
+
+	tstamper: timestamper@60000000 {
+		compatible = "ines,ptp-ctrl";
+		reg = <0x60000000 0x80>;
+	};
+
+	ethernet@80000000 {
+		...
+		mdio {
+			...
+			ethernet-phy@3 {
+				...
+				timestamper = <&tstamper 0>;
+			};
+		};
+	};
Documentation/devicetree/bindings/ptp/timestamper.txt new file +9 -0
new file mode 100644
index 000000000000..70d636350582
@@ -0,0 +1,41 @@
+Time stamps from MII bus snooping devices
+
+This binding supports non-PHY devices that snoop the MII bus and
+provide time stamps.  In contrast to PHY time stamping drivers (which
+can simply attach their interface directly to the PHY instance), stand
+alone MII time stamping drivers use this binding to specify the
+connection between the snooping device and a given network interface.
+
+Non-PHY MII time stamping drivers typically talk to the control

Documentation/devicetree/bindings/ptp/timestamper.txt line 19

Quoted 10 lines
Documentation/devicetree/bindings/ptp/timestamper.txt new file +10 -0
+interface over another bus like I2C, SPI, UART, or via a memory mapped
+peripheral.  This controller device is associated with one or more
+time stamping channels, each of which snoops on a MII bus.
+
+The "timestamper" property lives in a phy node and links a time
+stamping channel from the controller device to that phy's MII bus.
+
+Example:
+
+	tstamper: timestamper@10000000 {

unit-address needs a reg property.

Documentation/devicetree/bindings/ptp/timestamper.txt line 20

Quoted 1 line
Documentation/devicetree/bindings/ptp/timestamper.txt new file +1 -0
+		compatible = "ines,ts-ctrl";

Should be ines,ptp-ctrl?

Quoted 24 lines
Documentation/devicetree/bindings/ptp/timestamper.txt new file +21 -0
+	};
+
+	ethernet@20000000 {
+		mdio {
+			ethernet-phy@1 {
+				timestamper = <&tstamper 0>;
+			};
+		};
+	};
+
+	ethernet@30000000 {
+		mdio {
+			ethernet-phy@2 {
+				timestamper = <&tstamper 1>;
+			};
+		};
+	};
+
+In this example, time stamps from the MII bus attached to phy@1 will
+appear on time stamp channel 0 (zero), and those from phy@2 appear on
+channel 1.
Signature · 1 line
2.20.1
author maintainer
#23 lore raw

PATCH v6 9/11 net: mdio: of: Register discovered MII time stampers.

In reply to Richard Cochran

When parsing a PHY node, register its time stamper, if any, and attach the instance to the PHY device.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/phy/phy_device.c |  3 +++
 drivers/of/of_mdio.c         | 30 +++++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)
drivers/net/phy/phy_device.c +3 -0
index ee45838f90c9..debbda61e12b 100644
@@ -881,6 +881,9 @@ EXPORT_SYMBOL(phy_device_register);
  */
 void phy_device_remove(struct phy_device *phydev)
 {
+	if (phydev->mii_ts)
+		unregister_mii_timestamper(phydev->mii_ts);
+
 	device_del(&phydev->mdio.dev);
 
 	/* Assert the reset signal */
drivers/of/of_mdio.c +29 -1
index c6b87ce2b0cc..0b7aee235813 100644
@@ -42,14 +42,37 @@ static int of_get_phy_id(struct device_node *device, u32 *phy_id)
 	return -EINVAL;
 }
 
+static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
+{
+	struct of_phandle_args arg;
+	int err;
+
+	err = of_parse_phandle_with_fixed_args(node, "timestamper", 1, 0, &arg);
+
+	if (err == -ENOENT)
+		return NULL;
+	else if (err)
+		return ERR_PTR(err);
+
+	if (arg.args_count != 1)
+		return ERR_PTR(-EINVAL);
+
+	return register_mii_timestamper(arg.np, arg.args[0]);
+}
+
 static int of_mdiobus_register_phy(struct mii_bus *mdio,
 				    struct device_node *child, u32 addr)
 {
+	struct mii_timestamper *mii_ts;
 	struct phy_device *phy;
 	bool is_c45;
 	int rc;
 	u32 phy_id;
 
+	mii_ts = of_find_mii_timestamper(child);
+	if (IS_ERR(mii_ts))
+		return PTR_ERR(mii_ts);
+
 	is_c45 = of_device_is_compatible(child,
 					 "ethernet-phy-ieee802.3-c45");
 

@@ -57,11 +80,14 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
 		phy = phy_device_create(mdio, addr, phy_id, 0, NULL);
 	else
 		phy = get_phy_device(mdio, addr, is_c45);
-	if (IS_ERR(phy))
+	if (IS_ERR(phy)) {
+		unregister_mii_timestamper(mii_ts);
 		return PTR_ERR(phy);
+	}
 
 	rc = of_irq_get(child, 0);
 	if (rc == -EPROBE_DEFER) {
+		unregister_mii_timestamper(mii_ts);
 		phy_device_free(phy);
 		return rc;
 	}

@@ -90,10 +116,12 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
 	 * register it */
 	rc = phy_device_register(phy);
 	if (rc) {
+		unregister_mii_timestamper(mii_ts);
 		phy_device_free(phy);
 		of_node_put(child);
 		return rc;
 	}
+	phy->mii_ts = mii_ts;
 
 	dev_dbg(&mdio->dev, "registered phy %pOFn at address %i\n",
 		child, addr);
Signature · 1 line
2.20.1
maintainer Reviewed-by on v6 9/11
#25 lore raw

Re: [PATCH V6 net-next 09/11] net: mdio: of: Register discovered MII time stampers.

In reply to Richard Cochran
Quoted 9 lines · Richard Cochran

When parsing a PHY node, register its time stamper, if any, and attach the instance to the PHY device.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/phy/phy_device.c |  3 +++
 drivers/of/of_mdio.c         | 30 +++++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)
Reviewed-by Rob Herring <[email protected]>
author maintainer
#26 lore raw

PATCH v6 10/11 net: Introduce peer to peer one step PTP time stamping.

In reply to Richard Cochran

The 1588 standard defines one step operation for both Sync and PDelay_Resp messages. Up until now, hardware with P2P one step has been rare, and kernel support was lacking. This patch adds support of the mode in anticipation of new hardware developments.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 1 +
 include/uapi/linux/net_tstamp.h                  | 8 ++++++++
 net/core/dev_ioctl.c                             | 1 +
 3 files changed, 10 insertions(+)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +1 -0
index 192ff8d5da32..7343d7a28327 100644
@@ -15402,6 +15402,7 @@ int bnx2x_configure_ptp_filters(struct bnx2x *bp)
 		REG_WR(bp, rule, BNX2X_PTP_TX_ON_RULE_MASK);
 		break;
 	case HWTSTAMP_TX_ONESTEP_SYNC:
+	case HWTSTAMP_TX_ONESTEP_P2P:
 		BNX2X_ERR("One-step timestamping is not supported\n");
 		return -ERANGE;
 	}
include/uapi/linux/net_tstamp.h +8 -0
index e5b39721c6e4..f96e650d0af9 100644
@@ -90,6 +90,14 @@ enum hwtstamp_tx_types {
 	 * queue.
 	 */
 	HWTSTAMP_TX_ONESTEP_SYNC,
+
+	/*
+	 * Same as HWTSTAMP_TX_ONESTEP_SYNC, but also enables time
+	 * stamp insertion directly into PDelay_Resp packets. In this
+	 * case, neither transmitted Sync nor PDelay_Resp packets will
+	 * receive a time stamp via the socket error queue.
+	 */
+	HWTSTAMP_TX_ONESTEP_P2P,
 };
 
 /* possible values for hwtstamp_config->rx_filter */
net/core/dev_ioctl.c +1 -0
index 5163d900bb4f..dbaebbe573f0 100644
@@ -187,6 +187,7 @@ static int net_hwtstamp_validate(struct ifreq *ifr)
 	case HWTSTAMP_TX_OFF:
 	case HWTSTAMP_TX_ON:
 	case HWTSTAMP_TX_ONESTEP_SYNC:
+	case HWTSTAMP_TX_ONESTEP_P2P:
 		tx_type_valid = 1;
 		break;
 	}
Signature · 1 line
2.20.1
author maintainer
#27 lore raw

PATCH v6 11/11 ptp: Add a driver for InES time stamping IP core.

In reply to Richard Cochran

The InES at the ZHAW offers a PTP time stamping IP core. The FPGA logic recognizes and time stamps PTP frames on the MII bus. This patch adds a driver for the core along with a device tree binding to allow hooking the driver to MII buses.

Signed-off-by Richard Cochran <[email protected]>
---
 drivers/ptp/Kconfig    |  10 +
 drivers/ptp/Makefile   |   1 +
 drivers/ptp/ptp_ines.c | 859 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 870 insertions(+)
 create mode 100644 drivers/ptp/ptp_ines.c
drivers/ptp/Kconfig +10 -0
index b45d2b86d8ca..e466b3586754 100644
@@ -89,6 +89,16 @@ config DP83640_PHY
 	  In order for this to work, your MAC driver must also
 	  implement the skb_tx_timestamp() function.
 
+config PTP_1588_CLOCK_INES
+	tristate "ZHAW InES PTP time stamping IP core"
+	depends on NETWORK_PHY_TIMESTAMPING
+	depends on PHYLIB
+	depends on PTP_1588_CLOCK
+	help
+	  This driver adds support for using the ZHAW InES 1588 IP
+	  core.  This clock is only useful if the MII bus of your MAC
+	  is wired up to the core.
+
 config PTP_1588_CLOCK_PCH
 	tristate "Intel PCH EG20T as PTP clock"
 	depends on X86_32 || COMPILE_TEST
drivers/ptp/Makefile +1 -0
index 69a06f86a450..3fb91bebbaf7 100644
@@ -6,6 +6,7 @@
 ptp-y					:= ptp_clock.o ptp_chardev.o ptp_sysfs.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
 obj-$(CONFIG_PTP_1588_CLOCK_IXP46X)	+= ptp_ixp46x.o
 obj-$(CONFIG_PTP_1588_CLOCK_PCH)	+= ptp_pch.o
 obj-$(CONFIG_PTP_1588_CLOCK_KVM)	+= ptp_kvm.o
drivers/ptp/ptp_ines.c new file +859 -0

859 lines not on this page · raw message

Signature · 1 line
2.20.1
#28 lore raw

Re: [PATCH V6 net-next 11/11] ptp: Add a driver for InES time stamping IP core.

drivers/ptp/ptp_ines.c line 483

Quoted 3 lines · Richard Cochran
drivers/ptp/ptp_ines.c new file +3 -0
+	clkid = (u64 *)(data + offset + OFF_PTP_CLOCK_ID);
+	portn = (u16 *)(data + offset + OFF_PTP_PORT_NUM);
+	seqid = (u16 *)(data + offset + OFF_PTP_SEQUENCE_ID);

These should perhaps be __be types?

Looks like there is a few other sparse warnings in ptp_ines.c, would you mind addressing those?

Quoted 20 lines
drivers/ptp/ptp_ines.c new file +20 -0
+	if (tag_to_msgtype(ts->tag & 0x7) != (*msgtype & 0xf)) {
+		pr_debug("msgtype mismatch ts %hhu != skb %hhu\n",
+			 tag_to_msgtype(ts->tag & 0x7), *msgtype & 0xf);
+		return false;
+	}
+	if (cpu_to_be64(ts->clkid) != *clkid) {
+		pr_debug("clkid mismatch ts %llx != skb %llx\n",
+			 cpu_to_be64(ts->clkid), *clkid);
+		return false;
+	}
+	if (ts->portnum != ntohs(*portn)) {
+		pr_debug("portn mismatch ts %hu != skb %hu\n",
+			 ts->portnum, ntohs(*portn));
+		return false;
+	}
+	if (ts->seqid != ntohs(*seqid)) {
+		pr_debug("seqid mismatch ts %hu != skb %hu\n",
+			 ts->seqid, ntohs(*seqid));
+		return false;
+	}
author maintainer
#29 lore raw

Re: [PATCH V6 net-next 11/11] ptp: Add a driver for InES time stamping IP core.

Quoted 9 lines · Jakub Kicinski

drivers/ptp/ptp_ines.c line 483

Quoted 3 lines · Richard Cochran
drivers/ptp/ptp_ines.c new file +3 -0
+	clkid = (u64 *)(data + offset + OFF_PTP_CLOCK_ID);
+	portn = (u16 *)(data + offset + OFF_PTP_PORT_NUM);
+	seqid = (u16 *)(data + offset + OFF_PTP_SEQUENCE_ID);

These should perhaps be __be types?

Looks like there is a few other sparse warnings in ptp_ines.c, would you mind addressing those?

I saw the sparse warnings before (from one of the robots), but I decided that they are false positives. Or perhaps I don't appreciate what the warnings mean...

Take the 'clkid' pointer for example:

Quoted 5 lines

drivers/ptp/ptp_ines.c line 494

Quoted 5 lines
drivers/ptp/ptp_ines.c new file +5 -0
+	if (cpu_to_be64(ts->clkid) != *clkid) {
+		pr_debug("clkid mismatch ts %llx != skb %llx\n",
+			 cpu_to_be64(ts->clkid), *clkid);
+		return false;
+	}

The field that to which 'clkid' points is in network byte order. The code correctly converts ts->clkid (in CPU byte order) to network byte order before comparing it with the field.

So where is the error?

Thanks,
Richard

#30 lore raw

Re: [PATCH V6 net-next 11/11] ptp: Add a driver for InES time stamping IP core.

Quoted 27 lines · Richard Cochran
Quoted 9 lines · Jakub Kicinski

drivers/ptp/ptp_ines.c line 483

Quoted 3 lines · Richard Cochran
drivers/ptp/ptp_ines.c new file +3 -0
+	clkid = (u64 *)(data + offset + OFF_PTP_CLOCK_ID);
+	portn = (u16 *)(data + offset + OFF_PTP_PORT_NUM);
+	seqid = (u16 *)(data + offset + OFF_PTP_SEQUENCE_ID);

These should perhaps be __be types?

Looks like there is a few other sparse warnings in ptp_ines.c, would you mind addressing those?

I saw the sparse warnings before (from one of the robots), but I decided that they are false positives. Or perhaps I don't appreciate what the warnings mean...

Take the 'clkid' pointer for example:

Quoted 5 lines

drivers/ptp/ptp_ines.c line 494

Quoted 5 lines
drivers/ptp/ptp_ines.c new file +5 -0
+	if (cpu_to_be64(ts->clkid) != *clkid) {
+		pr_debug("clkid mismatch ts %llx != skb %llx\n",
+			 cpu_to_be64(ts->clkid), *clkid);
+		return false;
+	}

The field that to which 'clkid' points is in network byte order. The code correctly converts ts->clkid (in CPU byte order) to network byte order before comparing it with the field.

Quoted 1 line

So where is the error?

Not necessarily an error as much as a sparse warning, if the type of clkid was __be64 that'd make sparse happy.

This is what my build system spat out for a W=1 C=1 build:

../drivers/ptp/ptp_ines.c:490:13: warning: restricted __be64 degrades to integer ../drivers/ptp/ptp_ines.c:495:28: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:495:28: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:495:28: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:495:28: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:496:17: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:496:17: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:496:17: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:496:17: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:500:26: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:500:26: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:500:26: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:500:26: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:501:17: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:501:17: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:501:17: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:501:17: warning: cast to restricted __be16 ../drivers/ptp/ptp_ines.c:543:28: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:543:28: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:543:28: got unsigned int *
../drivers/ptp/ptp_ines.c:547:30: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:547:30: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:547:30: got unsigned int *
../drivers/ptp/ptp_ines.c:557:31: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:557:31: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:557:31: got unsigned int *
../drivers/ptp/ptp_ines.c:561:31: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:561:31: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:561:31: got unsigned int *
../drivers/ptp/ptp_ines.c:562:31: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:562:31: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:562:31: got unsigned int *
../drivers/ptp/ptp_ines.c:579:16: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:579:16: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:579:16: got unsigned int *
../drivers/ptp/ptp_ines.c:583:24: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:583:24: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:583:24: got unsigned int *
../drivers/ptp/ptp_ines.c:626:16: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:626:16: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:626:16: got unsigned int *
../drivers/ptp/ptp_ines.c:630:24: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:630:24: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:630:24: got unsigned int *
../drivers/ptp/ptp_ines.c:208:21: warning: incorrect type in assignment (different address spaces)
../drivers/ptp/ptp_ines.c:208:21: expected struct ines_global_registers *regs
../drivers/ptp/ptp_ines.c:208:21: got void [noderef] <asn:2> *base
../drivers/ptp/ptp_ines.c:225:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/ptp/ptp_ines.c:225:9: expected void volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:225:9: got unsigned int *
../drivers/ptp/ptp_ines.c:226:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/ptp/ptp_ines.c:226:9: expected void volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:226:9: got unsigned int *
../drivers/ptp/ptp_ines.c:228:9: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:228:9: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:228:9: got unsigned int *
../drivers/ptp/ptp_ines.c:229:9: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:229:9: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:229:9: got unsigned int *
../drivers/ptp/ptp_ines.c:230:9: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:230:9: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:230:9: got unsigned int *
../drivers/ptp/ptp_ines.c:231:9: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:231:9: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:231:9: got unsigned int *
../drivers/ptp/ptp_ines.c:235:17: warning: incorrect type in argument 2 (different address spaces)
../drivers/ptp/ptp_ines.c:235:17: expected void volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:235:17: got unsigned int *
../drivers/ptp/ptp_ines.c:313:28: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:313:28: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:313:28: got unsigned int *
../drivers/ptp/ptp_ines.c:318:30: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:318:30: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:318:30: got unsigned int *
../drivers/ptp/ptp_ines.c:326:30: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:326:30: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:326:30: got unsigned int *
../drivers/ptp/ptp_ines.c:330:30: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:330:30: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:330:30: got unsigned int *
../drivers/ptp/ptp_ines.c:331:30: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:331:30: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:331:30: got unsigned int *
../drivers/ptp/ptp_ines.c:401:21: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:401:21: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:401:21: got unsigned int *
../drivers/ptp/ptp_ines.c:405:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/ptp/ptp_ines.c:405:9: expected void volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:405:9: got unsigned int *
../drivers/ptp/ptp_ines.c:406:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/ptp/ptp_ines.c:406:9: expected void volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:406:9: got unsigned int *
../drivers/ptp/ptp_ines.c:407:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/ptp/ptp_ines.c:407:9: expected void volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:407:9: got unsigned int *
../drivers/ptp/ptp_ines.c:440:21: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:440:21: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:440:21: got unsigned int *
../drivers/ptp/ptp_ines.c:444:9: warning: incorrect type in argument 2 (different address spaces)
../drivers/ptp/ptp_ines.c:444:9: expected void volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:444:9: got unsigned int *
../drivers/ptp/ptp_ines.c:643:21: warning: incorrect type in argument 1 (different address spaces)
../drivers/ptp/ptp_ines.c:643:21: expected void const volatile [noderef] <asn:2> *addr
../drivers/ptp/ptp_ines.c:643:21: got unsigned int *

New errors added

maintainer
#31 lore raw

Re: [PATCH V6 net-next 11/11] ptp: Add a driver for InES time stamping IP core.

In reply to Richard Cochran
Quoted 25 lines
drivers/ptp/ptp_ines.c new file +25 -0
+static int ines_clock_init(struct ines_clock *clock, struct device_node *node,
+			   void __iomem *addr)
+{
+	unsigned long port_addr;
+	struct ines_port *port;
+	int i, j;
+
+	INIT_LIST_HEAD(&clock->list);
+	clock->node = node;
+	clock->base = addr;
+	clock->regs = clock->base;
+
+	for (i = 0; i < INES_N_PORTS; i++) {
+		port = &clock->port[i];
+		port_addr = (unsigned long) clock->base +
+			INES_PORT_OFFSET + i * INES_PORT_SIZE;
+		port->regs = (struct ines_port_registers *) port_addr;
+		port->clock = clock;
+		port->index = i;
+		INIT_DELAYED_WORK(&port->ts_work, ines_txtstamp_work);
+		spin_lock_init(&port->lock);
+		INIT_LIST_HEAD(&port->events);
+		INIT_LIST_HEAD(&port->pool);
+		for (j = 0; j < INES_MAX_EVENTS; j++)
+			list_add(&port->pool_data[j].list, &port->pool);

drivers/ptp/ptp_ines.c line 232

Quoted 10 lines
drivers/ptp/ptp_ines.c new file +10 -0
+	}
+
+	ines_write32(clock, 0xBEEF, test);
+	ines_write32(clock, 0xBEEF, test2);
+
+	pr_debug("ID      0x%x\n", ines_read32(clock, id));
+	pr_debug("TEST    0x%x\n", ines_read32(clock, test));
+	pr_debug("VERSION 0x%x\n", ines_read32(clock, version));
+	pr_debug("TEST2   0x%x\n", ines_read32(clock, test2));
+

Hi Richard

Using pr_ functions is frowned upon. You have a device, since this is a platform driver, please use dev_debug() etc.

Quoted 5 lines
drivers/ptp/ptp_ines.c new file +5 -0
+	for (i = 0; i < INES_N_PORTS; i++) {
+		port = &clock->port[i];
+		ines_write32(port, PORT_CONF, port_conf);
+	}
+

drivers/ptp/ptp_ines.c line 247

Quoted 10 lines
drivers/ptp/ptp_ines.c new file +10 -0
+	return 0;
+}
+
+static void ines_dump_ts(char *label, struct ines_timestamp *ts)
+{
+#ifdef DEBUG
+	pr_err("%s timestamp, tag=0x%04hx t=%llu.%9llu c=0x%llx p=%hu s=%hu\n",
+	       label, ts->tag, ts->sec, ts->nsec,
+	       ts->clkid, ts->portnum, ts->seqid);
+#endif

DEBUG using pr_err() is a bit unusual. dev_dbg() would be normal.

Quoted 8 lines
drivers/ptp/ptp_ines.c new file +8 -0
+static int ines_ptp_ctrl_probe(struct platform_device *pld)
+{
+	struct ines_clock *clock;
+	struct resource *res;
+	void __iomem *addr;
+	int err = 0;
+
+	res = platform_get_resource(pld, IORESOURCE_MEM, 0);

drivers/ptp/ptp_ines.c line 807

Quoted 10 lines
drivers/ptp/ptp_ines.c new file +10 -0
+	if (!res) {
+		dev_err(&pld->dev, "missing memory resource\n");
+		return -EINVAL;
+	}
+	addr = devm_ioremap_resource(&pld->dev, res);
+	if (IS_ERR(addr)) {
+		err = PTR_ERR(addr);
+		goto out;
+	}
+	clock = kzalloc(sizeof(*clock), GFP_KERNEL);

Do the different memory life cycles allow devm_ to be used here?