[PATCH 0/3] Subject: [PATCH 0/3] iio: adc: Add Axiado SARADC driver

[PATCH 0/3] Subject: [PATCH 0/3] iio: adc: Add Axiado SARADC driver

[Petar Stepanovic]

This series adds support for the SAR ADC controller found on Axiado
AX3000 and AX3005 SoCs.

The controller is a 10-bit ADC. AX3000 has sixteen input channels and
AX3005 has eight input channels. The driver uses SoC match data to
select the number of available channels for each compatible.

The driver supports single-shot voltage reads through the IIO subsystem
and uses the reference voltage regulator for scale calculation.

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
---
Petar Stepanovic (3):
      dt-bindings: iio: adc: add Axiado AX3000/AX3005 SARADC
      iio: adc: add Axiado SARADC driver
      MAINTAINERS: add Axiado SARADC driver entry

 .../bindings/iio/adc/axiado,ax3000-saradc.yaml     |  58 ++++++
 MAINTAINERS                                        |   8 +
 drivers/iio/adc/Kconfig                            |  11 ++
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/axiado_saradc.c                    | 218 +++++++++++++++++++++
 5 files changed, 296 insertions(+)
---
base-commit: 51f0c0b8545b23963afd5d43a8f56ee05bfa54da
change-id: 20260508-axiado-ax3000-ax3005-saradc-151aed5d25da

Best regards,
-- 
Petar Stepanovic <pstepanovic@axiado.com>

[PATCH 1/3] dt-bindings: iio: adc: add Axiado AX3000/AX3005 SARADC

[Petar Stepanovic]

The Axiado AX3000 and AX3005 SoCs include a 10-bit SAR ADC controller.
AX3000 supports 16 input channels, while AX3005 supports 8 input
channels.

Document the compatible strings, register region, clock, reference
voltage supply, and IIO channel cells.

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
---
 .../bindings/iio/adc/axiado,ax3000-saradc.yaml     | 58 ++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/axiado,ax3000-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/axiado,ax3000-saradc.yaml
new file mode 100644
index 000000000000..54592353a7b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/axiado,ax3000-saradc.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/axiado,ax3000-saradc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Axiado AX3000/AX3005 Successive Approximation Register ADC
+
+description:
+  The Axiado AX3000/AX3005 SAR ADC is a 10-bit ADC with sixteen input
+  channels on AX3000 and eight input channels on AX3005.
+
+maintainers:
+  - Petar Stepanovic <pstepanovic@axiado.com>
+  - Akhila Kavi <akavi@axiado.com>
+  - Prasad Bolisetty <pbolisetty@axiado.com>
+
+properties:
+  compatible:
+    enum:
+      - axiado,ax3000-saradc
+      - axiado,ax3005-saradc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  '#io-channel-cells':
+    const: 1
+
+  vref-supply:
+    description: Reference voltage regulator supplying the ADC
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - '#io-channel-cells'
+  - vref-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      saradc@806a0000 {
+        compatible = "axiado,ax3000-saradc";
+        reg = <0x0 0x806a0000 0x0 0x400>;
+        clocks = <&pclk>;
+        vref-supply = <&vref_reg>;
+        #io-channel-cells = <1>;
+      };
+    };

-- 
2.34.1

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

[Petar Stepanovic]

Add support for the SARADC controller found on Axiado AX3000 and
AX3005 SoCs.

The driver supports single-shot voltage reads through the IIO
subsystem. The number of available input channels is selected from
the SoC match data, allowing AX3000 and AX3005 variants to use the
same driver.

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
---
 drivers/iio/adc/Kconfig         |  11 ++
 drivers/iio/adc/Makefile        |   1 +
 drivers/iio/adc/axiado_saradc.c | 218 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 230 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a9dedbb8eb46..a35bba46beb0 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -631,6 +631,17 @@ config AT91_SAMA5D2_ADC
 	  To compile this driver as a module, choose M here: the module will be
 	  called at91-sama5d2_adc.
 
+config AXIADO_SARADC
+	tristate "Axiado SARADC driver"
+	depends on ARCH_AXIADO || COMPILE_TEST
+	depends on OF
+	help
+	  Say yes here to build support for the SARADC found in Axiado
+	  SoCs.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called axiado_saradc.
+
 config AXP20X_ADC
 	tristate "X-Powers AXP20X and AXP22X ADC driver"
 	depends on MFD_AXP20X
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 097357d146ba..96de0ce1d90a 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_ADI_AXI_ADC) += adi-axi-adc.o
 obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
 obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
+obj-$(CONFIG_AXIADO_SARADC) += axiado_saradc.o
 obj-$(CONFIG_AXP20X_ADC) += axp20x_adc.o
 obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
 obj-$(CONFIG_BCM_IPROC_ADC) += bcm_iproc_adc.o
