From: Ali Rouhi <arouhi@sitime.com>
To: "netdev-bot+sashiko@kernel.org" <netdev-bot+sashiko@kernel.org>
Cc: Jiri Pirko <jiri@resnulli.us>,
Vadim Fedorenko <vadim.fedorenko@linux.dev>,
Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>,
Ivan Vecera <ivecera@redhat.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Carolina Jubran <cjubran@nvidia.com>,
Oleg Zadorozhnyi <Oleg.Zadorozhnyi@devoxsoftware.com>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next v11 10/13] dpll: sit9531x: implement output pin state on a DPLL
Date: Fri, 9 Oct 2026 18:23:28 +0000 [thread overview]
Message-ID: <20261009182323.76166-8-arouhi@sitime.com> (raw)
In-Reply-To: <179116260402.434549.6445320171946602725@kernel.org>
On Mon, 5 Oct 2026, netdev-bot+sashiko@kernel.org wrote:
Replies inline, in the order of the summary list. All three are right
and all three are fixed in v12.
> [Severity: High]
> Can dpin->dpll_pin be NULL here if the device is being torn down?
>
> [...]
>
> Would it be safer to use the pin argument the core passes to this
> callback, which stays valid until dpll_pin_unregister() returns? Another
> option is to read dpin->dpll_pin once and skip the notification when it is
> NULL.
Right. sit9531x_dpll_pin_unregister() clears the handle before it can
take the core's lock, and the setter read it after more than a hundred
milliseconds of transfers with nothing between the two.
The setter now reads the handle once and sends no notification when it is
NULL; the clear and the read are a WRITE_ONCE/READ_ONCE pair. The poll's
own notifications need no guard, since the poll is cancelled before any
pin is unregistered.
> [Severity: Medium]
> Should there be an early return when the request matches the current state?
>
> [...]
>
> Also, the commit message calls the mute a per-output control that "leaves
> the divider alone". It doesn't mention that every request runs the
> device-wide programming sequence above.
Fixed: the Hi-Z pairs are read first and a request that matches them
returns without entering the programming state. Each state change that
does change something still runs the device-wide programming sequence,
and the commit message now says so.
> [Severity: Medium]
> Can a state change go unannounced when the read-back fails?
>
> [...]
>
> Should state_stale count as a possible change here? Alternatively, the
> later refresh could announce the new state if it differs from the cached
> one.
Fixed: the notification is sent when the cached state moved or when the
read-back failed and left it marked stale, so a subscriber is not left
with the old state; the getter's read-through supplies the value.
pw-bot: cr
next prev parent reply other threads:[~2026-10-09 18:23 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-30 23:37 [PATCH net-next v11 00/13] dpll: add SiTime SiT9531x DPLL clock driver Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 01/13] dt-bindings: dpll: allow hex unit addresses on output pins Ali Rouhi
2026-10-02 8:32 ` Krzysztof Kozlowski
2026-09-30 23:37 ` [PATCH net-next v11 02/13] dt-bindings: vendor-prefixes: add SiTime Corporation Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 03/13] dt-bindings: dpll: add SiTime SiT95316 clock generator Ali Rouhi
2026-10-05 1:09 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi
2026-10-06 3:22 ` Rob Herring (Arm)
2026-10-09 18:23 ` Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 05/13] dpll: sit9531x: read DPLL types and pin properties from system firmware Ali Rouhi
2026-10-05 1:09 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 04/13] dpll: add basic SiTime SiT9531x support Ali Rouhi
2026-10-05 1:09 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 06/13] dpll: sit9531x: register DPLL devices and pins Ali Rouhi
2026-10-05 1:09 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 07/13] dpll: sit9531x: implement input pin state on a DPLL Ali Rouhi
2026-10-05 1:10 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 08/13] dpll: sit9531x: add support to get and set priority on input pins Ali Rouhi
2026-10-05 1:10 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 10/13] dpll: sit9531x: implement output pin state on a DPLL Ali Rouhi
2026-10-05 1:10 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi [this message]
2026-09-30 23:37 ` [PATCH net-next v11 09/13] dpll: sit9531x: add support to get and set frequency on pins Ali Rouhi
2026-10-05 1:10 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 11/13] dpll: sit9531x: add support to adjust output phase Ali Rouhi
2026-10-05 1:10 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 12/13] dpll: sit9531x: add support to get phase offset on the connected input pin Ali Rouhi
2026-10-05 1:10 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 13/13] dpll: sit9531x: model the inter-PLL sync net as a pair of pins Ali Rouhi
2026-10-02 9:14 ` sashiko-bot
2026-10-05 1:10 ` netdev-bot+sashiko
2026-10-09 18:23 ` Ali Rouhi
2026-10-07 2:10 ` [PATCH net-next v11 00/13] dpll: add SiTime SiT9531x DPLL clock driver patchwork-bot+netdevbpf
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=20261009182323.76166-8-arouhi@sitime.com \
--to=arouhi@sitime.com \
--cc=Oleg.Zadorozhnyi@devoxsoftware.com \
--cc=arkadiusz.kubalewski@intel.com \
--cc=cjubran@nvidia.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=ivecera@redhat.com \
--cc=jiri@resnulli.us \
--cc=krzk+dt@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev-bot+sashiko@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=robh@kernel.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.