RE: [PATCH net] net: ethernet: ti: am65-cpts: adjust perout genf following ptp changes

[Keller, Jacob E]

> -----Original Message-----
> From: James Clark <jjc@xxxxxxxxxx>
> Sent: Thursday, October 29, 2026 9:23 PM
> To: Keller, Jacob E <jacob.e.keller@xxxxxxxxx>
> Cc: Andrew Lunn <andrew+netdev@xxxxxxx>; David S. Miller
> <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; Jakub
> Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>; Richard
> Cochran <richardcochran@xxxxxxxxx>; Vadim Fedorenko
> <vadim.fedorenko@xxxxxxxxx>; Kory Maincent
> <kory.maincent@xxxxxxxxxxx>; Sebastian Andrzej Siewior
> <bigeasy@xxxxxxxxxxxxx>; Martin Kaistra <martin.kaistra@xxxxxxxxxxxxx>;
> Grygorii Strashko <grygorii.strashko@xxxxxx>; Aksh Garg <a-garg7@xxxxxx>;
> netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net] net: ethernet: ti: am65-cpts: adjust perout genf
> following ptp changes
> 
> On 10/29/26 22:57, Jacob Keller wrote:
> > Makes sense. My only concern is how long does it take to finish the
> > adjustment to fix the PPS? Could that impact the rate at which you can
> > perform frequency adjustments? In typical deployments, frequency
> > adjustments happen at every PTP sync event, which could be many times
> > per second.
> 
> Hi Jacob, thanks for taking a look at this.
> 
> The register reads and writes happen over the SoC interconnect and so
> are fast: I timed clock_adjtime(ADJ_FREQUENCY) on a BeagleY-AI and each
> enabled GenF adds 0.84 us. I also tested with PTP sync events at 128
> syncs per second, and it worked fine.
> 
> > Also, is there any loss inbetween when the correction comes to each GenF
> > that could still create (a much smaller) inaccuracy between them that
> > compounds over time with each frequency adjustment? Or does this
> updated
> > correction keep everything aligned by the next pulse?
> 
> There is an issue here with how the hardware works, which I think means
> that it is not possible to guarantee that the periodic output is
> absolutely in sync with the PHC in all situations. The hardware has two
> separate counters: a timestamp counter used by the PHC and a GenF
> counter used by the periodic output. Each of these has its own PPM
> counter used to apply frequency corrections. Each PPM counter is a
> down-counter: it loads the period from the PPM registers, counts down to
> zero, makes a one-tick adjustment in the direction set at that moment,
> and then loads the period again (as described for GenF in
> 12.2.1.4.7.11.2 of the TRM). adjfine() just writes the PPM registers;
> the hardware does not give access to the PPM counters. The PPM counters
> are not in sync, so they can sample the frequency from their
> corresponding registers at different times, and the corrections applied
> to the timestamp and GenF counters can accordingly be different.
> 
> I tested that the hardware behaves consistently with the TRM. As before,
> the GENF0 output was looped back to EXTTS 0. Writing a constant
> frequency every 10 ms, there is no drift. But by writing a frequency
> that alternates between +0.1 and -0.1 ppm every 10 ms, I was able to
> produce a drift of 100 ns/s.
> 
> But in practice, this does not seem to be a significant problem, at
> least on my particular board. I tested with ptp4l locked to a
> GPS-disciplined grandmaster, at 1, 16 and 128 syncs a second, for 60, 10
> and 5 minutes, and did not see any drift. I should mention though that
> this board's crystal is 10 ppm slow, so the adjustment is large and the
> PPM counter periods correspondingly short. The results might be worse on
> a board with a more accurate crystal.
> 
> In any case, as far as I can see this is how the hardware is designed to
> work, and I think this patch makes it work as designed.
> 

Makes sense. Every system will have errors and the drift is unfortunate but nothing the driver can really do to change the hardware design. It seems likely to be very small and below the expected use cases in practice.

Reviewed-by: Jacob Keller <Jacob.e.keller@xxxxxxxxx>

> > Obviously this is still better than what was happening before where the
> > corrections weren't even applied.
> 
> Indeed.
> 
> James