On 10/9/23 08:51, Köry Maincent wrote:
From: Kory Maincent <kory.maincent@xxxxxxxxxxx> Change the API to select MAC default time stamping instead of the PHY. Indeed the PHY is closer to the wire therefore theoretically it have less delay than the MAC timestamping but the reality is different.
s/have/has/ or you need to make the subject plural.
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 precisethan the MAC.
-> different PHYs
The exception is for PHY designed specially for PTP case butthese board are not very widespread.
s/board/devices/ maybe?
For not breaking the compatibility I introduce an allowlist to reference all current PHYs that support time stamping and let them keep the old API behavior. The phy_set_timestamp function is called at each call of phy_attach_direct. In case of MAC driver using phylink this function is called when the interface is turned up. Then if the interface goes down and up again the last choice of timestamp will be overwritten by the default choice. A solution could be to cache the timestamp status but it can bring other issues. In case of SFP, if we change the module, it doesn't make sense to blindly re-set the timestamp back to PHY, if the new module has a PHY with mediocre timestamping capabilities. Signed-off-by: Kory Maincent <kory.maincent@xxxxxxxxxxx> --- 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. --- drivers/net/phy/phy_device.c | 68 ++++++++++++++++++++++++++++++++++++ include/linux/netdevice.h | 5 +++ net/core/dev.c | 3 ++ net/core/dev_ioctl.c | 36 +++++++++++-------- net/core/timestamping.c | 9 +++++ net/ethtool/common.c | 16 +++++++-- 6 files changed, 121 insertions(+), 16 deletions(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 2ce74593d6e4..2d5a6d57acb3 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1411,6 +1411,68 @@ int phy_sfp_probe(struct phy_device *phydev, } EXPORT_SYMBOL(phy_sfp_probe);+/* An allowlist for PHYs selected as default timesetamping.+ * Its use is to keep compatibility with old PTP API which is selecting + * these PHYs as default timestamping. + * The new API is selecting the MAC as default timestamping. + */ +const char * const phy_timestamping_allowlist[] = { + "Broadcom BCM5411", + "Broadcom BCM5421", + "Broadcom BCM54210E", + "Broadcom BCM5461", + "Broadcom BCM54612E", + "Broadcom BCM5464", + "Broadcom BCM5481", + "Broadcom BCM54810", + "Broadcom BCM54811", + "Broadcom BCM5482", + "Broadcom BCM50610", + "Broadcom BCM50610M", + "Broadcom BCM57780", + "Broadcom BCM5395", + "Broadcom BCM53125", + "Broadcom BCM53128", + "Broadcom BCM89610",
The list of Broadcom PHYs that you have is too broad, if you look at drivers/net/phy/bcm-phy-ptp.c only PHY_ID_BCM54210E is actually supported as of now.
The allowlist is not maintainable using the PHY device name, especially not without having a shared header file that is used by both phy_device.c and the individual PHY device driver. You are guaranteed that a name change on one side can be missed on the other.
Using PHY OUIs can be a tad complicated with C45 PHYs, so rather, I would suggest that each of those PHY device drivers be responsible for overidding the timestamping selection, you could even consider inventing a specific PHYLIB_TIMESTAMPING_LEGACY and then issue a warning within the PHY library to encourage a change (if this is even relevant).
#endif + + u32 ts_layer;
Likewise, would be preferable to use an enum type. -- Florian
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
