Message-ID: <911372f3-d941-44a8-bec2-dcc1c14d53dd@bootlin.com>
Date: Tue, 21 Oct 2025 10:02:01 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: 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>,
 Köry Maincent <kory.maincent@...tlin.com>,
 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 Jakub,

On 21/10/2025 03:03, Jakub Kicinski wrote:
> On Sat, 18 Oct 2025 09:42:57 +0200 Maxime Chevallier wrote:
>>> 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 :)
>>
>> Thanks for taking a look at this,
> 
> FWIW I dropped this form PW in an attempt to unblock testing of
> Russell's series.

Yup no problem for me

> I'm not convinced that the tsconfig API is correct
> here but I don't get how the HW works. Could you perhaps put together
> some pseudocode?

I'm not fully convinced either, devlink could also work. I already have 
a patchset ready that uses devlink for that.

Let's see if I can do a better job explaining this timestamping
parameter :


- In "fine" mode (mapped to precise qualifier if we go with the
tsinfo approach), which is the mode currently used in stmmac :

  u32 accumulator
  u32 addend
  u64 subsecond_increment

  at each ref ptp clock cycle {
    
    accumulator += addend;

    /* accumulator and addend are 32 bits. addend is adjusted
     * to control the drift of the target frequency.
     */
    if (accumulator overflows)
      current_time += subsecond_increment

  }

Main advantage: We can fine tune when we update the current_time, so
basically we can multiply or divide the ref ptp clock based on when
the accumulator overflows, and use the resulting clock as the
timestamp source.

In practice for stmmac, we set the addend to a value so that it takes
2 cycles to overflow, and we adjust the subsecond increment when
updating the PHC frequency. This gives a precision of 46-ish ns.

We do this by design as the ptp ref clk may be fairly slow (down to
5MHz in some cases). If we try to rely on accumulator overflowing at
every cycle to update the time, we'd have to overflow multiple times
per cycle, and the computation of the addend value simply overflows,
cf commit 91a2559 ("net: stmmac: Fix sub-second increment") [1]

This is basic PTP follower mode, we continuously update
the clock used for timestamping.


- In "coarse" mode :

  u32 subsecond_increment

  at each ref ptp clock cycle {
    current_time += subecond_increment
  }

We increment time at a fixed rate. The current_time can only be adjusted
by setting the entire current_time, which is jittery or even non-monotonically
increasing. However, we can use a 20ns increment (or even lower if the ref
clock is faster), and if our ref clock is externally provided by a high
precision source, this allows implementing Grand Master mode with the
highest possible precision.

[1] : https://lore.kernel.org/netdev/20200415122432.70972-2-julien.beraud@orolia.com/

Hopefully that's a bit clearer WRT what the HW can do.

Let me know if you need more clarifications on this, I'll send another
iteration once Russell's cleanup lands, using either devlink or tsconfig
depending on what we chose here.

Maxime

Powered by blists - more mailing lists