[PATCH v3 3/9] iio: adc: Add support for ad4062

Jorge Marques posted 9 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH v3 3/9] iio: adc: Add support for ad4062
Posted by Jorge Marques 1 week, 5 days ago
The AD4060/AD4062 are versatile, 16-bit/12-bit, successive approximation
register (SAR) analog-to-digital converter (ADC) with low-power and
threshold monitoring modes.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
---
 MAINTAINERS              |   1 +
 drivers/iio/adc/Kconfig  |  11 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad4062.c | 879 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 892 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8fc28b789d639..003f51cfb0d07 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1438,6 +1438,7 @@ S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4062.yaml
 F:	Documentation/iio/ad4062.rst
+F:	drivers/iio/adc/ad4062.c
 
 ANALOG DEVICES INC AD4080 DRIVER
 M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 58da8255525e4..e506dbe83f488 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -70,6 +70,17 @@ config AD4030
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad4030.
 
+config AD4062
+	tristate "Analog Devices AD4062 Driver"
+	depends on I3C
+	select REGMAP_I3C
+	help
+	  Say yes here to build support for Analog Devices AD4062 I3C analog
+	  to digital converters (ADC).
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad4062.
+
 config AD4080
 	tristate "Analog Devices AD4080 high speed ADC"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7cc8f9a12f763..a897252eeed40 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
 obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
 obj-$(CONFIG_AD4000) += ad4000.o
 obj-$(CONFIG_AD4030) += ad4030.o
+obj-$(CONFIG_AD4062) += ad4062.o
 obj-$(CONFIG_AD4080) += ad4080.o
 obj-$(CONFIG_AD4130) += ad4130.o
 obj-$(CONFIG_AD4170_4) += ad4170-4.o
diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
new file mode 100644
index 0000000000000..54f7f69e40879
--- /dev/null
+++ b/drivers/iio/adc/ad4062.c
@@ -0,0 +1,879 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Analog Devices AD4062 I3C ADC driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i3c/device.h>
+#include <linux/i3c/master.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/math.h>
+#include <linux/minmax.h>
+#include <linux/pm_runtime.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/units.h>
+#include <linux/unaligned.h>
+#include <linux/util_macros.h>
+
+#define AD4062_REG_INTERFACE_CONFIG_A			0x00
+#define AD4062_REG_DEVICE_CONFIG			0x02
+#define     AD4062_REG_DEVICE_CONFIG_POWER_MODE_MSK	GENMASK(1, 0)
+#define     AD4062_REG_DEVICE_CONFIG_LOW_POWER_MODE	3
+#define AD4062_REG_PROD_ID_1				0x05
+#define AD4062_REG_DEVICE_GRADE				0x06
+#define AD4062_REG_SCRATCH_PAD				0x0A
+#define AD4062_REG_VENDOR_H				0x0D
+#define AD4062_REG_STREAM_MODE				0x0E
+#define AD4062_REG_INTERFACE_STATUS			0x11
+#define AD4062_REG_MODE_SET				0x20
+#define     AD4062_REG_MODE_SET_ENTER_ADC		BIT(0)
+#define AD4062_REG_ADC_MODES				0x21
+#define     AD4062_REG_ADC_MODES_MODE_MSK		GENMASK(1, 0)
+#define AD4062_REG_ADC_CONFIG				0x22
+#define     AD4062_REG_ADC_CONFIG_REF_EN_MSK		BIT(5)
+#define     AD4062_REG_ADC_CONFIG_SCALE_EN_MSK		BIT(4)
+#define AD4062_REG_AVG_CONFIG				0x23
+#define AD4062_REG_GP_CONF				0x24
+#define     AD4062_REG_GP_CONF_MODE_MSK_1		GENMASK(6, 4)
+#define AD4062_REG_INTR_CONF				0x25
+#define     AD4062_REG_INTR_CONF_EN_MSK_1		GENMASK(5, 4)
+#define AD4062_REG_TIMER_CONFIG				0x27
+#define     AD4062_REG_TIMER_CONFIG_FS_MASK		GENMASK(7, 4)
+#define AD4062_REG_MON_VAL				0x2F
+#define AD4062_REG_ADC_IBI_EN				0x31
+#define AD4062_REG_ADC_IBI_EN_CONV_TRIGGER		BIT(2)
+#define AD4062_REG_FUSE_CRC				0x40
+#define AD4062_REG_DEVICE_STATUS			0x41
+#define     AD4062_REG_DEVICE_STATUS_DEVICE_RESET	BIT(6)
+#define AD4062_REG_IBI_STATUS				0x48
+#define AD4062_REG_CONV_READ_LSB			0x50
+#define AD4062_REG_CONV_TRIGGER				0x59
+#define AD4062_REG_CONV_AUTO				0x61
+#define AD4062_MAX_REG					AD4062_REG_CONV_AUTO
+
+#define AD4062_MON_VAL_MIDDLE_POINT	0x8000
+
+#define AD4062_I3C_VENDOR	0x0177
+#define AD4062_SOFT_RESET	0x81
+
+#define AD4060_MAX_AVG		0x7
+#define AD4062_MAX_AVG		0xB
+
+#define AD4062_GP_DRDY		0x2
+
+#define AD4062_INTR_EN_NEITHER	0x0
+
+#define AD4062_TCONV_NS		270
+
+enum ad4062_operation_mode {
+	AD4062_SAMPLE_MODE = 0x0,
+	AD4062_BURST_AVERAGING_MODE = 0x1,
+	AD4062_MONITOR_MODE = 0x3,
+};
+
+struct ad4062_chip_info {
+	const struct iio_chan_spec channels[1];
+	const char *name;
+	u16 prod_id;
+	u8 max_avg;
+};
+
+enum {
+	AD4062_SCAN_TYPE_SAMPLE,
+	AD4062_SCAN_TYPE_BURST_AVG,
+};
+
+static const struct iio_scan_type ad4062_scan_type_12_s[] = {
+	[AD4062_SCAN_TYPE_SAMPLE] = {
+		.sign = 's',
+		.realbits = 16,
+		.storagebits = 32,
+		.endianness = IIO_BE,
+	},
+	[AD4062_SCAN_TYPE_BURST_AVG] = {
+		.sign = 's',
+		.realbits = 16,
+		.storagebits = 32,
+		.endianness = IIO_BE,
+	},
+};
+
+static const struct iio_scan_type ad4062_scan_type_16_s[] = {
+	[AD4062_SCAN_TYPE_SAMPLE] = {
+		.sign = 's',
+		.realbits = 16,
+		.storagebits = 32,
+		.endianness = IIO_BE,
+	},
+	[AD4062_SCAN_TYPE_BURST_AVG] = {
+		.sign = 's',
+		.realbits = 24,
+		.storagebits = 32,
+		.endianness = IIO_BE,
+	},
+};
+
+static const unsigned int ad4062_conversion_freqs[] = {
+	2000000, 1000000, 300000, 100000,	/*  0 -  3 */
+	33300, 10000, 3000, 500,		/*  4 -  7 */
+	333, 250, 200, 166,			/*  8 - 11 */
+	140, 124, 111,				/* 12 - 15 */
+};
+
+struct ad4062_state {
+	const struct ad4062_chip_info *chip;
+	const struct ad4062_bus_ops *ops;
+	enum ad4062_operation_mode mode;
+	struct completion completion;
+	struct iio_trigger *trigger;
+	struct iio_dev *indio_dev;
+	struct i3c_device *i3cdev;
+	struct regmap *regmap;
+	int vref_uV;
+	unsigned int samp_freqs[ARRAY_SIZE(ad4062_conversion_freqs)];
+	union {
+		__be32 be32;
+		__be16 be16;
+		u8 bytes[4];
+	} buf __aligned(IIO_DMA_MINALIGN);
+	u16 sampling_frequency;
+	u8 oversamp_ratio;
+	u8 reg_addr_conv;
+};
+
+static const struct regmap_range ad4062_regmap_rd_ranges[] = {
+	regmap_reg_range(AD4062_REG_INTERFACE_CONFIG_A, AD4062_REG_DEVICE_GRADE),
+	regmap_reg_range(AD4062_REG_SCRATCH_PAD, AD4062_REG_INTERFACE_STATUS),
+	regmap_reg_range(AD4062_REG_MODE_SET, AD4062_REG_ADC_IBI_EN),
+	regmap_reg_range(AD4062_REG_FUSE_CRC, AD4062_REG_IBI_STATUS),
+	regmap_reg_range(AD4062_REG_CONV_READ_LSB, AD4062_REG_CONV_AUTO),
+};
+
+static const struct regmap_access_table ad4062_regmap_rd_table = {
+	.yes_ranges = ad4062_regmap_rd_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ad4062_regmap_rd_ranges),
+};
+
+static const struct regmap_range ad4062_regmap_wr_ranges[] = {
+	regmap_reg_range(AD4062_REG_INTERFACE_CONFIG_A, AD4062_REG_DEVICE_CONFIG),
+	regmap_reg_range(AD4062_REG_SCRATCH_PAD, AD4062_REG_SCRATCH_PAD),
+	regmap_reg_range(AD4062_REG_STREAM_MODE, AD4062_REG_INTERFACE_STATUS),
+	regmap_reg_range(AD4062_REG_MODE_SET, AD4062_REG_ADC_IBI_EN),
+	regmap_reg_range(AD4062_REG_FUSE_CRC, AD4062_REG_DEVICE_STATUS),
+};
+
+static const struct regmap_access_table ad4062_regmap_wr_table = {
+	.yes_ranges = ad4062_regmap_wr_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ad4062_regmap_wr_ranges),
+};
+
+static int ad4062_conversion_frequency_set(struct ad4062_state *st, u8 val)
+{
+	return regmap_write(st->regmap, AD4062_REG_TIMER_CONFIG,
+			    FIELD_PREP(AD4062_REG_TIMER_CONFIG_FS_MASK, val));
+}
+
+#define AD4062_CHAN(bits) {							\
+	.type = IIO_VOLTAGE,								\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_RAW) |				\
+				    BIT(IIO_CHAN_INFO_SCALE) |				\
+				    BIT(IIO_CHAN_INFO_CALIBSCALE) |			\
+				    BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),		\
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),	\
+	.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),		\
+	.indexed = 1,									\
+	.channel = 0,									\
+	.has_ext_scan_type = 1,								\
+	.ext_scan_type = ad4062_scan_type_##bits##_s,					\
+	.num_ext_scan_type = ARRAY_SIZE(ad4062_scan_type_##bits##_s),			\
+}
+
+static const struct ad4062_chip_info ad4060_chip_info = {
+	.name = "ad4060",
+	.channels = { AD4062_CHAN(12) },
+	.prod_id = 0x7A,
+	.max_avg = AD4060_MAX_AVG,
+};
+
+static const struct ad4062_chip_info ad4062_chip_info = {
+	.name = "ad4062",
+	.channels = { AD4062_CHAN(16) },
+	.prod_id = 0x7C,
+	.max_avg = AD4062_MAX_AVG,
+};
+
+static int ad4062_set_oversampling_ratio(struct ad4062_state *st, unsigned int val)
+{
+	const u32 _max = GENMASK(st->chip->max_avg, 0)  + 1;
+	const u32 _min = 1;
+	int ret;
+
+	if (!in_range(val, _min, _max))
+		return -EINVAL;
+
+	/* 1 disables oversampling */
+	val = ilog2(val);
+	if (val == 0) {
+		st->mode = AD4062_SAMPLE_MODE;
+	} else {
+		st->mode = AD4062_BURST_AVERAGING_MODE;
+		ret = regmap_write(st->regmap, AD4062_REG_AVG_CONFIG, val - 1);
+		if (ret)
+			return ret;
+	}
+	st->oversamp_ratio = val;
+
+	return 0;
+}
+
+static int ad4062_get_oversampling_ratio(struct ad4062_state *st,
+					 unsigned int *val)
+{
+	int ret, buf;
+
+	if (st->mode == AD4062_SAMPLE_MODE) {
+		*val = 1;
+		return 0;
+	}
+
+	ret = regmap_read(st->regmap, AD4062_REG_AVG_CONFIG, &buf);
+	if (ret)
+		return ret;
+
+	*val = BIT(buf + 1);
+	return 0;
+}
+
+static int ad4062_calc_sampling_frequency(unsigned int fosc, unsigned int oversamp_ratio)
+{
+	/* From datasheet p.31: (n_avg - 1)/fosc + tconv */
+	u32 n_avg = BIT(oversamp_ratio) - 1;
+	u32 period_ns = NSEC_PER_SEC / fosc;
+
+	/* Result is less than 1 Hz */
+	if (n_avg >= fosc)
+		return 1;
+
+	return NSEC_PER_SEC / (n_avg * period_ns + AD4062_TCONV_NS);
+}
+
+static int ad4062_populate_sampling_frequency(struct ad4062_state *st)
+{
+	for (u8 i = 0; i < ARRAY_SIZE(ad4062_conversion_freqs); i++)
+		st->samp_freqs[i] =
+			ad4062_calc_sampling_frequency(ad4062_conversion_freqs[i],
+						       st->oversamp_ratio);
+	return 0;
+}
+
+static int ad4062_get_sampling_frequency(struct ad4062_state *st, int *val)
+{
+	int freq = ad4062_conversion_freqs[st->sampling_frequency];
+
+	*val = ad4062_calc_sampling_frequency(freq, st->oversamp_ratio);
+	return 0;
+}
+
+static int ad4062_set_sampling_frequency(struct ad4062_state *st, int val)
+{
+	int ret;
+
+	ret = ad4062_populate_sampling_frequency(st);
+	if (ret)
+		return ret;
+
+	st->sampling_frequency = find_closest_descending(val, st->samp_freqs,
+							 ARRAY_SIZE(ad4062_conversion_freqs));
+	return 0;
+}
+
+static int ad4062_check_ids(struct ad4062_state *st)
+{
+	struct device *dev = &st->i3cdev->dev;
+	int ret;
+	u16 val;
+
+	ret = regmap_bulk_read(st->regmap, AD4062_REG_PROD_ID_1,
+			       &st->buf.be16, sizeof(st->buf.be16));
+	if (ret)
+		return ret;
+
+	val = get_unaligned_be16(st->buf.bytes);
+	if (val != st->chip->prod_id)
+		dev_warn(dev, "Production ID x%x does not match known values", val);
+
+	ret = regmap_bulk_read(st->regmap, AD4062_REG_VENDOR_H,
+			       &st->buf.be16, sizeof(st->buf.be16));
+	if (ret)
+		return ret;
+
+	val = get_unaligned_be16(st->buf.bytes);
+	if (val != AD4062_I3C_VENDOR) {
+		dev_err(dev, "Vendor ID x%x does not match expected value\n", val);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int ad4062_set_operation_mode(struct ad4062_state *st,
+				     enum ad4062_operation_mode mode)
+{
+	int ret;
+
+	if (mode == AD4062_BURST_AVERAGING_MODE) {
+		ret = ad4062_conversion_frequency_set(st, st->sampling_frequency);
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_update_bits(st->regmap, AD4062_REG_ADC_MODES,
+				 AD4062_REG_ADC_MODES_MODE_MSK, mode);
+	if (ret)
+		return ret;
+
+	return regmap_write(st->regmap, AD4062_REG_MODE_SET,
+			    AD4062_REG_MODE_SET_ENTER_ADC);
+}
+
+static int ad4062_soft_reset(struct ad4062_state *st)
+{
+	u8 val = AD4062_SOFT_RESET;
+	int ret;
+
+	ret = regmap_write(st->regmap, AD4062_REG_INTERFACE_CONFIG_A, val);
+	if (ret)
+		return ret;
+
+	/* Wait AD4062 treset time, datasheet p8 */
+	ndelay(60);
+
+	return 0;
+}
+
+static int ad4062_setup(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+			const bool *ref_sel)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+	const struct iio_scan_type *scan_type;
+	int ret;
+	u8 val;
+
+	scan_type = iio_get_current_scan_type(indio_dev, chan);
+	if (IS_ERR(scan_type))
+		return PTR_ERR(scan_type);
+
+	val = FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_1, AD4062_GP_DRDY);
+	ret = regmap_update_bits(st->regmap, AD4062_REG_GP_CONF,
+				 AD4062_REG_GP_CONF_MODE_MSK_1, val);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(st->regmap, AD4062_REG_ADC_CONFIG,
+				 AD4062_REG_ADC_CONFIG_REF_EN_MSK,
+				 FIELD_PREP(AD4062_REG_ADC_CONFIG_REF_EN_MSK,
+					    *ref_sel));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, AD4062_REG_DEVICE_STATUS,
+			   AD4062_REG_DEVICE_STATUS_DEVICE_RESET);
+	if (ret)
+		return ret;
+
+	val = FIELD_PREP(AD4062_REG_INTR_CONF_EN_MSK_1, AD4062_INTR_EN_NEITHER);
+	ret = regmap_update_bits(st->regmap, AD4062_REG_INTR_CONF,
+				 AD4062_REG_INTR_CONF_EN_MSK_1, val);
+	if (ret)
+		return ret;
+
+	put_unaligned_be16(AD4062_MON_VAL_MIDDLE_POINT, st->buf.bytes);
+	return regmap_bulk_write(st->regmap, AD4062_REG_MON_VAL,
+				 &st->buf.be16, sizeof(st->buf.be16));
+}
+
+static irqreturn_t ad4062_irq_handler_drdy(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct ad4062_state *st = iio_priv(indio_dev);
+
+	complete(&st->completion);
+
+	return IRQ_HANDLED;
+}
+
+static void ad4062_ibi_handler(struct i3c_device *i3cdev,
+			       const struct i3c_ibi_payload *payload)
+{
+	struct ad4062_state *st = i3cdev_get_drvdata(i3cdev);
+
+	complete(&st->completion);
+}
+
+static void ad4062_remove_ibi(void *data)
+{
+	struct i3c_device *i3cdev = data;
+
+	i3c_device_disable_ibi(i3cdev);
+	i3c_device_free_ibi(i3cdev);
+}
+
+static int ad4062_request_ibi(struct i3c_device *i3cdev)
+{
+	const struct i3c_ibi_setup ibireq = {
+		.max_payload_len = 1,
+		.num_slots = 1,
+		.handler = ad4062_ibi_handler,
+	};
+	int ret;
+
+	ret = i3c_device_request_ibi(i3cdev, &ibireq);
+	if (ret)
+		return ret;
+
+	ret = i3c_device_enable_ibi(i3cdev);
+	if (ret)
+		goto err_enable_ibi;
+
+	return devm_add_action_or_reset(&i3cdev->dev, ad4062_remove_ibi, i3cdev);
+
+err_enable_ibi:
+	i3c_device_free_ibi(i3cdev);
+	return ret;
+}
+
+static int ad4062_request_irq(struct iio_dev *indio_dev)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->i3cdev->dev;
+	int ret;
+
+	ret = fwnode_irq_get_byname(dev_fwnode(&st->i3cdev->dev), "gp1");
+	if (ret == -EPROBE_DEFER) {
+		return ret;
+	} else if (ret < 0) {
+		return regmap_update_bits(st->regmap, AD4062_REG_ADC_IBI_EN,
+					  AD4062_REG_ADC_IBI_EN_CONV_TRIGGER,
+					  AD4062_REG_ADC_IBI_EN_CONV_TRIGGER);
+	}
+	return devm_request_threaded_irq(dev, ret,
+					 ad4062_irq_handler_drdy,
+					 NULL, IRQF_ONESHOT, indio_dev->name,
+					 indio_dev);
+}
+
+static const int ad4062_oversampling_avail[] = {
+	1, 2, 4, 8, 16, 32, 64, 128,		/*  0 -  7 */
+	256, 512, 1024, 2048, 4096,		/*  8 - 12 */
+};
+
+static int ad4062_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, const int **vals,
+			     int *type, int *len, long mask)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*vals = ad4062_oversampling_avail;
+		*len = ARRAY_SIZE(ad4062_oversampling_avail);
+		*type = IIO_VAL_INT;
+
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = ad4062_populate_sampling_frequency(st);
+		if (ret)
+			return ret;
+		*vals = st->samp_freqs;
+		*len = st->oversamp_ratio ? ARRAY_SIZE(ad4062_conversion_freqs) : 1;
+		*type = IIO_VAL_INT;
+
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4062_get_chan_scale(struct iio_dev *indio_dev, int *val, int *val2)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+	const struct iio_scan_type *scan_type;
+
+	scan_type = iio_get_current_scan_type(indio_dev, st->chip->channels);
+	if (IS_ERR(scan_type))
+		return PTR_ERR(scan_type);
+
+	*val = (st->vref_uV * 2) / (MICRO / MILLI);
+
+	*val2 = scan_type->realbits - 1; /* signed */
+
+	return IIO_VAL_FRACTIONAL_LOG2;
+}
+
+static int ad4062_get_chan_calibscale(struct ad4062_state *st, int *val, int *val2)
+{
+	int ret;
+
+	ret = regmap_bulk_read(st->regmap, AD4062_REG_MON_VAL,
+			       &st->buf.be16, sizeof(st->buf.be16));
+	if (ret)
+		return ret;
+
+	/* From datasheet: code out = code in × mon_val/0x8000 */
+	*val = get_unaligned_be16(st->buf.bytes) * 2;
+	*val2 = 16;
+
+	return IIO_VAL_FRACTIONAL_LOG2;
+}
+
+static int ad4062_set_chan_calibscale(struct ad4062_state *st, int gain_int,
+				      int gain_frac)
+{
+	/* Divide numerator and denumerator by known great common divider */
+	const u32 mon_val = AD4062_MON_VAL_MIDDLE_POINT / 64;
+	const u32 micro = MICRO / 64;
+	const u32 gain_fp = gain_int * MICRO + gain_frac;
+	const u32 reg_val = DIV_ROUND_CLOSEST(gain_fp * mon_val, micro);
+	int ret;
+
+	/* Checks if the gain is in range and the value fits the field */
+	if (gain_int < 0 || gain_int > 1 || reg_val > BIT(16) - 1)
+		return -EINVAL;
+
+	put_unaligned_be16(reg_val, st->buf.bytes);
+	ret = regmap_bulk_write(st->regmap, AD4062_REG_MON_VAL,
+				&st->buf.be16, sizeof(st->buf.be16));
+	if (ret)
+		return ret;
+
+	/* Enable scale if gain is not equal to one */
+	return regmap_update_bits(st->regmap, AD4062_REG_ADC_CONFIG,
+				  AD4062_REG_ADC_CONFIG_SCALE_EN_MSK,
+				  FIELD_PREP(AD4062_REG_ADC_CONFIG_SCALE_EN_MSK,
+					     !(gain_int == 1 && gain_frac == 0)));
+}
+
+static int ad4062_read_chan_raw(struct ad4062_state *st, int *val)
+{
+	int ret;
+	struct i3c_device *i3cdev = st->i3cdev;
+	struct i3c_priv_xfer t0 = {
+		.data.out = &st->reg_addr_conv,
+		.len = sizeof(st->reg_addr_conv),
+		.rnw = false,
+	};
+	struct i3c_priv_xfer t1 = {
+		.data.in = &st->buf.be32,
+		.len = sizeof(st->buf.be32),
+		.rnw = true,
+	};
+
+	ACQUIRE(pm_runtime_active_try_enabled, pm)(&st->i3cdev->dev);
+	ret = ACQUIRE_ERR(pm_runtime_active_try_enabled, &pm);
+	if (ret)
+		return ret;
+
+	ret = ad4062_set_operation_mode(st, st->mode);
+	if (ret)
+		return ret;
+
+	reinit_completion(&st->completion);
+	/* Change address pointer to trigger conversion */
+	ret = i3c_device_do_priv_xfers(i3cdev, &t0, 1);
+	if (ret)
+		return ret;
+	/*
+	 * Single sample read should be used only for oversampling and
+	 * sampling frequency pairs that take less than 1 sec.
+	 */
+	ret = wait_for_completion_timeout(&st->completion,
+					  msecs_to_jiffies(1000));
+	if (!ret)
+		return -ETIMEDOUT;
+
+	ret = i3c_device_do_priv_xfers(i3cdev, &t1, 1);
+	if (ret)
+		return ret;
+	*val = get_unaligned_be32(st->buf.bytes);
+	return 0;
+}
+
+static int ad4062_read_raw_dispatch(struct ad4062_state *st,
+				    int *val, int *val2, long info)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		return ad4062_read_chan_raw(st, val);
+
+	case IIO_CHAN_INFO_CALIBSCALE:
+		return ad4062_get_chan_calibscale(st, val, val2);
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		return ad4062_get_oversampling_ratio(st, val);
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return ad4062_get_sampling_frequency(st, val);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4062_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long info)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (info == IIO_CHAN_INFO_SCALE)
+		return ad4062_get_chan_scale(indio_dev, val, val2);
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	ret = ad4062_read_raw_dispatch(st, val, val2, info);
+
+	iio_device_release_direct(indio_dev);
+	return ret ?: IIO_VAL_INT;
+}
+
+static int ad4062_write_raw_dispatch(struct ad4062_state *st, int val, int val2,
+				     long info)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		return ad4062_set_oversampling_ratio(st, val);
+
+	case IIO_CHAN_INFO_CALIBSCALE:
+		return ad4062_set_chan_calibscale(st, val, val2);
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return ad4062_set_sampling_frequency(st, val);
+
+	default:
+		return -EINVAL;
+	}
+};
+
+static int ad4062_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int val,
+			    int val2, long info)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	ret = ad4062_write_raw_dispatch(st, val, val2, info);
+
+	iio_device_release_direct(indio_dev);
+	return ret;
+}
+
+static int ad4062_debugfs_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+				     unsigned int writeval, unsigned int *readval)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+	else
+		return regmap_write(st->regmap, reg, writeval);
+}
+
+static int ad4062_get_current_scan_type(const struct iio_dev *indio_dev,
+					const struct iio_chan_spec *chan)
+{
+	struct ad4062_state *st = iio_priv(indio_dev);
+
+	return st->mode == AD4062_BURST_AVERAGING_MODE ?
+			   AD4062_SCAN_TYPE_BURST_AVG :
+			   AD4062_SCAN_TYPE_SAMPLE;
+}
+
+static const struct iio_info ad4062_info = {
+	.read_raw = ad4062_read_raw,
+	.write_raw = ad4062_write_raw,
+	.read_avail = ad4062_read_avail,
+	.get_current_scan_type = &ad4062_get_current_scan_type,
+	.debugfs_reg_access = &ad4062_debugfs_reg_access,
+};
+
+static const struct regmap_config ad4062_regmap_config = {
+	.name = "ad4062",
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = AD4062_MAX_REG,
+	.rd_table = &ad4062_regmap_rd_table,
+	.wr_table = &ad4062_regmap_wr_table,
+	.can_sleep = true,
+};
+
+static int ad4062_regulators_get(struct ad4062_state *st, bool *ref_sel)
+{
+	struct device *dev = &st->i3cdev->dev;
+	int ret;
+
+	ret = devm_regulator_get_enable(dev, "vio");
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to enable vio voltage\n");
+
+	st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "ref");
+	*ref_sel = st->vref_uV == -ENODEV;
+	if (st->vref_uV < 0 && !*ref_sel) {
+		return dev_err_probe(dev, st->vref_uV,
+				     "Failed to enable and read ref voltage\n");
+	}
+
+	if (*ref_sel) {
+		st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "vdd");
+		if (st->vref_uV < 0)
+			return dev_err_probe(dev, st->vref_uV,
+					     "Failed to enable and read vdd voltage\n");
+	} else {
+		ret = devm_regulator_get_enable(dev, "vdd");
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to enable vdd regulator\n");
+	}
+
+	return 0;
+}
+
+static const struct i3c_device_id ad4062_id_table[] = {
+	I3C_DEVICE(AD4062_I3C_VENDOR, ad4060_chip_info.prod_id, &ad4060_chip_info),
+	I3C_DEVICE(AD4062_I3C_VENDOR, ad4062_chip_info.prod_id, &ad4062_chip_info),
+	{ }
+};
+MODULE_DEVICE_TABLE(i3c, ad4062_id_table);
+
+static int ad4062_probe(struct i3c_device *i3cdev)
+{
+	const struct i3c_device_id *id = i3c_device_match_id(i3cdev, ad4062_id_table);
+	const struct ad4062_chip_info *chip = id->data;
+	struct device *dev = &i3cdev->dev;
+	struct iio_dev *indio_dev;
+	struct ad4062_state *st;
+	bool ref_sel;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->i3cdev = i3cdev;
+	i3cdev_set_drvdata(i3cdev, st);
+	init_completion(&st->completion);
+
+	ret = ad4062_regulators_get(st, &ref_sel);
+	if (ret)
+		return ret;
+
+	st->regmap = devm_regmap_init_i3c(i3cdev, &ad4062_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(dev, PTR_ERR(st->regmap),
+				     "Failed to initialize regmap\n");
+
+	st->mode = AD4062_SAMPLE_MODE;
+	st->chip = chip;
+	st->sampling_frequency = 0;
+	st->oversamp_ratio = 0;
+	st->indio_dev = indio_dev;
+	st->reg_addr_conv = AD4062_REG_CONV_TRIGGER;
+
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->num_channels = 1;
+	indio_dev->info = &ad4062_info;
+	indio_dev->name = chip->name;
+	indio_dev->channels = chip->channels;
+
+	ret = ad4062_soft_reset(st);
+	if (ret)
+		return dev_err_probe(dev, ret, "AD4062 failed to soft reset\n");
+
+	ret = ad4062_check_ids(st);
+	if (ret)
+		return ret;
+
+	ret = ad4062_setup(indio_dev, indio_dev->channels, &ref_sel);
+	if (ret)
+		return ret;
+
+	ret = ad4062_request_irq(indio_dev);
+	if (ret)
+		return ret;
+
+	pm_runtime_set_active(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable pm_runtime\n");
+
+	pm_runtime_set_autosuspend_delay(dev, 1000);
+	pm_runtime_use_autosuspend(dev);
+
+	ret = ad4062_request_ibi(i3cdev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request i3c ibi\n");
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static int ad4062_runtime_suspend(struct device *dev)
+{
+	struct ad4062_state *st = dev_get_drvdata(dev);
+
+	return regmap_write(st->regmap, AD4062_REG_DEVICE_CONFIG,
+			    FIELD_PREP(AD4062_REG_DEVICE_CONFIG_POWER_MODE_MSK,
+				       AD4062_REG_DEVICE_CONFIG_LOW_POWER_MODE));
+}
+
+static int ad4062_runtime_resume(struct device *dev)
+{
+	struct ad4062_state *st = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regmap_clear_bits(st->regmap, AD4062_REG_DEVICE_CONFIG,
+				AD4062_REG_DEVICE_CONFIG_POWER_MODE_MSK);
+	if (ret)
+		return ret;
+
+	/* Wait device functional blocks to power up */
+	fsleep(2 * USEC_PER_MSEC);
+	return 0;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(ad4062_pm_ops,
+				 ad4062_runtime_suspend, ad4062_runtime_resume, NULL);
+
+static struct i3c_driver ad4062_driver = {
+	.driver = {
+		.name = "ad4062",
+		.pm = pm_ptr(&ad4062_pm_ops),
+	},
+	.probe = ad4062_probe,
+	.id_table = ad4062_id_table,
+};
+module_i3c_driver(ad4062_driver);
+
+MODULE_AUTHOR("Jorge Marques <jorge.marques@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD4062");
+MODULE_LICENSE("GPL");

-- 
2.51.1

Re: [PATCH v3 3/9] iio: adc: Add support for ad4062
Posted by kernel test robot 1 week, 4 days ago
Hi Jorge,

kernel test robot noticed the following build errors:

[auto build test ERROR on f9e05791642810a0cf6237d39fafd6fec5e0b4bb]

url:    https://github.com/intel-lab-lkp/linux/commits/Jorge-Marques/dt-bindings-iio-adc-Add-adi-ad4062/20251206-033708
base:   f9e05791642810a0cf6237d39fafd6fec5e0b4bb
patch link:    https://lore.kernel.org/r/20251205-staging-ad4062-v3-3-8761355f9c66%40analog.com
patch subject: [PATCH v3 3/9] iio: adc: Add support for ad4062
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20251207/202512070145.nkD9ROxx-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251207/202512070145.nkD9ROxx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512070145.nkD9ROxx-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/adc/ad4062.c:762:49: error: initializer element is not a compile-time constant
           I3C_DEVICE(AD4062_I3C_VENDOR, ad4060_chip_info.prod_id, &ad4060_chip_info),
                                         ~~~~~~~~~~~~~~~~~^~~~~~~
   include/linux/i3c/device.h:151:14: note: expanded from macro 'I3C_DEVICE'
                   .part_id = _partid,                                     \
                              ^~~~~~~
   1 error generated.


vim +762 drivers/iio/adc/ad4062.c

   760	
   761	static const struct i3c_device_id ad4062_id_table[] = {
 > 762		I3C_DEVICE(AD4062_I3C_VENDOR, ad4060_chip_info.prod_id, &ad4060_chip_info),
   763		I3C_DEVICE(AD4062_I3C_VENDOR, ad4062_chip_info.prod_id, &ad4062_chip_info),
   764		{ }
   765	};
   766	MODULE_DEVICE_TABLE(i3c, ad4062_id_table);
   767	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v3 3/9] iio: adc: Add support for ad4062
Posted by Jonathan Cameron 1 week, 4 days ago
On Fri, 5 Dec 2025 16:12:04 +0100
Jorge Marques <jorge.marques@analog.com> wrote:

> The AD4060/AD4062 are versatile, 16-bit/12-bit, successive approximation
> register (SAR) analog-to-digital converter (ADC) with low-power and
> threshold monitoring modes.
> 
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Hi Jorge,

I replied late to some of the earlier review discussion as I've been
away.  Make sure you check those as well as this review as I may have
forgotten to repeat something here.

Thanks,

Jonathan

> diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
> new file mode 100644
> index 0000000000000..54f7f69e40879
> --- /dev/null
> +++ b/drivers/iio/adc/ad4062.c
> @@ -0,0 +1,879 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Analog Devices AD4062 I3C ADC driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i3c/device.h>
> +#include <linux/i3c/master.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>

What is this here for?  It is not needed in a typical modern IIO driver.
(One day I hope to finish getting rid of the remaining users and drop this
header!)

> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/math.h>
> +#include <linux/minmax.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +#include <linux/units.h>
> +#include <linux/unaligned.h>
> +#include <linux/util_macros.h>

> +static const struct iio_scan_type ad4062_scan_type_12_s[] = {
> +	[AD4062_SCAN_TYPE_SAMPLE] = {
> +		.sign = 's',
> +		.realbits = 16,

Not 12?

> +		.storagebits = 32,
Given we are doing data mangling anyway why not store in a 16 bit value.

BTW it would have been easier to spot issues with this if you'd introduced
the scan type stuff with the use of scans in the patch that adds buffered
support.  So please move this stuff there.

> +		.endianness = IIO_BE,
> +	},
> +	[AD4062_SCAN_TYPE_BURST_AVG] = {
> +		.sign = 's',
> +		.realbits = 16,
> +		.storagebits = 32,
> +		.endianness = IIO_BE,
> +	},
> +};

> +struct ad4062_state {
> +	const struct ad4062_chip_info *chip;
> +	const struct ad4062_bus_ops *ops;
> +	enum ad4062_operation_mode mode;
> +	struct completion completion;
> +	struct iio_trigger *trigger;
> +	struct iio_dev *indio_dev;
> +	struct i3c_device *i3cdev;
> +	struct regmap *regmap;
> +	int vref_uV;
> +	unsigned int samp_freqs[ARRAY_SIZE(ad4062_conversion_freqs)];
> +	union {
> +		__be32 be32;
> +		__be16 be16;
> +		u8 bytes[4];
> +	} buf __aligned(IIO_DMA_MINALIGN);
> +	u16 sampling_frequency;

See my response to the original thread about this.  This looks like
it breaks the dance we do to ensure DMA buffers don't share a cacheline
with anything else.

> +	u8 oversamp_ratio;
> +	u8 reg_addr_conv;
> +};

> +
> +static const struct ad4062_chip_info ad4060_chip_info = {
> +	.name = "ad4060",
> +	.channels = { AD4062_CHAN(12) },
> +	.prod_id = 0x7A,
> +	.max_avg = AD4060_MAX_AVG,

This is a little confusing. I guess it's the maximum register value, not the
number of samples averaged.  Perhaps rename.

> +};
> +
> +static const struct ad4062_chip_info ad4062_chip_info = {
> +	.name = "ad4062",
> +	.channels = { AD4062_CHAN(16) },
> +	.prod_id = 0x7C,
> +	.max_avg = AD4062_MAX_AVG,
> +};
> +
> +static int ad4062_set_oversampling_ratio(struct ad4062_state *st, unsigned int val)
> +{
> +	const u32 _max = GENMASK(st->chip->max_avg, 0)  + 1;

One too many spaces before +


> +static int ad4062_check_ids(struct ad4062_state *st)
> +{
> +	struct device *dev = &st->i3cdev->dev;
> +	int ret;
> +	u16 val;
> +
> +	ret = regmap_bulk_read(st->regmap, AD4062_REG_PROD_ID_1,
> +			       &st->buf.be16, sizeof(st->buf.be16));
> +	if (ret)
> +		return ret;
> +
> +	val = get_unaligned_be16(st->buf.bytes);
As below. use buf.be16 and aligned 

> +	if (val != st->chip->prod_id)
> +		dev_warn(dev, "Production ID x%x does not match known values", val);
> +
> +	ret = regmap_bulk_read(st->regmap, AD4062_REG_VENDOR_H,
> +			       &st->buf.be16, sizeof(st->buf.be16));
> +	if (ret)
> +		return ret;
> +
> +	val = get_unaligned_be16(st->buf.bytes);

As elsewhere, use the aligned conversions functions and buf.be16

> +	if (val != AD4062_I3C_VENDOR) {
> +		dev_err(dev, "Vendor ID x%x does not match expected value\n", val);
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}

...

> +static int ad4062_setup(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> +			const bool *ref_sel)
> +{
> +	struct ad4062_state *st = iio_priv(indio_dev);
> +	const struct iio_scan_type *scan_type;
> +	int ret;
> +	u8 val;
> +
> +	scan_type = iio_get_current_scan_type(indio_dev, chan);
> +	if (IS_ERR(scan_type))
> +		return PTR_ERR(scan_type);
> +
> +	val = FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_1, AD4062_GP_DRDY);
Not clear to me why you need a local variable val here, but not in the next
call. Wrapped similarly to that you'd have
	ret = regmap_update_bits(st->regmap, AD4062_REG_GP_CONF,
				 AD4062_REG_GP_CONF_MODE_MSK_1,
				 FIELD_PREP(AD4062_REG_GP_CONF_MODE_MSK_1,
					    AD4062_GP_DRDY));
Looks fine to me.  What I really after here is consistent style.
I slightly prefer inline and no local variable but I'd be happy with either
approach for all of them.
	
> +	ret = regmap_update_bits(st->regmap, AD4062_REG_GP_CONF,
> +				 AD4062_REG_GP_CONF_MODE_MSK_1, val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, AD4062_REG_ADC_CONFIG,
> +				 AD4062_REG_ADC_CONFIG_REF_EN_MSK,
> +				 FIELD_PREP(AD4062_REG_ADC_CONFIG_REF_EN_MSK,
> +					    *ref_sel));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4062_REG_DEVICE_STATUS,
> +			   AD4062_REG_DEVICE_STATUS_DEVICE_RESET);
> +	if (ret)
> +		return ret;
> +
> +	val = FIELD_PREP(AD4062_REG_INTR_CONF_EN_MSK_1, AD4062_INTR_EN_NEITHER);
> +	ret = regmap_update_bits(st->regmap, AD4062_REG_INTR_CONF,
> +				 AD4062_REG_INTR_CONF_EN_MSK_1, val);

As above, I don't really see val as adding value.

> +	if (ret)
> +		return ret;
> +
> +	put_unaligned_be16(AD4062_MON_VAL_MIDDLE_POINT, st->buf.bytes);

	st->buf.be16 = cpu_to_be16(AD4062_MON_VAL_MIDDLE_POINT);

Don't need the unaligned dance as be16 will be aligned.

> +	return regmap_bulk_write(st->regmap, AD4062_REG_MON_VAL,
> +				 &st->buf.be16, sizeof(st->buf.be16));
> +}

...

> +static void ad4062_remove_ibi(void *data)
> +{
> +	struct i3c_device *i3cdev = data;
> +
> +	i3c_device_disable_ibi(i3cdev);
> +	i3c_device_free_ibi(i3cdev);
> +}
> +
> +static int ad4062_request_ibi(struct i3c_device *i3cdev)
> +{
> +	const struct i3c_ibi_setup ibireq = {
> +		.max_payload_len = 1,
> +		.num_slots = 1,
> +		.handler = ad4062_ibi_handler,
> +	};
> +	int ret;
> +
> +	ret = i3c_device_request_ibi(i3cdev, &ibireq);
> +	if (ret)
> +		return ret;
> +
> +	ret = i3c_device_enable_ibi(i3cdev);
> +	if (ret)
> +		goto err_enable_ibi;
> +
> +	return devm_add_action_or_reset(&i3cdev->dev, ad4062_remove_ibi, i3cdev);
It would probably be cleaner to just register two separate callbacks.  Something like

	ret = i3c_device_request_ibi(i3cdev, &ibireq);
	if (ret)
		return ret;

	ret = devm_add_action_or_reset(&i3cdev->dev, ad4062_free_ibi, i3cdev);
	if (ret)
		return ret;

	ret = i3c_device_enable_ibi(i3cdev);
	if (ret)
		return ret;

	return devm_add_action_or_reset(&i3cdev->devm ad4062_unregister_ibi, i3cdev);

> +
> +err_enable_ibi:
> +	i3c_device_free_ibi(i3cdev);
> +	return ret;
> +}
> +
> +static int ad4062_request_irq(struct iio_dev *indio_dev)
> +{
> +	struct ad4062_state *st = iio_priv(indio_dev);
> +	struct device *dev = &st->i3cdev->dev;
> +	int ret;
> +
> +	ret = fwnode_irq_get_byname(dev_fwnode(&st->i3cdev->dev), "gp1");
> +	if (ret == -EPROBE_DEFER) {
> +		return ret;

you returned, no need to follow with else.
	if (ret == -EPROBE_DEFER)
		return ret;

	if (ret < 0)
		return regmap_update_bits()
(see discussion elsewhere about consistency around {} or not in these cases.)


> +	} else if (ret < 0) {
> +		return regmap_update_bits(st->regmap, AD4062_REG_ADC_IBI_EN,
> +					  AD4062_REG_ADC_IBI_EN_CONV_TRIGGER,
> +					  AD4062_REG_ADC_IBI_EN_CONV_TRIGGER);
> +	}
> +	return devm_request_threaded_irq(dev, ret,
> +					 ad4062_irq_handler_drdy,
> +					 NULL, IRQF_ONESHOT, indio_dev->name,
> +					 indio_dev);
> +}


> +static int ad4062_get_chan_calibscale(struct ad4062_state *st, int *val, int *val2)
> +{
> +	int ret;
> +
> +	ret = regmap_bulk_read(st->regmap, AD4062_REG_MON_VAL,
> +			       &st->buf.be16, sizeof(st->buf.be16));
> +	if (ret)
> +		return ret;
> +
> +	/* From datasheet: code out = code in × mon_val/0x8000 */
> +	*val = get_unaligned_be16(st->buf.bytes) * 2;

As above, use st->buf.be16 which we know is aligned so be16_to_cpu()


> +	*val2 = 16;
> +
> +	return IIO_VAL_FRACTIONAL_LOG2;
> +}
> +
> +static int ad4062_set_chan_calibscale(struct ad4062_state *st, int gain_int,
> +				      int gain_frac)
> +{
> +	/* Divide numerator and denumerator by known great common divider */
> +	const u32 mon_val = AD4062_MON_VAL_MIDDLE_POINT / 64;
> +	const u32 micro = MICRO / 64;
> +	const u32 gain_fp = gain_int * MICRO + gain_frac;
> +	const u32 reg_val = DIV_ROUND_CLOSEST(gain_fp * mon_val, micro);
> +	int ret;
> +
> +	/* Checks if the gain is in range and the value fits the field */
> +	if (gain_int < 0 || gain_int > 1 || reg_val > BIT(16) - 1)
> +		return -EINVAL;
> +
> +	put_unaligned_be16(reg_val, st->buf.bytes);

Same as above on using be16

> +	ret = regmap_bulk_write(st->regmap, AD4062_REG_MON_VAL,
> +				&st->buf.be16, sizeof(st->buf.be16));
> +	if (ret)
> +		return ret;
> +
> +	/* Enable scale if gain is not equal to one */
> +	return regmap_update_bits(st->regmap, AD4062_REG_ADC_CONFIG,
> +				  AD4062_REG_ADC_CONFIG_SCALE_EN_MSK,
> +				  FIELD_PREP(AD4062_REG_ADC_CONFIG_SCALE_EN_MSK,
> +					     !(gain_int == 1 && gain_frac == 0)));
> +}
> +
> +static int ad4062_read_chan_raw(struct ad4062_state *st, int *val)
> +{
> +	int ret;
> +	struct i3c_device *i3cdev = st->i3cdev;
> +	struct i3c_priv_xfer t0 = {

Can we give these names for what they are doing rather than t0 and t1?

> +		.data.out = &st->reg_addr_conv,
> +		.len = sizeof(st->reg_addr_conv),
> +		.rnw = false,
> +	};
> +	struct i3c_priv_xfer t1 = {
> +		.data.in = &st->buf.be32,
> +		.len = sizeof(st->buf.be32),
> +		.rnw = true,
> +	};
> +
> +	ACQUIRE(pm_runtime_active_try_enabled, pm)(&st->i3cdev->dev);

Rafael did propose a cleaner way of doing this. I'm not sure if it went
in during the merge window or not.  Take a look to see if it anything
has been added in the PM pull request (IIRC Rafael converted all existing
users to the new scheme in that patch set so should be easy to find.)

> +	ret = ACQUIRE_ERR(pm_runtime_active_try_enabled, &pm);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad4062_set_operation_mode(st, st->mode);
> +	if (ret)
> +		return ret;
> +
> +	reinit_completion(&st->completion);
> +	/* Change address pointer to trigger conversion */
> +	ret = i3c_device_do_priv_xfers(i3cdev, &t0, 1);
> +	if (ret)
> +		return ret;
> +	/*
> +	 * Single sample read should be used only for oversampling and
> +	 * sampling frequency pairs that take less than 1 sec.
> +	 */
> +	ret = wait_for_completion_timeout(&st->completion,
> +					  msecs_to_jiffies(1000));
> +	if (!ret)
> +		return -ETIMEDOUT;
> +
> +	ret = i3c_device_do_priv_xfers(i3cdev, &t1, 1);
> +	if (ret)
> +		return ret;
> +	*val = get_unaligned_be32(st->buf.bytes);

As with the be16 cases, we know the __be32 variable is aligned so can
use the be32_to_cpu() to do the conversion.

> +	return 0;
> +}
> +


> +static int ad4062_write_raw_dispatch(struct ad4062_state *st, int val, int val2,
> +				     long info)
> +{
> +	switch (info) {
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		return ad4062_set_oversampling_ratio(st, val);

Small thing but probably want to check if val2  == 0 in cases where it's
not used.

> +
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		return ad4062_set_chan_calibscale(st, val, val2);
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return ad4062_set_sampling_frequency(st, val);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +};


> +static int ad4062_regulators_get(struct ad4062_state *st, bool *ref_sel)
> +{
> +	struct device *dev = &st->i3cdev->dev;
> +	int ret;
> +
> +	ret = devm_regulator_get_enable(dev, "vio");
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to enable vio voltage\n");

Fairly sure that's 80 chars on one line so no need to wrap.

> +
> +	st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "ref");
> +	*ref_sel = st->vref_uV == -ENODEV;
> +	if (st->vref_uV < 0 && !*ref_sel) {

For consistency, no {} here or add them elsewhere for multiline statements
due to line breaks rather than multiple real lines.


> +		return dev_err_probe(dev, st->vref_uV,
> +				     "Failed to enable and read ref voltage\n");
> +	}
> +
> +	if (*ref_sel) {
> +		st->vref_uV = devm_regulator_get_enable_read_voltage(dev, "vdd");
> +		if (st->vref_uV < 0)
> +			return dev_err_probe(dev, st->vref_uV,
> +					     "Failed to enable and read vdd voltage\n");
> +	} else {
> +		ret = devm_regulator_get_enable(dev, "vdd");
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to enable vdd regulator\n");
> +	}
> +
> +	return 0;
> +}

>
Re: [PATCH v3 3/9] iio: adc: Add support for ad4062
Posted by Jorge Marques 1 week, 2 days ago
On Sat, Dec 06, 2025 at 05:34:59PM +0000, Jonathan Cameron wrote:
> On Fri, 5 Dec 2025 16:12:04 +0100
> Jorge Marques <jorge.marques@analog.com> wrote:
> 
> > The AD4060/AD4062 are versatile, 16-bit/12-bit, successive approximation
> > register (SAR) analog-to-digital converter (ADC) with low-power and
> > threshold monitoring modes.
> > 
> > Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Hi Jonathan,
> Hi Jorge,
> 
> I replied late to some of the earlier review discussion as I've been
> away.  Make sure you check those as well as this review as I may have
> forgotten to repeat something here.
> 
> Thanks,
> 
> Jonathan
> 
> > +#include <linux/iio/sysfs.h>
> 
> What is this here for?  It is not needed in a typical modern IIO driver.
> (One day I hope to finish getting rid of the remaining users and drop this
> header!)
> 
IIO_DEVICE_ATTR_RW for events/sampling_frequency in the events commit.
I will add only add at that commit.
> > +#include <linux/interrupt.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/math.h>
> > +#include <linux/minmax.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/property.h>
> > +#include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/string.h>
> > +#include <linux/types.h>
> > +#include <linux/units.h>
> > +#include <linux/unaligned.h>
> > +#include <linux/util_macros.h>
> 
> > +static const struct iio_scan_type ad4062_scan_type_12_s[] = {
> > +	[AD4062_SCAN_TYPE_SAMPLE] = {
> > +		.sign = 's',
> > +		.realbits = 16,
> 
> Not 12?
Yes, and for burst avg mode, 14 bits.
> 
> > +		.storagebits = 32,
> Given we are doing data mangling anyway why not store in a 16 bit value.
> 
> BTW it would have been easier to spot issues with this if you'd introduced
> the scan type stuff with the use of scans in the patch that adds buffered
> support.  So please move this stuff there.
>
This can be done, just note that for ad4062 in burst avg mode the
realbits is 24 bits, so the storagebits is 32 bits only on that case
and will requires a few conditionals to handle just this case.

To not overly complicated the logic, for ad4062 I will always read
32-bits still. st->reg_addr_conv then takes:
	// IBI Fallback
	st->reg_addr_conv = st->chip->prod_id == 0x7C ? AD4062_REG_CONV_TRIGGER_32BITS :
							AD4062_REG_CONV_TRIGGER_16BITS;
	// GPO IRQ
	st->reg_addr_conv = st->chip->prod_id == 0x7C ? AD4062_REG_CONV_READ_32BITS :
							AD4062_REG_CONV_READ_16BITS;

Then, for sample size:
	const bool is_32b = st->chip->prod_id == 0x7C;
	const size_t _sizeof = is_32b ? sizeof(st->buf.be32) : sizeof(st->buf.be16);
instead of
	const bool is_32b = st->mode == AD4062_BURST_AVERAGING_MODE && st->chip->prod_id == 0x7C;
	const size_t _sizeof = is_32b ? sizeof(st->buf.be32) : sizeof(st->buf.be16);
	+ extra st->reg_addr_conv_avg that may or may not be equal to
	st->reg_addr_conv.

Note that the header section of the I3C transfer (8-bits) occurs
at 1MHz, while the reading in 12.5MHz. I wouldn't go as far as say it is
negligible, but for the part, protocol and software overhead, it
wouldn't provide ground-breaking higher effective maximum
sampling frequency.
> > +		.endianness = IIO_BE,
> > +	},
> > +	[AD4062_SCAN_TYPE_BURST_AVG] = {
> > +		.sign = 's',
> > +		.realbits = 16,
> > +		.storagebits = 32,
> > +		.endianness = IIO_BE,
> > +	},
> > +};
> 
 
> > +
> > +static const struct ad4062_chip_info ad4060_chip_info = {
> > +	.name = "ad4060",
> > +	.channels = { AD4062_CHAN(12) },
> > +	.prod_id = 0x7A,
> > +	.max_avg = AD4060_MAX_AVG,
> 
> This is a little confusing. I guess it's the maximum register value, not the
> number of samples averaged.  Perhaps rename.
This isn't doing much, I will just replace with the value and bump the
type from u8 to u16.
ad4060 max_avg:  256
ad4062 max_avg: 4096
> 
> > +};
> > +
> > +static const struct ad4062_chip_info ad4062_chip_info = {
> > +	.name = "ad4062",
> > +	.channels = { AD4062_CHAN(16) },
> > +	.prod_id = 0x7C,
> > +	.max_avg = AD4062_MAX_AVG,
> > +};
> > +
> > +static int ad4062_set_oversampling_ratio(struct ad4062_state *st, unsigned int val)
> > +{
> > +	const u32 _max = GENMASK(st->chip->max_avg, 0)  + 1;
> 
> One too many spaces before +
> 
Here just
	const u32 _max = st->chip->avg_max;
...
> > +		.data.in = &st->buf.be32,
> > +		.len = sizeof(st->buf.be32),
> > +		.rnw = true,
> > +	};
> > +
> > +	ACQUIRE(pm_runtime_active_try_enabled, pm)(&st->i3cdev->dev);
> 
> Rafael did propose a cleaner way of doing this. I'm not sure if it went
> in during the merge window or not.  Take a look to see if it anything
> has been added in the PM pull request (IIRC Rafael converted all existing
> users to the new scheme in that patch set so should be easy to find.)
> 
Yep got merged.
https://github.com/torvalds/linux/commit/ef8057b07c72a817537856b98d6e7493b9404eaf
Will do
	PM_RUNTIME_ACQUIRE(&st->i3cdev->dev, pm);
	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
	if (ret)
		return ret;

> > +	ret = ACQUIRE_ERR(pm_runtime_active_try_enabled, &pm);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = ad4062_set_operation_mode(st, st->mode);
> > +	if (ret)
> > +		return ret;
> > +
Best regards,
Jorge
Re: [PATCH v3 3/9] iio: adc: Add support for ad4062
Posted by Jorge Marques 1 day, 11 hours ago
On Mon, Dec 08, 2025 at 10:16:35PM +0100, Jorge Marques wrote:
> On Sat, Dec 06, 2025 at 05:34:59PM +0000, Jonathan Cameron wrote:
> > On Fri, 5 Dec 2025 16:12:04 +0100
> > Jorge Marques <jorge.marques@analog.com> wrote:
> > 
> > > The AD4060/AD4062 are versatile, 16-bit/12-bit, successive approximation
> > > register (SAR) analog-to-digital converter (ADC) with low-power and
> > > threshold monitoring modes.
> > > 
> > > Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Hi Jonathan,
> Hi Jonathan,
> ...
> Yes, and for burst avg mode, 14 bits.
> > 
> > > +		.storagebits = 32,
> > Given we are doing data mangling anyway why not store in a 16 bit value.
> > 
> > BTW it would have been easier to spot issues with this if you'd introduced
> > the scan type stuff with the use of scans in the patch that adds buffered
> > support.  So please move this stuff there.
> >
> This can be done, just note that for ad4062 in burst avg mode the
> realbits is 24 bits, so the storagebits is 32 bits only on that case
> and will requires a few conditionals to handle just this case.
> 
> To not overly complicated the logic, for ad4062 I will always read
> 32-bits still. st->reg_addr_conv then takes:
> 	// IBI Fallback
> 	st->reg_addr_conv = st->chip->prod_id == 0x7C ? AD4062_REG_CONV_TRIGGER_32BITS :
> 							AD4062_REG_CONV_TRIGGER_16BITS;
> 	// GPO IRQ
> 	st->reg_addr_conv = st->chip->prod_id == 0x7C ? AD4062_REG_CONV_READ_32BITS :
> 							AD4062_REG_CONV_READ_16BITS;
> 
> Then, for sample size:
> 	const bool is_32b = st->chip->prod_id == 0x7C;
> 	const size_t _sizeof = is_32b ? sizeof(st->buf.be32) : sizeof(st->buf.be16);
> instead of
> 	const bool is_32b = st->mode == AD4062_BURST_AVERAGING_MODE && st->chip->prod_id == 0x7C;
> 	const size_t _sizeof = is_32b ? sizeof(st->buf.be32) : sizeof(st->buf.be16);
> 	+ extra st->reg_addr_conv_avg that may or may not be equal to
> 	st->reg_addr_conv.
> 
> Note that the header section of the I3C transfer (8-bits) occurs
> at 1MHz, while the reading in 12.5MHz. I wouldn't go as far as say it is
> negligible, but for the part, protocol and software overhead, it
> wouldn't provide ground-breaking higher effective maximum
> sampling frequency.

I went back to this and now I am properly using the already set iio_get_current_scan_type
to set the appropriate sample register and storagesize (to reduce the protocol overhead).
Both methods are inline and used once, but I believe having the wrapper
methods makes things clearer.
For read_raw, I am using the non-optimized, 4 bytes trigger_conv, in the
next version.

  /*
   * The AD4062 in burst averaging mode increases realbits from 16-bits to
   * 20-bits, increasing the storagebits from 16-bits to 32-bits.
   */
  static inline size_t ad4062_sizeof_storagebits(struct ad4062_state *st)
  {
  	const struct iio_scan_type *scan_type =
  		iio_get_current_scan_type(st->indio_dev, st->chip->channels);
  
  	return BITS_TO_BYTES(scan_type->storagebits);
  }
  
  /* Read registers only with realbits (no sign extension bytes) */
  static inline size_t ad4062_get_conv_addr(struct ad4062_state *st, size_t _sizeof)
  {
  	if (st->gpo_irq[1])
  		return _sizeof == sizeof(u32) ? AD4062_REG_CONV_READ_32BITS :
  						AD4062_REG_CONV_READ_16BITS;
  	return _sizeof == sizeof(u32) ? AD4062_REG_CONV_TRIGGER_32BITS :
  					AD4062_REG_CONV_TRIGGER_16BITS;
  }
  
  static int pm_ad4062_triggered_buffer_postenable(struct ad4062_state *st)
  {
  	int ret;
  
  	PM_RUNTIME_ACQUIRE(&st->i3cdev->dev, pm);
  	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
  	if (ret)
  		return ret;
  
  	if (st->wait_event)
  		return -EBUSY;
  
  	ret = ad4062_set_operation_mode(st, st->mode);
  	if (ret)
  		return ret;
  
  	st->conv_sizeof = ad4062_sizeof_storagebits(st);
  	st->conv_addr = ad4062_get_conv_addr(st, st->conv_sizeof);
  	...
  }

Best regards,
Jorge