Held off on replying any further till I verified my understanding with a
PTP architect on our side. Looks like my understanding was correct.

On Fri, 20 Jan, 2023 16:06:09 -0800 Jakub Kicinski [off-list ref] wrote:

On Fri, 20 Jan 2023 15:58:25 -0800 Jacob Keller wrote:

quoted

Sure. I guess what I don't understand is why or when one would want to
use adjphase instead of just performing frequency adjustment via the
.adjfine operation...

Especially since "gradual adjustment over time" means it will still be
slow to converge (just like adjusting frequency is today).

We should definitely improve the doc to explain the diff between them
and make sure that its more clear to driver implementations.

Our PTP architect also was subtly confused by the differences of adjtime
and adjphase when the adjphase patch was first introduced. Vincent's
follow up helped with better understanding the difference.

The way NVIDIA devices internally handle adjphase is to adjust the
frequency for a short period of time to make the small time offset
adjustments smooth (using some internal calculations) where the time
offset "nudge" is applied but frequency is also adjusted to prevent
immediate drift after that time adjustment. However, we aren't sure if
this is the only approach possible to achieve accurate corrections for
small offset adjustments with adjphase, so I would suggest that the
documentation be updated to state something discussing that adjphase is
expected to support small jumps in offset precisely without necessarily
bringing up frequency manipulation potentially done to achieve this.

Fair point, I assumed that the adjustment is precise (as in the device
is programmed with both the extra addend and number of cycles over
which to use it). Yielding an exact adjustment.

But then again, I also thought that .adjtime is supposed to be a precise
single-shot nudge, while most drivers just do:

time = read_time()
time += delta
write_time(time)

:S  So yeah, let's document..

Our architect used the following to compare adjtime and adjphase.

  time adjustment is a 'jump' in time while phase adjustment is a smooth correction.

I think the thing though is how that smooth correction for small offset
values is achieved is not very well defined/something the implementer
can define. Will update the comments for adjphase in my patch series
based on this.

quoted

It also makes it harder to justify mapping small .adjtime to .adjphase,
as it seems like .adjphase isn't required to adjust the offset
immediately. Perhaps the adjustment size is small enough that isn't a
big problem?