Re: [PATCH v2 2/2] iio: adc: Add a driver for the Ingenic X1600 ADC

[Jonathan Cameron]

On Sat, 24 Oct 2026 06:13:57 +0200
Olivier Louvignes <olivier@xxxxxxxxx> wrote:

> The X1600's SAR ADC has four auxiliary channels, each with its own start
> bit, interrupt flag and 12-bit result, and none of the touchscreen or
> battery engines of the JZ47xx ADCs that ingenic-adc drives. Add a small
> driver that exposes the four channels as voltages against the 3.3 V
> reference.
> 
> There is no datasheet here: the register layout and the clock dividers
> come from the X1600 driver of the vendor's 4.4 kernel.
> 
> Tested on a HiBy R1, whose play/pause and volume keys sit on channel 0
> behind adc-keys: all three read and report correctly.
> 
> Signed-off-by: Olivier Louvignes <olivier@xxxxxxxxx>

Just one trivial things in addition to the other reviewer comments.

> diff --git a/drivers/iio/adc/ingenic-x1600-adc.c b/drivers/iio/adc/ingenic-x1600-adc.c
> new file mode 100644
> index 000000000000..60683f26c693
> --- /dev/null
> +++ b/drivers/iio/adc/ingenic-x1600-adc.c

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

> +	ret = devm_request_irq(dev, irq, x1600_adc_irq, 0, dev_name(dev), adc);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to request irq\n");
> +
> +	/*
> +	 * Powered up for good: the keys poll it every few tens of milliseconds,
> +	 * and powering up takes 2 ms.
The keys aren't tightly coupled to this driver so make that a little vaguer.

A typically usecase makes use of .... for which polling is every few...
Hence leave powered up.

> +	 */
> +	writel(0, adc->base + X1600_ADC_REG_ENABLE);
> +	fsleep(2000);
> +	writel(0, adc->base + X1600_ADC_REG_CTRL);
> +	ret = devm_add_action_or_reset(dev, x1600_adc_power_down, adc);
> +	if (ret)
> +		return ret;