Message ID | 20231121-dev-iio-backend-v1-11-6a3d542eba35@analog.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | iio: add new backend framework | expand |
On Tue, 21 Nov 2023 11:20:24 +0100 Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote: > From: Nuno Sa <nuno.sa@analog.com> > > Use MMIO regmap interface. It makes things easier for manipulating bits. > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> Perhaps put this in the precursor set as well. Looks fine to me and will just be noise in the main discussion. Jonathan > --- > 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, >
On Mon, 2023-12-04 at 15:51 +0000, Jonathan Cameron wrote: > On Tue, 21 Nov 2023 11:20:24 +0100 > Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote: > > > From: Nuno Sa <nuno.sa@analog.com> > > > > Use MMIO regmap interface. It makes things easier for manipulating bits. > > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> > Perhaps put this in the precursor set as well. Looks fine to me and will just > be noise in the main discussion. > will do... - Nuno Sá
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,