On Fri, 2020-02-21 at 12:44 +0000, Jonathan Cameron wrote: > 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. ack > > > @@ -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.. there are more stuff in the pipeline particularly the IIO_CHAN_INFO_CALIBSCALE, IIO_CHAN_INFO_CALIBBIAS & IIO_CHAN_INFO_CALIBPHASE stuff; i can remove the switch for now, and add it later [when we need it]; maybe after some discussion, we might not needed it at all[?] no idea; this driver [in this form] is a rewrite from an older AXI-ADC driver that we've been developing and using internally; the HDL guys managed to cleanup their stuff; this is the Linux cleanup > > + 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. ack > > > + > > +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... > ack will remove > devm_iio_device_unregister for example. hmm; devm_iio_device_unregister() sounds like an interesting GSoC project > > > +{ > > + 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. Yeah. My bad; left-over from some mid-term rewrite. Thanks for catching. > > > + > > +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. > good point > > + 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. > hmmm; so, the 'regs_size' does get used eventually with some debugfs support being added; but maybe after another discussion a better idea could be found; i guess i can remove it for now > > > + 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