diff --git a/drivers/iio/adc/axiado_saradc.c b/drivers/iio/adc/axiado_saradc.c
new file mode 100644
index 000000000000..5ca711542a14
--- /dev/null
+++ b/drivers/iio/adc/axiado_saradc.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021-2026 Axiado Corporation
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/iio/iio.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+
+/* Register offsets */
+#define AX_SARADC_GLOBAL_CTRL 0x0004
+#define AX_SARADC_MANUAL_CTRL 0x0008
+#define AX_SARADC_DOUT 0x001C
+
+/* GLOBAL_CTRL fields */
+#define AX_SARADC_CH_EN_MASK GENMASK(31, 16)
+#define AX_SARADC_SAMPLE_MASK GENMASK(6, 5)
+#define AX_SARADC_MODE_MASK GENMASK(4, 3)
+#define AX_SARADC_PD BIT(2)
+#define AX_SARADC_ENABLE BIT(0)
+
+/* GLOBAL_CTRL values */
+#define AX_SARADC_SAMPLE_16 FIELD_PREP(AX_SARADC_SAMPLE_MASK, 0)
+#define AX_SARADC_MODE FIELD_PREP(AX_SARADC_MODE_MASK, 1)
+
+#define AX_SARADC_MANUAL_CTRL_EN(n) (BIT(0) | ((n) << 1))
+#define AX_RESOLUTION_BITS 10
+#define AX_SARADC_CONV_CYCLES 13
+
+struct axiado_saradc {
+	void __iomem *regs;
+	struct clk *clk;
+	unsigned long clk_rate;
+	int vref_uv;
+	struct mutex lock; /* Serializes ADC conversions. */
+};
+
+static int axiado_saradc_conversion(struct axiado_saradc *info,
+				    struct iio_chan_spec const *chan, int *val)
+{
+	unsigned long usecs;
+	/* Select the channel to be used and trigger conversion */
+	iowrite32(AX_SARADC_MANUAL_CTRL_EN(chan->channel),
+		  info->regs + AX_SARADC_MANUAL_CTRL);
+
+	/* Hardware requires 13 conversion cycles at clk_rate */
+	usecs = DIV_ROUND_UP(AX_SARADC_CONV_CYCLES * 1000000, info->clk_rate);
+	usleep_range(usecs, usecs + 10);
+
+	*val = ioread32(info->regs + AX_SARADC_DOUT) &
+	       GENMASK(AX_RESOLUTION_BITS - 1, 0);
+
+	/* Stop manual conversion */
+	iowrite32(0, info->regs + AX_SARADC_MANUAL_CTRL);
+	return 0;
+}
+
+static int axiado_saradc_read_raw(struct iio_dev *indio_dev,
+				  struct iio_chan_spec const *chan, int *val,
+				  int *val2, long mask)
+{
+	struct axiado_saradc *info = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&info->lock);
+		ret = axiado_saradc_conversion(info, chan, val);
+		mutex_unlock(&info->lock);
+		return ret ? ret : IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = info->vref_uv / 1000;
+		*val2 = AX_RESOLUTION_BITS;
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info axiado_saradc_iio_info = {
+	.read_raw = axiado_saradc_read_raw,
+};
+
+struct axiado_saradc_soc_data {
+	unsigned int num_channels;
+};
+
+static const struct axiado_saradc_soc_data ax3000_saradc_data = {
+	.num_channels = 16,
+};
+
+static const struct axiado_saradc_soc_data ax3005_saradc_data = {
+	.num_channels = 8,
+};
+
+#define AX_SARADC_CH(_index, _id)                                       \
+	{                                                               \
+		.type = IIO_VOLTAGE, .indexed = 1, .channel = (_index), \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
+		.datasheet_name = (_id), .scan_index = -1,              \
+		.scan_type = {                                          \
+			.sign = 'u',                                    \
+			.realbits = AX_RESOLUTION_BITS,                 \
+			.storagebits = 16,                              \
+			.endianness = IIO_CPU,                          \
+		},                                                      \
+	}
+
+static const struct iio_chan_spec axiado_saradc_iio_channels[] = {
+	AX_SARADC_CH(0, "adc0"),   AX_SARADC_CH(1, "adc1"),
+	AX_SARADC_CH(2, "adc2"),   AX_SARADC_CH(3, "adc3"),
+	AX_SARADC_CH(4, "adc4"),   AX_SARADC_CH(5, "adc5"),
+	AX_SARADC_CH(6, "adc6"),   AX_SARADC_CH(7, "adc7"),
+	AX_SARADC_CH(8, "adc8"),   AX_SARADC_CH(9, "adc9"),
+	AX_SARADC_CH(10, "adc10"), AX_SARADC_CH(11, "adc11"),
+	AX_SARADC_CH(12, "adc12"), AX_SARADC_CH(13, "adc13"),
+	AX_SARADC_CH(14, "adc14"), AX_SARADC_CH(15, "adc15"),
+};
+
+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);
+	if (IS_ERR(info->clk))
+		return PTR_ERR(info->clk);
+
+	info->clk_rate = clk_get_rate(info->clk);
+	if (!info->clk_rate)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "invalid clock rate\n");
+
+	info->vref_uv = devm_regulator_get_enable_read_voltage(&pdev->dev,
+							       "vref");
+	if (info->vref_uv < 0)
+		return dev_err_probe(&pdev->dev, info->vref_uv,
+				     "failed to get vref voltage\n");
+
+	soc_data = device_get_match_data(&pdev->dev);
+	if (!soc_data)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "failed to get match data\n");
+
+	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);
+
+	indio_dev->name = dev_name(&pdev->dev);
+	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;
+}
+
+static const struct of_device_id axiado_saradc_match[] = {
+	{
+		.compatible = "axiado,ax3000-saradc",
+		.data = &ax3000_saradc_data,
+	},
+	{
+		.compatible = "axiado,ax3005-saradc",
+		.data = &ax3005_saradc_data,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, axiado_saradc_match);
+
+static struct platform_driver axiado_saradc_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = axiado_saradc_match,
+	},
+	.probe = axiado_saradc_probe,
+};
+
+module_platform_driver(axiado_saradc_driver);
+
+MODULE_AUTHOR("AXIADO CORPORATION");
+MODULE_DESCRIPTION("AXIADO SARADC driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

[PATCH 3/3] MAINTAINERS: add Axiado SARADC driver entry

[Petar Stepanovic]

Add a MAINTAINERS entry for the Axiado SARADC binding and driver.

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b2040011a386..e6dadfa65ee0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4312,6 +4312,14 @@ S:	Orphan
 F:	Documentation/devicetree/bindings/sound/axentia,*
 F:	sound/soc/atmel/tse850-pcm5142.c
 
+AXIADO SARADC DRIVER
+M:	Petar Stepanovic <pstepanovic@axiado.com>
+M:	Akhila Kavi <akavi@axiado.com>
+M:	Prasad Bolisetty <pbolisetty@axiado.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/adc/axiado,ax3000-saradc.yaml
+F:	drivers/iio/adc/axiado_saradc.c
+
 AXIS ARTPEC ARM64 SoC SUPPORT
 M:	Jesper Nilsson <jesper.nilsson@axis.com>
 M:	Lars Persson <lars.persson@axis.com>

-- 
2.34.1

Re: [PATCH 3/3] MAINTAINERS: add Axiado SARADC driver entry

[Joshua Crofts]

On Thu, 28 May 2026 at 10:21, Petar Stepanovic <pstepanovic@axiado.com> wrote:
>
> Add a MAINTAINERS entry for the Axiado SARADC binding and driver.
>
> Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b2040011a386..e6dadfa65ee0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4312,6 +4312,14 @@ S:       Orphan
>  F:     Documentation/devicetree/bindings/sound/axentia,*
>  F:     sound/soc/atmel/tse850-pcm5142.c
>
> +AXIADO SARADC DRIVER
> +M:     Petar Stepanovic <pstepanovic@axiado.com>
> +M:     Akhila Kavi <akavi@axiado.com>
> +M:     Prasad Bolisetty <pbolisetty@axiado.com>
> +S:     Maintained
> +F:     Documentation/devicetree/bindings/iio/adc/axiado,ax3000-saradc.yaml

Fold the lines above into the dt-bindings patch. You're adding a new
file, so it's best
to have any changes to MAINTAINERS in the same patch so that it's already
documented when the patch is accepted.

> +F:     drivers/iio/adc/axiado_saradc.c

This line should be added in the patch with the actual driver code.

-- 
Kind regards

CJD

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

[sashiko-bot]

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

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

[Joshua Crofts]

On Thu, 28 May 2026 at 10:11, Petar Stepanovic <pstepanovic@axiado.com> wrote:
>
> Add support for the SARADC controller found on Axiado AX3000 and
> AX3005 SoCs.
>
> The driver supports single-shot voltage reads through the IIO
> subsystem. The number of available input channels is selected from
> the SoC match data, allowing AX3000 and AX3005 variants to use the
> same driver.
>
> Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
> ---

Hi Petar, a few comments inline. Additionally, Sashiko came back
with a few issues:
https://sashiko.dev/#/patchset/20260528-axiado-ax3000-ax3005-saradc-v1-0-345dd5f6608a%40axiado.com.

> +++ b/drivers/iio/adc/axiado_saradc.c
> @@ -0,0 +1,218 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2021-2026 Axiado Corporation
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/iio/iio.h>

IIO specific headers should follow the generic <linux/*> headers and
be grouped separately.

> +#include <linux/io.h>
> +#include <linux/kernel.h>

Don't use kernel.h when adding a new driver. The existing drivers in IIO
are currently being moved away from kernel.h, as it is a catch-all header.
You should include what you use manually (there is a tool for this, it's
called iwyu-tool).

> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +
> +/* Register offsets */
> +#define AX_SARADC_GLOBAL_CTRL 0x0004
> +#define AX_SARADC_MANUAL_CTRL 0x0008
> +#define AX_SARADC_DOUT 0x001C
> +
> +/* GLOBAL_CTRL fields */
> +#define AX_SARADC_CH_EN_MASK GENMASK(31, 16)
> +#define AX_SARADC_SAMPLE_MASK GENMASK(6, 5)
> +#define AX_SARADC_MODE_MASK GENMASK(4, 3)
> +#define AX_SARADC_PD BIT(2)
> +#define AX_SARADC_ENABLE BIT(0)
> +
> +/* GLOBAL_CTRL values */
> +#define AX_SARADC_SAMPLE_16 FIELD_PREP(AX_SARADC_SAMPLE_MASK, 0)
> +#define AX_SARADC_MODE FIELD_PREP(AX_SARADC_MODE_MASK, 1)
> +
> +#define AX_SARADC_MANUAL_CTRL_EN(n) (BIT(0) | ((n) << 1))
> +#define AX_RESOLUTION_BITS 10
> +#define AX_SARADC_CONV_CYCLES 13
> +
> +struct axiado_saradc {
> +       void __iomem *regs;
> +       struct clk *clk;
> +       unsigned long clk_rate;
> +       int vref_uv;
> +       struct mutex lock; /* Serializes ADC conversions. */
> +};
> +
> +static int axiado_saradc_conversion(struct axiado_saradc *info,
> +                                   struct iio_chan_spec const *chan, int *val)
> +{
> +       unsigned long usecs;
> +       /* Select the channel to be used and trigger conversion */
> +       iowrite32(AX_SARADC_MANUAL_CTRL_EN(chan->channel),
> +                 info->regs + AX_SARADC_MANUAL_CTRL);
> +
> +       /* Hardware requires 13 conversion cycles at clk_rate */
> +       usecs = DIV_ROUND_UP(AX_SARADC_CONV_CYCLES * 1000000, info->clk_rate);
> +       usleep_range(usecs, usecs + 10);

fsleep() is preferred over usleep_range(), it selects the optimal
sleep function while guaranteeing a sleep of at least usecs time.

> +
> +       *val = ioread32(info->regs + AX_SARADC_DOUT) &
> +              GENMASK(AX_RESOLUTION_BITS - 1, 0);
> +
> +       /* Stop manual conversion */
> +       iowrite32(0, info->regs + AX_SARADC_MANUAL_CTRL);
> +       return 0;
> +}
> +
> +static int axiado_saradc_read_raw(struct iio_dev *indio_dev,
> +                                 struct iio_chan_spec const *chan, int *val,

Small nit, but I'd move int * val onto the line along with val2 and mask
so it's logically separated.

> +                                 int *val2, long mask)
> +{
> +       struct axiado_saradc *info = iio_priv(indio_dev);
> +       int ret;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_RAW:
> +               mutex_lock(&info->lock);

I wanted to recommend using the guard(mutex) macro, but you're
only using a mutex once, so up to you.

> +               ret = axiado_saradc_conversion(info, chan, val);
> +               mutex_unlock(&info->lock);
> +               return ret ? ret : IIO_VAL_INT;
> +       case IIO_CHAN_INFO_SCALE:
> +               *val = info->vref_uv / 1000;
> +               *val2 = AX_RESOLUTION_BITS;
> +               return IIO_VAL_FRACTIONAL_LOG2;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static const struct iio_info axiado_saradc_iio_info = {
> +       .read_raw = axiado_saradc_read_raw,
> +};
> +
> +struct axiado_saradc_soc_data {
> +       unsigned int num_channels;
> +};
> +
> +static const struct axiado_saradc_soc_data ax3000_saradc_data = {
> +       .num_channels = 16,
> +};
> +
> +static const struct axiado_saradc_soc_data ax3005_saradc_data = {
> +       .num_channels = 8,
> +};
> +
> +#define AX_SARADC_CH(_index, _id)                                       \
> +       {                                                               \
> +               .type = IIO_VOLTAGE, .indexed = 1, .channel = (_index), \
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
> +               .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
> +               .datasheet_name = (_id), .scan_index = -1,              \
> +               .scan_type = {                                          \
> +                       .sign = 'u',                                    \
> +                       .realbits = AX_RESOLUTION_BITS,                 \
> +                       .storagebits = 16,                              \
> +                       .endianness = IIO_CPU,                          \
> +               },                                                      \
> +       }
> +
> +static const struct iio_chan_spec axiado_saradc_iio_channels[] = {
> +       AX_SARADC_CH(0, "adc0"),   AX_SARADC_CH(1, "adc1"),
> +       AX_SARADC_CH(2, "adc2"),   AX_SARADC_CH(3, "adc3"),
> +       AX_SARADC_CH(4, "adc4"),   AX_SARADC_CH(5, "adc5"),
> +       AX_SARADC_CH(6, "adc6"),   AX_SARADC_CH(7, "adc7"),
> +       AX_SARADC_CH(8, "adc8"),   AX_SARADC_CH(9, "adc9"),
> +       AX_SARADC_CH(10, "adc10"), AX_SARADC_CH(11, "adc11"),
> +       AX_SARADC_CH(12, "adc12"), AX_SARADC_CH(13, "adc13"),
> +       AX_SARADC_CH(14, "adc14"), AX_SARADC_CH(15, "adc15"),
> +};
> +
> +static int axiado_saradc_probe(struct platform_device *pdev)
> +{
> +       struct axiado_saradc *info;
> +       const struct axiado_saradc_soc_data *soc_data;

Use reverse christmas tree order.

> +       struct iio_dev *indio_dev;
> +       int ret;
> +       u32 reg;

Reg isn't a really good name, regval would be better.

> +
> +       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);
> +       if (IS_ERR(info->clk))
> +               return PTR_ERR(info->clk);
> +
> +       info->clk_rate = clk_get_rate(info->clk);
> +       if (!info->clk_rate)
> +               return dev_err_probe(&pdev->dev, -EINVAL,
> +                                    "invalid clock rate\n");
> +
> +       info->vref_uv = devm_regulator_get_enable_read_voltage(&pdev->dev,
> +                                                              "vref");
> +       if (info->vref_uv < 0)
> +               return dev_err_probe(&pdev->dev, info->vref_uv,
> +                                    "failed to get vref voltage\n");
> +
> +       soc_data = device_get_match_data(&pdev->dev);
> +       if (!soc_data)
> +               return dev_err_probe(&pdev->dev, -EINVAL,
> +                                    "failed to get match data\n");
> +
> +       mutex_init(&info->lock);

Use devm_mutex_init since everything else is devm.

> +       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);

Sashiko notes to add a small delay here to prevent a zero width pulse.

> +       iowrite32(reg, info->regs + AX_SARADC_GLOBAL_CTRL);
> +       indio_dev->name = dev_name(&pdev->dev);

Hmm, Sashiko may have a point here, this could break the ABI.
The name member is usually assigned a hardcoded string and
it shouldn't contain whitespace, dots or dashes, which dev_name()
could return.

> +       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");

You can just do `return devm_iio_device_register`.

> +       return 0;
> +}
> +

