Re: [PATCH net-next v2 1/2] dpll: zl3073x: update all DPLL channels on ref_sync_set

[Ivan Vecera]

Replies to Sashiko findings:

> Is this only a preparation change? ... Should this carry a Fixes: tag
> so it gets picked up for stable?

It is preparation only. The old code is correct — the DPLL core's
xa_for_each loop calls ref_sync_set once per registered DPLL, and
since zl3073x registers input pins with every channel, each channel
gets its own callback invocation with the correct pin_priv. All
channels receive the priority update. The bug would only appear
after patch 2 removes the xa_for_each, which is why this patch moves
the iteration into the driver first.

> Is the invariant in this comment accurate? ... 
zl3073x_dpll_pin_register()
> reads them with no lock at all ... can a ref-sync DPLL_CMD_PIN_SET on
> channel 0's pin enter the new loop and write zldev->chan[1].ref_prio
> while channel 1's zl3073x_dpll_pin_register() is reading 
chan[1].ref_prio?

No race. dpll_lock is held across the entire dpll_pin_register()
call (acquired in dpll_core.c) and across the entire netlink
DPLL_CMD_PIN_SET handler (acquired in dpll_netlink.c). The device
startup loop calls zl3073x_dpll_register() -> dpll_pin_register()
for each channel under dpll_lock, and ref_sync_set also runs under
dpll_lock, so they cannot overlap.

> Should the sync pin get a change notification for the other channels?

Good catch. Will add a sync_notify flag — set it when a channel's
priority is actually changed from non-NONE to NONE, and call
__dpll_pin_change_ntf(sync_pin->dpll_pin) after the loop if set.

> Is the downgrade of this failure to a warning intentional? ...
> The new comment calls the priority step advisory and "not fatal",
> which reads as the opposite of the changelog statement that "the
> priority update must cover all channels".

The ref sync pair configuration (zl3073x_ref_state_set) is the
essential operation whose failure is propagated. The per-channel
priority exclusion is a datasheet recommendation — the ref sync pair
functions regardless. Will align the changelog to say "recommended"
instead of "must". A mailbox failure here would indicate a serious
HW/bus problem that would surface through other error paths.

Thanks,
Ivan