Re: [PATCH v6 09/17] iio: adc: ad7768: Configure channel sampling profiles

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ad7768_channel_mode_mask() ambiguously combines logical and physical channel bitmasks, potentially writing to reserved or unmapped hardware bits.
--

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

iio: adc: ad7768: Configure channel sampling profiles
    
At buffered capture setup, select the fastest compatible power mode for
the enabled channel rates. Group channels into the two hardware mode
profiles and program their decimation, mode selection, and data clock.

> diff --git a/drivers/iio/adc/ad7768.c b/drivers/iio/adc/ad7768.c
> index 2020156eb6b61..868130ebcc8f1 100644
> --- a/drivers/iio/adc/ad7768.c
> +++ b/drivers/iio/adc/ad7768.c

[ ... ]

> @@ -204,6 +209,11 @@ static u8 ad7768_channel_mask(const struct ad7768_state *st, u8 ch)
>  	return BIT(st->chip_info->chan_map[ch]);
>  }
>  
> +static u8 ad7768_channel_mode_mask(const struct ad7768_state *st, u8 ch)
> +{
> +	return BIT(ch) | ad7768_channel_mask(st, ch);

[Severity: Medium]
Does this bitwise OR inadvertently combine the logical channel index and the
physical channel mapping?

When this is applied to the AD7768_REG_CH_MODE_SEL register, it could result
in writing to reserved or unmapped hardware bits, which might lead to
unpredictable hardware state.

> +}
> +
>  static u8 ad7768_all_standby_mask(const struct ad7768_state *st)
>  {
>  	return GENMASK(st->chip_info->num_channels - 1, 0);

[ ... ]

> @@ -478,11 +544,66 @@ static int ad7768_update_scan_mode(struct iio_dev *indio_dev,
>  	if (st->clock_source == AD7768_CLOCK_SOURCE_XTAL)
>  		__clear_bit(st->chip_info->num_channels / 2, &standby_mask);
>  
> +	for_each_set_bit(c, scan_mask, st->chip_info->num_channels) {
> +		unsigned int mask;
> +		int mode;
> +
> +		mode = ad7768_find_matching_mode(mode_used, mode_freq,
> +						 st->ch_freq[c]);
> +		if (mode < 0)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Over %d channel modes required\n",
> +					     AD7768_NUM_CHANNEL_MODES);
> +
> +		mode_freq[mode] = st->ch_freq[c];
> +		mode_used[mode] = true;
> +
> +		mask = ad7768_channel_mode_mask(st, c);
> +		ret = regmap_assign_bits(st->regmap, AD7768_REG_CH_MODE_SEL,
> +					 mask, mode);

[Severity: Medium]
This applies the multi-bit mask to the hardware configuration register.

Could this configure unintended channels or interfere with device compatibility?

> +		if (ret)
> +			return ret;
> +	}

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