All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kory Maincent <kory.maincent@bootlin.com>
To: Florian Fainelli <florian.fainelli@broadcom.com>,
	 Broadcom internal kernel review list
	<bcm-kernel-feedback-list@broadcom.com>,
	 Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	 Russell King <linux@armlinux.org.uk>,
	 "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	 Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	 Richard Cochran <richardcochran@gmail.com>,
	 Radu Pirea <radu-nicolae.pirea@oss.nxp.com>,
	 Jay Vosburgh <j.vosburgh@gmail.com>,
	Andy Gospodarek <andy@greyhouse.net>,
	 Nicolas Ferre <nicolas.ferre@microchip.com>,
	 Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	 Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	 Jonathan Corbet <corbet@lwn.net>,
	 Horatiu Vultur <horatiu.vultur@microchip.com>,
	UNGLinuxDriver@microchip.com,  Simon Horman <horms@kernel.org>,
	Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	netdev@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org,
	 Maxime Chevallier <maxime.chevallier@bootlin.com>,
	 Rahul Rameshbabu <rrameshbabu@nvidia.com>,
	 Kory Maincent <kory.maincent@bootlin.com>
Subject: [PATCH RFC net-next v8 04/13] net: Change the API of PHY default timestamp to MAC
Date: Fri, 16 Feb 2024 16:52:22 +0100	[thread overview]
Message-ID: <20240216-feature_ptp_netnext-v8-4-510f42f444fb@bootlin.com> (raw)
In-Reply-To: <20240216-feature_ptp_netnext-v8-0-510f42f444fb@bootlin.com>

Change the API to select MAC default time stamping instead of the PHY.
Indeed the PHY is closer to the wire therefore theoretically it has less
delay than the MAC timestamping but the reality is different. Due to lower
time stamping clock frequency, latency in the MDIO bus and no PHC hardware
synchronization between different PHY, the PHY PTP is often less precise
than the MAC. The exception is for PHY designed specially for PTP case but
these devices are not very widespread. For not breaking the compatibility
default_timestamp flag has been introduced in phy_device that is set by
the phy driver to know we are using the old API behavior.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Changes in v5:
- Extract the API change in this patch.
- Rename whitelist to allowlist.
- Set NETDEV_TIMESTAMPING in register_netdevice function.
- Add software timestamping case description in ts_info.

Change in v6:
- Replace the allowlist phy with a default_timestamp flag to know which
  phy is using old API behavior.
- Fix dereferenced of a possible null pointer.
- Follow timestamping layer naming update.
- Update timestamp default set between MAC and software.
- Update ts_info returned in case of software timestamping.

Change in v8:
- Reform the implementation to use a simple phy_is_default_hwtstamp helper
  instead of saving the hwtstamp in the net_device struct.
---
 drivers/net/phy/bcm-phy-ptp.c     |  3 +++
 drivers/net/phy/dp83640.c         |  3 +++
 drivers/net/phy/micrel.c          |  6 ++++++
 drivers/net/phy/mscc/mscc_ptp.c   |  3 +++
 drivers/net/phy/nxp-c45-tja11xx.c |  3 +++
 include/linux/phy.h               | 17 +++++++++++++++++
 net/core/dev_ioctl.c              |  8 +++-----
 net/core/timestamping.c           | 10 ++++++++--
 net/ethtool/common.c              |  2 +-
 9 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/bcm-phy-ptp.c b/drivers/net/phy/bcm-phy-ptp.c
index 617d384d4551..d3e825c951ee 100644
--- a/drivers/net/phy/bcm-phy-ptp.c
+++ b/drivers/net/phy/bcm-phy-ptp.c
@@ -931,6 +931,9 @@ struct bcm_ptp_private *bcm_ptp_probe(struct phy_device *phydev)
 		return ERR_CAST(clock);
 	priv->ptp_clock = clock;
 
+	/* Timestamp selected by default to keep legacy API */
+	phydev->default_timestamp = true;
+
 	priv->phydev = phydev;
 	bcm_ptp_init(priv);
 
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 5c42c47dc564..64fd1a109c0f 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -1450,6 +1450,9 @@ static int dp83640_probe(struct phy_device *phydev)
 	phydev->mii_ts = &dp83640->mii_ts;
 	phydev->priv = dp83640;
 
+	/* Timestamp selected by default to keep legacy API */
+	phydev->default_timestamp = true;
+
 	spin_lock_init(&dp83640->rx_lock);
 	skb_queue_head_init(&dp83640->rx_queue);
 	skb_queue_head_init(&dp83640->tx_queue);
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 9b6973581989..1c9eba331b01 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -3177,6 +3177,9 @@ static void lan8814_ptp_init(struct phy_device *phydev)
 	ptp_priv->mii_ts.ts_info  = lan8814_ts_info;
 
 	phydev->mii_ts = &ptp_priv->mii_ts;
+
+	/* Timestamp selected by default to keep legacy API */
+	phydev->default_timestamp = true;
 }
 
 static int lan8814_ptp_probe_once(struct phy_device *phydev)