I don't see any kind of cleanup procedure, like a remove() function
or devm_add_action_or_reset callback, is this intentional?

> +static const struct of_device_id axiado_saradc_match[] = {
> +       {
> +               .compatible = "axiado,ax3000-saradc",
> +               .data = &ax3000_saradc_data,
> +       },
> +       {
> +               .compatible = "axiado,ax3005-saradc",
> +               .data = &ax3005_saradc_data,
> +       },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, axiado_saradc_match);
> +
> +static struct platform_driver axiado_saradc_driver = {
> +       .driver = {
> +               .name = KBUILD_MODNAME,
> +               .of_match_table = axiado_saradc_match,
> +       },
> +       .probe = axiado_saradc_probe,
> +};
> +
> +module_platform_driver(axiado_saradc_driver);
> +
> +MODULE_AUTHOR("AXIADO CORPORATION");
> +MODULE_DESCRIPTION("AXIADO SARADC driver");
> +MODULE_LICENSE("GPL");
>
> --
> 2.34.1
>
>

-- 
Kind regards

CJD

Re: [PATCH 0/3] Subject: [PATCH 0/3] iio: adc: Add Axiado SARADC driver

[Jonathan Cameron]

On Thu, 28 May 2026 01:10:22 -0700
Petar Stepanovic <pstepanovic@axiado.com> wrote:

