Re: [PATCH v7 07/17] iio: adc: ad7768: Add power mode helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect register value is written to the MCLK_DIV bitfield for MEDIAN power mode, leading to hardware and software clock divergence.
--

commit 52c81df3b6fbeb2c9857e18cf49af9fcfe8ebeae
Author: Janani Sunil <janani.sunil@analog.com>

iio: adc: ad7768: Add power mode helper

Describe each power mode and its internal master-clock divisor in a
table. Replace the open-coded fast-mode setup with a helper that records
the active mode, preparing for dynamic mode selection.

> diff --git a/drivers/iio/adc/ad7768.c b/drivers/iio/adc/ad7768.c
> index 44ba19fb20962..be815b8c6766e 100644
> --- a/drivers/iio/adc/ad7768.c
> +++ b/drivers/iio/adc/ad7768.c
[ ... ]
> @@ -102,6 +104,17 @@ enum ad7768_clock_source {
>  	AD7768_CLOCK_SOURCE_LVDS,
>  };
>  
> +struct ad7768_power_mode_info {
> +	unsigned int mode;
> +	unsigned int mclk_div;
> +};
> +
> +static const struct ad7768_power_mode_info ad7768_power_modes[] = {
> +	{ .mode = AD7768_POWER_MODE_POWER_MODE_LOW, .mclk_div = 32 },
> +	{ .mode = AD7768_POWER_MODE_POWER_MODE_MEDIAN, .mclk_div = 8 },
> +	{ .mode = AD7768_POWER_MODE_POWER_MODE_FAST, .mclk_div = 4 },
> +};
> +
[ ... ]
> @@ -330,6 +344,32 @@ static int ad7768_sync(struct ad7768_state *st)
>  			       AD7768_DATA_CONTROL_SPI_SYNC);
>  }
>  
> +static int ad7768_set_power_mode(struct ad7768_state *st,
> +				 unsigned int mode_idx)
> +{
> +	const struct ad7768_power_mode_info *mode_info;
> +	int ret;
> +
> +	mode_info = &ad7768_power_modes[mode_idx];
> +	ret = regmap_update_bits(st->regmap, AD7768_REG_POWER_MODE,
> +				 AD7768_POWER_MODE_POWER_MODE_MSK |
> +				 AD7768_POWER_MODE_MCLK_DIV_MSK,
> +				 FIELD_PREP(AD7768_POWER_MODE_POWER_MODE_MSK,
> +					    mode_info->mode) |
> +				 FIELD_PREP(AD7768_POWER_MODE_MCLK_DIV_MSK,
> +					    mode_info->mode));

[Severity: High]
When configuring MEDIAN mode via ad7768_set_power_mode(), does this code
inadvertently configure the MCLK_DIV bitfield using the power mode
register value instead of the correct clock divider register code?

For MEDIAN power mode, mode_info->mode is 0x2. Writing 0x2 to the MCLK_DIV
bitfield sets the hardware divider to 4 (MCLK/4), instead of the expected
divider of 8 (MCLK/8, which requires register value 0x1).

Since ad7768_power_modes correctly defines mclk_div = 8 for MEDIAN mode, the
software will calculate output rates and delays based on a divider of 8, while
the hardware runs twice as fast.

> +	if (ret)
> +		return ret;
> +
> +	ret = ad7768_sync(st);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-ad7768-driver-v7-0-7f70b07627e1@analog.com?part=7