Thread (6 messages) flat view 6 messages, 2 authors, 1d ago

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

From: Ivan Vecera <ivecera@redhat.com>
Date: 2026-08-06 13:08:20
Also in: lkml

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help