> This series adds support for the SAR ADC controller found on Axiado
> AX3000 and AX3005 SoCs.
> 
> The controller is a 10-bit ADC. AX3000 has sixteen input channels and
> AX3005 has eight input channels. The driver uses SoC match data to
> select the number of available channels for each compatible.
> 
> The driver supports single-shot voltage reads through the IIO subsystem
> and uses the reference voltage regulator for scale calculation.
> 
> Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
Hi Petar

Welcome to the IIO subsystem.

I guess you already noticed, but something went wrong with your
patch title.

If this is an issue with a company email system or similar, consider
using the b4 tool and the web gateway that works with as it avoids
any corruption of threads or similar.

Thanks,

Jonathan

> ---
> Petar Stepanovic (3):
>       dt-bindings: iio: adc: add Axiado AX3000/AX3005 SARADC
>       iio: adc: add Axiado SARADC driver
>       MAINTAINERS: add Axiado SARADC driver entry
> 
>  .../bindings/iio/adc/axiado,ax3000-saradc.yaml     |  58 ++++++
>  MAINTAINERS                                        |   8 +
>  drivers/iio/adc/Kconfig                            |  11 ++
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/axiado_saradc.c                    | 218 +++++++++++++++++++++
>  5 files changed, 296 insertions(+)
> ---
> base-commit: 51f0c0b8545b23963afd5d43a8f56ee05bfa54da
> change-id: 20260508-axiado-ax3000-ax3005-saradc-151aed5d25da
> 
> Best regards,

