Re: [PATCH v6 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:
- [Medium] Splitting the update of AD7768_REG_POWER_MODE into two separate regmap_update_bits calls creates a transient hardware state where the power mode and clock divider are mismatched, and adds redundant SPI bus overhead.
--

commit 9e0a5b0981ecb80ffeddcff0bde57100dda96fba
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 707f3ae70a95a..71db57a2b9dd6 100644
> --- a/drivers/iio/adc/ad7768.c
> +++ b/drivers/iio/adc/ad7768.c

[ ... ]

> @@ -335,6 +349,36 @@ 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,
> +				 FIELD_PREP(AD7768_POWER_MODE_POWER_MODE_MSK,
> +					    mode_info->mode));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, AD7768_REG_POWER_MODE,
> +				 AD7768_POWER_MODE_MCLK_DIV_MSK,
> +				 FIELD_PREP(AD7768_POWER_MODE_MCLK_DIV_MSK,
> +					    mode_info->mode));

[Severity: Medium]
Does splitting this register update into two consecutive read-modify-write
operations to AD7768_REG_POWER_MODE create a problematic transient hardware
state?

Updating the analog power mode first and the digital clock divider second might
leave them temporarily mismatched. If the analog power mode is updated to a
lower state while the clock divider remains fast, could this violate internal
timing constraints and risk ADC glitches or unpredictable behavior?

Could these two fields be updated in a single regmap_update_bits() call to
avoid both the transient hardware state and the redundant SPI bus overhead?

> +	if (ret)
> +		return ret;
> +
> +	ret = ad7768_sync(st);
> +	if (ret)
> +		return ret;
> +
> +	st->power_mode_idx = mode_idx;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ad7768-driver-v6-0-e4378f946bfb@analog.com?part=7