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 <[email protected]>
---
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
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
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 |
> > 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
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
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 |
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