@@ -4613,6 +4616,9 @@ static int lan8841_probe(struct phy_device *phydev)
 
 	phydev->mii_ts = &ptp_priv->mii_ts;
 
+	/* Timestamp selected by default to keep legacy API */
+	phydev->default_timestamp = true;
+
 	return 0;
 }
 
diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
index eb0b032cb613..e66d20eff7c4 100644
--- a/drivers/net/phy/mscc/mscc_ptp.c
+++ b/drivers/net/phy/mscc/mscc_ptp.c
@@ -1570,6 +1570,9 @@ int vsc8584_ptp_probe(struct phy_device *phydev)
 		return PTR_ERR(vsc8531->load_save);
 	}
 
+	/* Timestamp selected by default to keep legacy API */
+	phydev->default_timestamp = true;
+
 	vsc8531->ptp->phydev = phydev;
 
 	return 0;
diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
index 3cf614b4cd52..d18c133e6013 100644
--- a/drivers/net/phy/nxp-c45-tja11xx.c
+++ b/drivers/net/phy/nxp-c45-tja11xx.c
@@ -1660,6 +1660,9 @@ static int nxp_c45_probe(struct phy_device *phydev)
 		priv->mii_ts.ts_info = nxp_c45_ts_info;
 		phydev->mii_ts = &priv->mii_ts;
 		ret = nxp_c45_init_ptp_clock(priv);
+
+		/* Timestamp selected by default to keep legacy API */
+		phydev->default_timestamp = true;
 	} else {
 		phydev_dbg(phydev, "PTP support not enabled even if the phy supports it");
 	}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index c2dda21b39e1..9a31243e9f7e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -607,6 +607,8 @@ struct macsec_ops;
  *                 handling shall be postponed until PHY has resumed
  * @irq_rerun: Flag indicating interrupts occurred while PHY was suspended,
  *             requiring a rerun of the interrupt handler after resume
+ * @default_timestamp: Flag indicating whether we are using the phy
+ *		       timestamp as the default one
  * @interface: enum phy_interface_t value
  * @possible_interfaces: bitmap if interface modes that the attached PHY
  *			 will switch between depending on media speed.
