[PATCH v4 0/2] iio: adc: Add Axiado SARADC driver

[PATCH v4 0/2] iio: adc: Add Axiado SARADC driver

[Petar Stepanovic]

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

A new driver is needed because this SAR ADC controller is a SoC-specific
hardware block used on Axiado SoCs. It has its own register layout,
channel enable handling, conversion control, and data readout sequence,
and it does not match any existing upstream IIO ADC driver.

AX3000 provides sixteen input channels, while AX3005 provides 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.

The datasheet is not publicly available. Public high-level product
information is available at:

  https://axiado.com/products/#AX3080

The register definitions and programming sequence used by this driver
are based on Axiado internal SoC documentation.

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
---
Changes in v4:
- Converted direct MMIO register access from readl()/writel() to the regmap API.
- Added REGMAP_MMIO as a Kconfig selection.
- Added error handling for regmap read and write operations.
- Ensured manual conversion is stopped even if reading the conversion result fails.
- Fixed the regulator error path to return the actual error code instead of the
  uninitialized reference voltage.
- Simplified the channel macro by generating datasheet_name from the
- channel index.
- Changed the channel array to use one channel entry per line.
- Link to v3: https://lore.kernel.org/r/20260622-axiado-ax3000-ax3005-saradc-v3-0-e57c7c7ae675@axiado.com

Changes in v3:
- Fixed vref regulator error handling.
- Added linux/units.h and used MICRO / MILLI in scale calculation.
- Reordered struct axiado_saradc members to improve the structure
  layout.
- Fixed indentation/alignment of multi-line FIELD_PREP(), GENMASK(), and
  writel() expressions.
- Removed the blank line before module_platform_driver().
- Link to v2: https://lore.kernel.org/r/20260611-axiado-ax3000-ax3005-saradc-v2-0-913c9de7c64c@axiado.com

Changes in v2:
- Fixed the devicetree example node name to use the generic ADC node name.
- Removed the explicit `depends on OF` from Kconfig.
- Cleaned up and reordered header includes.
- Added missing includes for `bits.h`, `clk.h`, `cleanup.h`, and `err.h`.
- Removed unused `linux/kernel.h` include.
- Renamed register offset macros to use the `_REG` suffix.
- Renamed register bitfield macros to include the register name prefix.
- Added separate macros for `GLOBAL_CTRL` and `MANUAL_CTRL` register
  fields and values.
- Replaced `iowrite32()` / `ioread32()` with `writel()` / `readl()`.
- Moved ADC conversion locking into `axiado_saradc_conversion()` using
  `guard(mutex)`.
- Replaced `usleep_range()` with `fsleep()`.
- Renamed `vref_uv` to `vref_uV`.
- Added SoC-specific device names in `axiado_saradc_soc_data`.
- Used the fixed SoC-specific name for `indio_dev->name`.
- Removed unused buffered scan configuration from IIO channels.
- Added a managed cleanup action to disable the SARADC hardware on driver
  unbind or probe failure.
- Switched to a local `struct device *dev` helper in probe.
- Used `devm_mutex_init()` for mutex initialization.
- Simplified error handling by using `dev_err_probe()`.
- Updated probe variable declarations to follow reverse Christmas tree
  order.
- Fixed the `of_device_id` terminator style.
- Replaced `KBUILD_MODNAME` with a fixed driver name string.
- Link to v1: https://lore.kernel.org/r/20260528-axiado-ax3000-ax3005-saradc-v1-0-345dd5f6608a@axiado.com

---
Petar Stepanovic (2):
      dt-bindings: iio: adc: add Axiado AX3000/AX3005 SARADC
      iio: adc: add Axiado SARADC driver

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

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

