Re: [PATCH net-next v9 13/15] dpll: sit9531x: add support to get fractional frequency offset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dpll: hardware debug registers left permanently unlocked after FFO read
--

commit 646045e3cead4abd726338075ac84407fac3714f
Author: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@devoxsoftware.com>
dpll: sit9531x: add support to get fractional frequency offset

This commit introduces support for reading the fractional frequency offset 
(FFO) of a PLL's reference on the sit9531x device. It achieves this by 
comparing the running divider of the feedback loop to the configured 
divider through the chip's debug window.

> diff --git a/drivers/dpll/sit9531x/core.c b/drivers/dpll/sit9531x/core.c
> index 356d1eac4ec54..88bfa63b29755 100644
> --- a/drivers/dpll/sit9531x/core.c
> +++ b/drivers/dpll/sit9531x/core.c
[ ... ]
> @@ -1224,6 +1224,76 @@ static int sit9531x_is_xo_doubler_enabled(struct sit9531x_dev *sitdev)
>  	return (~val >> SIT9531X_XO_DOUBLER_ENB_BIT) & 1u;
>  }
>  
> +/*
> + * sit9531x_dbg_sample - latch and read a signal pathway debug sample
> + * @sitdev:	device pointer
> + * @pll_idx:	PLL index (0-3)
> + * @read_code:	which tap of the pathway to sample
> + * @buf:	result, least significant byte first
> + * @len:	bytes to read, at most SIT9531X_DBG_DATA_BYTES
> + *
> + * Return: 0 on success, <0 on error
> + */
> +static int sit9531x_dbg_sample(struct sit9531x_dev *sitdev, u8 pll_idx,
> +			       u8 read_code, bool low_freq_clk,
> +			       u8 *buf, unsigned int len)
> +{
> +	unsigned int i;
> +	int rc;
> +	u8 v;
> +
> +	if (len > SIT9531X_DBG_DATA_BYTES)
> +		return -EINVAL;
> +
> +	rc = sit9531x_write_pll_u8(sitdev, pll_idx, SIT9531X_PLL_REG_DEBUG,
> +				   SIT9531X_PLL_DEBUG_UNLOCK);
> +	if (rc)
> +		return rc;

[Severity: Medium]
Does this error path leave the PLL debug registers permanently unlocked?

The debug window is unlocked at the beginning of sit9531x_dbg_sample() with
SIT9531X_PLL_DEBUG_UNLOCK, but it appears there is no corresponding lock
operation before returning. Leaving it open could expose the device to
unintended writes or internal state corruption.

[ ... ]
> +	for (i = 0; i < len; i++) {
> +		rc = sit9531x_read_pll_u8(sitdev, pll_idx,
> +					  SIT9531X_PLL_REG_DBG_DATA_0 + i,
> +					  &buf[i]);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	return 0;
> +}

[Severity: Medium]
Similarly, does the success path also leak the unlocked state?

As documented elsewhere in the driver for this hardware, leaving the debug
window open would mean normal monitoring permanently unlocks the block.
Callers of sit9531x_dbg_sample() do not appear to relock the registers upon
completion either.

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