* [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free
@ 2020-02-20 15:03 Alexandru Ardelean
2020-02-20 15:03 ` [PATCH 2/5] iio: adc: axi-adc: add support for AXI ADC IP core Alexandru Ardelean
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Alexandru Ardelean @ 2020-02-20 15:03 UTC (permalink / raw)
To: linux-kernel, linux-iio, devicetree; +Cc: robh+dt, jic23, Alexandru Ardelean
Currently, when using a 'iio_dmaengine_buffer_alloc()', an matching call to
'iio_dmaengine_buffer_free()' must be made.
With this change, this can be avoided by using
'devm_iio_dmaengine_buffer_alloc()'. The buffer will get free'd via the
device's devres handling.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
.../buffer/industrialio-buffer-dmaengine.c | 70 +++++++++++++++++++
include/linux/iio/buffer-dmaengine.h | 5 ++
2 files changed, 75 insertions(+)
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index b129693af0fd..eff89037e3f5 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -229,6 +229,76 @@ void iio_dmaengine_buffer_free(struct iio_buffer *buffer)
}
EXPORT_SYMBOL_GPL(iio_dmaengine_buffer_free);
+static void __devm_iio_dmaengine_buffer_free(struct device *dev, void *res)
+{
+ iio_dmaengine_buffer_free(*(struct iio_buffer **)res);
+}
+
+/**
+ * devm_iio_dmaengine_buffer_alloc() - Resource-managed iio_dmaengine_buffer_alloc()
+ * @dev: Parent device for the buffer
+ * @channel: DMA channel name, typically "rx".
+ *
+ * This allocates a new IIO buffer which internally uses the DMAengine framework
+ * to perform its transfers. The parent device will be used to request the DMA
+ * channel.
+ *
+ * Once done using the buffer iio_dmaengine_buffer_free() should be used to
+ * release it.
+ */
+struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
+ const char *channel)
+{
+ struct iio_buffer **bufferp, *buffer;
+
+ bufferp = devres_alloc(__devm_iio_dmaengine_buffer_free,
+ sizeof(*bufferp), GFP_KERNEL);
+ if (!bufferp)
+ return ERR_PTR(-ENOMEM);
+
+ buffer = iio_dmaengine_buffer_alloc(dev, channel);
+ if (!IS_ERR(buffer)) {
+ *bufferp = buffer;
+ devres_add(dev, bufferp);
+ } else {
+ devres_free(bufferp);
+ }
+
+ return buffer;
+}
+EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_alloc);
+
+static int devm_iio_dmaengine_buffer_match(struct device *dev, void *res,
+ void *data)
+{
+ struct iio_buffer **r = res;
+
+ if (!r || !*r) {
+ WARN_ON(!r || !*r);
+ return 0;
+ }
+
+ return *r == data;
+}
+
+/**
+ * devm_iio_dmaengine_buffer_free - iio_dmaengine_buffer_free
+ * @dev: Device this iio_buffer belongs to
+ * @buffer: The iio_buffer associated with the device
+ *
+ * Free buffer allocated with devm_iio_dmaengine_buffer_alloc().
+ */
+void devm_iio_dmaengine_buffer_free(struct device *dev,
+ struct iio_buffer *buffer)
+{
+ int rc;
+
+ rc = devres_release(dev, __devm_iio_dmaengine_buffer_free,
+ devm_iio_dmaengine_buffer_match, buffer);
+ WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_free);
+
MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
MODULE_DESCRIPTION("DMA buffer for the IIO framework");
MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/buffer-dmaengine.h b/include/linux/iio/buffer-dmaengine.h
index b3a57444a886..8dcd973d76c1 100644
--- a/include/linux/iio/buffer-dmaengine.h
+++ b/include/linux/iio/buffer-dmaengine.h
@@ -14,4 +14,9 @@ struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
const char *channel);
void iio_dmaengine_buffer_free(struct iio_buffer *buffer);
+struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
+ const char *channel);
+void devm_iio_dmaengine_buffer_free(struct device *dev,
+ struct iio_buffer *buffer);
+
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/5] iio: adc: axi-adc: add support for AXI ADC IP core
2020-02-20 15:03 [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free Alexandru Ardelean
@ 2020-02-20 15:03 ` Alexandru Ardelean
2020-02-21 12:44 ` Jonathan Cameron
2020-02-20 15:03 ` [PATCH 3/5] dt-bindings: iio: adc: add bindings doc for AXI ADC driver Alexandru Ardelean
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Alexandru Ardelean @ 2020-02-20 15:03 UTC (permalink / raw)
To: linux-kernel, linux-iio, devicetree
Cc: robh+dt, jic23, Michael Hennerich, Lars-Peter Clausen,
Alexandru Ardelean
From: Michael Hennerich <michael.hennerich@analog.com>
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@analog.com>
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
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
@@ -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 */
+ 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);
+
+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)
+{
+ 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;
+};
+
+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);
+ 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);
+ 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@analog.com>");
+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
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/5] dt-bindings: iio: adc: add bindings doc for AXI ADC driver
2020-02-20 15:03 [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free Alexandru Ardelean
2020-02-20 15:03 ` [PATCH 2/5] iio: adc: axi-adc: add support for AXI ADC IP core Alexandru Ardelean
@ 2020-02-20 15:03 ` Alexandru Ardelean
2020-02-20 20:35 ` Rob Herring
2020-02-20 15:03 ` [PATCH 4/5] iio: adc: ad9467: add support AD9467 ADC Alexandru Ardelean
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Alexandru Ardelean @ 2020-02-20 15:03 UTC (permalink / raw)
To: linux-kernel, linux-iio, devicetree; +Cc: robh+dt, jic23, Alexandru Ardelean
This change adds the bindings documentation for the AXI ADC driver.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
.../bindings/iio/adc/adi,axi-adc.yaml | 70 +++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
new file mode 100644
index 000000000000..a1c2630c6840
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,axi-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AXI ADC IP core
+
+maintainers:
+ - Michael Hennerich <michael.hennerich@analog.com>
+ - Alexandru Ardelean <alexandru.ardelean@analog.com>
+
+description: |
+ Analog Devices Generic AXI ADC IP core for interfacing an ADC device
+ with a high speed serial (JESD204B/C) or 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 this IP core will interface
+ to the data-lines of the ADC and handle the streaming of data into
+ memory via DMA.
+
+ https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
+
+properties:
+ compatible:
+ enum:
+ - adi,axi-adc-10.0.a
+
+ reg:
+ maxItems: 1
+
+ dmas:
+ description:
+ A reference to a DMA channel channel specifier.
+ maxItems: 1
+
+ dmas-names:
+ description:
+ The name of the DMA channel.
+ maxItems: 1
+
+ axi-adc-client:
+ description:
+ A reference to a the actual ADC to which this FPGA ADC interfaces to.
+ maxItems: 1
+
+required:
+ - compatible
+ - dmas
+ - reg
+ - axi-adc-client
+
+additionalProperties: false
+
+examples:
+ - |
+ fpga_axi {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ axi-adc@44a00000 {
+ compatible = "adi,axi-adc-10.0.a";
+ reg = <0x44a00000 0x10000>;
+ dmas = <&rx_dma 0>;
+ dma-names = "rx";
+
+ axi-adc-client = <&spi_adc>;
+ };
+ };
+...
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/5] iio: adc: ad9467: add support AD9467 ADC
2020-02-20 15:03 [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free Alexandru Ardelean
2020-02-20 15:03 ` [PATCH 2/5] iio: adc: axi-adc: add support for AXI ADC IP core Alexandru Ardelean
2020-02-20 15:03 ` [PATCH 3/5] dt-bindings: iio: adc: add bindings doc for AXI ADC driver Alexandru Ardelean
@ 2020-02-20 15:03 ` Alexandru Ardelean
2020-02-21 12:57 ` Jonathan Cameron
2020-02-20 15:03 ` [PATCH 5/5] dt-bindings: iio: adc: add bindings doc for " Alexandru Ardelean
2020-02-21 12:21 ` [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free Jonathan Cameron
4 siblings, 1 reply; 16+ messages in thread
From: Alexandru Ardelean @ 2020-02-20 15:03 UTC (permalink / raw)
To: linux-kernel, linux-iio, devicetree
Cc: robh+dt, jic23, Michael Hennerich, Lars-Peter Clausen,
Alexandru Ardelean
From: Michael Hennerich <michael.hennerich@analog.com>
The AD9467 is a 16-bit, monolithic, IF sampling analog-to-digital converter
(ADC). It is optimized for high performanceover wide bandwidths and ease of
use. The product operates at a 250 MSPS conversion rate and is designed for
wireless receivers, instrumentation, and test equipment that require a high
dynamic range. The ADC requires 1.8 V and 3.3 V power supplies and a low
voltage differential input clock for full performance operation. No
external reference or driver components are required for many applications.
Data outputs are LVDS compatible (ANSI-644 compatible) and include the
means to reduce the overall current needed for short trace distances.
Since the chip can operate at such high sample-rates (much higher than
classical interfaces), it requires that a DMA controller be used to
interface directly to the chip and push data into memory.
Typically, the AXI ADC IP core is used to interface with it.
Link: https://www.analog.com/media/en/technical-documentation/data-sheets/AD9467.pdf
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/Kconfig | 15 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad9467.c | 447 +++++++++++++++++++++++++++++++++++++++
3 files changed, 463 insertions(+)
create mode 100644 drivers/iio/adc/ad9467.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 6cd48a256122..229b8bc6f9b6 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -246,6 +246,21 @@ config AD799X
To compile this driver as a module, choose M here: the module will be
called ad799x.
+config AD9467
+ tristate "Analog Devices AD9467 High Speed ADC driver"
+ depends on SPI
+ select AXI_ADC
+ help
+ Say yes here to build support for Analog Devices:
+ * AD9467 16-Bit, 200 MSPS/250 MSPS Analog-to-Digital Converter
+
+ The driver requires the assistance of the AXI ADC IP core to operate,
+ since SPI is used for configuration only, while data has to be
+ streamed into memory via DMA.
+
+ To compile this driver as a module, choose M here: the module will be
+ called ad9467.
+
config ASPEED_ADC
tristate "Aspeed ADC"
depends on ARCH_ASPEED || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index e14fabd53246..5018220b8ec7 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_AD7793) += ad7793.o
obj-$(CONFIG_AD7887) += ad7887.o
obj-$(CONFIG_AD7949) += ad7949.o
obj-$(CONFIG_AD799X) += ad799x.o
+obj-$(CONFIG_AD9467) += ad9467.o
obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
obj-$(CONFIG_AT91_ADC) += at91_adc.o
obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
new file mode 100644
index 000000000000..f268bbb6bcf6
--- /dev/null
+++ b/drivers/iio/adc/ad9467.c
@@ -0,0 +1,447 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices AD9467 SPI ADC driver
+ *
+ * Copyright 2012-2020 Analog Devices Inc.
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#include <linux/clk.h>
+
+#include <linux/iio/adc/axi-adc.h>
+
+/*
+ * ADI High-Speed ADC common spi interface registers
+ * See Application-Note AN-877:
+ * https://www.analog.com/media/en/technical-documentation/application-notes/AN-877.pdf
+ */
+
+#define ADI_ADC_REG_CHIP_PORT_CONF 0x00
+#define ADI_ADC_REG_CHIP_ID 0x01
+#define ADI_ADC_REG_CHIP_GRADE 0x02
+#define ADI_ADC_REG_CHAN_INDEX 0x05
+#define ADI_ADC_REG_TRANSFER 0xFF
+#define ADI_ADC_REG_MODES 0x08
+#define ADI_ADC_REG_TEST_IO 0x0D
+#define ADI_ADC_REG_ADC_INPUT 0x0F
+#define ADI_ADC_REG_OFFSET 0x10
+#define ADI_ADC_REG_OUTPUT_MODE 0x14
+#define ADI_ADC_REG_OUTPUT_ADJUST 0x15
+#define ADI_ADC_REG_OUTPUT_PHASE 0x16
+#define ADI_ADC_REG_OUTPUT_DELAY 0x17
+#define ADI_ADC_REG_VREF 0x18
+#define ADI_ADC_REG_ANALOG_INPUT 0x2C
+
+/* ADI_ADC_REG_TEST_IO */
+#define ADI_ADC_TESTMODE_OFF 0x0
+#define ADI_ADC_TESTMODE_MIDSCALE_SHORT 0x1
+#define ADI_ADC_TESTMODE_POS_FULLSCALE 0x2
+#define ADI_ADC_TESTMODE_NEG_FULLSCALE 0x3
+#define ADI_ADC_TESTMODE_ALT_CHECKERBOARD 0x4
+#define ADI_ADC_TESTMODE_PN23_SEQ 0x5
+#define ADI_ADC_TESTMODE_PN9_SEQ 0x6
+#define ADI_ADC_TESTMODE_ONE_ZERO_TOGGLE 0x7
+#define ADI_ADC_TESTMODE_USER 0x8
+#define ADI_ADC_TESTMODE_BIT_TOGGLE 0x9
+#define ADI_ADC_TESTMODE_SYNC 0xA
+#define ADI_ADC_TESTMODE_ONE_BIT_HIGH 0xB
+#define ADI_ADC_TESTMODE_MIXED_BIT_FREQUENCY 0xC
+#define ADI_ADC_TESTMODE_RAMP 0xF
+
+/* ADI_ADC_REG_TRANSFER */
+#define ADI_ADC_TRANSFER_SYNC 0x1
+
+/* ADI_ADC_REG_OUTPUT_MODE */
+#define ADI_ADC_OUTPUT_MODE_OFFSET_BINARY 0x0
+#define ADI_ADC_OUTPUT_MODE_TWOS_COMPLEMENT 0x1
+#define ADI_ADC_OUTPUT_MODE_GRAY_CODE 0x2
+
+/* ADI_ADC_REG_OUTPUT_PHASE */
+#define ADI_ADC_OUTPUT_EVEN_ODD_MODE_EN 0x20
+#define ADI_ADC_INVERT_DCO_CLK 0x80
+
+/* ADI_ADC_REG_OUTPUT_DELAY */
+#define ADI_ADC_DCO_DELAY_ENABLE 0x80
+
+/*
+ * Analog Devices AD9467 16-Bit, 200/250 MSPS ADC
+ */
+
+#define CHIPID_AD9467 0x50
+#define AD9467_DEF_OUTPUT_MODE 0x08
+#define AD9467_REG_VREF_MASK 0x0F
+
+enum {
+ ID_AD9467,
+};
+
+struct ad9467_state {
+ struct spi_device *spi;
+ struct clk *clk;
+ 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)
+{
+ unsigned char buf[3];
+ int ret;
+
+ buf[0] = 0x80 | (reg >> 8);
+ buf[1] = reg & 0xFF;
+
+ ret = spi_write_then_read(spi, &buf[0], 2, &buf[2], 1);
+
+ if (ret < 0)
+ return ret;
+
+ return buf[2];
+}
+
+static int ad9467_spi_write(struct spi_device *spi, unsigned int reg,
+ unsigned int val)
+{
+ unsigned char buf[3];
+
+ buf[0] = reg >> 8;
+ buf[1] = reg & 0xFF;
+ buf[2] = val;
+
+ return spi_write(spi, buf, ARRAY_SIZE(buf));
+}
+
+static int ad9467_reg_access(struct axi_adc_conv *conv, unsigned int reg,
+ unsigned int writeval, unsigned int *readval)
+{
+ struct ad9467_state *st = axi_adc_conv_priv(conv);
+ struct spi_device *spi = st->spi;
+ int ret;
+
+ if (readval == NULL) {
+ ret = ad9467_spi_write(spi, reg, writeval);
+ ad9467_spi_write(spi, ADI_ADC_REG_TRANSFER,
+ ADI_ADC_TRANSFER_SYNC);
+ return ret;
+ }
+
+ ret = ad9467_spi_read(spi, reg);
+ if (ret < 0)
+ return ret;
+ *readval = ret;
+
+ return 0;
+}
+
+static const unsigned int ad9467_scale_table[][2] = {
+ {2000, 0}, {2100, 6}, {2200, 7},
+ {2300, 8}, {2400, 9}, {2500, 10},
+};
+
+static void __ad9467_get_scale(struct axi_adc_conv *conv, int index,
+ unsigned int *val, unsigned int *val2)
+{
+ const struct axi_adc_chip_info *info = conv->chip_info;
+ const struct iio_chan_spec *chan = &info->channels[0].iio_chan;
+ unsigned int tmp;
+
+ tmp = (info->scale_table[index][0] * 1000000ULL) >>
+ chan->scan_type.realbits;
+ *val = tmp / 1000000;
+ *val2 = tmp % 1000000;
+}
+
+#define AD9467_CHAN(_chan, _si, _bits, _sign) \
+{ \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .channel = _chan, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE) | \
+ BIT(IIO_CHAN_INFO_CALIBBIAS) | \
+ BIT(IIO_CHAN_INFO_CALIBPHASE) | \
+ BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .scan_index = _si, \
+ .scan_type = { \
+ .sign = _sign, \
+ .realbits = _bits, \
+ .storagebits = 16, \
+ .shift = 0, \
+ }, \
+}
+
+static const struct axi_adc_chan_spec ad9467_channels[] = {
+ {
+ .iio_chan = AD9467_CHAN(0, 0, 16, 'S'),
+ .num_lanes = 8,
+ },
+};
+
+static const struct axi_adc_chip_info ad9467_chip_info_tbl[] = {
+ [ID_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),
+ },
+};
+
+static int ad9467_get_scale(struct axi_adc_conv *conv, int *val, int *val2)
+{
+ const struct axi_adc_chip_info *info = conv->chip_info;
+ struct ad9467_state *st = axi_adc_conv_priv(conv);
+ unsigned int i, vref_val, vref_mask;
+
+ vref_val = ad9467_spi_read(st->spi, ADI_ADC_REG_VREF);
+
+ switch (info->id) {
+ case CHIPID_AD9467:
+ vref_mask = AD9467_REG_VREF_MASK;
+ break;
+ default:
+ vref_mask = 0xFFFF;
+ break;
+ }
+
+ vref_val &= vref_mask;
+
+ for (i = 0; i < info->num_scales; i++) {
+ if (vref_val == info->scale_table[i][1])
+ break;
+ }
+
+ if (i == info->num_scales)
+ return -ERANGE;
+
+ __ad9467_get_scale(conv, i, val, val2);
+
+ return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int ad9467_set_scale(struct axi_adc_conv *conv, int val, int val2)
+{
+ const struct axi_adc_chip_info *info = conv->chip_info;
+ struct ad9467_state *st = axi_adc_conv_priv(conv);
+ unsigned int scale_val[2];
+ unsigned int i;
+
+ if (val != 0)
+ return -EINVAL;
+
+ for (i = 0; i < info->num_scales; i++) {
+ __ad9467_get_scale(conv, i, &scale_val[0], &scale_val[1]);
+ if (scale_val[0] != val || scale_val[1] != val2)
+ continue;
+
+ ad9467_spi_write(st->spi, ADI_ADC_REG_VREF,
+ info->scale_table[i][1]);
+ ad9467_spi_write(st->spi, ADI_ADC_REG_TRANSFER,
+ ADI_ADC_TRANSFER_SYNC);
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int ad9467_read_raw(struct axi_adc_conv *conv,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long m)
+{
+ struct ad9467_state *st = axi_adc_conv_priv(conv);
+
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ return ad9467_get_scale(conv, val, val2);
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (!st->clk)
+ return -ENODEV;
+
+ *val = clk_get_rate(st->clk);
+
+ return IIO_VAL_INT;
+
+ }
+ return -EINVAL;
+}
+
+static int ad9467_write_raw(struct axi_adc_conv *conv,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ const struct axi_adc_chip_info *info = conv->chip_info;
+ struct ad9467_state *st = axi_adc_conv_priv(conv);
+ unsigned long r_clk;
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ return ad9467_set_scale(conv, val, val2);
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (!st->clk)
+ return -ENODEV;
+
+ if (chan->extend_name)
+ return -ENODEV;
+
+ r_clk = clk_round_rate(st->clk, val);
+ if (r_clk < 0 || r_clk > info->max_rate) {
+ dev_warn(&st->spi->dev,
+ "Error setting ADC sample rate %ld", r_clk);
+ return -EINVAL;
+ }
+
+ ret = clk_set_rate(st->clk, r_clk);
+ if (ret < 0)
+ return ret;
+
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad9467_outputmode_set(struct spi_device *spi, unsigned int mode)
+{
+ int ret;
+
+ ret = ad9467_spi_write(spi, ADI_ADC_REG_OUTPUT_MODE, mode);
+ if (ret < 0)
+ return ret;
+
+ return ad9467_spi_write(spi, ADI_ADC_REG_TRANSFER,
+ ADI_ADC_TRANSFER_SYNC);
+}
+
+static int ad9467_preenable_setup(struct axi_adc_conv *conv)
+{
+ struct ad9467_state *st = axi_adc_conv_priv(conv);
+
+ return ad9467_outputmode_set(st->spi, st->output_mode);
+}
+
+static int ad9467_setup(struct ad9467_state *st, unsigned int chip_id)
+{
+ switch (chip_id) {
+ case CHIPID_AD9467:
+ st->output_mode = AD9467_DEF_OUTPUT_MODE |
+ ADI_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void ad9467_clk_disable(void *data)
+{
+ struct ad9467_state *st = data;
+
+ clk_disable_unprepare(st->clk);
+}
+
+static int ad9467_probe(struct spi_device *spi)
+{
+ struct axi_adc_conv *conv;
+ struct ad9467_state *st;
+ unsigned int id;
+ int ret;
+
+ conv = devm_axi_adc_conv_register(&spi->dev, sizeof(*st));
+
+ if (IS_ERR(conv))
+ return PTR_ERR(conv);
+
+ st = axi_adc_conv_priv(conv);
+ st->spi = spi;
+
+ st->clk = devm_clk_get(&spi->dev, "sample-clock");
+ if (IS_ERR(st->clk))
+ return PTR_ERR(st->clk);
+
+ ret = clk_prepare_enable(st->clk);
+ if (ret < 0)
+ return ret;
+
+ ret = devm_add_action_or_reset(&spi->dev, ad9467_clk_disable, st);
+ if (ret)
+ return ret;
+
+ st->pwrdown_gpio = devm_gpiod_get_optional(&spi->dev, "powerdown",
+ GPIOD_OUT_LOW);
+ 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);
+ mdelay(10);
+ }
+
+ spi_set_drvdata(spi, st);
+
+ id = spi_get_device_id(spi)->driver_data;
+ conv->chip_info = &ad9467_chip_info_tbl[id];
+
+ id = ad9467_spi_read(spi, ADI_ADC_REG_CHIP_ID);
+ if (id != conv->chip_info->id) {
+ dev_err(&spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
+ return -ENODEV;
+ }
+
+ conv->reg_access = ad9467_reg_access;
+ conv->write_raw = ad9467_write_raw;
+ conv->read_raw = ad9467_read_raw;
+ conv->preenable_setup = ad9467_preenable_setup;
+
+ return ad9467_setup(st, id);
+}
+
+static const struct spi_device_id ad9467_id[] = {
+ { "ad9467", ID_AD9467 },
+ {}
+};
+MODULE_DEVICE_TABLE(spi, ad9467_id);
+
+static const struct of_device_id ad9467_of_match[] = {
+ { .compatible = "adi,ad9467" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ad9467_of_match);
+
+static struct spi_driver ad9467_driver = {
+ .driver = {
+ .name = "ad9467",
+ .of_match_table = ad9467_of_match,
+ },
+ .probe = ad9467_probe,
+ .id_table = ad9467_id,
+};
+module_spi_driver(ad9467_driver);
+
+MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD9467 ADC driver");
+MODULE_LICENSE("GPL v2");
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/5] dt-bindings: iio: adc: add bindings doc for AD9467 ADC
2020-02-20 15:03 [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free Alexandru Ardelean
` (2 preceding siblings ...)
2020-02-20 15:03 ` [PATCH 4/5] iio: adc: ad9467: add support AD9467 ADC Alexandru Ardelean
@ 2020-02-20 15:03 ` Alexandru Ardelean
2020-02-20 20:36 ` Rob Herring
2020-02-21 12:21 ` [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free Jonathan Cameron
4 siblings, 1 reply; 16+ messages in thread
From: Alexandru Ardelean @ 2020-02-20 15:03 UTC (permalink / raw)
To: linux-kernel, linux-iio, devicetree; +Cc: robh+dt, jic23, Alexandru Ardelean
This change adds the binding doc for the AD9467 ADC.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
.../bindings/iio/adc/adi,ad9467.yaml | 63 +++++++++++++++++++
1 file changed, 63 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
new file mode 100644
index 000000000000..e94d9ba294d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad9467.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD9467 High-Speed ADC
+
+maintainers:
+ - Michael Hennerich <michael.hennerich@analog.com>
+ - Alexandru Ardelean <alexandru.ardelean@analog.com>
+
+description: |
+ The AD9467 is a 16-bit, monolithic, IF sampling analog-to-digital
+ converter (ADC).
+
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD9467.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad9467
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clocks:
+ items:
+ - const: sample-clock
+
+ powerdown-gpios:
+ description:
+ Pin that controls the powerdown mode of the device.
+ maxItems: 1
+
+ reset-gpios:
+ description:
+ Reset pin for the device.
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "adi,ad9467";
+ reg = <0>;
+ };
+ };
+...
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/5] dt-bindings: iio: adc: add bindings doc for AXI ADC driver
2020-02-20 15:03 ` [PATCH 3/5] dt-bindings: iio: adc: add bindings doc for AXI ADC driver Alexandru Ardelean
@ 2020-02-20 20:35 ` Rob Herring
0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2020-02-20 20:35 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-kernel, linux-iio, devicetree, robh+dt, jic23, Alexandru Ardelean
On Thu, 20 Feb 2020 17:03:15 +0200, Alexandru Ardelean wrote:
> This change adds the bindings documentation for the AXI ADC driver.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> .../bindings/iio/adc/adi,axi-adc.yaml | 70 +++++++++++++++++++
> 1 file changed, 70 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.example.dt.yaml: axi-adc@44a00000: 'dma-names' does not match any of the regexes: 'pinctrl-[0-9]+'
See https://patchwork.ozlabs.org/patch/1241489
Please check and re-submit.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] dt-bindings: iio: adc: add bindings doc for AD9467 ADC
2020-02-20 15:03 ` [PATCH 5/5] dt-bindings: iio: adc: add bindings doc for " Alexandru Ardelean
@ 2020-02-20 20:36 ` Rob Herring
0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2020-02-20 20:36 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-kernel, linux-iio, devicetree, robh+dt, jic23, Alexandru Ardelean
On Thu, 20 Feb 2020 17:03:17 +0200, Alexandru Ardelean wrote:
> This change adds the binding doc for the AD9467 ADC.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> .../bindings/iio/adc/adi,ad9467.yaml | 63 +++++++++++++++++++
> 1 file changed, 63 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
Traceback (most recent call last):
File "/usr/local/bin/dt-mk-schema", line 32, in <module>
schemas = dtschema.process_schemas(args.schemas, core_schema=(not args.useronly))
File "/usr/local/lib/python3.6/dist-packages/dtschema/lib.py", line 475, in process_schemas
sch = process_schema(os.path.abspath(filename))
File "/usr/local/lib/python3.6/dist-packages/dtschema/lib.py", line 428, in process_schema
schema = load_schema(filename)
File "/usr/local/lib/python3.6/dist-packages/dtschema/lib.py", line 108, in load_schema
return do_load(os.path.join(schema_basedir, schema))
File "/usr/local/lib/python3.6/dist-packages/dtschema/lib.py", line 93, in do_load
return yaml.load(tmp)
File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/main.py", line 343, in load
return constructor.get_single_data()
File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/constructor.py", line 113, in get_single_data
return self.construct_document(node)
File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/constructor.py", line 123, in construct_document
for _dummy in generator:
File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/constructor.py", line 723, in construct_yaml_map
value = self.construct_mapping(node)
File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/constructor.py", line 440, in construct_mapping
return BaseConstructor.construct_mapping(self, node, deep=deep)
File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/constructor.py", line 257, in construct_mapping
if self.check_mapping_key(node, key_node, mapping, key, value):
File "/usr/local/lib/python3.6/dist-packages/ruamel/yaml/constructor.py", line 295, in check_mapping_key
raise DuplicateKeyError(*args)
ruamel.yaml.constructor.DuplicateKeyError: while constructing a mapping
in "<unicode string>", line 20, column 3
found duplicate key "clocks" with value "{}" (original value: "{}")
in "<unicode string>", line 30, column 3
To suppress this check see:
http://yaml.readthedocs.io/en/latest/api.html#duplicate-keys
Duplicate keys will become an error in future releases, and are errors
by default when using the new API.
Documentation/devicetree/bindings/Makefile:34: recipe for target 'Documentation/devicetree/bindings/processed-schema.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/processed-schema.yaml] Error 1
Makefile:1263: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1241490
Please check and re-submit.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free
2020-02-20 15:03 [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free Alexandru Ardelean
` (3 preceding siblings ...)
2020-02-20 15:03 ` [PATCH 5/5] dt-bindings: iio: adc: add bindings doc for " Alexandru Ardelean
@ 2020-02-21 12:21 ` Jonathan Cameron
2020-02-25 13:33 ` Ardelean, Alexandru
4 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2020-02-21 12:21 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-kernel, linux-iio, devicetree, robh+dt
On Thu, 20 Feb 2020 17:03:13 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> Currently, when using a 'iio_dmaengine_buffer_alloc()', an matching call to
> 'iio_dmaengine_buffer_free()' must be made.
>
> With this change, this can be avoided by using
> 'devm_iio_dmaengine_buffer_alloc()'. The buffer will get free'd via the
> device's devres handling.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> .../buffer/industrialio-buffer-dmaengine.c | 70 +++++++++++++++++++
> include/linux/iio/buffer-dmaengine.h | 5 ++
> 2 files changed, 75 insertions(+)
>
> diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> index b129693af0fd..eff89037e3f5 100644
> --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> @@ -229,6 +229,76 @@ void iio_dmaengine_buffer_free(struct iio_buffer *buffer)
> }
> EXPORT_SYMBOL_GPL(iio_dmaengine_buffer_free);
>
> +static void __devm_iio_dmaengine_buffer_free(struct device *dev, void *res)
> +{
> + iio_dmaengine_buffer_free(*(struct iio_buffer **)res);
> +}
> +
> +/**
> + * devm_iio_dmaengine_buffer_alloc() - Resource-managed iio_dmaengine_buffer_alloc()
> + * @dev: Parent device for the buffer
> + * @channel: DMA channel name, typically "rx".
> + *
> + * This allocates a new IIO buffer which internally uses the DMAengine framework
> + * to perform its transfers. The parent device will be used to request the DMA
> + * channel.
> + *
> + * Once done using the buffer iio_dmaengine_buffer_free() should be used to
> + * release it.
Umm. It really shouldn't!
> + */
> +struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
> + const char *channel)
> +{
> + struct iio_buffer **bufferp, *buffer;
> +
> + bufferp = devres_alloc(__devm_iio_dmaengine_buffer_free,
> + sizeof(*bufferp), GFP_KERNEL);
> + if (!bufferp)
> + return ERR_PTR(-ENOMEM);
> +
> + buffer = iio_dmaengine_buffer_alloc(dev, channel);
> + if (!IS_ERR(buffer)) {
> + *bufferp = buffer;
> + devres_add(dev, bufferp);
From a flow point of view I'd prefer.
if (IS_ERR(buffer) {
devres_free(buferp)
return buffer;
}
*bufferp = buffer;
devres_add(dev, bufferp);
return buffer;
> + } else {
> + devres_free(bufferp);
> + }
> +
> + return buffer;
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_alloc);
> +
> +static int devm_iio_dmaengine_buffer_match(struct device *dev, void *res,
> + void *data)
> +{
> + struct iio_buffer **r = res;
> +
> + if (!r || !*r) {
> + WARN_ON(!r || !*r);
> + return 0;
> + }
> +
> + return *r == data;
> +}
> +
> +/**
> + * devm_iio_dmaengine_buffer_free - iio_dmaengine_buffer_free
> + * @dev: Device this iio_buffer belongs to
> + * @buffer: The iio_buffer associated with the device
> + *
> + * Free buffer allocated with devm_iio_dmaengine_buffer_alloc().
> + */
> +void devm_iio_dmaengine_buffer_free(struct device *dev,
> + struct iio_buffer *buffer)
> +{
> + int rc;
> +
> + rc = devres_release(dev, __devm_iio_dmaengine_buffer_free,
> + devm_iio_dmaengine_buffer_match, buffer);
> + WARN_ON(rc);
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_free);
> +
> MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
> MODULE_DESCRIPTION("DMA buffer for the IIO framework");
> MODULE_LICENSE("GPL");
> diff --git a/include/linux/iio/buffer-dmaengine.h b/include/linux/iio/buffer-dmaengine.h
> index b3a57444a886..8dcd973d76c1 100644
> --- a/include/linux/iio/buffer-dmaengine.h
> +++ b/include/linux/iio/buffer-dmaengine.h
> @@ -14,4 +14,9 @@ struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev,
> const char *channel);
> void iio_dmaengine_buffer_free(struct iio_buffer *buffer);
>
> +struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
> + const char *channel);
> +void devm_iio_dmaengine_buffer_free(struct device *dev,
> + struct iio_buffer *buffer);
Please align parameters with opening bracket where possible.
Thanks,
Jonathan
> +
> #endif
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] iio: adc: axi-adc: add support for AXI ADC IP core
2020-02-20 15:03 ` [PATCH 2/5] iio: adc: axi-adc: add support for AXI ADC IP core Alexandru Ardelean
@ 2020-02-21 12:44 ` Jonathan Cameron
2020-02-25 13:51 ` Ardelean, Alexandru
0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2020-02-21 12:44 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-kernel, linux-iio, devicetree, robh+dt, Michael Hennerich,
Lars-Peter Clausen
On Thu, 20 Feb 2020 17:03:14 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
>
> 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@analog.com>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
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@analog.com>");
> +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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] iio: adc: ad9467: add support AD9467 ADC
2020-02-20 15:03 ` [PATCH 4/5] iio: adc: ad9467: add support AD9467 ADC Alexandru Ardelean
@ 2020-02-21 12:57 ` Jonathan Cameron
2020-02-25 15:21 ` Ardelean, Alexandru
2020-02-26 11:30 ` Ardelean, Alexandru
0 siblings, 2 replies; 16+ messages in thread
From: Jonathan Cameron @ 2020-02-21 12:57 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-kernel, linux-iio, devicetree, robh+dt, Michael Hennerich,
Lars-Peter Clausen
On Thu, 20 Feb 2020 17:03:16 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
>
> The AD9467 is a 16-bit, monolithic, IF sampling analog-to-digital converter
> (ADC). It is optimized for high performanceover wide bandwidths and ease of
> use. The product operates at a 250 MSPS conversion rate and is designed for
> wireless receivers, instrumentation, and test equipment that require a high
> dynamic range. The ADC requires 1.8 V and 3.3 V power supplies and a low
> voltage differential input clock for full performance operation. No
> external reference or driver components are required for many applications.
> Data outputs are LVDS compatible (ANSI-644 compatible) and include the
> means to reduce the overall current needed for short trace distances.
>
> Since the chip can operate at such high sample-rates (much higher than
> classical interfaces), it requires that a DMA controller be used to
> interface directly to the chip and push data into memory.
> Typically, the AXI ADC IP core is used to interface with it.
>
> Link: https://www.analog.com/media/en/technical-documentation/data-sheets/AD9467.pdf
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
A few minor things inline.
Jonathan
> ---
> drivers/iio/adc/Kconfig | 15 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad9467.c | 447 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 463 insertions(+)
> create mode 100644 drivers/iio/adc/ad9467.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 6cd48a256122..229b8bc6f9b6 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -246,6 +246,21 @@ config AD799X
> To compile this driver as a module, choose M here: the module will be
> called ad799x.
>
> +config AD9467
> + tristate "Analog Devices AD9467 High Speed ADC driver"
> + depends on SPI
> + select AXI_ADC
> + help
> + Say yes here to build support for Analog Devices:
> + * AD9467 16-Bit, 200 MSPS/250 MSPS Analog-to-Digital Converter
> +
> + The driver requires the assistance of the AXI ADC IP core to operate,
> + since SPI is used for configuration only, while data has to be
> + streamed into memory via DMA.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called ad9467.
> +
> config ASPEED_ADC
> tristate "Aspeed ADC"
> depends on ARCH_ASPEED || COMPILE_TEST
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index e14fabd53246..5018220b8ec7 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_AD7793) += ad7793.o
> obj-$(CONFIG_AD7887) += ad7887.o
> obj-$(CONFIG_AD7949) += ad7949.o
> obj-$(CONFIG_AD799X) += ad799x.o
> +obj-$(CONFIG_AD9467) += ad9467.o
> obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
> obj-$(CONFIG_AT91_ADC) += at91_adc.o
> obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> new file mode 100644
> index 000000000000..f268bbb6bcf6
> --- /dev/null
> +++ b/drivers/iio/adc/ad9467.c
> @@ -0,0 +1,447 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD9467 SPI ADC driver
> + *
> + * Copyright 2012-2020 Analog Devices Inc.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/spi/spi.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/of.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include <linux/clk.h>
> +
> +#include <linux/iio/adc/axi-adc.h>
> +
> +/*
> + * ADI High-Speed ADC common spi interface registers
> + * See Application-Note AN-877:
> + * https://www.analog.com/media/en/technical-documentation/application-notes/AN-877.pdf
> + */
> +
> +#define ADI_ADC_REG_CHIP_PORT_CONF 0x00
These prefixes should reflect the chip we are supporting here.
They aren't true for 'all' ADI parts.
You could come up with some 'generic' but not to generic prefix
if you prefer.
> +#define ADI_ADC_REG_CHIP_ID 0x01
> +#define ADI_ADC_REG_CHIP_GRADE 0x02
> +#define ADI_ADC_REG_CHAN_INDEX 0x05
> +#define ADI_ADC_REG_TRANSFER 0xFF
> +#define ADI_ADC_REG_MODES 0x08
> +#define ADI_ADC_REG_TEST_IO 0x0D
> +#define ADI_ADC_REG_ADC_INPUT 0x0F
> +#define ADI_ADC_REG_OFFSET 0x10
> +#define ADI_ADC_REG_OUTPUT_MODE 0x14
> +#define ADI_ADC_REG_OUTPUT_ADJUST 0x15
> +#define ADI_ADC_REG_OUTPUT_PHASE 0x16
> +#define ADI_ADC_REG_OUTPUT_DELAY 0x17
> +#define ADI_ADC_REG_VREF 0x18
> +#define ADI_ADC_REG_ANALOG_INPUT 0x2C
> +
> +/* ADI_ADC_REG_TEST_IO */
> +#define ADI_ADC_TESTMODE_OFF 0x0
> +#define ADI_ADC_TESTMODE_MIDSCALE_SHORT 0x1
> +#define ADI_ADC_TESTMODE_POS_FULLSCALE 0x2
> +#define ADI_ADC_TESTMODE_NEG_FULLSCALE 0x3
> +#define ADI_ADC_TESTMODE_ALT_CHECKERBOARD 0x4
> +#define ADI_ADC_TESTMODE_PN23_SEQ 0x5
> +#define ADI_ADC_TESTMODE_PN9_SEQ 0x6
> +#define ADI_ADC_TESTMODE_ONE_ZERO_TOGGLE 0x7
> +#define ADI_ADC_TESTMODE_USER 0x8
> +#define ADI_ADC_TESTMODE_BIT_TOGGLE 0x9
> +#define ADI_ADC_TESTMODE_SYNC 0xA
> +#define ADI_ADC_TESTMODE_ONE_BIT_HIGH 0xB
> +#define ADI_ADC_TESTMODE_MIXED_BIT_FREQUENCY 0xC
> +#define ADI_ADC_TESTMODE_RAMP 0xF
> +
> +/* ADI_ADC_REG_TRANSFER */
> +#define ADI_ADC_TRANSFER_SYNC 0x1
> +
> +/* ADI_ADC_REG_OUTPUT_MODE */
> +#define ADI_ADC_OUTPUT_MODE_OFFSET_BINARY 0x0
> +#define ADI_ADC_OUTPUT_MODE_TWOS_COMPLEMENT 0x1
> +#define ADI_ADC_OUTPUT_MODE_GRAY_CODE 0x2
> +
> +/* ADI_ADC_REG_OUTPUT_PHASE */
> +#define ADI_ADC_OUTPUT_EVEN_ODD_MODE_EN 0x20
> +#define ADI_ADC_INVERT_DCO_CLK 0x80
> +
> +/* ADI_ADC_REG_OUTPUT_DELAY */
> +#define ADI_ADC_DCO_DELAY_ENABLE 0x80
> +
> +/*
> + * Analog Devices AD9467 16-Bit, 200/250 MSPS ADC
> + */
> +
> +#define CHIPID_AD9467 0x50
> +#define AD9467_DEF_OUTPUT_MODE 0x08
> +#define AD9467_REG_VREF_MASK 0x0F
> +
> +enum {
> + ID_AD9467,
> +};
> +
> +struct ad9467_state {
> + struct spi_device *spi;
> + struct clk *clk;
> + 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)
> +{
> + unsigned char buf[3];
> + int ret;
> +
> + buf[0] = 0x80 | (reg >> 8);
> + buf[1] = reg & 0xFF;
> +
> + ret = spi_write_then_read(spi, &buf[0], 2, &buf[2], 1);
> +
> + if (ret < 0)
> + return ret;
> +
> + return buf[2];
> +}
> +
> +static int ad9467_spi_write(struct spi_device *spi, unsigned int reg,
> + unsigned int val)
> +{
> + unsigned char buf[3];
> +
> + buf[0] = reg >> 8;
> + buf[1] = reg & 0xFF;
> + buf[2] = val;
> +
> + return spi_write(spi, buf, ARRAY_SIZE(buf));
> +}
> +
> +static int ad9467_reg_access(struct axi_adc_conv *conv, unsigned int reg,
> + unsigned int writeval, unsigned int *readval)
> +{
> + struct ad9467_state *st = axi_adc_conv_priv(conv);
> + struct spi_device *spi = st->spi;
> + int ret;
> +
> + if (readval == NULL) {
> + ret = ad9467_spi_write(spi, reg, writeval);
> + ad9467_spi_write(spi, ADI_ADC_REG_TRANSFER,
> + ADI_ADC_TRANSFER_SYNC);
> + return ret;
> + }
> +
> + ret = ad9467_spi_read(spi, reg);
> + if (ret < 0)
> + return ret;
> + *readval = ret;
> +
> + return 0;
> +}
> +
> +static const unsigned int ad9467_scale_table[][2] = {
> + {2000, 0}, {2100, 6}, {2200, 7},
> + {2300, 8}, {2400, 9}, {2500, 10},
> +};
> +
> +static void __ad9467_get_scale(struct axi_adc_conv *conv, int index,
> + unsigned int *val, unsigned int *val2)
> +{
> + const struct axi_adc_chip_info *info = conv->chip_info;
> + const struct iio_chan_spec *chan = &info->channels[0].iio_chan;
> + unsigned int tmp;
> +
> + tmp = (info->scale_table[index][0] * 1000000ULL) >>
> + chan->scan_type.realbits;
> + *val = tmp / 1000000;
> + *val2 = tmp % 1000000;
> +}
> +
> +#define AD9467_CHAN(_chan, _si, _bits, _sign) \
> +{ \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .channel = _chan, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE) | \
> + BIT(IIO_CHAN_INFO_CALIBBIAS) | \
> + BIT(IIO_CHAN_INFO_CALIBPHASE) | \
These don't seem to be handled that I can see...
> + BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .scan_index = _si, \
> + .scan_type = { \
> + .sign = _sign, \
> + .realbits = _bits, \
> + .storagebits = 16, \
> + .shift = 0, \
shift of 0 is the "obvious" default so no need to specify it...
> + }, \
> +}
> +
> +static const struct axi_adc_chan_spec ad9467_channels[] = {
> + {
> + .iio_chan = AD9467_CHAN(0, 0, 16, 'S'),
> + .num_lanes = 8,
> + },
> +};
> +
> +static const struct axi_adc_chip_info ad9467_chip_info_tbl[] = {
> + [ID_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),
> + },
> +};
> +
> +static int ad9467_get_scale(struct axi_adc_conv *conv, int *val, int *val2)
> +{
> + const struct axi_adc_chip_info *info = conv->chip_info;
> + struct ad9467_state *st = axi_adc_conv_priv(conv);
> + unsigned int i, vref_val, vref_mask;
> +
> + vref_val = ad9467_spi_read(st->spi, ADI_ADC_REG_VREF);
> +
> + switch (info->id) {
> + case CHIPID_AD9467:
> + vref_mask = AD9467_REG_VREF_MASK;
> + break;
> + default:
> + vref_mask = 0xFFFF;
> + break;
> + }
> +
> + vref_val &= vref_mask;
> +
> + for (i = 0; i < info->num_scales; i++) {
> + if (vref_val == info->scale_table[i][1])
> + break;
> + }
> +
> + if (i == info->num_scales)
> + return -ERANGE;
> +
> + __ad9467_get_scale(conv, i, val, val2);
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> +}
> +
> +static int ad9467_set_scale(struct axi_adc_conv *conv, int val, int val2)
> +{
> + const struct axi_adc_chip_info *info = conv->chip_info;
> + struct ad9467_state *st = axi_adc_conv_priv(conv);
> + unsigned int scale_val[2];
> + unsigned int i;
> +
> + if (val != 0)
> + return -EINVAL;
> +
> + for (i = 0; i < info->num_scales; i++) {
> + __ad9467_get_scale(conv, i, &scale_val[0], &scale_val[1]);
> + if (scale_val[0] != val || scale_val[1] != val2)
> + continue;
> +
> + ad9467_spi_write(st->spi, ADI_ADC_REG_VREF,
> + info->scale_table[i][1]);
> + ad9467_spi_write(st->spi, ADI_ADC_REG_TRANSFER,
> + ADI_ADC_TRANSFER_SYNC);
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int ad9467_read_raw(struct axi_adc_conv *conv,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long m)
> +{
> + struct ad9467_state *st = axi_adc_conv_priv(conv);
> +
> + switch (m) {
> + case IIO_CHAN_INFO_SCALE:
> + return ad9467_get_scale(conv, val, val2);
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (!st->clk)
> + return -ENODEV;
> +
> + *val = clk_get_rate(st->clk);
> +
> + return IIO_VAL_INT;
> +
> + }
> + return -EINVAL;
I'd put that in a default in the switch as you may get warnings
from some static checkers otherwise.
> +}
> +
> +static int ad9467_write_raw(struct axi_adc_conv *conv,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + const struct axi_adc_chip_info *info = conv->chip_info;
> + struct ad9467_state *st = axi_adc_conv_priv(conv);
> + unsigned long r_clk;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + return ad9467_set_scale(conv, val, val2);
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (!st->clk)
> + return -ENODEV;
> +
> + if (chan->extend_name)
> + return -ENODEV;
> +
> + r_clk = clk_round_rate(st->clk, val);
> + if (r_clk < 0 || r_clk > info->max_rate) {
> + dev_warn(&st->spi->dev,
> + "Error setting ADC sample rate %ld", r_clk);
> + return -EINVAL;
> + }
> +
> + ret = clk_set_rate(st->clk, r_clk);
> + if (ret < 0)
> + return ret;
return clk_set_rate(st->clk, r_clk) is probably the same.
Might as well do early returns everywhere.
> +
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int ad9467_outputmode_set(struct spi_device *spi, unsigned int mode)
> +{
> + int ret;
> +
> + ret = ad9467_spi_write(spi, ADI_ADC_REG_OUTPUT_MODE, mode);
> + if (ret < 0)
> + return ret;
> +
> + return ad9467_spi_write(spi, ADI_ADC_REG_TRANSFER,
> + ADI_ADC_TRANSFER_SYNC);
> +}
> +
> +static int ad9467_preenable_setup(struct axi_adc_conv *conv)
> +{
> + struct ad9467_state *st = axi_adc_conv_priv(conv);
> +
> + return ad9467_outputmode_set(st->spi, st->output_mode);
> +}
> +
> +static int ad9467_setup(struct ad9467_state *st, unsigned int chip_id)
> +{
> + switch (chip_id) {
> + case CHIPID_AD9467:
> + st->output_mode = AD9467_DEF_OUTPUT_MODE |
> + ADI_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
> + break;
return 0 unless you are going to add anything after the switch statement.
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static void ad9467_clk_disable(void *data)
> +{
> + struct ad9467_state *st = data;
> +
> + clk_disable_unprepare(st->clk);
> +}
> +
> +static int ad9467_probe(struct spi_device *spi)
> +{
> + struct axi_adc_conv *conv;
> + struct ad9467_state *st;
> + unsigned int id;
> + int ret;
> +
> + conv = devm_axi_adc_conv_register(&spi->dev, sizeof(*st));
> +
No blank line between a call and it's error handler.
> + if (IS_ERR(conv))
> + return PTR_ERR(conv);
> +
> + st = axi_adc_conv_priv(conv);
> + st->spi = spi;
> +
> + st->clk = devm_clk_get(&spi->dev, "sample-clock");
> + if (IS_ERR(st->clk))
> + return PTR_ERR(st->clk);
> +
> + ret = clk_prepare_enable(st->clk);
> + if (ret < 0)
> + return ret;
> +
> + ret = devm_add_action_or_reset(&spi->dev, ad9467_clk_disable, st);
> + if (ret)
> + return ret;
> +
> + st->pwrdown_gpio = devm_gpiod_get_optional(&spi->dev, "powerdown",
> + GPIOD_OUT_LOW);
> + 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);
> + mdelay(10);
> + }
> +
> + spi_set_drvdata(spi, st);
> +
> + id = spi_get_device_id(spi)->driver_data;
> + conv->chip_info = &ad9467_chip_info_tbl[id];
> +
> + id = ad9467_spi_read(spi, ADI_ADC_REG_CHIP_ID);
> + if (id != conv->chip_info->id) {
> + dev_err(&spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
> + return -ENODEV;
> + }
> +
> + conv->reg_access = ad9467_reg_access;
> + conv->write_raw = ad9467_write_raw;
> + conv->read_raw = ad9467_read_raw;
> + conv->preenable_setup = ad9467_preenable_setup;
> +
> + return ad9467_setup(st, id);
> +}
> +
> +static const struct spi_device_id ad9467_id[] = {
> + { "ad9467", ID_AD9467 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(spi, ad9467_id);
> +
> +static const struct of_device_id ad9467_of_match[] = {
> + { .compatible = "adi,ad9467" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, ad9467_of_match);
> +
> +static struct spi_driver ad9467_driver = {
> + .driver = {
> + .name = "ad9467",
> + .of_match_table = ad9467_of_match,
> + },
> + .probe = ad9467_probe,
> + .id_table = ad9467_id,
This is something I've only just started raising in reviews.
If a driver can't realistically be instantiated without firmware
bindings, there isn't really any point in providing the id_table
that I can think of so please remove.
> +};
> +module_spi_driver(ad9467_driver);
> +
> +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD9467 ADC driver");
> +MODULE_LICENSE("GPL v2");
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free
2020-02-21 12:21 ` [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free Jonathan Cameron
@ 2020-02-25 13:33 ` Ardelean, Alexandru
2020-03-01 16:02 ` Jonathan Cameron
0 siblings, 1 reply; 16+ messages in thread
From: Ardelean, Alexandru @ 2020-02-25 13:33 UTC (permalink / raw)
To: jic23; +Cc: devicetree, linux-kernel, linux-iio, robh+dt
On Fri, 2020-02-21 at 12:21 +0000, Jonathan Cameron wrote:
> [External]
>
> On Thu, 20 Feb 2020 17:03:13 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > Currently, when using a 'iio_dmaengine_buffer_alloc()', an matching call to
> > 'iio_dmaengine_buffer_free()' must be made.
> >
> > With this change, this can be avoided by using
> > 'devm_iio_dmaengine_buffer_alloc()'. The buffer will get free'd via the
> > device's devres handling.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > .../buffer/industrialio-buffer-dmaengine.c | 70 +++++++++++++++++++
> > include/linux/iio/buffer-dmaengine.h | 5 ++
> > 2 files changed, 75 insertions(+)
> >
> > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > index b129693af0fd..eff89037e3f5 100644
> > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > @@ -229,6 +229,76 @@ void iio_dmaengine_buffer_free(struct iio_buffer
> > *buffer)
> > }
> > EXPORT_SYMBOL_GPL(iio_dmaengine_buffer_free);
> >
> > +static void __devm_iio_dmaengine_buffer_free(struct device *dev, void *res)
> > +{
> > + iio_dmaengine_buffer_free(*(struct iio_buffer **)res);
> > +}
> > +
> > +/**
> > + * devm_iio_dmaengine_buffer_alloc() - Resource-managed
> > iio_dmaengine_buffer_alloc()
> > + * @dev: Parent device for the buffer
> > + * @channel: DMA channel name, typically "rx".
> > + *
> > + * This allocates a new IIO buffer which internally uses the DMAengine
> > framework
> > + * to perform its transfers. The parent device will be used to request the
> > DMA
> > + * channel.
> > + *
> > + * Once done using the buffer iio_dmaengine_buffer_free() should be used to
> > + * release it.
>
> Umm. It really shouldn't!
Yeah.
Copy+paste.
My bad.
> > + */
> > +struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
> > + const char *channel)
> > +{
> > + struct iio_buffer **bufferp, *buffer;
> > +
> > + bufferp = devres_alloc(__devm_iio_dmaengine_buffer_free,
> > + sizeof(*bufferp), GFP_KERNEL);
> > + if (!bufferp)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + buffer = iio_dmaengine_buffer_alloc(dev, channel);
> > + if (!IS_ERR(buffer)) {
> > + *bufferp = buffer;
> > + devres_add(dev, bufferp);
>
> From a flow point of view I'd prefer.
>
> if (IS_ERR(buffer) {
> devres_free(buferp)
> return buffer;
> }
>
> *bufferp = buffer;
> devres_add(dev, bufferp);
>
> return buffer;
>
>
> > + } else {
> > + devres_free(bufferp);
> > + }
> > +
> > + return buffer;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_alloc);
> > +
> > +static int devm_iio_dmaengine_buffer_match(struct device *dev, void *res,
> > + void *data)
> > +{
> > + struct iio_buffer **r = res;
> > +
> > + if (!r || !*r) {
> > + WARN_ON(!r || !*r);
> > + return 0;
> > + }
> > +
> > + return *r == data;
> > +}
> > +
> > +/**
> > + * devm_iio_dmaengine_buffer_free - iio_dmaengine_buffer_free
> > + * @dev: Device this iio_buffer belongs to
> > + * @buffer: The iio_buffer associated with the device
> > + *
> > + * Free buffer allocated with devm_iio_dmaengine_buffer_alloc().
> > + */
> > +void devm_iio_dmaengine_buffer_free(struct device *dev,
> > + struct iio_buffer *buffer)
> > +{
> > + int rc;
> > +
> > + rc = devres_release(dev, __devm_iio_dmaengine_buffer_free,
> > + devm_iio_dmaengine_buffer_match, buffer);
> > + WARN_ON(rc);
> > +}
> > +EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_free);
Should I remove devm_iio_dmaengine_buffer_free() ?
There was a comment on the AXI ADC that may apply here as well, about maybe
removing it.
> > +
> > MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
> > MODULE_DESCRIPTION("DMA buffer for the IIO framework");
> > MODULE_LICENSE("GPL");
> > diff --git a/include/linux/iio/buffer-dmaengine.h
> > b/include/linux/iio/buffer-dmaengine.h
> > index b3a57444a886..8dcd973d76c1 100644
> > --- a/include/linux/iio/buffer-dmaengine.h
> > +++ b/include/linux/iio/buffer-dmaengine.h
> > @@ -14,4 +14,9 @@ struct iio_buffer *iio_dmaengine_buffer_alloc(struct
> > device *dev,
> > const char *channel);
> > void iio_dmaengine_buffer_free(struct iio_buffer *buffer);
> >
> > +struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
> > + const char *channel);
> > +void devm_iio_dmaengine_buffer_free(struct device *dev,
> > + struct iio_buffer *buffer);
> Please align parameters with opening bracket where possible.
>
ack
> Thanks,
>
> Jonathan
>
> > +
> > #endif
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] iio: adc: axi-adc: add support for AXI ADC IP core
2020-02-21 12:44 ` Jonathan Cameron
@ 2020-02-25 13:51 ` Ardelean, Alexandru
0 siblings, 0 replies; 16+ messages in thread
From: Ardelean, Alexandru @ 2020-02-25 13:51 UTC (permalink / raw)
To: jic23
Cc: Hennerich, Michael, devicetree, linux-kernel, linux-iio, robh+dt, lars
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@analog.com> wrote:
>
> > From: Michael Hennerich <michael.hennerich@analog.com>
> >
> > 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@analog.com>
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>
> 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@analog.com>");
> > +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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] iio: adc: ad9467: add support AD9467 ADC
2020-02-21 12:57 ` Jonathan Cameron
@ 2020-02-25 15:21 ` Ardelean, Alexandru
2020-02-26 11:30 ` Ardelean, Alexandru
1 sibling, 0 replies; 16+ messages in thread
From: Ardelean, Alexandru @ 2020-02-25 15:21 UTC (permalink / raw)
To: jic23
Cc: Hennerich, Michael, devicetree, linux-kernel, linux-iio, robh+dt, lars
On Fri, 2020-02-21 at 12:57 +0000, Jonathan Cameron wrote:
> On Thu, 20 Feb 2020 17:03:16 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > From: Michael Hennerich <michael.hennerich@analog.com>
> >
> > The AD9467 is a 16-bit, monolithic, IF sampling analog-to-digital converter
> > (ADC). It is optimized for high performanceover wide bandwidths and ease of
> > use. The product operates at a 250 MSPS conversion rate and is designed for
> > wireless receivers, instrumentation, and test equipment that require a high
> > dynamic range. The ADC requires 1.8 V and 3.3 V power supplies and a low
> > voltage differential input clock for full performance operation. No
> > external reference or driver components are required for many applications.
> > Data outputs are LVDS compatible (ANSI-644 compatible) and include the
> > means to reduce the overall current needed for short trace distances.
> >
> > Since the chip can operate at such high sample-rates (much higher than
> > classical interfaces), it requires that a DMA controller be used to
> > interface directly to the chip and push data into memory.
> > Typically, the AXI ADC IP core is used to interface with it.
> >
> > Link:
> > https://www.analog.com/media/en/technical-documentation/data-sheets/AD9467.pdf
> >
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>
> A few minor things inline.
>
> Jonathan
> > ---
> > drivers/iio/adc/Kconfig | 15 ++
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/ad9467.c | 447 +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 463 insertions(+)
> > create mode 100644 drivers/iio/adc/ad9467.c
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 6cd48a256122..229b8bc6f9b6 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -246,6 +246,21 @@ config AD799X
> > To compile this driver as a module, choose M here: the module will be
> > called ad799x.
> >
> > +config AD9467
> > + tristate "Analog Devices AD9467 High Speed ADC driver"
> > + depends on SPI
> > + select AXI_ADC
> > + help
> > + Say yes here to build support for Analog Devices:
> > + * AD9467 16-Bit, 200 MSPS/250 MSPS Analog-to-Digital Converter
> > +
> > + The driver requires the assistance of the AXI ADC IP core to operate,
> > + since SPI is used for configuration only, while data has to be
> > + streamed into memory via DMA.
> > +
> > + To compile this driver as a module, choose M here: the module will be
> > + called ad9467.
> > +
> > config ASPEED_ADC
> > tristate "Aspeed ADC"
> > depends on ARCH_ASPEED || COMPILE_TEST
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index e14fabd53246..5018220b8ec7 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -26,6 +26,7 @@ obj-$(CONFIG_AD7793) += ad7793.o
> > obj-$(CONFIG_AD7887) += ad7887.o
> > obj-$(CONFIG_AD7949) += ad7949.o
> > obj-$(CONFIG_AD799X) += ad799x.o
> > +obj-$(CONFIG_AD9467) += ad9467.o
> > obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
> > obj-$(CONFIG_AT91_ADC) += at91_adc.o
> > obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
> > diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> > new file mode 100644
> > index 000000000000..f268bbb6bcf6
> > --- /dev/null
> > +++ b/drivers/iio/adc/ad9467.c
> > @@ -0,0 +1,447 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Analog Devices AD9467 SPI ADC driver
> > + *
> > + * Copyright 2012-2020 Analog Devices Inc.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/err.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/of.h>
> > +
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +
> > +#include <linux/clk.h>
> > +
> > +#include <linux/iio/adc/axi-adc.h>
> > +
> > +/*
> > + * ADI High-Speed ADC common spi interface registers
> > + * See Application-Note AN-877:
> > + *
> > https://www.analog.com/media/en/technical-documentation/application-notes/AN-877.pdf
> > + */
> > +
> > +#define ADI_ADC_REG_CHIP_PORT_CONF 0x00
>
> These prefixes should reflect the chip we are supporting here.
> They aren't true for 'all' ADI parts.
>
> You could come up with some 'generic' but not to generic prefix
> if you prefer.
The regs are defined in the AN-877 doc references above, but yeah... they're not
very generic to all ADCs.
Mostly to this family [supported by this driver].
I'll change it.
>
> > +#define ADI_ADC_REG_CHIP_ID 0x01
> > +#define ADI_ADC_REG_CHIP_GRADE 0x02
> > +#define ADI_ADC_REG_CHAN_INDEX 0x05
> > +#define ADI_ADC_REG_TRANSFER 0xFF
> > +#define ADI_ADC_REG_MODES 0x08
> > +#define ADI_ADC_REG_TEST_IO 0x0D
> > +#define ADI_ADC_REG_ADC_INPUT 0x0F
> > +#define ADI_ADC_REG_OFFSET 0x10
> > +#define ADI_ADC_REG_OUTPUT_MODE 0x14
> > +#define ADI_ADC_REG_OUTPUT_ADJUST 0x15
> > +#define ADI_ADC_REG_OUTPUT_PHASE 0x16
> > +#define ADI_ADC_REG_OUTPUT_DELAY 0x17
> > +#define ADI_ADC_REG_VREF 0x18
> > +#define ADI_ADC_REG_ANALOG_INPUT 0x2C
> > +
> > +/* ADI_ADC_REG_TEST_IO */
> > +#define ADI_ADC_TESTMODE_OFF 0x0
> > +#define ADI_ADC_TESTMODE_MIDSCALE_SHORT 0x1
> > +#define ADI_ADC_TESTMODE_POS_FULLSCALE 0x2
> > +#define ADI_ADC_TESTMODE_NEG_FULLSCALE 0x3
> > +#define ADI_ADC_TESTMODE_ALT_CHECKERBOARD 0x4
> > +#define ADI_ADC_TESTMODE_PN23_SEQ 0x5
> > +#define ADI_ADC_TESTMODE_PN9_SEQ 0x6
> > +#define ADI_ADC_TESTMODE_ONE_ZERO_TOGGLE 0x7
> > +#define ADI_ADC_TESTMODE_USER 0x8
> > +#define ADI_ADC_TESTMODE_BIT_TOGGLE 0x9
> > +#define ADI_ADC_TESTMODE_SYNC 0xA
> > +#define ADI_ADC_TESTMODE_ONE_BIT_HIGH 0xB
> > +#define ADI_ADC_TESTMODE_MIXED_BIT_FREQUENCY 0xC
> > +#define ADI_ADC_TESTMODE_RAMP 0xF
> > +
> > +/* ADI_ADC_REG_TRANSFER */
> > +#define ADI_ADC_TRANSFER_SYNC 0x1
> > +
> > +/* ADI_ADC_REG_OUTPUT_MODE */
> > +#define ADI_ADC_OUTPUT_MODE_OFFSET_BINARY 0x0
> > +#define ADI_ADC_OUTPUT_MODE_TWOS_COMPLEMENT 0x1
> > +#define ADI_ADC_OUTPUT_MODE_GRAY_CODE 0x2
> > +
> > +/* ADI_ADC_REG_OUTPUT_PHASE */
> > +#define ADI_ADC_OUTPUT_EVEN_ODD_MODE_EN 0x20
> > +#define ADI_ADC_INVERT_DCO_CLK 0x80
> > +
> > +/* ADI_ADC_REG_OUTPUT_DELAY */
> > +#define ADI_ADC_DCO_DELAY_ENABLE 0x80
> > +
> > +/*
> > + * Analog Devices AD9467 16-Bit, 200/250 MSPS ADC
> > + */
> > +
> > +#define CHIPID_AD9467 0x50
> > +#define AD9467_DEF_OUTPUT_MODE 0x08
> > +#define AD9467_REG_VREF_MASK 0x0F
> > +
> > +enum {
> > + ID_AD9467,
> > +};
> > +
> > +struct ad9467_state {
> > + struct spi_device *spi;
> > + struct clk *clk;
> > + 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)
> > +{
> > + unsigned char buf[3];
> > + int ret;
> > +
> > + buf[0] = 0x80 | (reg >> 8);
> > + buf[1] = reg & 0xFF;
> > +
> > + ret = spi_write_then_read(spi, &buf[0], 2, &buf[2], 1);
> > +
> > + if (ret < 0)
> > + return ret;
> > +
> > + return buf[2];
> > +}
> > +
> > +static int ad9467_spi_write(struct spi_device *spi, unsigned int reg,
> > + unsigned int val)
> > +{
> > + unsigned char buf[3];
> > +
> > + buf[0] = reg >> 8;
> > + buf[1] = reg & 0xFF;
> > + buf[2] = val;
> > +
> > + return spi_write(spi, buf, ARRAY_SIZE(buf));
> > +}
> > +
> > +static int ad9467_reg_access(struct axi_adc_conv *conv, unsigned int reg,
> > + unsigned int writeval, unsigned int *readval)
> > +{
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > + struct spi_device *spi = st->spi;
> > + int ret;
> > +
> > + if (readval == NULL) {
> > + ret = ad9467_spi_write(spi, reg, writeval);
> > + ad9467_spi_write(spi, ADI_ADC_REG_TRANSFER,
> > + ADI_ADC_TRANSFER_SYNC);
> > + return ret;
> > + }
> > +
> > + ret = ad9467_spi_read(spi, reg);
> > + if (ret < 0)
> > + return ret;
> > + *readval = ret;
> > +
> > + return 0;
> > +}
> > +
> > +static const unsigned int ad9467_scale_table[][2] = {
> > + {2000, 0}, {2100, 6}, {2200, 7},
> > + {2300, 8}, {2400, 9}, {2500, 10},
> > +};
> > +
> > +static void __ad9467_get_scale(struct axi_adc_conv *conv, int index,
> > + unsigned int *val, unsigned int *val2)
> > +{
> > + const struct axi_adc_chip_info *info = conv->chip_info;
> > + const struct iio_chan_spec *chan = &info->channels[0].iio_chan;
> > + unsigned int tmp;
> > +
> > + tmp = (info->scale_table[index][0] * 1000000ULL) >>
> > + chan->scan_type.realbits;
> > + *val = tmp / 1000000;
> > + *val2 = tmp % 1000000;
> > +}
> > +
> > +#define AD9467_CHAN(_chan, _si, _bits, _sign)
> > \
> > +{ \
> > + .type = IIO_VOLTAGE, \
> > + .indexed = 1, \
> > + .channel = _chan, \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE) | \
> > + BIT(IIO_CHAN_INFO_CALIBBIAS) | \
> > + BIT(IIO_CHAN_INFO_CALIBPHASE) | \
>
> These don't seem to be handled that I can see...
>
Yeah.
My bad.
Left-over from the re-write.
The initial driver I wanted to send had these implemented, but decided to defer
them.
The driver/chap can operate without these.
> > + BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY), \
> > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> > + .scan_index = _si, \
> > + .scan_type = { \
> > + .sign = _sign, \
> > + .realbits = _bits, \
> > + .storagebits = 16, \
> > + .shift = 0, \
>
> shift of 0 is the "obvious" default so no need to specify it...
ack
>
> > + }, \
> > +}
> > +
> > +static const struct axi_adc_chan_spec ad9467_channels[] = {
> > + {
> > + .iio_chan = AD9467_CHAN(0, 0, 16, 'S'),
> > + .num_lanes = 8,
> > + },
> > +};
> > +
> > +static const struct axi_adc_chip_info ad9467_chip_info_tbl[] = {
> > + [ID_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),
> > + },
> > +};
> > +
> > +static int ad9467_get_scale(struct axi_adc_conv *conv, int *val, int *val2)
> > +{
> > + const struct axi_adc_chip_info *info = conv->chip_info;
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > + unsigned int i, vref_val, vref_mask;
> > +
> > + vref_val = ad9467_spi_read(st->spi, ADI_ADC_REG_VREF);
> > +
> > + switch (info->id) {
> > + case CHIPID_AD9467:
> > + vref_mask = AD9467_REG_VREF_MASK;
> > + break;
> > + default:
> > + vref_mask = 0xFFFF;
> > + break;
> > + }
> > +
> > + vref_val &= vref_mask;
> > +
> > + for (i = 0; i < info->num_scales; i++) {
> > + if (vref_val == info->scale_table[i][1])
> > + break;
> > + }
> > +
> > + if (i == info->num_scales)
> > + return -ERANGE;
> > +
> > + __ad9467_get_scale(conv, i, val, val2);
> > +
> > + return IIO_VAL_INT_PLUS_MICRO;
> > +}
> > +
> > +static int ad9467_set_scale(struct axi_adc_conv *conv, int val, int val2)
> > +{
> > + const struct axi_adc_chip_info *info = conv->chip_info;
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > + unsigned int scale_val[2];
> > + unsigned int i;
> > +
> > + if (val != 0)
> > + return -EINVAL;
> > +
> > + for (i = 0; i < info->num_scales; i++) {
> > + __ad9467_get_scale(conv, i, &scale_val[0], &scale_val[1]);
> > + if (scale_val[0] != val || scale_val[1] != val2)
> > + continue;
> > +
> > + ad9467_spi_write(st->spi, ADI_ADC_REG_VREF,
> > + info->scale_table[i][1]);
> > + ad9467_spi_write(st->spi, ADI_ADC_REG_TRANSFER,
> > + ADI_ADC_TRANSFER_SYNC);
> > + return 0;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int ad9467_read_raw(struct axi_adc_conv *conv,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long m)
> > +{
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > +
> > + switch (m) {
> > + case IIO_CHAN_INFO_SCALE:
> > + return ad9467_get_scale(conv, val, val2);
> > + case IIO_CHAN_INFO_SAMP_FREQ:
> > + if (!st->clk)
> > + return -ENODEV;
> > +
> > + *val = clk_get_rate(st->clk);
> > +
> > + return IIO_VAL_INT;
> > +
> > + }
> > + return -EINVAL;
>
> I'd put that in a default in the switch as you may get warnings
> from some static checkers otherwise.
ack
>
> > +}
> > +
> > +static int ad9467_write_raw(struct axi_adc_conv *conv,
> > + struct iio_chan_spec const *chan,
> > + int val, int val2, long mask)
> > +{
> > + const struct axi_adc_chip_info *info = conv->chip_info;
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > + unsigned long r_clk;
> > + int ret;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_SCALE:
> > + return ad9467_set_scale(conv, val, val2);
> > + case IIO_CHAN_INFO_SAMP_FREQ:
> > + if (!st->clk)
> > + return -ENODEV;
> > +
> > + if (chan->extend_name)
> > + return -ENODEV;
> > +
> > + r_clk = clk_round_rate(st->clk, val);
> > + if (r_clk < 0 || r_clk > info->max_rate) {
> > + dev_warn(&st->spi->dev,
> > + "Error setting ADC sample rate %ld", r_clk);
> > + return -EINVAL;
> > + }
> > +
> > + ret = clk_set_rate(st->clk, r_clk);
> > + if (ret < 0)
> > + return ret;
> return clk_set_rate(st->clk, r_clk) is probably the same.
> Might as well do early returns everywhere.
ack
>
> > +
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int ad9467_outputmode_set(struct spi_device *spi, unsigned int mode)
> > +{
> > + int ret;
> > +
> > + ret = ad9467_spi_write(spi, ADI_ADC_REG_OUTPUT_MODE, mode);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return ad9467_spi_write(spi, ADI_ADC_REG_TRANSFER,
> > + ADI_ADC_TRANSFER_SYNC);
> > +}
> > +
> > +static int ad9467_preenable_setup(struct axi_adc_conv *conv)
> > +{
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > +
> > + return ad9467_outputmode_set(st->spi, st->output_mode);
> > +}
> > +
> > +static int ad9467_setup(struct ad9467_state *st, unsigned int chip_id)
> > +{
> > + switch (chip_id) {
> > + case CHIPID_AD9467:
> > + st->output_mode = AD9467_DEF_OUTPUT_MODE |
> > + ADI_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
> > + break;
>
> return 0 unless you are going to add anything after the switch statement.
>
hmmm; i think the way i reworked it, i can move return 0 here
thanks
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void ad9467_clk_disable(void *data)
> > +{
> > + struct ad9467_state *st = data;
> > +
> > + clk_disable_unprepare(st->clk);
> > +}
> > +
> > +static int ad9467_probe(struct spi_device *spi)
> > +{
> > + struct axi_adc_conv *conv;
> > + struct ad9467_state *st;
> > + unsigned int id;
> > + int ret;
> > +
> > + conv = devm_axi_adc_conv_register(&spi->dev, sizeof(*st));
> > +
>
> No blank line between a call and it's error handler.
ack
>
> > + if (IS_ERR(conv))
> > + return PTR_ERR(conv);
> > +
> > + st = axi_adc_conv_priv(conv);
> > + st->spi = spi;
> > +
> > + st->clk = devm_clk_get(&spi->dev, "sample-clock");
> > + if (IS_ERR(st->clk))
> > + return PTR_ERR(st->clk);
> > +
> > + ret = clk_prepare_enable(st->clk);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = devm_add_action_or_reset(&spi->dev, ad9467_clk_disable, st);
> > + if (ret)
> > + return ret;
> > +
> > + st->pwrdown_gpio = devm_gpiod_get_optional(&spi->dev, "powerdown",
> > + GPIOD_OUT_LOW);
> > + 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);
> > + mdelay(10);
> > + }
> > +
> > + spi_set_drvdata(spi, st);
> > +
> > + id = spi_get_device_id(spi)->driver_data;
> > + conv->chip_info = &ad9467_chip_info_tbl[id];
> > +
> > + id = ad9467_spi_read(spi, ADI_ADC_REG_CHIP_ID);
> > + if (id != conv->chip_info->id) {
> > + dev_err(&spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
> > + return -ENODEV;
> > + }
> > +
> > + conv->reg_access = ad9467_reg_access;
> > + conv->write_raw = ad9467_write_raw;
> > + conv->read_raw = ad9467_read_raw;
> > + conv->preenable_setup = ad9467_preenable_setup;
> > +
> > + return ad9467_setup(st, id);
> > +}
> > +
> > +static const struct spi_device_id ad9467_id[] = {
> > + { "ad9467", ID_AD9467 },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(spi, ad9467_id);
> > +
> > +static const struct of_device_id ad9467_of_match[] = {
> > + { .compatible = "adi,ad9467" },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, ad9467_of_match);
> > +
> > +static struct spi_driver ad9467_driver = {
> > + .driver = {
> > + .name = "ad9467",
> > + .of_match_table = ad9467_of_match,
> > + },
> > + .probe = ad9467_probe,
> > + .id_table = ad9467_id,
>
> This is something I've only just started raising in reviews.
> If a driver can't realistically be instantiated without firmware
> bindings, there isn't really any point in providing the id_table
> that I can think of so please remove.
ack
will handle this;
i'll need to replace 'id = spi_get_device_id(spi)->driver_data;' but it's doable
>
> > +};
> > +module_spi_driver(ad9467_driver);
> > +
> > +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
> > +MODULE_DESCRIPTION("Analog Devices AD9467 ADC driver");
> > +MODULE_LICENSE("GPL v2");
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] iio: adc: ad9467: add support AD9467 ADC
2020-02-21 12:57 ` Jonathan Cameron
2020-02-25 15:21 ` Ardelean, Alexandru
@ 2020-02-26 11:30 ` Ardelean, Alexandru
2020-03-01 16:03 ` Jonathan Cameron
1 sibling, 1 reply; 16+ messages in thread
From: Ardelean, Alexandru @ 2020-02-26 11:30 UTC (permalink / raw)
To: jic23
Cc: Hennerich, Michael, devicetree, linux-kernel, linux-iio, robh+dt, lars
On Fri, 2020-02-21 at 12:57 +0000, Jonathan Cameron wrote:
> On Thu, 20 Feb 2020 17:03:16 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > From: Michael Hennerich <michael.hennerich@analog.com>
> >
> > The AD9467 is a 16-bit, monolithic, IF sampling analog-to-digital converter
> > (ADC). It is optimized for high performanceover wide bandwidths and ease of
> > use. The product operates at a 250 MSPS conversion rate and is designed for
> > wireless receivers, instrumentation, and test equipment that require a high
> > dynamic range. The ADC requires 1.8 V and 3.3 V power supplies and a low
> > voltage differential input clock for full performance operation. No
> > external reference or driver components are required for many applications.
> > Data outputs are LVDS compatible (ANSI-644 compatible) and include the
> > means to reduce the overall current needed for short trace distances.
> >
> > Since the chip can operate at such high sample-rates (much higher than
> > classical interfaces), it requires that a DMA controller be used to
> > interface directly to the chip and push data into memory.
> > Typically, the AXI ADC IP core is used to interface with it.
> >
> > Link:
> > https://www.analog.com/media/en/technical-documentation/data-sheets/AD9467.pdf
> >
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>
> A few minor things inline.
>
> Jonathan
> > ---
> > drivers/iio/adc/Kconfig | 15 ++
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/ad9467.c | 447 +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 463 insertions(+)
> > create mode 100644 drivers/iio/adc/ad9467.c
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 6cd48a256122..229b8bc6f9b6 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -246,6 +246,21 @@ config AD799X
> > To compile this driver as a module, choose M here: the module will be
> > called ad799x.
> >
> > +config AD9467
> > + tristate "Analog Devices AD9467 High Speed ADC driver"
> > + depends on SPI
> > + select AXI_ADC
> > + help
> > + Say yes here to build support for Analog Devices:
> > + * AD9467 16-Bit, 200 MSPS/250 MSPS Analog-to-Digital Converter
> > +
> > + The driver requires the assistance of the AXI ADC IP core to operate,
> > + since SPI is used for configuration only, while data has to be
> > + streamed into memory via DMA.
> > +
> > + To compile this driver as a module, choose M here: the module will be
> > + called ad9467.
> > +
> > config ASPEED_ADC
> > tristate "Aspeed ADC"
> > depends on ARCH_ASPEED || COMPILE_TEST
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index e14fabd53246..5018220b8ec7 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -26,6 +26,7 @@ obj-$(CONFIG_AD7793) += ad7793.o
> > obj-$(CONFIG_AD7887) += ad7887.o
> > obj-$(CONFIG_AD7949) += ad7949.o
> > obj-$(CONFIG_AD799X) += ad799x.o
> > +obj-$(CONFIG_AD9467) += ad9467.o
> > obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
> > obj-$(CONFIG_AT91_ADC) += at91_adc.o
> > obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
> > diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> > new file mode 100644
> > index 000000000000..f268bbb6bcf6
> > --- /dev/null
> > +++ b/drivers/iio/adc/ad9467.c
> > @@ -0,0 +1,447 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Analog Devices AD9467 SPI ADC driver
> > + *
> > + * Copyright 2012-2020 Analog Devices Inc.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/err.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/of.h>
> > +
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +
> > +#include <linux/clk.h>
> > +
> > +#include <linux/iio/adc/axi-adc.h>
> > +
> > +/*
> > + * ADI High-Speed ADC common spi interface registers
> > + * See Application-Note AN-877:
> > + *
> > https://www.analog.com/media/en/technical-documentation/application-notes/AN-877.pdf
> > + */
> > +
> > +#define ADI_ADC_REG_CHIP_PORT_CONF 0x00
>
> These prefixes should reflect the chip we are supporting here.
> They aren't true for 'all' ADI parts.
>
> You could come up with some 'generic' but not to generic prefix
> if you prefer.
How about AN877_ADC ?
>
> > +#define ADI_ADC_REG_CHIP_ID 0x01
> > +#define ADI_ADC_REG_CHIP_GRADE 0x02
> > +#define ADI_ADC_REG_CHAN_INDEX 0x05
> > +#define ADI_ADC_REG_TRANSFER 0xFF
> > +#define ADI_ADC_REG_MODES 0x08
> > +#define ADI_ADC_REG_TEST_IO 0x0D
> > +#define ADI_ADC_REG_ADC_INPUT 0x0F
> > +#define ADI_ADC_REG_OFFSET 0x10
> > +#define ADI_ADC_REG_OUTPUT_MODE 0x14
> > +#define ADI_ADC_REG_OUTPUT_ADJUST 0x15
> > +#define ADI_ADC_REG_OUTPUT_PHASE 0x16
> > +#define ADI_ADC_REG_OUTPUT_DELAY 0x17
> > +#define ADI_ADC_REG_VREF 0x18
> > +#define ADI_ADC_REG_ANALOG_INPUT 0x2C
> > +
> > +/* ADI_ADC_REG_TEST_IO */
> > +#define ADI_ADC_TESTMODE_OFF 0x0
> > +#define ADI_ADC_TESTMODE_MIDSCALE_SHORT 0x1
> > +#define ADI_ADC_TESTMODE_POS_FULLSCALE 0x2
> > +#define ADI_ADC_TESTMODE_NEG_FULLSCALE 0x3
> > +#define ADI_ADC_TESTMODE_ALT_CHECKERBOARD 0x4
> > +#define ADI_ADC_TESTMODE_PN23_SEQ 0x5
> > +#define ADI_ADC_TESTMODE_PN9_SEQ 0x6
> > +#define ADI_ADC_TESTMODE_ONE_ZERO_TOGGLE 0x7
> > +#define ADI_ADC_TESTMODE_USER 0x8
> > +#define ADI_ADC_TESTMODE_BIT_TOGGLE 0x9
> > +#define ADI_ADC_TESTMODE_SYNC 0xA
> > +#define ADI_ADC_TESTMODE_ONE_BIT_HIGH 0xB
> > +#define ADI_ADC_TESTMODE_MIXED_BIT_FREQUENCY 0xC
> > +#define ADI_ADC_TESTMODE_RAMP 0xF
> > +
> > +/* ADI_ADC_REG_TRANSFER */
> > +#define ADI_ADC_TRANSFER_SYNC 0x1
> > +
> > +/* ADI_ADC_REG_OUTPUT_MODE */
> > +#define ADI_ADC_OUTPUT_MODE_OFFSET_BINARY 0x0
> > +#define ADI_ADC_OUTPUT_MODE_TWOS_COMPLEMENT 0x1
> > +#define ADI_ADC_OUTPUT_MODE_GRAY_CODE 0x2
> > +
> > +/* ADI_ADC_REG_OUTPUT_PHASE */
> > +#define ADI_ADC_OUTPUT_EVEN_ODD_MODE_EN 0x20
> > +#define ADI_ADC_INVERT_DCO_CLK 0x80
> > +
> > +/* ADI_ADC_REG_OUTPUT_DELAY */
> > +#define ADI_ADC_DCO_DELAY_ENABLE 0x80
> > +
> > +/*
> > + * Analog Devices AD9467 16-Bit, 200/250 MSPS ADC
> > + */
> > +
> > +#define CHIPID_AD9467 0x50
> > +#define AD9467_DEF_OUTPUT_MODE 0x08
> > +#define AD9467_REG_VREF_MASK 0x0F
> > +
> > +enum {
> > + ID_AD9467,
> > +};
> > +
> > +struct ad9467_state {
> > + struct spi_device *spi;
> > + struct clk *clk;
> > + 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)
> > +{
> > + unsigned char buf[3];
> > + int ret;
> > +
> > + buf[0] = 0x80 | (reg >> 8);
> > + buf[1] = reg & 0xFF;
> > +
> > + ret = spi_write_then_read(spi, &buf[0], 2, &buf[2], 1);
> > +
> > + if (ret < 0)
> > + return ret;
> > +
> > + return buf[2];
> > +}
> > +
> > +static int ad9467_spi_write(struct spi_device *spi, unsigned int reg,
> > + unsigned int val)
> > +{
> > + unsigned char buf[3];
> > +
> > + buf[0] = reg >> 8;
> > + buf[1] = reg & 0xFF;
> > + buf[2] = val;
> > +
> > + return spi_write(spi, buf, ARRAY_SIZE(buf));
> > +}
> > +
> > +static int ad9467_reg_access(struct axi_adc_conv *conv, unsigned int reg,
> > + unsigned int writeval, unsigned int *readval)
> > +{
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > + struct spi_device *spi = st->spi;
> > + int ret;
> > +
> > + if (readval == NULL) {
> > + ret = ad9467_spi_write(spi, reg, writeval);
> > + ad9467_spi_write(spi, ADI_ADC_REG_TRANSFER,
> > + ADI_ADC_TRANSFER_SYNC);
> > + return ret;
> > + }
> > +
> > + ret = ad9467_spi_read(spi, reg);
> > + if (ret < 0)
> > + return ret;
> > + *readval = ret;
> > +
> > + return 0;
> > +}
> > +
> > +static const unsigned int ad9467_scale_table[][2] = {
> > + {2000, 0}, {2100, 6}, {2200, 7},
> > + {2300, 8}, {2400, 9}, {2500, 10},
> > +};
> > +
> > +static void __ad9467_get_scale(struct axi_adc_conv *conv, int index,
> > + unsigned int *val, unsigned int *val2)
> > +{
> > + const struct axi_adc_chip_info *info = conv->chip_info;
> > + const struct iio_chan_spec *chan = &info->channels[0].iio_chan;
> > + unsigned int tmp;
> > +
> > + tmp = (info->scale_table[index][0] * 1000000ULL) >>
> > + chan->scan_type.realbits;
> > + *val = tmp / 1000000;
> > + *val2 = tmp % 1000000;
> > +}
> > +
> > +#define AD9467_CHAN(_chan, _si, _bits, _sign)
> > \
> > +{ \
> > + .type = IIO_VOLTAGE, \
> > + .indexed = 1, \
> > + .channel = _chan, \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE) | \
> > + BIT(IIO_CHAN_INFO_CALIBBIAS) | \
> > + BIT(IIO_CHAN_INFO_CALIBPHASE) | \
>
> These don't seem to be handled that I can see...
>
> > + BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY), \
> > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> > + .scan_index = _si, \
> > + .scan_type = { \
> > + .sign = _sign, \
> > + .realbits = _bits, \
> > + .storagebits = 16, \
> > + .shift = 0, \
>
> shift of 0 is the "obvious" default so no need to specify it...
>
> > + }, \
> > +}
> > +
> > +static const struct axi_adc_chan_spec ad9467_channels[] = {
> > + {
> > + .iio_chan = AD9467_CHAN(0, 0, 16, 'S'),
> > + .num_lanes = 8,
> > + },
> > +};
> > +
> > +static const struct axi_adc_chip_info ad9467_chip_info_tbl[] = {
> > + [ID_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),
> > + },
> > +};
> > +
> > +static int ad9467_get_scale(struct axi_adc_conv *conv, int *val, int *val2)
> > +{
> > + const struct axi_adc_chip_info *info = conv->chip_info;
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > + unsigned int i, vref_val, vref_mask;
> > +
> > + vref_val = ad9467_spi_read(st->spi, ADI_ADC_REG_VREF);
> > +
> > + switch (info->id) {
> > + case CHIPID_AD9467:
> > + vref_mask = AD9467_REG_VREF_MASK;
> > + break;
> > + default:
> > + vref_mask = 0xFFFF;
> > + break;
> > + }
> > +
> > + vref_val &= vref_mask;
> > +
> > + for (i = 0; i < info->num_scales; i++) {
> > + if (vref_val == info->scale_table[i][1])
> > + break;
> > + }
> > +
> > + if (i == info->num_scales)
> > + return -ERANGE;
> > +
> > + __ad9467_get_scale(conv, i, val, val2);
> > +
> > + return IIO_VAL_INT_PLUS_MICRO;
> > +}
> > +
> > +static int ad9467_set_scale(struct axi_adc_conv *conv, int val, int val2)
> > +{
> > + const struct axi_adc_chip_info *info = conv->chip_info;
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > + unsigned int scale_val[2];
> > + unsigned int i;
> > +
> > + if (val != 0)
> > + return -EINVAL;
> > +
> > + for (i = 0; i < info->num_scales; i++) {
> > + __ad9467_get_scale(conv, i, &scale_val[0], &scale_val[1]);
> > + if (scale_val[0] != val || scale_val[1] != val2)
> > + continue;
> > +
> > + ad9467_spi_write(st->spi, ADI_ADC_REG_VREF,
> > + info->scale_table[i][1]);
> > + ad9467_spi_write(st->spi, ADI_ADC_REG_TRANSFER,
> > + ADI_ADC_TRANSFER_SYNC);
> > + return 0;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int ad9467_read_raw(struct axi_adc_conv *conv,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long m)
> > +{
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > +
> > + switch (m) {
> > + case IIO_CHAN_INFO_SCALE:
> > + return ad9467_get_scale(conv, val, val2);
> > + case IIO_CHAN_INFO_SAMP_FREQ:
> > + if (!st->clk)
> > + return -ENODEV;
> > +
> > + *val = clk_get_rate(st->clk);
> > +
> > + return IIO_VAL_INT;
> > +
> > + }
> > + return -EINVAL;
>
> I'd put that in a default in the switch as you may get warnings
> from some static checkers otherwise.
>
> > +}
> > +
> > +static int ad9467_write_raw(struct axi_adc_conv *conv,
> > + struct iio_chan_spec const *chan,
> > + int val, int val2, long mask)
> > +{
> > + const struct axi_adc_chip_info *info = conv->chip_info;
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > + unsigned long r_clk;
> > + int ret;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_SCALE:
> > + return ad9467_set_scale(conv, val, val2);
> > + case IIO_CHAN_INFO_SAMP_FREQ:
> > + if (!st->clk)
> > + return -ENODEV;
> > +
> > + if (chan->extend_name)
> > + return -ENODEV;
> > +
> > + r_clk = clk_round_rate(st->clk, val);
> > + if (r_clk < 0 || r_clk > info->max_rate) {
> > + dev_warn(&st->spi->dev,
> > + "Error setting ADC sample rate %ld", r_clk);
> > + return -EINVAL;
> > + }
> > +
> > + ret = clk_set_rate(st->clk, r_clk);
> > + if (ret < 0)
> > + return ret;
> return clk_set_rate(st->clk, r_clk) is probably the same.
> Might as well do early returns everywhere.
>
> > +
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int ad9467_outputmode_set(struct spi_device *spi, unsigned int mode)
> > +{
> > + int ret;
> > +
> > + ret = ad9467_spi_write(spi, ADI_ADC_REG_OUTPUT_MODE, mode);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return ad9467_spi_write(spi, ADI_ADC_REG_TRANSFER,
> > + ADI_ADC_TRANSFER_SYNC);
> > +}
> > +
> > +static int ad9467_preenable_setup(struct axi_adc_conv *conv)
> > +{
> > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > +
> > + return ad9467_outputmode_set(st->spi, st->output_mode);
> > +}
> > +
> > +static int ad9467_setup(struct ad9467_state *st, unsigned int chip_id)
> > +{
> > + switch (chip_id) {
> > + case CHIPID_AD9467:
> > + st->output_mode = AD9467_DEF_OUTPUT_MODE |
> > + ADI_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
> > + break;
>
> return 0 unless you are going to add anything after the switch statement.
>
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void ad9467_clk_disable(void *data)
> > +{
> > + struct ad9467_state *st = data;
> > +
> > + clk_disable_unprepare(st->clk);
> > +}
> > +
> > +static int ad9467_probe(struct spi_device *spi)
> > +{
> > + struct axi_adc_conv *conv;
> > + struct ad9467_state *st;
> > + unsigned int id;
> > + int ret;
> > +
> > + conv = devm_axi_adc_conv_register(&spi->dev, sizeof(*st));
> > +
>
> No blank line between a call and it's error handler.
>
> > + if (IS_ERR(conv))
> > + return PTR_ERR(conv);
> > +
> > + st = axi_adc_conv_priv(conv);
> > + st->spi = spi;
> > +
> > + st->clk = devm_clk_get(&spi->dev, "sample-clock");
> > + if (IS_ERR(st->clk))
> > + return PTR_ERR(st->clk);
> > +
> > + ret = clk_prepare_enable(st->clk);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = devm_add_action_or_reset(&spi->dev, ad9467_clk_disable, st);
> > + if (ret)
> > + return ret;
> > +
> > + st->pwrdown_gpio = devm_gpiod_get_optional(&spi->dev, "powerdown",
> > + GPIOD_OUT_LOW);
> > + 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);
> > + mdelay(10);
> > + }
> > +
> > + spi_set_drvdata(spi, st);
> > +
> > + id = spi_get_device_id(spi)->driver_data;
> > + conv->chip_info = &ad9467_chip_info_tbl[id];
> > +
> > + id = ad9467_spi_read(spi, ADI_ADC_REG_CHIP_ID);
> > + if (id != conv->chip_info->id) {
> > + dev_err(&spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
> > + return -ENODEV;
> > + }
> > +
> > + conv->reg_access = ad9467_reg_access;
> > + conv->write_raw = ad9467_write_raw;
> > + conv->read_raw = ad9467_read_raw;
> > + conv->preenable_setup = ad9467_preenable_setup;
> > +
> > + return ad9467_setup(st, id);
> > +}
> > +
> > +static const struct spi_device_id ad9467_id[] = {
> > + { "ad9467", ID_AD9467 },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(spi, ad9467_id);
> > +
> > +static const struct of_device_id ad9467_of_match[] = {
> > + { .compatible = "adi,ad9467" },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, ad9467_of_match);
> > +
> > +static struct spi_driver ad9467_driver = {
> > + .driver = {
> > + .name = "ad9467",
> > + .of_match_table = ad9467_of_match,
> > + },
> > + .probe = ad9467_probe,
> > + .id_table = ad9467_id,
>
> This is something I've only just started raising in reviews.
> If a driver can't realistically be instantiated without firmware
> bindings, there isn't really any point in providing the id_table
> that I can think of so please remove.
>
> > +};
> > +module_spi_driver(ad9467_driver);
> > +
> > +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
> > +MODULE_DESCRIPTION("Analog Devices AD9467 ADC driver");
> > +MODULE_LICENSE("GPL v2");
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free
2020-02-25 13:33 ` Ardelean, Alexandru
@ 2020-03-01 16:02 ` Jonathan Cameron
0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2020-03-01 16:02 UTC (permalink / raw)
To: Ardelean, Alexandru; +Cc: devicetree, linux-kernel, linux-iio, robh+dt
On Tue, 25 Feb 2020 13:33:59 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:
> On Fri, 2020-02-21 at 12:21 +0000, Jonathan Cameron wrote:
> > [External]
> >
> > On Thu, 20 Feb 2020 17:03:13 +0200
> > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> >
> > > Currently, when using a 'iio_dmaengine_buffer_alloc()', an matching call to
> > > 'iio_dmaengine_buffer_free()' must be made.
> > >
> > > With this change, this can be avoided by using
> > > 'devm_iio_dmaengine_buffer_alloc()'. The buffer will get free'd via the
> > > device's devres handling.
> > >
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > ---
> > > .../buffer/industrialio-buffer-dmaengine.c | 70 +++++++++++++++++++
> > > include/linux/iio/buffer-dmaengine.h | 5 ++
> > > 2 files changed, 75 insertions(+)
> > >
> > > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > index b129693af0fd..eff89037e3f5 100644
> > > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > @@ -229,6 +229,76 @@ void iio_dmaengine_buffer_free(struct iio_buffer
> > > *buffer)
> > > }
> > > EXPORT_SYMBOL_GPL(iio_dmaengine_buffer_free);
> > >
> > > +static void __devm_iio_dmaengine_buffer_free(struct device *dev, void *res)
> > > +{
> > > + iio_dmaengine_buffer_free(*(struct iio_buffer **)res);
> > > +}
> > > +
> > > +/**
> > > + * devm_iio_dmaengine_buffer_alloc() - Resource-managed
> > > iio_dmaengine_buffer_alloc()
> > > + * @dev: Parent device for the buffer
> > > + * @channel: DMA channel name, typically "rx".
> > > + *
> > > + * This allocates a new IIO buffer which internally uses the DMAengine
> > > framework
> > > + * to perform its transfers. The parent device will be used to request the
> > > DMA
> > > + * channel.
> > > + *
> > > + * Once done using the buffer iio_dmaengine_buffer_free() should be used to
> > > + * release it.
> >
> > Umm. It really shouldn't!
>
> Yeah.
> Copy+paste.
> My bad.
>
>
> > > + */
> > > +struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
> > > + const char *channel)
> > > +{
> > > + struct iio_buffer **bufferp, *buffer;
> > > +
> > > + bufferp = devres_alloc(__devm_iio_dmaengine_buffer_free,
> > > + sizeof(*bufferp), GFP_KERNEL);
> > > + if (!bufferp)
> > > + return ERR_PTR(-ENOMEM);
> > > +
> > > + buffer = iio_dmaengine_buffer_alloc(dev, channel);
> > > + if (!IS_ERR(buffer)) {
> > > + *bufferp = buffer;
> > > + devres_add(dev, bufferp);
> >
> > From a flow point of view I'd prefer.
> >
> > if (IS_ERR(buffer) {
> > devres_free(buferp)
> > return buffer;
> > }
> >
> > *bufferp = buffer;
> > devres_add(dev, bufferp);
> >
> > return buffer;
> >
> >
> > > + } else {
> > > + devres_free(bufferp);
> > > + }
> > > +
> > > + return buffer;
> > > +}
> > > +EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_alloc);
> > > +
> > > +static int devm_iio_dmaengine_buffer_match(struct device *dev, void *res,
> > > + void *data)
> > > +{
> > > + struct iio_buffer **r = res;
> > > +
> > > + if (!r || !*r) {
> > > + WARN_ON(!r || !*r);
> > > + return 0;
> > > + }
> > > +
> > > + return *r == data;
> > > +}
> > > +
> > > +/**
> > > + * devm_iio_dmaengine_buffer_free - iio_dmaengine_buffer_free
> > > + * @dev: Device this iio_buffer belongs to
> > > + * @buffer: The iio_buffer associated with the device
> > > + *
> > > + * Free buffer allocated with devm_iio_dmaengine_buffer_alloc().
> > > + */
> > > +void devm_iio_dmaengine_buffer_free(struct device *dev,
> > > + struct iio_buffer *buffer)
> > > +{
> > > + int rc;
> > > +
> > > + rc = devres_release(dev, __devm_iio_dmaengine_buffer_free,
> > > + devm_iio_dmaengine_buffer_match, buffer);
> > > + WARN_ON(rc);
> > > +}
> > > +EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_free);
>
> Should I remove devm_iio_dmaengine_buffer_free() ?
> There was a comment on the AXI ADC that may apply here as well, about maybe
> removing it.
yup. Drop it.
>
> > > +
> > > MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
> > > MODULE_DESCRIPTION("DMA buffer for the IIO framework");
> > > MODULE_LICENSE("GPL");
> > > diff --git a/include/linux/iio/buffer-dmaengine.h
> > > b/include/linux/iio/buffer-dmaengine.h
> > > index b3a57444a886..8dcd973d76c1 100644
> > > --- a/include/linux/iio/buffer-dmaengine.h
> > > +++ b/include/linux/iio/buffer-dmaengine.h
> > > @@ -14,4 +14,9 @@ struct iio_buffer *iio_dmaengine_buffer_alloc(struct
> > > device *dev,
> > > const char *channel);
> > > void iio_dmaengine_buffer_free(struct iio_buffer *buffer);
> > >
> > > +struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev,
> > > + const char *channel);
> > > +void devm_iio_dmaengine_buffer_free(struct device *dev,
> > > + struct iio_buffer *buffer);
> > Please align parameters with opening bracket where possible.
> >
>
> ack
>
> > Thanks,
> >
> > Jonathan
> >
> > > +
> > > #endif
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] iio: adc: ad9467: add support AD9467 ADC
2020-02-26 11:30 ` Ardelean, Alexandru
@ 2020-03-01 16:03 ` Jonathan Cameron
0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2020-03-01 16:03 UTC (permalink / raw)
To: Ardelean, Alexandru
Cc: Hennerich, Michael, devicetree, linux-kernel, linux-iio, robh+dt, lars
On Wed, 26 Feb 2020 11:30:33 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:
> On Fri, 2020-02-21 at 12:57 +0000, Jonathan Cameron wrote:
> > On Thu, 20 Feb 2020 17:03:16 +0200
> > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> >
> > > From: Michael Hennerich <michael.hennerich@analog.com>
> > >
> > > The AD9467 is a 16-bit, monolithic, IF sampling analog-to-digital converter
> > > (ADC). It is optimized for high performanceover wide bandwidths and ease of
> > > use. The product operates at a 250 MSPS conversion rate and is designed for
> > > wireless receivers, instrumentation, and test equipment that require a high
> > > dynamic range. The ADC requires 1.8 V and 3.3 V power supplies and a low
> > > voltage differential input clock for full performance operation. No
> > > external reference or driver components are required for many applications.
> > > Data outputs are LVDS compatible (ANSI-644 compatible) and include the
> > > means to reduce the overall current needed for short trace distances.
> > >
> > > Since the chip can operate at such high sample-rates (much higher than
> > > classical interfaces), it requires that a DMA controller be used to
> > > interface directly to the chip and push data into memory.
> > > Typically, the AXI ADC IP core is used to interface with it.
> > >
> > > Link:
> > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD9467.pdf
> > >
> > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> >
> > A few minor things inline.
> >
> > Jonathan
> > > ---
> > > drivers/iio/adc/Kconfig | 15 ++
> > > drivers/iio/adc/Makefile | 1 +
> > > drivers/iio/adc/ad9467.c | 447 +++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 463 insertions(+)
> > > create mode 100644 drivers/iio/adc/ad9467.c
> > >
> > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > > index 6cd48a256122..229b8bc6f9b6 100644
> > > --- a/drivers/iio/adc/Kconfig
> > > +++ b/drivers/iio/adc/Kconfig
> > > @@ -246,6 +246,21 @@ config AD799X
> > > To compile this driver as a module, choose M here: the module will be
> > > called ad799x.
> > >
> > > +config AD9467
> > > + tristate "Analog Devices AD9467 High Speed ADC driver"
> > > + depends on SPI
> > > + select AXI_ADC
> > > + help
> > > + Say yes here to build support for Analog Devices:
> > > + * AD9467 16-Bit, 200 MSPS/250 MSPS Analog-to-Digital Converter
> > > +
> > > + The driver requires the assistance of the AXI ADC IP core to operate,
> > > + since SPI is used for configuration only, while data has to be
> > > + streamed into memory via DMA.
> > > +
> > > + To compile this driver as a module, choose M here: the module will be
> > > + called ad9467.
> > > +
> > > config ASPEED_ADC
> > > tristate "Aspeed ADC"
> > > depends on ARCH_ASPEED || COMPILE_TEST
> > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > > index e14fabd53246..5018220b8ec7 100644
> > > --- a/drivers/iio/adc/Makefile
> > > +++ b/drivers/iio/adc/Makefile
> > > @@ -26,6 +26,7 @@ obj-$(CONFIG_AD7793) += ad7793.o
> > > obj-$(CONFIG_AD7887) += ad7887.o
> > > obj-$(CONFIG_AD7949) += ad7949.o
> > > obj-$(CONFIG_AD799X) += ad799x.o
> > > +obj-$(CONFIG_AD9467) += ad9467.o
> > > obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
> > > obj-$(CONFIG_AT91_ADC) += at91_adc.o
> > > obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
> > > diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> > > new file mode 100644
> > > index 000000000000..f268bbb6bcf6
> > > --- /dev/null
> > > +++ b/drivers/iio/adc/ad9467.c
> > > @@ -0,0 +1,447 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Analog Devices AD9467 SPI ADC driver
> > > + *
> > > + * Copyright 2012-2020 Analog Devices Inc.
> > > + */
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/device.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/spi/spi.h>
> > > +#include <linux/err.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/of.h>
> > > +
> > > +#include <linux/iio/iio.h>
> > > +#include <linux/iio/sysfs.h>
> > > +
> > > +#include <linux/clk.h>
> > > +
> > > +#include <linux/iio/adc/axi-adc.h>
> > > +
> > > +/*
> > > + * ADI High-Speed ADC common spi interface registers
> > > + * See Application-Note AN-877:
> > > + *
> > > https://www.analog.com/media/en/technical-documentation/application-notes/AN-877.pdf
> > > + */
> > > +
> > > +#define ADI_ADC_REG_CHIP_PORT_CONF 0x00
> >
> > These prefixes should reflect the chip we are supporting here.
> > They aren't true for 'all' ADI parts.
> >
> > You could come up with some 'generic' but not to generic prefix
> > if you prefer.
>
> How about AN877_ADC ?
I guess that works.
>
> >
> > > +#define ADI_ADC_REG_CHIP_ID 0x01
> > > +#define ADI_ADC_REG_CHIP_GRADE 0x02
> > > +#define ADI_ADC_REG_CHAN_INDEX 0x05
> > > +#define ADI_ADC_REG_TRANSFER 0xFF
> > > +#define ADI_ADC_REG_MODES 0x08
> > > +#define ADI_ADC_REG_TEST_IO 0x0D
> > > +#define ADI_ADC_REG_ADC_INPUT 0x0F
> > > +#define ADI_ADC_REG_OFFSET 0x10
> > > +#define ADI_ADC_REG_OUTPUT_MODE 0x14
> > > +#define ADI_ADC_REG_OUTPUT_ADJUST 0x15
> > > +#define ADI_ADC_REG_OUTPUT_PHASE 0x16
> > > +#define ADI_ADC_REG_OUTPUT_DELAY 0x17
> > > +#define ADI_ADC_REG_VREF 0x18
> > > +#define ADI_ADC_REG_ANALOG_INPUT 0x2C
> > > +
> > > +/* ADI_ADC_REG_TEST_IO */
> > > +#define ADI_ADC_TESTMODE_OFF 0x0
> > > +#define ADI_ADC_TESTMODE_MIDSCALE_SHORT 0x1
> > > +#define ADI_ADC_TESTMODE_POS_FULLSCALE 0x2
> > > +#define ADI_ADC_TESTMODE_NEG_FULLSCALE 0x3
> > > +#define ADI_ADC_TESTMODE_ALT_CHECKERBOARD 0x4
> > > +#define ADI_ADC_TESTMODE_PN23_SEQ 0x5
> > > +#define ADI_ADC_TESTMODE_PN9_SEQ 0x6
> > > +#define ADI_ADC_TESTMODE_ONE_ZERO_TOGGLE 0x7
> > > +#define ADI_ADC_TESTMODE_USER 0x8
> > > +#define ADI_ADC_TESTMODE_BIT_TOGGLE 0x9
> > > +#define ADI_ADC_TESTMODE_SYNC 0xA
> > > +#define ADI_ADC_TESTMODE_ONE_BIT_HIGH 0xB
> > > +#define ADI_ADC_TESTMODE_MIXED_BIT_FREQUENCY 0xC
> > > +#define ADI_ADC_TESTMODE_RAMP 0xF
> > > +
> > > +/* ADI_ADC_REG_TRANSFER */
> > > +#define ADI_ADC_TRANSFER_SYNC 0x1
> > > +
> > > +/* ADI_ADC_REG_OUTPUT_MODE */
> > > +#define ADI_ADC_OUTPUT_MODE_OFFSET_BINARY 0x0
> > > +#define ADI_ADC_OUTPUT_MODE_TWOS_COMPLEMENT 0x1
> > > +#define ADI_ADC_OUTPUT_MODE_GRAY_CODE 0x2
> > > +
> > > +/* ADI_ADC_REG_OUTPUT_PHASE */
> > > +#define ADI_ADC_OUTPUT_EVEN_ODD_MODE_EN 0x20
> > > +#define ADI_ADC_INVERT_DCO_CLK 0x80
> > > +
> > > +/* ADI_ADC_REG_OUTPUT_DELAY */
> > > +#define ADI_ADC_DCO_DELAY_ENABLE 0x80
> > > +
> > > +/*
> > > + * Analog Devices AD9467 16-Bit, 200/250 MSPS ADC
> > > + */
> > > +
> > > +#define CHIPID_AD9467 0x50
> > > +#define AD9467_DEF_OUTPUT_MODE 0x08
> > > +#define AD9467_REG_VREF_MASK 0x0F
> > > +
> > > +enum {
> > > + ID_AD9467,
> > > +};
> > > +
> > > +struct ad9467_state {
> > > + struct spi_device *spi;
> > > + struct clk *clk;
> > > + 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)
> > > +{
> > > + unsigned char buf[3];
> > > + int ret;
> > > +
> > > + buf[0] = 0x80 | (reg >> 8);
> > > + buf[1] = reg & 0xFF;
> > > +
> > > + ret = spi_write_then_read(spi, &buf[0], 2, &buf[2], 1);
> > > +
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + return buf[2];
> > > +}
> > > +
> > > +static int ad9467_spi_write(struct spi_device *spi, unsigned int reg,
> > > + unsigned int val)
> > > +{
> > > + unsigned char buf[3];
> > > +
> > > + buf[0] = reg >> 8;
> > > + buf[1] = reg & 0xFF;
> > > + buf[2] = val;
> > > +
> > > + return spi_write(spi, buf, ARRAY_SIZE(buf));
> > > +}
> > > +
> > > +static int ad9467_reg_access(struct axi_adc_conv *conv, unsigned int reg,
> > > + unsigned int writeval, unsigned int *readval)
> > > +{
> > > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > > + struct spi_device *spi = st->spi;
> > > + int ret;
> > > +
> > > + if (readval == NULL) {
> > > + ret = ad9467_spi_write(spi, reg, writeval);
> > > + ad9467_spi_write(spi, ADI_ADC_REG_TRANSFER,
> > > + ADI_ADC_TRANSFER_SYNC);
> > > + return ret;
> > > + }
> > > +
> > > + ret = ad9467_spi_read(spi, reg);
> > > + if (ret < 0)
> > > + return ret;
> > > + *readval = ret;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const unsigned int ad9467_scale_table[][2] = {
> > > + {2000, 0}, {2100, 6}, {2200, 7},
> > > + {2300, 8}, {2400, 9}, {2500, 10},
> > > +};
> > > +
> > > +static void __ad9467_get_scale(struct axi_adc_conv *conv, int index,
> > > + unsigned int *val, unsigned int *val2)
> > > +{
> > > + const struct axi_adc_chip_info *info = conv->chip_info;
> > > + const struct iio_chan_spec *chan = &info->channels[0].iio_chan;
> > > + unsigned int tmp;
> > > +
> > > + tmp = (info->scale_table[index][0] * 1000000ULL) >>
> > > + chan->scan_type.realbits;
> > > + *val = tmp / 1000000;
> > > + *val2 = tmp % 1000000;
> > > +}
> > > +
> > > +#define AD9467_CHAN(_chan, _si, _bits, _sign)
> > > \
> > > +{ \
> > > + .type = IIO_VOLTAGE, \
> > > + .indexed = 1, \
> > > + .channel = _chan, \
> > > + .info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE) | \
> > > + BIT(IIO_CHAN_INFO_CALIBBIAS) | \
> > > + BIT(IIO_CHAN_INFO_CALIBPHASE) | \
> >
> > These don't seem to be handled that I can see...
> >
> > > + BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY), \
> > > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> > > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> > > + .scan_index = _si, \
> > > + .scan_type = { \
> > > + .sign = _sign, \
> > > + .realbits = _bits, \
> > > + .storagebits = 16, \
> > > + .shift = 0, \
> >
> > shift of 0 is the "obvious" default so no need to specify it...
> >
> > > + }, \
> > > +}
> > > +
> > > +static const struct axi_adc_chan_spec ad9467_channels[] = {
> > > + {
> > > + .iio_chan = AD9467_CHAN(0, 0, 16, 'S'),
> > > + .num_lanes = 8,
> > > + },
> > > +};
> > > +
> > > +static const struct axi_adc_chip_info ad9467_chip_info_tbl[] = {
> > > + [ID_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),
> > > + },
> > > +};
> > > +
> > > +static int ad9467_get_scale(struct axi_adc_conv *conv, int *val, int *val2)
> > > +{
> > > + const struct axi_adc_chip_info *info = conv->chip_info;
> > > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > > + unsigned int i, vref_val, vref_mask;
> > > +
> > > + vref_val = ad9467_spi_read(st->spi, ADI_ADC_REG_VREF);
> > > +
> > > + switch (info->id) {
> > > + case CHIPID_AD9467:
> > > + vref_mask = AD9467_REG_VREF_MASK;
> > > + break;
> > > + default:
> > > + vref_mask = 0xFFFF;
> > > + break;
> > > + }
> > > +
> > > + vref_val &= vref_mask;
> > > +
> > > + for (i = 0; i < info->num_scales; i++) {
> > > + if (vref_val == info->scale_table[i][1])
> > > + break;
> > > + }
> > > +
> > > + if (i == info->num_scales)
> > > + return -ERANGE;
> > > +
> > > + __ad9467_get_scale(conv, i, val, val2);
> > > +
> > > + return IIO_VAL_INT_PLUS_MICRO;
> > > +}
> > > +
> > > +static int ad9467_set_scale(struct axi_adc_conv *conv, int val, int val2)
> > > +{
> > > + const struct axi_adc_chip_info *info = conv->chip_info;
> > > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > > + unsigned int scale_val[2];
> > > + unsigned int i;
> > > +
> > > + if (val != 0)
> > > + return -EINVAL;
> > > +
> > > + for (i = 0; i < info->num_scales; i++) {
> > > + __ad9467_get_scale(conv, i, &scale_val[0], &scale_val[1]);
> > > + if (scale_val[0] != val || scale_val[1] != val2)
> > > + continue;
> > > +
> > > + ad9467_spi_write(st->spi, ADI_ADC_REG_VREF,
> > > + info->scale_table[i][1]);
> > > + ad9467_spi_write(st->spi, ADI_ADC_REG_TRANSFER,
> > > + ADI_ADC_TRANSFER_SYNC);
> > > + return 0;
> > > + }
> > > +
> > > + return -EINVAL;
> > > +}
> > > +
> > > +static int ad9467_read_raw(struct axi_adc_conv *conv,
> > > + struct iio_chan_spec const *chan,
> > > + int *val, int *val2, long m)
> > > +{
> > > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > > +
> > > + switch (m) {
> > > + case IIO_CHAN_INFO_SCALE:
> > > + return ad9467_get_scale(conv, val, val2);
> > > + case IIO_CHAN_INFO_SAMP_FREQ:
> > > + if (!st->clk)
> > > + return -ENODEV;
> > > +
> > > + *val = clk_get_rate(st->clk);
> > > +
> > > + return IIO_VAL_INT;
> > > +
> > > + }
> > > + return -EINVAL;
> >
> > I'd put that in a default in the switch as you may get warnings
> > from some static checkers otherwise.
> >
> > > +}
> > > +
> > > +static int ad9467_write_raw(struct axi_adc_conv *conv,
> > > + struct iio_chan_spec const *chan,
> > > + int val, int val2, long mask)
> > > +{
> > > + const struct axi_adc_chip_info *info = conv->chip_info;
> > > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > > + unsigned long r_clk;
> > > + int ret;
> > > +
> > > + switch (mask) {
> > > + case IIO_CHAN_INFO_SCALE:
> > > + return ad9467_set_scale(conv, val, val2);
> > > + case IIO_CHAN_INFO_SAMP_FREQ:
> > > + if (!st->clk)
> > > + return -ENODEV;
> > > +
> > > + if (chan->extend_name)
> > > + return -ENODEV;
> > > +
> > > + r_clk = clk_round_rate(st->clk, val);
> > > + if (r_clk < 0 || r_clk > info->max_rate) {
> > > + dev_warn(&st->spi->dev,
> > > + "Error setting ADC sample rate %ld", r_clk);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = clk_set_rate(st->clk, r_clk);
> > > + if (ret < 0)
> > > + return ret;
> > return clk_set_rate(st->clk, r_clk) is probably the same.
> > Might as well do early returns everywhere.
> >
> > > +
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int ad9467_outputmode_set(struct spi_device *spi, unsigned int mode)
> > > +{
> > > + int ret;
> > > +
> > > + ret = ad9467_spi_write(spi, ADI_ADC_REG_OUTPUT_MODE, mode);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + return ad9467_spi_write(spi, ADI_ADC_REG_TRANSFER,
> > > + ADI_ADC_TRANSFER_SYNC);
> > > +}
> > > +
> > > +static int ad9467_preenable_setup(struct axi_adc_conv *conv)
> > > +{
> > > + struct ad9467_state *st = axi_adc_conv_priv(conv);
> > > +
> > > + return ad9467_outputmode_set(st->spi, st->output_mode);
> > > +}
> > > +
> > > +static int ad9467_setup(struct ad9467_state *st, unsigned int chip_id)
> > > +{
> > > + switch (chip_id) {
> > > + case CHIPID_AD9467:
> > > + st->output_mode = AD9467_DEF_OUTPUT_MODE |
> > > + ADI_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
> > > + break;
> >
> > return 0 unless you are going to add anything after the switch statement.
> >
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void ad9467_clk_disable(void *data)
> > > +{
> > > + struct ad9467_state *st = data;
> > > +
> > > + clk_disable_unprepare(st->clk);
> > > +}
> > > +
> > > +static int ad9467_probe(struct spi_device *spi)
> > > +{
> > > + struct axi_adc_conv *conv;
> > > + struct ad9467_state *st;
> > > + unsigned int id;
> > > + int ret;
> > > +
> > > + conv = devm_axi_adc_conv_register(&spi->dev, sizeof(*st));
> > > +
> >
> > No blank line between a call and it's error handler.
> >
> > > + if (IS_ERR(conv))
> > > + return PTR_ERR(conv);
> > > +
> > > + st = axi_adc_conv_priv(conv);
> > > + st->spi = spi;
> > > +
> > > + st->clk = devm_clk_get(&spi->dev, "sample-clock");
> > > + if (IS_ERR(st->clk))
> > > + return PTR_ERR(st->clk);
> > > +
> > > + ret = clk_prepare_enable(st->clk);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + ret = devm_add_action_or_reset(&spi->dev, ad9467_clk_disable, st);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + st->pwrdown_gpio = devm_gpiod_get_optional(&spi->dev, "powerdown",
> > > + GPIOD_OUT_LOW);
> > > + 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);
> > > + mdelay(10);
> > > + }
> > > +
> > > + spi_set_drvdata(spi, st);
> > > +
> > > + id = spi_get_device_id(spi)->driver_data;
> > > + conv->chip_info = &ad9467_chip_info_tbl[id];
> > > +
> > > + id = ad9467_spi_read(spi, ADI_ADC_REG_CHIP_ID);
> > > + if (id != conv->chip_info->id) {
> > > + dev_err(&spi->dev, "Unrecognized CHIP_ID 0x%X\n", id);
> > > + return -ENODEV;
> > > + }
> > > +
> > > + conv->reg_access = ad9467_reg_access;
> > > + conv->write_raw = ad9467_write_raw;
> > > + conv->read_raw = ad9467_read_raw;
> > > + conv->preenable_setup = ad9467_preenable_setup;
> > > +
> > > + return ad9467_setup(st, id);
> > > +}
> > > +
> > > +static const struct spi_device_id ad9467_id[] = {
> > > + { "ad9467", ID_AD9467 },
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(spi, ad9467_id);
> > > +
> > > +static const struct of_device_id ad9467_of_match[] = {
> > > + { .compatible = "adi,ad9467" },
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ad9467_of_match);
> > > +
> > > +static struct spi_driver ad9467_driver = {
> > > + .driver = {
> > > + .name = "ad9467",
> > > + .of_match_table = ad9467_of_match,
> > > + },
> > > + .probe = ad9467_probe,
> > > + .id_table = ad9467_id,
> >
> > This is something I've only just started raising in reviews.
> > If a driver can't realistically be instantiated without firmware
> > bindings, there isn't really any point in providing the id_table
> > that I can think of so please remove.
> >
> > > +};
> > > +module_spi_driver(ad9467_driver);
> > > +
> > > +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
> > > +MODULE_DESCRIPTION("Analog Devices AD9467 ADC driver");
> > > +MODULE_LICENSE("GPL v2");
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-03-01 16:03 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20 15:03 [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free Alexandru Ardelean
2020-02-20 15:03 ` [PATCH 2/5] iio: adc: axi-adc: add support for AXI ADC IP core Alexandru Ardelean
2020-02-21 12:44 ` Jonathan Cameron
2020-02-25 13:51 ` Ardelean, Alexandru
2020-02-20 15:03 ` [PATCH 3/5] dt-bindings: iio: adc: add bindings doc for AXI ADC driver Alexandru Ardelean
2020-02-20 20:35 ` Rob Herring
2020-02-20 15:03 ` [PATCH 4/5] iio: adc: ad9467: add support AD9467 ADC Alexandru Ardelean
2020-02-21 12:57 ` Jonathan Cameron
2020-02-25 15:21 ` Ardelean, Alexandru
2020-02-26 11:30 ` Ardelean, Alexandru
2020-03-01 16:03 ` Jonathan Cameron
2020-02-20 15:03 ` [PATCH 5/5] dt-bindings: iio: adc: add bindings doc for " Alexandru Ardelean
2020-02-20 20:36 ` Rob Herring
2020-02-21 12:21 ` [PATCH 1/5] iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free Jonathan Cameron
2020-02-25 13:33 ` Ardelean, Alexandru
2020-03-01 16:02 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).