[PATCH v4 1/2] 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>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/iio/adc/axiado,ax3000-saradc.yaml     | 63 ++++++++++++++++++++++
 MAINTAINERS                                        |  8 +++
 2 files changed, 71 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..b910852aa56f
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/axiado,ax3000-saradc.yaml
@@ -0,0 +1,63 @@
+# 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
+
+  clock-names:
+    const: saradc
+
+  '#io-channel-cells':
+    const: 1
+
+  vref-supply:
+    description: Reference voltage regulator supplying the ADC
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#io-channel-cells'
+  - vref-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      adc@806a0000 {
+        compatible = "axiado,ax3000-saradc";
+        reg = <0x0 0x806a0000 0x0 0x400>;
+        clocks = <&pclk>;
+        clock-names = "saradc";
+        vref-supply = <&vref_reg>;
+        #io-channel-cells = <1>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index b2040011a386..d6f4febfe4fd 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>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/adc/axiado,ax3000-saradc.yaml
+
 AXIS ARTPEC ARM64 SoC SUPPORT
 M:	Jesper Nilsson <jesper.nilsson@axis.com>
 M:	Lars Persson <lars.persson@axis.com>

-- 
2.34.1

[PATCH v4 2/2] 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>
---
 MAINTAINERS                     |   1 +
 drivers/iio/adc/Kconfig         |  11 ++
 drivers/iio/adc/Makefile        |   1 +
 drivers/iio/adc/axiado_saradc.c | 286 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 299 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d6f4febfe4fd..d2e99c2260fa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4319,6 +4319,7 @@ M:	Prasad Bolisetty <pbolisetty@axiado.com>
 L:	linux-iio@vger.kernel.org
 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>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a9dedbb8eb46..c6fe026dc70e 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
+	select REGMAP_MMIO
+	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..b802fceb9c7b
--- /dev/null
+++ b/drivers/iio/adc/axiado_saradc.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021-2026 Axiado Corporation
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.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/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/units.h>
+
+#include <linux/iio/iio.h>
+
+/* Register offsets */
+#define AX_SARADC_GLOBAL_CTRL_REG 0x0004
+#define AX_SARADC_MANUAL_CTRL_REG 0x0008
+#define AX_SARADC_DOUT_REG 0x001C
+
+/* GLOBAL_CTRL register fields */
+#define AX_SARADC_GLOBAL_CTRL_CH_EN_MASK	GENMASK(31, 16)
+#define AX_SARADC_GLOBAL_CTRL_SAMPLE_MASK	GENMASK(6, 5)
+#define AX_SARADC_GLOBAL_CTRL_MODE_MASK		GENMASK(4, 3)
+#define AX_SARADC_GLOBAL_CTRL_PD		BIT(2)
+#define AX_SARADC_GLOBAL_CTRL_ENABLE		BIT(0)
+
+/* GLOBAL_CTRL register values */
+#define AX_SARADC_GLOBAL_CTRL_SAMPLE_16	\
+	FIELD_PREP(AX_SARADC_GLOBAL_CTRL_SAMPLE_MASK, 0)
+
+#define AX_SARADC_GLOBAL_CTRL_MODE_MANUAL	\
+	FIELD_PREP(AX_SARADC_GLOBAL_CTRL_MODE_MASK, 1)
+
+/* MANUAL_CTRL register fields */
+#define AX_SARADC_MANUAL_CTRL_ENABLE           BIT(0)
+#define AX_SARADC_MANUAL_CTRL_CH_SEL_MASK      GENMASK(4, 1)
+
+#define AX_SARADC_MANUAL_CTRL_EN(ch)	\
+	(AX_SARADC_MANUAL_CTRL_ENABLE |	\
+	FIELD_PREP(AX_SARADC_MANUAL_CTRL_CH_SEL_MASK, ch))
+
+#define AX_RESOLUTION_BITS 10
+#define AX_SARADC_CONV_CYCLES 13
+#define AX_SARADC_CONV_DELAY_MARGIN_US 10
+
+struct axiado_saradc {
+	struct regmap *regmap;
+	struct clk *clk;
+	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,
+};
+
+
+static int axiado_saradc_conversion(struct axiado_saradc *info,
+				    struct iio_chan_spec const *chan, int *val)
+{
+	unsigned long usecs;
+	unsigned int regval;
+	int stop_ret;
+	int ret;
+
+	guard(mutex)(&info->lock);
+
+	/* Select the channel to be used and trigger conversion */
+	ret = regmap_write(info->regmap, AX_SARADC_MANUAL_CTRL_REG,
+			   AX_SARADC_MANUAL_CTRL_EN(chan->channel));
+	if (ret)
+		return ret;
+
+
+	/* Hardware requires 13 conversion cycles at clk_rate */
+	usecs = DIV_ROUND_UP(AX_SARADC_CONV_CYCLES * USEC_PER_SEC,
+			     info->clk_rate);
+	fsleep(usecs + AX_SARADC_CONV_DELAY_MARGIN_US);
+
+	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;
+
+	*val = regval & GENMASK(AX_RESOLUTION_BITS - 1, 0);
+	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:
+		ret = axiado_saradc_conversion(info, chan, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = info->vref_uV / (MICRO / MILLI);
+		*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 {
+	const char *name;
+	unsigned int num_channels;
+};
+
+static const struct axiado_saradc_soc_data ax3000_saradc_data = {
+	.name = "ax3000_saradc",
+	.num_channels = 16,
+};
+
+static const struct axiado_saradc_soc_data ax3005_saradc_data = {
+	.name = "ax3005_saradc",
+	.num_channels = 8,
+};
+
+#define AX_SARADC_CH(_index)                                            \
+	{                                                               \
+		.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 = "adc" #_index,                        \
+	}
+
+static const struct iio_chan_spec axiado_saradc_iio_channels[] = {
+	AX_SARADC_CH(0),
+	AX_SARADC_CH(1),
+	AX_SARADC_CH(2),
+	AX_SARADC_CH(3),
+	AX_SARADC_CH(4),
+	AX_SARADC_CH(5),
+	AX_SARADC_CH(6),
+	AX_SARADC_CH(7),
+	AX_SARADC_CH(8),
+	AX_SARADC_CH(9),
+	AX_SARADC_CH(10),
+	AX_SARADC_CH(11),
+	AX_SARADC_CH(12),
+	AX_SARADC_CH(13),
+	AX_SARADC_CH(14),
+	AX_SARADC_CH(15),
+};
+
+static void axiado_saradc_disable(void *data)
+{
+	struct axiado_saradc *info = data;
+
+	regmap_write(info->regmap, AX_SARADC_GLOBAL_CTRL_REG,
+		     AX_SARADC_GLOBAL_CTRL_PD);
+}
+
+static int axiado_saradc_probe(struct platform_device *pdev)
+{
+	const struct axiado_saradc_soc_data *soc_data;
+	struct device *dev = &pdev->dev;
+	struct axiado_saradc *info;
+	struct iio_dev *indio_dev;
+	void __iomem *regs;
+	u32 regval;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*info));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	info = iio_priv(indio_dev);
+
+	regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	info->regmap = devm_regmap_init_mmio(dev, regs,
+					     &axiado_saradc_regmap_config);
+	if (IS_ERR(info->regmap))
+		return PTR_ERR(info->regmap);
+
+	info->clk = devm_clk_get_enabled(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(dev, -EINVAL, "invalid clock rate\n");
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to get vref voltage\n");
+
+	info->vref_uV = ret;
+
+	soc_data = device_get_match_data(dev);
+	if (!soc_data)
+		return dev_err_probe(dev, -EINVAL, "failed to get match data\n");
+
+	ret = devm_mutex_init(dev, &info->lock);
+	if (ret)
+		return ret;
+
+	regval = FIELD_PREP(AX_SARADC_GLOBAL_CTRL_CH_EN_MASK,
+			    GENMASK(soc_data->num_channels - 1, 0)) |
+		 AX_SARADC_GLOBAL_CTRL_SAMPLE_16 |
+		 AX_SARADC_GLOBAL_CTRL_MODE_MANUAL |
+		 AX_SARADC_GLOBAL_CTRL_ENABLE;
+
+	ret = regmap_write(info->regmap, AX_SARADC_GLOBAL_CTRL_REG,
+			   AX_SARADC_GLOBAL_CTRL_PD);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(info->regmap, AX_SARADC_GLOBAL_CTRL_REG, regval);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, axiado_saradc_disable, info);
+	if (ret)
+		return ret;
+
+	indio_dev->name = soc_data->name;
+	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;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+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 =  "axiado-saradc",
+		.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 v4 2/2] iio: adc: add Axiado SARADC driver

[Andy Shevchenko]

On Thu, Jul 16, 2026 at 10:53:02PM -0700, Petar Stepanovic 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.

...

Please, follow IWYU principle.

> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>

> +#include <linux/device.h>

You can omit this since you have platform_device.h.

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

+ math.h

> +#include <linux/mod_devicetable.h>

No to this header in a new code, Uwe did some rework recently.

> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>

+ types.h // uXX

> +#include <linux/units.h>

...

> +/* Register offsets */
> +#define AX_SARADC_GLOBAL_CTRL_REG 0x0004
> +#define AX_SARADC_MANUAL_CTRL_REG 0x0008
> +#define AX_SARADC_DOUT_REG 0x001C

Be consistent, use tabs to indent values of the offsets.

...

> +/* GLOBAL_CTRL register values */
> +#define AX_SARADC_GLOBAL_CTRL_SAMPLE_16	\
> +	FIELD_PREP(AX_SARADC_GLOBAL_CTRL_SAMPLE_MASK, 0)
> +
> +#define AX_SARADC_GLOBAL_CTRL_MODE_MANUAL	\
> +	FIELD_PREP(AX_SARADC_GLOBAL_CTRL_MODE_MASK, 1)

FIELD_PREP_CONST() in both cases.

...

> +#define AX_SARADC_MANUAL_CTRL_EN(ch)	\
> +	(AX_SARADC_MANUAL_CTRL_ENABLE |	\
> +	FIELD_PREP(AX_SARADC_MANUAL_CTRL_CH_SEL_MASK, ch))

Missing parentheses for ch. Also wrong indentation of the second line, missing
one space.

...

> +#define AX_RESOLUTION_BITS 10
> +#define AX_SARADC_CONV_CYCLES 13
> +#define AX_SARADC_CONV_DELAY_MARGIN_US 10

Again, be consistent, use tabs to indent values.

...

> +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.

> +	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?

> +};
> +
> +

