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

[Petar Stepanovic]

On 7/17/2026 10:34 AM, Andy Shevchenko wrote:
> ...
>
>> +struct axiado_saradc {
>> +     struct regmap *regmap;
>> +     struct clk *clk;
> Makes no sense to keep it here, your code takes the rate and uses that,
> I do not see how clk is being used right now. Perhaps you have plans
> for power management? But then add it when it's needed and being used.

Hi Andy, thanks for review.
Agreed. The clock is only used during probe to obtain its rate and calculate the conversion delay, so there is no need to keep it in the device structure. I will make it a local probe variable and remove `clk` from `struct axiado_saradc`.

>> +     struct mutex lock; /* Serializes ADC conversions. */
>> +     unsigned long clk_rate;
>> +     int vref_uV;
>> +};
>> +
>> +static const struct regmap_config axiado_saradc_regmap_config = {
>> +     .reg_bits = 32,
>> +     .val_bits = 32,
>> +     .reg_stride = 4,
>> +     .max_register = AX_SARADC_DOUT_REG,
> No cache?

No cache is intentional. The registers represent transient control, status, and conversion data, so accesses should always go directly to the hardware. Since REGCACHE_NONE is the default, it is not specified explicitly.

> ...
>
>> +     ret = regmap_read(info->regmap, AX_SARADC_DOUT_REG, &regval);
>> +
>> +     /* Stop manual conversion */
>> +     stop_ret = regmap_write(info->regmap, AX_SARADC_MANUAL_CTRL_REG, 0);
>> +
>> +     if (ret)
>> +             return ret;
>> +     if (stop_ret)
>> +             return stop_ret;
> Why do we care about stop error? Isn't it the best effort we can do?

Agreed. Stopping the manual conversion is a best-effort cleanup operation, and there is no useful recovery action if it fails. I will drop |stop_ret| and preserve only the result of regmap_read().

>
>> +     *val = regval & GENMASK(AX_RESOLUTION_BITS - 1, 0);
> Is device responding always in native endianess?

The SARADC registers are little-endian. `regmap_read()` returns the register value in CPU endianness, so applying the mask directly is correct. I will explicitly set `.val_format_endian = REGMAP_ENDIAN_LITTLE` in the regmap configuration to document the hardware endianness.

Best regards,
Petar