Re: [PATCH 2/5] iio: adc: axi-adc: add support for AXI ADC IP core
From: Jonathan Cameron
Date: Fri Feb 21 2020 - 07:44:29 EST
On Thu, 20 Feb 2020 17:03:14 +0200
Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote:
> From: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
>
> This change adds support for the Analog Devices Generic AXI ADC IP core.
> The IP core is used for interfacing with analog-to-digital (ADC) converters
> that require either a high-speed serial interface (JESD204B/C) or a source
> synchronous parallel interface (LVDS/CMOS).
>
> Usually, some other interface type (i.e SPI) is used as a control interface
> for the actual ADC, while the IP core (controlled via this driver), will
> interface to the data-lines of the ADC and handle the streaming of data
> into memory via DMA.
>
> Because of this, the AXI ADC driver needs the other SPI-ADC driver to
> register with it. The SPI-ADC needs to be register via the SPI framework,
> while the AXI ADC registers as a platform driver. The two cannot be ordered
> in a hierarchy as both drivers have their own registers, and trying to
> organize this [in a hierarchy becomes] problematic when trying to map
> memory/registers.
>
> There are some modes where the AXI ADC can operate as standalone ADC, but
> those will be implemented at a later point in time.
>
> Link: https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
>
> Signed-off-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
In general looks good to me. A few specific comments inline.
Thanks,
Jonathan
> ---
> drivers/iio/adc/Kconfig | 20 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/axi-adc.c | 622 ++++++++++++++++++++++++++++++++
> include/linux/iio/adc/axi-adc.h | 79 ++++
> 4 files changed, 722 insertions(+)
> create mode 100644 drivers/iio/adc/axi-adc.c
> create mode 100644 include/linux/iio/adc/axi-adc.h
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f4da821c4022..6cd48a256122 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -282,6 +282,26 @@ config AT91_SAMA5D2_ADC
> To compile this driver as a module, choose M here: the module will be
> called at91-sama5d2_adc.
>
> +config AXI_ADC
> + tristate "Analog Devices Generic AXI ADC IP core driver"
> + select IIO_BUFFER
> + select IIO_BUFFER_HW_CONSUMER
> + select IIO_BUFFER_DMAENGINE
> + help
> + Say yes here to build support for Analog Devices Generic
> + AXI ADC IP core. The IP core is used for interfacing with
> + analog-to-digital (ADC) converters that require either a high-speed
> + serial interface (JESD204B/C) or a source synchronous parallel
> + interface (LVDS/CMOS).
> + Typically (for such devices) SPI will be used for configuration only,
> + while this IP core handles the streaming of data into memory via DMA.
> +
> + Link: https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
> + If unsure, say N (but it's safe to say "Y").
> +
> + To compile this driver as a module, choose M here: the
> + module will be called axi-adc.
> +
> config AXP20X_ADC
> tristate "X-Powers AXP20X and AXP22X ADC driver"
> depends on MFD_AXP20X
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 8462455b4228..e14fabd53246 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_AD799X) += ad799x.o
> obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
> obj-$(CONFIG_AT91_ADC) += at91_adc.o
> obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
> +obj-$(CONFIG_AXI_ADC) += axi-adc.o
> obj-$(CONFIG_AXP20X_ADC) += axp20x_adc.o
> obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
> obj-$(CONFIG_BCM_IPROC_ADC) += bcm_iproc_adc.o
> diff --git a/drivers/iio/adc/axi-adc.c b/drivers/iio/adc/axi-adc.c
> new file mode 100644
> index 000000000000..9ddd64fdab2d
> --- /dev/null
> +++ b/drivers/iio/adc/axi-adc.c
I suspect this may not be the only AXI based ADC interface in the
world. As such, prefix with adi-axi perhaps.
> @@ -0,0 +1,622 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices Generic AXI ADC IP core
> + * Link: https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
> + *
> + * Copyright 2012-2020 Analog Devices Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/buffer-dmaengine.h>
> +
> +#include <linux/fpga/adi-axi-common.h>
> +#include <linux/iio/adc/axi-adc.h>
> +
> +/**
> + * Register definitions:
> + * https://wiki.analog.com/resources/fpga/docs/axi_adc_ip#register_map
> + */
> +
> +#define AXI_ADC_UPPER16_MSK GENMASK(31, 16)
> +#define AXI_ADC_UPPER16_SET(x) FIELD_PREP(AXI_ADC_UPPER16_MSK, x)
> +#define AXI_ADC_UPPER16_GET(x) FIELD_GET(AXI_ADC_UPPER16_MSK, x)
> +
> +#define AXI_ADC_LOWER16_MSK GENMASK(15, 0)
> +#define AXI_ADC_LOWER16_SET(x) FIELD_PREP(AXI_ADC_UPPER16_MSK, x)
> +#define AXI_ADC_LOWER16_GET(x) FIELD_GET(AXI_ADC_LOWER16_MSK, x)
> +
> +/* ADC controls */
> +
> +#define AXI_ADC_REG_RSTN 0x0040
> +#define AXI_ADC_MMCM_RSTN BIT(1)
> +#define AXI_ADC_RSTN BIT(0)
> +
> +#define AXI_ADC_REG_CNTRL 0x0044
> +#define AXI_ADC_R1_MODE BIT(2)
> +#define AXI_ADC_DDR_EDGESEL BIT(1)
> +#define AXI_ADC_PIN_MODE BIT(0)
> +
> +#define AXI_ADC_REG_CLK_FREQ 0x0054
> +#define AXI_ADC_REG_CLK_RATIO 0x0058
> +
> +#define AXI_ADC_REG_STATUS 0x005C
> +#define AXI_ADC_MUX_PN_ERR BIT(3)
> +#define AXI_ADC_MUX_PN_OOS BIT(2)
> +#define AXI_ADC_MUX_OVER_RANGE BIT(1)
> +#define AXI_ADC_STATUS BIT(0)
> +
> +#define AXI_ADC_REG_DRP_CNTRL 0x0070
> +#define AXI_ADC_DRP_SEL BIT(29)
> +#define AXI_ADC_DRP_RWN BIT(28)
> +#define AXI_ADC_DRP_ADDRESS_MSK GENMASK(27, 16)
> +#define AXI_ADC_DRP_ADDRESS_SET(x) \
> + FIELD_PREP(AXI_ADC_DRP_ADDRESS_MSK, x)
> +#define AXI_ADC_DRP_ADDRESS_GET(x) \
> + FIELD_GET(AXI_ADC_DRP_ADDRESS_MSK, x)
> +#define AXI_ADC_DRP_WDATA_SET AXI_ADC_LOWER16_SET
> +#define AXI_ADC_DRP_WDATA_GET AXI_ADC_LOWER16_GET
> +
> +#define AXI_REG_DRP_STATUS 0x0074
> +#define AXI_ADC_DRP_STATUS BIT(16)
> +#define AXI_ADC_DRP_RDATA_SET AXI_ADC_LOWER16_SET
> +#define AXI_ADC_DRP_RDATA_GET AXI_ADC_LOWER16_GET
> +
> +#define AXI_ADC_REG_DMA_STATUS 0x0088
> +#define AXI_ADC_DMA_OVF BIT(2)
> +#define AXI_ADC_DMA_UNF BIT(1)
> +#define AXI_ADC_DMA_STATUS BIT(0)
> +
> +#define ADI_REG_DMA_BUSWIDTH 0x008C
> +#define AXI_ADC_REG_GP_CONTROL 0x00BC
> +#define AXI_ADC_REG_ADC_DP_DISABLE 0x00C0
> +
> +/* ADC Channel controls */
> +
> +#define AXI_ADC_REG_CHAN_CNTRL(c) (0x0400 + (c) * 0x40)
> +#define AXI_ADC_PN_SEL BIT(10)
> +#define AXI_ADC_IQCOR_ENB BIT(9)
> +#define AXI_ADC_DCFILT_ENB BIT(8)
> +#define AXI_ADC_FORMAT_SIGNEXT BIT(6)
> +#define AXI_ADC_FORMAT_TYPE BIT(5)
> +#define AXI_ADC_FORMAT_ENABLE BIT(4)
> +#define AXI_ADC_PN23_TYPE BIT(1)
> +#define AXI_ADC_ENABLE BIT(0)
> +
> +#define AXI_ADC_REG_CHAN_STATUS(c) (0x0404 + (c) * 0x40)
> +#define AXI_ADC_PN_ERR BIT(2)
> +#define AXI_ADC_PN_OOS BIT(1)
> +#define AXI_ADC_OVER_RANGE BIT(0)
> +
> +#define AXI_ADC_REG_CHAN_CNTRL_1(c) (0x0410 + (c) * 0x40)
> +#define AXI_ADC_DCFILT_OFFSET_MSK AXI_ADC_UPPER16_MSK
> +#define AXI_ADC_DCFILT_OFFSET_SET AXI_ADC_UPPER16_SET
> +#define AXI_ADC_DCFILT_OFFSET_GET AXI_ADC_UPPER16_GET
> +#define AXI_ADC_DCFILT_COEFF_MSK AXI_ADC_LOWER16_MSK
> +#define AXI_ADC_DCFILT_COEFF_SET AXI_ADC_LOWER16_SET
> +#define AXI_ADC_DCFILT_COEFF_GET AXI_ADC_LOWER16_GET
> +
> +#define AXI_ADC_REG_CHAN_CNTRL_2(c) (0x0414 + (c) * 0x40)
> +#define AXI_ADC_IQCOR_COEFF_1_MSK AXI_ADC_UPPER16_MSK
> +#define AXI_ADC_IQCOR_COEFF_1_SET AXI_ADC_UPPER16_SET
> +#define AXI_ADC_IQCOR_COEFF_1_GET AXI_ADC_UPPER16_GET
> +#define AXI_ADC_IQCOR_COEFF_2_MSK AXI_ADC_LOWER16_MSK
> +#define AXI_ADC_IQCOR_COEFF_2_SET AXI_ADC_LOWER16_SET
> +#define AXI_ADC_IQCOR_COEFF_2_GET AXI_ADC_LOWER16_GET
> +
> +/* format is 1.1.14 (sign, integer and fractional bits) */
> +#define AXI_ADC_IQCOR_INT_1 0x4000UL
> +#define AXI_ADC_IQCOR_SIGN_BIT BIT(15)
> +/* The constant below is (2 * PI * 0x4000), where 0x4000 is AXI_ADC_IQCOR_INT_1 */
> +#define AXI_ADC_2_X_PI_X_INT_1 102944ULL
> +
> +#define AXI_ADC_REG_CHAN_CNTRL_3(c) (0x0418 + (c) * 0x40)
> +#define AXI_ADC_ADC_PN_SEL_MSK AXI_ADC_UPPER16_MSK
> +#define AXI_ADC_ADC_PN_SEL_SET AXI_ADC_UPPER16_SET
> +#define AXI_ADC_ADC_PN_SEL_GET AXI_ADC_UPPER16_GET
> +#define AXI_ADC_ADC_DATA_SEL_MSK AXI_ADC_LOWER16_MSK
> +#define AXI_ADC_ADC_DATA_SEL_SET AXI_ADC_LOWER16_SET
> +#define AXI_ADC_ADC_DATA_SEL_GET AXI_ADC_LOWER16_GET
> +
> +#define AXI_ADC_REG_CHAN_USR_CNTRL_2(c) (0x0424 + (c) * 0x40)
> +#define AXI_ADC_USR_DECIMATION_M_MSK AXI_ADC_UPPER16_MSK
> +#define AXI_ADC_USR_DECIMATION_M_SET AXI_ADC_UPPER16_SET
> +#define AXI_ADC_USR_DECIMATION_M_GET AXI_ADC_UPPER16_GET
> +#define AXI_ADC_USR_DECIMATION_N_MSK AXI_ADC_LOWER16_MSK
> +#define AXI_ADC_USR_DECIMATION_N_SET AXI_ADC_LOWER16_SET
> +#define AXI_ADC_USR_DECIMATION_N_GET AXI_ADC_LOWER16_GET
> +
> +/* debugfs direct register access */
> +#define DEBUGFS_DRA_PCORE_REG_MAGIC BIT(31)
> +
> +struct axi_adc_core_info {
> + unsigned int version;
> +};
> +
> +struct axi_adc_state {
> + struct mutex lock;
> +
> + struct axi_adc_client *client;
> + void __iomem *regs;
> + unsigned int regs_size;
> +};
> +
> +struct axi_adc_client {
> + struct list_head entry;
> + struct axi_adc_conv conv;
> + struct axi_adc_state *state;
> + struct device *dev;
> + const struct axi_adc_core_info *info;
> +};
> +
> +static LIST_HEAD(axi_adc_registered_clients);
> +static DEFINE_MUTEX(axi_adc_registered_clients_lock);
> +
> +static struct axi_adc_client *axi_adc_conv_to_client(struct axi_adc_conv *conv)
> +{
> + if (!conv)
> + return NULL;
> + return container_of(conv, struct axi_adc_client, conv);
> +}
> +
> +void *axi_adc_conv_priv(struct axi_adc_conv *conv)
> +{
> + struct axi_adc_client *cl = axi_adc_conv_to_client(conv);
> +
> + if (!cl)
> + return NULL;
> +
> + return (char *)cl + ALIGN(sizeof(struct axi_adc_client), IIO_ALIGN);
> +}
> +EXPORT_SYMBOL_GPL(axi_adc_conv_priv);
> +
> +static void axi_adc_write(struct axi_adc_state *st, unsigned int reg,
> + unsigned int val)
> +{
> + iowrite32(val, st->regs + reg);
> +}
> +
> +static unsigned int axi_adc_read(struct axi_adc_state *st, unsigned int reg)
> +{
> + return ioread32(st->regs + reg);
> +}
> +
> +static int axi_adc_config_dma_buffer(struct device *dev,
> + struct iio_dev *indio_dev)
> +{
> + struct iio_buffer *buffer;
> + const char *dma_name;
> +
> + if (!device_property_present(dev, "dmas"))
> + return 0;
> +
> + if (device_property_read_string(dev, "dma-names", &dma_name))
> + dma_name = "rx";
> +
> + buffer = devm_iio_dmaengine_buffer_alloc(indio_dev->dev.parent,
> + dma_name);
> + if (IS_ERR(buffer))
> + return PTR_ERR(buffer);
> +
> + indio_dev->modes |= INDIO_BUFFER_HARDWARE;
> + iio_device_attach_buffer(indio_dev, buffer);
> +
> + return 0;
> +}
> +
> +static int axi_adc_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct axi_adc_state *st = iio_priv(indio_dev);
> + struct axi_adc_conv *conv = &st->client->conv;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + /* fall-through */
> + default:
> + if (!conv->read_raw)
> + return -ENOSYS;
> +
> + return conv->read_raw(conv, chan, val, val2, mask);
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int axi_adc_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct axi_adc_state *st = iio_priv(indio_dev);
> + struct axi_adc_conv *conv = &st->client->conv;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + /* fall-through */
Umm. Got to ask. If you only have one option and a default, why have
the option? Or indeed the switch statement at all..
> + default:
> + if (!conv->write_raw)
> + return -ENOSYS;
> +
> + return conv->write_raw(conv, chan, val, val2, mask);
> + }
> +}
> +
> +static int axi_adc_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct axi_adc_state *st = iio_priv(indio_dev);
> + struct axi_adc_conv *conv = &st->client->conv;
> + unsigned int i, ctrl;
> +
> + for (i = 0; i < conv->chip_info->num_channels; i++) {
> + ctrl = axi_adc_read(st, AXI_ADC_REG_CHAN_CNTRL(i));
> +
> + if (test_bit(i, scan_mask))
> + ctrl |= AXI_ADC_ENABLE;
> + else
> + ctrl &= ~AXI_ADC_ENABLE;
> +
> + axi_adc_write(st, AXI_ADC_REG_CHAN_CNTRL(i), ctrl);
> + }
> +
> + return 0;
> +}
> +
> +struct axi_adc_conv *axi_adc_conv_register(struct device *dev, int sizeof_priv)
> +{
> + struct axi_adc_client *cl;
> + size_t alloc_size;
> +
> + alloc_size = sizeof(struct axi_adc_client);
> + if (sizeof_priv) {
> + alloc_size = ALIGN(alloc_size, IIO_ALIGN);
> + alloc_size += sizeof_priv;
> + }
> + alloc_size += IIO_ALIGN - 1;
> +
> + cl = kzalloc(alloc_size, GFP_KERNEL);
> + if (!cl)
> + return ERR_PTR(-ENOMEM);
> +
> + mutex_lock(&axi_adc_registered_clients_lock);
> +
> + get_device(dev);
> + cl->dev = dev;
> +
> + list_add_tail(&cl->entry, &axi_adc_registered_clients);
> +
> + mutex_unlock(&axi_adc_registered_clients_lock);
> +
> + return &cl->conv;
> +}
> +EXPORT_SYMBOL_GPL(axi_adc_conv_register);
> +
> +void axi_adc_conv_unregister(struct axi_adc_conv *conv)
> +{
> + struct axi_adc_client *cl = axi_adc_conv_to_client(conv);
> +
> + if (!cl)
> + return;
> +
> + mutex_lock(&axi_adc_registered_clients_lock);
> +
> + put_device(cl->dev);
> + list_del(&cl->entry);
> + kfree(cl);
> +
> + mutex_unlock(&axi_adc_registered_clients_lock);
> +}
> +EXPORT_SYMBOL(axi_adc_conv_unregister);
You could avoid exporting the non devm versions to encourage people
to only use the managed ones.
> +
> +static void devm_axi_adc_conv_release(struct device *dev, void *res)
> +{
> + axi_adc_conv_unregister(*(struct axi_adc_conv **)res);
> +}
> +
> +static int devm_axi_adc_conv_match(struct device *dev, void *res, void *data)
> +{
> + struct axi_adc_conv **r = res;
> +
> + return *r == data;
> +}
> +
> +struct axi_adc_conv *devm_axi_adc_conv_register(struct device *dev,
> + int sizeof_priv)
> +{
> + struct axi_adc_conv **ptr, *conv;
> +
> + ptr = devres_alloc(devm_axi_adc_conv_release, sizeof(*ptr),
> + GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + conv = axi_adc_conv_register(dev, sizeof_priv);
> + if (IS_ERR(conv)) {
> + devres_free(ptr);
> + return ERR_CAST(conv);
> + }
> +
> + *ptr = conv;
> + devres_add(dev, ptr);
> +
> + return conv;
> +}
> +EXPORT_SYMBOL_GPL(devm_axi_adc_conv_register);
> +
> +void devm_axi_adc_conv_unregister(struct device *dev,
> + struct axi_adc_conv *conv)
Note that there is no 'need' to provide devm unregister functions
if it is unlikely a driver will actually use them.
May well be better to clean the ones in here out until we
actually need them. If nothing else having them may encourage
bad driver architecture.
hohum. I should probably just remove the ones in the IIO core
that never get used as well...
devm_iio_device_unregister for example.
> +{
> + int rc;
> +
> + rc = devres_release(dev, devm_axi_adc_conv_release,
> + devm_axi_adc_conv_match, conv);
> + WARN_ON(rc);
> +}
> +EXPORT_SYMBOL_GPL(devm_axi_adc_conv_unregister);
> +
> +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 axi_adc_state *st = iio_priv(indio_dev);
> + struct axi_adc_conv *conv = &st->client->conv;
> + size_t len = 0;
> + int i;
> +
> + if (!conv->chip_info->num_scales) {
> + buf[0] = '\n';
> + return 1;
> + }
> +
> + 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);
> +
> +static struct attribute *axi_adc_attributes[] = {
> + &iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group axi_adc_attribute_group = {
> + .attrs = axi_adc_attributes,
> +};
> +
> +static const struct iio_info axi_adc_info = {
> + .read_raw = &axi_adc_read_raw,
> + .write_raw = &axi_adc_write_raw,
> + .attrs = &axi_adc_attribute_group,
> + .update_scan_mode = &axi_adc_update_scan_mode,
> +};
> +
> +static const struct axi_adc_core_info axi_adc_10_0_a_info = {
> + .version = ADI_AXI_PCORE_VER(10, 0, 'a'),
> +};
> +
> +/* Match table for of_platform binding */
> +static const struct of_device_id axi_adc_of_match[] = {
> + { .compatible = "adi,axi-adc-10.0.a", .data = &axi_adc_10_0_a_info },
> + { /* end of list */ },
> +};
> +MODULE_DEVICE_TABLE(of, axi_adc_of_match);
> +
> +struct axi_adc_client *axi_adc_attach_client(struct device *dev)
> +{
> + const struct of_device_id *id;
> + struct axi_adc_client *cl;
> + struct device_node *cln;
> +
> + if (!dev->of_node) {
> + dev_err(dev, "DT node is null\n");
> + return ERR_PTR(-ENODEV);
> + }
> +
> + id = of_match_node(axi_adc_of_match, dev->of_node);
> + if (!id)
> + return ERR_PTR(-ENODEV);
> +
> + cln = of_parse_phandle(dev->of_node, "axi-adc-client", 0);
> + if (!cln) {
> + dev_err(dev, "No 'axi-adc-client' node defined\n");
> + return ERR_PTR(-ENODEV);
> + }
> +
> + mutex_lock(&axi_adc_registered_clients_lock);
> +
> + list_for_each_entry(cl, &axi_adc_registered_clients, entry) {
> + if (!cl->dev)
> + continue;
> + if (cl->dev->of_node == cln) {
> + if (!try_module_get(dev->driver->owner)) {
> + mutex_unlock(&axi_adc_registered_clients_lock);
> + return ERR_PTR(-ENODEV);
> + }
> + get_device(dev);
> + cl->info = id->data;
> + mutex_unlock(&axi_adc_registered_clients_lock);
> + return cl;
> + }
> + }
> +
> + mutex_unlock(&axi_adc_registered_clients_lock);
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int axi_adc_setup_channels(struct device *dev, struct axi_adc_state *st)
> +{
> + struct axi_adc_conv *conv = conv = &st->client->conv;
> + unsigned int val;
> + int i, ret;
> +
> + if (conv->preenable_setup) {
> + ret = conv->preenable_setup(conv);
> + if (ret)
> + return ret;
> + }
> +
> + for (i = 0; i < conv->chip_info->num_channels; i++) {
> + if (i & 1)
> + val = AXI_ADC_IQCOR_COEFF_2_SET(AXI_ADC_IQCOR_INT_1);
> + else
> + val = AXI_ADC_IQCOR_COEFF_1_SET(AXI_ADC_IQCOR_INT_1);
> + axi_adc_write(st, AXI_ADC_REG_CHAN_CNTRL_2(i), val);
> +
> + axi_adc_write(st, AXI_ADC_REG_CHAN_CNTRL(i),
> + AXI_ADC_FORMAT_SIGNEXT | AXI_ADC_FORMAT_ENABLE |
> + AXI_ADC_IQCOR_ENB | AXI_ADC_ENABLE);
> + }
> +
> + return 0;
> +}
> +
> +static int axi_adc_alloc_channels(struct iio_dev *indio_dev,
> + struct axi_adc_conv *conv)
> +{
> + unsigned int i, num = conv->chip_info->num_channels;
> + struct device *dev = indio_dev->dev.parent;
> + struct iio_chan_spec *channels;
> +
> + channels = devm_kcalloc(dev, num, sizeof(*channels), GFP_KERNEL);
> + if (!channels)
> + return -ENOMEM;
> +
> + for (i = 0; i < conv->chip_info->num_channels; i++)
> + channels[i] = conv->chip_info->channels->iio_chan;
> +
> + indio_dev->num_channels = num;
> + indio_dev->channels = channels;
> +
> + return 0;
> +}
> +
> +struct axi_adc_cleanup_data {
> + struct axi_adc_state *st;
> + struct axi_adc_client *cl;
> +};
Doesn't seem to be used.
> +
> +static void axi_adc_cleanup(void *data)
> +{
> + struct axi_adc_client *cl = data;
> +
> + put_device(cl->dev);
> + module_put(cl->dev->driver->owner);
> +}
> +
> +static int axi_adc_probe(struct platform_device *pdev)
> +{
> + struct axi_adc_conv *conv;
> + struct iio_dev *indio_dev;
> + struct axi_adc_client *cl;
> + struct axi_adc_state *st;
> + struct resource *mem;
> + unsigned int ver;
> + int ret;
> +
> + cl = axi_adc_attach_client(&pdev->dev);
> + if (IS_ERR(cl))
> + return PTR_ERR(cl);
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*st));
> + if (indio_dev == NULL)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + st->client = cl;
> + cl->state = st;
> + mutex_init(&st->lock);
> +
> + ret = devm_add_action_or_reset(&pdev->dev, axi_adc_cleanup, cl);
This is unwinding things in axi_adc_attach_client, so should be
called immediately after that, not with the iio device allocation
in between.
> + if (ret)
> + return ret;
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + st->regs_size = resource_size(mem);
> + st->regs = devm_ioremap_resource(&pdev->dev, mem);
Can we use devm_platform_ioremap_resource here?
We grab regs_size but don't seem to use it.
> + if (IS_ERR(st->regs))
> + return PTR_ERR(st->regs);
> +
> + conv = &st->client->conv;
> +
> + /* Reset HDL Core */
> + axi_adc_write(st, AXI_ADC_REG_RSTN, 0);
> + mdelay(10);
> + axi_adc_write(st, AXI_ADC_REG_RSTN, AXI_ADC_MMCM_RSTN);
> + mdelay(10);
> + axi_adc_write(st, AXI_ADC_REG_RSTN, AXI_ADC_RSTN | AXI_ADC_MMCM_RSTN);
> +
> + ver = axi_adc_read(st, ADI_AXI_REG_VERSION);
> +
> + if (cl->info->version > ver) {
> + dev_err(&pdev->dev,
> + "IP core version is too old. Expected %d.%.2d.%c, Reported %d.%.2d.%c\n",
> + ADI_AXI_PCORE_VER_MAJOR(cl->info->version),
> + ADI_AXI_PCORE_VER_MINOR(cl->info->version),
> + ADI_AXI_PCORE_VER_PATCH(cl->info->version),
> + ADI_AXI_PCORE_VER_MAJOR(ver),
> + ADI_AXI_PCORE_VER_MINOR(ver),
> + ADI_AXI_PCORE_VER_PATCH(ver));
> + return -ENODEV;
> + }
> +
> + indio_dev->info = &axi_adc_info;
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->name = pdev->dev.of_node->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = axi_adc_alloc_channels(indio_dev, conv);
> + if (ret)
> + return ret;
> +
> + ret = axi_adc_config_dma_buffer(&pdev->dev, indio_dev);
> + if (ret)
> + return ret;
> +
> + ret = axi_adc_setup_channels(&pdev->dev, st);
> + if (ret)
> + return ret;
> +
> + ret = devm_iio_device_register(&pdev->dev, indio_dev);
> + if (ret)
> + return ret;
> +
> + dev_info(&pdev->dev, "AXI ADC IP core (%d.%.2d.%c) probed\n",
> + ADI_AXI_PCORE_VER_MAJOR(ver),
> + ADI_AXI_PCORE_VER_MINOR(ver),
> + ADI_AXI_PCORE_VER_PATCH(ver));
> +
> + return 0;
> +}
> +
> +static struct platform_driver axi_adc_driver = {
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = axi_adc_of_match,
> + },
> + .probe = axi_adc_probe,
> +};
> +
> +module_platform_driver(axi_adc_driver);
> +
> +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("Analog Devices Generic AXI ADC IP core driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/iio/adc/axi-adc.h b/include/linux/iio/adc/axi-adc.h
> new file mode 100644
> index 000000000000..d367c442dc52
> --- /dev/null
> +++ b/include/linux/iio/adc/axi-adc.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Analog Devices Generic AXI ADC IP core driver/library
> + * Link: https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
> + *
> + * Copyright 2012-2020 Analog Devices Inc.
> + */
> +#ifndef __AXI_ADC_H__
> +#define __AXI_ADC_H__
> +
> +struct device;
> +
> +/**
> + * struct axi_adc_chan_spec - AXI ADC channel wrapper
> + * maps IIO channel data with AXI ADC specifics
> + * @iio_chan IIO channel specification
> + * @num_lanes Number of lanes per channel
> + */
> +struct axi_adc_chan_spec {
> + struct iio_chan_spec iio_chan;
> + unsigned int num_lanes;
> +};
> +
> +/**
> + * struct axi_adc_chip_info - Chip specific information
> + * @name Chip name
> + * @id Chip ID (usually product ID)
> + * @channels Channel specifications of type @struct axi_adc_chan_spec
> + * @num_channels Number of @channels
> + * @scale_table Supported scales by the chip; tuples of 2 ints
> + * @num_scales Number of scales in the table
> + * @max_rate Maximum sampling rate supported by the device
> + */
> +struct axi_adc_chip_info {
> + const char *name;
> + unsigned int id;
> +
> + const struct axi_adc_chan_spec *channels;
> + unsigned int num_channels;
> +
> + const unsigned int (*scale_table)[2];
> + int num_scales;
> +
> + unsigned long max_rate;
> +};
> +
> +/**
> + * struct axi_adc_conv - data of the ADC attached to the AXI ADC
> + * @chip_info chip info details for the client ADC
> + * @preenable_setup op to run in the client before enabling the AXI ADC
> + * @read_raw IIO read_raw hook for the client ADC
> + * @write_raw IIO write_raw hook for the client ADC
> + */
> +struct axi_adc_conv {
> + const struct axi_adc_chip_info *chip_info;
> +
> + int (*preenable_setup)(struct axi_adc_conv *conv);
> + int (*reg_access)(struct axi_adc_conv *conv, unsigned int reg,
> + unsigned int writeval, unsigned int *readval);
> + int (*read_raw)(struct axi_adc_conv *conv,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask);
> + int (*write_raw)(struct axi_adc_conv *conv,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask);
> +};
> +
> +struct axi_adc_conv *axi_adc_conv_register(struct device *dev,
> + int sizeof_priv);
> +void axi_adc_conv_unregister(struct axi_adc_conv *conv);
> +
> +struct axi_adc_conv *devm_axi_adc_conv_register(struct device *dev,
> + int sizeof_priv);
> +void devm_axi_adc_conv_unregister(struct device *dev,
> + struct axi_adc_conv *conv);
> +
> +void *axi_adc_conv_priv(struct axi_adc_conv *conv);
> +
> +#endif