Thread (59 messages) 59 messages, 8 authors, 2023-01-25

Re: [net-next 03/15] net/mlx5: Add adjphase function to support hardware-only offset control

From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Date: 2023-01-20 05:28:03

On Thu, 19 Jan, 2023 21:08:42 -0800 Jakub Kicinski [off-list ref] wrote:
On Thu, 19 Jan 2023 20:26:04 -0800 Rahul Rameshbabu wrote:
quoted
One of my concerns with doing this is breaking userspace expectations.
In linuxptp, there is a configuration setting "write_phase_mode" and an
expectation that when adjphase is called, there will not be a fallback
to adjtime. This because adjphase is used in situations where small fine
tuning is explicitly needed, so the errors would indicate a logical or
situational error.
I don't mean fallback - just do what you do in mlx5 directly in 
the core. The driver already does:

if delta < MAX
	use precise method
else
	use coarse method
Oh, I see. I was thinking we were discussing ADJ_OFFSET in the
ptp_clock_adjtime function in the ptp core stack. The suggestion you are
proposing would be for the ADJ_SETOFFSET operation, and I agree with
this. Thanks for the clarification.
quoted
Quoting Vincent Cheng, the author of the adjphase functionality in the
ptp core stack.

-----BEGIN QUOTE-----
  adjtime modifies HW counter with a value to move the 1 PPS abruptly to new location.
  adjphase modifies the frequency to quickly nudge the 1 PPS to new location
and also includes a HW filter to smooth out the adjustments and fine tune
frequency.

  Continuous small offset adjustments using adjtime, likley see sudden shifts
of the 1 PPS. The 1 PPS probably disappears and re-appears.
  Continuous small offset adjustments using adjphase, should see continuous 1 PPS.

  adjtime is good for large offset corrections
  adjphase is good for small offset corrections to allow HW filter to control
the frequency instead of relying on SW filter.
Hm, so are you saying that:

adjtime(delta):
	clock += delta

but:

adjfreq(delta):
Did you mean adjphase here?
	on clock tick & while delta > 0:
		clock += small_value
		delta -= small_value

because from looking at mlx5 driver code its unclear whether the
implementation does a precise but one shot adjustment or gradual
adjustments.
The pseudo code your drafted is accurate otherwise. The lack of clarity
in our driver comes from leaving the responsibility of that smooth
gradual transition (to keep in sync with the clock frequency while
running) up to the device.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help