Re: [PATCH v6 2/2] iio: adc: add Axiado SARADC driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The conversion delay calculation does not wait long enough because it ignores the 16 hardware samples configured.
--

commit 266b1b7c1e54b1e96b8a907b30e2e13101402f9d
Author: Petar Stepanovic <pstepanovic@axiado.com>

iio: adc: add Axiado SARADC driver

This commit adds support for the SARADC controller found on Axiado AX3000
and AX3005 SoCs. The driver supports single-shot voltage reads through the
IIO subsystem.

> diff --git a/drivers/iio/adc/axiado_saradc.c b/drivers/iio/adc/axiado_saradc.c
> new file mode 100644
> index 0000000000000..699ee31616fcd
> --- /dev/null
> +++ b/drivers/iio/adc/axiado_saradc.c

[ ... ]

> +static int axiado_saradc_conversion(struct axiado_saradc *info,
> +				    struct iio_chan_spec const *chan, int *val)
> +{
> +	unsigned long usecs;
> +	unsigned int regval;
> +	int ret;
> +
> +	guard(mutex)(&info->lock);
> +
> +	/* Select the channel to be used and trigger conversion. */
> +	ret = regmap_write(info->regmap, AX_SARADC_MANUAL_CTRL_REG,
> +			   AX_SARADC_MANUAL_CTRL_ENABLE |
> +			   FIELD_PREP(AX_SARADC_MANUAL_CTRL_CH_SEL_MASK, chan->channel));
> +	if (ret)
> +		return ret;
> +
> +	/* Hardware requires 13 conversion cycles at clk_rate. */
> +	usecs = DIV_ROUND_UP(AX_SARADC_CONV_CYCLES * USEC_PER_SEC, info->clk_rate);
> +	fsleep(usecs + AX_SARADC_CONV_DELAY_MARGIN_US);

[Severity: High]
Will this delay wait long enough for the hardware to finish the conversion?

The delay calculation here only accounts for a single sample (13 conversion
cycles). However, looking at axiado_saradc_probe(), the hardware is
configured to average 16 samples by setting AX_SARADC_GLOBAL_CTRL_SAMPLE_16
in the global control register:

	regval = FIELD_PREP(AX_SARADC_GLOBAL_CTRL_CH_EN_MASK,
			    GENMASK(soc_data->num_channels - 1, 0)) |
		 FIELD_PREP(AX_SARADC_GLOBAL_CTRL_SAMPLE_MASK,
			    AX_SARADC_GLOBAL_CTRL_SAMPLE_16) |
		 ...

Since the hardware actually takes 16 samples (requiring 208 cycles total),
could reading AX_SARADC_DOUT_REG right after this shorter delay return stale
or incomplete data?

> +
> +	ret = regmap_read(info->regmap, AX_SARADC_DOUT_REG, &regval);
> +
> +	/* Best effort to stop manual conversion. */
> +	regmap_write(info->regmap, AX_SARADC_MANUAL_CTRL_REG, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-axiado-ax3000-ax3005-saradc-v6-0-4cea467e4167@axiado.com?part=2