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

From: Woojung.Huh
Date: Wed Apr 17 2024 - 14:49:58 EST


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