Sorry to jump in, just wanted to add a few details,

On 15/05/2020 02:37, Richard Cochran wrote:

On Thu, May 14, 2020 at 05:09:01PM +0200, Olivier Dautricourt wrote:

quoted

   My issue is that the default behavior of the stmmac driver is to
   set the mac into fine mode which implies to continuously do frequency
   adjustment, So if i'm not mistaken using the nullf servo will not address
   that issue even if we are not doing explicit clock_adjtime syscalls.

Why not use the external time stamps from your GPS to do one of the
following?

A) Feed them into a user space PI controller and do frequency
    correction, or

B) Feed the offsets into the hardware PLL (if that is present) using
    the new PHC ADJ_OFFSET support (found in net-next).

I don't see a need for changing any kernel interfaces at all.  If you
think there is such a need, then you will have to make some kind of
argument or justification for it.

I think that B could work, but it doesn't solve the issue with the adjustment
mode of the mac's clock.
I understand this may be very specific to stmmac but the problem is still
there.

This MAC's Timestamping Clock has 2 adjustment modes :
- Fine
- Coarse

It has the following registers :
- sub-second-increment (32bits)
- addend (32bits)
- timestamp-control (to set the mode to fine or coarse)

It also has an accumulator (32bits)

-> When in fine mode, which is the current default,
- The sub-second-increment (in ns) is added to the clock's time when the
accumulator overflows
- At each clock cycle of ptp_ref_clock, the value of addend is added to the
the accumulator

The frequency adjustments of the mac's clock are done by changing the value of
addend, so that the number of clock cycles that make the accumulator overflow
slightly change.

The value of sub-second-increment is set to 2 / ptp_clk_freq, because using an
accumulator with the same number of bits as the addend register makes it
impossible to overflow at every addition.

This mode allows to implement a PTP Slave, constantly adjusting the clock's freq
to match the master.

-> In coarse mode, the accumulator and the addend register are not used and the
value of sub-second-increment is added to the clock at every ptp_ref_clock
cycle.
So the value of sub-second-increment can be set to 1 / ptp_clk_freq but fine
adjustments of the clocks frequency are not possible.

This mode allows to implement a Grand Master where we can feed the stmmac's ptp
ref clock input with a frequency that's controlled externally, making it
useless to do frequency adjustments with the MAC.

We want our devices to be able to switch from Master to Slave at runtime.

To make things short it implies that :

1- In coarse mode, the accuracy of the timestamps is twice what it is in fine
mode (loosing precision on the capability to adjust the mac clock's frequency)

2- I don't think modes can be switched gracefully while the timestamping clk is
active. (2 registers need to be modified : sub-second-increment and control)

1 makes it quite valuable to be able to switch mode. 2 means that this
cannot be done when doing the clock adjustment (offset vs. freq adj. for
instance).

So the question is what interface could we use to configure a timestamping
clock that has more than one functioning mode and which mode can be changed at
runtime, but not while timestamping is running ?

Another thing to note is that based on your comments, I understand that the way
the stmmac's clock is handled now, i.e configuring and enabling the clock when
enabling hardware timestamping may not be right, and we may want to split
enabling the clock and enabling hardware timestamping, but we'd still
need to be able to switch the mode of adjustement of the clock.

I hope this helps a bit,
Julien