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

["Nuno Sá" <noname.nuno@gmail.com>]

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á