Re: [PATCH net-next v8 3/3] dpll: zl3073x: add PTP clock support

[Ivan Vecera <ivecera@redhat.com>]

Sashiko comments with my replies:

 > Should PTP_1588_CLOCK be a hard dependency here?

Yes. This was explicitly requested by Jakub in the v2 review [1].

[1] https://lore.kernel.org/netdev/20260722135848.2d401ada@kernel.org/

 > Does settime64() need to compensate for the deferred ToD load?
 > [...] does this leave the clock up to a second behind the requested
 > time on every clock_settime()?

settime64 is a coarse "slam the clock" operation. gettimex64() does
not return stale values — it calls tod_ready_wait() which blocks
until the pending WR_NEXT_1HZ completes. The PTP servo handles any
residual offset in subsequent adjtime calls.

 > Also, ts.tv_nsec is written raw. If a caller passes a non-zero
 > tv_nsec, does the ToD second boundary end up displaced from the
 > DPLL 1 Hz / output 1PPS phase?

Yes, a non-zero tv_nsec displaces the ToD second boundary from the
1PPS edge. This matches the hardware behavior — settime64 sets
the ToD counter to whatever value is requested. Phase coherence
between ToD and outputs is maintained by adjtime which routes
sub-second adjustments through phase step or TIE write.

 > Is this error return reachable after the seconds are already
 > committed? [...] Does the caller then get a failure for an
 > adjustment whose seconds part is already applied?

If tod_ready_wait fails, we cannot be certain that WR_NEXT_1HZ was
actually applied. Returning the error is the safer choice — masking
it would silently report success when the adjustment may not have
been applied at all.

 > Does the same pending WR_NEXT_1HZ hazard exist across callback
 > invocations? [...]
 > Would calling zl3073x_chan_tod_ready_wait() unconditionally before
 > the phase step / TIE write cover this?

Phase step and TIE write operate on independent hardware blocks
(output phase adjustment and DPLL input phase respectively) and do
not conflict with a pending ToD write. Any subsequent ToD operation
goes through tod_ready_wait which waits for the pending write to
complete. The cross-callback sequence described (settime64 followed
immediately by adjphase) does not occur in practice — these are
fundamentally different operations used at different stages of PTP
clock management.

 > zl3073x_dpll_alloc() and zl3073x_dpll_free() are moved verbatim
 > [...] Could it be dropped or split into a separate
 > no-functional-change patch?

The move is needed because the new PTP registration helper
zl3073x_dpll_ptp_register() must be defined before
zl3073x_dpll_alloc() which references ptp_info. Splitting this
into a separate patch was done in v3 at Jakub's request but was
squashed back when the refactored helper was dropped along with
perout support.

Thanks,
Ivan