On Mon, Nov 20, 2023 at 03:53:44PM +0100, Köry Maincent wrote: > I did thought about it but I got stuck by the case of hardware timestamping > without PHC. Richard explained the reason of its existence here: > https://lore.kernel.org/netdev/ZS3MKWlnPqTe8gkq@xxxxxxxxxxxxxxxxxx/#t > > Maybe I got a bit stuck in my implementation and should investigate more your > proposition and how to deal with this case. Do you have an idea on how to > solve it? I would take what Richard said with a grain of salt, and interpret as "there exists hardware with hwts but w/o PHC, and that may work for marginal use cases", not that "we should design having that as a first-class citizen in mind". If there is any need for me to point out that such kind of driver isn't a first class citizen, then here's an experiment: diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c index e993ed04ab57..5755f54197b9 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c +++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c @@ -842,13 +842,7 @@ static int enetc_get_ts_info(struct net_device *ndev, { int *phc_idx; - phc_idx = symbol_get(enetc_phc_index); - if (phc_idx) { - info->phc_index = *phc_idx; - symbol_put(enetc_phc_index); - } else { - info->phc_index = -1; - } + info->phc_index = -1; #ifdef CONFIG_FSL_ENETC_PTP_CLOCK info->so_timestamping = SOF_TIMESTAMPING_TX_HARDWARE | When elected as master, it kinda works, and does synchronize with a slave, even if ptp4l gets confused about the phc_index being -1. But when elected as a slave by the BMCA, ptp4l gets confused and thinks that phc_index == -1 means that it's supposed to use software timestamping. $ ptp4l -i eno0 -2 -P -m -s ptp4l[1185.594]: port 1: INITIALIZING to LISTENING on INIT_COMPLETE ptp4l[1185.598]: port 0: INITIALIZING to LISTENING on INIT_COMPLETE ptp4l[1186.887]: port 1: new foreign master 00049f.fffe.05f628-1 ptp4l[1190.889]: selected best master clock 00049f.fffe.05f628 ptp4l[1190.890]: port 1: LISTENING to UNCALIBRATED on RS_SLAVE ptp4l[1191.891]: master offset 37000003850 s0 freq +100000000 path delay 823 ptp4l[1192.896]: master offset 3[ 1235.693485] systemd-journald[125]: Time jumped backwards, rotating. 7000003848 s1 freq +99999998 path delay 822 ptp4l[1193.603]: clockcheck: clock jumped forward or running faster than expected! ptp4l[1193.892]: clockcheck: clock jumped forward or running faster than expected! ptp4l[1193.893]: master offset 37000003852 s0 freq +99999998 path delay 822 ptp4l[1194.342]: clockcheck: clock jumped forward or running faster than expected! ptp4l[1194.604]: clockcheck: clock jumped forward or running faster than expected! So, I guess the only thing we need to do to this kind of setup is not do too much harm to it. We break nothing if we make the phc_index the central aspect of hwts layer selection - except for the fact that such a MAC won't be able to change its timestamping layer to be a PHY. If we wanted to add such a capability to that MAC driver, the obvious way to solve the lack of a PHC is to create a PHC that returns -EOPNOTSUPP for all of its ptp_clock_info operations (gettime, settime etc). It may possibly be that, in the worst case, ptp4l needs to probe for each syscall on the NIC's PHC being operational before deciding what can be done with it. But that's already an improvement over the current handling to make it more graceful, it's not to keep things on par. > > Hmm, can you please explain what is the reason why software timestamping > > can't coexist with PHY timestamping? It is a genuine question to which I > > don't have an answer - I haven't used PHY timestamping. It must be > > something specific to that, since I do know that MAC + software > > timestamping work simultaneously just fine. > > The software timestamp is managed through the MAC driver calling > skb_tx_timestamp() function. The PHY driver does not call it, that's why there > is no software timestamping in PHY driver capabilities. Also the PHY driver > doesn't know if the MAC driver support it so it currently can not coexist with > PHY timestamping. I don't understand. Documentation/networking/timestamping.rst says that skb_tx_timestamp() is one of the actual _mechanisms_ through which phylib timestamping works. It's called by the parent MAC driver, and mii_ts->txtstamp() hooks onto that. So in some situations, the PHY timestamping core _piggybacks_ on top of software timestamping support in the MAC. Where I agree is that the PHY driver has no business in deciding whether the interface should report the socket option flags for software timestamping or not. But I still don't see a proof that they can't coexist. What you need to explain is what makes said software timestamping unusable in the presence of PHY timestamping - to justify this separate software timestamping layer in your UAPI proposal.
