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.