Message-ID: <b2c58580-d891-4d10-b3dd-572f7f98c6fe@bootlin.com>
Date: Mon, 20 Oct 2025 11:32:37 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Kory Maincent <kory.maincent@...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
Subject: Re: [PATCH net-next 2/3] net: stmmac: Allow supporting coarse
 adjustment mode

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.

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 ?

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.

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 ?

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'm opened for both approached TBH :)

Maxime

Powered by blists - more mailing lists