diff mbox series

[net-next,v1,1/4] net: phy: Add TimeSync delay query support to PHYlib API

Message ID 20240417164316.1755299-2-o.rempel@pengutronix.de (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series add support for TimeSync path delays | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1356 this patch: 1356
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 951 this patch: 951
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1374 this patch: 1374
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 112 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc fail Errors and warnings before: 71 this patch: 72
netdev/source_inline success Was 0 now: 0

Commit Message

Oleksij Rempel April 17, 2024, 4:43 p.m. UTC
Add a new phy_get_timesync_data_path_delays() function, to the PHY
device API. This function enables querying the ingress and egress
TimeSync delays for PHY devices, as specified in IEEE 802.3-2022
sections 30.13.1.3 to 30.13.1.6. The function adds the capability to
obtain the average delays in nanoseconds, which can be used to
compensate for time variations added by the PHY to PTP packets.

Since most PHYs do not provide register-based delay information, PHY
drivers should supply this data, typically dependent on the interface
type (MII, RGMII, etc.) and link speed. The MAC driver, or consumer of
this API, is expected to invoke this function upon link establishment to
accurately compensate for any PHY-induced time discrepancies.

Compensating for this delay is crucial. If it is not addressed, the PHY
delays may exceed the 800ns threshold set by the 802.1as standard (the
gPTP standard). This situation classifies the link as a gPTP domain
boundary and excludes the device from synchronization processes. While
some switches and devices allow configurations that exceed this
threshold, such adjustments are non-compliant with the standard and may
not operate seamlessly across different devices or configurations.
Additionally, addressing the path delay asymmetry is vital. Identical
PHYs may not exhibit noticeable asymmetry impacting PTP time offset;
however, different PHY types and vendors can introduce significant
asymmetries that require manual adjustment for each device.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phy_device.c | 57 ++++++++++++++++++++++++++++++++++++
 include/linux/phy.h          | 31 ++++++++++++++++++++
 2 files changed, 88 insertions(+)

Comments

Andrew Lunn April 17, 2024, 6:33 p.m. UTC | #1
On Wed, Apr 17, 2024 at 06:43:13PM +0200, Oleksij Rempel wrote:
> Add a new phy_get_timesync_data_path_delays() function, to the PHY
> device API. This function enables querying the ingress and egress
> TimeSync delays for PHY devices, as specified in IEEE 802.3-2022
> sections 30.13.1.3 to 30.13.1.6. The function adds the capability to
> obtain the average delays in nanoseconds, which can be used to
> compensate for time variations added by the PHY to PTP packets.
> 
> Since most PHYs do not provide register-based delay information, PHY
> drivers should supply this data, typically dependent on the interface
> type (MII, RGMII, etc.) and link speed. The MAC driver, or consumer of
> this API, is expected to invoke this function upon link establishment to
> accurately compensate for any PHY-induced time discrepancies.

So your intention is that this function is called from within the
adjust_link callback? Hence there is no locking being performed
because the lock is already held?

> +/**
> + * phy_get_timesync_data_path_delays - get the TimeSync data path ingress/egress
> + *                                     delays
> + * @phydev: phy_device struct
> + * @tx_delay_ns: pointer to the transmit delay in nanoseconds
> + * @rx_delay_ns: pointer to the receive delay in nanoseconds
> + *
> + * This function is used to get the TimeSync data path ingress/egress delays
> + * as described in IEEE 802.3-2022 sections:
> + * 30.13.1.3 aTimeSyncDelayTXmax, 30.13.1.4 aTimeSyncDelayTXmin,
> + * 30.13.1.5 aTimeSyncDelayRXmax and 30.13.1.6 aTimeSyncDelayRXmin.
> + *
> + * The delays are returned in nanoseconds and can be used to compensate time
> + * added by the PHY to the PTP packets.

Please document the context this function should be used in. If users
use it outside of the adjust_link callback, the locking will be
wrong....

> + *
> + * Returns 0 on success, negative value on failure.

I think kdocs now requires a : after Returns ?

> +	/**
> +	 * @get_timesync_data_path_delays: Get the PHY time sync delay values
> +	 * @dev: PHY device
> +	 * @tsd: PHY time sync delay values
> +	 *
> +	 * Returns 0 on success, or an error code.

Same here.

     Andrew
Woojung Huh April 17, 2024, 6:48 p.m. UTC | #2
Hi

> +/**
> + * phy_get_timesync_data_path_delays - get the TimeSync data path
> ingress/egress
> + *                                     delays
> + * @phydev: phy_device struct
> + * @tx_delay_ns: pointer to the transmit delay in nanoseconds
> + * @rx_delay_ns: pointer to the receive delay in nanoseconds
> + *
> + * This function is used to get the TimeSync data path ingress/egress
> delays
> + * as described in IEEE 802.3-2022 sections:
> + * 30.13.1.3 aTimeSyncDelayTXmax, 30.13.1.4 aTimeSyncDelayTXmin,
> + * 30.13.1.5 aTimeSyncDelayRXmax and 30.13.1.6 aTimeSyncDelayRXmin.
> + *
> + * The delays are returned in nanoseconds and can be used to compensate
> time
> + * added by the PHY to the PTP packets.
> + *
> + * Returns 0 on success, negative value on failure.
> + */
> +int phy_get_timesync_data_path_delays(struct phy_device *phydev,
> +                                     u64 *tx_delay_ns, u64 *rx_delay_ns)
> +{
> +       struct phy_timesync_delay tsd = { 0 };
> +       int err;
> +
> +       if (!phydev->drv->get_timesync_data_path_delays)
> +               return -EOPNOTSUPP;
> +
> +       if (!tx_delay_ns || !rx_delay_ns)
> +               return -EINVAL;
> +
> +       err = phydev->drv->get_timesync_data_path_delays(phydev, &tsd);
> +       if (err)
> +               return err;
> +
> +       if ((!tsd.tx_max_delay_ns && !tsd.tx_min_delay_ns) ||
> +           (!tsd.rx_max_delay_ns && !tsd.rx_min_delay_ns)) {
> +               phydev_err(phydev, "Invalid TimeSync data path delays\n");
> +               return -EINVAL;
> +       }

Because all 4 variables are u64, it can be zero technically.
I think the condition of "max >= min" could be better option for validation
because actual *_delay_ns is average of min and max value.

> +
> +       if (tsd.tx_max_delay_ns && tsd.tx_min_delay_ns)
> +               *tx_delay_ns = (tsd.tx_max_delay_ns + tsd.tx_min_delay_ns) /
> 2;
> +       else if (tsd.tx_max_delay_ns)
> +               *tx_delay_ns = tsd.tx_max_delay_ns;
> +       else
> +               *tx_delay_ns = tsd.tx_min_delay_ns;
> +
> +       if (tsd.rx_max_delay_ns && tsd.rx_min_delay_ns)
> +               *rx_delay_ns = (tsd.rx_max_delay_ns + tsd.rx_min_delay_ns) /
> 2;
> +       else if (tsd.rx_max_delay_ns)
> +               *rx_delay_ns = tsd.rx_max_delay_ns;
> +       else
> +               *rx_delay_ns = tsd.rx_min_delay_ns;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(phy_get_timesync_data_path_delays);
> +

Thanks.
Woojung
diff mbox series

Patch

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 616bd7ba46cbf..3ded9280ab831 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3216,6 +3216,63 @@  s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev,
 }
 EXPORT_SYMBOL(phy_get_internal_delay);
 
+/**
+ * phy_get_timesync_data_path_delays - get the TimeSync data path ingress/egress
+ *                                     delays
+ * @phydev: phy_device struct
+ * @tx_delay_ns: pointer to the transmit delay in nanoseconds
+ * @rx_delay_ns: pointer to the receive delay in nanoseconds
+ *
+ * This function is used to get the TimeSync data path ingress/egress delays
+ * as described in IEEE 802.3-2022 sections:
+ * 30.13.1.3 aTimeSyncDelayTXmax, 30.13.1.4 aTimeSyncDelayTXmin,
+ * 30.13.1.5 aTimeSyncDelayRXmax and 30.13.1.6 aTimeSyncDelayRXmin.
+ *
+ * The delays are returned in nanoseconds and can be used to compensate time
+ * added by the PHY to the PTP packets.
+ *
+ * Returns 0 on success, negative value on failure.
+ */
+int phy_get_timesync_data_path_delays(struct phy_device *phydev,
+				      u64 *tx_delay_ns, u64 *rx_delay_ns)
+{
+	struct phy_timesync_delay tsd = { 0 };
+	int err;
+
+	if (!phydev->drv->get_timesync_data_path_delays)
+		return -EOPNOTSUPP;
+
+	if (!tx_delay_ns || !rx_delay_ns)
+		return -EINVAL;
+
+	err = phydev->drv->get_timesync_data_path_delays(phydev, &tsd);
+	if (err)
+		return err;
+
+	if ((!tsd.tx_max_delay_ns && !tsd.tx_min_delay_ns) ||
+	    (!tsd.rx_max_delay_ns && !tsd.rx_min_delay_ns)) {
+		phydev_err(phydev, "Invalid TimeSync data path delays\n");
+		return -EINVAL;
+	}
+
+	if (tsd.tx_max_delay_ns && tsd.tx_min_delay_ns)
+		*tx_delay_ns = (tsd.tx_max_delay_ns + tsd.tx_min_delay_ns) / 2;
+	else if (tsd.tx_max_delay_ns)
+		*tx_delay_ns = tsd.tx_max_delay_ns;
+	else
+		*tx_delay_ns = tsd.tx_min_delay_ns;
+
+	if (tsd.rx_max_delay_ns && tsd.rx_min_delay_ns)
+		*rx_delay_ns = (tsd.rx_max_delay_ns + tsd.rx_min_delay_ns) / 2;
+	else if (tsd.rx_max_delay_ns)
+		*rx_delay_ns = tsd.rx_max_delay_ns;
+	else
+		*rx_delay_ns = tsd.rx_min_delay_ns;
+
+	return 0;
+}
+EXPORT_SYMBOL(phy_get_timesync_data_path_delays);
+
 static int phy_led_set_brightness(struct led_classdev *led_cdev,
 				  enum led_brightness value)
 {
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3ddfe7fe781aa..6021e3c6cebb2 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -893,6 +893,24 @@  struct phy_led {
 
 #define to_phy_led(d) container_of(d, struct phy_led, led_cdev)
 
+/**
+ * struct phy_timesync_delay - PHY time sync delay values
+ * @tx_max_delay_ns: Maximum delay for transmit path in nanoseconds. Corresponds
+ * to IEEE 802.3 2022 - 30.13.1.3 aTimeSyncDelayTXmax.
+ * @tx_min_delay_ns: Minimum delay for transmit path in nanoseconds. Corresponds
+ * to IEEE 802.3 2022 - 30.13.1.4 aTimeSyncDelayTXmin.
+ * @rx_max_delay_ns: Maximum delay for receive path in nanoseconds. Corresponds
+ * to IEEE 802.3 2022 - 30.13.1.5 aTimeSyncDelayRXmax
+ * @rx_min_delay_ns: Minimum delay for receive path in nanoseconds. Corresponds
+ * to IEEE 802.3 2022 - 30.13.1.6 aTimeSyncDelayRXmin.
+ */
+struct phy_timesync_delay {
+	u64 tx_max_delay_ns;
+	u64 tx_min_delay_ns;
+	u64 rx_max_delay_ns;
+	u64 rx_min_delay_ns;
+};
+
 /**
  * struct phy_driver - Driver structure for a particular PHY type
  *
@@ -1182,6 +1200,16 @@  struct phy_driver {
 	 */
 	int (*led_polarity_set)(struct phy_device *dev, int index,
 				unsigned long modes);
+
+	/**
+	 * @get_timesync_data_path_delays: Get the PHY time sync delay values
+	 * @dev: PHY device
+	 * @tsd: PHY time sync delay values
+	 *
+	 * Returns 0 on success, or an error code.
+	 */
+	int (*get_timesync_data_path_delays)(struct phy_device *dev,
+					     struct phy_timesync_delay *tsd);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),		\
 				      struct phy_driver, mdiodrv)
@@ -1991,6 +2019,9 @@  void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause);
 s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev,
 			   const int *delay_values, int size, bool is_rx);
 
+int phy_get_timesync_data_path_delays(struct phy_device *phydev,
+				      u64 *tx_delay_ns, u64 *rx_delay_ns);
+
 void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv,
 		       bool *tx_pause, bool *rx_pause);