Re: [PATCH 1/3] dt-bindings: iio: adc: add Axiado AX3000/AX3005 SARADC

[Jonathan Cameron]

On Thu, 28 May 2026 01:10:23 -0700
Petar Stepanovic <pstepanovic@axiado.com> wrote:

> The Axiado AX3000 and AX3005 SoCs include a 10-bit SAR ADC controller.
> AX3000 supports 16 input channels, while AX3005 supports 8 input
> channels.
> 
> Document the compatible strings, register region, clock, reference
> voltage supply, and IIO channel cells.
> 
> Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
Hi Petar,

One minor thing inline. Otherwise looks good to me.

Thanks,

Jonathan

> ---
>  .../bindings/iio/adc/axiado,ax3000-saradc.yaml     | 58 ++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/axiado,ax3000-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/axiado,ax3000-saradc.yaml
> new file mode 100644
> index 000000000000..54592353a7b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/axiado,ax3000-saradc.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/axiado,ax3000-saradc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Axiado AX3000/AX3005 Successive Approximation Register ADC
> +
> +description:
> +  The Axiado AX3000/AX3005 SAR ADC is a 10-bit ADC with sixteen input
> +  channels on AX3000 and eight input channels on AX3005.
> +
> +maintainers:
> +  - Petar Stepanovic <pstepanovic@axiado.com>
> +  - Akhila Kavi <akavi@axiado.com>
> +  - Prasad Bolisetty <pbolisetty@axiado.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - axiado,ax3000-saradc
> +      - axiado,ax3005-saradc
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  '#io-channel-cells':
> +    const: 1
> +
> +  vref-supply:
> +    description: Reference voltage regulator supplying the ADC
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - '#io-channel-cells'
> +  - vref-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      saradc@806a0000 {

Generic names for nodes in DT. So just adc@806a0000

> +        compatible = "axiado,ax3000-saradc";
> +        reg = <0x0 0x806a0000 0x0 0x400>;
> +        clocks = <&pclk>;
> +        vref-supply = <&vref_reg>;
> +        #io-channel-cells = <1>;
> +      };
> +    };
> 

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

