Re: [PATCH net-next v11 10/13] dpll: sit9531x: implement output pin state on a DPLL

[Ali Rouhi]

On Mon, 5 Oct 2026, netdev-bot+sashiko@xxxxxxxxxx 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