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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The SARADC hardware block is enabled during probe but is never explicitly disabled on driver unbind or probe failure.
- [Medium] The power-down (PD) bit is toggled with no delay, resulting in an ineffective reset or power cycle pulse.
- [Medium] The IIO device name is initialized with `dev_name()`, which violates the IIO sysfs ABI.
--

commit 14677b4fe42ae6002d14ca13458223da2d275626
Author: Petar Stepanovic <pstepanovic@axiado.com>

iio: adc: add Axiado SARADC driver

Add support for the SARADC controller found on Axiado AX3000 and AX3005 SoCs.
The driver supports single-shot voltage reads through the IIO subsystem,
dynamically selecting the number of available channels from SoC match data.

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

[ ... ]

> +static int axiado_saradc_probe(struct platform_device *pdev)
> +{

[ ... ]

> +	mutex_init(&info->lock);
> +	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;
> +
> +	iowrite32(AX_SARADC_PD, info->regs + AX_SARADC_GLOBAL_CTRL);
> +	iowrite32(reg, info->regs + AX_SARADC_GLOBAL_CTRL);

[Severity: Medium]
Is this reset or power cycle sequence effective without a delay?

The power-down (PD) bit is toggled with no delay between writes, and without
a read-back to flush the write buffer. This typically results in a
zero-width pulse that hardware might not register.

Does the hardware require a short sleep or a dummy read here to ensure the
analog components properly reset?

> +
> +	indio_dev->name = dev_name(&pdev->dev);

[Severity: Medium]
Will this name format break the IIO sysfs ABI?

The driver initializes the IIO device name using dev_name(). For device tree
nodes, this usually includes the bus address and a dot, such as
12340000.saradc.

The IIO sysfs ABI dictates that the device name attribute must not contain
whitespace, dashes, or dots. Could a hardcoded driver string or the
compatible string be used instead to avoid breaking userspace applications?

> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->info = &axiado_saradc_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = axiado_saradc_iio_channels;
> +	indio_dev->num_channels = soc_data->num_channels;
> +
> +	ret = devm_iio_device_register(&pdev->dev, indio_dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "failed to register IIO device\n");
> +
> +	return 0;
> +}

[Severity: Medium]
Is there a missing cleanup step to disable the SARADC hardware block?

During probe, the hardware is enabled by writing AX_SARADC_ENABLE to the
global control register. However, the driver does not seem to register a
devm action or provide a remove callback to clear this bit and power down
the ADC.

When the driver unbinds or probe fails, standard devres will gate the clock
and disable the regulator while the ADC block is still logically active.
Does disabling clocks on active hardware lead to system bus lockups or power
leaks on these SoCs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-axiado-ax3000-ax3005-saradc-v1-0-345dd5f6608a@axiado.com?part=2