* [PATCH net-next v1 0/4] add support for TimeSync path delays
@ 2024-04-17 16:43 Oleksij Rempel
2024-04-17 16:43 ` [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query support to PHYlib API Oleksij Rempel
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Oleksij Rempel @ 2024-04-17 16:43 UTC (permalink / raw)
To: Alexandre Torgue, Jose Abreu, David S. Miller, Andrew Lunn,
Heiner Kallweit, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Woojung Huh, Arun Ramadoss, Richard Cochran,
Russell King
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
linux-stm32
Add support for TimeSync path delay information to the PHY framework to
allow PHY driver provide path delay information and extend STMMAC to
make use of it.
Oleksij Rempel (4):
net: phy: Add TimeSync delay query support to PHYlib API
net: phy: micrel: lan8841: set default PTP latency values
net: phy: realtek: provide TimeSync data path delays for RTL8211E
net: stmmac: use delays reported by the PHY driver to correct MAC
propagation delay
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +
.../ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 4 ++
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 17 +++++-
drivers/net/phy/micrel.c | 55 +++++++++++++++++-
drivers/net/phy/phy_device.c | 57 +++++++++++++++++++
drivers/net/phy/realtek.c | 42 ++++++++++++++
include/linux/phy.h | 31 ++++++++++
7 files changed, 206 insertions(+), 2 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query support to PHYlib API
2024-04-17 16:43 [PATCH net-next v1 0/4] add support for TimeSync path delays Oleksij Rempel
@ 2024-04-17 16:43 ` Oleksij Rempel
2024-04-17 18:33 ` Andrew Lunn
2024-04-17 18:48 ` Woojung.Huh
2024-04-17 16:43 ` [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values Oleksij Rempel
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Oleksij Rempel @ 2024-04-17 16:43 UTC (permalink / raw)
To: Alexandre Torgue, Jose Abreu, David S. Miller, Andrew Lunn,
Heiner Kallweit, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Woojung Huh, Arun Ramadoss, Richard Cochran,
Russell King
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
linux-stm32
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(+)
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);
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values
2024-04-17 16:43 [PATCH net-next v1 0/4] add support for TimeSync path delays Oleksij Rempel
2024-04-17 16:43 ` [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query support to PHYlib API Oleksij Rempel
@ 2024-04-17 16:43 ` Oleksij Rempel
2024-04-17 18:39 ` Andrew Lunn
2024-04-17 16:43 ` [PATCH net-next v1 3/4] net: phy: realtek: provide TimeSync data path delays for RTL8211E Oleksij Rempel
2024-04-17 16:43 ` [PATCH net-next v1 4/4] net: stmmac: use delays reported by the PHY driver to correct MAC propagation delay Oleksij Rempel
3 siblings, 1 reply; 15+ messages in thread
From: Oleksij Rempel @ 2024-04-17 16:43 UTC (permalink / raw)
To: Alexandre Torgue, Jose Abreu, David S. Miller, Andrew Lunn,
Heiner Kallweit, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Woojung Huh, Arun Ramadoss, Richard Cochran,
Russell King
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
linux-stm32
Set default PTP latency values to provide realistic path delay
measurements and reflecting internal PHY latency asymetry.
This values are based on ptp4l measurements for the path delay against
identical PHY as link partner and latency asymmetry extracted from
documented SOF Latency values of this PHY.
Documented SOF Latency values are:
TX 138ns/RX 430ns @ 1000Mbps
TX 140ns/RX 615ns @ 100Mbps (fixed latency mode)
TX 140ns/RX 488-524ns @ 100Mbps (variable latency mode)
TX 654ns/227-2577ns @ 10Mbps
Calculated asymmetry:
292ns @ 1000Mbps
238ns @ 100Mbps
1923ns @ 10Mbps
Except of ptp4l based tests RGMII-PHY-PHY-RGMII path delay was measured
to verify if values are in sane range. Following LAN8841 + LAN8841 RGMII
delays are measured:
583ns @ 1000Mbps
1080ns @ 100Mbps
15200ns @ 10Mbps
Without configuring compensation registers ptp4l reported following
path delay results:
~467ns @ 1000Mbps
~544ns @ 100Mbps
~9688ns @ 10Mbps
Magnetic + Cable + Magnetic delay in this setup is about 5ns.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/phy/micrel.c | 55 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 54 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index ddb50a0e2bc82..5831706e81623 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -3405,6 +3405,20 @@ static int lan8814_probe(struct phy_device *phydev)
#define LAN8841_BTRX_POWER_DOWN_BTRX_CH_C BIT(5)
#define LAN8841_BTRX_POWER_DOWN_BTRX_CH_D BIT(7)
#define LAN8841_ADC_CHANNEL_MASK 198
+#define LAN8841_PTP_RX_LATENCY_10M 328
+#define LAN8841_PTP_TX_LATENCY_10M 329
+#define LAN8841_PTP_RX_LATENCY_100M 330
+#define LAN8841_PTP_TX_LATENCY_100M 331
+#define LAN8841_PTP_RX_LATENCY_1000M 332
+#define LAN8841_PTP_TX_LATENCY_1000M 333
+
+#define LAN8841_PTP_RX_LATENCY_10M_VAL 5803
+#define LAN8841_PTP_TX_LATENCY_10M_VAL 3880
+#define LAN8841_PTP_RX_LATENCY_100M_VAL 443
+#define LAN8841_PTP_TX_LATENCY_100M_VAL 95
+#define LAN8841_PTP_RX_LATENCY_1000M_VAL 377
+#define LAN8841_PTP_TX_LATENCY_1000M_VAL 85
+
#define LAN8841_PTP_RX_PARSE_L2_ADDR_EN 370
#define LAN8841_PTP_RX_PARSE_IP_ADDR_EN 371
#define LAN8841_PTP_RX_VERSION 374
@@ -3421,6 +3435,45 @@ static int lan8814_probe(struct phy_device *phydev)
#define LAN8841_PTP_INSERT_TS_EN BIT(0)
#define LAN8841_PTP_INSERT_TS_32BIT BIT(1)
+static int lan8841_ptp_latency_init(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
+ LAN8841_PTP_RX_LATENCY_10M,
+ LAN8841_PTP_RX_LATENCY_10M_VAL);
+ if (ret)
+ return ret;
+
+ ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
+ LAN8841_PTP_TX_LATENCY_10M,
+ LAN8841_PTP_TX_LATENCY_10M_VAL);
+ if (ret)
+ return ret;
+
+ ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
+ LAN8841_PTP_RX_LATENCY_100M,
+ LAN8841_PTP_RX_LATENCY_100M_VAL);
+ if (ret)
+ return ret;
+
+ ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
+ LAN8841_PTP_TX_LATENCY_100M,
+ LAN8841_PTP_TX_LATENCY_100M_VAL);
+ if (ret)
+ return ret;
+
+ ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
+ LAN8841_PTP_RX_LATENCY_1000M,
+ LAN8841_PTP_RX_LATENCY_1000M_VAL);
+ if (ret)
+ return ret;
+
+ return phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
+ LAN8841_PTP_TX_LATENCY_1000M,
+ LAN8841_PTP_TX_LATENCY_1000M_VAL);
+}
+
static int lan8841_config_init(struct phy_device *phydev)
{
int ret;
@@ -3500,7 +3553,7 @@ static int lan8841_config_init(struct phy_device *phydev)
LAN8841_MMD0_REGISTER_17_DROP_OPT(2) |
LAN8841_MMD0_REGISTER_17_XMIT_TOG_TX_DIS);
- return 0;
+ return lan8841_ptp_latency_init(phydev);
}
#define LAN8841_OUTPUT_CTRL 25
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next v1 3/4] net: phy: realtek: provide TimeSync data path delays for RTL8211E
2024-04-17 16:43 [PATCH net-next v1 0/4] add support for TimeSync path delays Oleksij Rempel
2024-04-17 16:43 ` [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query support to PHYlib API Oleksij Rempel
2024-04-17 16:43 ` [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values Oleksij Rempel
@ 2024-04-17 16:43 ` Oleksij Rempel
2024-04-18 5:39 ` kernel test robot
2024-04-17 16:43 ` [PATCH net-next v1 4/4] net: stmmac: use delays reported by the PHY driver to correct MAC propagation delay Oleksij Rempel
3 siblings, 1 reply; 15+ messages in thread
From: Oleksij Rempel @ 2024-04-17 16:43 UTC (permalink / raw)
To: Alexandre Torgue, Jose Abreu, David S. Miller, Andrew Lunn,
Heiner Kallweit, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Woojung Huh, Arun Ramadoss, Richard Cochran,
Russell King
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
linux-stm32
Provide default data path delays for RTL8211E.
The measurements was done against with iMX8MP STMMAC and LAN8841 as the
link partner.
This values was calculated based on RGMII-PHY-PHY-RGMII measurements,
where the link partner is LAN8841. Following values was measured:
- data flow from RTL8211E to LAN8841:
746ns @ 1000Mbps
1770ns @ 100Mbps
932000ns @ 10Mbps
- data flow from LAN8841 to RTL8211E:
594ns @ 1000Mbps
1130ns @ 100Mbps
8920ns @ 10Mbps
Before this patch ptp4l reported following path delays:
~610ns @ 1000Mbps
~942ns @ 100Mbps
~465998ns @ 10Mbps
PPS offset compared to grand master was:
~ -114ns @ 1000Mbps
~ -215ns @ 100Mbps
~ -465998ns @ 10Mbps
Magnetic - Cable - Magnetic - delay in this setup was about 5ns.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/phy/realtek.c | 42 +++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 1fa70427b2a26..e39fec8d166b9 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -221,6 +221,47 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
return err;
}
+static int rtl8211e_get_timesync_data_path_delays(struct phy_device *phydev,
+ struct phy_timesync_delay *tsd)
+{
+ phydev_warn(phydev, "Time stamping is not supported\n");
+
+ switch (phydev->interface) {
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ /* The values are measured with RTL8211E and LAN8841 as link
+ * partners and confirmed with i211 to be in sane range.
+ */
+ if (phydev->speed == SPEED_1000) {
+ tsd->tx_min_delay_ns = 326;
+ tsd->rx_min_delay_ns = 406;
+ return 0;
+ } else if (phydev->speed == SPEED_100) {
+ tsd->tx_min_delay_ns = 703;
+ tsd->rx_min_delay_ns = 621;
+ return 0;
+ } else if (phydev->speed == SPEED_10) {
+ /* This value is suspiciously big, with atypical
+ * shift to Egress side. This value is confirmed
+ * by measuring RGMII-PHY-PHY-RGMII path delay.
+ * Similar results are confirmed with LAN8841 and i211
+ * as link partners.
+ */
+ tsd->tx_min_delay_ns = 920231;
+ tsd->rx_min_delay_ns = 1674;
+ return 0;
+ }
+ default:
+ break;
+ }
+
+ phydev_warn(phydev, "Not tested or not supported modes for path delay values\n");
+
+ return -EOPNOTSUPP;
+}
+
static int rtl8211f_config_intr(struct phy_device *phydev)
{
u16 val;
@@ -935,6 +976,7 @@ static struct phy_driver realtek_drvs[] = {
.resume = genphy_resume,
.read_page = rtl821x_read_page,
.write_page = rtl821x_write_page,
+ .get_timesync_data_path_delays = rtl8211e_get_timesync_data_path_delays,
}, {
PHY_ID_MATCH_EXACT(0x001cc916),
.name = "RTL8211F Gigabit Ethernet",
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next v1 4/4] net: stmmac: use delays reported by the PHY driver to correct MAC propagation delay
2024-04-17 16:43 [PATCH net-next v1 0/4] add support for TimeSync path delays Oleksij Rempel
` (2 preceding siblings ...)
2024-04-17 16:43 ` [PATCH net-next v1 3/4] net: phy: realtek: provide TimeSync data path delays for RTL8211E Oleksij Rempel
@ 2024-04-17 16:43 ` Oleksij Rempel
2024-04-17 19:13 ` Serge Semin
3 siblings, 1 reply; 15+ messages in thread
From: Oleksij Rempel @ 2024-04-17 16:43 UTC (permalink / raw)
To: Alexandre Torgue, Jose Abreu, David S. Miller, Andrew Lunn,
Heiner Kallweit, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Woojung Huh, Arun Ramadoss, Richard Cochran,
Russell King
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
linux-stm32
Now after PHY drivers are able to report data path delays, we can use
them in the MAC drivers for the delay correction.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 ++
.../ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 4 ++++
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 17 ++++++++++++++++-
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index dddcaa9220cc3..6db54ce33ea72 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -288,6 +288,8 @@ struct stmmac_priv {
u32 sub_second_inc;
u32 systime_flags;
u32 adv_ts;
+ u64 phy_tx_delay_ns;
+ u64 phy_rx_delay_ns;
int use_riwt;
int irq_wake;
rwlock_t ptp_lock;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
index f05bd757dfe52..bbf284cb7cc2a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
@@ -71,6 +71,8 @@ static void hwtstamp_correct_latency(struct stmmac_priv *priv)
/* MAC-internal ingress latency */
scaled_ns = readl(ioaddr + PTP_TS_INGR_LAT);
+ scaled_ns += priv->phy_rx_delay_ns << 16;
+
/* See section 11.7.2.5.3.1 "Ingress Correction" on page 4001 of
* i.MX8MP Applications Processor Reference Manual Rev. 1, 06/2021
*/
@@ -95,6 +97,8 @@ static void hwtstamp_correct_latency(struct stmmac_priv *priv)
/* MAC-internal egress latency */
scaled_ns = readl(ioaddr + PTP_TS_EGR_LAT);
+ scaled_ns += priv->phy_tx_delay_ns << 16;
+
reg_tsec = scaled_ns >> 16;
reg_tsecsns = scaled_ns & 0xff00;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fe3498e86de9d..30c7c278b7062 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1097,8 +1097,23 @@ static void stmmac_mac_link_up(struct phylink_config *config,
if (priv->dma_cap.fpesel)
stmmac_fpe_link_state_handle(priv, true);
- if (priv->plat->flags & STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY)
+ if (priv->plat->flags & STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY) {
+ int ret = 0;
+
+ if (phy)
+ ret = phy_get_timesync_data_path_delays(phy,
+ &priv->phy_tx_delay_ns,
+ &priv->phy_rx_delay_ns);
+ if (!phy || ret) {
+ if (ret != -EOPNOTSUPP)
+ netdev_err(priv->dev, "Failed to get PHY delay: %pe\n",
+ ERR_PTR(ret));
+ priv->phy_tx_delay_ns = 0;
+ priv->phy_rx_delay_ns = 0;
+ }
+
stmmac_hwtstamp_correct_latency(priv, priv);
+ }
}
static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query support to PHYlib API
2024-04-17 16:43 ` [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query support to PHYlib API Oleksij Rempel
@ 2024-04-17 18:33 ` Andrew Lunn
2024-04-17 18:48 ` Woojung.Huh
1 sibling, 0 replies; 15+ messages in thread
From: Andrew Lunn @ 2024-04-17 18:33 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Alexandre Torgue, Jose Abreu, David S. Miller, Heiner Kallweit,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
Woojung Huh, Arun Ramadoss, Richard Cochran, Russell King,
kernel, linux-kernel, netdev, UNGLinuxDriver, linux-stm32
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values
2024-04-17 16:43 ` [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values Oleksij Rempel
@ 2024-04-17 18:39 ` Andrew Lunn
2024-04-17 20:12 ` Oleksij Rempel
0 siblings, 1 reply; 15+ messages in thread
From: Andrew Lunn @ 2024-04-17 18:39 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Alexandre Torgue, Jose Abreu, David S. Miller, Heiner Kallweit,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
Woojung Huh, Arun Ramadoss, Richard Cochran, Russell King,
kernel, linux-kernel, netdev, UNGLinuxDriver, linux-stm32
On Wed, Apr 17, 2024 at 06:43:14PM +0200, Oleksij Rempel wrote:
> Set default PTP latency values to provide realistic path delay
> measurements and reflecting internal PHY latency asymetry.
>
> This values are based on ptp4l measurements for the path delay against
> identical PHY as link partner and latency asymmetry extracted from
> documented SOF Latency values of this PHY.
>
> Documented SOF Latency values are:
> TX 138ns/RX 430ns @ 1000Mbps
> TX 140ns/RX 615ns @ 100Mbps (fixed latency mode)
> TX 140ns/RX 488-524ns @ 100Mbps (variable latency mode)
> TX 654ns/227-2577ns @ 10Mbps
Does Half Duplex vs Full Duplex make a difference here?
> +static int lan8841_ptp_latency_init(struct phy_device *phydev)
> +{
> + int ret;
> +
> + ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> + LAN8841_PTP_RX_LATENCY_10M,
> + LAN8841_PTP_RX_LATENCY_10M_VAL);
> + if (ret)
> + return ret;
> +
> + ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> + LAN8841_PTP_TX_LATENCY_10M,
> + LAN8841_PTP_TX_LATENCY_10M_VAL);
> + if (ret)
> + return ret;
> +
> + ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> + LAN8841_PTP_RX_LATENCY_100M,
> + LAN8841_PTP_RX_LATENCY_100M_VAL);
> + if (ret)
> + return ret;
> +
> + ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> + LAN8841_PTP_TX_LATENCY_100M,
> + LAN8841_PTP_TX_LATENCY_100M_VAL);
> + if (ret)
> + return ret;
> +
> + ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> + LAN8841_PTP_RX_LATENCY_1000M,
> + LAN8841_PTP_RX_LATENCY_1000M_VAL);
> + if (ret)
> + return ret;
> +
> + return phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> + LAN8841_PTP_TX_LATENCY_1000M,
> + LAN8841_PTP_TX_LATENCY_1000M_VAL);
> +}
What affect does this have on systems which have already applied
adjustments in user space to correct for this? Will this cause
regressions for such systems?
I know Richard has rejected changes like this in the past.
Andrew
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query support to PHYlib API
2024-04-17 16:43 ` [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query support to PHYlib API Oleksij Rempel
2024-04-17 18:33 ` Andrew Lunn
@ 2024-04-17 18:48 ` Woojung.Huh
1 sibling, 0 replies; 15+ messages in thread
From: Woojung.Huh @ 2024-04-17 18:48 UTC (permalink / raw)
To: o.rempel, alexandre.torgue, joabreu, davem, andrew, hkallweit1,
edumazet, kuba, pabeni, mcoquelin.stm32, Arun.Ramadoss,
richardcochran, linux
Cc: kernel, linux-kernel, netdev, UNGLinuxDriver, linux-stm32
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v1 4/4] net: stmmac: use delays reported by the PHY driver to correct MAC propagation delay
2024-04-17 16:43 ` [PATCH net-next v1 4/4] net: stmmac: use delays reported by the PHY driver to correct MAC propagation delay Oleksij Rempel
@ 2024-04-17 19:13 ` Serge Semin
0 siblings, 0 replies; 15+ messages in thread
From: Serge Semin @ 2024-04-17 19:13 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Alexandre Torgue, Jose Abreu, David S. Miller, Andrew Lunn,
Heiner Kallweit, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Woojung Huh, Arun Ramadoss, Richard Cochran,
Russell King, kernel, linux-kernel, netdev, UNGLinuxDriver,
linux-stm32
On Wed, Apr 17, 2024 at 06:43:16PM +0200, Oleksij Rempel wrote:
> Now after PHY drivers are able to report data path delays, we can use
> them in the MAC drivers for the delay correction.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 ++
> .../ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 4 ++++
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 17 ++++++++++++++++-
> 3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index dddcaa9220cc3..6db54ce33ea72 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -288,6 +288,8 @@ struct stmmac_priv {
> u32 sub_second_inc;
> u32 systime_flags;
> u32 adv_ts;
> + u64 phy_tx_delay_ns;
> + u64 phy_rx_delay_ns;
What's the point in adding these fields to the private data if you
retrieve the delays and use them in a single place? Just extend the
stmmac_hwtstamp_correct_latency() arguments list and pass the delays
as the function parameters.
-Serge(y)
> int use_riwt;
> int irq_wake;
> rwlock_t ptp_lock;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
> index f05bd757dfe52..bbf284cb7cc2a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
> @@ -71,6 +71,8 @@ static void hwtstamp_correct_latency(struct stmmac_priv *priv)
> /* MAC-internal ingress latency */
> scaled_ns = readl(ioaddr + PTP_TS_INGR_LAT);
>
> + scaled_ns += priv->phy_rx_delay_ns << 16;
> +
> /* See section 11.7.2.5.3.1 "Ingress Correction" on page 4001 of
> * i.MX8MP Applications Processor Reference Manual Rev. 1, 06/2021
> */
> @@ -95,6 +97,8 @@ static void hwtstamp_correct_latency(struct stmmac_priv *priv)
> /* MAC-internal egress latency */
> scaled_ns = readl(ioaddr + PTP_TS_EGR_LAT);
>
> + scaled_ns += priv->phy_tx_delay_ns << 16;
> +
> reg_tsec = scaled_ns >> 16;
> reg_tsecsns = scaled_ns & 0xff00;
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index fe3498e86de9d..30c7c278b7062 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1097,8 +1097,23 @@ static void stmmac_mac_link_up(struct phylink_config *config,
> if (priv->dma_cap.fpesel)
> stmmac_fpe_link_state_handle(priv, true);
>
> - if (priv->plat->flags & STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY)
> + if (priv->plat->flags & STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY) {
> + int ret = 0;
> +
> + if (phy)
> + ret = phy_get_timesync_data_path_delays(phy,
> + &priv->phy_tx_delay_ns,
> + &priv->phy_rx_delay_ns);
> + if (!phy || ret) {
> + if (ret != -EOPNOTSUPP)
> + netdev_err(priv->dev, "Failed to get PHY delay: %pe\n",
> + ERR_PTR(ret));
> + priv->phy_tx_delay_ns = 0;
> + priv->phy_rx_delay_ns = 0;
> + }
> +
> stmmac_hwtstamp_correct_latency(priv, priv);
> + }
> }
>
> static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values
2024-04-17 18:39 ` Andrew Lunn
@ 2024-04-17 20:12 ` Oleksij Rempel
2024-04-17 20:23 ` Andrew Lunn
0 siblings, 1 reply; 15+ messages in thread
From: Oleksij Rempel @ 2024-04-17 20:12 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alexandre Torgue, Jose Abreu, David S. Miller, Heiner Kallweit,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
Woojung Huh, Arun Ramadoss, Richard Cochran, Russell King,
kernel, linux-kernel, netdev, UNGLinuxDriver, linux-stm32
On Wed, Apr 17, 2024 at 08:39:54PM +0200, Andrew Lunn wrote:
> On Wed, Apr 17, 2024 at 06:43:14PM +0200, Oleksij Rempel wrote:
> > Set default PTP latency values to provide realistic path delay
> > measurements and reflecting internal PHY latency asymetry.
> >
> > This values are based on ptp4l measurements for the path delay against
> > identical PHY as link partner and latency asymmetry extracted from
> > documented SOF Latency values of this PHY.
> >
> > Documented SOF Latency values are:
> > TX 138ns/RX 430ns @ 1000Mbps
> > TX 140ns/RX 615ns @ 100Mbps (fixed latency mode)
> > TX 140ns/RX 488-524ns @ 100Mbps (variable latency mode)
> > TX 654ns/227-2577ns @ 10Mbps
Here is a typo 2277-2577ns
> Does Half Duplex vs Full Duplex make a difference here?
Yes, Half Duplex will be even less predictable. It would make no sense to
use it for the time sync.
> > +static int lan8841_ptp_latency_init(struct phy_device *phydev)
> > +{
> > + int ret;
> > +
> > + ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> > + LAN8841_PTP_RX_LATENCY_10M,
> > + LAN8841_PTP_RX_LATENCY_10M_VAL);
> > + if (ret)
> > + return ret;
> > +
> > + ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> > + LAN8841_PTP_TX_LATENCY_10M,
> > + LAN8841_PTP_TX_LATENCY_10M_VAL);
> > + if (ret)
> > + return ret;
> > +
> > + ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> > + LAN8841_PTP_RX_LATENCY_100M,
> > + LAN8841_PTP_RX_LATENCY_100M_VAL);
> > + if (ret)
> > + return ret;
> > +
> > + ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> > + LAN8841_PTP_TX_LATENCY_100M,
> > + LAN8841_PTP_TX_LATENCY_100M_VAL);
> > + if (ret)
> > + return ret;
> > +
> > + ret = phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> > + LAN8841_PTP_RX_LATENCY_1000M,
> > + LAN8841_PTP_RX_LATENCY_1000M_VAL);
> > + if (ret)
> > + return ret;
> > +
> > + return phy_write_mmd(phydev, KSZ9131RN_MMD_COMMON_CTRL_REG,
> > + LAN8841_PTP_TX_LATENCY_1000M,
> > + LAN8841_PTP_TX_LATENCY_1000M_VAL);
> > +}
>
> What affect does this have on systems which have already applied
> adjustments in user space to correct for this? Will this cause
> regressions for such systems?
Yes.
> I know Richard has rejected changes like this in the past.
In this case I would need to extend the ethtool interface. The driver
should provide recommended values and the user space can optionally
read them and optionally write them to the HW.
Get Recommended TimeSync Data Path Delays
Command Name: ETHTOOL_G_TS_DELAYS_RECOMMENDED
Description: This command retrieves the recommended TimeSync data path
delays for transmit and receive paths, typically based on the interface
type and link speed. This values are not stable.
Get Currently Active TimeSync Data Path Delays
Command Name: ETHTOOL_G_TS_DELAYS_ACTIVE
Description: This command retrieves the currently active TimeSync data path
delays that are being applied by the PHY. This is useful for real-time
diagnostics and monitoring.
Set Currently Active TimeSync Data Path Delays
Command Name: ETHTOOL_S_TS_DELAYS_ACTIVE
Description: This command sets the currently active TimeSync data path
delays. This would allow the system administrator or the network management
system to manually adjust the TimeSync delays to either align them with the
recommended values or to tweak them for specific network conditions or
compliance requirements.
What do you think about this?
Should the delay value be bound to the link mode or only to the speed?
What if we have multiple components with delays? SoC/MAC specific delays,
interface converters RGMII to xMII, etc? Should the ethtool interface provide
summ of all delays in the chain, or we need to have access to each separately?
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values
2024-04-17 20:12 ` Oleksij Rempel
@ 2024-04-17 20:23 ` Andrew Lunn
2024-04-19 6:46 ` Richard Cochran
0 siblings, 1 reply; 15+ messages in thread
From: Andrew Lunn @ 2024-04-17 20:23 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Alexandre Torgue, Jose Abreu, David S. Miller, Heiner Kallweit,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
Woojung Huh, Arun Ramadoss, Richard Cochran, Russell King,
kernel, linux-kernel, netdev, UNGLinuxDriver, linux-stm32
> > What affect does this have on systems which have already applied
> > adjustments in user space to correct for this? Will this cause
> > regressions for such systems?
>
> Yes.
>
> > I know Richard has rejected changes like this in the past.
>
> In this case I would need to extend the ethtool interface. The driver
> should provide recommended values and the user space can optionally
> read them and optionally write them to the HW.
I suggest you go read older messages from Richard. It was a discussion
with Microchip about one of their PHYs.
Andrew
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v1 3/4] net: phy: realtek: provide TimeSync data path delays for RTL8211E
2024-04-17 16:43 ` [PATCH net-next v1 3/4] net: phy: realtek: provide TimeSync data path delays for RTL8211E Oleksij Rempel
@ 2024-04-18 5:39 ` kernel test robot
0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2024-04-18 5:39 UTC (permalink / raw)
To: Oleksij Rempel, Alexandre Torgue, Jose Abreu, David S. Miller,
Andrew Lunn, Heiner Kallweit, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Woojung Huh, Arun Ramadoss,
Richard Cochran, Russell King
Cc: llvm, oe-kbuild-all, netdev, Oleksij Rempel, kernel,
linux-kernel, UNGLinuxDriver, linux-stm32
Hi Oleksij,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Oleksij-Rempel/net-phy-Add-TimeSync-delay-query-support-to-PHYlib-API/20240418-004607
base: net-next/main
patch link: https://lore.kernel.org/r/20240417164316.1755299-4-o.rempel%40pengutronix.de
patch subject: [PATCH net-next v1 3/4] net: phy: realtek: provide TimeSync data path delays for RTL8211E
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240418/202404181340.89g7TIG1-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240418/202404181340.89g7TIG1-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404181340.89g7TIG1-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/net/phy/realtek.c:278:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
default:
^
drivers/net/phy/realtek.c:278:2: note: insert 'break;' to avoid fall-through
default:
^
break;
1 warning generated.
vim +278 drivers/net/phy/realtek.c
245
246 static int rtl8211e_get_timesync_data_path_delays(struct phy_device *phydev,
247 struct phy_timesync_delay *tsd)
248 {
249 phydev_warn(phydev, "Time stamping is not supported\n");
250
251 switch (phydev->interface) {
252 case PHY_INTERFACE_MODE_RGMII:
253 case PHY_INTERFACE_MODE_RGMII_RXID:
254 case PHY_INTERFACE_MODE_RGMII_TXID:
255 case PHY_INTERFACE_MODE_RGMII_ID:
256 /* The values are measured with RTL8211E and LAN8841 as link
257 * partners and confirmed with i211 to be in sane range.
258 */
259 if (phydev->speed == SPEED_1000) {
260 tsd->tx_min_delay_ns = 326;
261 tsd->rx_min_delay_ns = 406;
262 return 0;
263 } else if (phydev->speed == SPEED_100) {
264 tsd->tx_min_delay_ns = 703;
265 tsd->rx_min_delay_ns = 621;
266 return 0;
267 } else if (phydev->speed == SPEED_10) {
268 /* This value is suspiciously big, with atypical
269 * shift to Egress side. This value is confirmed
270 * by measuring RGMII-PHY-PHY-RGMII path delay.
271 * Similar results are confirmed with LAN8841 and i211
272 * as link partners.
273 */
274 tsd->tx_min_delay_ns = 920231;
275 tsd->rx_min_delay_ns = 1674;
276 return 0;
277 }
> 278 default:
279 break;
280 }
281
282 phydev_warn(phydev, "Not tested or not supported modes for path delay values\n");
283
284 return -EOPNOTSUPP;
285 }
286
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values
2024-04-17 20:23 ` Andrew Lunn
@ 2024-04-19 6:46 ` Richard Cochran
2024-04-23 7:07 ` Oleksij Rempel
0 siblings, 1 reply; 15+ messages in thread
From: Richard Cochran @ 2024-04-19 6:46 UTC (permalink / raw)
To: Andrew Lunn
Cc: Oleksij Rempel, Alexandre Torgue, Jose Abreu, David S. Miller,
Heiner Kallweit, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Woojung Huh, Arun Ramadoss, Russell King,
kernel, linux-kernel, netdev, UNGLinuxDriver, linux-stm32
On Wed, Apr 17, 2024 at 10:23:07PM +0200, Andrew Lunn wrote:
> I suggest you go read older messages from Richard. It was a discussion
> with Microchip about one of their PHYs.
My 2 cents:
User space has all of the hooks needed to configure corrections for a
given setup.
Hard coding corrections in device drivers is bound to fail, based on
prior experience with Vendors not knowing or caring how their products
actually work. Vendors will publish value X one year, then delete the
info (to avoid embarrassment), then publish the new value Y, once
customers have forgotten about X.
So, prudent users will always calibrate their beloved systems, not
trusting the Vendors to provide anything close to reasonable.
Ergo, adding new magical correction in a kernel release causes
regressions for prudent users.
But, in the end, that doesn't matter, because prudent users are used
to being abused by well-meaning yet misguided device driver authors.
Prudent users are wise, and they will re-calibrate their systems
before rolling out an updated kernel.
After all, device driver authors leave them no other choice.
Thanks,
Richard
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values
2024-04-19 6:46 ` Richard Cochran
@ 2024-04-23 7:07 ` Oleksij Rempel
2024-04-24 6:10 ` Richard Cochran
0 siblings, 1 reply; 15+ messages in thread
From: Oleksij Rempel @ 2024-04-23 7:07 UTC (permalink / raw)
To: Richard Cochran
Cc: Andrew Lunn, Alexandre Torgue, Jose Abreu, David S. Miller,
Heiner Kallweit, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Woojung Huh, Arun Ramadoss, Russell King,
kernel, linux-kernel, netdev, UNGLinuxDriver, linux-stm32
Hi Richard,
On Thu, Apr 18, 2024 at 11:46:48PM -0700, Richard Cochran wrote:
> On Wed, Apr 17, 2024 at 10:23:07PM +0200, Andrew Lunn wrote:
> > I suggest you go read older messages from Richard. It was a discussion
> > with Microchip about one of their PHYs.
>
> My 2 cents:
>
> User space has all of the hooks needed to configure corrections for a
> given setup.
>
> Hard coding corrections in device drivers is bound to fail, based on
> prior experience with Vendors not knowing or caring how their products
> actually work. Vendors will publish value X one year, then delete the
> info (to avoid embarrassment), then publish the new value Y, once
> customers have forgotten about X.
>
> So, prudent users will always calibrate their beloved systems, not
> trusting the Vendors to provide anything close to reasonable.
>
> Ergo, adding new magical correction in a kernel release causes
> regressions for prudent users.
>
> But, in the end, that doesn't matter, because prudent users are used
> to being abused by well-meaning yet misguided device driver authors.
>
> Prudent users are wise, and they will re-calibrate their systems
> before rolling out an updated kernel.
Ok, i see. Thank you for your feedback.
Are the recommended FOSS projects managing calibration values per-
linkmode/port/device in user space?
What is recommended way for calibration? Using some recommended device?
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values
2024-04-23 7:07 ` Oleksij Rempel
@ 2024-04-24 6:10 ` Richard Cochran
0 siblings, 0 replies; 15+ messages in thread
From: Richard Cochran @ 2024-04-24 6:10 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andrew Lunn, Alexandre Torgue, Jose Abreu, David S. Miller,
Heiner Kallweit, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maxime Coquelin, Woojung Huh, Arun Ramadoss, Russell King,
kernel, linux-kernel, netdev, UNGLinuxDriver, linux-stm32
On Tue, Apr 23, 2024 at 09:07:59AM +0200, Oleksij Rempel wrote:
> Are the recommended FOSS projects managing calibration values per-
> linkmode/port/device in user space?
No, I haven't seen this. I think the vendors should provide the
numbers, like in the data sheet. This is more useful and flexible
than letting vendors hard code the numbers into the source code of
device drivers.
> What is recommended way for calibration? Using some recommended device?
You can try the "Calibration procedures" in IEEE 1588-2019, Annex N.
Or if you have end to end PPS outputs (from the GM server and the
client) then you can simply compare them with an oscilloscope and
correct any static offset with the ptp4l "delayAsymmetry"
configuration option.
Or if you have auxiliary event inputs on both server and client, feed
a pulse generator into both, compare time stamps, etc.
Also linuxptp supports Meinberg's NetSync Monitor method.
Also there are commercial vendors that rent/sell test equipment for
PTP networks.
So there are many possibilities.
HTH,
Richard
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-04-24 6:10 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-17 16:43 [PATCH net-next v1 0/4] add support for TimeSync path delays Oleksij Rempel
2024-04-17 16:43 ` [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query support to PHYlib API Oleksij Rempel
2024-04-17 18:33 ` Andrew Lunn
2024-04-17 18:48 ` Woojung.Huh
2024-04-17 16:43 ` [PATCH net-next v1 2/4] net: phy: micrel: lan8841: set default PTP latency values Oleksij Rempel
2024-04-17 18:39 ` Andrew Lunn
2024-04-17 20:12 ` Oleksij Rempel
2024-04-17 20:23 ` Andrew Lunn
2024-04-19 6:46 ` Richard Cochran
2024-04-23 7:07 ` Oleksij Rempel
2024-04-24 6:10 ` Richard Cochran
2024-04-17 16:43 ` [PATCH net-next v1 3/4] net: phy: realtek: provide TimeSync data path delays for RTL8211E Oleksij Rempel
2024-04-18 5:39 ` kernel test robot
2024-04-17 16:43 ` [PATCH net-next v1 4/4] net: stmmac: use delays reported by the PHY driver to correct MAC propagation delay Oleksij Rempel
2024-04-17 19:13 ` Serge Semin
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.