netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Julien Beraud <julien.beraud@orolia.com>
To: Richard Cochran <richardcochran@gmail.com>,
	Olivier Dautricourt <olivier.dautricourt@orolia.com>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>,
	Alexandre Torgue <alexandre.torgue@st.com>,
	Jose Abreu <joabreu@synopsys.com>,
	"David S . Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org
Subject: Re: [PATCH 0/3] Patch series for a PTP Grandmaster use case using stmmac/gmac3 ptp clock
Date: Fri, 15 May 2020 15:26:47 +0200	[thread overview]
Message-ID: <3a14f417-1ae1-9434-5532-4b3387f25d18@orolia.com> (raw)
In-Reply-To: <20200515003706.GB18192@localhost>

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:
>>    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

  reply	other threads:[~2020-05-15 13:26 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-14 10:28 [PATCH 0/3] Patch series for a PTP Grandmaster use case using stmmac/gmac3 ptp clock Olivier Dautricourt
2020-05-14 10:28 ` [PATCH 1/3] net: stmmac: gmac3: add auxiliary snapshot support Olivier Dautricourt
2020-05-14 10:28 ` [PATCH 2/3] net: uapi: Add HWTSTAMP_FLAGS_ADJ_FINE/ADJ_COARSE Olivier Dautricourt
2020-05-14 13:38   ` Richard Cochran
2020-05-14 15:20     ` Olivier Dautricourt
2020-05-15  0:29       ` Richard Cochran
2020-05-14 10:28 ` [PATCH 3/3] net: stmmac: Support coarse mode through ioctl Olivier Dautricourt
2020-05-27  3:55   ` Richard Cochran
2020-06-03 16:12     ` Olivier Dautricourt
2020-05-14 13:53 ` [PATCH 0/3] Patch series for a PTP Grandmaster use case using stmmac/gmac3 ptp clock Richard Cochran
2020-05-14 15:09   ` Olivier Dautricourt
2020-05-15  0:37     ` Richard Cochran
2020-05-15 13:26       ` Julien Beraud [this message]
2020-05-15 23:30         ` Richard Cochran
2020-05-25 10:00           ` Olivier Dautricourt
2020-05-27  4:05         ` Richard Cochran
2020-06-03 15:17           ` Olivier Dautricourt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3a14f417-1ae1-9434-5532-4b3387f25d18@orolia.com \
    --to=julien.beraud@orolia.com \
    --cc=alexandre.torgue@st.com \
    --cc=davem@davemloft.net \
    --cc=joabreu@synopsys.com \
    --cc=netdev@vger.kernel.org \
    --cc=olivier.dautricourt@orolia.com \
    --cc=peppe.cavallaro@st.com \
    --cc=richardcochran@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).