[PATCH v2 0/8] iio: ad9467 and axi-adc cleanups

[PATCH v2 0/8] iio: ad9467 and axi-adc cleanups

[Nuno Sa via B4 Relay]

As requested in [1], this is a preparatory series with some basic cleanups for
the ad9467 and adi-axi-adc drivers.

This means that the iio backend series [2] will depend on this one but
hopefully we'll quickly get this one in.

[1]: https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#ma7ed8a58d747e78e1ff2273c2d6cdd11de22f177
[2]: https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f5175273b81dbfe40b7f0daffcdc67d6cb8ff

---
Changes in v2:
- Patch 1.
  * Use IS_ERR_OR_NULL() and PTR_ERR_OR_ZERO();
  * Improved commit message (mention it's inverted polarity).
- Patch 4:
  * Properly allocate enough memory for the scale array;
  * Use devm_kmalloc_array() as we don't need to zero initialize it;
  * Don't multiple by 2 in the for() loop. We just want to go over the
    rows.
 
- Link to v1: https://lore.kernel.org/r/20231205-iio-backend-prep-v1-0-7c9bc18d612b@analog.com

---
Nuno Sa (8):
      iio: adc: ad9467: fix reset gpio handling
      iio: adc: ad9467: don't ignore error codes
      iio: adc: ad9467: add mutex to struct ad9467_state
      iio: adc: ad9467: fix scale setting
      iio: adc: ad9467: use spi_get_device_match_data()
      iio: adc: ad9467: use chip_info variables instead of array
      iio: adc: ad9467: use the more common !val NULL check
      iio: adc: adi-axi-adc: convert to regmap

 drivers/iio/adc/ad9467.c            | 207 +++++++++++++++++++++++-------------
 drivers/iio/adc/adi-axi-adc.c       | 159 ++++++++++++---------------
 include/linux/iio/adc/adi-axi-adc.h |   4 +
 3 files changed, 204 insertions(+), 166 deletions(-)
---
base-commit: 5c8f90655a7bdb6232b2cea6503df16367b11a53
change-id: 20231205-iio-backend-prep-4d96ef364080
--

Thanks!
- Nuno Sá

[PATCH v2 1/8] iio: adc: ad9467: fix reset gpio handling

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

The reset gpio was being handled with inverted polarity. This means that
as far as gpiolib is concerned we were actually leaving the pin asserted
(in theory, this would mean reset). However, inverting the polarity in
devicetree made things work. Fix it by doing it the proper way and how
gpiolib expects it to be done.

While at it, moved the handling to it's own function and dropped
'reset_gpio' from the 'struct ad9467_state' as we only need it during
probe. On top of that, refactored things so that we now request the gpio
asserted (i.e in reset) and then de-assert it. Also note that we now use
gpiod_set_value_cansleep() instead of gpiod_direction_output() as we
already request the pin as output.

Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/ad9467.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index 39eccc28debe..4fb9e48dc782 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -121,7 +121,6 @@ struct ad9467_state {
 	unsigned int			output_mode;
 
 	struct gpio_desc		*pwrdown_gpio;
-	struct gpio_desc		*reset_gpio;
 };
 
 static int ad9467_spi_read(struct spi_device *spi, unsigned int reg)
@@ -378,6 +377,21 @@ static int ad9467_preenable_setup(struct adi_axi_adc_conv *conv)
 	return ad9467_outputmode_set(st->spi, st->output_mode);
 }
 
+static int ad9467_reset(struct device *dev)
+{
+	struct gpio_desc *gpio;
+
+	gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR_OR_NULL(gpio))
+		return PTR_ERR_OR_ZERO(gpio);
+
+	fsleep(1);
+	gpiod_set_value_cansleep(gpio, 0);
+	fsleep(10 * USEC_PER_MSEC);
+
+	return 0;
+}
+
 static int ad9467_probe(struct spi_device *spi)
 {
 	const struct ad9467_chip_info *info;
@@ -408,18 +422,9 @@ static int ad9467_probe(struct spi_device *spi)
 	if (IS_ERR(st->pwrdown_gpio))
 		return PTR_ERR(st->pwrdown_gpio);
 
-	st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
-						 GPIOD_OUT_LOW);
-	if (IS_ERR(st->reset_gpio))
-		return PTR_ERR(st->reset_gpio);
-
-	if (st->reset_gpio) {
-		udelay(1);
-		ret = gpiod_direction_output(st->reset_gpio, 1);
-		if (ret)
-			return ret;
-		mdelay(10);
-	}
+	ret = ad9467_reset(&spi->dev);
+	if (ret)
+		return ret;
 
 	conv->chip_info = &info->axi_adc_info;
 

-- 
2.43.0

[PATCH v2 2/8] iio: adc: ad9467: don't ignore error codes

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

