Re: [PATCH net-next v13 08/12] dpll: sit9531x: add support to get and set frequency on pins

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dpll: sit9531x: byte order reversal when reading DIVN registers
--

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

dpll: sit9531x: add support to get and set frequency on pins

This commit adds the ability to get and set pin frequencies for the sit9531x
DPLL driver. It reads and writes DIVN registers to compute output frequency
based on VCO rate and a divider.

> diff --git a/drivers/dpll/sit9531x/core.c b/drivers/dpll/sit9531x/core.c
> index f3dbe949f219b..875a5fe783d5c 100644
> --- a/drivers/dpll/sit9531x/core.c
> +++ b/drivers/dpll/sit9531x/core.c
[ ... ]
> @@ -1342,6 +1481,675 @@ int sit9531x_input_prio_add(struct sit9531x_dev *sitdev, u8 pll_idx,
[ ... ]
> +static int sit9531x_divn_static(struct sit9531x_dev *sitdev, u8 pll_idx,
> +				s64 *divn)
> +{
> +	u32 int_part, fracn_raw = 0, fracd_raw = 0;
> +	u64 fracd;
> +	s64 fracn;
> +	int rc, i;
> +	u8 v;
> +
> +	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
> +				  SIT9531X_PLL_REG_DIVN_INT, &v);
> +	if (rc)
> +		return rc;
> +	int_part = v;
> +
> +	for (i = 3; i >= 0; i--) {
> +		rc = sit9531x_read_pll_u8(sitdev, pll_idx,
> +					  SIT9531X_PLL_REG_DIVN_NUM + i, &v);
> +		if (rc)
> +			return rc;
> +		fracn_raw = (fracn_raw << 8) | v;
> +	}

[Severity: Medium]
Since the device registers are big-endian, does this loop reverse the byte
order? By looping i from 3 down to 0 and shifting the result left, the
value from the highest address (which should be the LSB) is placed into
the most significant byte of fracn_raw, which could result in incorrect
VCO frequency calculations.

> +
> +	for (i = 3; i >= 0; i--) {
> +		rc = sit9531x_read_pll_u8(sitdev, pll_idx,
> +					  SIT9531X_PLL_REG_DIVN_DEN + i, &v);
> +		if (rc)
> +			return rc;
> +		fracd_raw = (fracd_raw << 8) | v;
> +	}

[Severity: Medium]
Similarly for the denominator, is the byte order also reversed here by
reading the highest address first and shifting it into the MSB?

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