On Thu, 2023-05-25 at 14:08 +0200, Paolo Abeni wrote:

On Tue, 2023-05-23 at 13:54 -0700, Rahul Rameshbabu wrote:

quoted

Advertise the maximum offset the .adjphase callback is capable of
supporting in nanoseconds for IDT ClockMatrix devices.

Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Vincent Cheng <redacted>
Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
---
 drivers/ptp/ptp_clockmatrix.c | 36 +++++++++++++++++------------------
 drivers/ptp/ptp_clockmatrix.h |  2 +-
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c
index c9d451bf89e2..f6f9d4adce04 100644
--- a/drivers/ptp/ptp_clockmatrix.c
+++ b/drivers/ptp/ptp_clockmatrix.c

@@ -1692,14 +1692,23 @@ static int initialize_dco_operating_mode(struct idtcm_channel *channel)
 /* PTP Hardware Clock interface */
 
 /*
- * Maximum absolute value for write phase offset in picoseconds
- *
- * @channel:  channel
- * @delta_ns: delta in nanoseconds
+ * Maximum absolute value for write phase offset in nanoseconds
  *
  * Destination signed register is 32-bit register in resolution of 50ps
  *
- * 0x7fffffff * 50 =  2147483647 * 50 = 107374182350
+ * 0x7fffffff * 50 =  2147483647 * 50 = 107374182350 ps
+ * Represent 107374182350 ps as 107374182 ns
+ */
+static s32 idtcm_getmaxphase(struct ptp_clock_info *ptp __always_unused)
+{
+	return MAX_ABS_WRITE_PHASE_NANOSECONDS;
+}

This introduces a functional change WRT the current code. Prior to this
patch ClockMatrix tries to adjust phase delta even above
MAX_ABS_WRITE_PHASE_NANOSECONDS, limiting the delta to such value.
After this patch it will error out.

Perhaps a more conservative approach would be keeping the existing
logic in _idtcm_adjphase and let idtcm_getmaxphase return  
S32_MAX?

Note that even that will error out for delta == S32_MIN so perhaps an
API change to allow the driver specify unlimited delta would be useful
(possibly regardless of the above).

What about allowing drivers with no getmaxphase() callback, meaning
such drivers allow adjusting unlimited phase delta? 

Thanks!

Paolo