Re: [PATCH net-next v15 10/13] 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 to consider:
- [Medium] dpll: sit9531x: missing unmute rollback in
  sit9531x_output_hiz_write()
--

commit 4e6433aa7f8c3d99e32f4350064261aad1b40ccd
Author: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@devoxsoftware.com>

dpll: sit9531x: implement output pin state on a DPLL

This commit implements state_on_dpll_get and state_on_dpll_set for output
pins by forcing or releasing the output slot's Hi-Z override registers and
registers routed output pins with the DPLL core.

> diff --git a/drivers/dpll/sit9531x/core.c b/drivers/dpll/sit9531x/core.c
> index c7ab33993c288..458de502bc8f7 100644
> --- a/drivers/dpll/sit9531x/core.c
> +++ b/drivers/dpll/sit9531x/core.c

[ ... ]

> +/*
> + * Mute or unmute every Hi-Z pair of a slot. Caller must already be in the
> + * programming state.
> + */
> +static int sit9531x_output_hiz_write(struct sit9531x_dev *sitdev, u8 slot,
> +				     bool mute)
> +{
> +	u8 mask[SIT9531X_HIZ_PAIRS], state[SIT9531X_HIZ_PAIRS];
> +	struct sit9531x_hiz_pair p[SIT9531X_HIZ_PAIRS];
> +	int i, rc, undo_rc;
> +
> +	sit9531x_output_get_hiz_regs(slot, p);
> +
> +	if (!mute) {
> +		for (i = 0; i < SIT9531X_HIZ_PAIRS; i++) {
> +			rc = sit9531x_hiz_set_bit(sitdev, p[i].mask, p[i].bit,
> +						  false);
> +			if (rc)
> +				return rc;
> +		}
> +		return 0;
> +	}

[Severity: Medium]
Should the !mute loop roll back already-cleared p[i].mask bits if a later
sit9531x_hiz_set_bit() call fails, like the mute branch below does?

sit9531x_output_enable() still runs sit9531x_prg_commit() on error.  For a
CMOS output where clearing SIT9531X_HIZ_CMOS_P succeeds and clearing
SIT9531X_HIZ_CMOS_N fails, the commit leaves OutP driving and OutN forced to
Hi-Z, and sit9531x_output_forced_hiz() then marks the pin enabled because
only one pad is muted.  sit9531x_dpll_output_pin_state_on_dpll_set() fails
the request but sends a change notification reporting the half-unmuted pin
as DPLL_PIN_STATE_CONNECTED.

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