Single blank line is enough.

...

> +static int axiado_saradc_conversion(struct axiado_saradc *info,
> +				    struct iio_chan_spec const *chan, int *val)
> +{
> +	unsigned long usecs;
> +	unsigned int regval;
> +	int stop_ret;
> +	int ret;
> +
> +	guard(mutex)(&info->lock);
> +
> +	/* Select the channel to be used and trigger conversion */
> +	ret = regmap_write(info->regmap, AX_SARADC_MANUAL_CTRL_REG,
> +			   AX_SARADC_MANUAL_CTRL_EN(chan->channel));
> +	if (ret)
> +		return ret;
> +
> +

Ditto.

> +	/* Hardware requires 13 conversion cycles at clk_rate */
> +	usecs = DIV_ROUND_UP(AX_SARADC_CONV_CYCLES * USEC_PER_SEC,
> +			     info->clk_rate);
> +	fsleep(usecs + AX_SARADC_CONV_DELAY_MARGIN_US);

> +	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?

> +	*val = regval & GENMASK(AX_RESOLUTION_BITS - 1, 0);

Is device responding always in native endianess?

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

Split logically:

static int axiado_saradc_read_raw(struct iio_dev *indio_dev,
				  struct iio_chan_spec const *chan,
				  int *val, int *val2, long mask)

...

> +static void axiado_saradc_disable(void *data)
> +{
> +	struct axiado_saradc *info = data;
> +
> +	regmap_write(info->regmap, AX_SARADC_GLOBAL_CTRL_REG,
> +		     AX_SARADC_GLOBAL_CTRL_PD);
> +}

Supply regmap instead of info and make this simpler

static void axiado_saradc_disable(void *map)
{
	regmap_write(map, AX_SARADC_GLOBAL_CTRL_REG, AX_SARADC_GLOBAL_CTRL_PD);
}

...

> +	regval = FIELD_PREP(AX_SARADC_GLOBAL_CTRL_CH_EN_MASK,
> +			    GENMASK(soc_data->num_channels - 1, 0)) |
> +		 AX_SARADC_GLOBAL_CTRL_SAMPLE_16 |
> +		 AX_SARADC_GLOBAL_CTRL_MODE_MANUAL |
> +		 AX_SARADC_GLOBAL_CTRL_ENABLE;

This is not used in the below call, move it closer to its user.

> +	ret = regmap_write(info->regmap, AX_SARADC_GLOBAL_CTRL_REG,
> +			   AX_SARADC_GLOBAL_CTRL_PD);

With

	struct regmap *map;

at the top, this and other will be shorter and easier to follow.
And I would dare to use a single line:

	ret = regmap_write(map, AX_SARADC_GLOBAL_CTRL_REG, AX_SARADC_GLOBAL_CTRL_PD);

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(info->regmap, AX_SARADC_GLOBAL_CTRL_REG, regval);
> +	if (ret)
> +		return ret;

> +	ret = devm_add_action_or_reset(dev, axiado_saradc_disable, info);

	ret = devm_add_action_or_reset(dev, axiado_saradc_disable, map);

> +	if (ret)
> +		return ret;

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Fri, Jul 17, 2026 at 11:35:01AM +0300, Andy Shevchenko wrote:
> On Thu, Jul 16, 2026 at 10:53:02PM -0700, Petar Stepanovic wrote:

...

> > +static void axiado_saradc_disable(void *data)
> > +{
> > +	struct axiado_saradc *info = data;
> > +
> > +	regmap_write(info->regmap, AX_SARADC_GLOBAL_CTRL_REG,
> > +		     AX_SARADC_GLOBAL_CTRL_PD);
> > +}
> 
> Supply regmap instead of info and make this simpler
> 
> static void axiado_saradc_disable(void *map)
> {
> 	regmap_write(map, AX_SARADC_GLOBAL_CTRL_REG, AX_SARADC_GLOBAL_CTRL_PD);
> }
> 
> ...
> 
> > +	regval = FIELD_PREP(AX_SARADC_GLOBAL_CTRL_CH_EN_MASK,
> > +			    GENMASK(soc_data->num_channels - 1, 0)) |
> > +		 AX_SARADC_GLOBAL_CTRL_SAMPLE_16 |
> > +		 AX_SARADC_GLOBAL_CTRL_MODE_MANUAL |
> > +		 AX_SARADC_GLOBAL_CTRL_ENABLE;
> 
> This is not used in the below call, move it closer to its user.
> 
> > +	ret = regmap_write(info->regmap, AX_SARADC_GLOBAL_CTRL_REG,
> > +			   AX_SARADC_GLOBAL_CTRL_PD);
> 
> With
> 
> 	struct regmap *map;
> 
> at the top, this and other will be shorter and easier to follow.
> And I would dare to use a single line:
> 
> 	ret = regmap_write(map, AX_SARADC_GLOBAL_CTRL_REG, AX_SARADC_GLOBAL_CTRL_PD);

Looking closer at this I think it's a leftover? Since it's an action that does
disable. This one should only enable chip, right?

> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_write(info->regmap, AX_SARADC_GLOBAL_CTRL_REG, regval);
> > +	if (ret)
> > +		return ret;
> 
> > +	ret = devm_add_action_or_reset(dev, axiado_saradc_disable, info);
> 
> 	ret = devm_add_action_or_reset(dev, axiado_saradc_disable, map);
> 
> > +	if (ret)
> > +		return ret;

-- 
With Best Regards,
Andy Shevchenko

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

[Jonathan Cameron]

On Fri, 17 Jul 2026 11:34:56 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Thu, Jul 16, 2026 at 10:53:02PM -0700, Petar Stepanovic 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.  
> 
Andy already commented everywhere I'd have said things, so just
a few places where I'd take a different approach to resolving stuff.

Thanks,

Jonathan

> 
> ...
> 
> > +/* GLOBAL_CTRL register values */
> > +#define AX_SARADC_GLOBAL_CTRL_SAMPLE_16	\
> > +	FIELD_PREP(AX_SARADC_GLOBAL_CTRL_SAMPLE_MASK, 0)
> > +
> > +#define AX_SARADC_GLOBAL_CTRL_MODE_MANUAL	\
> > +	FIELD_PREP(AX_SARADC_GLOBAL_CTRL_MODE_MASK, 1)  
> 
> FIELD_PREP_CONST() in both cases.
> 
For this second one I'd rather see defines for the values the
field takes and then FIELD_PREP only used inline.

#define AX_SARADC_GLOBAL_CTL_MODE_something 0
#define AX_SARADC_GLOBAL_CTL_MODE_MANUAL 1

To me that acts as clearer documentation of what is
going on at the point of the write.

Similar applies for the SAMPLE_MASK field where I'd define the
value 0 with a name then have the FIELD_PREP() inline.


> ...
> 
> > +#define AX_SARADC_MANUAL_CTRL_EN(ch)	\
> > +	(AX_SARADC_MANUAL_CTRL_ENABLE |	\
> > +	FIELD_PREP(AX_SARADC_MANUAL_CTRL_CH_SEL_MASK, ch))  
> 
> Missing parentheses for ch. Also wrong indentation of the second line, missing
> one space.
> 

I wonder if it would be more readable to just push that inline rather
than having this define at all.

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

[Uwe Kleine-König]

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

Hello,

On Thu, Jul 16, 2026 at 10:53:02PM -0700, Petar Stepanovic wrote:
> +#include <linux/mod_devicetable.h>
> [...]
> +#include <linux/platform_device.h>

Please don't explicitly include <linux/mod_devicetable.h>. I'm working
on removing that header and <linux/platform_device.h> already provides
of_device_id.

Best regards
Uwe

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