[Jonathan Cameron]

On Thu, 28 May 2026 11:02:05 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Thu, 28 May 2026 at 10:11, Petar Stepanovic <pstepanovic@axiado.com> wrote:
> >
> > Add support for the SARADC controller found on Axiado AX3000 and
> > AX3005 SoCs.
> >
> > The driver supports single-shot voltage reads through the IIO
> > subsystem. The number of available input channels is selected from
> > the SoC match data, allowing AX3000 and AX3005 variants to use the
> > same driver.
> >
> > Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
> > ---  
> 
> Hi Petar, a few comments inline. Additionally, Sashiko came back
> with a few issues:
> https://sashiko.dev/#/patchset/20260528-axiado-ax3000-ax3005-saradc-v1-0-345dd5f6608a%40axiado.com.

I'll review on top given it's a short driver so Joshua hasn't cropped anything.

Mind you seems like Joshua's email client has replaced tabs with spaces
which briefly made me wonder if the patches were corrupted!

Joshua caught most of the larger things, so mostly formatting and similar
from me.

Note that convention is to leave a new series on list for around a week before
posting an update. That lets more reviewers have the opportunity to take a look
and cuts down on how many revisions we end up with.
I'd soften that a little in this case as it is a relative small driver but wait
until next week before sending a v2.

Jonathan

> 
> > +++ b/drivers/iio/adc/axiado_saradc.c
> > @@ -0,0 +1,218 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2021-2026 Axiado Corporation
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/iio/iio.h>  
> 
> IIO specific headers should follow the generic <linux/*> headers and
> be grouped separately.
> 
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>  
> 
> Don't use kernel.h when adding a new driver. The existing drivers in IIO
> are currently being moved away from kernel.h, as it is a catch-all header.
> You should include what you use manually (there is a tool for this, it's
> called iwyu-tool).
> 
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +/* Register offsets */
If you name them to imply they are registers then it will
be easier to spot if they end up in the wrong place in calls.
AX_SARADC_GLOBAL_CTRL_REG
etc

> > +#define AX_SARADC_GLOBAL_CTRL 0x0004
> > +#define AX_SARADC_MANUAL_CTRL 0x0008
> > +#define AX_SARADC_DOUT 0x001C
> > +
> > +/* GLOBAL_CTRL fields */
Include enough of the register in the name to ensure that
is obvious.

AX_SARADC_GLOBAL_CTRL_CH_EN_MASK
etc

> > +#define AX_SARADC_CH_EN_MASK GENMASK(31, 16)
> > +#define AX_SARADC_SAMPLE_MASK GENMASK(6, 5)
> > +#define AX_SARADC_MODE_MASK GENMASK(4, 3)
> > +#define AX_SARADC_PD BIT(2)
> > +#define AX_SARADC_ENABLE BIT(0)
> > +
> > +/* GLOBAL_CTRL values */
> > +#define AX_SARADC_SAMPLE_16 FIELD_PREP(AX_SARADC_SAMPLE_MASK, 0)
> > +#define AX_SARADC_MODE FIELD_PREP(AX_SARADC_MODE_MASK, 1)
> > +
> > +#define AX_SARADC_MANUAL_CTRL_EN(n) (BIT(0) | ((n) << 1))

Add a define for that BIT(0) to give us a little more info
on what it is. Also a mask for that field you are writing (n) into
and use FIELD_PREP() for that rather than a shift.
Doing all that will be a little more code, but will act as documentation
of what is int eh register.


