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