Re: [PATCH net-next 2/3] net: stmmac: Allow supporting coarse adjustment mode

[Kory Maincent <kory.maincent@bootlin.com>]

On Sat, 18 Oct 2025 09:42:57 +0200
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

> Hi Jakub,
> 
> On 18/10/2025 03:23, Jakub Kicinski wrote:
> > On Wed, 15 Oct 2025 12:27:22 +0200 Maxime Chevallier wrote:  
> >> The DWMAC1000 supports 2 timestamping configurations to configure how
> >> frequency adjustments are made to the ptp_clock, as well as the reported
> >> timestamp values.
> >>
> >> There was a previous attempt at upstreaming support for configuring this
> >> mode by Olivier Dautricourt and Julien Beraud a few years back [1]
> >>
> >> In a nutshell, the timestamping can be either set in fine mode or in
> >> coarse mode.
> >>
> >> In fine mode, which is the default, we use the overflow of an accumulator
> >> to trigger frequency adjustments, but by doing so we lose precision on the
> >> timetamps that are produced by the timestamping unit. The main drawback
> >> is that the sub-second increment value, used to generate timestamps, can't
> >> be set to lower than (2 / ptp_clock_freq).
> >>
> >> The "fine" qualification comes from the frequent frequency adjustments we
> >> are able to do, which is perfect for a PTP follower usecase.
> >>
> >> In Coarse mode, we don't do frequency adjustments based on an
> >> accumulator overflow. We can therefore have very fine subsecond
> >> increment values, allowing for better timestamping precision. However
> >> this mode works best when the ptp clock frequency is adjusted based on
> >> an external signal, such as a PPS input produced by a GPS clock. This
> >> mode is therefore perfect for a Grand-master usecase.
> >>
> >> We therefore attempt to map these 2 modes with the newly introduced
> >> hwtimestamp qualifiers (precise and approx).
> >>
> >> Precise mode is mapped to stmmac fine mode, and is the expected default,
> >> suitable for all cases and perfect for follower mode
> >>
> >> Approx mode is mapped to coarse mode, suitable for Grand-master.  
> > 
> > I failed to understand what this device does and what the problem is :(
> > 
> > What is your ptp_clock_freq? Isn't it around 50MHz typically? 
> > So 2 / ptp_freq is 40nsec (?), not too bad?  
> 
> That's not too bad indeed, but it makes a difference when acting as
> Grand Master, especially in this case because you don't need to
> perform clock adjustments (it's sync'd through PPS in), so we might
> as well take this opportunity to improve the TS.
> 
> > 
> > My recollection of the idea behind that timestamping providers
> > was that you can configure different filters for different providers.
> > IOW that you'd be able to say:
> >  - [precise] Rx stamp PTP packets 
> >  -  [approx] Rx stamp all packets
> > not that you'd configure precision of one piece of HW..  
> 
> So far it looks like only one provider is enabled at a given time, my
> understanding was that the qualifier would be used in case there
> are multiple timestampers on the data path, to select the better one
> (e.g. a PHY that supports TS, a MAC that supports TS, we use the 
> best out of the two).

No, we do not support multiple timestampers at the same time.
For that IIUC we would have to add a an ID of the source in the packet. I
remember people were talking about modifying cmsg. 
This qualifier is indeed a first step to walk this path but I don't think
people are currently working on adding this support for now. 

> However I agree with your comments, that's exactly the kind of feedback
> I was looking for. This work has been tried several times now each
> time with a different uAPI path, I'm OK to consider that this is out
> of the scope of the hwprov feature.
> 
> > If the HW really needs it, just lob a devlink param at it?  
> 
> I'm totally OK with that. I'm not well versed into devlink, working mostly
> with embedded devices with simple-ish NICs, most of them don't use devlink.
> Let me give it a try then :)

meh, I kind of dislike using devlink here. As I said using timestamping
qualifier is a fist step for the multiple timestamping support. If one day we
will add this support, if there is other implementation it will add burden on
the development to track and change all the other implementation. Why don't we
always use this qualifier parameter even if it is not really for simultaneous
timestamping to avoid any future wrong development choice.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com