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

[Petar Stepanovic <pstepanovic@axiado.com>]

On 6/5/2026 8:26 PM, Andy Shevchenko wrote:
>> +config AXIADO_SARADC
>> +     tristate "Axiado SARADC driver"
>> +     depends on ARCH_AXIADO || COMPILE_TEST
>> +     depends on OF
> No, in IIO we want a good justification on non-agnostic requirements.
> Why can't this device driver be agnostic?
>
Hi Andy,

Thanks for the review. I will address the comments in v2.

You are right. The driver does not have a hard dependency on OF.
It only uses the OF match table for Device Tree based probing, but the
driver itself can use generic device APIs and does not need to be limited
to OF-only systems.

I will drop the `depends on OF` dependency and use generic match-data
handling where applicable.

...

>> +     iowrite32(AX_SARADC_MANUAL_CTRL_EN(chan->channel),
>> +               info->regs + AX_SARADC_MANUAL_CTRL);
> Why not writel()?

There is no special reason to use iowrite32() here. This is a normal
MMIO resource, so I will switch the driver to readl()/writel().

...

>> +static int axiado_saradc_probe(struct platform_device *pdev)
>> +{
>> +     struct axiado_saradc *info;
>> +     const struct axiado_saradc_soc_data *soc_data;
>> +     struct iio_dev *indio_dev;
>> +     int ret;
>> +     u32 reg;
>> +
>> +     indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
>> +     if (!indio_dev)
>> +             return -ENOMEM;
>> +
>> +     info = iio_priv(indio_dev);
>> +
>> +     info->regs = devm_platform_ioremap_resource(pdev, 0);
>> +     if (IS_ERR(info->regs))
>> +             return PTR_ERR(info->regs);
>> +
>> +     info->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> Why no name? It will make harder for the next generations of HW in case they
> want more than one clock to be used.

This IP block currently has only one clock input, so I used the unnamed clock.
Since there is no ambiguity for the current hardware, I would prefer to keep
the binding simple unless a named clock is required.

...

>> +     reg = FIELD_PREP(AX_SARADC_CH_EN_MASK,
>> +                      GENMASK(soc_data->num_channels - 1, 0)) |
>> +           AX_SARADC_SAMPLE_16 | AX_SARADC_MODE | AX_SARADC_ENABLE;
> FIELD_PREP_CONST() ?

FIELD_PREP_CONST() cannot be used here because the channel-enable value is
derived from soc_data->num_channels, which is runtime match data.


Best regards,
Petar