> > +#define AX_RESOLUTION_BITS 10
> > +#define AX_SARADC_CONV_CYCLES 13
> > +
> > +struct axiado_saradc {
> > +       void __iomem *regs;
> > +       struct clk *clk;
> > +       unsigned long clk_rate;
> > +       int vref_uv;
> > +       struct mutex lock; /* Serializes ADC conversions. */
> > +};
> > +
> > +static int axiado_saradc_conversion(struct axiado_saradc *info,
> > +                                   struct iio_chan_spec const *chan, int *val)
> > +{
> > +       unsigned long usecs;

Blank line between declarations and code.

As below I'd have

	guard(mutex)(&info->lock);

in here rather than at caller.  Simpler scope and makes the association with
adc conversion more obvious.


> > +       /* Select the channel to be used and trigger conversion */
> > +       iowrite32(AX_SARADC_MANUAL_CTRL_EN(chan->channel),
> > +                 info->regs + AX_SARADC_MANUAL_CTRL);
> > +
> > +       /* Hardware requires 13 conversion cycles at clk_rate */
> > +       usecs = DIV_ROUND_UP(AX_SARADC_CONV_CYCLES * 1000000, info->clk_rate);
> > +       usleep_range(usecs, usecs + 10);  
> 
> fsleep() is preferred over usleep_range(), it selects the optimal
> sleep function while guaranteeing a sleep of at least usecs time.
> 
> > +
> > +       *val = ioread32(info->regs + AX_SARADC_DOUT) &
> > +              GENMASK(AX_RESOLUTION_BITS - 1, 0);
> > +
> > +       /* Stop manual conversion */
> > +       iowrite32(0, info->regs + AX_SARADC_MANUAL_CTRL);

Blank line here.  Always nice to separate 'simple returns' like this
so the eye sees them more easily when reading code.

> > +       return 0;
> > +}
> > +
> > +static int axiado_saradc_read_raw(struct iio_dev *indio_dev,
> > +                                 struct iio_chan_spec const *chan, int *val,  
> 
> Small nit, but I'd move int * val onto the line along with val2 and mask
> so it's logically separated.
> 
> > +                                 int *val2, long mask)
> > +{
> > +       struct axiado_saradc *info = iio_priv(indio_dev);
> > +       int ret;
> > +
> > +       switch (mask) {
> > +       case IIO_CHAN_INFO_RAW:
> > +               mutex_lock(&info->lock);  
> 
> I wanted to recommend using the guard(mutex) macro, but you're
> only using a mutex once, so up to you.

I'd move it into the axiado_saradc_conversion();
+ use guard() in there as that already has nicely defined scope.

> 
> > +               ret = axiado_saradc_conversion(info, chan, val);
> > +               mutex_unlock(&info->lock);
> > +               return ret ? ret : IIO_VAL_INT;
> > +       case IIO_CHAN_INFO_SCALE:
> > +               *val = info->vref_uv / 1000;
> > +               *val2 = AX_RESOLUTION_BITS;
> > +               return IIO_VAL_FRACTIONAL_LOG2;
> > +
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +}
> > +
> > +static const struct iio_info axiado_saradc_iio_info = {
> > +       .read_raw = axiado_saradc_read_raw,
> > +};
> > +
> > +struct axiado_saradc_soc_data {
> > +       unsigned int num_channels;
> > +};
> > +
> > +static const struct axiado_saradc_soc_data ax3000_saradc_data = {
> > +       .num_channels = 16,
> > +};
> > +
> > +static const struct axiado_saradc_soc_data ax3005_saradc_data = {
> > +       .num_channels = 8,
As below, add a name to these so that we can report the part number
via sysfs name attribute. Something like "ax3005_saradc"
> > +};
> > +
> > +#define AX_SARADC_CH(_index, _id)                                       \
> > +       {                                                               \
> > +               .type = IIO_VOLTAGE, .indexed = 1, .channel = (_index), \

Go with 1 per line as it will be a bit longer but easier to read.

> > +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),           \
> > +               .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
> > +               .datasheet_name = (_id), .scan_index = -1,              \
> > +               .scan_type = {                                          \
> > +                       .sign = 'u',                                    \
> > +                       .realbits = AX_RESOLUTION_BITS,                 \
> > +                       .storagebits = 16,                              \
> > +                       .endianness = IIO_CPU,                          \

You aren't yet doing buffered capture so don't introduce scan_type yet. It's
just unused data so leave it as 0s (c default) to avoid implication it's used.
Same for scan_index. 

> > +               },                                                      \
> > +       }
> > +
> > +static const struct iio_chan_spec axiado_saradc_iio_channels[] = {
> > +       AX_SARADC_CH(0, "adc0"),   AX_SARADC_CH(1, "adc1"),
> > +       AX_SARADC_CH(2, "adc2"),   AX_SARADC_CH(3, "adc3"),
> > +       AX_SARADC_CH(4, "adc4"),   AX_SARADC_CH(5, "adc5"),
> > +       AX_SARADC_CH(6, "adc6"),   AX_SARADC_CH(7, "adc7"),
> > +       AX_SARADC_CH(8, "adc8"),   AX_SARADC_CH(9, "adc9"),
> > +       AX_SARADC_CH(10, "adc10"), AX_SARADC_CH(11, "adc11"),
> > +       AX_SARADC_CH(12, "adc12"), AX_SARADC_CH(13, "adc13"),
> > +       AX_SARADC_CH(14, "adc14"), AX_SARADC_CH(15, "adc15"),
> > +};
> > +
> > +static int axiado_saradc_probe(struct platform_device *pdev)
> > +{
> > +       struct axiado_saradc *info;
> > +       const struct axiado_saradc_soc_data *soc_data;  
> 
> Use reverse christmas tree order.
> 
> > +       struct iio_dev *indio_dev;
> > +       int ret;
> > +       u32 reg;  
> 
> Reg isn't a really good name, regval would be better.

Agreed - people sometimes use reg for the address, regval avoids
that possible confusion.

> 
> > +
> > +       indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
A common IIO convention is to have the device used by devm calls , dev_err
etc in a local variable.

	struct device *dev = &pdev->dev;
Shortens all the lines a little and we shouldn't have code directly
touching the dev inside the iio_dev (see below!)

> > +       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);
> > +       if (IS_ERR(info->clk))
> > +               return PTR_ERR(info->clk);
> > +
> > +       info->clk_rate = clk_get_rate(info->clk);
> > +       if (!info->clk_rate)
> > +               return dev_err_probe(&pdev->dev, -EINVAL,
> > +                                    "invalid clock rate\n");
		return dev_err_probe(dev, -EINVAL, "invalid clock rate\n");
