* [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC
@ 2020-03-06 11:00 Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 1/8] include: fpga: adi-axi-common.h: fixup whitespace tab -> space Alexandru Ardelean
` (7 more replies)
0 siblings, 8 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2020-03-06 11:00 UTC (permalink / raw)
To: linux-iio, linux-kernel, devicetree; +Cc: jic23, robh+dt, Alexandru Ardelean
This changeset adds support for the AD9467 LVDS High-Speed ADC.
In order to support it, support for an FPGA ADI AXI ADC is added in this
set.
This uses the current support for IIO buffer DMAEngine.
Changelog v7 -> v8:
* in 'iio: adc: adi-axi-adc: add support for AXI ADC IP core'
- updated register definitions and bits to newer format/docs; the ref driver wasn't really up-to-date
-- prefixed bit names with reg-name to avoid bit definition colisions; that makes some macros longer, but at least the format is consistent
- using dev_name(&pdev->dev) for indio_dev->name
- moved reset to own axi_adc_reset() function; may be re-used later
- some re-formatting/alignment changes
- address ENOSYS checkpatch complaint; changed with EOPNOTSUPP
Changelog v6 -> v7:
* Fixed dt-schema build for adi,axi-adc.yaml based on Rob's suggestion
- added '$ref: /schemas/types.yaml#/definitions/phandle' to 'adi,adc-dev'
- dropped 'maxItems' from 'adi,adc-dev'
Changelog v5 -> v6
* fix URLs; got changed during rename
https://wiki.analog.com/resources/fpga/docs/adi_axi_adc_ip ->
https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
- noticed while working on the AXI DAC driver
Changelog v4 -> v5:
* update drivers/iio/adc/Kconfig note about module name; omitted during first rename
- 'module will be called axi-adc.' -> 'module will be called adi-axi-adc.'
Changelog v3 -> v4:
* addressed Rob's dt-remarks
- change 'adi-axi-adc-client' prop to 'adi,adc-dev'
Changelog v2 -> v3:
* addressed compiler warning
Changelog v1 -> v2:
* first series was added a bit hastily
* addressed 'make dt_binding_check' complaints; seems I missed a few when running the check;
* added missing patches to include/linux/fpga/adi-axi-common.h
- 'include: fpga: adi-axi-common.h: fixup whitespace tab -> space'
- 'include: fpga: adi-axi-common.h: add version helper macros'
* patch 'iio: buffer-dmaengine: add dev-managed calls for buffer alloc/free'
- remove copy+pasted comment for 'devm_iio_dmaengine_buffer_alloc()'
- removed devm_iio_dmaengine_buffer_free() ; hopefully it might never be needed
- fix-up alignment for devm_iio_dmaengine_buffer_alloc() in header
* patch 'iio: adc: adi-axi-adc: add support for AXI ADC IP core'
- renamed axi-adc.c -> adi-axi-adc.c & Kconfig symbol
- prefix all axi_adc -> adi_axi_adc
- removed switch statement in axi_adc_read_raw() & axi_adc_write_raw()
- remove axi_adc_chan_spec ; replaced with iio_chan_spec directly ; will think of a simpler solution for extra chan params
- removed left-over 'struct axi_adc_cleanup_data'
- moved 'devm_add_action_or_reset()' call right after 'adi_axi_adc_attach_client()'
- switched to using 'devm_platform_ioremap_resource()'
* patch 'iio: adc: ad9467: add support AD9467 ADC'
- renamed ADI_ADC reg prefixes to AN877_ADC
- dropped 'info_mask_separate' field in AD9467_CHAN - will be re-added later when driver gets more features; was left-over from the initial ref driver
- remove .shift = 0, in AD9467_CHAN
- renamed 'sample-clock' -> 'adc-clock'
- direct returns in ad9467_read_raw() & ad9467_write_raw() & ad9467_setup() switch statements
- removed blank line after devm_axi_adc_conv_register()
- removed ad9467_id & reworked to use ad9467_of_match
Alexandru Ardelean (6):
include: fpga: adi-axi-common.h: fixup whitespace tab -> space
include: fpga: adi-axi-common.h: add version helper macros
iio: buffer-dmaengine: use %zu specifier for sprintf(align)
iio: buffer-dmaengine: add dev-managed calls for buffer alloc
dt-bindings: iio: adc: add bindings doc for AXI ADC driver
dt-bindings: iio: adc: add bindings doc for AD9467 ADC
Michael Hennerich (2):
iio: adc: adi-axi-adc: add support for AXI ADC IP core
iio: adc: ad9467: add support AD9467 ADC
.../bindings/iio/adc/adi,ad9467.yaml | 65 ++
.../bindings/iio/adc/adi,axi-adc.yaml | 63 ++
drivers/iio/adc/Kconfig | 35 +
drivers/iio/adc/Makefile | 2 +
drivers/iio/adc/ad9467.c | 432 ++++++++++++
drivers/iio/adc/adi-axi-adc.c | 618 ++++++++++++++++++
.../buffer/industrialio-buffer-dmaengine.c | 41 +-
include/linux/fpga/adi-axi-common.h | 6 +-
include/linux/iio/adc/adi-axi-adc.h | 63 ++
include/linux/iio/buffer-dmaengine.h | 3 +
10 files changed, 1326 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
create mode 100644 drivers/iio/adc/ad9467.c
create mode 100644 drivers/iio/adc/adi-axi-adc.c
create mode 100644 include/linux/iio/adc/adi-axi-adc.h
--
2.20.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v8 1/8] include: fpga: adi-axi-common.h: fixup whitespace tab -> space
2020-03-06 11:00 [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC Alexandru Ardelean
@ 2020-03-06 11:00 ` Alexandru Ardelean
2020-03-07 14:25 ` Jonathan Cameron
2020-03-06 11:00 ` [PATCH v8 2/8] include: fpga: adi-axi-common.h: add version helper macros Alexandru Ardelean
` (6 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Alexandru Ardelean @ 2020-03-06 11:00 UTC (permalink / raw)
To: linux-iio, linux-kernel, devicetree; +Cc: jic23, robh+dt, Alexandru Ardelean
The initial version use a tab between '#define' & 'ADI_AXI_REG_VERSION'.
This changes it to space. The change is purely cosmetic.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
include/linux/fpga/adi-axi-common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/fpga/adi-axi-common.h b/include/linux/fpga/adi-axi-common.h
index 7fc95d5c95bb..ebd4e07ae3d8 100644
--- a/include/linux/fpga/adi-axi-common.h
+++ b/include/linux/fpga/adi-axi-common.h
@@ -11,7 +11,7 @@
#ifndef ADI_AXI_COMMON_H_
#define ADI_AXI_COMMON_H_
-#define ADI_AXI_REG_VERSION 0x0000
+#define ADI_AXI_REG_VERSION 0x0000
#define ADI_AXI_PCORE_VER(major, minor, patch) \
(((major) << 16) | ((minor) << 8) | (patch))
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 2/8] include: fpga: adi-axi-common.h: add version helper macros
2020-03-06 11:00 [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 1/8] include: fpga: adi-axi-common.h: fixup whitespace tab -> space Alexandru Ardelean
@ 2020-03-06 11:00 ` Alexandru Ardelean
2020-03-07 14:26 ` Jonathan Cameron
2020-03-06 11:00 ` [PATCH v8 3/8] iio: buffer-dmaengine: use %zu specifier for sprintf(align) Alexandru Ardelean
` (5 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Alexandru Ardelean @ 2020-03-06 11:00 UTC (permalink / raw)
To: linux-iio, linux-kernel, devicetree; +Cc: jic23, robh+dt, Alexandru Ardelean
The format for all ADI AXI IP cores is the same.
i.e. 'major.minor.patch'.
This patch adds the helper macros to be re-used in ADI AXI drivers.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
include/linux/fpga/adi-axi-common.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/linux/fpga/adi-axi-common.h b/include/linux/fpga/adi-axi-common.h
index ebd4e07ae3d8..141ac3f251e6 100644
--- a/include/linux/fpga/adi-axi-common.h
+++ b/include/linux/fpga/adi-axi-common.h
@@ -16,4 +16,8 @@
#define ADI_AXI_PCORE_VER(major, minor, patch) \
(((major) << 16) | ((minor) << 8) | (patch))
+#define ADI_AXI_PCORE_VER_MAJOR(version) (((version) >> 16) & 0xff)
+#define ADI_AXI_PCORE_VER_MINOR(version) (((version) >> 8) & 0xff)
+#define ADI_AXI_PCORE_VER_PATCH(version) ((version) & 0xff)
+
#endif /* ADI_AXI_COMMON_H_ */
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 3/8] iio: buffer-dmaengine: use %zu specifier for sprintf(align)
2020-03-06 11:00 [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 1/8] include: fpga: adi-axi-common.h: fixup whitespace tab -> space Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 2/8] include: fpga: adi-axi-common.h: add version helper macros Alexandru Ardelean
@ 2020-03-06 11:00 ` Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 4/8] iio: buffer-dmaengine: add dev-managed calls for buffer alloc Alexandru Ardelean
` (4 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2020-03-06 11:00 UTC (permalink / raw)
To: linux-iio, linux-kernel, devicetree
Cc: jic23, robh+dt, Alexandru Ardelean, kbuild test robot
The 'size_t' type behaves differently on 64-bit architectures, and causes
compiler a warning of the sort "format '%u' expects argument of type
'unsigned int', but argument 3 has type 'size_t {aka long unsigned int}'".
This change adds the correct specifier for the 'align' field.
Fixes: 4538c18568099 ("iio: buffer-dmaengine: Report buffer length requirements")
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/buffer/industrialio-buffer-dmaengine.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index b129693af0fd..94da3b1ca3a2 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -134,7 +134,7 @@ static ssize_t iio_dmaengine_buffer_get_length_align(struct device *dev,
struct dmaengine_buffer *dmaengine_buffer =
iio_buffer_to_dmaengine_buffer(indio_dev->buffer);
- return sprintf(buf, "%u\n", dmaengine_buffer->align);
+ return sprintf(buf, "%zu\n", dmaengine_buffer->align);
}
static IIO_DEVICE_ATTR(length_align_bytes, 0444,
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 4/8] iio: buffer-dmaengine: add dev-managed calls for buffer alloc
2020-03-06 11:00 [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC Alexandru Ardelean
` (2 preceding siblings ...)
2020-03-06 11:00 ` [PATCH v8 3/8] iio: buffer-dmaengine: use %zu specifier for sprintf(align) Alexandru Ardelean
@ 2020-03-06 11:00 ` Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 5/8] iio: adc: adi-axi-adc: add support for AXI ADC IP core Alexandru Ardelean
` (3 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2020-03-06 11:00 UTC (permalink / raw)
To: linux-iio, linux-kernel, devicetree; +Cc: jic23, robh+dt, 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 | 39 +++++++++++++++++++
include/linux/iio/buffer-dmaengine.h | 3 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index 94da3b1ca3a2..6dedf12b69a4 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -229,6 +229,45 @@ 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.
+ *
+ * The buffer will be automatically de-allocated once the device gets destroyed.
+ */
+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)) {
+ devres_free(bufferp);
+ return buffer;
+ }
+
+ *bufferp = buffer;
+ devres_add(dev, bufferp);
+
+ return buffer;
+}
+EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_alloc);
+
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..0e503db71289 100644
--- a/include/linux/iio/buffer-dmaengine.h
+++ b/include/linux/iio/buffer-dmaengine.h
@@ -14,4 +14,7 @@ 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);
+
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 5/8] iio: adc: adi-axi-adc: add support for AXI ADC IP core
2020-03-06 11:00 [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC Alexandru Ardelean
` (3 preceding siblings ...)
2020-03-06 11:00 ` [PATCH v8 4/8] iio: buffer-dmaengine: add dev-managed calls for buffer alloc Alexandru Ardelean
@ 2020-03-06 11:00 ` Alexandru Ardelean
2020-03-07 14:54 ` Jonathan Cameron
2020-03-06 11:00 ` [PATCH v8 6/8] dt-bindings: iio: adc: add bindings doc for AXI ADC driver Alexandru Ardelean
` (2 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Alexandru Ardelean @ 2020-03-06 11:00 UTC (permalink / raw)
To: linux-iio, linux-kernel, devicetree
Cc: jic23, robh+dt, 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/adi-axi-adc.c | 618 ++++++++++++++++++++++++++++
include/linux/iio/adc/adi-axi-adc.h | 63 +++
4 files changed, 702 insertions(+)
create mode 100644 drivers/iio/adc/adi-axi-adc.c
create mode 100644 include/linux/iio/adc/adi-axi-adc.h
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index f4da821c4022..445070abf376 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -246,6 +246,26 @@ config AD799X
To compile this driver as a module, choose M here: the module will be
called ad799x.
+config ADI_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 adi-axi-adc.
+
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 8462455b4228..7c6594d049f9 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_ADI_AXI_ADC) += adi-axi-adc.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/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
new file mode 100644
index 000000000000..17ee20015d71
--- /dev/null
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -0,0 +1,618 @@
+// 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/adi-axi-adc.h>
+
+/**
+ * Register definitions:
+ * https://wiki.analog.com/resources/fpga/docs/axi_adc_ip#register_map
+ */
+
+#define LOWER5_MSK GENMASK(4, 0)
+#define LOWER5_SET(x) FIELD_PREP(LOWER5_MSK, x)
+#define LOWER5_GET(x) FIELD_GET(LOWER5_MSK, x)
+
+#define LOWER8_MSK GENMASK(7, 0)
+#define LOWER8_SET(x) FIELD_PREP(LOWER8_MSK, x)
+#define LOWER8_GET(x) FIELD_GET(LOWER8_MSK, x)
+
+#define UPPER16_MSK GENMASK(31, 16)
+#define UPPER16_SET(x) FIELD_PREP(UPPER16_MSK, x)
+#define UPPER16_GET(x) FIELD_GET(UPPER16_MSK, x)
+
+#define LOWER16_MSK GENMASK(15, 0)
+#define LOWER16_SET(x) FIELD_PREP(LOWER16_MSK, x)
+#define LOWER16_GET(x) FIELD_GET(LOWER16_MSK, x)
+
+/* ADC controls */
+
+#define REG_RSTN 0x0040
+#define REG_RSTN_CE_N BIT(2)
+#define REG_RSTN_MMCM_RSTN BIT(1)
+#define REG_RSTN_RSTN BIT(0)
+
+#define REG_CNTRL 0x0044
+#define REG_CNTRL_SYNC BIT(3)
+#define REG_CNTRL_R1_MODE BIT(2)
+#define REG_CNTRL_DDR_EDGESEL BIT(1)
+#define REG_CNTRL_PIN_MODE BIT(0)
+
+#define REG_CLK_FREQ 0x0054
+#define REG_CLK_RATIO 0x0058
+
+#define REG_STATUS 0x005C
+#define REG_STATUS_PN_ERR BIT(3)
+#define REG_STATUS_PN_OOS BIT(2)
+#define REG_STATUS_OVER_RANGE BIT(1)
+#define REG_STATUS_STATUS BIT(0)
+
+#define REG_SYNC_STATUS 0x0068
+#define REG_SYNC_STATUS_SYNC BIT(0)
+
+#define REG_DRP_CNTRL 0x0070
+#define REG_DRP_CNTRL_DRP_RWN BIT(28)
+#define REG_DRP_CNTRL_DRP_ADDRESS_MSK GENMASK(27, 16)
+#define REG_DRP_CNTRL_DRP_ADDRESS_SET(x) \
+ FIELD_PREP(REG_DRP_CNTRL_DRP_ADDRESS_MSK, x)
+#define REG_DRP_CNTRL_DRP_ADDRESS_GET(x) \
+ FIELD_GET(REG_DRP_CNTRL_DRP_ADDRESS_MSK, x)
+
+#define REG_DRP_STATUS 0x0074
+#define REG_DRP_STATUS_DRP_LOCKED BIT(17)
+#define REG_DRP_STATUS_DRP_STATUS BIT(16)
+
+#define REG_DRP_WDATA 0x0078
+#define REG_DRP_WDATA_SET LOWER16_SET
+#define REG_DRP_WDATA_GET LOWER16_GET
+
+#define REG_DRP_RDATA 0x007C
+#define REG_DRP_RDATA_SET LOWER16_SET
+#define REG_DRP_RDATA_GET LOWER16_GET
+
+#define REG_UI_STATUS 0x0088
+#define REG_UI_STATUS_OVF BIT(2)
+#define REG_UI_STATUS_UNF BIT(1)
+
+#define REG_USR_CNTRL_1 0x00A0
+#define REG_USR_CNTRL_1_USR_CHANMAX_MSK LOWER8_MSK
+#define REG_USR_CNTRL_1_USR_CHANMAX_SET LOWER8_SET
+#define REG_USR_CNTRL_1_USR_CHANMAX_GET LOWER8_GET
+
+#define REG_ADC_START_CODE 0x00A4
+#define REG_ADC_GPIO_IN 0x00B8
+#define REG_ADC_GPIO_OUT 0x00BC
+
+#define REG_PPS_COUNTER 0x00C0
+
+#define REG_PPS_STATUS 0x00C4
+#define REG_PPS_STATUS_PPS_STATUS BIT(0)
+
+/* ADC Channel controls */
+
+#define REG_CHAN_CNTRL(c) (0x0400 + (c) * 0x40)
+#define REG_CHAN_CNTRL_LB_OWR BIT(11)
+#define REG_CHAN_CNTRL_PN_SEL_OWR BIT(10)
+#define REG_CHAN_CNTRL_IQCOR_ENB BIT(9)
+#define REG_CHAN_CNTRL_DCFILT_ENB BIT(8)
+#define REG_CHAN_CNTRL_FORMAT_SIGNEXT BIT(6)
+#define REG_CHAN_CNTRL_FORMAT_TYPE BIT(5)
+#define REG_CHAN_CNTRL_FORMAT_ENABLE BIT(4)
+#define REG_CHAN_CNTRL_ADC_PN_TYPE_OWR BIT(1)
+#define REG_CHAN_CNTRL_ENABLE BIT(0)
+
+#define REG_CHAN_CNTRL_DEFAULTS \
+ (REG_CHAN_CNTRL_FORMAT_SIGNEXT | REG_CHAN_CNTRL_FORMAT_ENABLE | \
+ REG_CHAN_CNTRL_IQCOR_ENB | REG_CHAN_CNTRL_ENABLE)
+
+#define REG_CHAN_STATUS(c) (0x0404 + (c) * 0x40)
+#define REG_CHAN_STATUS_PN_ERR BIT(2)
+#define REG_CHAN_STATUS_PN_OOS BIT(1)
+#define REG_CHAN_STATUS_OVER_RANGE BIT(0)
+
+#define REG_CHAN_CNTRL_1(c) (0x0410 + (c) * 0x40)
+#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_MSK UPPER16_MSK
+#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_SET UPPER16_SET
+#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_GET UPPER16_GET
+#define REG_CHAN_CNTRL_1_DCFILT_COEFF_MSK LOWER16_MSK
+#define REG_CHAN_CNTRL_1_DCFILT_COEFF_SET LOWER16_SET
+#define REG_CHAN_CNTRL_1_DCFILT_COEFF_GET LOWER16_GET
+
+#define REG_CHAN_CNTRL_2(c) (0x0414 + (c) * 0x40)
+#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_MSK UPPER16_MSK
+#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_SET UPPER16_SET
+#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_GET UPPER16_GET
+#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_MSK LOWER16_MSK
+#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_SET LOWER16_SET
+#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_GET LOWER16_GET
+
+/* format is 1.1.14 (sign, integer and fractional bits) */
+#define IQCOR_INT_1 0x4000UL
+#define IQCOR_SIGN_BIT BIT(15)
+
+#define REG_CHAN_CNTRL_3(c) (0x0418 + (c) * 0x40)
+#define REG_CHAN_CNTRL_3_ADC_PN_SEL_MSK UPPER16_MSK
+#define REG_CHAN_CNTRL_3_ADC_PN_SEL_SET UPPER16_SET
+#define REG_CHAN_CNTRL_3_ADC_PN_SEL_GET UPPER16_GET
+#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_MSK LOWER16_MSK
+#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_SET LOWER16_SET
+#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_GET LOWER16_GET
+
+#define REG_CHAN_USR_CNTRL_1(c) (0x0420 + (c) * 0x40)
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BE BIT(25)
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SIGNED BIT(24)
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK GENMASK(23, 16)
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_SET(x) \
+ FIELD_PREP(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK, x)
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_GET \
+ FIELD_GET(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK, x)
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK GENMASK(15, 8)
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_SET(x) \
+ FIELD_PREP(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK, x)
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_GET(x) \
+ FIELD_GET(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK, x)
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_MSK LOWER8_MSK
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_SET LOWER8_SET
+#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_GET LOWER8_GET
+
+#define REG_CHAN_USR_CNTRL_2(c) (0x0424 + (c) * 0x40)
+#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_MSK UPPER16_MSK
+#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_SET UPPER16_SET
+#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_GET UPPER16_GET
+#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_MSK LOWER16_MSK
+#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_SET LOWER16_SET
+#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_GET LOWER16_GET
+
+/* IO Delay controls */
+#define __REG_DELAY_CONTROL(base, lane) ((base) + ((lane) * 0x04))
+
+#define REG_DELAY_CONTROL(lane) \
+ __REG_DELAY_CONTROL(0x0800, lane)
+#define REG_DELAY_CONTROL_MSK LOWER5_MSK
+#define REG_DELAY_CONTROL_SET LOWER5_SET
+#define REG_DELAY_CONTROL_GET LOWER5_GET
+
+struct adi_axi_adc_core_info {
+ unsigned int version;
+};
+
+struct adi_axi_adc_state {
+ struct mutex lock;
+
+ struct adi_axi_adc_client *client;
+ void __iomem *regs;
+};
+
+struct adi_axi_adc_client {
+ struct list_head entry;
+ struct adi_axi_adc_conv conv;
+ struct adi_axi_adc_state *state;
+ struct device *dev;
+ const struct adi_axi_adc_core_info *info;
+};
+
+static LIST_HEAD(registered_clients);
+static DEFINE_MUTEX(registered_clients_lock);
+
+static struct adi_axi_adc_client *conv_to_client(struct adi_axi_adc_conv *conv)
+{
+ if (!conv)
+ return NULL;
+ return container_of(conv, struct adi_axi_adc_client, conv);
+}
+
+void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv)
+{
+ struct adi_axi_adc_client *cl = conv_to_client(conv);
+
+ if (!cl)
+ return NULL;
+
+ return (char *)cl + ALIGN(sizeof(struct adi_axi_adc_client), IIO_ALIGN);
+}
+EXPORT_SYMBOL_GPL(adi_axi_adc_conv_priv);
+
+static void adi_axi_adc_write(struct adi_axi_adc_state *st,
+ unsigned int reg,
+ unsigned int val)
+{
+ iowrite32(val, st->regs + reg);
+}
+
+static unsigned int adi_axi_adc_read(struct adi_axi_adc_state *st,
+ unsigned int reg)
+{
+ return ioread32(st->regs + reg);
+}
+
+static int adi_axi_adc_config_dma_buffer(struct device *dev,
+ struct iio_dev *indio_dev)
+{
+ 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 adi_axi_adc_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct adi_axi_adc_state *st = iio_priv(indio_dev);
+ struct adi_axi_adc_conv *conv = &st->client->conv;
+
+ if (!conv->read_raw)
+ return -EOPNOTSUPP;
+
+ return conv->read_raw(conv, chan, val, val2, mask);
+}
+
+static int adi_axi_adc_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct adi_axi_adc_state *st = iio_priv(indio_dev);
+ struct adi_axi_adc_conv *conv = &st->client->conv;
+
+ if (!conv->write_raw)
+ return -EOPNOTSUPP;
+
+ return conv->write_raw(conv, chan, val, val2, mask);
+}
+
+static int adi_axi_adc_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ struct adi_axi_adc_state *st = iio_priv(indio_dev);
+ struct adi_axi_adc_conv *conv = &st->client->conv;
+ unsigned int i, ctrl;
+
+ for (i = 0; i < conv->chip_info->num_channels; i++) {
+ ctrl = adi_axi_adc_read(st, REG_CHAN_CNTRL(i));
+
+ if (test_bit(i, scan_mask))
+ ctrl |= REG_CHAN_CNTRL_ENABLE;
+ else
+ ctrl &= ~REG_CHAN_CNTRL_ENABLE;
+
+ adi_axi_adc_write(st, REG_CHAN_CNTRL(i), ctrl);
+ }
+
+ return 0;
+}
+
+static struct adi_axi_adc_conv *adi_axi_adc_conv_register(struct device *dev,
+ int sizeof_priv)
+{
+ struct adi_axi_adc_client *cl;
+ size_t alloc_size;
+
+ alloc_size = sizeof(struct adi_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(®istered_clients_lock);
+
+ get_device(dev);
+ cl->dev = dev;
+
+ list_add_tail(&cl->entry, ®istered_clients);
+
+ mutex_unlock(®istered_clients_lock);
+
+ return &cl->conv;
+}
+
+static void adi_axi_adc_conv_unregister(struct adi_axi_adc_conv *conv)
+{
+ struct adi_axi_adc_client *cl = conv_to_client(conv);
+
+ if (!cl)
+ return;
+
+ mutex_lock(®istered_clients_lock);
+
+ put_device(cl->dev);
+ list_del(&cl->entry);
+ kfree(cl);
+
+ mutex_unlock(®istered_clients_lock);
+}
+
+static void devm_adi_axi_adc_conv_release(struct device *dev, void *res)
+{
+ adi_axi_adc_conv_unregister(*(struct adi_axi_adc_conv **)res);
+}
+
+struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,
+ int sizeof_priv)
+{
+ struct adi_axi_adc_conv **ptr, *conv;
+
+ ptr = devres_alloc(devm_adi_axi_adc_conv_release, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ conv = adi_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_adi_axi_adc_conv_register);
+
+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 adi_axi_adc_state *st = iio_priv(indio_dev);
+ struct adi_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 *adi_axi_adc_attributes[] = {
+ &iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group adi_axi_adc_attribute_group = {
+ .attrs = adi_axi_adc_attributes,
+};
+
+static const struct iio_info adi_axi_adc_info = {
+ .read_raw = &adi_axi_adc_read_raw,
+ .write_raw = &adi_axi_adc_write_raw,
+ .attrs = &adi_axi_adc_attribute_group,
+ .update_scan_mode = &adi_axi_adc_update_scan_mode,
+};
+
+static const struct adi_axi_adc_core_info adi_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 adi_axi_adc_of_match[] = {
+ { .compatible = "adi,axi-adc-10.0.a", .data = &adi_axi_adc_10_0_a_info },
+ { /* end of list */ },
+};
+MODULE_DEVICE_TABLE(of, adi_axi_adc_of_match);
+
+struct adi_axi_adc_client *adi_axi_adc_attach_client(struct device *dev)
+{
+ const struct of_device_id *id;
+ struct adi_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(adi_axi_adc_of_match, dev->of_node);
+ if (!id)
+ return ERR_PTR(-ENODEV);
+
+ cln = of_parse_phandle(dev->of_node, "adi,adc-dev", 0);
+ if (!cln) {
+ dev_err(dev, "No 'adi,adc-dev' node defined\n");
+ return ERR_PTR(-ENODEV);
+ }
+
+ mutex_lock(®istered_clients_lock);
+
+ list_for_each_entry(cl, ®istered_clients, entry) {
+ if (!cl->dev)
+ continue;
+ if (cl->dev->of_node == cln) {
+ if (!try_module_get(dev->driver->owner)) {
+ mutex_unlock(®istered_clients_lock);
+ return ERR_PTR(-ENODEV);
+ }
+ get_device(dev);
+ cl->info = id->data;
+ mutex_unlock(®istered_clients_lock);
+ return cl;
+ }
+ }
+
+ mutex_unlock(®istered_clients_lock);
+
+ return ERR_PTR(-EPROBE_DEFER);
+}
+
+static int adi_axi_adc_setup_channels(struct device *dev,
+ struct adi_axi_adc_state *st)
+{
+ struct adi_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 = REG_CHAN_CNTRL_2_IQCOR_COEFF_2_SET(IQCOR_INT_1);
+ else
+ val = REG_CHAN_CNTRL_2_IQCOR_COEFF_1_SET(IQCOR_INT_1);
+ adi_axi_adc_write(st, REG_CHAN_CNTRL_2(i), val);
+
+ adi_axi_adc_write(st, REG_CHAN_CNTRL(i),
+ REG_CHAN_CNTRL_DEFAULTS);
+ }
+
+ return 0;
+}
+
+static void axi_adc_reset(struct adi_axi_adc_state *st)
+{
+ adi_axi_adc_write(st, REG_RSTN, 0);
+ mdelay(10);
+ adi_axi_adc_write(st, REG_RSTN, REG_RSTN_MMCM_RSTN);
+ mdelay(10);
+ adi_axi_adc_write(st, REG_RSTN,
+ REG_RSTN_RSTN | REG_RSTN_MMCM_RSTN);
+}
+
+static void adi_axi_adc_cleanup(void *data)
+{
+ struct adi_axi_adc_client *cl = data;
+
+ put_device(cl->dev);
+ module_put(cl->dev->driver->owner);
+}
+
+static int adi_axi_adc_probe(struct platform_device *pdev)
+{
+ struct adi_axi_adc_conv *conv;
+ struct iio_dev *indio_dev;
+ struct adi_axi_adc_client *cl;
+ struct adi_axi_adc_state *st;
+ unsigned int ver;
+ int ret;
+
+ cl = adi_axi_adc_attach_client(&pdev->dev);
+ if (IS_ERR(cl))
+ return PTR_ERR(cl);
+
+ ret = devm_add_action_or_reset(&pdev->dev, adi_axi_adc_cleanup, cl);
+ if (ret)
+ return ret;
+
+ 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);
+
+ st->regs = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(st->regs))
+ return PTR_ERR(st->regs);
+
+ conv = &st->client->conv;
+
+ axi_adc_reset(st);
+
+ ver = adi_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 = &adi_axi_adc_info;
+ indio_dev->dev.parent = &pdev->dev;
+ indio_dev->name = dev_name(&pdev->dev);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->num_channels = conv->chip_info->num_channels;
+ indio_dev->channels = conv->chip_info->channels;
+
+ ret = adi_axi_adc_config_dma_buffer(&pdev->dev, indio_dev);
+ if (ret)
+ return ret;
+
+ ret = adi_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 adi_axi_adc_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = adi_axi_adc_of_match,
+ },
+ .probe = adi_axi_adc_probe,
+};
+module_platform_driver(adi_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/adi-axi-adc.h b/include/linux/iio/adc/adi-axi-adc.h
new file mode 100644
index 000000000000..57974944b30b
--- /dev/null
+++ b/include/linux/iio/adc/adi-axi-adc.h
@@ -0,0 +1,63 @@
+/* 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 __ADI_AXI_ADC_H__
+#define __ADI_AXI_ADC_H__
+
+struct device;
+struct iio_chan_spec;
+
+/**
+ * struct adi_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 adi_axi_adc_chip_info {
+ const char *name;
+ unsigned int id;
+
+ const struct iio_chan_spec *channels;
+ unsigned int num_channels;
+
+ const unsigned int (*scale_table)[2];
+ int num_scales;
+
+ unsigned long max_rate;
+};
+
+/**
+ * struct adi_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 adi_axi_adc_conv {
+ const struct adi_axi_adc_chip_info *chip_info;
+
+ int (*preenable_setup)(struct adi_axi_adc_conv *conv);
+ int (*reg_access)(struct adi_axi_adc_conv *conv, unsigned int reg,
+ unsigned int writeval, unsigned int *readval);
+ int (*read_raw)(struct adi_axi_adc_conv *conv,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask);
+ int (*write_raw)(struct adi_axi_adc_conv *conv,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask);
+};
+
+struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,
+ int sizeof_priv);
+
+void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv);
+
+#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 6/8] dt-bindings: iio: adc: add bindings doc for AXI ADC driver
2020-03-06 11:00 [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC Alexandru Ardelean
` (4 preceding siblings ...)
2020-03-06 11:00 ` [PATCH v8 5/8] iio: adc: adi-axi-adc: add support for AXI ADC IP core Alexandru Ardelean
@ 2020-03-06 11:00 ` Alexandru Ardelean
2020-03-07 14:55 ` Jonathan Cameron
2020-03-06 11:00 ` [PATCH v8 7/8] iio: adc: ad9467: add support AD9467 ADC Alexandru Ardelean
2020-03-06 11:01 ` [PATCH v8 8/8] dt-bindings: iio: adc: add bindings doc for " Alexandru Ardelean
7 siblings, 1 reply; 22+ messages in thread
From: Alexandru Ardelean @ 2020-03-06 11:00 UTC (permalink / raw)
To: linux-iio, linux-kernel, devicetree; +Cc: jic23, robh+dt, 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 | 63 +++++++++++++++++++
1 file changed, 63 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..6bd80e241f40
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.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,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:
+ maxItems: 1
+
+ dma-names:
+ maxItems: 1
+ items:
+ - const: rx
+
+ adi,adc-dev:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ A reference to a the actual ADC to which this FPGA ADC interfaces to.
+
+required:
+ - compatible
+ - dmas
+ - reg
+ - adi,adc-dev
+
+additionalProperties: false
+
+examples:
+ - |
+ axi-adc@44a00000 {
+ compatible = "adi,axi-adc-10.0.a";
+ reg = <0x44a00000 0x10000>;
+ dmas = <&rx_dma 0>;
+ dma-names = "rx";
+
+ adi,adc-dev = <&spi_adc>;
+ };
+...
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v8 7/8] iio: adc: ad9467: add support AD9467 ADC
2020-03-06 11:00 [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC Alexandru Ardelean
` (5 preceding siblings ...)
2020-03-06 11:00 ` [PATCH v8 6/8] dt-bindings: iio: adc: add bindings doc for AXI ADC driver Alexandru Ardelean
@ 2020-03-06 11:00 ` Alexandru Ardelean
2020-03-07 15:05 ` Jonathan Cameron
2020-03-06 11:01 ` [PATCH v8 8/8] dt-bindings: iio: adc: add bindings doc for " Alexandru Ardelean
7 siblings, 1 reply; 22+ messages in thread
From: Alexandru Ardelean @ 2020-03-06 11:00 UTC (permalink / raw)
To: linux-iio, linux-kernel, devicetree
Cc: jic23, robh+dt, 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 | 432 +++++++++++++++++++++++++++++++++++++++
3 files changed, 448 insertions(+)
create mode 100644 drivers/iio/adc/ad9467.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 445070abf376..a0796510f9d4 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 ADI_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 ADI_AXI_ADC
tristate "Analog Devices Generic AXI ADC IP core driver"
select IIO_BUFFER
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7c6594d049f9..59722770a654 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_ADI_AXI_ADC) += adi-axi-adc.o
obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
obj-$(CONFIG_AT91_ADC) += at91_adc.o
diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
new file mode 100644
index 000000000000..35324bd1f0ee
--- /dev/null
+++ b/drivers/iio/adc/ad9467.c
@@ -0,0 +1,432 @@
+// 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/adi-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 AN877_ADC_REG_CHIP_PORT_CONF 0x00
+#define AN877_ADC_REG_CHIP_ID 0x01
+#define AN877_ADC_REG_CHIP_GRADE 0x02
+#define AN877_ADC_REG_CHAN_INDEX 0x05
+#define AN877_ADC_REG_TRANSFER 0xFF
+#define AN877_ADC_REG_MODES 0x08
+#define AN877_ADC_REG_TEST_IO 0x0D
+#define AN877_ADC_REG_ADC_INPUT 0x0F
+#define AN877_ADC_REG_OFFSET 0x10
+#define AN877_ADC_REG_OUTPUT_MODE 0x14
+#define AN877_ADC_REG_OUTPUT_ADJUST 0x15
+#define AN877_ADC_REG_OUTPUT_PHASE 0x16
+#define AN877_ADC_REG_OUTPUT_DELAY 0x17
+#define AN877_ADC_REG_VREF 0x18
+#define AN877_ADC_REG_ANALOG_INPUT 0x2C
+
+/* AN877_ADC_REG_TEST_IO */
+#define AN877_ADC_TESTMODE_OFF 0x0
+#define AN877_ADC_TESTMODE_MIDSCALE_SHORT 0x1
+#define AN877_ADC_TESTMODE_POS_FULLSCALE 0x2
+#define AN877_ADC_TESTMODE_NEG_FULLSCALE 0x3
+#define AN877_ADC_TESTMODE_ALT_CHECKERBOARD 0x4
+#define AN877_ADC_TESTMODE_PN23_SEQ 0x5
+#define AN877_ADC_TESTMODE_PN9_SEQ 0x6
+#define AN877_ADC_TESTMODE_ONE_ZERO_TOGGLE 0x7
+#define AN877_ADC_TESTMODE_USER 0x8
+#define AN877_ADC_TESTMODE_BIT_TOGGLE 0x9
+#define AN877_ADC_TESTMODE_SYNC 0xA
+#define AN877_ADC_TESTMODE_ONE_BIT_HIGH 0xB
+#define AN877_ADC_TESTMODE_MIXED_BIT_FREQUENCY 0xC
+#define AN877_ADC_TESTMODE_RAMP 0xF
+
+/* AN877_ADC_REG_TRANSFER */
+#define AN877_ADC_TRANSFER_SYNC 0x1
+
+/* AN877_ADC_REG_OUTPUT_MODE */
+#define AN877_ADC_OUTPUT_MODE_OFFSET_BINARY 0x0
+#define AN877_ADC_OUTPUT_MODE_TWOS_COMPLEMENT 0x1
+#define AN877_ADC_OUTPUT_MODE_GRAY_CODE 0x2
+
+/* AN877_ADC_REG_OUTPUT_PHASE */
+#define AN877_ADC_OUTPUT_EVEN_ODD_MODE_EN 0x20
+#define AN877_ADC_INVERT_DCO_CLK 0x80
+
+/* AN877_ADC_REG_OUTPUT_DELAY */
+#define AN877_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 adi_axi_adc_conv *conv, unsigned int reg,
+ unsigned int writeval, unsigned int *readval)
+{
+ struct ad9467_state *st = adi_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, AN877_ADC_REG_TRANSFER,
+ AN877_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 adi_axi_adc_conv *conv, int index,
+ unsigned int *val, unsigned int *val2)
+{
+ const struct adi_axi_adc_chip_info *info = conv->chip_info;
+ const struct iio_chan_spec *chan = &info->channels[0];
+ 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_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, \
+ }, \
+}
+
+static const struct iio_chan_spec ad9467_channels[] = {
+ AD9467_CHAN(0, 0, 16, 'S'),
+};
+
+static const struct adi_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 adi_axi_adc_conv *conv, int *val, int *val2)
+{
+ const struct adi_axi_adc_chip_info *info = conv->chip_info;
+ struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
+ unsigned int i, vref_val, vref_mask;
+
+ vref_val = ad9467_spi_read(st->spi, AN877_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 adi_axi_adc_conv *conv, int val, int val2)
+{
+ const struct adi_axi_adc_chip_info *info = conv->chip_info;
+ struct ad9467_state *st = adi_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, AN877_ADC_REG_VREF,
+ info->scale_table[i][1]);
+ ad9467_spi_write(st->spi, AN877_ADC_REG_TRANSFER,
+ AN877_ADC_TRANSFER_SYNC);
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int ad9467_read_raw(struct adi_axi_adc_conv *conv,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long m)
+{
+ struct ad9467_state *st = adi_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;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad9467_write_raw(struct adi_axi_adc_conv *conv,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ const struct adi_axi_adc_chip_info *info = conv->chip_info;
+ struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
+ unsigned long r_clk;
+
+ 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;
+ }
+
+ return clk_set_rate(st->clk, r_clk);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad9467_outputmode_set(struct spi_device *spi, unsigned int mode)
+{
+ int ret;
+
+ ret = ad9467_spi_write(spi, AN877_ADC_REG_OUTPUT_MODE, mode);
+ if (ret < 0)
+ return ret;
+
+ return ad9467_spi_write(spi, AN877_ADC_REG_TRANSFER,
+ AN877_ADC_TRANSFER_SYNC);
+}
+
+static int ad9467_preenable_setup(struct adi_axi_adc_conv *conv)
+{
+ struct ad9467_state *st = adi_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 |
+ AN877_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static void ad9467_clk_disable(void *data)
+{
+ struct ad9467_state *st = data;
+
+ clk_disable_unprepare(st->clk);
+}
+
+static const struct of_device_id ad9467_of_match[] = {
+ { .compatible = "adi,ad9467", .data = (void *)ID_AD9467, },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ad9467_of_match);
+
+static int ad9467_probe(struct spi_device *spi)
+{
+ const struct of_device_id *oid;
+ struct adi_axi_adc_conv *conv;
+ struct ad9467_state *st;
+ unsigned int id;
+ int ret;
+
+ if (!spi->dev.of_node) {
+ dev_err(&spi->dev, "DT node is null\n");
+ return -ENODEV;
+ }
+
+ oid = of_match_node(ad9467_of_match, spi->dev.of_node);
+ if (!oid)
+ return -ENODEV;
+
+ conv = devm_adi_axi_adc_conv_register(&spi->dev, sizeof(*st));
+ if (IS_ERR(conv))
+ return PTR_ERR(conv);
+
+ st = adi_axi_adc_conv_priv(conv);
+ st->spi = spi;
+
+ st->clk = devm_clk_get(&spi->dev, "adc-clk");
+ 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 = (unsigned int)oid->data;
+ conv->chip_info = &ad9467_chip_info_tbl[id];
+
+ id = ad9467_spi_read(spi, AN877_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 struct spi_driver ad9467_driver = {
+ .driver = {
+ .name = "ad9467",
+ .of_match_table = ad9467_of_match,
+ },
+ .probe = ad9467_probe,
+};
+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] 22+ messages in thread
* [PATCH v8 8/8] dt-bindings: iio: adc: add bindings doc for AD9467 ADC
2020-03-06 11:00 [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC Alexandru Ardelean
` (6 preceding siblings ...)
2020-03-06 11:00 ` [PATCH v8 7/8] iio: adc: ad9467: add support AD9467 ADC Alexandru Ardelean
@ 2020-03-06 11:01 ` Alexandru Ardelean
7 siblings, 0 replies; 22+ messages in thread
From: Alexandru Ardelean @ 2020-03-06 11:01 UTC (permalink / raw)
To: linux-iio, linux-kernel, devicetree; +Cc: jic23, robh+dt, 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 | 65 +++++++++++++++++++
1 file changed, 65 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..c4f57fa6aad1
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad9467.yaml
@@ -0,0 +1,65 @@
+# 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
+
+ clock-names:
+ items:
+ - const: adc-clk
+
+ 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>;
+ clocks = <&adc_clk>;
+ clock-names = "adc-clk";
+ };
+ };
+...
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v8 1/8] include: fpga: adi-axi-common.h: fixup whitespace tab -> space
2020-03-06 11:00 ` [PATCH v8 1/8] include: fpga: adi-axi-common.h: fixup whitespace tab -> space Alexandru Ardelean
@ 2020-03-07 14:25 ` Jonathan Cameron
2020-03-07 19:56 ` Moritz Fischer
0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2020-03-07 14:25 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, devicetree, robh+dt, Moritz Fischer
On Fri, 6 Mar 2020 13:00:53 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> The initial version use a tab between '#define' & 'ADI_AXI_REG_VERSION'.
> This changes it to space. The change is purely cosmetic.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Whilst this is trivial it still needs an ack from relevant maintainer
for that directory. Moritz I think...
Jonathan
> ---
> include/linux/fpga/adi-axi-common.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/fpga/adi-axi-common.h b/include/linux/fpga/adi-axi-common.h
> index 7fc95d5c95bb..ebd4e07ae3d8 100644
> --- a/include/linux/fpga/adi-axi-common.h
> +++ b/include/linux/fpga/adi-axi-common.h
> @@ -11,7 +11,7 @@
> #ifndef ADI_AXI_COMMON_H_
> #define ADI_AXI_COMMON_H_
>
> -#define ADI_AXI_REG_VERSION 0x0000
> +#define ADI_AXI_REG_VERSION 0x0000
>
> #define ADI_AXI_PCORE_VER(major, minor, patch) \
> (((major) << 16) | ((minor) << 8) | (patch))
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 2/8] include: fpga: adi-axi-common.h: add version helper macros
2020-03-06 11:00 ` [PATCH v8 2/8] include: fpga: adi-axi-common.h: add version helper macros Alexandru Ardelean
@ 2020-03-07 14:26 ` Jonathan Cameron
2020-03-07 19:56 ` Moritz Fischer
0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2020-03-07 14:26 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, devicetree, robh+dt, Moritz Fischer
On Fri, 6 Mar 2020 13:00:54 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> The format for all ADI AXI IP cores is the same.
> i.e. 'major.minor.patch'.
>
> This patch adds the helper macros to be re-used in ADI AXI drivers.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Again, trivial but needs a Moritz ack as it's his subsystem.
> ---
> include/linux/fpga/adi-axi-common.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/linux/fpga/adi-axi-common.h b/include/linux/fpga/adi-axi-common.h
> index ebd4e07ae3d8..141ac3f251e6 100644
> --- a/include/linux/fpga/adi-axi-common.h
> +++ b/include/linux/fpga/adi-axi-common.h
> @@ -16,4 +16,8 @@
> #define ADI_AXI_PCORE_VER(major, minor, patch) \
> (((major) << 16) | ((minor) << 8) | (patch))
>
> +#define ADI_AXI_PCORE_VER_MAJOR(version) (((version) >> 16) & 0xff)
> +#define ADI_AXI_PCORE_VER_MINOR(version) (((version) >> 8) & 0xff)
> +#define ADI_AXI_PCORE_VER_PATCH(version) ((version) & 0xff)
> +
> #endif /* ADI_AXI_COMMON_H_ */
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 5/8] iio: adc: adi-axi-adc: add support for AXI ADC IP core
2020-03-06 11:00 ` [PATCH v8 5/8] iio: adc: adi-axi-adc: add support for AXI ADC IP core Alexandru Ardelean
@ 2020-03-07 14:54 ` Jonathan Cameron
2020-03-09 11:34 ` Ardelean, Alexandru
0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2020-03-07 14:54 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, devicetree, robh+dt, Michael Hennerich,
Lars-Peter Clausen
On Fri, 6 Mar 2020 13:00:57 +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>
Mostly looks fine, but a few nitpicks from rereading it.
Thanks,
Jonathan
> ---
> drivers/iio/adc/Kconfig | 20 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/adi-axi-adc.c | 618 ++++++++++++++++++++++++++++
> include/linux/iio/adc/adi-axi-adc.h | 63 +++
> 4 files changed, 702 insertions(+)
> create mode 100644 drivers/iio/adc/adi-axi-adc.c
> create mode 100644 include/linux/iio/adc/adi-axi-adc.h
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f4da821c4022..445070abf376 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -246,6 +246,26 @@ config AD799X
> To compile this driver as a module, choose M here: the module will be
> called ad799x.
>
> +config ADI_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 adi-axi-adc.
> +
> 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 8462455b4228..7c6594d049f9 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_ADI_AXI_ADC) += adi-axi-adc.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/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> new file mode 100644
> index 000000000000..17ee20015d71
> --- /dev/null
> +++ b/drivers/iio/adc/adi-axi-adc.c
> @@ -0,0 +1,618 @@
> +// 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/adi-axi-adc.h>
> +
> +/**
> + * Register definitions:
> + * https://wiki.analog.com/resources/fpga/docs/axi_adc_ip#register_map
> + */
> +
> +#define LOWER5_MSK GENMASK(4, 0)
> +#define LOWER5_SET(x) FIELD_PREP(LOWER5_MSK, x)
> +#define LOWER5_GET(x) FIELD_GET(LOWER5_MSK, x)
Whilst it may cause more repetition, I'd rather just see these used where
relevant inline. That will be slightly easier to review.
> +
> +#define LOWER8_MSK GENMASK(7, 0)
> +#define LOWER8_SET(x) FIELD_PREP(LOWER8_MSK, x)
> +#define LOWER8_GET(x) FIELD_GET(LOWER8_MSK, x)
> +
> +#define UPPER16_MSK GENMASK(31, 16)
> +#define UPPER16_SET(x) FIELD_PREP(UPPER16_MSK, x)
> +#define UPPER16_GET(x) FIELD_GET(UPPER16_MSK, x)
> +
> +#define LOWER16_MSK GENMASK(15, 0)
> +#define LOWER16_SET(x) FIELD_PREP(LOWER16_MSK, x)
> +#define LOWER16_GET(x) FIELD_GET(LOWER16_MSK, x)
> +
> +/* ADC controls */
> +
> +#define REG_RSTN 0x0040
Usual stuff: These should be prefixed with driver relevant prefix
maybe ADI_AXI_
> +#define REG_RSTN_CE_N BIT(2)
> +#define REG_RSTN_MMCM_RSTN BIT(1)
> +#define REG_RSTN_RSTN BIT(0)
> +
> +#define REG_CNTRL 0x0044
> +#define REG_CNTRL_SYNC BIT(3)
> +#define REG_CNTRL_R1_MODE BIT(2)
> +#define REG_CNTRL_DDR_EDGESEL BIT(1)
> +#define REG_CNTRL_PIN_MODE BIT(0)
> +
> +#define REG_CLK_FREQ 0x0054
> +#define REG_CLK_RATIO 0x0058
> +
> +#define REG_STATUS 0x005C
> +#define REG_STATUS_PN_ERR BIT(3)
> +#define REG_STATUS_PN_OOS BIT(2)
> +#define REG_STATUS_OVER_RANGE BIT(1)
> +#define REG_STATUS_STATUS BIT(0)
> +
> +#define REG_SYNC_STATUS 0x0068
> +#define REG_SYNC_STATUS_SYNC BIT(0)
> +
> +#define REG_DRP_CNTRL 0x0070
> +#define REG_DRP_CNTRL_DRP_RWN BIT(28)
> +#define REG_DRP_CNTRL_DRP_ADDRESS_MSK GENMASK(27, 16)
> +#define REG_DRP_CNTRL_DRP_ADDRESS_SET(x) \
> + FIELD_PREP(REG_DRP_CNTRL_DRP_ADDRESS_MSK, x)
> +#define REG_DRP_CNTRL_DRP_ADDRESS_GET(x) \
> + FIELD_GET(REG_DRP_CNTRL_DRP_ADDRESS_MSK, x)
> +
> +#define REG_DRP_STATUS 0x0074
> +#define REG_DRP_STATUS_DRP_LOCKED BIT(17)
> +#define REG_DRP_STATUS_DRP_STATUS BIT(16)
> +
> +#define REG_DRP_WDATA 0x0078
> +#define REG_DRP_WDATA_SET LOWER16_SET
> +#define REG_DRP_WDATA_GET LOWER16_GET
As mentioned above, I'd rather see the mask here. Unless used
an awful lot, I'd rather just see the FIELD_PREP and FIELD_GET
used inline rather than defining a bunch of _SET/_GET only some
of which seem to be used so far.
> +
> +#define REG_DRP_RDATA 0x007C
> +#define REG_DRP_RDATA_SET LOWER16_SET
> +#define REG_DRP_RDATA_GET LOWER16_GET
> +
> +#define REG_UI_STATUS 0x0088
> +#define REG_UI_STATUS_OVF BIT(2)
> +#define REG_UI_STATUS_UNF BIT(1)
> +
> +#define REG_USR_CNTRL_1 0x00A0
> +#define REG_USR_CNTRL_1_USR_CHANMAX_MSK LOWER8_MSK
> +#define REG_USR_CNTRL_1_USR_CHANMAX_SET LOWER8_SET
> +#define REG_USR_CNTRL_1_USR_CHANMAX_GET LOWER8_GET
> +
> +#define REG_ADC_START_CODE 0x00A4
> +#define REG_ADC_GPIO_IN 0x00B8
> +#define REG_ADC_GPIO_OUT 0x00BC
> +
> +#define REG_PPS_COUNTER 0x00C0
> +
> +#define REG_PPS_STATUS 0x00C4
> +#define REG_PPS_STATUS_PPS_STATUS BIT(0)
> +
> +/* ADC Channel controls */
> +
> +#define REG_CHAN_CNTRL(c) (0x0400 + (c) * 0x40)
> +#define REG_CHAN_CNTRL_LB_OWR BIT(11)
> +#define REG_CHAN_CNTRL_PN_SEL_OWR BIT(10)
> +#define REG_CHAN_CNTRL_IQCOR_ENB BIT(9)
> +#define REG_CHAN_CNTRL_DCFILT_ENB BIT(8)
> +#define REG_CHAN_CNTRL_FORMAT_SIGNEXT BIT(6)
> +#define REG_CHAN_CNTRL_FORMAT_TYPE BIT(5)
> +#define REG_CHAN_CNTRL_FORMAT_ENABLE BIT(4)
> +#define REG_CHAN_CNTRL_ADC_PN_TYPE_OWR BIT(1)
> +#define REG_CHAN_CNTRL_ENABLE BIT(0)
> +
> +#define REG_CHAN_CNTRL_DEFAULTS \
> + (REG_CHAN_CNTRL_FORMAT_SIGNEXT | REG_CHAN_CNTRL_FORMAT_ENABLE | \
> + REG_CHAN_CNTRL_IQCOR_ENB | REG_CHAN_CNTRL_ENABLE)
> +
> +#define REG_CHAN_STATUS(c) (0x0404 + (c) * 0x40)
> +#define REG_CHAN_STATUS_PN_ERR BIT(2)
> +#define REG_CHAN_STATUS_PN_OOS BIT(1)
> +#define REG_CHAN_STATUS_OVER_RANGE BIT(0)
> +
> +#define REG_CHAN_CNTRL_1(c) (0x0410 + (c) * 0x40)
> +#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_MSK UPPER16_MSK
> +#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_SET UPPER16_SET
> +#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_GET UPPER16_GET
> +#define REG_CHAN_CNTRL_1_DCFILT_COEFF_MSK LOWER16_MSK
> +#define REG_CHAN_CNTRL_1_DCFILT_COEFF_SET LOWER16_SET
> +#define REG_CHAN_CNTRL_1_DCFILT_COEFF_GET LOWER16_GET
> +
> +#define REG_CHAN_CNTRL_2(c) (0x0414 + (c) * 0x40)
> +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_MSK UPPER16_MSK
> +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_SET UPPER16_SET
> +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_GET UPPER16_GET
> +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_MSK LOWER16_MSK
> +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_SET LOWER16_SET
> +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_GET LOWER16_GET
> +
> +/* format is 1.1.14 (sign, integer and fractional bits) */
> +#define IQCOR_INT_1 0x4000UL
> +#define IQCOR_SIGN_BIT BIT(15)
> +
> +#define REG_CHAN_CNTRL_3(c) (0x0418 + (c) * 0x40)
> +#define REG_CHAN_CNTRL_3_ADC_PN_SEL_MSK UPPER16_MSK
> +#define REG_CHAN_CNTRL_3_ADC_PN_SEL_SET UPPER16_SET
> +#define REG_CHAN_CNTRL_3_ADC_PN_SEL_GET UPPER16_GET
> +#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_MSK LOWER16_MSK
> +#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_SET LOWER16_SET
> +#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_GET LOWER16_GET
> +
> +#define REG_CHAN_USR_CNTRL_1(c) (0x0420 + (c) * 0x40)
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BE BIT(25)
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SIGNED BIT(24)
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK GENMASK(23, 16)
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_SET(x) \
> + FIELD_PREP(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK, x)
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_GET \
> + FIELD_GET(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK, x)
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK GENMASK(15, 8)
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_SET(x) \
> + FIELD_PREP(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK, x)
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_GET(x) \
> + FIELD_GET(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK, x)
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_MSK LOWER8_MSK
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_SET LOWER8_SET
> +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_GET LOWER8_GET
> +
> +#define REG_CHAN_USR_CNTRL_2(c) (0x0424 + (c) * 0x40)
> +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_MSK UPPER16_MSK
> +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_SET UPPER16_SET
> +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_GET UPPER16_GET
> +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_MSK LOWER16_MSK
> +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_SET LOWER16_SET
> +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_GET LOWER16_GET
> +
> +/* IO Delay controls */
> +#define __REG_DELAY_CONTROL(base, lane) ((base) + ((lane) * 0x04))
> +
> +#define REG_DELAY_CONTROL(lane) \
> + __REG_DELAY_CONTROL(0x0800, lane)
> +#define REG_DELAY_CONTROL_MSK LOWER5_MSK
> +#define REG_DELAY_CONTROL_SET LOWER5_SET
> +#define REG_DELAY_CONTROL_GET LOWER5_GET
> +
> +struct adi_axi_adc_core_info {
> + unsigned int version;
> +};
> +
> +struct adi_axi_adc_state {
> + struct mutex lock;
> +
> + struct adi_axi_adc_client *client;
> + void __iomem *regs;
> +};
> +
> +struct adi_axi_adc_client {
> + struct list_head entry;
> + struct adi_axi_adc_conv conv;
> + struct adi_axi_adc_state *state;
> + struct device *dev;
> + const struct adi_axi_adc_core_info *info;
> +};
> +
> +static LIST_HEAD(registered_clients);
> +static DEFINE_MUTEX(registered_clients_lock);
> +
> +static struct adi_axi_adc_client *conv_to_client(struct adi_axi_adc_conv *conv)
> +{
> + if (!conv)
> + return NULL;
> + return container_of(conv, struct adi_axi_adc_client, conv);
> +}
> +
> +void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv)
> +{
> + struct adi_axi_adc_client *cl = conv_to_client(conv);
> +
> + if (!cl)
> + return NULL;
> +
> + return (char *)cl + ALIGN(sizeof(struct adi_axi_adc_client), IIO_ALIGN);
> +}
> +EXPORT_SYMBOL_GPL(adi_axi_adc_conv_priv);
> +
> +static void adi_axi_adc_write(struct adi_axi_adc_state *st,
> + unsigned int reg,
> + unsigned int val)
> +{
> + iowrite32(val, st->regs + reg);
> +}
> +
> +static unsigned int adi_axi_adc_read(struct adi_axi_adc_state *st,
> + unsigned int reg)
> +{
> + return ioread32(st->regs + reg);
> +}
> +
> +static int adi_axi_adc_config_dma_buffer(struct device *dev,
> + struct iio_dev *indio_dev)
> +{
> + 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 adi_axi_adc_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct adi_axi_adc_state *st = iio_priv(indio_dev);
> + struct adi_axi_adc_conv *conv = &st->client->conv;
> +
> + if (!conv->read_raw)
> + return -EOPNOTSUPP;
> +
> + return conv->read_raw(conv, chan, val, val2, mask);
> +}
> +
> +static int adi_axi_adc_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct adi_axi_adc_state *st = iio_priv(indio_dev);
> + struct adi_axi_adc_conv *conv = &st->client->conv;
> +
> + if (!conv->write_raw)
> + return -EOPNOTSUPP;
> +
> + return conv->write_raw(conv, chan, val, val2, mask);
> +}
> +
> +static int adi_axi_adc_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct adi_axi_adc_state *st = iio_priv(indio_dev);
> + struct adi_axi_adc_conv *conv = &st->client->conv;
> + unsigned int i, ctrl;
> +
> + for (i = 0; i < conv->chip_info->num_channels; i++) {
> + ctrl = adi_axi_adc_read(st, REG_CHAN_CNTRL(i));
> +
> + if (test_bit(i, scan_mask))
> + ctrl |= REG_CHAN_CNTRL_ENABLE;
> + else
> + ctrl &= ~REG_CHAN_CNTRL_ENABLE;
> +
> + adi_axi_adc_write(st, REG_CHAN_CNTRL(i), ctrl);
> + }
> +
> + return 0;
> +}
> +
> +static struct adi_axi_adc_conv *adi_axi_adc_conv_register(struct device *dev,
> + int sizeof_priv)
> +{
> + struct adi_axi_adc_client *cl;
> + size_t alloc_size;
> +
> + alloc_size = sizeof(struct adi_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(®istered_clients_lock);
> +
> + get_device(dev);
> + cl->dev = dev;
> +
> + list_add_tail(&cl->entry, ®istered_clients);
> +
> + mutex_unlock(®istered_clients_lock);
> +
> + return &cl->conv;
> +}
> +
> +static void adi_axi_adc_conv_unregister(struct adi_axi_adc_conv *conv)
> +{
> + struct adi_axi_adc_client *cl = conv_to_client(conv);
> +
> + if (!cl)
> + return;
> +
> + mutex_lock(®istered_clients_lock);
> +
> + put_device(cl->dev);
> + list_del(&cl->entry);
This isn't reverse of register. Why?
Should be list_del then put_device.
> + kfree(cl);
To reverse the register precisely the kfree(cl) should be outside
the lock...
> +
> + mutex_unlock(®istered_clients_lock);
> +}
> +
> +static void devm_adi_axi_adc_conv_release(struct device *dev, void *res)
> +{
> + adi_axi_adc_conv_unregister(*(struct adi_axi_adc_conv **)res);
> +}
> +
> +struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,
> + int sizeof_priv)
> +{
> + struct adi_axi_adc_conv **ptr, *conv;
> +
> + ptr = devres_alloc(devm_adi_axi_adc_conv_release, sizeof(*ptr),
> + GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + conv = adi_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_adi_axi_adc_conv_register);
> +
> +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 adi_axi_adc_state *st = iio_priv(indio_dev);
> + struct adi_axi_adc_conv *conv = &st->client->conv;
> + size_t len = 0;
> + int i;
> +
> + if (!conv->chip_info->num_scales) {
> + buf[0] = '\n';
> + return 1;
I'd prefer to see this done using the core available handling.
That way drivers that don't provide this would just not register
it in the first place. It will be a characteristic of the channels.
> + }
> +
> + 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 *adi_axi_adc_attributes[] = {
> + &iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group adi_axi_adc_attribute_group = {
> + .attrs = adi_axi_adc_attributes,
> +};
> +
> +static const struct iio_info adi_axi_adc_info = {
> + .read_raw = &adi_axi_adc_read_raw,
> + .write_raw = &adi_axi_adc_write_raw,
> + .attrs = &adi_axi_adc_attribute_group,
> + .update_scan_mode = &adi_axi_adc_update_scan_mode,
> +};
> +
> +static const struct adi_axi_adc_core_info adi_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 adi_axi_adc_of_match[] = {
> + { .compatible = "adi,axi-adc-10.0.a", .data = &adi_axi_adc_10_0_a_info },
> + { /* end of list */ },
> +};
> +MODULE_DEVICE_TABLE(of, adi_axi_adc_of_match);
> +
> +struct adi_axi_adc_client *adi_axi_adc_attach_client(struct device *dev)
> +{
> + const struct of_device_id *id;
> + struct adi_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(adi_axi_adc_of_match, dev->of_node);
> + if (!id)
> + return ERR_PTR(-ENODEV);
> +
> + cln = of_parse_phandle(dev->of_node, "adi,adc-dev", 0);
> + if (!cln) {
> + dev_err(dev, "No 'adi,adc-dev' node defined\n");
> + return ERR_PTR(-ENODEV);
> + }
> +
> + mutex_lock(®istered_clients_lock);
> +
> + list_for_each_entry(cl, ®istered_clients, entry) {
> + if (!cl->dev)
> + continue;
> + if (cl->dev->of_node == cln) {
> + if (!try_module_get(dev->driver->owner)) {
> + mutex_unlock(®istered_clients_lock);
> + return ERR_PTR(-ENODEV);
> + }
> + get_device(dev);
> + cl->info = id->data;
> + mutex_unlock(®istered_clients_lock);
> + return cl;
> + }
> + }
> +
> + mutex_unlock(®istered_clients_lock);
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int adi_axi_adc_setup_channels(struct device *dev,
> + struct adi_axi_adc_state *st)
> +{
> + struct adi_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 = REG_CHAN_CNTRL_2_IQCOR_COEFF_2_SET(IQCOR_INT_1);
> + else
> + val = REG_CHAN_CNTRL_2_IQCOR_COEFF_1_SET(IQCOR_INT_1);
> + adi_axi_adc_write(st, REG_CHAN_CNTRL_2(i), val);
> +
> + adi_axi_adc_write(st, REG_CHAN_CNTRL(i),
> + REG_CHAN_CNTRL_DEFAULTS);
> + }
> +
> + return 0;
> +}
> +
> +static void axi_adc_reset(struct adi_axi_adc_state *st)
> +{
> + adi_axi_adc_write(st, REG_RSTN, 0);
> + mdelay(10);
> + adi_axi_adc_write(st, REG_RSTN, REG_RSTN_MMCM_RSTN);
> + mdelay(10);
> + adi_axi_adc_write(st, REG_RSTN,
> + REG_RSTN_RSTN | REG_RSTN_MMCM_RSTN);
> +}
> +
> +static void adi_axi_adc_cleanup(void *data)
> +{
> + struct adi_axi_adc_client *cl = data;
> +
> + put_device(cl->dev);
> + module_put(cl->dev->driver->owner);
> +}
> +
> +static int adi_axi_adc_probe(struct platform_device *pdev)
> +{
> + struct adi_axi_adc_conv *conv;
> + struct iio_dev *indio_dev;
> + struct adi_axi_adc_client *cl;
> + struct adi_axi_adc_state *st;
> + unsigned int ver;
> + int ret;
> +
> + cl = adi_axi_adc_attach_client(&pdev->dev);
> + if (IS_ERR(cl))
> + return PTR_ERR(cl);
> +
> + ret = devm_add_action_or_reset(&pdev->dev, adi_axi_adc_cleanup, cl);
> + if (ret)
> + return ret;
> +
> + 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);
> +
> + st->regs = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(st->regs))
> + return PTR_ERR(st->regs);
> +
> + conv = &st->client->conv;
> +
> + axi_adc_reset(st);
> +
> + ver = adi_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 = &adi_axi_adc_info;
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->name = dev_name(&pdev->dev);
What does this result in? Given this is supposed to be a part number
I kind of expected this to be hardcoded to "adi-axi-adc" or similar.
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->num_channels = conv->chip_info->num_channels;
> + indio_dev->channels = conv->chip_info->channels;
> +
> + ret = adi_axi_adc_config_dma_buffer(&pdev->dev, indio_dev);
> + if (ret)
> + return ret;
> +
> + ret = adi_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 adi_axi_adc_driver = {
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = adi_axi_adc_of_match,
> + },
> + .probe = adi_axi_adc_probe,
> +};
> +module_platform_driver(adi_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/adi-axi-adc.h b/include/linux/iio/adc/adi-axi-adc.h
> new file mode 100644
> index 000000000000..57974944b30b
> --- /dev/null
> +++ b/include/linux/iio/adc/adi-axi-adc.h
> @@ -0,0 +1,63 @@
> +/* 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 __ADI_AXI_ADC_H__
> +#define __ADI_AXI_ADC_H__
> +
> +struct device;
> +struct iio_chan_spec;
> +
> +/**
> + * struct adi_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 adi_axi_adc_chip_info {
> + const char *name;
> + unsigned int id;
> +
> + const struct iio_chan_spec *channels;
> + unsigned int num_channels;
> +
> + const unsigned int (*scale_table)[2];
> + int num_scales;
> +
> + unsigned long max_rate;
> +};
> +
> +/**
> + * struct adi_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
Run the kernel doc script over this and it would have moaned...
No reg_access.
> + * @read_raw IIO read_raw hook for the client ADC
> + * @write_raw IIO write_raw hook for the client ADC
> + */
> +struct adi_axi_adc_conv {
> + const struct adi_axi_adc_chip_info *chip_info;
> +
> + int (*preenable_setup)(struct adi_axi_adc_conv *conv);
> + int (*reg_access)(struct adi_axi_adc_conv *conv, unsigned int reg,
> + unsigned int writeval, unsigned int *readval);
> + int (*read_raw)(struct adi_axi_adc_conv *conv,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask);
> + int (*write_raw)(struct adi_axi_adc_conv *conv,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask);
> +};
> +
> +struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,
> + int sizeof_priv);
> +
> +void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv);
> +
> +#endif
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 6/8] dt-bindings: iio: adc: add bindings doc for AXI ADC driver
2020-03-06 11:00 ` [PATCH v8 6/8] dt-bindings: iio: adc: add bindings doc for AXI ADC driver Alexandru Ardelean
@ 2020-03-07 14:55 ` Jonathan Cameron
0 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2020-03-07 14:55 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, devicetree, robh+dt, Moritz Fischer
On Fri, 6 Mar 2020 13:00:58 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> This change adds the bindings documentation for the AXI ADC driver.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Looks fine to me, but I'll be wanting an ack from a DT maintainer.
Thanks,
Jonathan
> ---
> .../bindings/iio/adc/adi,axi-adc.yaml | 63 +++++++++++++++++++
> 1 file changed, 63 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..6bd80e241f40
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.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,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:
> + maxItems: 1
> +
> + dma-names:
> + maxItems: 1
> + items:
> + - const: rx
> +
> + adi,adc-dev:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + A reference to a the actual ADC to which this FPGA ADC interfaces to.
> +
> +required:
> + - compatible
> + - dmas
> + - reg
> + - adi,adc-dev
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + axi-adc@44a00000 {
> + compatible = "adi,axi-adc-10.0.a";
> + reg = <0x44a00000 0x10000>;
> + dmas = <&rx_dma 0>;
> + dma-names = "rx";
> +
> + adi,adc-dev = <&spi_adc>;
> + };
> +...
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 7/8] iio: adc: ad9467: add support AD9467 ADC
2020-03-06 11:00 ` [PATCH v8 7/8] iio: adc: ad9467: add support AD9467 ADC Alexandru Ardelean
@ 2020-03-07 15:05 ` Jonathan Cameron
2020-03-09 11:51 ` Ardelean, Alexandru
0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2020-03-07 15:05 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, linux-kernel, devicetree, robh+dt, Michael Hennerich,
Lars-Peter Clausen
On Fri, 6 Mar 2020 13:00:59 +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 but otherwise looks good to me..
> ---
> drivers/iio/adc/Kconfig | 15 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ad9467.c | 432 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 448 insertions(+)
> create mode 100644 drivers/iio/adc/ad9467.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 445070abf376..a0796510f9d4 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.
>
...
> +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);
Why not split buf into send part and receive? Might make it slightly
more readable for no actual cost..
> +
> + if (ret < 0)
> + return ret;
> +
> + return buf[2];
> +}
...
> +static int ad9467_write_raw(struct adi_axi_adc_conv *conv,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + const struct adi_axi_adc_chip_info *info = conv->chip_info;
> + struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
> + unsigned long r_clk;
> +
> + 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)
This is a very 'odd' test. Why?
> + 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;
> + }
> +
> + return clk_set_rate(st->clk, r_clk);
> + default:
> + return -EINVAL;
> + }
> +}
> +
...
> +static int ad9467_probe(struct spi_device *spi)
> +{
> + const struct of_device_id *oid;
> + struct adi_axi_adc_conv *conv;
> + struct ad9467_state *st;
> + unsigned int id;
> + int ret;
> +
> + if (!spi->dev.of_node) {
> + dev_err(&spi->dev, "DT node is null\n");
> + return -ENODEV;
Silly question for you. Can this happen? We can only probe this
if it is in DT and hence there must be a node to get here I think.
> + }
> +
> + oid = of_match_node(ad9467_of_match, spi->dev.of_node);
> + if (!oid)
> + return -ENODEV;
You only ever want the data field so you can get that directly.
of_device_get_match_data
> +
> + conv = devm_adi_axi_adc_conv_register(&spi->dev, sizeof(*st));
> + if (IS_ERR(conv))
> + return PTR_ERR(conv);
> +
> + st = adi_axi_adc_conv_priv(conv);
> + st->spi = spi;
> +
> + st->clk = devm_clk_get(&spi->dev, "adc-clk");
> + 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 = (unsigned int)oid->data;
> + conv->chip_info = &ad9467_chip_info_tbl[id];
> +
> + id = ad9467_spi_read(spi, AN877_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 struct spi_driver ad9467_driver = {
> + .driver = {
> + .name = "ad9467",
> + .of_match_table = ad9467_of_match,
> + },
> + .probe = ad9467_probe,
> +};
> +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] 22+ messages in thread
* Re: [PATCH v8 2/8] include: fpga: adi-axi-common.h: add version helper macros
2020-03-07 14:26 ` Jonathan Cameron
@ 2020-03-07 19:56 ` Moritz Fischer
2020-03-09 13:58 ` Ardelean, Alexandru
0 siblings, 1 reply; 22+ messages in thread
From: Moritz Fischer @ 2020-03-07 19:56 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Alexandru Ardelean, linux-iio, linux-kernel, devicetree, robh+dt,
Moritz Fischer
On Sat, Mar 07, 2020 at 02:26:04PM +0000, Jonathan Cameron wrote:
> On Fri, 6 Mar 2020 13:00:54 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > The format for all ADI AXI IP cores is the same.
> > i.e. 'major.minor.patch'.
> >
> > This patch adds the helper macros to be re-used in ADI AXI drivers.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
>
> Again, trivial but needs a Moritz ack as it's his subsystem.
I had originally asked to not put it under include/linux/fpga, but alas,
now it's here :)
It never made much sense imho to drop it under linux/fpga just because
it's a hardware implemented in an FPGA....
Cheers,
Moritz
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 1/8] include: fpga: adi-axi-common.h: fixup whitespace tab -> space
2020-03-07 14:25 ` Jonathan Cameron
@ 2020-03-07 19:56 ` Moritz Fischer
2020-03-09 13:49 ` Ardelean, Alexandru
0 siblings, 1 reply; 22+ messages in thread
From: Moritz Fischer @ 2020-03-07 19:56 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Alexandru Ardelean, linux-iio, linux-kernel, devicetree, robh+dt,
Moritz Fischer
On Sat, Mar 07, 2020 at 02:25:21PM +0000, Jonathan Cameron wrote:
> On Fri, 6 Mar 2020 13:00:53 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > The initial version use a tab between '#define' & 'ADI_AXI_REG_VERSION'.
> > This changes it to space. The change is purely cosmetic.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
> Whilst this is trivial it still needs an ack from relevant maintainer
> for that directory. Moritz I think...
>
> Jonathan
>
> > ---
> > include/linux/fpga/adi-axi-common.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/fpga/adi-axi-common.h b/include/linux/fpga/adi-axi-common.h
> > index 7fc95d5c95bb..ebd4e07ae3d8 100644
> > --- a/include/linux/fpga/adi-axi-common.h
> > +++ b/include/linux/fpga/adi-axi-common.h
> > @@ -11,7 +11,7 @@
> > #ifndef ADI_AXI_COMMON_H_
> > #define ADI_AXI_COMMON_H_
> >
> > -#define ADI_AXI_REG_VERSION 0x0000
> > +#define ADI_AXI_REG_VERSION 0x0000
> >
> > #define ADI_AXI_PCORE_VER(major, minor, patch) \
> > (((major) << 16) | ((minor) << 8) | (patch))
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 5/8] iio: adc: adi-axi-adc: add support for AXI ADC IP core
2020-03-07 14:54 ` Jonathan Cameron
@ 2020-03-09 11:34 ` Ardelean, Alexandru
2020-03-09 15:15 ` Ardelean, Alexandru
0 siblings, 1 reply; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-03-09 11:34 UTC (permalink / raw)
To: jic23
Cc: Hennerich, Michael, devicetree, linux-kernel, linux-iio, robh+dt, lars
On Sat, 2020-03-07 at 14:54 +0000, Jonathan Cameron wrote:
> [External]
>
> On Fri, 6 Mar 2020 13:00:57 +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>
> Mostly looks fine, but a few nitpicks from rereading it.
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/iio/adc/Kconfig | 20 +
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/adi-axi-adc.c | 618 ++++++++++++++++++++++++++++
> > include/linux/iio/adc/adi-axi-adc.h | 63 +++
> > 4 files changed, 702 insertions(+)
> > create mode 100644 drivers/iio/adc/adi-axi-adc.c
> > create mode 100644 include/linux/iio/adc/adi-axi-adc.h
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index f4da821c4022..445070abf376 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -246,6 +246,26 @@ config AD799X
> > To compile this driver as a module, choose M here: the module will be
> > called ad799x.
> >
> > +config ADI_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 adi-axi-adc.
> > +
> > 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 8462455b4228..7c6594d049f9 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_ADI_AXI_ADC) += adi-axi-adc.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/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> > new file mode 100644
> > index 000000000000..17ee20015d71
> > --- /dev/null
> > +++ b/drivers/iio/adc/adi-axi-adc.c
> > @@ -0,0 +1,618 @@
> > +// 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/adi-axi-adc.h>
> > +
> > +/**
> > + * Register definitions:
> > + * https://wiki.analog.com/resources/fpga/docs/axi_adc_ip#register_map
> > + */
> > +
> > +#define LOWER5_MSK GENMASK(4, 0)
> > +#define LOWER5_SET(x) FIELD_PREP(LOWER5_MSK, x)
> > +#define LOWER5_GET(x) FIELD_GET(LOWER5_MSK, x)
>
> Whilst it may cause more repetition, I'd rather just see these used where
> relevant inline. That will be slightly easier to review.
ack
i wasn't too decided on whether these need to be defined yet
>
> > +
> > +#define LOWER8_MSK GENMASK(7, 0)
> > +#define LOWER8_SET(x) FIELD_PREP(LOWER8_MSK, x)
> > +#define LOWER8_GET(x) FIELD_GET(LOWER8_MSK, x)
> > +
> > +#define UPPER16_MSK GENMASK(31, 16)
> > +#define UPPER16_SET(x) FIELD_PREP(UPPER16_MSK, x)
> > +#define UPPER16_GET(x) FIELD_GET(UPPER16_MSK, x)
> > +
> > +#define LOWER16_MSK GENMASK(15, 0)
> > +#define LOWER16_SET(x) FIELD_PREP(LOWER16_MSK, x)
> > +#define LOWER16_GET(x) FIELD_GET(LOWER16_MSK, x)
> > +
> > +/* ADC controls */
> > +
> > +#define REG_RSTN 0x0040
>
> Usual stuff: These should be prefixed with driver relevant prefix
> maybe ADI_AXI_
my only concern [about the prefix] is that it makes the macro-names too long;
after re-looking at these reg definitions, what bothered me is that some of the
bit-field-names collide; so i went with concatenanting reg-names + reg-bitnames,
which made them look too long [so i removed the prefix]
there's also the possibility of going back to the regmap-doc and shortening
these reg-name/bitnames;
but they've been like this for a while, and i admit going to the HDL team makes
me sometimes lazy;
i'll re-add teh ADI_AXI prefix; and will see how these look
>
> > +#define REG_RSTN_CE_N BIT(2)
> > +#define REG_RSTN_MMCM_RSTN BIT(1)
> > +#define REG_RSTN_RSTN BIT(0)
> > +
> > +#define REG_CNTRL 0x0044
> > +#define REG_CNTRL_SYNC BIT(3)
> > +#define REG_CNTRL_R1_MODE BIT(2)
> > +#define REG_CNTRL_DDR_EDGESEL BIT(1)
> > +#define REG_CNTRL_PIN_MODE BIT(0)
> > +
> > +#define REG_CLK_FREQ 0x0054
> > +#define REG_CLK_RATIO 0x0058
> > +
> > +#define REG_STATUS 0x005C
> > +#define REG_STATUS_PN_ERR BIT(3)
> > +#define REG_STATUS_PN_OOS BIT(2)
> > +#define REG_STATUS_OVER_RANGE BIT(1)
> > +#define REG_STATUS_STATUS BIT(0)
> > +
> > +#define REG_SYNC_STATUS 0x0068
> > +#define REG_SYNC_STATUS_SYNC BIT(0)
> > +
> > +#define REG_DRP_CNTRL 0x0070
> > +#define REG_DRP_CNTRL_DRP_RWN BIT(28)
> > +#define REG_DRP_CNTRL_DRP_ADDRESS_MSK GENMASK(27, 16)
> > +#define REG_DRP_CNTRL_DRP_ADDRESS_SET(x) \
> > + FIELD_PREP(REG_DRP_CNTRL_DRP_ADDRESS_MSK, x)
> > +#define REG_DRP_CNTRL_DRP_ADDRESS_GET(x) \
> > + FIELD_GET(REG_DRP_CNTRL_DRP_ADDRESS_MSK, x)
> > +
> > +#define REG_DRP_STATUS 0x0074
> > +#define REG_DRP_STATUS_DRP_LOCKED BIT(17)
> > +#define REG_DRP_STATUS_DRP_STATUS BIT(16)
> > +
> > +#define REG_DRP_WDATA 0x0078
> > +#define REG_DRP_WDATA_SET LOWER16_SET
> > +#define REG_DRP_WDATA_GET LOWER16_GET
>
> As mentioned above, I'd rather see the mask here. Unless used
> an awful lot, I'd rather just see the FIELD_PREP and FIELD_GET
> used inline rather than defining a bunch of _SET/_GET only some
> of which seem to be used so far.
>
sure;
will remove all these _SET/_GET shorthands
a lot of them will be used;
and we may implement all reg-definitions at some point;
this driver is a re-write/adaptation of this one:
https://github.com/analogdevicesinc/linux/blob/master/drivers/iio/adc/cf_axi_adc_core.c
which has grown quite a lot and trying to clean it up is... ¯\_(ツ)_/¯
> > +
> > +#define REG_DRP_RDATA 0x007C
> > +#define REG_DRP_RDATA_SET LOWER16_SET
> > +#define REG_DRP_RDATA_GET LOWER16_GET
> > +
> > +#define REG_UI_STATUS 0x0088
> > +#define REG_UI_STATUS_OVF BIT(2)
> > +#define REG_UI_STATUS_UNF BIT(1)
> > +
> > +#define REG_USR_CNTRL_1 0x00A0
> > +#define REG_USR_CNTRL_1_USR_CHANMAX_MSK LOWER8_MSK
> > +#define REG_USR_CNTRL_1_USR_CHANMAX_SET LOWER8_SET
> > +#define REG_USR_CNTRL_1_USR_CHANMAX_GET LOWER8_GET
> > +
> > +#define REG_ADC_START_CODE 0x00A4
> > +#define REG_ADC_GPIO_IN 0x00B8
> > +#define REG_ADC_GPIO_OUT 0x00BC
> > +
> > +#define REG_PPS_COUNTER 0x00C0
> > +
> > +#define REG_PPS_STATUS 0x00C4
> > +#define REG_PPS_STATUS_PPS_STATUS BIT(0)
> > +
> > +/* ADC Channel controls */
> > +
> > +#define REG_CHAN_CNTRL(c) (0x0400 + (c) * 0x40)
> > +#define REG_CHAN_CNTRL_LB_OWR BIT(11)
> > +#define REG_CHAN_CNTRL_PN_SEL_OWR BIT(10)
> > +#define REG_CHAN_CNTRL_IQCOR_ENB BIT(9)
> > +#define REG_CHAN_CNTRL_DCFILT_ENB BIT(8)
> > +#define REG_CHAN_CNTRL_FORMAT_SIGNEXT BIT(6)
> > +#define REG_CHAN_CNTRL_FORMAT_TYPE BIT(5)
> > +#define REG_CHAN_CNTRL_FORMAT_ENABLE BIT(4)
> > +#define REG_CHAN_CNTRL_ADC_PN_TYPE_OWR BIT(1)
> > +#define REG_CHAN_CNTRL_ENABLE BIT(0)
> > +
> > +#define REG_CHAN_CNTRL_DEFAULTS \
> > + (REG_CHAN_CNTRL_FORMAT_SIGNEXT | REG_CHAN_CNTRL_FORMAT_ENABLE | \
> > + REG_CHAN_CNTRL_IQCOR_ENB | REG_CHAN_CNTRL_ENABLE)
> > +
> > +#define REG_CHAN_STATUS(c) (0x0404 + (c) * 0x40)
> > +#define REG_CHAN_STATUS_PN_ERR BIT(2)
> > +#define REG_CHAN_STATUS_PN_OOS BIT(1)
> > +#define REG_CHAN_STATUS_OVER_RANGE BIT(0)
> > +
> > +#define REG_CHAN_CNTRL_1(c) (0x0410 + (c) * 0x40)
> > +#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_MSK UPPER16_MSK
> > +#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_SET UPPER16_SET
> > +#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_GET UPPER16_GET
> > +#define REG_CHAN_CNTRL_1_DCFILT_COEFF_MSK LOWER16_MSK
> > +#define REG_CHAN_CNTRL_1_DCFILT_COEFF_SET LOWER16_SET
> > +#define REG_CHAN_CNTRL_1_DCFILT_COEFF_GET LOWER16_GET
> > +
> > +#define REG_CHAN_CNTRL_2(c) (0x0414 + (c) * 0x40)
> > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_MSK UPPER16_MSK
> > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_SET UPPER16_SET
> > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_GET UPPER16_GET
> > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_MSK LOWER16_MSK
> > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_SET LOWER16_SET
> > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_GET LOWER16_GET
> > +
> > +/* format is 1.1.14 (sign, integer and fractional bits) */
> > +#define IQCOR_INT_1 0x4000UL
> > +#define IQCOR_SIGN_BIT BIT(15)
> > +
> > +#define REG_CHAN_CNTRL_3(c) (0x0418 + (c) * 0x40)
> > +#define REG_CHAN_CNTRL_3_ADC_PN_SEL_MSK UPPER16_MSK
> > +#define REG_CHAN_CNTRL_3_ADC_PN_SEL_SET UPPER16_SET
> > +#define REG_CHAN_CNTRL_3_ADC_PN_SEL_GET UPPER16_GET
> > +#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_MSK LOWER16_MSK
> > +#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_SET LOWER16_SET
> > +#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_GET LOWER16_GET
> > +
> > +#define REG_CHAN_USR_CNTRL_1(c) (0x0420
> > + (c) * 0x40)
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BE BIT(25)
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SIGNED BIT(24)
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK GENMASK(
> > 23, 16)
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_SET(x) \
> > + FIELD_PREP(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK, x)
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_GET \
> > + FIELD_GET(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK, x)
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK GENMASK(15, 8)
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_SET(x) \
> > + FIELD_PREP(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK, x)
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_GET(x) \
> > + FIELD_GET(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK, x)
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_MSK LOWER8_M
> > SK
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_SET LOWER8_S
> > ET
> > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_GET LOWER8_G
> > ET
> > +
> > +#define REG_CHAN_USR_CNTRL_2(c) (0x0424 + (c) *
> > 0x40)
> > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_MSK UPPER16_MSK
> > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_SET UPPER16_SET
> > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_GET UPPER16_GET
> > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_MSK LOWER16_MSK
> > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_SET LOWER16_SET
> > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_GET LOWER16_GET
> > +
> > +/* IO Delay controls */
> > +#define __REG_DELAY_CONTROL(base, lane) ((base) + ((lane) *
> > 0x04))
> > +
> > +#define REG_DELAY_CONTROL(lane) \
> > + __REG_DELAY_CONTROL(0x0800, lane)
> > +#define REG_DELAY_CONTROL_MSK LOWER5_MSK
> > +#define REG_DELAY_CONTROL_SET LOWER5_SET
> > +#define REG_DELAY_CONTROL_GET LOWER5_GET
> > +
> > +struct adi_axi_adc_core_info {
> > + unsigned int version;
> > +};
> > +
> > +struct adi_axi_adc_state {
> > + struct mutex lock;
> > +
> > + struct adi_axi_adc_client *client;
> > + void __iomem *regs;
> > +};
> > +
> > +struct adi_axi_adc_client {
> > + struct list_head entry;
> > + struct adi_axi_adc_conv conv;
> > + struct adi_axi_adc_state *state;
> > + struct device *dev;
> > + const struct adi_axi_adc_core_info *info;
> > +};
> > +
> > +static LIST_HEAD(registered_clients);
> > +static DEFINE_MUTEX(registered_clients_lock);
> > +
> > +static struct adi_axi_adc_client *conv_to_client(struct adi_axi_adc_conv
> > *conv)
> > +{
> > + if (!conv)
> > + return NULL;
> > + return container_of(conv, struct adi_axi_adc_client, conv);
> > +}
> > +
> > +void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv)
> > +{
> > + struct adi_axi_adc_client *cl = conv_to_client(conv);
> > +
> > + if (!cl)
> > + return NULL;
> > +
> > + return (char *)cl + ALIGN(sizeof(struct adi_axi_adc_client), IIO_ALIGN);
> > +}
> > +EXPORT_SYMBOL_GPL(adi_axi_adc_conv_priv);
> > +
> > +static void adi_axi_adc_write(struct adi_axi_adc_state *st,
> > + unsigned int reg,
> > + unsigned int val)
> > +{
> > + iowrite32(val, st->regs + reg);
> > +}
> > +
> > +static unsigned int adi_axi_adc_read(struct adi_axi_adc_state *st,
> > + unsigned int reg)
> > +{
> > + return ioread32(st->regs + reg);
> > +}
> > +
> > +static int adi_axi_adc_config_dma_buffer(struct device *dev,
> > + struct iio_dev *indio_dev)
> > +{
> > + 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 adi_axi_adc_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long mask)
> > +{
> > + struct adi_axi_adc_state *st = iio_priv(indio_dev);
> > + struct adi_axi_adc_conv *conv = &st->client->conv;
> > +
> > + if (!conv->read_raw)
> > + return -EOPNOTSUPP;
> > +
> > + return conv->read_raw(conv, chan, val, val2, mask);
> > +}
> > +
> > +static int adi_axi_adc_write_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int val, int val2, long mask)
> > +{
> > + struct adi_axi_adc_state *st = iio_priv(indio_dev);
> > + struct adi_axi_adc_conv *conv = &st->client->conv;
> > +
> > + if (!conv->write_raw)
> > + return -EOPNOTSUPP;
> > +
> > + return conv->write_raw(conv, chan, val, val2, mask);
> > +}
> > +
> > +static int adi_axi_adc_update_scan_mode(struct iio_dev *indio_dev,
> > + const unsigned long *scan_mask)
> > +{
> > + struct adi_axi_adc_state *st = iio_priv(indio_dev);
> > + struct adi_axi_adc_conv *conv = &st->client->conv;
> > + unsigned int i, ctrl;
> > +
> > + for (i = 0; i < conv->chip_info->num_channels; i++) {
> > + ctrl = adi_axi_adc_read(st, REG_CHAN_CNTRL(i));
> > +
> > + if (test_bit(i, scan_mask))
> > + ctrl |= REG_CHAN_CNTRL_ENABLE;
> > + else
> > + ctrl &= ~REG_CHAN_CNTRL_ENABLE;
> > +
> > + adi_axi_adc_write(st, REG_CHAN_CNTRL(i), ctrl);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct adi_axi_adc_conv *adi_axi_adc_conv_register(struct device
> > *dev,
> > + int sizeof_priv)
> > +{
> > + struct adi_axi_adc_client *cl;
> > + size_t alloc_size;
> > +
> > + alloc_size = sizeof(struct adi_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(®istered_clients_lock);
> > +
> > + get_device(dev);
> > + cl->dev = dev;
> > +
> > + list_add_tail(&cl->entry, ®istered_clients);
> > +
> > + mutex_unlock(®istered_clients_lock);
> > +
> > + return &cl->conv;
> > +}
> > +
> > +static void adi_axi_adc_conv_unregister(struct adi_axi_adc_conv *conv)
> > +{
> > + struct adi_axi_adc_client *cl = conv_to_client(conv);
> > +
> > + if (!cl)
> > + return;
> > +
> > + mutex_lock(®istered_clients_lock);
> > +
> > + put_device(cl->dev);
> > + list_del(&cl->entry);
> This isn't reverse of register. Why?
> Should be list_del then put_device.
>
> > + kfree(cl);
>
> To reverse the register precisely the kfree(cl) should be outside
> the lock...
yeah; i was a bit careless/relaxed when writing this bit of code;
will update;
>
> > +
> > + mutex_unlock(®istered_clients_lock);
> > +}
> > +
> > +static void devm_adi_axi_adc_conv_release(struct device *dev, void *res)
> > +{
> > + adi_axi_adc_conv_unregister(*(struct adi_axi_adc_conv **)res);
> > +}
> > +
> > +struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,
> > + int sizeof_priv)
> > +{
> > + struct adi_axi_adc_conv **ptr, *conv;
> > +
> > + ptr = devres_alloc(devm_adi_axi_adc_conv_release, sizeof(*ptr),
> > + GFP_KERNEL);
> > + if (!ptr)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + conv = adi_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_adi_axi_adc_conv_register);
> > +
> > +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 adi_axi_adc_state *st = iio_priv(indio_dev);
> > + struct adi_axi_adc_conv *conv = &st->client->conv;
> > + size_t len = 0;
> > + int i;
> > +
> > + if (!conv->chip_info->num_scales) {
> > + buf[0] = '\n';
> > + return 1;
>
> I'd prefer to see this done using the core available handling.
> That way drivers that don't provide this would just not register
> it in the first place. It will be a characteristic of the channels.
ack
will take a look closer here;
i am a bit vague [at this point] on how to do it; but i'll figure it out
>
> > + }
> > +
> > + 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 *adi_axi_adc_attributes[] = {
> > + &iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
> > + NULL,
> > +};
> > +
> > +static const struct attribute_group adi_axi_adc_attribute_group = {
> > + .attrs = adi_axi_adc_attributes,
> > +};
> > +
> > +static const struct iio_info adi_axi_adc_info = {
> > + .read_raw = &adi_axi_adc_read_raw,
> > + .write_raw = &adi_axi_adc_write_raw,
> > + .attrs = &adi_axi_adc_attribute_group,
> > + .update_scan_mode = &adi_axi_adc_update_scan_mode,
> > +};
> > +
> > +static const struct adi_axi_adc_core_info adi_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 adi_axi_adc_of_match[] = {
> > + { .compatible = "adi,axi-adc-10.0.a", .data = &adi_axi_adc_10_0_a_info
> > },
> > + { /* end of list */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, adi_axi_adc_of_match);
> > +
> > +struct adi_axi_adc_client *adi_axi_adc_attach_client(struct device *dev)
> > +{
> > + const struct of_device_id *id;
> > + struct adi_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(adi_axi_adc_of_match, dev->of_node);
> > + if (!id)
> > + return ERR_PTR(-ENODEV);
> > +
> > + cln = of_parse_phandle(dev->of_node, "adi,adc-dev", 0);
> > + if (!cln) {
> > + dev_err(dev, "No 'adi,adc-dev' node defined\n");
> > + return ERR_PTR(-ENODEV);
> > + }
> > +
> > + mutex_lock(®istered_clients_lock);
> > +
> > + list_for_each_entry(cl, ®istered_clients, entry) {
> > + if (!cl->dev)
> > + continue;
> > + if (cl->dev->of_node == cln) {
> > + if (!try_module_get(dev->driver->owner)) {
> > + mutex_unlock(®istered_clients_lock);
> > + return ERR_PTR(-ENODEV);
> > + }
> > + get_device(dev);
> > + cl->info = id->data;
> > + mutex_unlock(®istered_clients_lock);
> > + return cl;
> > + }
> > + }
> > +
> > + mutex_unlock(®istered_clients_lock);
> > +
> > + return ERR_PTR(-EPROBE_DEFER);
> > +}
> > +
> > +static int adi_axi_adc_setup_channels(struct device *dev,
> > + struct adi_axi_adc_state *st)
> > +{
> > + struct adi_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 = REG_CHAN_CNTRL_2_IQCOR_COEFF_2_SET(IQCOR_INT_1);
> > + else
> > + val = REG_CHAN_CNTRL_2_IQCOR_COEFF_1_SET(IQCOR_INT_1);
> > + adi_axi_adc_write(st, REG_CHAN_CNTRL_2(i), val);
> > +
> > + adi_axi_adc_write(st, REG_CHAN_CNTRL(i),
> > + REG_CHAN_CNTRL_DEFAULTS);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void axi_adc_reset(struct adi_axi_adc_state *st)
> > +{
> > + adi_axi_adc_write(st, REG_RSTN, 0);
> > + mdelay(10);
> > + adi_axi_adc_write(st, REG_RSTN, REG_RSTN_MMCM_RSTN);
> > + mdelay(10);
> > + adi_axi_adc_write(st, REG_RSTN,
> > + REG_RSTN_RSTN | REG_RSTN_MMCM_RSTN);
> > +}
> > +
> > +static void adi_axi_adc_cleanup(void *data)
> > +{
> > + struct adi_axi_adc_client *cl = data;
> > +
> > + put_device(cl->dev);
> > + module_put(cl->dev->driver->owner);
> > +}
> > +
> > +static int adi_axi_adc_probe(struct platform_device *pdev)
> > +{
> > + struct adi_axi_adc_conv *conv;
> > + struct iio_dev *indio_dev;
> > + struct adi_axi_adc_client *cl;
> > + struct adi_axi_adc_state *st;
> > + unsigned int ver;
> > + int ret;
> > +
> > + cl = adi_axi_adc_attach_client(&pdev->dev);
> > + if (IS_ERR(cl))
> > + return PTR_ERR(cl);
> > +
> > + ret = devm_add_action_or_reset(&pdev->dev, adi_axi_adc_cleanup, cl);
> > + if (ret)
> > + return ret;
> > +
> > + 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);
> > +
> > + st->regs = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(st->regs))
> > + return PTR_ERR(st->regs);
> > +
> > + conv = &st->client->conv;
> > +
> > + axi_adc_reset(st);
> > +
> > + ver = adi_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 = &adi_axi_adc_info;
> > + indio_dev->dev.parent = &pdev->dev;
> > + indio_dev->name = dev_name(&pdev->dev);
>
> What does this result in? Given this is supposed to be a part number
> I kind of expected this to be hardcoded to "adi-axi-adc" or similar.
ack;
"adi-axi-adc" it is
>
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > + indio_dev->num_channels = conv->chip_info->num_channels;
> > + indio_dev->channels = conv->chip_info->channels;
> > +
> > + ret = adi_axi_adc_config_dma_buffer(&pdev->dev, indio_dev);
> > + if (ret)
> > + return ret;
> > +
> > + ret = adi_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 adi_axi_adc_driver = {
> > + .driver = {
> > + .name = KBUILD_MODNAME,
> > + .of_match_table = adi_axi_adc_of_match,
> > + },
> > + .probe = adi_axi_adc_probe,
> > +};
> > +module_platform_driver(adi_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/adi-axi-adc.h
> > b/include/linux/iio/adc/adi-axi-adc.h
> > new file mode 100644
> > index 000000000000..57974944b30b
> > --- /dev/null
> > +++ b/include/linux/iio/adc/adi-axi-adc.h
> > @@ -0,0 +1,63 @@
> > +/* 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 __ADI_AXI_ADC_H__
> > +#define __ADI_AXI_ADC_H__
> > +
> > +struct device;
> > +struct iio_chan_spec;
> > +
> > +/**
> > + * struct adi_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 adi_axi_adc_chip_info {
> > + const char *name;
> > + unsigned int id;
> > +
> > + const struct iio_chan_spec *channels;
> > + unsigned int num_channels;
> > +
> > + const unsigned int (*scale_table)[2];
> > + int num_scales;
> > +
> > + unsigned long max_rate;
> > +};
> > +
> > +/**
> > + * struct adi_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
>
> Run the kernel doc script over this and it would have moaned...
>
> No reg_access.
ack;
i'll need to start adapting that script to some CI
it's on my todo-list
>
> > + * @read_raw IIO read_raw hook for the client ADC
> > + * @write_raw IIO write_raw hook for the client ADC
> > + */
> > +struct adi_axi_adc_conv {
> > + const struct adi_axi_adc_chip_info *chip_info;
> > +
> > + int (*preenable_setup)(struct adi_axi_adc_conv *conv);
> > + int (*reg_access)(struct adi_axi_adc_conv *conv, unsigned int reg,
> > + unsigned int writeval, unsigned int *readval);
> > + int (*read_raw)(struct adi_axi_adc_conv *conv,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long mask);
> > + int (*write_raw)(struct adi_axi_adc_conv *conv,
> > + struct iio_chan_spec const *chan,
> > + int val, int val2, long mask);
> > +};
> > +
> > +struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device *dev,
> > + int sizeof_priv);
> > +
> > +void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv);
> > +
> > +#endif
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 7/8] iio: adc: ad9467: add support AD9467 ADC
2020-03-07 15:05 ` Jonathan Cameron
@ 2020-03-09 11:51 ` Ardelean, Alexandru
0 siblings, 0 replies; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-03-09 11:51 UTC (permalink / raw)
To: jic23
Cc: Hennerich, Michael, devicetree, linux-kernel, linux-iio, robh+dt, lars
On Sat, 2020-03-07 at 15:05 +0000, Jonathan Cameron wrote:
> [External]
>
> On Fri, 6 Mar 2020 13:00:59 +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 but otherwise looks good to me..
>
> > ---
> > drivers/iio/adc/Kconfig | 15 ++
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/ad9467.c | 432 +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 448 insertions(+)
> > create mode 100644 drivers/iio/adc/ad9467.c
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 445070abf376..a0796510f9d4 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.
> >
> ...
> > +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);
>
> Why not split buf into send part and receive? Might make it slightly
> more readable for no actual cost..
sure
>
> > +
> > + if (ret < 0)
> > + return ret;
> > +
> > + return buf[2];
> > +}
> ...
>
> > +static int ad9467_write_raw(struct adi_axi_adc_conv *conv,
> > + struct iio_chan_spec const *chan,
> > + int val, int val2, long mask)
> > +{
> > + const struct adi_axi_adc_chip_info *info = conv->chip_info;
> > + struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
> > + unsigned long r_clk;
> > +
> > + 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)
>
> This is a very 'odd' test. Why?
left-over from the original driver; and it slipped when i adapted from that;
no idea why it was added; and git history is not helping either;
will drop;
>
> > + 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;
> > + }
> > +
> > + return clk_set_rate(st->clk, r_clk);
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> ...
> > +static int ad9467_probe(struct spi_device *spi)
> > +{
> > + const struct of_device_id *oid;
> > + struct adi_axi_adc_conv *conv;
> > + struct ad9467_state *st;
> > + unsigned int id;
> > + int ret;
> > +
> > + if (!spi->dev.of_node) {
> > + dev_err(&spi->dev, "DT node is null\n");
> > + return -ENODEV;
>
> Silly question for you. Can this happen? We can only probe this
> if it is in DT and hence there must be a node to get here I think.
good point;
will drop;
looks like something i added inertially
>
> > + }
> > +
> > + oid = of_match_node(ad9467_of_match, spi->dev.of_node);
> > + if (!oid)
> > + return -ENODEV;
>
> You only ever want the data field so you can get that directly.
> of_device_get_match_data
ack
>
> > +
> > + conv = devm_adi_axi_adc_conv_register(&spi->dev, sizeof(*st));
> > + if (IS_ERR(conv))
> > + return PTR_ERR(conv);
> > +
> > + st = adi_axi_adc_conv_priv(conv);
> > + st->spi = spi;
> > +
> > + st->clk = devm_clk_get(&spi->dev, "adc-clk");
> > + 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 = (unsigned int)oid->data;
> > + conv->chip_info = &ad9467_chip_info_tbl[id];
> > +
> > + id = ad9467_spi_read(spi, AN877_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 struct spi_driver ad9467_driver = {
> > + .driver = {
> > + .name = "ad9467",
> > + .of_match_table = ad9467_of_match,
> > + },
> > + .probe = ad9467_probe,
> > +};
> > +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] 22+ messages in thread
* Re: [PATCH v8 1/8] include: fpga: adi-axi-common.h: fixup whitespace tab -> space
2020-03-07 19:56 ` Moritz Fischer
@ 2020-03-09 13:49 ` Ardelean, Alexandru
0 siblings, 0 replies; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-03-09 13:49 UTC (permalink / raw)
To: mdf, jic23; +Cc: devicetree, linux-kernel, linux-iio, robh+dt
On Sat, 2020-03-07 at 11:56 -0800, Moritz Fischer wrote:
> On Sat, Mar 07, 2020 at 02:25:21PM +0000, Jonathan Cameron wrote:
> > On Fri, 6 Mar 2020 13:00:53 +0200
> > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> >
> > > The initial version use a tab between '#define' & 'ADI_AXI_REG_VERSION'.
> > > This changes it to space. The change is purely cosmetic.
> > >
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> Acked-by: Moritz Fischer <mdf@kernel.org>
I'm still a bit vague on who maintains what.
The initial version of this header did not get submitted to Moritz's attention
Apologies for that.
It came to exist as a result of upstreaming parts of the ADI AXI DMAC driver.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d27ac2e02bf256d4e824e7c1e1e1afa2b96cefcc
The initial complaint was that some magic numbers were used and then I added
this header, which is a smaller version of the one we have in the ADI tree.
Thanks
Alex
>
> > Whilst this is trivial it still needs an ack from relevant maintainer
> > for that directory. Moritz I think...
> >
> > Jonathan
> >
> > > ---
> > > include/linux/fpga/adi-axi-common.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/fpga/adi-axi-common.h b/include/linux/fpga/adi-
> > > axi-common.h
> > > index 7fc95d5c95bb..ebd4e07ae3d8 100644
> > > --- a/include/linux/fpga/adi-axi-common.h
> > > +++ b/include/linux/fpga/adi-axi-common.h
> > > @@ -11,7 +11,7 @@
> > > #ifndef ADI_AXI_COMMON_H_
> > > #define ADI_AXI_COMMON_H_
> > >
> > > -#define ADI_AXI_REG_VERSION 0x0000
> > > +#define ADI_AXI_REG_VERSION 0x0000
> > >
> > > #define ADI_AXI_PCORE_VER(major, minor, patch) \
> > > (((major) << 16) | ((minor) << 8) | (patch))
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 2/8] include: fpga: adi-axi-common.h: add version helper macros
2020-03-07 19:56 ` Moritz Fischer
@ 2020-03-09 13:58 ` Ardelean, Alexandru
0 siblings, 0 replies; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-03-09 13:58 UTC (permalink / raw)
To: mdf, jic23; +Cc: devicetree, linux-kernel, linux-iio, robh+dt
On Sat, 2020-03-07 at 11:56 -0800, Moritz Fischer wrote:
> [External]
>
> On Sat, Mar 07, 2020 at 02:26:04PM +0000, Jonathan Cameron wrote:
> > On Fri, 6 Mar 2020 13:00:54 +0200
> > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> >
> > > The format for all ADI AXI IP cores is the same.
> > > i.e. 'major.minor.patch'.
> > >
> > > This patch adds the helper macros to be re-used in ADI AXI drivers.
> > >
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> Acked-by: Moritz Fischer <mdf@kernel.org>
>
> > Again, trivial but needs a Moritz ack as it's his subsystem.
>
> I had originally asked to not put it under include/linux/fpga, but alas,
> now it's here :)
>
> It never made much sense imho to drop it under linux/fpga just because
> it's a hardware implemented in an FPGA....
We can always move it.
I don't remember about any discussion on this matter.
Or maybe I wasn't included.
Or maybe I have some severe case of amnesia or carelessness for omitting
threads. I am terrible at following threads.
Apologies for anything on my part.
If you propose another location, I can spin-up a patch on it.
These reg-definitions are common to all ADI HDL regs.
Maybe more may come up as stuff gets upstreamed.
The full-blown/internal version we have is:
https://github.com/analogdevicesinc/linux/blob/master/include/linux/fpga/adi-axi-common.h
It tries to define some things that are common between Intel, Xilinx and ADI IP
cores across [these and hopefully other] FPGA boards.
I'm not saying it's doing a good job of that at the moment.
¯\_(ツ)_/¯
Thanks
Alex
>
> Cheers,
> Moritz
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 5/8] iio: adc: adi-axi-adc: add support for AXI ADC IP core
2020-03-09 11:34 ` Ardelean, Alexandru
@ 2020-03-09 15:15 ` Ardelean, Alexandru
2020-03-15 9:23 ` Jonathan Cameron
0 siblings, 1 reply; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-03-09 15:15 UTC (permalink / raw)
To: jic23
Cc: Hennerich, Michael, devicetree, linux-kernel, linux-iio, robh+dt, lars
On Mon, 2020-03-09 at 11:34 +0000, Ardelean, Alexandru wrote:
> [External]
>
> On Sat, 2020-03-07 at 14:54 +0000, Jonathan Cameron wrote:
> > [External]
> >
> > On Fri, 6 Mar 2020 13:00:57 +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>
> > Mostly looks fine, but a few nitpicks from rereading it.
> >
> > Thanks,
> >
> > Jonathan
> >
> > > ---
> > > drivers/iio/adc/Kconfig | 20 +
> > > drivers/iio/adc/Makefile | 1 +
> > > drivers/iio/adc/adi-axi-adc.c | 618 ++++++++++++++++++++++++++++
> > > include/linux/iio/adc/adi-axi-adc.h | 63 +++
> > > 4 files changed, 702 insertions(+)
> > > create mode 100644 drivers/iio/adc/adi-axi-adc.c
> > > create mode 100644 include/linux/iio/adc/adi-axi-adc.h
> > >
> > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > > index f4da821c4022..445070abf376 100644
> > > --- a/drivers/iio/adc/Kconfig
> > > +++ b/drivers/iio/adc/Kconfig
> > > @@ -246,6 +246,26 @@ config AD799X
> > > To compile this driver as a module, choose M here: the module will be
> > > called ad799x.
> > >
> > > +config ADI_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 adi-axi-adc.
> > > +
> > > 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 8462455b4228..7c6594d049f9 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_ADI_AXI_ADC) += adi-axi-adc.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/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> > > new file mode 100644
> > > index 000000000000..17ee20015d71
> > > --- /dev/null
> > > +++ b/drivers/iio/adc/adi-axi-adc.c
> > > @@ -0,0 +1,618 @@
> > > +// 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/adi-axi-adc.h>
> > > +
> > > +/**
> > > + * Register definitions:
> > > + * https://wiki.analog.com/resources/fpga/docs/axi_adc_ip#register_map
> > > + */
> > > +
> > > +#define LOWER5_MSK GENMASK(4, 0)
> > > +#define LOWER5_SET(x) FIELD_PREP(LOWER5_MSK, x)
> > > +#define LOWER5_GET(x) FIELD_GET(LOWER5_MSK, x)
> >
> > Whilst it may cause more repetition, I'd rather just see these used where
> > relevant inline. That will be slightly easier to review.
>
> ack
> i wasn't too decided on whether these need to be defined yet
>
> > > +
> > > +#define LOWER8_MSK GENMASK(7, 0)
> > > +#define LOWER8_SET(x) FIELD_PREP(LOWER8_MSK, x)
> > > +#define LOWER8_GET(x) FIELD_GET(LOWER8_MSK, x)
> > > +
> > > +#define UPPER16_MSK GENMASK(31, 16)
> > > +#define UPPER16_SET(x) FIELD_PREP(UPPER16_MSK, x)
> > > +#define UPPER16_GET(x) FIELD_GET(UPPER16_MSK, x)
> > > +
> > > +#define LOWER16_MSK GENMASK(15, 0)
> > > +#define LOWER16_SET(x) FIELD_PREP(LOWER16_MSK, x)
> > > +#define LOWER16_GET(x) FIELD_GET(LOWER16_MSK, x)
> > > +
> > > +/* ADC controls */
> > > +
> > > +#define REG_RSTN 0x0040
> >
> > Usual stuff: These should be prefixed with driver relevant prefix
> > maybe ADI_AXI_
>
> my only concern [about the prefix] is that it makes the macro-names too long;
> after re-looking at these reg definitions, what bothered me is that some of
> the
> bit-field-names collide; so i went with concatenanting reg-names + reg-
> bitnames,
> which made them look too long [so i removed the prefix]
>
> there's also the possibility of going back to the regmap-doc and shortening
> these reg-name/bitnames;
> but they've been like this for a while, and i admit going to the HDL team
> makes
> me sometimes lazy;
>
> i'll re-add teh ADI_AXI prefix; and will see how these look
i talked to HDL
so, we'll have a round of renaming these [in the docs];
but now, i'm wondering if it's ok to drop the regs that are [currently] unused
and add them when functionality gets later-added;
in the meantime, the names can be re-worked/shortened/prettify-ed;
we'll also need to re-do an inventory of the current HDL IP cores and see how
the regmaps hold-up/match against the docs;
>
> > > +#define REG_RSTN_CE_N BIT(2)
> > > +#define REG_RSTN_MMCM_RSTN BIT(1)
> > > +#define REG_RSTN_RSTN BIT(0)
> > > +
> > > +#define REG_CNTRL 0x0044
> > > +#define REG_CNTRL_SYNC BIT(3)
> > > +#define REG_CNTRL_R1_MODE BIT(2)
> > > +#define REG_CNTRL_DDR_EDGESEL BIT(1)
> > > +#define REG_CNTRL_PIN_MODE BIT(0)
> > > +
> > > +#define REG_CLK_FREQ 0x0054
> > > +#define REG_CLK_RATIO 0x0058
> > > +
> > > +#define REG_STATUS 0x005C
> > > +#define REG_STATUS_PN_ERR BIT(3)
> > > +#define REG_STATUS_PN_OOS BIT(2)
> > > +#define REG_STATUS_OVER_RANGE BIT(1)
> > > +#define REG_STATUS_STATUS BIT(0)
> > > +
> > > +#define REG_SYNC_STATUS 0x0068
> > > +#define REG_SYNC_STATUS_SYNC BIT(0)
> > > +
> > > +#define REG_DRP_CNTRL 0x0070
> > > +#define REG_DRP_CNTRL_DRP_RWN BIT(28)
> > > +#define REG_DRP_CNTRL_DRP_ADDRESS_MSK GENMASK(27, 16)
> > > +#define REG_DRP_CNTRL_DRP_ADDRESS_SET(x) \
> > > + FIELD_PREP(REG_DRP_CNTRL_DRP_ADDRESS_MSK, x)
> > > +#define REG_DRP_CNTRL_DRP_ADDRESS_GET(x) \
> > > + FIELD_GET(REG_DRP_CNTRL_DRP_ADDRESS_MSK, x)
> > > +
> > > +#define REG_DRP_STATUS 0x0074
> > > +#define REG_DRP_STATUS_DRP_LOCKED BIT(17)
> > > +#define REG_DRP_STATUS_DRP_STATUS BIT(16)
> > > +
> > > +#define REG_DRP_WDATA 0x0078
> > > +#define REG_DRP_WDATA_SET LOWER16_SET
> > > +#define REG_DRP_WDATA_GET LOWER16_GET
> >
> > As mentioned above, I'd rather see the mask here. Unless used
> > an awful lot, I'd rather just see the FIELD_PREP and FIELD_GET
> > used inline rather than defining a bunch of _SET/_GET only some
> > of which seem to be used so far.
> >
>
> sure;
> will remove all these _SET/_GET shorthands
>
> a lot of them will be used;
> and we may implement all reg-definitions at some point;
>
> this driver is a re-write/adaptation of this one:
> https://github.com/analogdevicesinc/linux/blob/master/drivers/iio/adc/cf_axi_adc_core.c
>
> which has grown quite a lot and trying to clean it up is... ¯\_(ツ)_/¯
>
>
> > > +
> > > +#define REG_DRP_RDATA 0x007C
> > > +#define REG_DRP_RDATA_SET LOWER16_SET
> > > +#define REG_DRP_RDATA_GET LOWER16_GET
> > > +
> > > +#define REG_UI_STATUS 0x0088
> > > +#define REG_UI_STATUS_OVF BIT(2)
> > > +#define REG_UI_STATUS_UNF BIT(1)
> > > +
> > > +#define REG_USR_CNTRL_1 0x00A0
> > > +#define REG_USR_CNTRL_1_USR_CHANMAX_MSK LOWER8_MSK
> > > +#define REG_USR_CNTRL_1_USR_CHANMAX_SET LOWER8_SET
> > > +#define REG_USR_CNTRL_1_USR_CHANMAX_GET LOWER8_GET
> > > +
> > > +#define REG_ADC_START_CODE 0x00A4
> > > +#define REG_ADC_GPIO_IN 0x00B8
> > > +#define REG_ADC_GPIO_OUT 0x00BC
> > > +
> > > +#define REG_PPS_COUNTER 0x00C0
> > > +
> > > +#define REG_PPS_STATUS 0x00C4
> > > +#define REG_PPS_STATUS_PPS_STATUS BIT(0)
> > > +
> > > +/* ADC Channel controls */
> > > +
> > > +#define REG_CHAN_CNTRL(c) (0x0400 + (c) * 0x40)
> > > +#define REG_CHAN_CNTRL_LB_OWR BIT(11)
> > > +#define REG_CHAN_CNTRL_PN_SEL_OWR BIT(10)
> > > +#define REG_CHAN_CNTRL_IQCOR_ENB BIT(9)
> > > +#define REG_CHAN_CNTRL_DCFILT_ENB BIT(8)
> > > +#define REG_CHAN_CNTRL_FORMAT_SIGNEXT BIT(6)
> > > +#define REG_CHAN_CNTRL_FORMAT_TYPE BIT(5)
> > > +#define REG_CHAN_CNTRL_FORMAT_ENABLE BIT(4)
> > > +#define REG_CHAN_CNTRL_ADC_PN_TYPE_OWR BIT(1)
> > > +#define REG_CHAN_CNTRL_ENABLE BIT(0)
> > > +
> > > +#define REG_CHAN_CNTRL_DEFAULTS \
> > > + (REG_CHAN_CNTRL_FORMAT_SIGNEXT | REG_CHAN_CNTRL_FORMAT_ENABLE | \
> > > + REG_CHAN_CNTRL_IQCOR_ENB | REG_CHAN_CNTRL_ENABLE)
> > > +
> > > +#define REG_CHAN_STATUS(c) (0x0404 + (c) * 0x40)
> > > +#define REG_CHAN_STATUS_PN_ERR BIT(2)
> > > +#define REG_CHAN_STATUS_PN_OOS BIT(1)
> > > +#define REG_CHAN_STATUS_OVER_RANGE BIT(0)
> > > +
> > > +#define REG_CHAN_CNTRL_1(c) (0x0410 + (c) * 0x40)
> > > +#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_MSK UPPER16_MSK
> > > +#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_SET UPPER16_SET
> > > +#define REG_CHAN_CNTRL_1_DCFILT_OFFSET_GET UPPER16_GET
> > > +#define REG_CHAN_CNTRL_1_DCFILT_COEFF_MSK LOWER16_MSK
> > > +#define REG_CHAN_CNTRL_1_DCFILT_COEFF_SET LOWER16_SET
> > > +#define REG_CHAN_CNTRL_1_DCFILT_COEFF_GET LOWER16_GET
> > > +
> > > +#define REG_CHAN_CNTRL_2(c) (0x0414 + (c) * 0x40)
> > > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_MSK UPPER16_MSK
> > > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_SET UPPER16_SET
> > > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_1_GET UPPER16_GET
> > > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_MSK LOWER16_MSK
> > > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_SET LOWER16_SET
> > > +#define REG_CHAN_CNTRL_2_IQCOR_COEFF_2_GET LOWER16_GET
> > > +
> > > +/* format is 1.1.14 (sign, integer and fractional bits) */
> > > +#define IQCOR_INT_1 0x4000UL
> > > +#define IQCOR_SIGN_BIT BIT(15)
> > > +
> > > +#define REG_CHAN_CNTRL_3(c) (0x0418 + (c) * 0x40)
> > > +#define REG_CHAN_CNTRL_3_ADC_PN_SEL_MSK UPPER16_MSK
> > > +#define REG_CHAN_CNTRL_3_ADC_PN_SEL_SET UPPER16_SET
> > > +#define REG_CHAN_CNTRL_3_ADC_PN_SEL_GET UPPER16_GET
> > > +#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_MSK LOWER16_MSK
> > > +#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_SET LOWER16_SET
> > > +#define REG_CHAN_CNTRL_3_ADC_DATA_SEL_GET LOWER16_GET
> > > +
> > > +#define REG_CHAN_USR_CNTRL_1(c) (0x0420
> > > + (c) * 0x40)
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BE BIT(25)
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SIGNED BIT(24)
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK GENMASK(
> > > 23, 16)
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_SET(x) \
> > > + FIELD_PREP(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK, x)
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_GET \
> > > + FIELD_GET(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_SHIFT_MSK, x)
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK GENMASK(
> > > 15, 8)
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_SET(x) \
> > > + FIELD_PREP(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK, x)
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_GET(x) \
> > > + FIELD_GET(REG_CHAN_USR_CNTRL_1_USR_DATATYPE_TOTAL_BITS_MSK, x)
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_MSK LOWER8_M
> > > SK
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_SET LOWER8_S
> > > ET
> > > +#define REG_CHAN_USR_CNTRL_1_USR_DATATYPE_BITS_GET LOWER8_G
> > > ET
> > > +
> > > +#define REG_CHAN_USR_CNTRL_2(c) (0x0424 + (c) *
> > > 0x40)
> > > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_MSK UPPER16_MSK
> > > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_SET UPPER16_SET
> > > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_M_GET UPPER16_GET
> > > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_MSK LOWER16_MSK
> > > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_SET LOWER16_SET
> > > +#define REG_CHAN_USR_CNTRL_2_USR_DECIMATION_N_GET LOWER16_GET
> > > +
> > > +/* IO Delay controls */
> > > +#define __REG_DELAY_CONTROL(base, lane) ((base) + ((lane) *
> > > 0x04))
> > > +
> > > +#define REG_DELAY_CONTROL(lane) \
> > > + __REG_DELAY_CONTROL(0x0800, lane)
> > > +#define REG_DELAY_CONTROL_MSK LOWER5_MSK
> > > +#define REG_DELAY_CONTROL_SET LOWER5_SET
> > > +#define REG_DELAY_CONTROL_GET LOWER5_GET
> > > +
> > > +struct adi_axi_adc_core_info {
> > > + unsigned int version;
> > > +};
> > > +
> > > +struct adi_axi_adc_state {
> > > + struct mutex lock;
> > > +
> > > + struct adi_axi_adc_client *client;
> > > + void __iomem *regs;
> > > +};
> > > +
> > > +struct adi_axi_adc_client {
> > > + struct list_head entry;
> > > + struct adi_axi_adc_conv conv;
> > > + struct adi_axi_adc_state *state;
> > > + struct device *dev;
> > > + const struct adi_axi_adc_core_info *info;
> > > +};
> > > +
> > > +static LIST_HEAD(registered_clients);
> > > +static DEFINE_MUTEX(registered_clients_lock);
> > > +
> > > +static struct adi_axi_adc_client *conv_to_client(struct adi_axi_adc_conv
> > > *conv)
> > > +{
> > > + if (!conv)
> > > + return NULL;
> > > + return container_of(conv, struct adi_axi_adc_client, conv);
> > > +}
> > > +
> > > +void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv)
> > > +{
> > > + struct adi_axi_adc_client *cl = conv_to_client(conv);
> > > +
> > > + if (!cl)
> > > + return NULL;
> > > +
> > > + return (char *)cl + ALIGN(sizeof(struct adi_axi_adc_client), IIO_ALIGN);
> > > +}
> > > +EXPORT_SYMBOL_GPL(adi_axi_adc_conv_priv);
> > > +
> > > +static void adi_axi_adc_write(struct adi_axi_adc_state *st,
> > > + unsigned int reg,
> > > + unsigned int val)
> > > +{
> > > + iowrite32(val, st->regs + reg);
> > > +}
> > > +
> > > +static unsigned int adi_axi_adc_read(struct adi_axi_adc_state *st,
> > > + unsigned int reg)
> > > +{
> > > + return ioread32(st->regs + reg);
> > > +}
> > > +
> > > +static int adi_axi_adc_config_dma_buffer(struct device *dev,
> > > + struct iio_dev *indio_dev)
> > > +{
> > > + 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 adi_axi_adc_read_raw(struct iio_dev *indio_dev,
> > > + struct iio_chan_spec const *chan,
> > > + int *val, int *val2, long mask)
> > > +{
> > > + struct adi_axi_adc_state *st = iio_priv(indio_dev);
> > > + struct adi_axi_adc_conv *conv = &st->client->conv;
> > > +
> > > + if (!conv->read_raw)
> > > + return -EOPNOTSUPP;
> > > +
> > > + return conv->read_raw(conv, chan, val, val2, mask);
> > > +}
> > > +
> > > +static int adi_axi_adc_write_raw(struct iio_dev *indio_dev,
> > > + struct iio_chan_spec const *chan,
> > > + int val, int val2, long mask)
> > > +{
> > > + struct adi_axi_adc_state *st = iio_priv(indio_dev);
> > > + struct adi_axi_adc_conv *conv = &st->client->conv;
> > > +
> > > + if (!conv->write_raw)
> > > + return -EOPNOTSUPP;
> > > +
> > > + return conv->write_raw(conv, chan, val, val2, mask);
> > > +}
> > > +
> > > +static int adi_axi_adc_update_scan_mode(struct iio_dev *indio_dev,
> > > + const unsigned long *scan_mask)
> > > +{
> > > + struct adi_axi_adc_state *st = iio_priv(indio_dev);
> > > + struct adi_axi_adc_conv *conv = &st->client->conv;
> > > + unsigned int i, ctrl;
> > > +
> > > + for (i = 0; i < conv->chip_info->num_channels; i++) {
> > > + ctrl = adi_axi_adc_read(st, REG_CHAN_CNTRL(i));
> > > +
> > > + if (test_bit(i, scan_mask))
> > > + ctrl |= REG_CHAN_CNTRL_ENABLE;
> > > + else
> > > + ctrl &= ~REG_CHAN_CNTRL_ENABLE;
> > > +
> > > + adi_axi_adc_write(st, REG_CHAN_CNTRL(i), ctrl);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static struct adi_axi_adc_conv *adi_axi_adc_conv_register(struct device
> > > *dev,
> > > + int sizeof_priv)
> > > +{
> > > + struct adi_axi_adc_client *cl;
> > > + size_t alloc_size;
> > > +
> > > + alloc_size = sizeof(struct adi_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(®istered_clients_lock);
> > > +
> > > + get_device(dev);
> > > + cl->dev = dev;
> > > +
> > > + list_add_tail(&cl->entry, ®istered_clients);
> > > +
> > > + mutex_unlock(®istered_clients_lock);
> > > +
> > > + return &cl->conv;
> > > +}
> > > +
> > > +static void adi_axi_adc_conv_unregister(struct adi_axi_adc_conv *conv)
> > > +{
> > > + struct adi_axi_adc_client *cl = conv_to_client(conv);
> > > +
> > > + if (!cl)
> > > + return;
> > > +
> > > + mutex_lock(®istered_clients_lock);
> > > +
> > > + put_device(cl->dev);
> > > + list_del(&cl->entry);
> > This isn't reverse of register. Why?
> > Should be list_del then put_device.
> >
> > > + kfree(cl);
> >
> > To reverse the register precisely the kfree(cl) should be outside
> > the lock...
>
> yeah; i was a bit careless/relaxed when writing this bit of code;
> will update;
>
> > > +
> > > + mutex_unlock(®istered_clients_lock);
> > > +}
> > > +
> > > +static void devm_adi_axi_adc_conv_release(struct device *dev, void *res)
> > > +{
> > > + adi_axi_adc_conv_unregister(*(struct adi_axi_adc_conv **)res);
> > > +}
> > > +
> > > +struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device
> > > *dev,
> > > + int sizeof_priv)
> > > +{
> > > + struct adi_axi_adc_conv **ptr, *conv;
> > > +
> > > + ptr = devres_alloc(devm_adi_axi_adc_conv_release, sizeof(*ptr),
> > > + GFP_KERNEL);
> > > + if (!ptr)
> > > + return ERR_PTR(-ENOMEM);
> > > +
> > > + conv = adi_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_adi_axi_adc_conv_register);
> > > +
> > > +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 adi_axi_adc_state *st = iio_priv(indio_dev);
> > > + struct adi_axi_adc_conv *conv = &st->client->conv;
> > > + size_t len = 0;
> > > + int i;
> > > +
> > > + if (!conv->chip_info->num_scales) {
> > > + buf[0] = '\n';
> > > + return 1;
> >
> > I'd prefer to see this done using the core available handling.
> > That way drivers that don't provide this would just not register
> > it in the first place. It will be a characteristic of the channels.
>
> ack
> will take a look closer here;
> i am a bit vague [at this point] on how to do it; but i'll figure it out
>
> > > + }
> > > +
> > > + 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 *adi_axi_adc_attributes[] = {
> > > + &iio_dev_attr_in_voltage_scale_available.dev_attr.attr,
> > > + NULL,
> > > +};
> > > +
> > > +static const struct attribute_group adi_axi_adc_attribute_group = {
> > > + .attrs = adi_axi_adc_attributes,
> > > +};
> > > +
> > > +static const struct iio_info adi_axi_adc_info = {
> > > + .read_raw = &adi_axi_adc_read_raw,
> > > + .write_raw = &adi_axi_adc_write_raw,
> > > + .attrs = &adi_axi_adc_attribute_group,
> > > + .update_scan_mode = &adi_axi_adc_update_scan_mode,
> > > +};
> > > +
> > > +static const struct adi_axi_adc_core_info adi_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 adi_axi_adc_of_match[] = {
> > > + { .compatible = "adi,axi-adc-10.0.a", .data = &adi_axi_adc_10_0_a_info
> > > },
> > > + { /* end of list */ },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, adi_axi_adc_of_match);
> > > +
> > > +struct adi_axi_adc_client *adi_axi_adc_attach_client(struct device *dev)
> > > +{
> > > + const struct of_device_id *id;
> > > + struct adi_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(adi_axi_adc_of_match, dev->of_node);
> > > + if (!id)
> > > + return ERR_PTR(-ENODEV);
> > > +
> > > + cln = of_parse_phandle(dev->of_node, "adi,adc-dev", 0);
> > > + if (!cln) {
> > > + dev_err(dev, "No 'adi,adc-dev' node defined\n");
> > > + return ERR_PTR(-ENODEV);
> > > + }
> > > +
> > > + mutex_lock(®istered_clients_lock);
> > > +
> > > + list_for_each_entry(cl, ®istered_clients, entry) {
> > > + if (!cl->dev)
> > > + continue;
> > > + if (cl->dev->of_node == cln) {
> > > + if (!try_module_get(dev->driver->owner)) {
> > > + mutex_unlock(®istered_clients_lock);
> > > + return ERR_PTR(-ENODEV);
> > > + }
> > > + get_device(dev);
> > > + cl->info = id->data;
> > > + mutex_unlock(®istered_clients_lock);
> > > + return cl;
> > > + }
> > > + }
> > > +
> > > + mutex_unlock(®istered_clients_lock);
> > > +
> > > + return ERR_PTR(-EPROBE_DEFER);
> > > +}
> > > +
> > > +static int adi_axi_adc_setup_channels(struct device *dev,
> > > + struct adi_axi_adc_state *st)
> > > +{
> > > + struct adi_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 = REG_CHAN_CNTRL_2_IQCOR_COEFF_2_SET(IQCOR_INT_1);
> > > + else
> > > + val = REG_CHAN_CNTRL_2_IQCOR_COEFF_1_SET(IQCOR_INT_1);
> > > + adi_axi_adc_write(st, REG_CHAN_CNTRL_2(i), val);
> > > +
> > > + adi_axi_adc_write(st, REG_CHAN_CNTRL(i),
> > > + REG_CHAN_CNTRL_DEFAULTS);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void axi_adc_reset(struct adi_axi_adc_state *st)
> > > +{
> > > + adi_axi_adc_write(st, REG_RSTN, 0);
> > > + mdelay(10);
> > > + adi_axi_adc_write(st, REG_RSTN, REG_RSTN_MMCM_RSTN);
> > > + mdelay(10);
> > > + adi_axi_adc_write(st, REG_RSTN,
> > > + REG_RSTN_RSTN | REG_RSTN_MMCM_RSTN);
> > > +}
> > > +
> > > +static void adi_axi_adc_cleanup(void *data)
> > > +{
> > > + struct adi_axi_adc_client *cl = data;
> > > +
> > > + put_device(cl->dev);
> > > + module_put(cl->dev->driver->owner);
> > > +}
> > > +
> > > +static int adi_axi_adc_probe(struct platform_device *pdev)
> > > +{
> > > + struct adi_axi_adc_conv *conv;
> > > + struct iio_dev *indio_dev;
> > > + struct adi_axi_adc_client *cl;
> > > + struct adi_axi_adc_state *st;
> > > + unsigned int ver;
> > > + int ret;
> > > +
> > > + cl = adi_axi_adc_attach_client(&pdev->dev);
> > > + if (IS_ERR(cl))
> > > + return PTR_ERR(cl);
> > > +
> > > + ret = devm_add_action_or_reset(&pdev->dev, adi_axi_adc_cleanup, cl);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + 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);
> > > +
> > > + st->regs = devm_platform_ioremap_resource(pdev, 0);
> > > + if (IS_ERR(st->regs))
> > > + return PTR_ERR(st->regs);
> > > +
> > > + conv = &st->client->conv;
> > > +
> > > + axi_adc_reset(st);
> > > +
> > > + ver = adi_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 = &adi_axi_adc_info;
> > > + indio_dev->dev.parent = &pdev->dev;
> > > + indio_dev->name = dev_name(&pdev->dev);
> >
> > What does this result in? Given this is supposed to be a part number
> > I kind of expected this to be hardcoded to "adi-axi-adc" or similar.
>
> ack;
> "adi-axi-adc" it is
>
>
> > > + indio_dev->modes = INDIO_DIRECT_MODE;
> > > + indio_dev->num_channels = conv->chip_info->num_channels;
> > > + indio_dev->channels = conv->chip_info->channels;
> > > +
> > > + ret = adi_axi_adc_config_dma_buffer(&pdev->dev, indio_dev);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = adi_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 adi_axi_adc_driver = {
> > > + .driver = {
> > > + .name = KBUILD_MODNAME,
> > > + .of_match_table = adi_axi_adc_of_match,
> > > + },
> > > + .probe = adi_axi_adc_probe,
> > > +};
> > > +module_platform_driver(adi_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/adi-axi-adc.h
> > > b/include/linux/iio/adc/adi-axi-adc.h
> > > new file mode 100644
> > > index 000000000000..57974944b30b
> > > --- /dev/null
> > > +++ b/include/linux/iio/adc/adi-axi-adc.h
> > > @@ -0,0 +1,63 @@
> > > +/* 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 __ADI_AXI_ADC_H__
> > > +#define __ADI_AXI_ADC_H__
> > > +
> > > +struct device;
> > > +struct iio_chan_spec;
> > > +
> > > +/**
> > > + * struct adi_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 adi_axi_adc_chip_info {
> > > + const char *name;
> > > + unsigned int id;
> > > +
> > > + const struct iio_chan_spec *channels;
> > > + unsigned int num_channels;
> > > +
> > > + const unsigned int (*scale_table)[2];
> > > + int num_scales;
> > > +
> > > + unsigned long max_rate;
> > > +};
> > > +
> > > +/**
> > > + * struct adi_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
> >
> > Run the kernel doc script over this and it would have moaned...
> >
> > No reg_access.
>
> ack;
> i'll need to start adapting that script to some CI
> it's on my todo-list
>
> > > + * @read_raw IIO read_raw hook for the client ADC
> > > + * @write_raw IIO write_raw hook for the client ADC
> > > + */
> > > +struct adi_axi_adc_conv {
> > > + const struct adi_axi_adc_chip_info *chip_info;
> > > +
> > > + int (*preenable_setup)(struct adi_axi_adc_conv *conv);
> > > + int (*reg_access)(struct adi_axi_adc_conv *conv, unsigned int reg,
> > > + unsigned int writeval, unsigned int *readval);
> > > + int (*read_raw)(struct adi_axi_adc_conv *conv,
> > > + struct iio_chan_spec const *chan,
> > > + int *val, int *val2, long mask);
> > > + int (*write_raw)(struct adi_axi_adc_conv *conv,
> > > + struct iio_chan_spec const *chan,
> > > + int val, int val2, long mask);
> > > +};
> > > +
> > > +struct adi_axi_adc_conv *devm_adi_axi_adc_conv_register(struct device
> > > *dev,
> > > + int sizeof_priv);
> > > +
> > > +void *adi_axi_adc_conv_priv(struct adi_axi_adc_conv *conv);
> > > +
> > > +#endif
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v8 5/8] iio: adc: adi-axi-adc: add support for AXI ADC IP core
2020-03-09 15:15 ` Ardelean, Alexandru
@ 2020-03-15 9:23 ` Jonathan Cameron
0 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2020-03-15 9:23 UTC (permalink / raw)
To: Ardelean, Alexandru
Cc: Hennerich, Michael, devicetree, linux-kernel, linux-iio, robh+dt, lars
...
> > > > +#define REG_RSTN 0x0040
> > >
> > > Usual stuff: These should be prefixed with driver relevant prefix
> > > maybe ADI_AXI_
> >
> > my only concern [about the prefix] is that it makes the macro-names too long;
> > after re-looking at these reg definitions, what bothered me is that some of
> > the
> > bit-field-names collide; so i went with concatenanting reg-names + reg-
> > bitnames,
> > which made them look too long [so i removed the prefix]
> >
> > there's also the possibility of going back to the regmap-doc and shortening
> > these reg-name/bitnames;
> > but they've been like this for a while, and i admit going to the HDL team
> > makes
> > me sometimes lazy;
> >
> > i'll re-add teh ADI_AXI prefix; and will see how these look
>
> i talked to HDL
> so, we'll have a round of renaming these [in the docs];
>
> but now, i'm wondering if it's ok to drop the regs that are [currently] unused
> and add them when functionality gets later-added;
Absolutely. Its in theory better to always do that, but sometimes it's
just easier to copy type the whole datasheet register map into a driver
so we tend to let it go.
> in the meantime, the names can be re-worked/shortened/prettify-ed;
> we'll also need to re-do an inventory of the current HDL IP cores and see how
> the regmaps hold-up/match against the docs;
I'd not worry too much on shortening these. A bit of ugly code never
really hurt anyone if it's just a long define.
Of course, if it makes sense anyway, always good to tidy up naming.
Jonathan
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2020-03-15 9:23 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-06 11:00 [PATCH v8 0/8] iio: adi-axi-adc,ad9647: Add support for AD9467 ADC Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 1/8] include: fpga: adi-axi-common.h: fixup whitespace tab -> space Alexandru Ardelean
2020-03-07 14:25 ` Jonathan Cameron
2020-03-07 19:56 ` Moritz Fischer
2020-03-09 13:49 ` Ardelean, Alexandru
2020-03-06 11:00 ` [PATCH v8 2/8] include: fpga: adi-axi-common.h: add version helper macros Alexandru Ardelean
2020-03-07 14:26 ` Jonathan Cameron
2020-03-07 19:56 ` Moritz Fischer
2020-03-09 13:58 ` Ardelean, Alexandru
2020-03-06 11:00 ` [PATCH v8 3/8] iio: buffer-dmaengine: use %zu specifier for sprintf(align) Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 4/8] iio: buffer-dmaengine: add dev-managed calls for buffer alloc Alexandru Ardelean
2020-03-06 11:00 ` [PATCH v8 5/8] iio: adc: adi-axi-adc: add support for AXI ADC IP core Alexandru Ardelean
2020-03-07 14:54 ` Jonathan Cameron
2020-03-09 11:34 ` Ardelean, Alexandru
2020-03-09 15:15 ` Ardelean, Alexandru
2020-03-15 9:23 ` Jonathan Cameron
2020-03-06 11:00 ` [PATCH v8 6/8] dt-bindings: iio: adc: add bindings doc for AXI ADC driver Alexandru Ardelean
2020-03-07 14:55 ` Jonathan Cameron
2020-03-06 11:00 ` [PATCH v8 7/8] iio: adc: ad9467: add support AD9467 ADC Alexandru Ardelean
2020-03-07 15:05 ` Jonathan Cameron
2020-03-09 11:51 ` Ardelean, Alexandru
2020-03-06 11:01 ` [PATCH v8 8/8] dt-bindings: iio: adc: add bindings doc for " Alexandru Ardelean
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).