@@ -672,6 +674,8 @@ struct phy_device {
 	unsigned irq_suspended:1;
 	unsigned irq_rerun:1;
 
+	unsigned default_timestamp:1;
+
 	int rate_matching;
 
 	enum phy_state state;
@@ -1613,6 +1617,19 @@ static inline void phy_txtstamp(struct phy_device *phydev, struct sk_buff *skb,
 	phydev->mii_ts->txtstamp(phydev->mii_ts, skb, type);
 }
 
+/**
+ * phy_is_default_hwtstamp - return true if phy is the default hw timestamp
+ * @phydev: Pointer to phy_device
+ *
+ * This is used to get default timestamping device taking into account
+ * the new API choice, which is selecting the timestamping from MAC by
+ * default if the phydev does not have default_timestamp flag enabled.
+ */
+static inline bool phy_is_default_hwtstamp(struct phy_device *phydev)
+{
+	return phy_has_hwtstamp(phydev) && phydev->default_timestamp;
+}
+
 /**
  * phy_is_internal - Convenience function for testing if a PHY is internal
  * @phydev: the phy_device struct
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 847254fd7f13..3342834597cd 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -260,9 +260,7 @@ static int dev_eth_ioctl(struct net_device *dev,
  * @dev: Network device
  * @cfg: Timestamping configuration structure
  *
- * Helper for enforcing a common policy that phylib timestamping, if available,
- * should take precedence in front of hardware timestamping provided by the
- * netdev.
+ * Helper for calling the default hardware provider timestamping.
  *
  * Note: phy_mii_ioctl() only handles SIOCSHWTSTAMP (not SIOCGHWTSTAMP), and
  * there only exists a phydev->mii_ts->hwtstamp() method. So this will return
@@ -272,7 +270,7 @@ static int dev_eth_ioctl(struct net_device *dev,
 int dev_get_hwtstamp_phylib(struct net_device *dev,
 			    struct kernel_hwtstamp_config *cfg)
 {
-	if (phy_has_hwtstamp(dev->phydev))
+	if (phy_is_default_hwtstamp(dev->phydev))
 		return phy_hwtstamp_get(dev->phydev, cfg);
 
 	return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
@@ -329,7 +327,7 @@ int dev_set_hwtstamp_phylib(struct net_device *dev,
 			    struct netlink_ext_ack *extack)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
-	bool phy_ts = phy_has_hwtstamp(dev->phydev);
+	bool phy_ts = phy_is_default_hwtstamp(dev->phydev);
 	struct kernel_hwtstamp_config old_cfg = {};
 	bool changed = false;
 	int err;
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index 04840697fe79..891bfc2f62fd 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -25,7 +25,10 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
 	struct sk_buff *clone;
 	unsigned int type;
 
-	if (!skb->sk)
+	if (!skb->sk || !skb->dev)
+		return;
+
+	if (!phy_is_default_hwtstamp(skb->dev->phydev))
 		return;
 
 	type = classify(skb);
@@ -47,7 +50,10 @@ bool skb_defer_rx_timestamp(struct sk_buff *skb)
 	struct mii_timestamper *mii_ts;
 	unsigned int type;
 
-	if (!skb->dev || !skb->dev->phydev || !skb->dev->phydev->mii_ts)
+	if (!skb->dev)
+		return false;
+
+	if (!phy_is_default_hwtstamp(skb->dev->phydev))
 		return false;
 
 	if (skb_headroom(skb) < ETH_HLEN)
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index ce486cec346c..e56bde53cd5c 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -637,7 +637,7 @@ int __ethtool_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
 	memset(info, 0, sizeof(*info));
 	info->cmd = ETHTOOL_GET_TS_INFO;
 
-	if (phy_has_tsinfo(phydev))
+	if (phy_is_default_hwtstamp(phydev) && phy_has_tsinfo(phydev))
 		return phy_ts_info(phydev, info);
 	if (ops->get_ts_info)
 		return ops->get_ts_info(dev, info);

-- 
2.25.1


  parent reply	other threads:[~2024-02-16 15:53 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-16 15:52 [PATCH RFC net-next v8 00/13] net: Make timestamping selectable Kory Maincent
2024-02-16 15:52 ` [PATCH RFC net-next v8 01/13] net_tstamp: Add TIMESTAMPING SOFTWARE and HARDWARE mask Kory Maincent
2024-02-16 18:43   ` Florian Fainelli
2024-02-16 15:52 ` [PATCH RFC net-next v8 02/13] net: Make dev_get_hwtstamp_phylib accessible Kory Maincent
2024-02-16 18:44   ` Florian Fainelli
2024-02-16 15:52 ` [PATCH RFC net-next v8 03/13] net: Make net_hwtstamp_validate accessible Kory Maincent
2024-02-16 18:44   ` Florian Fainelli
2024-02-16 15:52 ` Kory Maincent [this message]
2024-02-16 18:09   ` [PATCH RFC net-next v8 04/13] net: Change the API of PHY default timestamp to MAC Rahul Rameshbabu
2024-02-17 17:07     ` Andrew Lunn
2024-02-17 21:05       ` Rahul Rameshbabu
2024-02-17 22:10         ` Andrew Lunn
2024-02-20 20:17           ` Rahul Rameshbabu
2024-02-19 13:29     ` Köry Maincent
2024-02-19 16:11       ` Russell King (Oracle)
2024-02-20 16:20         ` Köry Maincent
2024-02-20 20:39         ` Rahul Rameshbabu
2024-02-16 18:52   ` Florian Fainelli
2024-02-16 15:52 ` [PATCH RFC net-next v8 05/13] net: net_tstamp: Add unspec field to hwtstamp_source enumeration Kory Maincent
2024-02-16 15:52 ` [PATCH RFC net-next v8 06/13] net: Add struct kernel_ethtool_ts_info Kory Maincent
2024-02-16 18:27   ` Rahul Rameshbabu
2024-02-19 10:57     ` Köry Maincent
2024-02-16 15:52 ` [PATCH RFC net-next v8 07/13] ptp: Move from simple ida to xarray Kory Maincent
2024-02-16 15:52 ` [PATCH RFC net-next v8 08/13] ptp: Add phc source and helpers to register specific PTP clock or get information Kory Maincent
2024-02-16 15:52 ` [PATCH RFC net-next v8 09/13] net: Add the possibility to support a selected hwtstamp in netdevice Kory Maincent
2024-02-16 15:52 ` [PATCH RFC net-next v8 10/13] net: netdevsim: ptp_mock: Convert to netdev_ptp_clock_register Kory Maincent
2024-02-16 19:48   ` Rahul Rameshbabu
2024-02-16 15:52 ` [PATCH RFC net-next v8 11/13] net: macb: " Kory Maincent
2024-02-16 15:52 ` [PATCH RFC net-next v8 12/13] net: ethtool: tsinfo: Add support for hwtstamp provider and get/set hwtstamp config Kory Maincent
2024-02-16 15:52 ` [PATCH RFC net-next v8 13/13] netlink: specs: tsinfo: Enhance netlink attributes and add a set command Kory Maincent

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240216-feature_ptp_netnext-v8-4-510f42f444fb@bootlin.com \
    --to=kory.maincent@bootlin.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=andy@greyhouse.net \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=florian.fainelli@broadcom.com \
    --cc=hkallweit1@gmail.com \
    --cc=horatiu.vultur@microchip.com \
    --cc=horms@kernel.org \
    --cc=j.vosburgh@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.ferre@microchip.com \
    --cc=pabeni@redhat.com \
    --cc=radu-nicolae.pirea@oss.nxp.com \
    --cc=richardcochran@gmail.com \
    --cc=rrameshbabu@nvidia.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=vladimir.oltean@nxp.com \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.