On 7/26/2024 12:04 PM, Kory Maincent wrote: > Hello Jacob, > > Thanks a lot for your full review! > > On Wed, 17 Jul 2024 10:35:20 -0700 > Jacob Keller <jacob.e.keller@xxxxxxxxx> wrote: > >> On 7/9/2024 6:53 AM, Kory Maincent wrote: >> [...] >> >> Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx> >> >> One thing which applies more broadly to the whole series, but I see the >> focus right now is on selecting between NETDEV and PHYLIB. >> >> For ice (E800 series) hardware, the timestamps are captured by the PHY, >> but its not managed by phylib, its managed by firmware. In our case we >> would obviously report NETDEV in this case. The hardware only has one >> timestamp point and the fact that it happens at the PHY layer is not >> relevant since you can't select or change it. >> >> There are some future plans in the work for hardware based on the ixgbe >> driver which could timestamp at either the MAC or PHY (with varying >> trade-offs in precision vs what can be timestamped), and (perhaps >> unfortunately), the PHY would likely not manageable by phylib. >> >> There is also the possibility of something like DMA or completion >> timestamps which are distinct from MAC timestamps. But again can have >> varying trade offs. > > As we already discussed in older version of this patch series the > hwtstamp qualifier will be used to select between IEEE 1588 timestamp or DMA > timestamp. See patch 8 : > +/* > + * Possible type of htstamp provider. Mainly "precise" the default one > + * is for IEEE 1588 quality and "approx" is for NICs DMA point. > + */ > > We could add other enumeration values in the future if needed, to manage new > cases. > > Just figured out there is a NIT in the doc. h*w*tstamp. > Ah, perfect, thanks for the clarification! >> I'm hopeful this work can be extended somehow to enable selection >> between the different mechanisms, even when the kernel device being >> represented is the same netdev. > > Another nice features would be the support for simultaneous hardware timestamp > but I sadly won't be able to work on this. > > Regards, Yes this would be useful, though I think we're somewhat limited by the API that returns to userspace currently.
