From: Ivan Vecera <ivecera@redhat.com>
To: netdev@vger.kernel.org
Cc: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>,
Jakub Kicinski <kuba@kernel.org>, Jiri Pirko <jiri@resnulli.us>,
Paolo Abeni <pabeni@redhat.com>, Petr Oros <poros@redhat.com>,
Prathosh Satish <Prathosh.Satish@microchip.com>,
Richard Cochran <richardcochran@gmail.com>,
Shuah Khan <skhan@linuxfoundation.org>,
Vadim Fedorenko <vadim.fedorenko@linux.dev>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v2 1/2] dpll: zl3073x: update all DPLL channels on ref_sync_set
Date: Thu, 6 Aug 2026 15:07:00 +0200 [thread overview]
Message-ID: <b2d3902f-2e93-4385-9749-976725424802@redhat.com> (raw)
In-Reply-To: <20260803120245.56046-2-ivecera@redhat.com>
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
next prev parent reply other threads:[~2026-08-06 13:08 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-03 12:02 [PATCH net-next v2 0/2] dpll: use pin owner's dpll ref for pin-level set callbacks Ivan Vecera
2026-08-03 12:02 ` [PATCH net-next v2 1/2] dpll: zl3073x: update all DPLL channels on ref_sync_set Ivan Vecera
2026-08-06 13:07 ` Ivan Vecera [this message]
2026-08-03 12:02 ` [PATCH net-next v2 2/2] dpll: use pin owner's dpll ref for pin-level attribute setting Ivan Vecera
2026-08-06 9:41 ` Paolo Abeni
2026-08-06 13:11 ` Ivan Vecera
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b2d3902f-2e93-4385-9749-976725424802@redhat.com \
--to=ivecera@redhat.com \
--cc=Prathosh.Satish@microchip.com \
--cc=arkadiusz.kubalewski@intel.com \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=poros@redhat.com \
--cc=richardcochran@gmail.com \
--cc=skhan@linuxfoundation.org \
--cc=vadim.fedorenko@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.