daniel-p-carvalho commented on code in PR #3791:
URL: https://github.com/apache/nuttx-apps/pull/3791#discussion_r4088297517


##########
netutils/ptpd/ptpd.c:
##########
@@ -116,6 +122,13 @@ struct ptp_state_s
 
   int tx_socket;
 
+  /* Hardware TX timestamp retrieval: consecutive failures, and whether it
+   * was given up on because the driver does not provide the timestamps.
+   */
+
+  unsigned int hwts_tx_failures;

Review Comment:
   Good point, and it made me look at how Linux itself solves this: `ethtool -T 
<iface>` / `SIOCETHTOOL` with `ETHTOOL_GET_TS_INFO` lets a driver statically 
declare `SOF_TIMESTAMPING_TX_HARDWARE` support, so `ptp4l` never has to probe 
by trial and error - it reads the capability once at startup, the same way it 
reads `SOF_TIMESTAMPING_RX_HARDWARE` for RX. NuttX already has the RX half of 
that symmetry (`NETDEV_RX_STAMP` in `d_features`), just not the TX half.
   
   So instead of a command-line option (which still leaves the daemon guessing 
at runtime, just with an extra knob to get wrong), I'd rather add a 
`NETDEV_TX_STAMP` flag mirroring `NETDEV_RX_STAMP`, have drivers that support 
it (starting with the STM32 one this PR exercises) declare it, and have `ptpd` 
read that once instead of counting failures. That also matches how a *genuine* 
runtime failure on an interface that already declared support should be 
handled: not a silent fallback, but a fault - the same way `ptp4l` takes a port 
to `FAULTY` when a TX timestamp does not show up on the error queue in time, 
rather than quietly switching timestamping mode underneath.
   
   That is a `nuttx` core change (`d_features` is shared by every driver), not 
an `nuttx-apps` one, so I would rather not block this PR and #3782 on it - both 
have been open and reviewed for a while now, and this one has been validated on 
real hardware overnight without issues. I will open a follow-up PR against 
`apache/nuttx` for `NETDEV_TX_STAMP`, and a second one here to switch `ptpd` to 
read it once that lands. Does that sequencing work for you?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to