Make sure functions that return errors are not ignored.

Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/ad9467.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index 4fb9e48dc782..d3c98c4eccd3 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -162,9 +162,10 @@ static int ad9467_reg_access(struct adi_axi_adc_conv *conv, unsigned int reg,
 
 	if (readval == NULL) {
 		ret = ad9467_spi_write(spi, reg, writeval);
-		ad9467_spi_write(spi, AN877_ADC_REG_TRANSFER,
-				 AN877_ADC_TRANSFER_SYNC);
-		return ret;
+		if (ret)
+			return ret;
+		return ad9467_spi_write(spi, AN877_ADC_REG_TRANSFER,
+					AN877_ADC_TRANSFER_SYNC);
 	}
 
 	ret = ad9467_spi_read(spi, reg);
@@ -272,10 +273,13 @@ static int ad9467_get_scale(struct adi_axi_adc_conv *conv, int *val, int *val2)
 	const struct ad9467_chip_info *info1 = to_ad9467_chip_info(info);
 	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
 	unsigned int i, vref_val;
+	int ret;
 
-	vref_val = ad9467_spi_read(st->spi, AN877_ADC_REG_VREF);
+	ret = ad9467_spi_read(st->spi, AN877_ADC_REG_VREF);
+	if (ret < 0)
+		return ret;
 
-	vref_val &= info1->vref_mask;
+	vref_val = ret & info1->vref_mask;
 
 	for (i = 0; i < info->num_scales; i++) {
 		if (vref_val == info->scale_table[i][1])
@@ -296,6 +300,7 @@ static int ad9467_set_scale(struct adi_axi_adc_conv *conv, int val, int val2)
 	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
 	unsigned int scale_val[2];
 	unsigned int i;
+	int ret;
 
 	if (val != 0)
 		return -EINVAL;
@@ -305,11 +310,13 @@ static int ad9467_set_scale(struct adi_axi_adc_conv *conv, int val, int val2)
 		if (scale_val[0] != val || scale_val[1] != val2)
 			continue;
 
-		ad9467_spi_write(st->spi, AN877_ADC_REG_VREF,
-				 info->scale_table[i][1]);
-		ad9467_spi_write(st->spi, AN877_ADC_REG_TRANSFER,
-				 AN877_ADC_TRANSFER_SYNC);
-		return 0;
+		ret = ad9467_spi_write(st->spi, AN877_ADC_REG_VREF,
+				       info->scale_table[i][1]);
+		if (ret < 0)
+			return ret;
+
+		return ad9467_spi_write(st->spi, AN877_ADC_REG_TRANSFER,
+					AN877_ADC_TRANSFER_SYNC);
 	}
 
 	return -EINVAL;

-- 
2.43.0

[PATCH v2 3/8] iio: adc: ad9467: add mutex to struct ad9467_state

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

When calling ad9467_set_scale(), multiple calls to ad9467_spi_write()
are done which means we need to properly protect the whole operation so
we are sure we will be in a sane state if two concurrent calls occur.

Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/ad9467.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index d3c98c4eccd3..104c6d394a3e 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -4,8 +4,9 @@
  *
  * Copyright 2012-2020 Analog Devices Inc.
  */
-
+#include <linux/cleanup.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
@@ -121,6 +122,8 @@ struct ad9467_state {
 	unsigned int			output_mode;
 
 	struct gpio_desc		*pwrdown_gpio;
+	/* ensure consistent state obtained on multiple related accesses */
+	struct mutex			lock;
 };
 
 static int ad9467_spi_read(struct spi_device *spi, unsigned int reg)
@@ -161,6 +164,7 @@ static int ad9467_reg_access(struct adi_axi_adc_conv *conv, unsigned int reg,
 	int ret;
 
 	if (readval == NULL) {
+		guard(mutex)(&st->lock);
 		ret = ad9467_spi_write(spi, reg, writeval);
 		if (ret)
 			return ret;
@@ -310,6 +314,7 @@ static int ad9467_set_scale(struct adi_axi_adc_conv *conv, int val, int val2)
 		if (scale_val[0] != val || scale_val[1] != val2)
 			continue;
 
+		guard(mutex)(&st->lock);
 		ret = ad9467_spi_write(st->spi, AN877_ADC_REG_VREF,
 				       info->scale_table[i][1]);
 		if (ret < 0)

-- 
2.43.0

Re: [PATCH v2 3/8] iio: adc: ad9467: add mutex to struct ad9467_state

[David Lechner]

On Thu, Dec 7, 2023 at 6:39 AM Nuno Sa via B4 Relay
<devnull+nuno.sa.analog.com@kernel.org> wrote:
>
> From: Nuno Sa <nuno.sa@analog.com>
>
> When calling ad9467_set_scale(), multiple calls to ad9467_spi_write()
> are done which means we need to properly protect the whole operation so
> we are sure we will be in a sane state if two concurrent calls occur.
>
> Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---

Reviewed-by: David Lechner <dlechner@baylibre.com>

[PATCH v2 4/8] iio: adc: ad9467: fix scale setting

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

When reading in_voltage_scale we can get something like:

root@analog:/sys/bus/iio/devices/iio:device2# cat in_voltage_scale
0.038146

However, when reading the available options:

root@analog:/sys/bus/iio/devices/iio:device2# cat
in_voltage_scale_available
2000.000000 2100.000006 2200.000007 2300.000008 2400.000009 2500.000010

which does not make sense. Moreover, when trying to set a new scale we
get an error because there's no call to __ad9467_get_scale() to give us
values as given when reading in_voltage_scale. Fix it by computing the
available scales during probe and properly pass the list when
.read_available() is called.

While at it, change to use .read_available() from iio_info. Also note
that to properly fix this, adi-axi-adc.c has to be changed accordingly.

Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/ad9467.c            | 47 +++++++++++++++++++++++
 drivers/iio/adc/adi-axi-adc.c       | 74 ++++++++-----------------------------
 include/linux/iio/adc/adi-axi-adc.h |  4 ++
 3 files changed, 66 insertions(+), 59 deletions(-)

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index 104c6d394a3e..f668313730cb 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -120,6 +120,7 @@ struct ad9467_state {
 	struct spi_device		*spi;
 	struct clk			*clk;
 	unsigned int			output_mode;
+	unsigned int                    (*scales)[2];
 
 	struct gpio_desc		*pwrdown_gpio;
 	/* ensure consistent state obtained on multiple related accesses */
@@ -216,6 +217,7 @@ static void __ad9467_get_scale(struct adi_axi_adc_conv *conv, int index,
 	.channel = _chan,						\
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |		\
 		BIT(IIO_CHAN_INFO_SAMP_FREQ),				\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE), \
 	.scan_index = _si,						\
 	.scan_type = {							\
 		.sign = _sign,						\
@@ -370,6 +372,26 @@ static int ad9467_write_raw(struct adi_axi_adc_conv *conv,
 	}
 }
 
+static int ad9467_read_avail(struct adi_axi_adc_conv *conv,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	const struct adi_axi_adc_chip_info *info = conv->chip_info;
+	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (const int *)st->scales;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		/* Values are stored in a 2D matrix */
+		*length = info->num_scales * 2;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int ad9467_outputmode_set(struct spi_device *spi, unsigned int mode)
 {
 	int ret;
@@ -382,6 +404,26 @@ static int ad9467_outputmode_set(struct spi_device *spi, unsigned int mode)
 				AN877_ADC_TRANSFER_SYNC);
 }
 
+static int ad9467_scale_fill(struct adi_axi_adc_conv *conv)
+{
+	const struct adi_axi_adc_chip_info *info = conv->chip_info;
+	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
+	unsigned int i, val1, val2;
+
+	st->scales = devm_kmalloc_array(&st->spi->dev, info->num_scales,
+					sizeof(*st->scales), GFP_KERNEL);
+	if (!st->scales)
+		return -ENOMEM;
+
+	for (i = 0; i < info->num_scales; i++) {
+		__ad9467_get_scale(conv, i, &val1, &val2);
+		st->scales[i][0] = val1;
+		st->scales[i][1] = val2;
+	}
+
+	return 0;
+}
+
 static int ad9467_preenable_setup(struct adi_axi_adc_conv *conv)
 {
 	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
@@ -440,6 +482,10 @@ static int ad9467_probe(struct spi_device *spi)
 
 	conv->chip_info = &info->axi_adc_info;
 
+	ret = ad9467_scale_fill(conv);
+	if (ret)
+		return ret;
+
 	id = ad9467_spi_read(spi, AN877_ADC_REG_CHIP_ID);
 	if (id != conv->chip_info->id) {
 		dev_err(&spi->dev, "Mismatch CHIP_ID, got 0x%X, expected 0x%X\n",
@@ -450,6 +496,7 @@ static int ad9467_probe(struct spi_device *spi)
 	conv->reg_access = ad9467_reg_access;
 	conv->write_raw = ad9467_write_raw;
 	conv->read_raw = ad9467_read_raw;
+	conv->read_avail = ad9467_read_avail;
 	conv->preenable_setup = ad9467_preenable_setup;
 
 	st->output_mode = info->default_output_mode |
diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index aff0532a974a..ae83ada7f9f2 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -144,6 +144,20 @@ static int adi_axi_adc_write_raw(struct iio_dev *indio_dev,
 	return conv->write_raw(conv, chan, val, val2, mask);
 }
 
+static int adi_axi_adc_read_avail(struct iio_dev *indio_dev,
+				  struct iio_chan_spec const *chan,
+				  const int **vals, int *type, int *length,
+				  long mask)
+{
+	struct adi_axi_adc_state *st = iio_priv(indio_dev);
+	struct adi_axi_adc_conv *conv = &st->client->conv;
+
+	if (!conv->read_avail)
+		return -EOPNOTSUPP;
+
+	return conv->read_avail(conv, chan, vals, type, length, mask);
+}
+
 static int adi_axi_adc_update_scan_mode(struct iio_dev *indio_dev,
 					const unsigned long *scan_mask)
 {
@@ -228,69 +242,11 @@ struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,
 }
 EXPORT_SYMBOL_NS_GPL(devm_adi_axi_adc_conv_register, IIO_ADI_AXI);
 
-static ssize_t in_voltage_scale_available_show(struct device *dev,
-					       struct device_attribute *attr,
-					       char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct adi_axi_adc_state *st = iio_priv(indio_dev);
-	struct adi_axi_adc_conv *conv = &st->client->conv;
-	size_t len = 0;
-	int i;
-
-	for (i = 0; i < conv->chip_info->num_scales; i++) {
-		const unsigned int *s = conv->chip_info->scale_table[i];
-
-		len += scnprintf(buf + len, PAGE_SIZE - len,
-				 "%u.%06u ", s[0], s[1]);
-	}
-	buf[len - 1] = '\n';
-
-	return len;
-}
-
-static IIO_DEVICE_ATTR_RO(in_voltage_scale_available, 0);
-
-enum {
-	ADI_AXI_ATTR_SCALE_AVAIL,
-};
-
-#define ADI_AXI_ATTR(_en_, _file_)			\
-	[ADI_AXI_ATTR_##_en_] = &iio_dev_attr_##_file_.dev_attr.attr
-
-static struct attribute *adi_axi_adc_attributes[] = {
-	ADI_AXI_ATTR(SCALE_AVAIL, in_voltage_scale_available),
-	NULL
-};
-
-static umode_t axi_adc_attr_is_visible(struct kobject *kobj,
-				       struct attribute *attr, int n)
-{
-	struct device *dev = kobj_to_dev(kobj);
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct adi_axi_adc_state *st = iio_priv(indio_dev);
-	struct adi_axi_adc_conv *conv = &st->client->conv;
-
-	switch (n) {
-	case ADI_AXI_ATTR_SCALE_AVAIL:
-		if (!conv->chip_info->num_scales)
-			return 0;
-		return attr->mode;
-	default:
-		return attr->mode;
-	}
-}
-
-static const struct attribute_group adi_axi_adc_attribute_group = {
-	.attrs = adi_axi_adc_attributes,
-	.is_visible = axi_adc_attr_is_visible,
-};
-
 static const struct iio_info adi_axi_adc_info = {
 	.read_raw = &adi_axi_adc_read_raw,
 	.write_raw = &adi_axi_adc_write_raw,
-	.attrs = &adi_axi_adc_attribute_group,
 	.update_scan_mode = &adi_axi_adc_update_scan_mode,
+	.read_avail = &adi_axi_adc_read_avail,
 };
 
 static const struct adi_axi_adc_core_info adi_axi_adc_10_0_a_info = {
diff --git a/include/linux/iio/adc/adi-axi-adc.h b/include/linux/iio/adc/adi-axi-adc.h
index 52620e5b8052..b7904992d561 100644
--- a/include/linux/iio/adc/adi-axi-adc.h
+++ b/include/linux/iio/adc/adi-axi-adc.h
@@ -41,6 +41,7 @@ struct adi_axi_adc_chip_info {
  * @reg_access		IIO debugfs_reg_access hook for the client ADC
  * @read_raw		IIO read_raw hook for the client ADC
  * @write_raw		IIO write_raw hook for the client ADC
+ * @read_avail		IIO read_avail hook for the client ADC
  */
 struct adi_axi_adc_conv {
 	const struct adi_axi_adc_chip_info		*chip_info;
@@ -54,6 +55,9 @@ struct adi_axi_adc_conv {
 	int (*write_raw)(struct adi_axi_adc_conv *conv,
 			 struct iio_chan_spec const *chan,
 			 int val, int val2, long mask);
+	int (*read_avail)(struct adi_axi_adc_conv *conv,
+			  struct iio_chan_spec const *chan,
+			  const int **val, int *type, int *length, long mask);
 };
 
 struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,

-- 
2.43.0

Re: [PATCH v2 4/8] iio: adc: ad9467: fix scale setting

[David Lechner]

On Thu, Dec 7, 2023 at 6:39 AM Nuno Sa via B4 Relay
<devnull+nuno.sa.analog.com@kernel.org> wrote:
>
> From: Nuno Sa <nuno.sa@analog.com>
>
> When reading in_voltage_scale we can get something like:
>
> root@analog:/sys/bus/iio/devices/iio:device2# cat in_voltage_scale
> 0.038146
>
> However, when reading the available options:
>
> root@analog:/sys/bus/iio/devices/iio:device2# cat
> in_voltage_scale_available
> 2000.000000 2100.000006 2200.000007 2300.000008 2400.000009 2500.000010
>
> which does not make sense. Moreover, when trying to set a new scale we
> get an error because there's no call to __ad9467_get_scale() to give us
> values as given when reading in_voltage_scale. Fix it by computing the
> available scales during probe and properly pass the list when
> .read_available() is called.
>
> While at it, change to use .read_available() from iio_info. Also note
> that to properly fix this, adi-axi-adc.c has to be changed accordingly.
>
> Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---

Reviewed-by: David Lechner <dlechner@baylibre.com>

[PATCH v2 5/8] iio: adc: ad9467: use spi_get_device_match_data()

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

Make use of spi_get_device_match_data() to simplify things.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/ad9467.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index f668313730cb..b16d28c1adcb 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -454,9 +454,7 @@ static int ad9467_probe(struct spi_device *spi)
 	unsigned int id;
 	int ret;
 
-	info = of_device_get_match_data(&spi->dev);
-	if (!info)
-		info = (void *)spi_get_device_id(spi)->driver_data;
+	info = spi_get_device_match_data(spi);
 	if (!info)
 		return -ENODEV;
 

-- 
2.43.0

[PATCH v2 6/8] iio: adc: ad9467: use chip_info variables instead of array

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

Instead of having an array and keeping IDs for each entry of the array,
just have a chip_info struct per device.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/ad9467.c | 89 +++++++++++++++++++++++-------------------------
 1 file changed, 43 insertions(+), 46 deletions(-)

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index b16d28c1adcb..c5ed62cc8646 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -101,12 +101,6 @@
 #define AD9467_DEF_OUTPUT_MODE		0x08
 #define AD9467_REG_VREF_MASK		0x0F
 
-enum {
-	ID_AD9265,
-	ID_AD9434,
-	ID_AD9467,
-};
-
 struct ad9467_chip_info {
 	struct adi_axi_adc_chip_info	axi_adc_info;
 	unsigned int			default_output_mode;
@@ -234,43 +228,46 @@ static const struct iio_chan_spec ad9467_channels[] = {
 	AD9467_CHAN(0, 0, 16, 'S'),
 };
 
-static const struct ad9467_chip_info ad9467_chip_tbl[] = {
-	[ID_AD9265] = {
-		.axi_adc_info = {
-			.id = CHIPID_AD9265,
-			.max_rate = 125000000UL,
-			.scale_table = ad9265_scale_table,
-			.num_scales = ARRAY_SIZE(ad9265_scale_table),
-			.channels = ad9467_channels,
-			.num_channels = ARRAY_SIZE(ad9467_channels),
-		},
-		.default_output_mode = AD9265_DEF_OUTPUT_MODE,
-		.vref_mask = AD9265_REG_VREF_MASK,
+static const struct ad9467_chip_info ad9467_chip_tbl = {
+	.axi_adc_info = {
+		.name = "ad9467",
+		.id = CHIPID_AD9467,
+		.max_rate = 250000000UL,
+		.scale_table = ad9467_scale_table,
+		.num_scales = ARRAY_SIZE(ad9467_scale_table),
+		.channels = ad9467_channels,
+		.num_channels = ARRAY_SIZE(ad9467_channels),
 	},
-	[ID_AD9434] = {
-		.axi_adc_info = {
-			.id = CHIPID_AD9434,
-			.max_rate = 500000000UL,
-			.scale_table = ad9434_scale_table,
-			.num_scales = ARRAY_SIZE(ad9434_scale_table),
-			.channels = ad9434_channels,
-			.num_channels = ARRAY_SIZE(ad9434_channels),
-		},
-		.default_output_mode = AD9434_DEF_OUTPUT_MODE,
-		.vref_mask = AD9434_REG_VREF_MASK,
+	.default_output_mode = AD9467_DEF_OUTPUT_MODE,
+	.vref_mask = AD9467_REG_VREF_MASK,
+};
+
+static const struct ad9467_chip_info ad9434_chip_tbl = {
+	.axi_adc_info = {
+		.name = "ad9434",
+		.id = CHIPID_AD9434,
+		.max_rate = 500000000UL,
+		.scale_table = ad9434_scale_table,
+		.num_scales = ARRAY_SIZE(ad9434_scale_table),
+		.channels = ad9434_channels,
+		.num_channels = ARRAY_SIZE(ad9434_channels),
 	},
-	[ID_AD9467] = {
-		.axi_adc_info = {
-			.id = CHIPID_AD9467,
-			.max_rate = 250000000UL,
-			.scale_table = ad9467_scale_table,
-			.num_scales = ARRAY_SIZE(ad9467_scale_table),
-			.channels = ad9467_channels,
-			.num_channels = ARRAY_SIZE(ad9467_channels),
-		},
-		.default_output_mode = AD9467_DEF_OUTPUT_MODE,
-		.vref_mask = AD9467_REG_VREF_MASK,
+	.default_output_mode = AD9434_DEF_OUTPUT_MODE,
+	.vref_mask = AD9434_REG_VREF_MASK,
+};
+
+static const struct ad9467_chip_info ad9265_chip_tbl = {
+	.axi_adc_info = {
+		.name = "ad9265",
+		.id = CHIPID_AD9265,
+		.max_rate = 125000000UL,
+		.scale_table = ad9265_scale_table,
+		.num_scales = ARRAY_SIZE(ad9265_scale_table),
+		.channels = ad9467_channels,
+		.num_channels = ARRAY_SIZE(ad9467_channels),
 	},
+	.default_output_mode = AD9265_DEF_OUTPUT_MODE,
+	.vref_mask = AD9265_REG_VREF_MASK,
 };
 
 static int ad9467_get_scale(struct adi_axi_adc_conv *conv, int *val, int *val2)
@@ -504,17 +501,17 @@ static int ad9467_probe(struct spi_device *spi)
 }
 
 static const struct of_device_id ad9467_of_match[] = {
-	{ .compatible = "adi,ad9265", .data = &ad9467_chip_tbl[ID_AD9265], },
-	{ .compatible = "adi,ad9434", .data = &ad9467_chip_tbl[ID_AD9434], },
-	{ .compatible = "adi,ad9467", .data = &ad9467_chip_tbl[ID_AD9467], },
+	{ .compatible = "adi,ad9265", .data = &ad9265_chip_tbl, },
+	{ .compatible = "adi,ad9434", .data = &ad9434_chip_tbl, },
+	{ .compatible = "adi,ad9467", .data = &ad9467_chip_tbl, },
 	{}
 };
 MODULE_DEVICE_TABLE(of, ad9467_of_match);
 
 static const struct spi_device_id ad9467_ids[] = {
-	{ "ad9265", (kernel_ulong_t)&ad9467_chip_tbl[ID_AD9265] },
-	{ "ad9434", (kernel_ulong_t)&ad9467_chip_tbl[ID_AD9434] },
-	{ "ad9467", (kernel_ulong_t)&ad9467_chip_tbl[ID_AD9467] },
+	{ "ad9265", (kernel_ulong_t)&ad9265_chip_tbl },
+	{ "ad9434", (kernel_ulong_t)&ad9434_chip_tbl },
+	{ "ad9467", (kernel_ulong_t)&ad9467_chip_tbl },
 	{}
 };
 MODULE_DEVICE_TABLE(spi, ad9467_ids);

-- 
2.43.0

[PATCH v2 7/8] iio: adc: ad9467: use the more common !val NULL check

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

Check !val instead of directing checking for NULL (val == NULL).
No functional changes intended.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/ad9467.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index c5ed62cc8646..6581fce4ba95 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -158,7 +158,7 @@ static int ad9467_reg_access(struct adi_axi_adc_conv *conv, unsigned int reg,
 	struct spi_device *spi = st->spi;
 	int ret;
 
-	if (readval == NULL) {
+	if (!readval) {
 		guard(mutex)(&st->lock);
 		ret = ad9467_spi_write(spi, reg, writeval);
 		if (ret)

-- 
2.43.0

[PATCH v2 8/8] iio: adc: adi-axi-adc: convert to regmap

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

Use MMIO regmap interface. It makes things easier for manipulating bits.

Reviewed-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/adi-axi-adc.c | 85 ++++++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index ae83ada7f9f2..c247ff1541d2 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
 #include <linux/iio/iio.h>
@@ -62,7 +63,7 @@ struct adi_axi_adc_state {
 	struct mutex				lock;
 
 	struct adi_axi_adc_client		*client;
-	void __iomem				*regs;
+	struct regmap				*regmap;
 };
 
 struct adi_axi_adc_client {
@@ -90,19 +91,6 @@ void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv)
 }
 EXPORT_SYMBOL_NS_GPL(adi_axi_adc_conv_priv, IIO_ADI_AXI);
 
-static void adi_axi_adc_write(struct adi_axi_adc_state *st,
-			      unsigned int reg,
-			      unsigned int val)
-{
-	iowrite32(val, st->regs + reg);
-}
-
-static unsigned int adi_axi_adc_read(struct adi_axi_adc_state *st,
-				     unsigned int reg)
-{
-	return ioread32(st->regs + reg);
-}
-
 static int adi_axi_adc_config_dma_buffer(struct device *dev,
 					 struct iio_dev *indio_dev)
 {
@@ -163,17 +151,20 @@ static int adi_axi_adc_update_scan_mode(struct iio_dev *indio_dev,
 {
 	struct adi_axi_adc_state *st = iio_priv(indio_dev);
 	struct adi_axi_adc_conv *conv = &st->client->conv;
-	unsigned int i, ctrl;
+	unsigned int i;
+	int ret;
 
 	for (i = 0; i < conv->chip_info->num_channels; i++) {
-		ctrl = adi_axi_adc_read(st, ADI_AXI_REG_CHAN_CTRL(i));
-
 		if (test_bit(i, scan_mask))
-			ctrl |= ADI_AXI_REG_CHAN_CTRL_ENABLE;
+			ret = regmap_set_bits(st->regmap,
+					      ADI_AXI_REG_CHAN_CTRL(i),
+					      ADI_AXI_REG_CHAN_CTRL_ENABLE);
 		else
-			ctrl &= ~ADI_AXI_REG_CHAN_CTRL_ENABLE;
-
-		adi_axi_adc_write(st, ADI_AXI_REG_CHAN_CTRL(i), ctrl);
+			ret = regmap_clear_bits(st->regmap,
+						ADI_AXI_REG_CHAN_CTRL(i),
+						ADI_AXI_REG_CHAN_CTRL_ENABLE);
+		if (ret)
+			return ret;
 	}
 
 	return 0;
@@ -310,21 +301,32 @@ static int adi_axi_adc_setup_channels(struct device *dev,
 	}
 
 	for (i = 0; i < conv->chip_info->num_channels; i++) {
-		adi_axi_adc_write(st, ADI_AXI_REG_CHAN_CTRL(i),
-				  ADI_AXI_REG_CHAN_CTRL_DEFAULTS);
+		ret = regmap_write(st->regmap, ADI_AXI_REG_CHAN_CTRL(i),
+				   ADI_AXI_REG_CHAN_CTRL_DEFAULTS);
+		if (ret)
+			return ret;
 	}
 
 	return 0;
 }
 
-static void axi_adc_reset(struct adi_axi_adc_state *st)
+static int axi_adc_reset(struct adi_axi_adc_state *st)
 {
-	adi_axi_adc_write(st, ADI_AXI_REG_RSTN, 0);
+	int ret;
+
+	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN, 0);
+	if (ret)
+		return ret;
+
 	mdelay(10);
-	adi_axi_adc_write(st, ADI_AXI_REG_RSTN, ADI_AXI_REG_RSTN_MMCM_RSTN);
+	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN,
+			   ADI_AXI_REG_RSTN_MMCM_RSTN);
+	if (ret)
+		return ret;
+
 	mdelay(10);
-	adi_axi_adc_write(st, ADI_AXI_REG_RSTN,
-			  ADI_AXI_REG_RSTN_RSTN | ADI_AXI_REG_RSTN_MMCM_RSTN);
+	return regmap_write(st->regmap, ADI_AXI_REG_RSTN,
+			    ADI_AXI_REG_RSTN_RSTN | ADI_AXI_REG_RSTN_MMCM_RSTN);
 }
 
 static void adi_axi_adc_cleanup(void *data)
@@ -335,12 +337,20 @@ static void adi_axi_adc_cleanup(void *data)
 	module_put(cl->dev->driver->owner);
 }
 
+static const struct regmap_config axi_adc_regmap_config = {
+	.val_bits = 32,
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.max_register = 0x0800,
+};
+
 static int adi_axi_adc_probe(struct platform_device *pdev)
 {
 	struct adi_axi_adc_conv *conv;
 	struct iio_dev *indio_dev;
 	struct adi_axi_adc_client *cl;
 	struct adi_axi_adc_state *st;
+	void __iomem *base;
 	unsigned int ver;
 	int ret;
 
@@ -361,15 +371,24 @@ static int adi_axi_adc_probe(struct platform_device *pdev)
 	cl->state = st;
 	mutex_init(&st->lock);
 
-	st->regs = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(st->regs))
-		return PTR_ERR(st->regs);
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	st->regmap = devm_regmap_init_mmio(&pdev->dev, base,
+					   &axi_adc_regmap_config);
+	if (IS_ERR(st->regmap))
+		return PTR_ERR(st->regmap);
 
 	conv = &st->client->conv;
 
-	axi_adc_reset(st);
+	ret = axi_adc_reset(st);
+	if (ret)
+		return ret;
 
-	ver = adi_axi_adc_read(st, ADI_AXI_REG_VERSION);
+	ret = regmap_read(st->regmap, ADI_AXI_REG_VERSION, &ver);
+	if (ret)
+		return ret;
 
 	if (cl->info->version > ver) {
 		dev_err(&pdev->dev,

-- 
2.43.0

Re: [PATCH v2 8/8] iio: adc: adi-axi-adc: convert to regmap

[Nuno Sá]

On Thu, 2023-12-07 at 13:39 +0100, Nuno Sa via B4 Relay wrote:
> From: Nuno Sa <nuno.sa@analog.com>
> 
> Use MMIO regmap interface. It makes things easier for manipulating bits.
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
>  drivers/iio/adc/adi-axi-adc.c | 85 ++++++++++++++++++++++++++----------------
> -
>  1 file changed, 52 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> index ae83ada7f9f2..c247ff1541d2 100644
> --- a/drivers/iio/adc/adi-axi-adc.c
> +++ b/drivers/iio/adc/adi-axi-adc.c
> @@ -14,6 +14,7 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
> +#include <linux/regmap.h>
>  #include <linux/slab.h>
>  
>  #include <linux/iio/iio.h>
> @@ -62,7 +63,7 @@ struct adi_axi_adc_state {
>  	struct mutex				lock;
>  
>  	struct adi_axi_adc_client		*client;
> -	void __iomem				*regs;
> +	struct regmap				*regmap;
>  };
>  
>  struct adi_axi_adc_client {
> @@ -90,19 +91,6 @@ void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv)
>  }
>  EXPORT_SYMBOL_NS_GPL(adi_axi_adc_conv_priv, IIO_ADI_AXI);
>  
> -static void adi_axi_adc_write(struct adi_axi_adc_state *st,
> -			      unsigned int reg,
> -			      unsigned int val)
> -{
> -	iowrite32(val, st->regs + reg);
> -}
> -
> -static unsigned int adi_axi_adc_read(struct adi_axi_adc_state *st,
> -				     unsigned int reg)
> -{
> -	return ioread32(st->regs + reg);
> -}
> -
>  static int adi_axi_adc_config_dma_buffer(struct device *dev,
>  					 struct iio_dev *indio_dev)
>  {
> @@ -163,17 +151,20 @@ static int adi_axi_adc_update_scan_mode(struct iio_dev
> *indio_dev,
>  {
>  	struct adi_axi_adc_state *st = iio_priv(indio_dev);
>  	struct adi_axi_adc_conv *conv = &st->client->conv;
> -	unsigned int i, ctrl;
> +	unsigned int i;
> +	int ret;
>  
>  	for (i = 0; i < conv->chip_info->num_channels; i++) {
> -		ctrl = adi_axi_adc_read(st, ADI_AXI_REG_CHAN_CTRL(i));
> -
>  		if (test_bit(i, scan_mask))
> -			ctrl |= ADI_AXI_REG_CHAN_CTRL_ENABLE;
> +			ret = regmap_set_bits(st->regmap,
> +					      ADI_AXI_REG_CHAN_CTRL(i),
> +					      ADI_AXI_REG_CHAN_CTRL_ENABLE);
>  		else
> -			ctrl &= ~ADI_AXI_REG_CHAN_CTRL_ENABLE;
> -
> -		adi_axi_adc_write(st, ADI_AXI_REG_CHAN_CTRL(i), ctrl);
> +			ret = regmap_clear_bits(st->regmap,
> +						ADI_AXI_REG_CHAN_CTRL(i),
> +						ADI_AXI_REG_CHAN_CTRL_ENABLE)
> ;
> +		if (ret)
> +			return ret;
>  	}
>  
>  	return 0;
> @@ -310,21 +301,32 @@ static int adi_axi_adc_setup_channels(struct device
> *dev,
>  	}
>  
>  	for (i = 0; i < conv->chip_info->num_channels; i++) {
> -		adi_axi_adc_write(st, ADI_AXI_REG_CHAN_CTRL(i),
> -				  ADI_AXI_REG_CHAN_CTRL_DEFAULTS);
> +		ret = regmap_write(st->regmap, ADI_AXI_REG_CHAN_CTRL(i),
> +				   ADI_AXI_REG_CHAN_CTRL_DEFAULTS);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	return 0;
>  }
>  
> -static void axi_adc_reset(struct adi_axi_adc_state *st)
> +static int axi_adc_reset(struct adi_axi_adc_state *st)
>  {
> -	adi_axi_adc_write(st, ADI_AXI_REG_RSTN, 0);
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN, 0);
> +	if (ret)
> +		return ret;
> +
>  	mdelay(10);
> -	adi_axi_adc_write(st, ADI_AXI_REG_RSTN, ADI_AXI_REG_RSTN_MMCM_RSTN);
> +	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN,
> +			   ADI_AXI_REG_RSTN_MMCM_RSTN);
> +	if (ret)
> +		return ret;
> +
>  	mdelay(10);
> -	adi_axi_adc_write(st, ADI_AXI_REG_RSTN,
> -			  ADI_AXI_REG_RSTN_RSTN |
> ADI_AXI_REG_RSTN_MMCM_RSTN);
> +	return regmap_write(st->regmap, ADI_AXI_REG_RSTN,
> +			    ADI_AXI_REG_RSTN_RSTN |
> ADI_AXI_REG_RSTN_MMCM_RSTN);
>  }
>  
>  static void adi_axi_adc_cleanup(void *data)
> @@ -335,12 +337,20 @@ static void adi_axi_adc_cleanup(void *data)
>  	module_put(cl->dev->driver->owner);
>  }
>  
> +static const struct regmap_config axi_adc_regmap_config = {
> +	.val_bits = 32,
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = 0x0800,
> +};
> +
>  static int adi_axi_adc_probe(struct platform_device *pdev)
>  {
>  	struct adi_axi_adc_conv *conv;
>  	struct iio_dev *indio_dev;
>  	struct adi_axi_adc_client *cl;
>  	struct adi_axi_adc_state *st;
> +	void __iomem *base;
>  	unsigned int ver;
>  	int ret;
>  
> @@ -361,15 +371,24 @@ static int adi_axi_adc_probe(struct platform_device
> *pdev)
>  	cl->state = st;
>  	mutex_init(&st->lock);
>  
> -	st->regs = devm_platform_ioremap_resource(pdev, 0);
> -	if (IS_ERR(st->regs))
> -		return PTR_ERR(st->regs);
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	st->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> +					   &axi_adc_regmap_config);
> +	if (IS_ERR(st->regmap))
> +		return PTR_ERR(st->regmap);
>  
>  	conv = &st->client->conv;
>  
> -	axi_adc_reset(st);
> +	ret = axi_adc_reset(st);
> +	if (ret)
> +		return ret;
>  
> -	ver = adi_axi_adc_read(st, ADI_AXI_REG_VERSION);
> +	ret = regmap_read(st->regmap, ADI_AXI_REG_VERSION, &ver);
> +	if (ret)
> +		return ret;
>  
>  	if (cl->info->version > ver) {
>  		dev_err(&pdev->dev,
> 

Hi Jonathan,

I'm not seeing this series yet applied in the togreg branch so maybe I'm still
on time. This patch is missing the proper Kconfig change:


diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index af56df63beff..10e0e340cdae 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -292,7 +292,7 @@ config ADI_AXI_ADC
        select IIO_BUFFER
        select IIO_BUFFER_HW_CONSUMER
        select IIO_BUFFER_DMAENGINE
-       depends on HAS_IOMEM
+       select REGMAP_MMIO
        depends on OF
        help
          Say yes here to build support for Analog Devices Generic


Just realized this when working on the v3 for IIO backends. Please let me know
if I should send a follow up patch or if I can still send a v3 on the
"iio: ad9467 and axi-adc cleanups" series.

- Nuno Sá

Re: [PATCH v2 8/8] iio: adc: adi-axi-adc: convert to regmap

[Jonathan Cameron]

On Mon, 11 Dec 2023 14:18:43 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Thu, 2023-12-07 at 13:39 +0100, Nuno Sa via B4 Relay wrote:
> > From: Nuno Sa <nuno.sa@analog.com>
> > 
> > Use MMIO regmap interface. It makes things easier for manipulating bits.
> > 
> > Reviewed-by: David Lechner <dlechner@baylibre.com>
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> >  drivers/iio/adc/adi-axi-adc.c | 85 ++++++++++++++++++++++++++----------------
> > -
> >  1 file changed, 52 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> > index ae83ada7f9f2..c247ff1541d2 100644
> > --- a/drivers/iio/adc/adi-axi-adc.c
> > +++ b/drivers/iio/adc/adi-axi-adc.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/of.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/property.h>
> > +#include <linux/regmap.h>
> >  #include <linux/slab.h>
> >  
> >  #include <linux/iio/iio.h>
> > @@ -62,7 +63,7 @@ struct adi_axi_adc_state {
> >  	struct mutex				lock;
> >  
> >  	struct adi_axi_adc_client		*client;
> > -	void __iomem				*regs;
> > +	struct regmap				*regmap;
> >  };
> >  
> >  struct adi_axi_adc_client {
> > @@ -90,19 +91,6 @@ void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv)
> >  }
> >  EXPORT_SYMBOL_NS_GPL(adi_axi_adc_conv_priv, IIO_ADI_AXI);
> >  
> > -static void adi_axi_adc_write(struct adi_axi_adc_state *st,
> > -			      unsigned int reg,
> > -			      unsigned int val)
> > -{
> > -	iowrite32(val, st->regs + reg);
> > -}
> > -
> > -static unsigned int adi_axi_adc_read(struct adi_axi_adc_state *st,
> > -				     unsigned int reg)
> > -{
> > -	return ioread32(st->regs + reg);
> > -}
> > -
> >  static int adi_axi_adc_config_dma_buffer(struct device *dev,
> >  					 struct iio_dev *indio_dev)
> >  {
> > @@ -163,17 +151,20 @@ static int adi_axi_adc_update_scan_mode(struct iio_dev
> > *indio_dev,
> >  {
> >  	struct adi_axi_adc_state *st = iio_priv(indio_dev);
> >  	struct adi_axi_adc_conv *conv = &st->client->conv;
> > -	unsigned int i, ctrl;
> > +	unsigned int i;
> > +	int ret;
> >  
> >  	for (i = 0; i < conv->chip_info->num_channels; i++) {
> > -		ctrl = adi_axi_adc_read(st, ADI_AXI_REG_CHAN_CTRL(i));
> > -
> >  		if (test_bit(i, scan_mask))
> > -			ctrl |= ADI_AXI_REG_CHAN_CTRL_ENABLE;
> > +			ret = regmap_set_bits(st->regmap,
> > +					      ADI_AXI_REG_CHAN_CTRL(i),
> > +					      ADI_AXI_REG_CHAN_CTRL_ENABLE);
> >  		else
> > -			ctrl &= ~ADI_AXI_REG_CHAN_CTRL_ENABLE;
> > -
> > -		adi_axi_adc_write(st, ADI_AXI_REG_CHAN_CTRL(i), ctrl);
> > +			ret = regmap_clear_bits(st->regmap,
> > +						ADI_AXI_REG_CHAN_CTRL(i),
> > +						ADI_AXI_REG_CHAN_CTRL_ENABLE)
> > ;
> > +		if (ret)
> > +			return ret;
> >  	}
> >  
> >  	return 0;
> > @@ -310,21 +301,32 @@ static int adi_axi_adc_setup_channels(struct device
> > *dev,
> >  	}
> >  
> >  	for (i = 0; i < conv->chip_info->num_channels; i++) {
> > -		adi_axi_adc_write(st, ADI_AXI_REG_CHAN_CTRL(i),
> > -				  ADI_AXI_REG_CHAN_CTRL_DEFAULTS);
> > +		ret = regmap_write(st->regmap, ADI_AXI_REG_CHAN_CTRL(i),
> > +				   ADI_AXI_REG_CHAN_CTRL_DEFAULTS);
> > +		if (ret)
> > +			return ret;
> >  	}
> >  
> >  	return 0;
> >  }
> >  
> > -static void axi_adc_reset(struct adi_axi_adc_state *st)
> > +static int axi_adc_reset(struct adi_axi_adc_state *st)
> >  {
> > -	adi_axi_adc_write(st, ADI_AXI_REG_RSTN, 0);
> > +	int ret;
> > +
> > +	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN, 0);
> > +	if (ret)
> > +		return ret;
> > +
> >  	mdelay(10);
> > -	adi_axi_adc_write(st, ADI_AXI_REG_RSTN, ADI_AXI_REG_RSTN_MMCM_RSTN);
> > +	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN,
> > +			   ADI_AXI_REG_RSTN_MMCM_RSTN);
> > +	if (ret)
> > +		return ret;
> > +
> >  	mdelay(10);
> > -	adi_axi_adc_write(st, ADI_AXI_REG_RSTN,
> > -			  ADI_AXI_REG_RSTN_RSTN |
> > ADI_AXI_REG_RSTN_MMCM_RSTN);
> > +	return regmap_write(st->regmap, ADI_AXI_REG_RSTN,
> > +			    ADI_AXI_REG_RSTN_RSTN |
> > ADI_AXI_REG_RSTN_MMCM_RSTN);
> >  }
> >  
> >  static void adi_axi_adc_cleanup(void *data)
> > @@ -335,12 +337,20 @@ static void adi_axi_adc_cleanup(void *data)
> >  	module_put(cl->dev->driver->owner);
> >  }
> >  
> > +static const struct regmap_config axi_adc_regmap_config = {
> > +	.val_bits = 32,
> > +	.reg_bits = 32,
> > +	.reg_stride = 4,
> > +	.max_register = 0x0800,
> > +};
> > +
> >  static int adi_axi_adc_probe(struct platform_device *pdev)
> >  {
> >  	struct adi_axi_adc_conv *conv;
> >  	struct iio_dev *indio_dev;
> >  	struct adi_axi_adc_client *cl;
> >  	struct adi_axi_adc_state *st;
> > +	void __iomem *base;
> >  	unsigned int ver;
> >  	int ret;
> >  
> > @@ -361,15 +371,24 @@ static int adi_axi_adc_probe(struct platform_device
> > *pdev)
> >  	cl->state = st;
> >  	mutex_init(&st->lock);
> >  
> > -	st->regs = devm_platform_ioremap_resource(pdev, 0);
> > -	if (IS_ERR(st->regs))
> > -		return PTR_ERR(st->regs);
> > +	base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(base))
> > +		return PTR_ERR(base);
> > +
> > +	st->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> > +					   &axi_adc_regmap_config);
> > +	if (IS_ERR(st->regmap))
> > +		return PTR_ERR(st->regmap);
> >  
> >  	conv = &st->client->conv;
> >  
> > -	axi_adc_reset(st);
> > +	ret = axi_adc_reset(st);
> > +	if (ret)
> > +		return ret;
> >  
> > -	ver = adi_axi_adc_read(st, ADI_AXI_REG_VERSION);
> > +	ret = regmap_read(st->regmap, ADI_AXI_REG_VERSION, &ver);
> > +	if (ret)
> > +		return ret;
> >  
> >  	if (cl->info->version > ver) {
> >  		dev_err(&pdev->dev,
> >   
> 
> Hi Jonathan,
> 
> I'm not seeing this series yet applied in the togreg branch so maybe I'm still
> on time. This patch is missing the proper Kconfig change:

> 
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index af56df63beff..10e0e340cdae 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -292,7 +292,7 @@ config ADI_AXI_ADC
>         select IIO_BUFFER
>         select IIO_BUFFER_HW_CONSUMER
>         select IIO_BUFFER_DMAENGINE
> -       depends on HAS_IOMEM
> +       select REGMAP_MMIO
>         depends on OF
>         help
>           Say yes here to build support for Analog Devices Generic
> 
> 
> Just realized this when working on the v3 for IIO backends. Please let me know
> if I should send a follow up patch or if I can still send a v3 on the
> "iio: ad9467 and axi-adc cleanups" series.
Oops. I forgot to push out yesterday.  Anyhow, would still have been fine to fix
this as always about a week before I push out as togreg unless we are near
the merge window.

Fixed up.
Jonathan

> 
> - Nuno Sá

Re: [PATCH v2 8/8] iio: adc: adi-axi-adc: convert to regmap

[Nuno Sá]

On Mon, 2023-12-11 at 19:16 +0000, Jonathan Cameron wrote:
> On Mon, 11 Dec 2023 14:18:43 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> > On Thu, 2023-12-07 at 13:39 +0100, Nuno Sa via B4 Relay wrote:
> > > From: Nuno Sa <nuno.sa@analog.com>
> > > 
> > > Use MMIO regmap interface. It makes things easier for manipulating bits.
> > > 
> > > Reviewed-by: David Lechner <dlechner@baylibre.com>
> > > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > > ---
> > >  drivers/iio/adc/adi-axi-adc.c | 85 ++++++++++++++++++++++++++------------
> > > ----
> > > -
> > >  1 file changed, 52 insertions(+), 33 deletions(-)
> > > 
> > > diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> > > index ae83ada7f9f2..c247ff1541d2 100644
> > > --- a/drivers/iio/adc/adi-axi-adc.c
> > > +++ b/drivers/iio/adc/adi-axi-adc.c
> > > @@ -14,6 +14,7 @@
> > >  #include <linux/of.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/property.h>
> > > +#include <linux/regmap.h>
> > >  #include <linux/slab.h>
> > >  
> > >  #include <linux/iio/iio.h>
> > > @@ -62,7 +63,7 @@ struct adi_axi_adc_state {
> > >  	struct mutex				lock;
> > >  
> > >  	struct adi_axi_adc_client		*client;
> > > -	void __iomem				*regs;
> > > +	struct regmap				*regmap;
> > >  };
> > >  
> > >  struct adi_axi_adc_client {
> > > @@ -90,19 +91,6 @@ void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv
> > > *conv)
> > >  }
> > >  EXPORT_SYMBOL_NS_GPL(adi_axi_adc_conv_priv, IIO_ADI_AXI);
> > >  
> > > -static void adi_axi_adc_write(struct adi_axi_adc_state *st,
> > > -			      unsigned int reg,
> > > -			      unsigned int val)
> > > -{
> > > -	iowrite32(val, st->regs + reg);
> > > -}
> > > -
> > > -static unsigned int adi_axi_adc_read(struct adi_axi_adc_state *st,
> > > -				     unsigned int reg)
> > > -{
> > > -	return ioread32(st->regs + reg);
> > > -}
> > > -
> > >  static int adi_axi_adc_config_dma_buffer(struct device *dev,
> > >  					 struct iio_dev *indio_dev)
> > >  {
> > > @@ -163,17 +151,20 @@ static int adi_axi_adc_update_scan_mode(struct
> > > iio_dev
> > > *indio_dev,
> > >  {
> > >  	struct adi_axi_adc_state *st = iio_priv(indio_dev);
> > >  	struct adi_axi_adc_conv *conv = &st->client->conv;
> > > -	unsigned int i, ctrl;
> > > +	unsigned int i;
> > > +	int ret;
> > >  
> > >  	for (i = 0; i < conv->chip_info->num_channels; i++) {
> > > -		ctrl = adi_axi_adc_read(st, ADI_AXI_REG_CHAN_CTRL(i));
> > > -
> > >  		if (test_bit(i, scan_mask))
> > > -			ctrl |= ADI_AXI_REG_CHAN_CTRL_ENABLE;
> > > +			ret = regmap_set_bits(st->regmap,
> > > +					      ADI_AXI_REG_CHAN_CTRL(i),
> > > +					     
> > > ADI_AXI_REG_CHAN_CTRL_ENABLE);
> > >  		else
> > > -			ctrl &= ~ADI_AXI_REG_CHAN_CTRL_ENABLE;
> > > -
> > > -		adi_axi_adc_write(st, ADI_AXI_REG_CHAN_CTRL(i), ctrl);
> > > +			ret = regmap_clear_bits(st->regmap,
> > > +						ADI_AXI_REG_CHAN_CTRL(i),
> > > +						ADI_AXI_REG_CHAN_CTRL_ENA
> > > BLE)
> > > ;
> > > +		if (ret)
> > > +			return ret;
> > >  	}
> > >  
> > >  	return 0;
> > > @@ -310,21 +301,32 @@ static int adi_axi_adc_setup_channels(struct device
> > > *dev,
> > >  	}
> > >  
> > >  	for (i = 0; i < conv->chip_info->num_channels; i++) {
> > > -		adi_axi_adc_write(st, ADI_AXI_REG_CHAN_CTRL(i),
> > > -				  ADI_AXI_REG_CHAN_CTRL_DEFAULTS);
> > > +		ret = regmap_write(st->regmap, ADI_AXI_REG_CHAN_CTRL(i),
> > > +				   ADI_AXI_REG_CHAN_CTRL_DEFAULTS);
> > > +		if (ret)
> > > +			return ret;
> > >  	}
> > >  
> > >  	return 0;
> > >  }
> > >  
> > > -static void axi_adc_reset(struct adi_axi_adc_state *st)
> > > +static int axi_adc_reset(struct adi_axi_adc_state *st)
> > >  {
> > > -	adi_axi_adc_write(st, ADI_AXI_REG_RSTN, 0);
> > > +	int ret;
> > > +
> > > +	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN, 0);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > >  	mdelay(10);
> > > -	adi_axi_adc_write(st, ADI_AXI_REG_RSTN,
> > > ADI_AXI_REG_RSTN_MMCM_RSTN);
> > > +	ret = regmap_write(st->regmap, ADI_AXI_REG_RSTN,
> > > +			   ADI_AXI_REG_RSTN_MMCM_RSTN);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > >  	mdelay(10);
> > > -	adi_axi_adc_write(st, ADI_AXI_REG_RSTN,
> > > -			  ADI_AXI_REG_RSTN_RSTN |
> > > ADI_AXI_REG_RSTN_MMCM_RSTN);
> > > +	return regmap_write(st->regmap, ADI_AXI_REG_RSTN,
> > > +			    ADI_AXI_REG_RSTN_RSTN |
> > > ADI_AXI_REG_RSTN_MMCM_RSTN);
> > >  }
> > >  
> > >  static void adi_axi_adc_cleanup(void *data)
> > > @@ -335,12 +337,20 @@ static void adi_axi_adc_cleanup(void *data)
> > >  	module_put(cl->dev->driver->owner);
> > >  }
> > >  
> > > +static const struct regmap_config axi_adc_regmap_config = {
> > > +	.val_bits = 32,
> > > +	.reg_bits = 32,
> > > +	.reg_stride = 4,
> > > +	.max_register = 0x0800,
> > > +};
> > > +
> > >  static int adi_axi_adc_probe(struct platform_device *pdev)
> > >  {
> > >  	struct adi_axi_adc_conv *conv;
> > >  	struct iio_dev *indio_dev;
> > >  	struct adi_axi_adc_client *cl;
> > >  	struct adi_axi_adc_state *st;
> > > +	void __iomem *base;
> > >  	unsigned int ver;
> > >  	int ret;
> > >  
> > > @@ -361,15 +371,24 @@ static int adi_axi_adc_probe(struct platform_device
> > > *pdev)
> > >  	cl->state = st;
> > >  	mutex_init(&st->lock);
> > >  
> > > -	st->regs = devm_platform_ioremap_resource(pdev, 0);
> > > -	if (IS_ERR(st->regs))
> > > -		return PTR_ERR(st->regs);
> > > +	base = devm_platform_ioremap_resource(pdev, 0);
> > > +	if (IS_ERR(base))
> > > +		return PTR_ERR(base);
> > > +
> > > +	st->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> > > +					   &axi_adc_regmap_config);
> > > +	if (IS_ERR(st->regmap))
> > > +		return PTR_ERR(st->regmap);
> > >  
> > >  	conv = &st->client->conv;
> > >  
> > > -	axi_adc_reset(st);
> > > +	ret = axi_adc_reset(st);
> > > +	if (ret)
> > > +		return ret;
> > >  
> > > -	ver = adi_axi_adc_read(st, ADI_AXI_REG_VERSION);
> > > +	ret = regmap_read(st->regmap, ADI_AXI_REG_VERSION, &ver);
> > > +	if (ret)
> > > +		return ret;
> > >  
> > >  	if (cl->info->version > ver) {
> > >  		dev_err(&pdev->dev,
> > >   
> > 
> > Hi Jonathan,
> > 
> > I'm not seeing this series yet applied in the togreg branch so maybe I'm
> > still
> > on time. This patch is missing the proper Kconfig change:
> 
> > 
> > 
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index af56df63beff..10e0e340cdae 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -292,7 +292,7 @@ config ADI_AXI_ADC
> >         select IIO_BUFFER
> >         select IIO_BUFFER_HW_CONSUMER
> >         select IIO_BUFFER_DMAENGINE
> > -       depends on HAS_IOMEM
> > +       select REGMAP_MMIO
> >         depends on OF
> >         help
> >           Say yes here to build support for Analog Devices Generic
> > 
> > 
> > Just realized this when working on the v3 for IIO backends. Please let me
> > know
> > if I should send a follow up patch or if I can still send a v3 on the
> > "iio: ad9467 and axi-adc cleanups" series.
> Oops. I forgot to push out yesterday.  Anyhow, would still have been fine to
> fix
> this as always about a week before I push out as togreg unless we are near
> the merge window.
> 
> Fixed up.
> Jonathan

Thanks!
- Nuno Sá

Re: [PATCH v2 0/8] iio: ad9467 and axi-adc cleanups

[Jonathan Cameron]

On Thu, 07 Dec 2023 13:39:23 +0100
Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> As requested in [1], this is a preparatory series with some basic cleanups for
> the ad9467 and adi-axi-adc drivers.
> 
> This means that the iio backend series [2] will depend on this one but
> hopefully we'll quickly get this one in.
> 
> [1]: https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#ma7ed8a58d747e78e1ff2273c2d6cdd11de22f177
> [2]: https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f5175273b81dbfe40b7f0daffcdc67d6cb8ff
> 
Series applied to the togreg branch of iio.git.
Note I haven't marked the fixes for stable, or taken them through the fixes tree.
If you want them backported then please request that after the next merge window

Thanks,

Jonathan

> ---
> Changes in v2:
> - Patch 1.
>   * Use IS_ERR_OR_NULL() and PTR_ERR_OR_ZERO();
>   * Improved commit message (mention it's inverted polarity).
> - Patch 4:
>   * Properly allocate enough memory for the scale array;
>   * Use devm_kmalloc_array() as we don't need to zero initialize it;
>   * Don't multiple by 2 in the for() loop. We just want to go over the
>     rows.
>  
> - Link to v1: https://lore.kernel.org/r/20231205-iio-backend-prep-v1-0-7c9bc18d612b@analog.com
> 
> ---
> Nuno Sa (8):
>       iio: adc: ad9467: fix reset gpio handling
>       iio: adc: ad9467: don't ignore error codes
>       iio: adc: ad9467: add mutex to struct ad9467_state
>       iio: adc: ad9467: fix scale setting
>       iio: adc: ad9467: use spi_get_device_match_data()
>       iio: adc: ad9467: use chip_info variables instead of array
>       iio: adc: ad9467: use the more common !val NULL check
>       iio: adc: adi-axi-adc: convert to regmap
> 
>  drivers/iio/adc/ad9467.c            | 207 +++++++++++++++++++++++-------------
>  drivers/iio/adc/adi-axi-adc.c       | 159 ++++++++++++---------------
>  include/linux/iio/adc/adi-axi-adc.h |   4 +
>  3 files changed, 204 insertions(+), 166 deletions(-)
> ---
> base-commit: 5c8f90655a7bdb6232b2cea6503df16367b11a53
> change-id: 20231205-iio-backend-prep-4d96ef364080
> --
> 
> Thanks!
> - Nuno Sá
> 

Re: [PATCH v2 0/8] iio: ad9467 and axi-adc cleanups

[Nuno Sá]

On Sun, 2023-12-10 at 11:43 +0000, Jonathan Cameron wrote:
> On Thu, 07 Dec 2023 13:39:23 +0100
> Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:
> 
> > As requested in [1], this is a preparatory series with some basic cleanups
> > for
> > the ad9467 and adi-axi-adc drivers.
> > 
> > This means that the iio backend series [2] will depend on this one but
> > hopefully we'll quickly get this one in.
> > 
> > [1]:
> > https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#ma7ed8a58d747e78e1ff2273c2d6cdd11de22f177
> > [2]:
> > https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f5175273b81dbfe40b7f0daffcdc67d6cb8ff
> > 
> Series applied to the togreg branch of iio.git.
> Note I haven't marked the fixes for stable, or taken them through the fixes
> tree.
> If you want them backported then please request that after the next merge
> window
> 

Nope, I think I even mention somewhere that I added the Fixes: tag kind of just
for correctness but up to you to backport them :)

Thanks for applying!
- Nuno Sá