Message-ID: <20251020145214.64186fc9@kmaincent-XPS-13-7390>
Date: Mon, 20 Oct 2025 14:52:14 +0200
From: Kory Maincent <kory.maincent@...tlin.com>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc: Jakub Kicinski <kuba@...nel.org>, Alexandre Torgue
 <alexandre.torgue@...s.st.com>, Jose Abreu <joabreu@...opsys.com>, Andrew
 Lunn <andrew+netdev@...n.ch>, davem@...emloft.net, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Maxime Coquelin
 <mcoquelin.stm32@...il.com>, Richard Cochran <richardcochran@...il.com>,
 Russell King <linux@...linux.org.uk>, Alexis Lothoré
 <alexis.lothore@...tlin.com>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>, netdev@...r.kernel.org,
 linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, Willem
 de Bruijn <willemdebruijn.kernel@...il.com>
Subject: Re: [PATCH net-next 2/3] net: stmmac: Allow supporting coarse
 adjustment mode

On Mon, 20 Oct 2025 11:32:37 +0200
Maxime Chevallier <maxime.chevallier@...tlin.com> wrote:

> Hi Köry,
> 
> On 20/10/2025 11:00, Kory Maincent wrote:
> > On Sat, 18 Oct 2025 09:42:57 +0200
> > Maxime Chevallier <maxime.chevallier@...tlin.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.
> >  
> 
> On my side I've implemented the devlink-based approach, and I have to say i'm
> not so unhappy with it :) At least I don't have the feeling this is bending
> the API to fit one specific case.

Indeed I don't think so, but my idea was to generalize the selection of
the timestamp provider source to one API even if it is only one clock for two
different qualifiers.
 
> The thing is that the qualifier model doesn't fully map to the situation we
> have in stmmac.
> 
> The stmmac coarse/fine adjustment doesn't only changes the timestamping
> behaviour, but also the ptp_clock adjustment mode. 
> 
> So changing the qualifier here will have a side effect on the PTP clock,
> do we accept that as part of the hwprov timestamping API ?

Yes, I see the timestamp source as a couple of a qualifier plus a PTP
clock index therefore if we change the timestamp source it is intended to have
side effect.

> Should we use this API for coarse/fine stmmac config, I agree with your
> previous comment of adding a dedicated qualifier that explicitely says
> that using this qualifier comes with side effects, with the risk of
> paving the way for lots of modes being added for driver-specific scenarios.

I am not really a PTP in the field user but maybe there is a limited number of
generic qualifier possible. Here we could have a qualifier for better frequency
precision and one for better timestamping precision. I don't think we will end
with tons of different qualifiers.
Maybe PTP maintainers and users like Richard or Willem have pointers on the
number of possible qualifier?

> Another thing with the stmmac implem is that we don't truly have 2
> timestampers (1 approx and 1 precise), but rather only one whose precision
> can be adjusted. Does it really make sense here to have the qualifier
> writeable for the same timestamper ?

I do think so.

> Of course the netlink tsinfo/tsconfig is more appealing due to its generic
> nature, but OTHO I don't want to introduce ill-defined behaviours in that
> API with this series. The multiple timestamper work still makes a ton of
> sense for MAC+PHY timestamping setups :)

I think that is where it would be nice to have a review from Richard or
Willem on this to give us pointers on what is existing in the PTP world and if
using a qualifier makes sense.

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

Powered by blists - more mailing lists