Re: [PATCH v6 07/17] iio: adc: at91-sama5d2_adc: adapt the driver for sama7d65

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The temperature channel on SAMA7D65 (and SAMA7G5) is broken for buffered IIO capture and causes hardware corruption when enabled.
- [High] Channel 0 is incorrectly treated as a touchscreen channel for SAMA7D65 (and SAMA7G5), causing invalid register writes and hardware corruption.
--

--- Patch [7]: [PATCH v6 07/17] iio: adc: at91-sama5d2_adc: adapt the driver for sama7d65 ---
commit d121e4a04e57309a9a2c8f55746f85c0f6a300ed
Author: Varshini Rajendran <varshini.rajendran@microchip.com>

iio: adc: at91-sama5d2_adc: adapt the driver for sama7d65
    
This commit adds support for the sama7d65 ADC by introducing a new
compatible string and platform data struct. It also updates the Kconfig
help text to mention the SAMA7 SoC family. The main difference highlighted
is the calibration data layout.

> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 3e9feb824a66f..c1e5139a058ca 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c

[ ... ]

> @@ -766,6 +775,24 @@ static const struct at91_adc_platform sama7g5_platform = {
>  	.temp_calib_layout = &sama7g5_temp_calib,
>  };
>  
> +static const struct at91_adc_platform sama7d65_platform = {
> +	.layout = &sama7g5_layout,
> +	.adc_channels = &at91_sama7g5_adc_channels,
> +	.nr_channels = AT91_SAMA7G5_SINGLE_CHAN_CNT +
> +		       AT91_SAMA7G5_DIFF_CHAN_CNT +
> +		       AT91_SAMA7G5_TEMP_CHAN_CNT,
> +	.max_channels = ARRAY_SIZE(at91_sama7g5_adc_channels),
> +	.max_index = AT91_SAMA7G5_MAX_CHAN_IDX,

[Severity: High]
This is a pre-existing issue, but it appears to be propagated to the new
sama7d65 SoC. Can this max_index value break buffered IIO capture for the
temperature channel?

The driver sets max_index to 25 (AT91_SAMA7G5_MAX_CHAN_IDX), but the
temperature channel uses scan_index 31.

When a user enables only the temperature channel (scan_index 31),
at91_adc_current_chan_is_touch() checks only up to max_index + 1 (26):

drivers/iio/adc/at91-sama5d2_adc.c:at91_adc_current_chan_is_touch() {
    ...
	return !!bitmap_subset(indio_dev->active_scan_mask,
			       &st->touch_st.channels_bitmask,
			       st->soc_info.platform->max_index + 1);
}

Since bit 31 is beyond this size, the first 26 bits of active_scan_mask are 0,
making bitmap_subset() incorrectly return true.

This misidentifies the temperature channel as a touchscreen, invoking
at91_adc_configure_touch() which writes to TSMR (offset 0 on this SoC),
corrupting the ADC Control Register (CR) and breaking the device.

> +	.hw_trig_cnt = AT91_SAMA7G5_HW_TRIG_CNT,
> +	.osr_mask = GENMASK(18, 16),
> +	.oversampling_avail = { 1, 4, 16, 64, 256 },
> +	.oversampling_avail_no = 5,
> +	.chan_realbits = 16,
> +	.temp_sensor = true,
> +	.temp_chan = AT91_SAMA7G5_ADC_TEMP_CHANNEL,
> +	.temp_calib_layout = &sama7d65_temp_calib,
> +};

[Severity: High]
This isn't a bug introduced by this patch, but the same pattern is copied
here. Could leaving the touch channel fields uninitialized incorrectly treat
ADC channel 0 as a touchscreen?

The sama7d65_platform struct does not initialize touch_chan_x, touch_chan_y,
or touch_chan_p, so they default to 0.

During at91_adc_probe(), bitmap_set() unconditionally sets these bits in the
touchscreen channels_bitmask:

drivers/iio/adc/at91-sama5d2_adc.c:at91_adc_probe() {
    ...
	bitmap_set(&st->touch_st.channels_bitmask,
		   st->soc_info.platform->touch_chan_x, 1);
    ...
}

When channel 0 is used for buffered capture, at91_adc_current_chan_is_touch()
incorrectly evaluates to true.

This misconfigures the ADC and causes writes to touchscreen registers which
are undefined for this SoC (evaluating to offset 0x0), corrupting the ADC
Control Register (CR).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806074024.531259-1-varshini.rajendran@microchip.com?part=7