Re: [PATCH net-next v9 09/15] dpll: sit9531x: implement output pin state on a DPLL

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dpll: sit9531x: swallow error from sit9531x_output_state_refresh()
--

commit f159d5a1914892eafb68d095a92591412da0d7b2
Author: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@devoxsoftware.com>

dpll: sit9531x: implement output pin state on a DPLL

An output is driven by its PLL rather than selected by it, so its state
says whether it carries a signal: connected while it drives, disconnected
while it is muted. Setting the state mutes or un-mutes it by forcing the
pad to Hi-Z, the only per-output control the device offers that leaves the
divider alone.

> diff --git a/drivers/dpll/sit9531x/dpll.c b/drivers/dpll/sit9531x/dpll.c
> --- a/drivers/dpll/sit9531x/dpll.c
> +++ b/drivers/dpll/sit9531x/dpll.c

[ ... ]

> @@ -736,10 +736,91 @@ sit9531x_dpll_output_pin_frequency_set(const struct dpll_pin *pin,

[ ... ]

> +static int
> +sit9531x_dpll_output_pin_state_on_dpll_get(const struct dpll_pin *pin,
> +					   void *pin_priv,
> +					   const struct dpll_device *dpll,
> +					   void *dpll_priv,
> +					   enum dpll_pin_state *state,
> +					   struct netlink_ext_ack *extack)
> +{
> +	struct sit9531x_dpll_pin *dpin = pin_priv;
> +	struct sit9531x_dpll *sitdpll = dpll_priv;
> +	struct sit9531x_dev *sitdev = sitdpll->dev;
> +	const struct sit9531x_out *out;
> +
> +	/*
> +	 * A mute whose read-back failed left the cache unconfirmed; there is
> +	 * no poll of output state to correct it, so read it here rather than
> +	 * report a value that may predate the request.
> +	 */
> +	if (sitdev->out[dpin->id].state_stale) {
> +		mutex_lock(&sitdev->multiop_lock);
> +		sit9531x_output_state_refresh(sitdev, dpin->id);
> +		mutex_unlock(&sitdev->multiop_lock);
> +	}
> +
> +	out = sit9531x_out_state_get(sitdev, dpin->id);
> +	*state = out->enabled ? DPLL_PIN_STATE_CONNECTED
> +			      : DPLL_PIN_STATE_DISCONNECTED;
> +	return 0;
> +}

[Severity: Medium]
Does this code swallow the error code returned by
sit9531x_output_state_refresh()?

Looking at sit9531x_dpll_output_pin_state_on_dpll_get() in
drivers/dpll/sit9531x/dpll.c, if a user requests the DPLL pin state via
netlink while the pin's state_stale flag is true, and the subsequent
I2C read fails in sit9531x_output_state_refresh(), it appears this
function will still return 0 (success) to userspace.

Should this propagate the I2C error instead of returning a potentially
stale and unconfirmed hardware state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915000015.80480-1-arouhi@sitime.com?part=9