Is a good example of where using local dev shortens things enough that
we save on line wrapping. As below - I'm fine if for some of the others they
go a little over 80 chars.

> > +
> > +       info->vref_uv = devm_regulator_get_enable_read_voltage(&pdev->dev,
> > +                                                              "vref");

For IIO we tend to be a little flexible on line length when we get someting
that would  be a lot nicer if we went slightly over 80 chars.  So put this one
on one line.

> > +       if (info->vref_uv < 0)
> > +               return dev_err_probe(&pdev->dev, info->vref_uv,
> > +                                    "failed to get vref voltage\n");
> > +
> > +       soc_data = device_get_match_data(&pdev->dev);
> > +       if (!soc_data)
> > +               return dev_err_probe(&pdev->dev, -EINVAL,
> > +                                    "failed to get match data\n");
> > +
> > +       mutex_init(&info->lock);  
> 
> Use devm_mutex_init since everything else is devm.
> 
> > +       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);  
> 
> Sashiko notes to add a small delay here to prevent a zero width pulse.

That would be true if an external signal. For internal SoC signals everything
should be fine without or documented.  So just check to see if there is a documented
reset hold time. If not this is fine I think. Sashiko is seeing a lot of gpio
style resets where that feedback would be valid.

> 
> > +       iowrite32(reg, info->regs + AX_SARADC_GLOBAL_CTRL);
> > +       indio_dev->name = dev_name(&pdev->dev);  
> 
> Hmm, Sashiko may have a point here, this could break the ABI.
> The name member is usually assigned a hardcoded string and
> it shouldn't contain whitespace, dots or dashes, which dev_name()
> could return.

All true, but critical bit is this is the part number, not something
originating in the kernel device naming schemes.  Given two supported
parts, ideal would be to provide the number for each.  Just put a
string in your soc_data and use that.


> 
> > +       indio_dev->dev.parent = &pdev->dev;

There should be no need to do this - it's done inside
devm_iio_device_alloc().  Shout if that doesn't work for some reason or
I'm missing that you have an unusual parent and need to override the default.

> > +       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");  
> 
> You can just do `return devm_iio_device_register`.
Sometime we should do an audit of that function to make sure all the
plausible error paths (i.e. wrong config data etc) definitely print.
They should - so it should only be -ENOMEM that gets here without
providing info, but more than possible one is missing.

Anyhow not a question for this driver!

> 
> > +       return 0;
> > +}
> > +  
> 
> I don't see any kind of cleanup procedure, like a remove() function
> or devm_add_action_or_reset callback, is this intentional?
> 
> > +static const struct of_device_id axiado_saradc_match[] = {
> > +       {
> > +               .compatible = "axiado,ax3000-saradc",
> > +               .data = &ax3000_saradc_data,
> > +       },
> > +       {
> > +               .compatible = "axiado,ax3005-saradc",
> > +               .data = &ax3005_saradc_data,
> > +       },
> > +       {},
	{ }

For that terminating entry.
No comma as it always needs to be the last one; the spacing is an IIO
convention in the interests of consistency across drivers.

> > +};
> > +MODULE_DEVICE_TABLE(of, axiado_saradc_match);
> > +
> > +static struct platform_driver axiado_saradc_driver = {
> > +       .driver = {
> > +               .name = KBUILD_MODNAME,
> > +               .of_match_table = axiado_saradc_match,
> > +       },
> > +       .probe = axiado_saradc_probe,
> > +};
> > +
> > +module_platform_driver(axiado_saradc_driver);
> > +
> > +MODULE_AUTHOR("AXIADO CORPORATION");
> > +MODULE_DESCRIPTION("AXIADO SARADC driver");
> > +MODULE_LICENSE("GPL");
> >
> > --
> > 2.34.1
> >
> >  
> 

Re: [PATCH 1/3] dt-bindings: iio: adc: add Axiado AX3000/AX3005 SARADC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

On Thu, May 28, 2026 at 01:10:23AM -0700, Petar Stepanovic wrote:
> The Axiado AX3000 and AX3005 SoCs include a 10-bit SAR ADC controller.
> AX3000 supports 16 input channels, while AX3005 supports 8 input
> channels.
> 
> Document the compatible strings, register region, clock, reference
> voltage supply, and IIO channel cells.
> 
> Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>

With Jonathan's correction:
Acked-by: Conor Dooley <conor.dooley@microchip.com>

pw-bot: changes-requested


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]