public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] iio: adc: xilinx-xadc: Add I2C interface support for System Management Wizard
@ 2026-03-23  7:45 Sai Krishna Potthuri
  2026-03-23  7:45 ` [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files Sai Krishna Potthuri
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Sai Krishna Potthuri @ 2026-03-23  7:45 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sa, Andy Shevchenko,
	Michal Simek, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-arm-kernel, linux-kernel,
	saikrishna12468, git, Sai Krishna Potthuri

The existing driver only supported AXI memory-mapped access to the System
Management Wizard IP. This series extends the driver to support I2C-based
access, which is particularly useful for System Controller usecases.

Key Changes:
- Split the xilinx-xadc-core.c file into two files(xilinx-xadc-core.c and
  xilinx-xadc-platform.c)
- Add required helper functions and callbacks
- Add channel configuration via callback mechanism
- New I2C driver for UltraScale+ System Management Wizard for basic
  voltage and temperature monitoring
- Converted text binding to YAML schema format

Note: We are working on x86 platform support where fixed channel
configuration is used(no DT support). The .setup_channels() function
pointer introduced in patch 2/4 enables different channel configuration
approaches for various platforms.

Changes in v2:
-> 1/4 - Split the xilinx-xadc-core.c file into two files
         xilinx-xadc-core.c and xilinx-xadc-platform.c(comments from Andy).
-> 2/4 - Referred as .setup_channels instead of setup_channels.
-> 3/4 - Created separate functions for i2c read and write.
       - Created separate file for i2c interface handling.
-> 4/4(comments from Krzysztof)
       - Removed $defs and use it directly under xlnx,channels.
       - Documented the error information in the commit message due to
         vendor prefix properties.
       - Kept only one example as there is not much differences.


Sai Krishna Potthuri (4):
  iio: adc: xilinx-xadc: Split driver into core and platform files
  iio: adc: xilinx-xadc: Add .setup_channels() to struct xadc_ops
  iio: adc: xilinx-xadc: Add I2C interface support
  dt-bindings: iio: adc: xlnx,axi-xadc: convert to DT schema

 .../bindings/iio/adc/xilinx-xadc.txt          | 141 ---
 .../bindings/iio/adc/xlnx,axi-xadc.yaml       | 154 ++++
 drivers/iio/adc/Kconfig                       |  23 +-
 drivers/iio/adc/Makefile                      |   6 +-
 drivers/iio/adc/xilinx-xadc-core.c            | 814 +++---------------
 drivers/iio/adc/xilinx-xadc-i2c.c             | 215 +++++
 drivers/iio/adc/xilinx-xadc-platform.c        | 668 ++++++++++++++
 drivers/iio/adc/xilinx-xadc.h                 |  33 +
 8 files changed, 1193 insertions(+), 861 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
 create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,axi-xadc.yaml
 create mode 100644 drivers/iio/adc/xilinx-xadc-i2c.c
 create mode 100644 drivers/iio/adc/xilinx-xadc-platform.c

-- 
2.25.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files
  2026-03-23  7:45 [PATCH v2 0/4] iio: adc: xilinx-xadc: Add I2C interface support for System Management Wizard Sai Krishna Potthuri
@ 2026-03-23  7:45 ` Sai Krishna Potthuri
  2026-03-23 10:47   ` Andy Shevchenko
                     ` (2 more replies)
  2026-03-23  7:45 ` [PATCH v2 2/4] iio: adc: xilinx-xadc: Add .setup_channels() to struct xadc_ops Sai Krishna Potthuri
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 14+ messages in thread
From: Sai Krishna Potthuri @ 2026-03-23  7:45 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sa, Andy Shevchenko,
	Michal Simek, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-arm-kernel, linux-kernel,
	saikrishna12468, git, Sai Krishna Potthuri

Split the xilinx-xadc-core.c into separate core and platform specific
files to prepare for I2C interface support.

xilinx-xadc-core.c is reorganized as follows:
xilinx-xadc-core.c:
  - Platform-independent IIO/ADC operations
  - Channel definitions and management
  - Buffer and trigger management
  - Device tree parsing

xilinx-xadc-platform.c:
  - ZYNQ platform (FIFO-based) register access and interrupt handling
  - AXI platform (memory-mapped) register access and interrupt handling
  - Platform-specific setup and configuration
  - Platform device probe function

Update Kconfig to introduce XILINX_XADC_CORE as a helper module selected
by XILINX_XADC and update Makefile to build the split modules:
  - xilinx-xadc-common.o (core + events)
  - xilinx-xadc-platform.o (platform-specific)

Reorganized the code and No behavioral changes.

Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
---
 drivers/iio/adc/Kconfig                |   8 +-
 drivers/iio/adc/Makefile               |   5 +-
 drivers/iio/adc/xilinx-xadc-core.c     | 790 +++----------------------
 drivers/iio/adc/xilinx-xadc-platform.c | 665 +++++++++++++++++++++
 drivers/iio/adc/xilinx-xadc.h          |  30 +
 5 files changed, 779 insertions(+), 719 deletions(-)
 create mode 100644 drivers/iio/adc/xilinx-xadc-platform.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index ea3ba1397392..a4a7556f4016 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1744,11 +1744,15 @@ config VIPERBOARD_ADC
 	  To compile this driver as a module, choose M here: the module will be
 	  called viperboard_adc.
 
+config XILINX_XADC_CORE
+	tristate
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+
 config XILINX_XADC
 	tristate "Xilinx XADC driver"
 	depends on HAS_IOMEM
-	select IIO_BUFFER
-	select IIO_TRIGGERED_BUFFER
+	select XILINX_XADC_CORE
 	help
 	  Say yes here to have support for the Xilinx 7 Series XADC or
 	  UltraScale/UltraScale+ System Management Wizard.
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 09ae6edb2650..1b05176f0098 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -154,5 +154,6 @@ obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
 obj-$(CONFIG_VF610_ADC) += vf610_adc.o
 obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
 obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o
-xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o
-obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o
+xilinx-xadc-common-y := xilinx-xadc-core.o xilinx-xadc-events.o
+obj-$(CONFIG_XILINX_XADC_CORE) += xilinx-xadc-common.o
+obj-$(CONFIG_XILINX_XADC) += xilinx-xadc-platform.o
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index e257c1b94a5f..268e46e5349c 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -11,588 +11,37 @@
  *  - AXI XADC interface: Xilinx PG019
  */
 
-#include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
-#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/overflow.h>
-#include <linux/platform_device.h>
 #include <linux/property.h>
 #include <linux/slab.h>
-#include <linux/sysfs.h>
 
 #include <linux/iio/buffer.h>
 #include <linux/iio/events.h>
 #include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
 #include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 
 #include "xilinx-xadc.h"
 
-static const unsigned int XADC_ZYNQ_UNMASK_TIMEOUT = 500;
-
-/* ZYNQ register definitions */
-#define XADC_ZYNQ_REG_CFG	0x00
-#define XADC_ZYNQ_REG_INTSTS	0x04
-#define XADC_ZYNQ_REG_INTMSK	0x08
-#define XADC_ZYNQ_REG_STATUS	0x0c
-#define XADC_ZYNQ_REG_CFIFO	0x10
-#define XADC_ZYNQ_REG_DFIFO	0x14
-#define XADC_ZYNQ_REG_CTL		0x18
-
-#define XADC_ZYNQ_CFG_ENABLE		BIT(31)
-#define XADC_ZYNQ_CFG_CFIFOTH_MASK	(0xf << 20)
-#define XADC_ZYNQ_CFG_CFIFOTH_OFFSET	20
-#define XADC_ZYNQ_CFG_DFIFOTH_MASK	(0xf << 16)
-#define XADC_ZYNQ_CFG_DFIFOTH_OFFSET	16
-#define XADC_ZYNQ_CFG_WEDGE		BIT(13)
-#define XADC_ZYNQ_CFG_REDGE		BIT(12)
-#define XADC_ZYNQ_CFG_TCKRATE_MASK	(0x3 << 8)
-#define XADC_ZYNQ_CFG_TCKRATE_DIV2	(0x0 << 8)
-#define XADC_ZYNQ_CFG_TCKRATE_DIV4	(0x1 << 8)
-#define XADC_ZYNQ_CFG_TCKRATE_DIV8	(0x2 << 8)
-#define XADC_ZYNQ_CFG_TCKRATE_DIV16	(0x3 << 8)
-#define XADC_ZYNQ_CFG_IGAP_MASK		0x1f
-#define XADC_ZYNQ_CFG_IGAP(x)		(x)
-
-#define XADC_ZYNQ_INT_CFIFO_LTH		BIT(9)
-#define XADC_ZYNQ_INT_DFIFO_GTH		BIT(8)
-#define XADC_ZYNQ_INT_ALARM_MASK	0xff
-#define XADC_ZYNQ_INT_ALARM_OFFSET	0
-
-#define XADC_ZYNQ_STATUS_CFIFO_LVL_MASK	(0xf << 16)
-#define XADC_ZYNQ_STATUS_CFIFO_LVL_OFFSET	16
-#define XADC_ZYNQ_STATUS_DFIFO_LVL_MASK	(0xf << 12)
-#define XADC_ZYNQ_STATUS_DFIFO_LVL_OFFSET	12
-#define XADC_ZYNQ_STATUS_CFIFOF		BIT(11)
-#define XADC_ZYNQ_STATUS_CFIFOE		BIT(10)
-#define XADC_ZYNQ_STATUS_DFIFOF		BIT(9)
-#define XADC_ZYNQ_STATUS_DFIFOE		BIT(8)
-#define XADC_ZYNQ_STATUS_OT		BIT(7)
-#define XADC_ZYNQ_STATUS_ALM(x)		BIT(x)
-
-#define XADC_ZYNQ_CTL_RESET		BIT(4)
-
-#define XADC_ZYNQ_CMD_NOP		0x00
-#define XADC_ZYNQ_CMD_READ		0x01
-#define XADC_ZYNQ_CMD_WRITE		0x02
-
-#define XADC_ZYNQ_CMD(cmd, addr, data) (((cmd) << 26) | ((addr) << 16) | (data))
-
-/* AXI register definitions */
-#define XADC_AXI_REG_RESET		0x00
-#define XADC_AXI_REG_STATUS		0x04
-#define XADC_AXI_REG_ALARM_STATUS	0x08
-#define XADC_AXI_REG_CONVST		0x0c
-#define XADC_AXI_REG_XADC_RESET		0x10
-#define XADC_AXI_REG_GIER		0x5c
-#define XADC_AXI_REG_IPISR		0x60
-#define XADC_AXI_REG_IPIER		0x68
-
-/* 7 Series */
-#define XADC_7S_AXI_ADC_REG_OFFSET	0x200
-
-/* UltraScale */
-#define XADC_US_AXI_ADC_REG_OFFSET	0x400
-
-#define XADC_AXI_RESET_MAGIC		0xa
-#define XADC_AXI_GIER_ENABLE		BIT(31)
-
-#define XADC_AXI_INT_EOS		BIT(4)
-#define XADC_AXI_INT_ALARM_MASK		0x3c0f
-
-#define XADC_FLAGS_BUFFERED BIT(0)
-#define XADC_FLAGS_IRQ_OPTIONAL BIT(1)
-
-/*
- * The XADC hardware supports a samplerate of up to 1MSPS. Unfortunately it does
- * not have a hardware FIFO. Which means an interrupt is generated for each
- * conversion sequence. At 1MSPS sample rate the CPU in ZYNQ7000 is completely
- * overloaded by the interrupts that it soft-lockups. For this reason the driver
- * limits the maximum samplerate 150kSPS. At this rate the CPU is fairly busy,
- * but still responsive.
- */
-#define XADC_MAX_SAMPLERATE 150000
-
-static void xadc_write_reg(struct xadc *xadc, unsigned int reg,
-	uint32_t val)
+void xadc_write_reg(struct xadc *xadc, unsigned int reg, uint32_t val)
 {
 	writel(val, xadc->base + reg);
 }
+EXPORT_SYMBOL_GPL(xadc_write_reg);
 
-static void xadc_read_reg(struct xadc *xadc, unsigned int reg,
-	uint32_t *val)
+void xadc_read_reg(struct xadc *xadc, unsigned int reg, uint32_t *val)
 {
 	*val = readl(xadc->base + reg);
 }
+EXPORT_SYMBOL_GPL(xadc_read_reg);
 
-/*
- * The ZYNQ interface uses two asynchronous FIFOs for communication with the
- * XADC. Reads and writes to the XADC register are performed by submitting a
- * request to the command FIFO (CFIFO), once the request has been completed the
- * result can be read from the data FIFO (DFIFO). The method currently used in
- * this driver is to submit the request for a read/write operation, then go to
- * sleep and wait for an interrupt that signals that a response is available in
- * the data FIFO.
- */
-
-static void xadc_zynq_write_fifo(struct xadc *xadc, uint32_t *cmd,
-	unsigned int n)
-{
-	unsigned int i;
-
-	for (i = 0; i < n; i++)
-		xadc_write_reg(xadc, XADC_ZYNQ_REG_CFIFO, cmd[i]);
-}
-
-static void xadc_zynq_drain_fifo(struct xadc *xadc)
-{
-	uint32_t status, tmp;
-
-	xadc_read_reg(xadc, XADC_ZYNQ_REG_STATUS, &status);
-
-	while (!(status & XADC_ZYNQ_STATUS_DFIFOE)) {
-		xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &tmp);
-		xadc_read_reg(xadc, XADC_ZYNQ_REG_STATUS, &status);
-	}
-}
-
-static void xadc_zynq_update_intmsk(struct xadc *xadc, unsigned int mask,
-	unsigned int val)
-{
-	xadc->zynq_intmask &= ~mask;
-	xadc->zynq_intmask |= val;
-
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTMSK,
-		xadc->zynq_intmask | xadc->zynq_masked_alarm);
-}
-
-static int xadc_zynq_write_adc_reg(struct xadc *xadc, unsigned int reg,
-	uint16_t val)
-{
-	uint32_t cmd[1];
-	uint32_t tmp;
-	int ret;
-
-	spin_lock_irq(&xadc->lock);
-	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH,
-			XADC_ZYNQ_INT_DFIFO_GTH);
-
-	reinit_completion(&xadc->completion);
-
-	cmd[0] = XADC_ZYNQ_CMD(XADC_ZYNQ_CMD_WRITE, reg, val);
-	xadc_zynq_write_fifo(xadc, cmd, ARRAY_SIZE(cmd));
-	xadc_read_reg(xadc, XADC_ZYNQ_REG_CFG, &tmp);
-	tmp &= ~XADC_ZYNQ_CFG_DFIFOTH_MASK;
-	tmp |= 0 << XADC_ZYNQ_CFG_DFIFOTH_OFFSET;
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_CFG, tmp);
-
-	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH, 0);
-	spin_unlock_irq(&xadc->lock);
-
-	ret = wait_for_completion_interruptible_timeout(&xadc->completion, HZ);
-	if (ret == 0)
-		ret = -EIO;
-	else
-		ret = 0;
-
-	xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &tmp);
-
-	return ret;
-}
-
-static int xadc_zynq_read_adc_reg(struct xadc *xadc, unsigned int reg,
-	uint16_t *val)
-{
-	uint32_t cmd[2];
-	uint32_t resp, tmp;
-	int ret;
-
-	cmd[0] = XADC_ZYNQ_CMD(XADC_ZYNQ_CMD_READ, reg, 0);
-	cmd[1] = XADC_ZYNQ_CMD(XADC_ZYNQ_CMD_NOP, 0, 0);
-
-	spin_lock_irq(&xadc->lock);
-	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH,
-			XADC_ZYNQ_INT_DFIFO_GTH);
-	xadc_zynq_drain_fifo(xadc);
-	reinit_completion(&xadc->completion);
-
-	xadc_zynq_write_fifo(xadc, cmd, ARRAY_SIZE(cmd));
-	xadc_read_reg(xadc, XADC_ZYNQ_REG_CFG, &tmp);
-	tmp &= ~XADC_ZYNQ_CFG_DFIFOTH_MASK;
-	tmp |= 1 << XADC_ZYNQ_CFG_DFIFOTH_OFFSET;
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_CFG, tmp);
-
-	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH, 0);
-	spin_unlock_irq(&xadc->lock);
-	ret = wait_for_completion_interruptible_timeout(&xadc->completion, HZ);
-	if (ret == 0)
-		ret = -EIO;
-	if (ret < 0)
-		return ret;
-
-	xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &resp);
-	xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &resp);
-
-	*val = resp & 0xffff;
-
-	return 0;
-}
-
-static unsigned int xadc_zynq_transform_alarm(unsigned int alarm)
-{
-	return ((alarm & 0x80) >> 4) |
-		((alarm & 0x78) << 1) |
-		(alarm & 0x07);
-}
-
-/*
- * The ZYNQ threshold interrupts are level sensitive. Since we can't make the
- * threshold condition go way from within the interrupt handler, this means as
- * soon as a threshold condition is present we would enter the interrupt handler
- * again and again. To work around this we mask all active thresholds interrupts
- * in the interrupt handler and start a timer. In this timer we poll the
- * interrupt status and only if the interrupt is inactive we unmask it again.
- */
-static void xadc_zynq_unmask_worker(struct work_struct *work)
-{
-	struct xadc *xadc = container_of(work, struct xadc, zynq_unmask_work.work);
-	unsigned int misc_sts, unmask;
-
-	xadc_read_reg(xadc, XADC_ZYNQ_REG_STATUS, &misc_sts);
-
-	misc_sts &= XADC_ZYNQ_INT_ALARM_MASK;
-
-	spin_lock_irq(&xadc->lock);
-
-	/* Clear those bits which are not active anymore */
-	unmask = (xadc->zynq_masked_alarm ^ misc_sts) & xadc->zynq_masked_alarm;
-	xadc->zynq_masked_alarm &= misc_sts;
-
-	/* Also clear those which are masked out anyway */
-	xadc->zynq_masked_alarm &= ~xadc->zynq_intmask;
-
-	/* Clear the interrupts before we unmask them */
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTSTS, unmask);
-
-	xadc_zynq_update_intmsk(xadc, 0, 0);
-
-	spin_unlock_irq(&xadc->lock);
-
-	/* if still pending some alarm re-trigger the timer */
-	if (xadc->zynq_masked_alarm) {
-		schedule_delayed_work(&xadc->zynq_unmask_work,
-				msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
-	}
-
-}
-
-static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
-{
-	struct iio_dev *indio_dev = devid;
-	struct xadc *xadc = iio_priv(indio_dev);
-	uint32_t status;
-
-	xadc_read_reg(xadc, XADC_ZYNQ_REG_INTSTS, &status);
-
-	status &= ~(xadc->zynq_intmask | xadc->zynq_masked_alarm);
-
-	if (!status)
-		return IRQ_NONE;
-
-	spin_lock(&xadc->lock);
-
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTSTS, status);
-
-	if (status & XADC_ZYNQ_INT_DFIFO_GTH) {
-		xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH,
-			XADC_ZYNQ_INT_DFIFO_GTH);
-		complete(&xadc->completion);
-	}
-
-	status &= XADC_ZYNQ_INT_ALARM_MASK;
-	if (status) {
-		xadc->zynq_masked_alarm |= status;
-		/*
-		 * mask the current event interrupt,
-		 * unmask it when the interrupt is no more active.
-		 */
-		xadc_zynq_update_intmsk(xadc, 0, 0);
-
-		xadc_handle_events(indio_dev,
-				xadc_zynq_transform_alarm(status));
-
-		/* unmask the required interrupts in timer. */
-		schedule_delayed_work(&xadc->zynq_unmask_work,
-				msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
-	}
-	spin_unlock(&xadc->lock);
-
-	return IRQ_HANDLED;
-}
-
-#define XADC_ZYNQ_TCK_RATE_MAX 50000000
-#define XADC_ZYNQ_IGAP_DEFAULT 20
-#define XADC_ZYNQ_PCAP_RATE_MAX 200000000
-
-static int xadc_zynq_setup(struct platform_device *pdev,
-	struct iio_dev *indio_dev, int irq)
-{
-	struct xadc *xadc = iio_priv(indio_dev);
-	unsigned long pcap_rate;
-	unsigned int tck_div;
-	unsigned int div;
-	unsigned int igap;
-	unsigned int tck_rate;
-	int ret;
-
-	/* TODO: Figure out how to make igap and tck_rate configurable */
-	igap = XADC_ZYNQ_IGAP_DEFAULT;
-	tck_rate = XADC_ZYNQ_TCK_RATE_MAX;
-
-	xadc->zynq_intmask = ~0;
-
-	pcap_rate = clk_get_rate(xadc->clk);
-	if (!pcap_rate)
-		return -EINVAL;
-
-	if (pcap_rate > XADC_ZYNQ_PCAP_RATE_MAX) {
-		ret = clk_set_rate(xadc->clk,
-				   (unsigned long)XADC_ZYNQ_PCAP_RATE_MAX);
-		if (ret)
-			return ret;
-	}
-
-	if (tck_rate > pcap_rate / 2) {
-		div = 2;
-	} else {
-		div = pcap_rate / tck_rate;
-		if (pcap_rate / div > XADC_ZYNQ_TCK_RATE_MAX)
-			div++;
-	}
-
-	if (div <= 3)
-		tck_div = XADC_ZYNQ_CFG_TCKRATE_DIV2;
-	else if (div <= 7)
-		tck_div = XADC_ZYNQ_CFG_TCKRATE_DIV4;
-	else if (div <= 15)
-		tck_div = XADC_ZYNQ_CFG_TCKRATE_DIV8;
-	else
-		tck_div = XADC_ZYNQ_CFG_TCKRATE_DIV16;
-
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_CTL, XADC_ZYNQ_CTL_RESET);
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_CTL, 0);
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTSTS, ~0);
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTMSK, xadc->zynq_intmask);
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_CFG, XADC_ZYNQ_CFG_ENABLE |
-			XADC_ZYNQ_CFG_REDGE | XADC_ZYNQ_CFG_WEDGE |
-			tck_div | XADC_ZYNQ_CFG_IGAP(igap));
-
-	if (pcap_rate > XADC_ZYNQ_PCAP_RATE_MAX) {
-		ret = clk_set_rate(xadc->clk, pcap_rate);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
-static unsigned long xadc_zynq_get_dclk_rate(struct xadc *xadc)
-{
-	unsigned int div;
-	uint32_t val;
-
-	xadc_read_reg(xadc, XADC_ZYNQ_REG_CFG, &val);
-
-	switch (val & XADC_ZYNQ_CFG_TCKRATE_MASK) {
-	case XADC_ZYNQ_CFG_TCKRATE_DIV4:
-		div = 4;
-		break;
-	case XADC_ZYNQ_CFG_TCKRATE_DIV8:
-		div = 8;
-		break;
-	case XADC_ZYNQ_CFG_TCKRATE_DIV16:
-		div = 16;
-		break;
-	default:
-		div = 2;
-		break;
-	}
-
-	return clk_get_rate(xadc->clk) / div;
-}
-
-static void xadc_zynq_update_alarm(struct xadc *xadc, unsigned int alarm)
-{
-	unsigned long flags;
-	uint32_t status;
-
-	/* Move OT to bit 7 */
-	alarm = ((alarm & 0x08) << 4) | ((alarm & 0xf0) >> 1) | (alarm & 0x07);
-
-	spin_lock_irqsave(&xadc->lock, flags);
-
-	/* Clear previous interrupts if any. */
-	xadc_read_reg(xadc, XADC_ZYNQ_REG_INTSTS, &status);
-	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTSTS, status & alarm);
-
-	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_ALARM_MASK,
-		~alarm & XADC_ZYNQ_INT_ALARM_MASK);
-
-	spin_unlock_irqrestore(&xadc->lock, flags);
-}
-
-static const struct xadc_ops xadc_zynq_ops = {
-	.read = xadc_zynq_read_adc_reg,
-	.write = xadc_zynq_write_adc_reg,
-	.setup = xadc_zynq_setup,
-	.get_dclk_rate = xadc_zynq_get_dclk_rate,
-	.interrupt_handler = xadc_zynq_interrupt_handler,
-	.update_alarm = xadc_zynq_update_alarm,
-	.type = XADC_TYPE_S7,
-	/* Temp in C = (val * 503.975) / 2**bits - 273.15 */
-	.temp_scale = 503975,
-	.temp_offset = 273150,
-};
-
-static const unsigned int xadc_axi_reg_offsets[] = {
-	[XADC_TYPE_S7] = XADC_7S_AXI_ADC_REG_OFFSET,
-	[XADC_TYPE_US] = XADC_US_AXI_ADC_REG_OFFSET,
-};
-
-static int xadc_axi_read_adc_reg(struct xadc *xadc, unsigned int reg,
-	uint16_t *val)
-{
-	uint32_t val32;
-
-	xadc_read_reg(xadc, xadc_axi_reg_offsets[xadc->ops->type] + reg * 4,
-		&val32);
-	*val = val32 & 0xffff;
-
-	return 0;
-}
-
-static int xadc_axi_write_adc_reg(struct xadc *xadc, unsigned int reg,
-	uint16_t val)
-{
-	xadc_write_reg(xadc, xadc_axi_reg_offsets[xadc->ops->type] + reg * 4,
-		val);
-
-	return 0;
-}
-
-static int xadc_axi_setup(struct platform_device *pdev,
-	struct iio_dev *indio_dev, int irq)
-{
-	struct xadc *xadc = iio_priv(indio_dev);
-
-	xadc_write_reg(xadc, XADC_AXI_REG_RESET, XADC_AXI_RESET_MAGIC);
-	xadc_write_reg(xadc, XADC_AXI_REG_GIER, XADC_AXI_GIER_ENABLE);
-
-	return 0;
-}
-
-static irqreturn_t xadc_axi_interrupt_handler(int irq, void *devid)
-{
-	struct iio_dev *indio_dev = devid;
-	struct xadc *xadc = iio_priv(indio_dev);
-	uint32_t status, mask;
-	unsigned int events;
-
-	xadc_read_reg(xadc, XADC_AXI_REG_IPISR, &status);
-	xadc_read_reg(xadc, XADC_AXI_REG_IPIER, &mask);
-	status &= mask;
-
-	if (!status)
-		return IRQ_NONE;
-
-	if ((status & XADC_AXI_INT_EOS) && xadc->trigger)
-		iio_trigger_poll(xadc->trigger);
-
-	if (status & XADC_AXI_INT_ALARM_MASK) {
-		/*
-		 * The order of the bits in the AXI-XADC status register does
-		 * not match the order of the bits in the XADC alarm enable
-		 * register. xadc_handle_events() expects the events to be in
-		 * the same order as the XADC alarm enable register.
-		 */
-		events = (status & 0x000e) >> 1;
-		events |= (status & 0x0001) << 3;
-		events |= (status & 0x3c00) >> 6;
-		xadc_handle_events(indio_dev, events);
-	}
-
-	xadc_write_reg(xadc, XADC_AXI_REG_IPISR, status);
-
-	return IRQ_HANDLED;
-}
-
-static void xadc_axi_update_alarm(struct xadc *xadc, unsigned int alarm)
-{
-	uint32_t val;
-	unsigned long flags;
-
-	/*
-	 * The order of the bits in the AXI-XADC status register does not match
-	 * the order of the bits in the XADC alarm enable register. We get
-	 * passed the alarm mask in the same order as in the XADC alarm enable
-	 * register.
-	 */
-	alarm = ((alarm & 0x07) << 1) | ((alarm & 0x08) >> 3) |
-			((alarm & 0xf0) << 6);
-
-	spin_lock_irqsave(&xadc->lock, flags);
-	xadc_read_reg(xadc, XADC_AXI_REG_IPIER, &val);
-	val &= ~XADC_AXI_INT_ALARM_MASK;
-	val |= alarm;
-	xadc_write_reg(xadc, XADC_AXI_REG_IPIER, val);
-	spin_unlock_irqrestore(&xadc->lock, flags);
-}
-
-static unsigned long xadc_axi_get_dclk(struct xadc *xadc)
-{
-	return clk_get_rate(xadc->clk);
-}
-
-static const struct xadc_ops xadc_7s_axi_ops = {
-	.read = xadc_axi_read_adc_reg,
-	.write = xadc_axi_write_adc_reg,
-	.setup = xadc_axi_setup,
-	.get_dclk_rate = xadc_axi_get_dclk,
-	.update_alarm = xadc_axi_update_alarm,
-	.interrupt_handler = xadc_axi_interrupt_handler,
-	.flags = XADC_FLAGS_BUFFERED | XADC_FLAGS_IRQ_OPTIONAL,
-	.type = XADC_TYPE_S7,
-	/* Temp in C = (val * 503.975) / 2**bits - 273.15 */
-	.temp_scale = 503975,
-	.temp_offset = 273150,
-};
-
-static const struct xadc_ops xadc_us_axi_ops = {
-	.read = xadc_axi_read_adc_reg,
-	.write = xadc_axi_write_adc_reg,
-	.setup = xadc_axi_setup,
-	.get_dclk_rate = xadc_axi_get_dclk,
-	.update_alarm = xadc_axi_update_alarm,
-	.interrupt_handler = xadc_axi_interrupt_handler,
-	.flags = XADC_FLAGS_BUFFERED | XADC_FLAGS_IRQ_OPTIONAL,
-	.type = XADC_TYPE_US,
-	/**
-	 * Values below are for UltraScale+ (SYSMONE4) using internal reference.
-	 * See https://docs.xilinx.com/v/u/en-US/ug580-ultrascale-sysmon
-	 */
-	.temp_scale = 509314,
-	.temp_offset = 280231,
-};
-
-static int _xadc_update_adc_reg(struct xadc *xadc, unsigned int reg,
-	uint16_t mask, uint16_t val)
+static int _xadc_update_adc_reg(struct xadc *xadc, unsigned int reg, u16 mask, u16 val)
 {
 	uint16_t tmp;
 	int ret;
@@ -604,8 +53,7 @@ static int _xadc_update_adc_reg(struct xadc *xadc, unsigned int reg,
 	return _xadc_write_adc_reg(xadc, reg, (tmp & ~mask) | val);
 }
 
-static int xadc_update_adc_reg(struct xadc *xadc, unsigned int reg,
-	uint16_t mask, uint16_t val)
+static int xadc_update_adc_reg(struct xadc *xadc, unsigned int reg, u16 mask, u16 val)
 {
 	int ret;
 
@@ -621,12 +69,11 @@ static unsigned long xadc_get_dclk_rate(struct xadc *xadc)
 	return xadc->ops->get_dclk_rate(xadc);
 }
 
-static int xadc_update_scan_mode(struct iio_dev *indio_dev,
-	const unsigned long *mask)
+static int xadc_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *mask)
 {
 	struct xadc *xadc = iio_priv(indio_dev);
-	size_t n;
 	void *data;
+	size_t n;
 
 	n = bitmap_weight(mask, iio_get_masklength(indio_dev));
 
@@ -698,9 +145,8 @@ static irqreturn_t xadc_trigger_handler(int irq, void *p)
 static int xadc_trigger_set_state(struct iio_trigger *trigger, bool state)
 {
 	struct xadc *xadc = iio_trigger_get_drvdata(trigger);
+	unsigned int convst, val;
 	unsigned long flags;
-	unsigned int convst;
-	unsigned int val;
 	int ret = 0;
 
 	mutex_lock(&xadc->mutex);
@@ -718,7 +164,7 @@ static int xadc_trigger_set_state(struct iio_trigger *trigger, bool state)
 				convst = 0;
 		}
 		ret = _xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF0_EC,
-					convst);
+					   convst);
 		if (ret)
 			goto err_out;
 	} else {
@@ -745,8 +191,7 @@ static const struct iio_trigger_ops xadc_trigger_ops = {
 	.set_trigger_state = &xadc_trigger_set_state,
 };
 
-static struct iio_trigger *xadc_alloc_trigger(struct iio_dev *indio_dev,
-	const char *name)
+static struct iio_trigger *xadc_alloc_trigger(struct iio_dev *indio_dev, const char *name)
 {
 	struct device *dev = indio_dev->dev.parent;
 	struct iio_trigger *trig;
@@ -813,12 +258,12 @@ static int xadc_get_seq_mode(struct xadc *xadc, unsigned long scan_mode)
 	return XADC_CONF1_SEQ_SIMULTANEOUS;
 }
 
-static int xadc_postdisable(struct iio_dev *indio_dev)
+int xadc_postdisable(struct iio_dev *indio_dev)
 {
 	struct xadc *xadc = iio_priv(indio_dev);
 	unsigned long scan_mask;
 	int ret;
-	int i;
+	u32 i;
 
 	scan_mask = 1; /* Run calibration as part of the sequence */
 	for (i = 0; i < indio_dev->num_channels; i++)
@@ -840,6 +285,7 @@ static int xadc_postdisable(struct iio_dev *indio_dev)
 
 	return xadc_power_adc_b(xadc, XADC_CONF1_SEQ_CONTINUOUS);
 }
+EXPORT_SYMBOL_GPL(xadc_postdisable);
 
 static int xadc_preenable(struct iio_dev *indio_dev)
 {
@@ -894,7 +340,7 @@ static const struct iio_buffer_setup_ops xadc_buffer_ops = {
 	.postdisable = &xadc_postdisable,
 };
 
-static int xadc_read_samplerate(struct xadc *xadc)
+int xadc_read_samplerate(struct xadc *xadc)
 {
 	unsigned int div;
 	uint16_t val16;
@@ -910,9 +356,10 @@ static int xadc_read_samplerate(struct xadc *xadc)
 
 	return xadc_get_dclk_rate(xadc) / div / 26;
 }
+EXPORT_SYMBOL_GPL(xadc_read_samplerate);
 
-static int xadc_read_raw(struct iio_dev *indio_dev,
-	struct iio_chan_spec const *chan, int *val, int *val2, long info)
+static int xadc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+			 int *val, int *val2, long info)
 {
 	struct xadc *xadc = iio_priv(indio_dev);
 	unsigned int bits = chan->scan_type.realbits;
@@ -978,7 +425,37 @@ static int xadc_read_raw(struct iio_dev *indio_dev,
 	}
 }
 
-static int xadc_write_samplerate(struct xadc *xadc, int val)
+int xadc_setup_buffer_and_triggers(struct device *dev, struct iio_dev *indio_dev,
+				   struct xadc *xadc, int irq)
+{
+	int ret;
+
+	if (!(xadc->ops->flags & XADC_FLAGS_BUFFERED))
+		return 0;
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+					      &iio_pollfunc_store_time,
+					      &xadc_trigger_handler,
+					      &xadc_buffer_ops);
+	if (ret)
+		return ret;
+
+	if (irq > 0) {
+		xadc->convst_trigger = xadc_alloc_trigger(indio_dev, "convst");
+		if (IS_ERR(xadc->convst_trigger))
+			return PTR_ERR(xadc->convst_trigger);
+
+		xadc->samplerate_trigger = xadc_alloc_trigger(indio_dev,
+							      "samplerate");
+		if (IS_ERR(xadc->samplerate_trigger))
+			return PTR_ERR(xadc->samplerate_trigger);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xadc_setup_buffer_and_triggers);
+
+int xadc_write_samplerate(struct xadc *xadc, int val)
 {
 	unsigned long clk_rate = xadc_get_dclk_rate(xadc);
 	unsigned int div;
@@ -1014,6 +491,7 @@ static int xadc_write_samplerate(struct xadc *xadc, int val)
 	return xadc_update_adc_reg(xadc, XADC_REG_CONF2, XADC_CONF2_DIV_MASK,
 		div << XADC_CONF2_DIV_OFFSET);
 }
+EXPORT_SYMBOL_GPL(xadc_write_samplerate);
 
 static int xadc_write_raw(struct iio_dev *indio_dev,
 	struct iio_chan_spec const *chan, int val, int val2, long info)
@@ -1175,21 +653,6 @@ static const struct iio_info xadc_info = {
 	.update_scan_mode = &xadc_update_scan_mode,
 };
 
-static const struct of_device_id xadc_of_match_table[] = {
-	{
-		.compatible = "xlnx,zynq-xadc-1.00.a",
-		.data = &xadc_zynq_ops
-	}, {
-		.compatible = "xlnx,axi-xadc-1.00.a",
-		.data = &xadc_7s_axi_ops
-	}, {
-		.compatible = "xlnx,system-management-wiz-1.3",
-		.data = &xadc_us_axi_ops
-	},
-	{ }
-};
-MODULE_DEVICE_TABLE(of, xadc_of_match_table);
-
 static int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq)
 {
 	struct device *dev = indio_dev->dev.parent;
@@ -1298,156 +761,53 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq)
 	return 0;
 }
 
-static const char * const xadc_type_names[] = {
+const char * const xadc_type_names[] = {
 	[XADC_TYPE_S7] = "xadc",
 	[XADC_TYPE_US] = "xilinx-system-monitor",
 };
 
-static void xadc_cancel_delayed_work(void *data)
+struct iio_dev *xadc_device_setup(struct device *dev, int size,
+				  const struct xadc_ops **ops)
 {
-	struct delayed_work *work = data;
-
-	cancel_delayed_work_sync(work);
-}
-
-static int xadc_probe(struct platform_device *pdev)
-{
-	struct device *dev = &pdev->dev;
-	const struct xadc_ops *ops;
 	struct iio_dev *indio_dev;
-	unsigned int bipolar_mask;
-	unsigned int conf0;
-	struct xadc *xadc;
-	int ret;
-	int irq;
-	int i;
-
-	ops = device_get_match_data(dev);
-	if (!ops)
-		return -EINVAL;
 
-	irq = platform_get_irq_optional(pdev, 0);
-	if (irq < 0 &&
-	    (irq != -ENXIO || !(ops->flags & XADC_FLAGS_IRQ_OPTIONAL)))
-		return irq;
+	*ops = device_get_match_data(dev);
+	if (!*ops)
+		return ERR_PTR(-ENODEV);
 
-	indio_dev = devm_iio_device_alloc(dev, sizeof(*xadc));
+	indio_dev = devm_iio_device_alloc(dev, size);
 	if (!indio_dev)
-		return -ENOMEM;
-
-	xadc = iio_priv(indio_dev);
-	xadc->ops = ops;
-	init_completion(&xadc->completion);
-	mutex_init(&xadc->mutex);
-	spin_lock_init(&xadc->lock);
-	INIT_DELAYED_WORK(&xadc->zynq_unmask_work, xadc_zynq_unmask_worker);
-
-	xadc->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(xadc->base))
-		return PTR_ERR(xadc->base);
+		return ERR_PTR(-ENOMEM);
 
-	indio_dev->name = xadc_type_names[xadc->ops->type];
-	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->name = xadc_type_names[(*ops)->type];
 	indio_dev->info = &xadc_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	ret = xadc_parse_dt(indio_dev, &conf0, irq);
-	if (ret)
-		return ret;
-
-	if (xadc->ops->flags & XADC_FLAGS_BUFFERED) {
-		ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
-						      &iio_pollfunc_store_time,
-						      &xadc_trigger_handler,
-						      &xadc_buffer_ops);
-		if (ret)
-			return ret;
-
-		if (irq > 0) {
-			xadc->convst_trigger = xadc_alloc_trigger(indio_dev, "convst");
-			if (IS_ERR(xadc->convst_trigger))
-				return PTR_ERR(xadc->convst_trigger);
-
-			xadc->samplerate_trigger = xadc_alloc_trigger(indio_dev,
-				"samplerate");
-			if (IS_ERR(xadc->samplerate_trigger))
-				return PTR_ERR(xadc->samplerate_trigger);
-		}
-	}
-
-	xadc->clk = devm_clk_get_enabled(dev, NULL);
-	if (IS_ERR(xadc->clk))
-		return PTR_ERR(xadc->clk);
-
-	/*
-	 * Make sure not to exceed the maximum samplerate since otherwise the
-	 * resulting interrupt storm will soft-lock the system.
-	 */
-	if (xadc->ops->flags & XADC_FLAGS_BUFFERED) {
-		ret = xadc_read_samplerate(xadc);
-		if (ret < 0)
-			return ret;
-
-		if (ret > XADC_MAX_SAMPLERATE) {
-			ret = xadc_write_samplerate(xadc, XADC_MAX_SAMPLERATE);
-			if (ret < 0)
-				return ret;
-		}
-	}
-
-	if (irq > 0) {
-		ret = devm_request_irq(dev, irq, xadc->ops->interrupt_handler,
-				       0, dev_name(dev), indio_dev);
-		if (ret)
-			return ret;
-
-		ret = devm_add_action_or_reset(dev, xadc_cancel_delayed_work,
-					       &xadc->zynq_unmask_work);
-		if (ret)
-			return ret;
-	}
-
-	ret = xadc->ops->setup(pdev, indio_dev, irq);
-	if (ret)
-		return ret;
+	return indio_dev;
+}
+EXPORT_SYMBOL_GPL(xadc_device_setup);
 
-	for (i = 0; i < 16; i++)
-		xadc_read_adc_reg(xadc, XADC_REG_THRESHOLD(i),
-			&xadc->threshold[i]);
+int xadc_device_configure(struct device *dev, struct iio_dev *indio_dev,
+			  int irq, unsigned int *conf0,
+			  unsigned int *bipolar_mask)
+{
+	int ret;
+	u32 i;
 
-	ret = xadc_write_adc_reg(xadc, XADC_REG_CONF0, conf0);
+	ret = xadc_parse_dt(indio_dev, conf0, irq);
 	if (ret)
 		return ret;
 
-	bipolar_mask = 0;
+	*bipolar_mask = 0;
 	for (i = 0; i < indio_dev->num_channels; i++) {
 		if (indio_dev->channels[i].scan_type.sign == 's')
-			bipolar_mask |= BIT(indio_dev->channels[i].scan_index);
+			*bipolar_mask |= BIT(indio_dev->channels[i].scan_index);
 	}
 
-	ret = xadc_write_adc_reg(xadc, XADC_REG_INPUT_MODE(0), bipolar_mask);
-	if (ret)
-		return ret;
-
-	ret = xadc_write_adc_reg(xadc, XADC_REG_INPUT_MODE(1),
-		bipolar_mask >> 16);
-	if (ret)
-		return ret;
-
-	/* Go to non-buffered mode */
-	xadc_postdisable(indio_dev);
-
-	return devm_iio_device_register(dev, indio_dev);
+	return 0;
 }
-
-static struct platform_driver xadc_driver = {
-	.probe = xadc_probe,
-	.driver = {
-		.name = "xadc",
-		.of_match_table = xadc_of_match_table,
-	},
-};
-module_platform_driver(xadc_driver);
+EXPORT_SYMBOL_GPL(xadc_device_configure);
 
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
-MODULE_DESCRIPTION("Xilinx XADC IIO driver");
+MODULE_DESCRIPTION("Xilinx XADC IIO core driver");
diff --git a/drivers/iio/adc/xilinx-xadc-platform.c b/drivers/iio/adc/xilinx-xadc-platform.c
new file mode 100644
index 000000000000..f1ffbf5cff42
--- /dev/null
+++ b/drivers/iio/adc/xilinx-xadc-platform.c
@@ -0,0 +1,665 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx XADC platform driver
+ *
+ * Copyright 2013-2014 Analog Devices Inc.
+ *  Author: Lars-Peter Clausen <lars@metafoo.de>
+ *
+ * Documentation for the parts can be found at:
+ *  - XADC hardmacro: Xilinx UG480
+ *  - ZYNQ XADC interface: Xilinx UG585
+ *  - AXI XADC interface: Xilinx PG019
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+
+#include "xilinx-xadc.h"
+
+static const unsigned int XADC_ZYNQ_UNMASK_TIMEOUT = 500;
+
+/* ZYNQ register definitions */
+#define XADC_ZYNQ_REG_CFG	0x00
+#define XADC_ZYNQ_REG_INTSTS	0x04
+#define XADC_ZYNQ_REG_INTMSK	0x08
+#define XADC_ZYNQ_REG_STATUS	0x0c
+#define XADC_ZYNQ_REG_CFIFO	0x10
+#define XADC_ZYNQ_REG_DFIFO	0x14
+#define XADC_ZYNQ_REG_CTL		0x18
+
+#define XADC_ZYNQ_CFG_ENABLE		BIT(31)
+#define XADC_ZYNQ_CFG_CFIFOTH_MASK	(0xf << 20)
+#define XADC_ZYNQ_CFG_CFIFOTH_OFFSET	20
+#define XADC_ZYNQ_CFG_DFIFOTH_MASK	(0xf << 16)
+#define XADC_ZYNQ_CFG_DFIFOTH_OFFSET	16
+#define XADC_ZYNQ_CFG_WEDGE		BIT(13)
+#define XADC_ZYNQ_CFG_REDGE		BIT(12)
+#define XADC_ZYNQ_CFG_TCKRATE_MASK	(0x3 << 8)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV2	(0x0 << 8)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV4	(0x1 << 8)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV8	(0x2 << 8)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV16	(0x3 << 8)
+#define XADC_ZYNQ_CFG_IGAP_MASK		0x1f
+#define XADC_ZYNQ_CFG_IGAP(x)		(x)
+
+#define XADC_ZYNQ_INT_CFIFO_LTH		BIT(9)
+#define XADC_ZYNQ_INT_DFIFO_GTH		BIT(8)
+#define XADC_ZYNQ_INT_ALARM_MASK	0xff
+#define XADC_ZYNQ_INT_ALARM_OFFSET	0
+
+#define XADC_ZYNQ_STATUS_CFIFO_LVL_MASK	(0xf << 16)
+#define XADC_ZYNQ_STATUS_CFIFO_LVL_OFFSET	16
+#define XADC_ZYNQ_STATUS_DFIFO_LVL_MASK	(0xf << 12)
+#define XADC_ZYNQ_STATUS_DFIFO_LVL_OFFSET	12
+#define XADC_ZYNQ_STATUS_CFIFOF		BIT(11)
+#define XADC_ZYNQ_STATUS_CFIFOE		BIT(10)
+#define XADC_ZYNQ_STATUS_DFIFOF		BIT(9)
+#define XADC_ZYNQ_STATUS_DFIFOE		BIT(8)
+#define XADC_ZYNQ_STATUS_OT		BIT(7)
+#define XADC_ZYNQ_STATUS_ALM(x)		BIT(x)
+
+#define XADC_ZYNQ_CTL_RESET		BIT(4)
+
+#define XADC_ZYNQ_CMD_NOP		0x00
+#define XADC_ZYNQ_CMD_READ		0x01
+#define XADC_ZYNQ_CMD_WRITE		0x02
+
+#define XADC_ZYNQ_CMD(cmd, addr, data) (((cmd) << 26) | ((addr) << 16) | (data))
+
+/* AXI register definitions */
+#define XADC_AXI_REG_RESET		0x00
+#define XADC_AXI_REG_STATUS		0x04
+#define XADC_AXI_REG_ALARM_STATUS	0x08
+#define XADC_AXI_REG_CONVST		0x0c
+#define XADC_AXI_REG_XADC_RESET		0x10
+#define XADC_AXI_REG_GIER		0x5c
+#define XADC_AXI_REG_IPISR		0x60
+#define XADC_AXI_REG_IPIER		0x68
+
+/* 7 Series */
+#define XADC_7S_AXI_ADC_REG_OFFSET	0x200
+/* UltraScale */
+#define XADC_US_AXI_ADC_REG_OFFSET	0x400
+#define XADC_AXI_RESET_MAGIC		0xa
+#define XADC_AXI_GIER_ENABLE		BIT(31)
+#define XADC_AXI_INT_EOS		BIT(4)
+#define XADC_AXI_INT_ALARM_MASK		0x3c0f
+
+/*
+ * The ZYNQ interface uses two asynchronous FIFOs for communication with the
+ * XADC. Reads and writes to the XADC register are performed by submitting a
+ * request to the command FIFO (CFIFO), once the request has been completed the
+ * result can be read from the data FIFO (DFIFO). The method currently used in
+ * this driver is to submit the request for a read/write operation, then go to
+ * sleep and wait for an interrupt that signals that a response is available in
+ * the data FIFO.
+ */
+static void xadc_zynq_write_fifo(struct xadc *xadc, uint32_t *cmd, unsigned int n)
+{
+	unsigned int i;
+
+	for (i = 0; i < n; i++)
+		xadc_write_reg(xadc, XADC_ZYNQ_REG_CFIFO, cmd[i]);
+}
+
+static void xadc_zynq_drain_fifo(struct xadc *xadc)
+{
+	u32 status, tmp;
+
+	xadc_read_reg(xadc, XADC_ZYNQ_REG_STATUS, &status);
+
+	while (!(status & XADC_ZYNQ_STATUS_DFIFOE)) {
+		xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &tmp);
+		xadc_read_reg(xadc, XADC_ZYNQ_REG_STATUS, &status);
+	}
+}
+
+static void xadc_zynq_update_intmsk(struct xadc *xadc, unsigned int mask, unsigned int val)
+{
+	xadc->zynq_intmask &= ~mask;
+	xadc->zynq_intmask |= val;
+
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTMSK, xadc->zynq_intmask | xadc->zynq_masked_alarm);
+}
+
+static int xadc_zynq_write_adc_reg(struct xadc *xadc, unsigned int reg, uint16_t val)
+{
+	u32 cmd[1], tmp;
+	int ret;
+
+	spin_lock_irq(&xadc->lock);
+	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH, XADC_ZYNQ_INT_DFIFO_GTH);
+
+	reinit_completion(&xadc->completion);
+
+	cmd[0] = XADC_ZYNQ_CMD(XADC_ZYNQ_CMD_WRITE, reg, val);
+	xadc_zynq_write_fifo(xadc, cmd, ARRAY_SIZE(cmd));
+	xadc_read_reg(xadc, XADC_ZYNQ_REG_CFG, &tmp);
+	tmp &= ~XADC_ZYNQ_CFG_DFIFOTH_MASK;
+	tmp |= 0 << XADC_ZYNQ_CFG_DFIFOTH_OFFSET;
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_CFG, tmp);
+
+	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH, 0);
+	spin_unlock_irq(&xadc->lock);
+
+	ret = wait_for_completion_interruptible_timeout(&xadc->completion, HZ);
+	if (ret == 0)
+		ret = -EIO;
+	else
+		ret = 0;
+
+	xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &tmp);
+
+	return ret;
+}
+
+static int xadc_zynq_read_adc_reg(struct xadc *xadc, unsigned int reg, uint16_t *val)
+{
+	u32 cmd[2], resp, tmp;
+	int ret;
+
+	cmd[0] = XADC_ZYNQ_CMD(XADC_ZYNQ_CMD_READ, reg, 0);
+	cmd[1] = XADC_ZYNQ_CMD(XADC_ZYNQ_CMD_NOP, 0, 0);
+
+	spin_lock_irq(&xadc->lock);
+	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH, XADC_ZYNQ_INT_DFIFO_GTH);
+	xadc_zynq_drain_fifo(xadc);
+	reinit_completion(&xadc->completion);
+
+	xadc_zynq_write_fifo(xadc, cmd, ARRAY_SIZE(cmd));
+	xadc_read_reg(xadc, XADC_ZYNQ_REG_CFG, &tmp);
+	tmp &= ~XADC_ZYNQ_CFG_DFIFOTH_MASK;
+	tmp |= 1 << XADC_ZYNQ_CFG_DFIFOTH_OFFSET;
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_CFG, tmp);
+
+	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH, 0);
+	spin_unlock_irq(&xadc->lock);
+	ret = wait_for_completion_interruptible_timeout(&xadc->completion, HZ);
+	if (ret == 0)
+		ret = -EIO;
+	if (ret < 0)
+		return ret;
+
+	xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &resp);
+	xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &resp);
+
+	*val = resp & 0xffff;
+
+	return 0;
+}
+
+static unsigned int xadc_zynq_transform_alarm(unsigned int alarm)
+{
+	return ((alarm & 0x80) >> 4) |
+		((alarm & 0x78) << 1) |
+		(alarm & 0x07);
+}
+
+/*
+ * The ZYNQ threshold interrupts are level sensitive. Since we can't make the
+ * threshold condition go way from within the interrupt handler, this means as
+ * soon as a threshold condition is present we would enter the interrupt handler
+ * again and again. To work around this we mask all active thresholds interrupts
+ * in the interrupt handler and start a timer. In this timer we poll the
+ * interrupt status and only if the interrupt is inactive we unmask it again.
+ */
+static void xadc_zynq_unmask_worker(struct work_struct *work)
+{
+	struct xadc *xadc = container_of(work, struct xadc, zynq_unmask_work.work);
+	unsigned int misc_sts, unmask;
+
+	xadc_read_reg(xadc, XADC_ZYNQ_REG_STATUS, &misc_sts);
+
+	misc_sts &= XADC_ZYNQ_INT_ALARM_MASK;
+
+	spin_lock_irq(&xadc->lock);
+
+	/* Clear those bits which are not active anymore */
+	unmask = (xadc->zynq_masked_alarm ^ misc_sts) & xadc->zynq_masked_alarm;
+	xadc->zynq_masked_alarm &= misc_sts;
+
+	/* Also clear those which are masked out anyway */
+	xadc->zynq_masked_alarm &= ~xadc->zynq_intmask;
+
+	/* Clear the interrupts before we unmask them */
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTSTS, unmask);
+
+	xadc_zynq_update_intmsk(xadc, 0, 0);
+
+	spin_unlock_irq(&xadc->lock);
+
+	/* if still pending some alarm re-trigger the timer */
+	if (xadc->zynq_masked_alarm)
+		schedule_delayed_work(&xadc->zynq_unmask_work,
+				      msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
+}
+
+static irqreturn_t xadc_zynq_interrupt_handler(int irq, void *devid)
+{
+	struct iio_dev *indio_dev = devid;
+	struct xadc *xadc = iio_priv(indio_dev);
+	u32 status;
+
+	xadc_read_reg(xadc, XADC_ZYNQ_REG_INTSTS, &status);
+
+	status &= ~(xadc->zynq_intmask | xadc->zynq_masked_alarm);
+
+	if (!status)
+		return IRQ_NONE;
+
+	spin_lock(&xadc->lock);
+
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTSTS, status);
+
+	if (status & XADC_ZYNQ_INT_DFIFO_GTH) {
+		xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH,
+					XADC_ZYNQ_INT_DFIFO_GTH);
+		complete(&xadc->completion);
+	}
+
+	status &= XADC_ZYNQ_INT_ALARM_MASK;
+	if (status) {
+		xadc->zynq_masked_alarm |= status;
+		/*
+		 * mask the current event interrupt,
+		 * unmask it when the interrupt is no more active.
+		 */
+		xadc_zynq_update_intmsk(xadc, 0, 0);
+
+		xadc_handle_events(indio_dev, xadc_zynq_transform_alarm(status));
+
+		/* unmask the required interrupts in timer. */
+		schedule_delayed_work(&xadc->zynq_unmask_work,
+				      msecs_to_jiffies(XADC_ZYNQ_UNMASK_TIMEOUT));
+	}
+	spin_unlock(&xadc->lock);
+
+	return IRQ_HANDLED;
+}
+
+#define XADC_ZYNQ_TCK_RATE_MAX 50000000
+#define XADC_ZYNQ_IGAP_DEFAULT 20
+#define XADC_ZYNQ_PCAP_RATE_MAX 200000000
+
+static int xadc_zynq_setup(struct platform_device *pdev, struct iio_dev *indio_dev, int irq)
+{
+	unsigned int tck_div, div, igap, tck_rate;
+	struct xadc *xadc = iio_priv(indio_dev);
+	unsigned long pcap_rate;
+	int ret;
+
+	/* TODO: Figure out how to make igap and tck_rate configurable */
+	igap = XADC_ZYNQ_IGAP_DEFAULT;
+	tck_rate = XADC_ZYNQ_TCK_RATE_MAX;
+
+	xadc->zynq_intmask = ~0;
+
+	pcap_rate = clk_get_rate(xadc->clk);
+	if (!pcap_rate)
+		return -EINVAL;
+
+	if (pcap_rate > XADC_ZYNQ_PCAP_RATE_MAX) {
+		ret = clk_set_rate(xadc->clk, (unsigned long)XADC_ZYNQ_PCAP_RATE_MAX);
+		if (ret)
+			return ret;
+	}
+
+	if (tck_rate > pcap_rate / 2) {
+		div = 2;
+	} else {
+		div = pcap_rate / tck_rate;
+		if (pcap_rate / div > XADC_ZYNQ_TCK_RATE_MAX)
+			div++;
+	}
+
+	if (div <= 3)
+		tck_div = XADC_ZYNQ_CFG_TCKRATE_DIV2;
+	else if (div <= 7)
+		tck_div = XADC_ZYNQ_CFG_TCKRATE_DIV4;
+	else if (div <= 15)
+		tck_div = XADC_ZYNQ_CFG_TCKRATE_DIV8;
+	else
+		tck_div = XADC_ZYNQ_CFG_TCKRATE_DIV16;
+
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_CTL, XADC_ZYNQ_CTL_RESET);
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_CTL, 0);
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTSTS, ~0);
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTMSK, xadc->zynq_intmask);
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_CFG, XADC_ZYNQ_CFG_ENABLE |
+		       XADC_ZYNQ_CFG_REDGE | XADC_ZYNQ_CFG_WEDGE |
+		       tck_div | XADC_ZYNQ_CFG_IGAP(igap));
+
+	if (pcap_rate > XADC_ZYNQ_PCAP_RATE_MAX) {
+		ret = clk_set_rate(xadc->clk, pcap_rate);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static unsigned long xadc_zynq_get_dclk_rate(struct xadc *xadc)
+{
+	unsigned int div;
+	u32 val;
+
+	xadc_read_reg(xadc, XADC_ZYNQ_REG_CFG, &val);
+
+	switch (val & XADC_ZYNQ_CFG_TCKRATE_MASK) {
+	case XADC_ZYNQ_CFG_TCKRATE_DIV4:
+		div = 4;
+		break;
+	case XADC_ZYNQ_CFG_TCKRATE_DIV8:
+		div = 8;
+		break;
+	case XADC_ZYNQ_CFG_TCKRATE_DIV16:
+		div = 16;
+		break;
+	default:
+		div = 2;
+		break;
+	}
+
+	return clk_get_rate(xadc->clk) / div;
+}
+
+static void xadc_zynq_update_alarm(struct xadc *xadc, unsigned int alarm)
+{
+	unsigned long flags;
+	u32 status;
+
+	/* Move OT to bit 7 */
+	alarm = ((alarm & 0x08) << 4) | ((alarm & 0xf0) >> 1) | (alarm & 0x07);
+
+	spin_lock_irqsave(&xadc->lock, flags);
+
+	/* Clear previous interrupts if any. */
+	xadc_read_reg(xadc, XADC_ZYNQ_REG_INTSTS, &status);
+	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTSTS, status & alarm);
+
+	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_ALARM_MASK,
+				~alarm & XADC_ZYNQ_INT_ALARM_MASK);
+
+	spin_unlock_irqrestore(&xadc->lock, flags);
+}
+
+static const struct xadc_ops xadc_zynq_ops = {
+	.read = xadc_zynq_read_adc_reg,
+	.write = xadc_zynq_write_adc_reg,
+	.setup = xadc_zynq_setup,
+	.get_dclk_rate = xadc_zynq_get_dclk_rate,
+	.interrupt_handler = xadc_zynq_interrupt_handler,
+	.update_alarm = xadc_zynq_update_alarm,
+	.type = XADC_TYPE_S7,
+	/* Temp in C = (val * 503.975) / 2**bits - 273.15 */
+	.temp_scale = 503975,
+	.temp_offset = 273150,
+};
+
+static const unsigned int xadc_axi_reg_offsets[] = {
+	[XADC_TYPE_S7] = XADC_7S_AXI_ADC_REG_OFFSET,
+	[XADC_TYPE_US] = XADC_US_AXI_ADC_REG_OFFSET,
+};
+
+static int xadc_axi_read_adc_reg(struct xadc *xadc, unsigned int reg, uint16_t *val)
+{
+	u32 val32;
+
+	xadc_read_reg(xadc, xadc_axi_reg_offsets[xadc->ops->type] + reg * 4, &val32);
+	*val = val32 & 0xffff;
+
+	return 0;
+}
+
+static int xadc_axi_write_adc_reg(struct xadc *xadc, unsigned int reg, u16 val)
+{
+	xadc_write_reg(xadc, xadc_axi_reg_offsets[xadc->ops->type] + reg * 4, val);
+
+	return 0;
+}
+
+static int xadc_axi_setup(struct platform_device *pdev, struct iio_dev *indio_dev, int irq)
+{
+	struct xadc *xadc = iio_priv(indio_dev);
+
+	xadc_write_reg(xadc, XADC_AXI_REG_RESET, XADC_AXI_RESET_MAGIC);
+	xadc_write_reg(xadc, XADC_AXI_REG_GIER, XADC_AXI_GIER_ENABLE);
+
+	return 0;
+}
+
+static irqreturn_t xadc_axi_interrupt_handler(int irq, void *devid)
+{
+	struct iio_dev *indio_dev = devid;
+	struct xadc *xadc = iio_priv(indio_dev);
+	u32 status, mask;
+	unsigned int events;
+
+	xadc_read_reg(xadc, XADC_AXI_REG_IPISR, &status);
+	xadc_read_reg(xadc, XADC_AXI_REG_IPIER, &mask);
+	status &= mask;
+
+	if (!status)
+		return IRQ_NONE;
+
+	if ((status & XADC_AXI_INT_EOS) && xadc->trigger)
+		iio_trigger_poll(xadc->trigger);
+
+	if (status & XADC_AXI_INT_ALARM_MASK) {
+		/*
+		 * The order of the bits in the AXI-XADC status register does
+		 * not match the order of the bits in the XADC alarm enable
+		 * register. xadc_handle_events() expects the events to be in
+		 * the same order as the XADC alarm enable register.
+		 */
+		events = (status & 0x000e) >> 1;
+		events |= (status & 0x0001) << 3;
+		events |= (status & 0x3c00) >> 6;
+		xadc_handle_events(indio_dev, events);
+	}
+
+	xadc_write_reg(xadc, XADC_AXI_REG_IPISR, status);
+
+	return IRQ_HANDLED;
+}
+
+static void xadc_axi_update_alarm(struct xadc *xadc, unsigned int alarm)
+{
+	u32 val;
+	unsigned long flags;
+
+	/*
+	 * The order of the bits in the AXI-XADC status register does not match
+	 * the order of the bits in the XADC alarm enable register. We get
+	 * passed the alarm mask in the same order as in the XADC alarm enable
+	 * register.
+	 */
+	alarm = ((alarm & 0x07) << 1) | ((alarm & 0x08) >> 3) |
+			((alarm & 0xf0) << 6);
+
+	spin_lock_irqsave(&xadc->lock, flags);
+	xadc_read_reg(xadc, XADC_AXI_REG_IPIER, &val);
+	val &= ~XADC_AXI_INT_ALARM_MASK;
+	val |= alarm;
+	xadc_write_reg(xadc, XADC_AXI_REG_IPIER, val);
+	spin_unlock_irqrestore(&xadc->lock, flags);
+}
+
+static unsigned long xadc_axi_get_dclk(struct xadc *xadc)
+{
+	return clk_get_rate(xadc->clk);
+}
+
+static const struct xadc_ops xadc_7s_axi_ops = {
+	.read = xadc_axi_read_adc_reg,
+	.write = xadc_axi_write_adc_reg,
+	.setup = xadc_axi_setup,
+	.get_dclk_rate = xadc_axi_get_dclk,
+	.update_alarm = xadc_axi_update_alarm,
+	.interrupt_handler = xadc_axi_interrupt_handler,
+	.flags = XADC_FLAGS_BUFFERED | XADC_FLAGS_IRQ_OPTIONAL,
+	.type = XADC_TYPE_S7,
+	/* Temp in C = (val * 503.975) / 2**bits - 273.15 */
+	.temp_scale = 503975,
+	.temp_offset = 273150,
+};
+
+static const struct xadc_ops xadc_us_axi_ops = {
+	.read = xadc_axi_read_adc_reg,
+	.write = xadc_axi_write_adc_reg,
+	.setup = xadc_axi_setup,
+	.get_dclk_rate = xadc_axi_get_dclk,
+	.update_alarm = xadc_axi_update_alarm,
+	.interrupt_handler = xadc_axi_interrupt_handler,
+	.flags = XADC_FLAGS_BUFFERED | XADC_FLAGS_IRQ_OPTIONAL,
+	.type = XADC_TYPE_US,
+	/**
+	 * Values below are for UltraScale+ (SYSMONE4) using internal reference.
+	 * See https://docs.xilinx.com/v/u/en-US/ug580-ultrascale-sysmon
+	 */
+	.temp_scale = 509314,
+	.temp_offset = 280231,
+};
+
+static const struct of_device_id xadc_of_match_table[] = {
+	{
+		.compatible = "xlnx,zynq-xadc-1.00.a",
+		.data = &xadc_zynq_ops
+	}, {
+		.compatible = "xlnx,axi-xadc-1.00.a",
+		.data = &xadc_7s_axi_ops
+	}, {
+		.compatible = "xlnx,system-management-wiz-1.3",
+		.data = &xadc_us_axi_ops
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, xadc_of_match_table);
+
+static void xadc_cancel_delayed_work(void *data)
+{
+	struct delayed_work *work = data;
+
+	cancel_delayed_work_sync(work);
+}
+
+static int xadc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct xadc_ops *ops;
+	struct iio_dev *indio_dev;
+	unsigned int bipolar_mask;
+	unsigned int conf0;
+	struct xadc *xadc;
+	int ret;
+	int irq;
+	u32 i;
+
+	indio_dev = xadc_device_setup(dev, sizeof(*xadc), &ops);
+	if (IS_ERR(indio_dev))
+		return PTR_ERR(indio_dev);
+
+	irq = platform_get_irq_optional(pdev, 0);
+	if (irq < 0 &&
+	    (irq != -ENXIO || !(ops->flags & XADC_FLAGS_IRQ_OPTIONAL)))
+		return irq;
+
+	xadc = iio_priv(indio_dev);
+	xadc->ops = ops;
+	init_completion(&xadc->completion);
+	mutex_init(&xadc->mutex);
+	spin_lock_init(&xadc->lock);
+	INIT_DELAYED_WORK(&xadc->zynq_unmask_work, xadc_zynq_unmask_worker);
+
+	xadc->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(xadc->base))
+		return PTR_ERR(xadc->base);
+
+	ret = xadc_device_configure(dev, indio_dev, irq, &conf0, &bipolar_mask);
+	if (ret)
+		return ret;
+
+	ret = xadc_setup_buffer_and_triggers(dev, indio_dev, xadc, irq);
+	if (ret)
+		return ret;
+
+	xadc->clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(xadc->clk))
+		return PTR_ERR(xadc->clk);
+
+	/*
+	 * Make sure not to exceed the maximum samplerate since otherwise the
+	 * resulting interrupt storm will soft-lock the system.
+	 */
+	if (xadc->ops->flags & XADC_FLAGS_BUFFERED) {
+		ret = xadc_read_samplerate(xadc);
+		if (ret < 0)
+			return ret;
+
+		if (ret > XADC_MAX_SAMPLERATE) {
+			ret = xadc_write_samplerate(xadc, XADC_MAX_SAMPLERATE);
+			if (ret < 0)
+				return ret;
+		}
+	}
+
+	if (irq > 0) {
+		ret = devm_request_irq(dev, irq, xadc->ops->interrupt_handler,
+				       0, dev_name(dev), indio_dev);
+		if (ret)
+			return ret;
+
+		ret = devm_add_action_or_reset(dev, xadc_cancel_delayed_work,
+					       &xadc->zynq_unmask_work);
+		if (ret)
+			return ret;
+	}
+
+	ret = xadc->ops->setup(pdev, indio_dev, irq);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < 16; i++)
+		xadc_read_adc_reg(xadc, XADC_REG_THRESHOLD(i), &xadc->threshold[i]);
+
+	ret = xadc_write_adc_reg(xadc, XADC_REG_CONF0, conf0);
+	if (ret)
+		return ret;
+
+	ret = xadc_write_adc_reg(xadc, XADC_REG_INPUT_MODE(0), bipolar_mask);
+	if (ret)
+		return ret;
+
+	ret = xadc_write_adc_reg(xadc, XADC_REG_INPUT_MODE(1), bipolar_mask >> 16);
+	if (ret)
+		return ret;
+
+	/* Go to non-buffered mode */
+	xadc_postdisable(indio_dev);
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static struct platform_driver xadc_driver = {
+	.probe = xadc_probe,
+	.driver = {
+		.name = "xadc",
+		.of_match_table = xadc_of_match_table,
+	},
+};
+module_platform_driver(xadc_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
+MODULE_DESCRIPTION("Xilinx XADC platform driver");
diff --git a/drivers/iio/adc/xilinx-xadc.h b/drivers/iio/adc/xilinx-xadc.h
index b4d9d4683117..a2d208fbd13b 100644
--- a/drivers/iio/adc/xilinx-xadc.h
+++ b/drivers/iio/adc/xilinx-xadc.h
@@ -211,4 +211,34 @@ static inline int xadc_write_adc_reg(struct xadc *xadc, unsigned int reg,
 #define XADC_THRESHOLD_VCCPAUX_MIN	0xe
 #define XADC_THRESHOLD_VCCODDR_MIN	0xf
 
+/*
+ * The XADC hardware supports a samplerate of up to 1MSPS. Unfortunately it does
+ * not have a hardware FIFO. Which means an interrupt is generated for each
+ * conversion sequence. At 1MSPS sample rate the CPU in ZYNQ7000 is completely
+ * overloaded by the interrupts that it soft-lockups. For this reason the driver
+ * limits the maximum samplerate 150kSPS. At this rate the CPU is fairly busy,
+ * but still responsive.
+ */
+#define XADC_MAX_SAMPLERATE 150000
+
+#define XADC_FLAGS_BUFFERED BIT(0)
+#define XADC_FLAGS_IRQ_OPTIONAL BIT(1)
+
+/* AXI register definitions needed by core */
+#define XADC_AXI_REG_IPISR		0x60
+#define XADC_AXI_REG_IPIER		0x68
+#define XADC_AXI_INT_EOS		BIT(4)
+
+void xadc_write_reg(struct xadc *xadc, unsigned int reg, uint32_t val);
+void xadc_read_reg(struct xadc *xadc, unsigned int reg, uint32_t *val);
+struct iio_dev *xadc_device_setup(struct device *dev, int size,
+				  const struct xadc_ops **ops);
+int xadc_device_configure(struct device *dev, struct iio_dev *indio_dev,
+			  int irq, unsigned int *conf0, unsigned int *bipolar_mask);
+int xadc_read_samplerate(struct xadc *xadc);
+int xadc_write_samplerate(struct xadc *xadc, int val);
+int xadc_setup_buffer_and_triggers(struct device *dev, struct iio_dev *indio_dev,
+				   struct xadc *xadc, int irq);
+int xadc_postdisable(struct iio_dev *indio_dev);
+
 #endif
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/4] iio: adc: xilinx-xadc: Add .setup_channels() to struct xadc_ops
  2026-03-23  7:45 [PATCH v2 0/4] iio: adc: xilinx-xadc: Add I2C interface support for System Management Wizard Sai Krishna Potthuri
  2026-03-23  7:45 ` [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files Sai Krishna Potthuri
@ 2026-03-23  7:45 ` Sai Krishna Potthuri
  2026-03-23  7:45 ` [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support Sai Krishna Potthuri
  2026-03-23  7:45 ` [PATCH v2 4/4] dt-bindings: iio: adc: xlnx,axi-xadc: convert to DT schema Sai Krishna Potthuri
  3 siblings, 0 replies; 14+ messages in thread
From: Sai Krishna Potthuri @ 2026-03-23  7:45 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sa, Andy Shevchenko,
	Michal Simek, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-arm-kernel, linux-kernel,
	saikrishna12468, git, Sai Krishna Potthuri

Add .setup_channels() function pointer to struct xadc_ops to enable
different interfaces to have custom channel setup logic.

Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
---
 drivers/iio/adc/xilinx-xadc-core.c     | 6 ++++--
 drivers/iio/adc/xilinx-xadc-platform.c | 3 +++
 drivers/iio/adc/xilinx-xadc.h          | 2 ++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 268e46e5349c..7fbf55f8e0bb 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -653,7 +653,7 @@ static const struct iio_info xadc_info = {
 	.update_scan_mode = &xadc_update_scan_mode,
 };
 
-static int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq)
+int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq)
 {
 	struct device *dev = indio_dev->dev.parent;
 	struct xadc *xadc = iio_priv(indio_dev);
@@ -760,6 +760,7 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(xadc_parse_dt);
 
 const char * const xadc_type_names[] = {
 	[XADC_TYPE_S7] = "xadc",
@@ -791,10 +792,11 @@ int xadc_device_configure(struct device *dev, struct iio_dev *indio_dev,
 			  int irq, unsigned int *conf0,
 			  unsigned int *bipolar_mask)
 {
+	struct xadc *xadc = iio_priv(indio_dev);
 	int ret;
 	u32 i;
 
-	ret = xadc_parse_dt(indio_dev, conf0, irq);
+	ret = xadc->ops->setup_channels(indio_dev, conf0, irq);
 	if (ret)
 		return ret;
 
diff --git a/drivers/iio/adc/xilinx-xadc-platform.c b/drivers/iio/adc/xilinx-xadc-platform.c
index f1ffbf5cff42..9015b131daa3 100644
--- a/drivers/iio/adc/xilinx-xadc-platform.c
+++ b/drivers/iio/adc/xilinx-xadc-platform.c
@@ -401,6 +401,7 @@ static const struct xadc_ops xadc_zynq_ops = {
 	.get_dclk_rate = xadc_zynq_get_dclk_rate,
 	.interrupt_handler = xadc_zynq_interrupt_handler,
 	.update_alarm = xadc_zynq_update_alarm,
+	.setup_channels = xadc_parse_dt,
 	.type = XADC_TYPE_S7,
 	/* Temp in C = (val * 503.975) / 2**bits - 273.15 */
 	.temp_scale = 503975,
@@ -508,6 +509,7 @@ static const struct xadc_ops xadc_7s_axi_ops = {
 	.get_dclk_rate = xadc_axi_get_dclk,
 	.update_alarm = xadc_axi_update_alarm,
 	.interrupt_handler = xadc_axi_interrupt_handler,
+	.setup_channels = xadc_parse_dt,
 	.flags = XADC_FLAGS_BUFFERED | XADC_FLAGS_IRQ_OPTIONAL,
 	.type = XADC_TYPE_S7,
 	/* Temp in C = (val * 503.975) / 2**bits - 273.15 */
@@ -522,6 +524,7 @@ static const struct xadc_ops xadc_us_axi_ops = {
 	.get_dclk_rate = xadc_axi_get_dclk,
 	.update_alarm = xadc_axi_update_alarm,
 	.interrupt_handler = xadc_axi_interrupt_handler,
+	.setup_channels = xadc_parse_dt,
 	.flags = XADC_FLAGS_BUFFERED | XADC_FLAGS_IRQ_OPTIONAL,
 	.type = XADC_TYPE_US,
 	/**
diff --git a/drivers/iio/adc/xilinx-xadc.h b/drivers/iio/adc/xilinx-xadc.h
index a2d208fbd13b..feec8ef76e4f 100644
--- a/drivers/iio/adc/xilinx-xadc.h
+++ b/drivers/iio/adc/xilinx-xadc.h
@@ -82,6 +82,7 @@ struct xadc_ops {
 	void (*update_alarm)(struct xadc *xadc, unsigned int alarm);
 	unsigned long (*get_dclk_rate)(struct xadc *xadc);
 	irqreturn_t (*interrupt_handler)(int irq, void *devid);
+	int (*setup_channels)(struct iio_dev *indio_dev, unsigned int *conf, int irq);
 
 	unsigned int flags;
 	enum xadc_type type;
@@ -233,6 +234,7 @@ void xadc_write_reg(struct xadc *xadc, unsigned int reg, uint32_t val);
 void xadc_read_reg(struct xadc *xadc, unsigned int reg, uint32_t *val);
 struct iio_dev *xadc_device_setup(struct device *dev, int size,
 				  const struct xadc_ops **ops);
+int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq);
 int xadc_device_configure(struct device *dev, struct iio_dev *indio_dev,
 			  int irq, unsigned int *conf0, unsigned int *bipolar_mask);
 int xadc_read_samplerate(struct xadc *xadc);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support
  2026-03-23  7:45 [PATCH v2 0/4] iio: adc: xilinx-xadc: Add I2C interface support for System Management Wizard Sai Krishna Potthuri
  2026-03-23  7:45 ` [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files Sai Krishna Potthuri
  2026-03-23  7:45 ` [PATCH v2 2/4] iio: adc: xilinx-xadc: Add .setup_channels() to struct xadc_ops Sai Krishna Potthuri
@ 2026-03-23  7:45 ` Sai Krishna Potthuri
  2026-03-23 10:52   ` Andy Shevchenko
  2026-03-23 20:26   ` Jonathan Cameron
  2026-03-23  7:45 ` [PATCH v2 4/4] dt-bindings: iio: adc: xlnx,axi-xadc: convert to DT schema Sai Krishna Potthuri
  3 siblings, 2 replies; 14+ messages in thread
From: Sai Krishna Potthuri @ 2026-03-23  7:45 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sa, Andy Shevchenko,
	Michal Simek, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-arm-kernel, linux-kernel,
	saikrishna12468, git, Sai Krishna Potthuri

Add I2C interface support for Xilinx System Management Wizard IP along
with the existing AXI memory-mapped interface. This support enables
monitoring the voltage and temperature on UltraScale+ devices where the
System Management Wizard is connected via I2C.

Key changes:
- Implement 32-bit DRP(Dynamic Reconfiguration Port) packet format as per
  Xilinx PG185 specification.
- Add separate I2C probe with xadc_i2c_of_match_table to handle same
  compatible string("xlnx,system-management-wiz-1.3") on I2C bus.
- Implement delayed version of hardware initialization for I2C interface
  to handle the case where System Management Wizard IP is not ready during
  the I2C probe.
- Add NULL checks for get_dclk_rate callback function in sampling rate
  functions to support interfaces without clock control
- Create separate iio_info structure(xadc_i2c_info) without event
  callbacks for I2C devices
- Add xadc_i2c_transaction() function to handle I2C read/write operations
- Add XADC_TYPE_US_I2C type to distinguish I2C interface from AXI

Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
---
 drivers/iio/adc/Kconfig            |  15 ++
 drivers/iio/adc/Makefile           |   1 +
 drivers/iio/adc/xilinx-xadc-core.c |  28 +++-
 drivers/iio/adc/xilinx-xadc-i2c.c  | 215 +++++++++++++++++++++++++++++
 drivers/iio/adc/xilinx-xadc.h      |   1 +
 5 files changed, 256 insertions(+), 4 deletions(-)
 create mode 100644 drivers/iio/adc/xilinx-xadc-i2c.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a4a7556f4016..5a3956a5c086 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1767,6 +1767,21 @@ config XILINX_XADC
 	  The driver can also be build as a module. If so, the module will be called
 	  xilinx-xadc.
 
+config XILINX_XADC_I2C
+	tristate "Xilinx System Management Wizard I2C Interface support"
+	depends on I2C
+	select XILINX_XADC_CORE
+	help
+	  Say yes here to allow accessing the System Management
+	  Wizard on UltraScale+ devices via I2C.
+
+	  This provides voltage and temperature monitoring capabilities
+	  through the same IIO sysfs interface, but using I2C communication
+	  protocol.
+
+	  The driver can also be build as a module. If so, the module will be called
+	  xilinx-xadc-i2c.
+
 config XILINX_AMS
 	tristate "Xilinx AMS driver"
 	depends on ARCH_ZYNQMP || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 1b05176f0098..2dc08c9d82cc 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -157,3 +157,4 @@ obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o
 xilinx-xadc-common-y := xilinx-xadc-core.o xilinx-xadc-events.o
 obj-$(CONFIG_XILINX_XADC_CORE) += xilinx-xadc-common.o
 obj-$(CONFIG_XILINX_XADC) += xilinx-xadc-platform.o
+obj-$(CONFIG_XILINX_XADC_I2C) += xilinx-xadc-i2c.o
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 7fbf55f8e0bb..383bd93676ec 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -222,7 +222,8 @@ static int xadc_power_adc_b(struct xadc *xadc, unsigned int seq_mode)
 	 * non-existing ADC-B powers down the main ADC, so just return and don't
 	 * do anything.
 	 */
-	if (xadc->ops->type == XADC_TYPE_US)
+	if (xadc->ops->type == XADC_TYPE_US ||
+	    xadc->ops->type == XADC_TYPE_US_I2C)
 		return 0;
 
 	/* Powerdown the ADC-B when it is not needed. */
@@ -245,7 +246,8 @@ static int xadc_get_seq_mode(struct xadc *xadc, unsigned long scan_mode)
 	unsigned int aux_scan_mode = scan_mode >> 16;
 
 	/* UltraScale has only one ADC and supports only continuous mode */
-	if (xadc->ops->type == XADC_TYPE_US)
+	if (xadc->ops->type == XADC_TYPE_US ||
+	    xadc->ops->type == XADC_TYPE_US_I2C)
 		return XADC_CONF1_SEQ_CONTINUOUS;
 
 	if (xadc->external_mux_mode == XADC_EXTERNAL_MUX_DUAL)
@@ -346,6 +348,9 @@ int xadc_read_samplerate(struct xadc *xadc)
 	uint16_t val16;
 	int ret;
 
+	if (!xadc->ops->get_dclk_rate)
+		return -EOPNOTSUPP;
+
 	ret = xadc_read_adc_reg(xadc, XADC_REG_CONF2, &val16);
 	if (ret)
 		return ret;
@@ -457,9 +462,14 @@ EXPORT_SYMBOL_GPL(xadc_setup_buffer_and_triggers);
 
 int xadc_write_samplerate(struct xadc *xadc, int val)
 {
-	unsigned long clk_rate = xadc_get_dclk_rate(xadc);
+	unsigned long clk_rate;
 	unsigned int div;
 
+	if (!xadc->ops->get_dclk_rate)
+		return -EOPNOTSUPP;
+
+	clk_rate = xadc_get_dclk_rate(xadc);
+
 	if (!clk_rate)
 		return -EINVAL;
 
@@ -653,6 +663,11 @@ static const struct iio_info xadc_info = {
 	.update_scan_mode = &xadc_update_scan_mode,
 };
 
+static const struct iio_info xadc_i2c_info = {
+	.read_raw = &xadc_read_raw,
+	.write_raw = &xadc_write_raw,
+};
+
 int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq)
 {
 	struct device *dev = indio_dev->dev.parent;
@@ -765,6 +780,7 @@ EXPORT_SYMBOL_GPL(xadc_parse_dt);
 const char * const xadc_type_names[] = {
 	[XADC_TYPE_S7] = "xadc",
 	[XADC_TYPE_US] = "xilinx-system-monitor",
+	[XADC_TYPE_US_I2C] = "xilinx-system-monitor",
 };
 
 struct iio_dev *xadc_device_setup(struct device *dev, int size,
@@ -781,7 +797,11 @@ struct iio_dev *xadc_device_setup(struct device *dev, int size,
 		return ERR_PTR(-ENOMEM);
 
 	indio_dev->name = xadc_type_names[(*ops)->type];
-	indio_dev->info = &xadc_info;
+	if ((*ops)->type == XADC_TYPE_US_I2C)
+		indio_dev->info = &xadc_i2c_info;
+	else
+		indio_dev->info = &xadc_info;
+
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	return indio_dev;
diff --git a/drivers/iio/adc/xilinx-xadc-i2c.c b/drivers/iio/adc/xilinx-xadc-i2c.c
new file mode 100644
index 000000000000..3d802b907260
--- /dev/null
+++ b/drivers/iio/adc/xilinx-xadc-i2c.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx XADC I2C Interface Driver
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ *
+ * This driver implements I2C interface support for Xilinx System Management
+ * Wizard IP on UltraScale+ devices. It uses the 32-bit DRP (Dynamic
+ * Reconfiguration Port) packet format as per Xilinx PG185 specification.
+ */
+
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+
+#include <linux/iio/iio.h>
+
+#include "xilinx-xadc.h"
+
+#define XADC_I2C_READ_DATA_SIZE		2
+#define XADC_I2C_WRITE_DATA_SIZE	4	/* 32-bit DRP packet */
+#define XADC_I2C_INSTR_READ		BIT(2)
+#define XADC_I2C_INSTR_WRITE		BIT(3)
+
+#define XADC_I2C_DRP_DATA0_MASK		GENMASK(7, 0)
+#define XADC_I2C_DRP_DATA1_MASK		GENMASK(15, 8)
+#define XADC_I2C_DRP_ADDR_MASK		GENMASK(7, 0)
+
+#define XADC_INPUT_MODE_BITS		16
+
+struct xadc_i2c {
+	struct xadc xadc;
+	struct i2c_client *client;
+	bool hw_initialized;
+	unsigned int conf0;
+	unsigned int bipolar_mask;
+};
+
+static int xadc_i2c_read_transaction(struct xadc *xadc, unsigned int reg, u16 *val)
+{
+	struct xadc_i2c *xadc_i2c = container_of(xadc, struct xadc_i2c, xadc);
+	char write_buffer[XADC_I2C_WRITE_DATA_SIZE] = { 0 };
+	struct i2c_client *client = xadc_i2c->client;
+	char read_buffer[XADC_I2C_READ_DATA_SIZE];
+	int ret;
+
+	write_buffer[2] = FIELD_GET(XADC_I2C_DRP_ADDR_MASK, reg);
+	write_buffer[3] = XADC_I2C_INSTR_READ;
+
+	ret = i2c_master_send(client, write_buffer, XADC_I2C_WRITE_DATA_SIZE);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_master_recv(client, read_buffer, XADC_I2C_READ_DATA_SIZE);
+	if (ret < 0)
+		return ret;
+
+	*val = FIELD_PREP(XADC_I2C_DRP_DATA0_MASK, read_buffer[0]) |
+	       FIELD_PREP(XADC_I2C_DRP_DATA1_MASK, read_buffer[1]);
+
+	return 0;
+}
+
+static int xadc_i2c_write_transaction(struct xadc *xadc, unsigned int reg, u16 val)
+{
+	struct xadc_i2c *xadc_i2c = container_of(xadc, struct xadc_i2c, xadc);
+	struct i2c_client *client = xadc_i2c->client;
+	char write_buffer[XADC_I2C_WRITE_DATA_SIZE];
+	int ret;
+
+	write_buffer[0] = FIELD_GET(XADC_I2C_DRP_DATA0_MASK, val);
+	write_buffer[1] = FIELD_GET(XADC_I2C_DRP_DATA1_MASK, val);
+	write_buffer[2] = FIELD_GET(XADC_I2C_DRP_ADDR_MASK, reg);
+	write_buffer[3] = XADC_I2C_INSTR_WRITE;
+
+	ret = i2c_master_send(client, write_buffer, XADC_I2C_WRITE_DATA_SIZE);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int xadc_hardware_init(struct xadc *xadc)
+{
+	struct xadc_i2c *xadc_i2c = container_of(xadc, struct xadc_i2c, xadc);
+	int ret;
+	u32 i;
+
+	for (i = 0; i < ARRAY_SIZE(xadc->threshold); i++) {
+		ret = xadc_i2c_read_transaction(xadc, XADC_REG_THRESHOLD(i),
+						&xadc->threshold[i]);
+		if (ret)
+			return ret;
+	}
+
+	ret = xadc_i2c_write_transaction(xadc, XADC_REG_CONF0, xadc_i2c->conf0);
+	if (ret)
+		return ret;
+
+	ret = xadc_i2c_write_transaction(xadc, XADC_REG_INPUT_MODE(0),
+					 xadc_i2c->bipolar_mask);
+	if (ret)
+		return ret;
+
+	ret = xadc_i2c_write_transaction(xadc, XADC_REG_INPUT_MODE(1),
+					 xadc_i2c->bipolar_mask >> XADC_INPUT_MODE_BITS);
+	if (ret)
+		return ret;
+
+	xadc_i2c->hw_initialized = true;
+
+	return 0;
+}
+
+static int xadc_i2c_read_reg(struct xadc *xadc, unsigned int reg, u16 *val)
+{
+	struct xadc_i2c *xadc_i2c = container_of(xadc, struct xadc_i2c, xadc);
+
+	if (!xadc_i2c->hw_initialized) {
+		int ret;
+
+		ret = xadc_hardware_init(xadc);
+		if (ret)
+			return ret;
+	}
+
+	return xadc_i2c_read_transaction(xadc, reg, val);
+}
+
+static int xadc_i2c_write_reg(struct xadc *xadc, unsigned int reg, u16 val)
+{
+	struct xadc_i2c *xadc_i2c = container_of(xadc, struct xadc_i2c, xadc);
+
+	if (!xadc_i2c->hw_initialized) {
+		int ret;
+
+		ret = xadc_hardware_init(xadc);
+		if (ret)
+			return ret;
+	}
+
+	return xadc_i2c_write_transaction(xadc, reg, val);
+}
+
+static const struct xadc_ops xadc_system_mgmt_wiz_i2c_ops = {
+	.read = xadc_i2c_read_reg,
+	.write = xadc_i2c_write_reg,
+	.setup_channels = xadc_parse_dt,
+	.type = XADC_TYPE_US_I2C,
+	.temp_scale = 509314,
+	.temp_offset = 280231,
+};
+
+static int xadc_i2c_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	unsigned int conf0, bipolar_mask;
+	const struct xadc_ops *ops;
+	struct iio_dev *indio_dev;
+	struct xadc_i2c *xadc_i2c;
+	struct xadc *xadc;
+	int ret;
+
+	indio_dev = xadc_device_setup(dev, sizeof(*xadc_i2c), &ops);
+	if (IS_ERR(indio_dev))
+		return PTR_ERR(indio_dev);
+
+	xadc_i2c = iio_priv(indio_dev);
+	xadc_i2c->client = client;
+	xadc = &xadc_i2c->xadc;
+	xadc->clk = NULL;
+	xadc->ops = ops;
+	mutex_init(&xadc->mutex);
+	spin_lock_init(&xadc->lock);
+
+	ret = xadc_device_configure(dev, indio_dev, 0, &conf0, &bipolar_mask);
+	if (ret) {
+		dev_err(dev, "Failed to setup the device: %d\n", ret);
+		return ret;
+	}
+
+	i2c_set_clientdata(client, indio_dev);
+	xadc_i2c->conf0 = conf0;
+	xadc_i2c->bipolar_mask = bipolar_mask;
+	xadc_i2c->hw_initialized = false;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id xadc_i2c_of_match_table[] = {
+	{
+		.compatible = "xlnx,system-management-wiz-1.3",
+		.data = &xadc_system_mgmt_wiz_i2c_ops,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, xadc_i2c_of_match_table);
+
+static struct i2c_driver xadc_i2c_driver = {
+	.probe = xadc_i2c_probe,
+	.driver = {
+		.name = "xadc-i2c",
+		.of_match_table = xadc_i2c_of_match_table,
+	},
+};
+module_i2c_driver(xadc_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>");
+MODULE_DESCRIPTION("Xilinx XADC I2C Interface Driver");
diff --git a/drivers/iio/adc/xilinx-xadc.h b/drivers/iio/adc/xilinx-xadc.h
index feec8ef76e4f..d0c64b5f55f1 100644
--- a/drivers/iio/adc/xilinx-xadc.h
+++ b/drivers/iio/adc/xilinx-xadc.h
@@ -72,6 +72,7 @@ struct xadc {
 enum xadc_type {
 	XADC_TYPE_S7, /* Series 7 */
 	XADC_TYPE_US, /* UltraScale and UltraScale+ */
+	XADC_TYPE_US_I2C, /* UltraScale+ I2C interface */
 };
 
 struct xadc_ops {
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/4] dt-bindings: iio: adc: xlnx,axi-xadc: convert to DT schema
  2026-03-23  7:45 [PATCH v2 0/4] iio: adc: xilinx-xadc: Add I2C interface support for System Management Wizard Sai Krishna Potthuri
                   ` (2 preceding siblings ...)
  2026-03-23  7:45 ` [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support Sai Krishna Potthuri
@ 2026-03-23  7:45 ` Sai Krishna Potthuri
  2026-03-23 13:24   ` Rob Herring (Arm)
  3 siblings, 1 reply; 14+ messages in thread
From: Sai Krishna Potthuri @ 2026-03-23  7:45 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sa, Andy Shevchenko,
	Michal Simek, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-arm-kernel, linux-kernel,
	saikrishna12468, git, Sai Krishna Potthuri

Convert the xilinx-xadc.txt to DT schema and remove the old text binding.
Update xilinx-xadc binding path to YAML format in MAINTAINERS file.

The 'xlnx,channels' property is a container node(type: object) with
a vendor prefix, which triggers the below warning:
  "properties:xlnx,channels:type: 'boolean' was expected"

This cannot be changed because the original text binding specified
'xlnx,channels' since 2014, driver explicitly looks up "xlnx,channels"
using device_get_named_child_node() and removing the vendor prefix
would break backward compatibility with existing device trees.
Also, keep all vendor-prefixed properties from the original binding to
maintain backward compatibility with existing devicetrees and driver code.

Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
---
 .../bindings/iio/adc/xilinx-xadc.txt          | 141 ----------------
 .../bindings/iio/adc/xlnx,axi-xadc.yaml       | 154 ++++++++++++++++++
 2 files changed, 154 insertions(+), 141 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
 create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,axi-xadc.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
deleted file mode 100644
index f42e18078376..000000000000
--- a/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
+++ /dev/null
@@ -1,141 +0,0 @@
-Xilinx XADC device driver
-
-This binding document describes the bindings for the Xilinx 7 Series XADC as well
-as the UltraScale/UltraScale+ System Monitor.
-
-The Xilinx XADC is an ADC that can be found in the Series 7 FPGAs from Xilinx.
-The XADC has a DRP interface for communication. Currently two different
-frontends for the DRP interface exist. One that is only available on the ZYNQ
-family as a hardmacro in the SoC portion of the ZYNQ. The other one is available
-on all series 7 platforms and is a softmacro with a AXI interface. This binding
-document describes the bindings for both of them since the bindings are very
-similar.
-
-The Xilinx System Monitor is an ADC that is found in the UltraScale and
-UltraScale+ FPGAs from Xilinx. The System Monitor provides a DRP interface for
-communication. Xilinx provides a standard IP core that can be used to access the
-System Monitor through an AXI interface in the FPGA fabric. This IP core is
-called the Xilinx System Management Wizard. This document describes the bindings
-for this IP.
-
-Required properties:
-	- compatible: Should be one of
-		* "xlnx,zynq-xadc-1.00.a": When using the ZYNQ device
-		  configuration interface to interface to the XADC hardmacro.
-		* "xlnx,axi-xadc-1.00.a": When using the axi-xadc pcore to
-		  interface to the XADC hardmacro.
-		* "xlnx,system-management-wiz-1.3": When using the
-		  Xilinx System Management Wizard fabric IP core to access the
-		  UltraScale and UltraScale+ System Monitor.
-	- reg: Address and length of the register set for the device
-	- interrupts: Interrupt for the XADC control interface.
-	- clocks: When using the ZYNQ this must be the ZYNQ PCAP clock,
-	  when using the axi-xadc or the axi-system-management-wizard this must be
-	  the clock that provides the clock to the AXI bus interface of the core.
-
-Optional properties:
-	- xlnx,external-mux:
-		* "none": No external multiplexer is used, this is the default
-		  if the property is omitted.
-		* "single": External multiplexer mode is used with one
-		   multiplexer.
-		* "dual": External multiplexer mode is used with two
-		  multiplexers for simultaneous sampling.
-	- xlnx,external-mux-channel: Configures which pair of pins is used to
-	  sample data in external mux mode.
-	  Valid values for single external multiplexer mode are:
-		0: VP/VN
-		1: VAUXP[0]/VAUXN[0]
-		2: VAUXP[1]/VAUXN[1]
-		...
-		16: VAUXP[15]/VAUXN[15]
-	  Valid values for dual external multiplexer mode are:
-		1: VAUXP[0]/VAUXN[0] - VAUXP[8]/VAUXN[8]
-		2: VAUXP[1]/VAUXN[1] - VAUXP[9]/VAUXN[9]
-		...
-		8: VAUXP[7]/VAUXN[7] - VAUXP[15]/VAUXN[15]
-
-	  This property needs to be present if the device is configured for
-	  external multiplexer mode (either single or dual). If the device is
-	  not using external multiplexer mode the property is ignored.
-	- xnlx,channels: List of external channels that are connected to the ADC
-	  Required properties:
-		* #address-cells: Should be 1.
-		* #size-cells: Should be 0.
-
-	  The child nodes of this node represent the external channels which are
-	  connected to the ADC. If the property is no present no external
-	  channels will be assumed to be connected.
-
-	  Each child node represents one channel and has the following
-	  properties:
-		Required properties:
-			* reg: Pair of pins the channel is connected to.
-				0: VP/VN
-				1: VAUXP[0]/VAUXN[0]
-				2: VAUXP[1]/VAUXN[1]
-				...
-				16: VAUXP[15]/VAUXN[15]
-			  Note each channel number should only be used at most
-			  once.
-		Optional properties:
-			* xlnx,bipolar: If set the channel is used in bipolar
-			  mode.
-
-
-Examples:
-	xadc@f8007100 {
-		compatible = "xlnx,zynq-xadc-1.00.a";
-		reg = <0xf8007100 0x20>;
-		interrupts = <0 7 4>;
-		interrupt-parent = <&gic>;
-		clocks = <&pcap_clk>;
-
-		xlnx,channels {
-			#address-cells = <1>;
-			#size-cells = <0>;
-			channel@0 {
-				reg = <0>;
-			};
-			channel@1 {
-				reg = <1>;
-			};
-			channel@8 {
-				reg = <8>;
-			};
-		};
-	};
-
-	xadc@43200000 {
-		compatible = "xlnx,axi-xadc-1.00.a";
-		reg = <0x43200000 0x1000>;
-		interrupts = <0 53 4>;
-		interrupt-parent = <&gic>;
-		clocks = <&fpga1_clk>;
-
-		xlnx,channels {
-			#address-cells = <1>;
-			#size-cells = <0>;
-			channel@0 {
-				reg = <0>;
-				xlnx,bipolar;
-			};
-		};
-	};
-
-	adc@80000000 {
-		compatible = "xlnx,system-management-wiz-1.3";
-		reg = <0x80000000 0x1000>;
-		interrupts = <0 81 4>;
-		interrupt-parent = <&gic>;
-		clocks = <&fpga1_clk>;
-
-		xlnx,channels {
-			#address-cells = <1>;
-			#size-cells = <0>;
-			channel@0 {
-				reg = <0>;
-				xlnx,bipolar;
-			};
-		};
-	};
diff --git a/Documentation/devicetree/bindings/iio/adc/xlnx,axi-xadc.yaml b/Documentation/devicetree/bindings/iio/adc/xlnx,axi-xadc.yaml
new file mode 100644
index 000000000000..e384928d60eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/xlnx,axi-xadc.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/xlnx,axi-xadc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx XADC and System Monitor ADC
+
+description:
+  The Xilinx XADC is an ADC that can be found in the Series 7 FPGAs from
+  Xilinx. The XADC has a DRP interface for communication. Currently two
+  different frontends for the DRP interface exist. One that is only available
+  on the ZYNQ family as a hardmacro in the SoC portion of the ZYNQ. The other
+  one is available on all series 7 platforms and is a softmacro with an AXI
+  interface.
+
+  The Xilinx System Monitor is an ADC that is found in the UltraScale and
+  UltraScale+ FPGAs from Xilinx. The System Monitor provides a DRP interface
+  for communication. Xilinx provides a standard IP core that can be used to
+  access the System Monitor through an AXI interface in the FPGA fabric. This
+  IP core is called the Xilinx System Management Wizard.
+
+maintainers:
+  - Lars-Peter Clausen <lars@metafoo.de>
+  - Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
+
+properties:
+  compatible:
+    enum:
+      - xlnx,zynq-xadc-1.00.a
+      - xlnx,axi-xadc-1.00.a
+      - xlnx,system-management-wiz-1.3
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description:
+      When using the ZYNQ this must be the ZYNQ PCAP clock,
+      when using the axi-xadc or the axi-system-management-wizard this must be
+      the clock that provides the clock to the AXI bus interface of the core.
+
+  xlnx,external-mux:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - none
+      - single
+      - dual
+    default: none
+    description: |
+      External multiplexer configuration:
+      - "none": No external multiplexer is used
+      - "single": External multiplexer mode is used with one multiplexer
+      - "dual": External multiplexer mode is used with two multiplexers
+        for simultaneous sampling
+
+  xlnx,external-mux-channel:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 16
+    description: |
+      Configures which pair of pins is used to sample data in external mux mode.
+      Valid values for single external multiplexer mode are 0-16:
+      0: VP/VN
+      1-16: VAUXP[0-15]/VAUXN[0-15]
+      Valid values for dual external multiplexer mode are 1-8:
+      1-8: VAUXP[0-7]/VAUXN[0-7] - VAUXP[8-15]/VAUXN[8-15]
+      This property needs to be present if the device is configured for
+      external multiplexer mode (either single or dual).
+
+  xlnx,channels:
+    description: List of external channels that are connected to the ADC
+    type: object
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^channel@([0-9]|1[0-6])$":
+        type: object
+        properties:
+          reg:
+            minimum: 0
+            maximum: 16
+            description: |
+              Pair of pins the channel is connected to:
+                0: VP/VN
+                1-16: VAUXP[0-15]/VAUXN[0-15]
+              Note each channel number should only be used at most once.
+
+          xlnx,bipolar:
+            type: boolean
+            description: If set, the channel is used in bipolar mode
+
+        required:
+          - reg
+
+        additionalProperties: false
+
+    required:
+      - '#address-cells'
+      - '#size-cells'
+
+    additionalProperties: false
+
+allOf:
+  - if:
+      required:
+        - xlnx,external-mux
+      properties:
+        xlnx,external-mux:
+          enum:
+            - single
+            - dual
+    then:
+      required:
+        - xlnx,external-mux-channel
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    adc@f8007100 {
+        compatible = "xlnx,zynq-xadc-1.00.a";
+        reg = <0xf8007100 0x20>;
+        interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&pcap_clk>;
+
+        xlnx,channels {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            channel@0 {
+                reg = <0>;
+            };
+            channel@1 {
+                reg = <1>;
+            };
+            channel@8 {
+                reg = <8>;
+            };
+        };
+    };
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files
  2026-03-23  7:45 ` [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files Sai Krishna Potthuri
@ 2026-03-23 10:47   ` Andy Shevchenko
  2026-03-26  5:39   ` kernel test robot
  2026-03-27 18:58   ` kernel test robot
  2 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2026-03-23 10:47 UTC (permalink / raw)
  To: Sai Krishna Potthuri
  Cc: Jonathan Cameron, David Lechner, Nuno Sa, Andy Shevchenko,
	Michal Simek, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-iio, devicetree, linux-arm-kernel, linux-kernel,
	saikrishna12468, git

On Mon, Mar 23, 2026 at 01:15:02PM +0530, Sai Krishna Potthuri wrote:
> Split the xilinx-xadc-core.c into separate core and platform specific
> files to prepare for I2C interface support.
> 
> xilinx-xadc-core.c is reorganized as follows:
> xilinx-xadc-core.c:
>   - Platform-independent IIO/ADC operations
>   - Channel definitions and management
>   - Buffer and trigger management
>   - Device tree parsing
> 
> xilinx-xadc-platform.c:
>   - ZYNQ platform (FIFO-based) register access and interrupt handling
>   - AXI platform (memory-mapped) register access and interrupt handling
>   - Platform-specific setup and configuration
>   - Platform device probe function
> 
> Update Kconfig to introduce XILINX_XADC_CORE as a helper module selected
> by XILINX_XADC and update Makefile to build the split modules:
>   - xilinx-xadc-common.o (core + events)
>   - xilinx-xadc-platform.o (platform-specific)
> 
> Reorganized the code and No behavioral changes.

...

> +void xadc_write_reg(struct xadc *xadc, unsigned int reg, uint32_t val)
>  {
>  	writel(val, xadc->base + reg);
>  }
> +EXPORT_SYMBOL_GPL(xadc_write_reg);
>  
> -static void xadc_read_reg(struct xadc *xadc, unsigned int reg,
> -	uint32_t *val)
> +void xadc_read_reg(struct xadc *xadc, unsigned int reg, uint32_t *val)
>  {
>  	*val = readl(xadc->base + reg);
>  }
> +EXPORT_SYMBOL_GPL(xadc_read_reg);

Use namespace.

...

> -static int _xadc_update_adc_reg(struct xadc *xadc, unsigned int reg,
> -	uint16_t mask, uint16_t val)
> +static int _xadc_update_adc_reg(struct xadc *xadc, unsigned int reg, u16 mask, u16 val)

This is unrelated. Make it a separate change "Switch to use kernel types"
or alike.

...

> -static int xadc_update_scan_mode(struct iio_dev *indio_dev,
> -	const unsigned long *mask)
> +static int xadc_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *mask)

This is unrelated indentation change. Either drop or move to a separate patch.

...

>  	struct xadc *xadc = iio_priv(indio_dev);
> -	size_t n;
>  	void *data;
> +	size_t n;

Ditto.

...

>  static int xadc_trigger_set_state(struct iio_trigger *trigger, bool state)
>  {
>  	struct xadc *xadc = iio_trigger_get_drvdata(trigger);
> +	unsigned int convst, val;
>  	unsigned long flags;
> -	unsigned int convst;
> -	unsigned int val;
>  	int ret = 0;

This shouldn't be done at all, one variable per line is fine and readable.

...

>  		ret = _xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF0_EC,
> -					convst);
> +					   convst);

Separate patch for indentation.

...

> -static struct iio_trigger *xadc_alloc_trigger(struct iio_dev *indio_dev,
> -	const char *name)
> +static struct iio_trigger *xadc_alloc_trigger(struct iio_dev *indio_dev, const char *name)

Ditto and anything similar should go to a separate patch.

...


> -static int xadc_postdisable(struct iio_dev *indio_dev)
> +int xadc_postdisable(struct iio_dev *indio_dev)
>  {
>  	struct xadc *xadc = iio_priv(indio_dev);
>  	unsigned long scan_mask;
>  	int ret;

> -	int i;
> +	u32 i;

Why?

>  
>  	scan_mask = 1; /* Run calibration as part of the sequence */
>  	for (i = 0; i < indio_dev->num_channels; i++)

...

> +EXPORT_SYMBOL_GPL(xadc_postdisable);

Add namespace.

...

> +EXPORT_SYMBOL_GPL(xadc_read_samplerate);

Ditto and so on...

...

> -static int xadc_write_samplerate(struct xadc *xadc, int val)
> +int xadc_setup_buffer_and_triggers(struct device *dev, struct iio_dev *indio_dev,
> +				   struct xadc *xadc, int irq)

At this time do you need all three first parameters? I think it may be two or
even one.

> +{
> +	int ret;
> +
> +	if (!(xadc->ops->flags & XADC_FLAGS_BUFFERED))
> +		return 0;
> +
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> +					      &iio_pollfunc_store_time,
> +					      &xadc_trigger_handler,
> +					      &xadc_buffer_ops);
> +	if (ret)
> +		return ret;

> +	if (irq > 0) {

	/* The feature is optional */
	if (irq < 0)
		return 0;

> +		xadc->convst_trigger = xadc_alloc_trigger(indio_dev, "convst");
> +		if (IS_ERR(xadc->convst_trigger))
> +			return PTR_ERR(xadc->convst_trigger);
> +
> +		xadc->samplerate_trigger = xadc_alloc_trigger(indio_dev,
> +							      "samplerate");
> +		if (IS_ERR(xadc->samplerate_trigger))
> +			return PTR_ERR(xadc->samplerate_trigger);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(xadc_setup_buffer_and_triggers);

Namespace.

...

> -static const char * const xadc_type_names[] = {
> +const char * const xadc_type_names[] = {
>  	[XADC_TYPE_S7] = "xadc",
>  	[XADC_TYPE_US] = "xilinx-system-monitor",
>  };

Why this change without export? Is it fine?

...

> +	*ops = device_get_match_data(dev);

> +	if (!*ops)
> +		return ERR_PTR(-ENODEV);

Just drop it. we expect driver to have it.

...

> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>

> +#include <linux/kernel.h>

No driver should use this header (there might be rare exceptions, but not
here).

> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>

...

> +static const unsigned int XADC_ZYNQ_UNMASK_TIMEOUT = 500;

Unit suffix?

...

> +#define XADC_ZYNQ_CFG_CFIFOTH_MASK	(0xf << 20)

> +#define XADC_ZYNQ_CFG_DFIFOTH_MASK	(0xf << 16)

Yeah, these all _MASK (here and elsewhere) should use GENMASK().

...

> +static void xadc_zynq_write_fifo(struct xadc *xadc, uint32_t *cmd, unsigned int n)

Why not u32?

> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < n; i++)

Can be

	for (unsigned int i = 0; i < n; i++)

> +		xadc_write_reg(xadc, XADC_ZYNQ_REG_CFIFO, cmd[i]);
> +}

...

> +static void xadc_zynq_drain_fifo(struct xadc *xadc)
> +{
> +	u32 status, tmp;
> +
> +	xadc_read_reg(xadc, XADC_ZYNQ_REG_STATUS, &status);

	/* Needs a comment explaining why there will be no infinite loop */

> +	while (!(status & XADC_ZYNQ_STATUS_DFIFOE)) {
> +		xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &tmp);
> +		xadc_read_reg(xadc, XADC_ZYNQ_REG_STATUS, &status);
> +	}
> +}

...

> +static void xadc_zynq_update_intmsk(struct xadc *xadc, unsigned int mask, unsigned int val)
> +{
> +	xadc->zynq_intmask &= ~mask;
> +	xadc->zynq_intmask |= val;

Standard pattern is to have it on a single line

	xadc->zynq_intmask = (xadc->zynq_intmask & ~mask) | (val & mask);

> +	xadc_write_reg(xadc, XADC_ZYNQ_REG_INTMSK, xadc->zynq_intmask | xadc->zynq_masked_alarm);
> +}

...

> +static int xadc_zynq_write_adc_reg(struct xadc *xadc, unsigned int reg, uint16_t val)
> +{
> +	u32 cmd[1], tmp;
> +	int ret;
> +
> +	spin_lock_irq(&xadc->lock);

Are you going to use cleanup.h?

> +	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH, XADC_ZYNQ_INT_DFIFO_GTH);
> +
> +	reinit_completion(&xadc->completion);
> +
> +	cmd[0] = XADC_ZYNQ_CMD(XADC_ZYNQ_CMD_WRITE, reg, val);
> +	xadc_zynq_write_fifo(xadc, cmd, ARRAY_SIZE(cmd));
> +	xadc_read_reg(xadc, XADC_ZYNQ_REG_CFG, &tmp);
> +	tmp &= ~XADC_ZYNQ_CFG_DFIFOTH_MASK;
> +	tmp |= 0 << XADC_ZYNQ_CFG_DFIFOTH_OFFSET;
> +	xadc_write_reg(xadc, XADC_ZYNQ_REG_CFG, tmp);
> +
> +	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH, 0);
> +	spin_unlock_irq(&xadc->lock);
> +
> +	ret = wait_for_completion_interruptible_timeout(&xadc->completion, HZ);
> +	if (ret == 0)
> +		ret = -EIO;
> +	else

> +		ret = 0;

This seems quite wrong. If you use interruptible version, why ignoring the
error codes?

> +	xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &tmp);
> +
> +	return ret;
> +}
> +
> +static int xadc_zynq_read_adc_reg(struct xadc *xadc, unsigned int reg, uint16_t *val)

u16

> +{
> +	u32 cmd[2], resp, tmp;
> +	int ret;
> +
> +	cmd[0] = XADC_ZYNQ_CMD(XADC_ZYNQ_CMD_READ, reg, 0);
> +	cmd[1] = XADC_ZYNQ_CMD(XADC_ZYNQ_CMD_NOP, 0, 0);
> +
> +	spin_lock_irq(&xadc->lock);
> +	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH, XADC_ZYNQ_INT_DFIFO_GTH);
> +	xadc_zynq_drain_fifo(xadc);
> +	reinit_completion(&xadc->completion);
> +
> +	xadc_zynq_write_fifo(xadc, cmd, ARRAY_SIZE(cmd));
> +	xadc_read_reg(xadc, XADC_ZYNQ_REG_CFG, &tmp);

> +	tmp &= ~XADC_ZYNQ_CFG_DFIFOTH_MASK;
> +	tmp |= 1 << XADC_ZYNQ_CFG_DFIFOTH_OFFSET;

Are you going to use bitfield.h?

> +	xadc_write_reg(xadc, XADC_ZYNQ_REG_CFG, tmp);
> +
> +	xadc_zynq_update_intmsk(xadc, XADC_ZYNQ_INT_DFIFO_GTH, 0);
> +	spin_unlock_irq(&xadc->lock);
> +	ret = wait_for_completion_interruptible_timeout(&xadc->completion, HZ);
> +	if (ret == 0)
> +		ret = -EIO;
> +	if (ret < 0)
> +		return ret;

As per above.

> +	xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &resp);
> +	xadc_read_reg(xadc, XADC_ZYNQ_REG_DFIFO, &resp);
> +
> +	*val = resp & 0xffff;

Unneeded mask.

> +	return 0;
> +}

> +static unsigned int xadc_zynq_transform_alarm(unsigned int alarm)
> +{
> +	return ((alarm & 0x80) >> 4) |
> +		((alarm & 0x78) << 1) |
> +		(alarm & 0x07);

One line. Also add a comment explaining this (perhaps with a reference to
datasheet).

> +}

...

> +#define XADC_ZYNQ_TCK_RATE_MAX 50000000
> +#define XADC_ZYNQ_IGAP_DEFAULT 20
> +#define XADC_ZYNQ_PCAP_RATE_MAX 200000000

Unit suffixes?

...

I stopped here. I think what you need is to clean up and modernize the driver
first (see my comments above) and only when it's ready, start splitting it.
The split itself can also be done in a few steps (you can try to prepare
patches with `... -M -C --histogram ...` to see when it will look better).

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support
  2026-03-23  7:45 ` [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support Sai Krishna Potthuri
@ 2026-03-23 10:52   ` Andy Shevchenko
  2026-03-23 11:31     ` Sai Krishna Potthuri
  2026-03-23 20:26   ` Jonathan Cameron
  1 sibling, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2026-03-23 10:52 UTC (permalink / raw)
  To: Sai Krishna Potthuri
  Cc: Jonathan Cameron, David Lechner, Nuno Sa, Andy Shevchenko,
	Michal Simek, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-iio, devicetree, linux-arm-kernel, linux-kernel,
	saikrishna12468, git

On Mon, Mar 23, 2026 at 01:15:04PM +0530, Sai Krishna Potthuri wrote:
> Add I2C interface support for Xilinx System Management Wizard IP along
> with the existing AXI memory-mapped interface. This support enables
> monitoring the voltage and temperature on UltraScale+ devices where the
> System Management Wizard is connected via I2C.
> 
> Key changes:
> - Implement 32-bit DRP(Dynamic Reconfiguration Port) packet format as per
>   Xilinx PG185 specification.
> - Add separate I2C probe with xadc_i2c_of_match_table to handle same
>   compatible string("xlnx,system-management-wiz-1.3") on I2C bus.
> - Implement delayed version of hardware initialization for I2C interface
>   to handle the case where System Management Wizard IP is not ready during
>   the I2C probe.
> - Add NULL checks for get_dclk_rate callback function in sampling rate
>   functions to support interfaces without clock control
> - Create separate iio_info structure(xadc_i2c_info) without event
>   callbacks for I2C devices
> - Add xadc_i2c_transaction() function to handle I2C read/write operations
> - Add XADC_TYPE_US_I2C type to distinguish I2C interface from AXI

...

> -	if (xadc->ops->type == XADC_TYPE_US)
> +	if (xadc->ops->type == XADC_TYPE_US ||
> +	    xadc->ops->type == XADC_TYPE_US_I2C)

Can be one line.

>  		return 0;

...

>  	/* UltraScale has only one ADC and supports only continuous mode */
> -	if (xadc->ops->type == XADC_TYPE_US)
> +	if (xadc->ops->type == XADC_TYPE_US ||
> +	    xadc->ops->type == XADC_TYPE_US_I2C)


Ditto.

>  		return XADC_CONF1_SEQ_CONTINUOUS;

...

>  int xadc_write_samplerate(struct xadc *xadc, int val)
>  {
> -	unsigned long clk_rate = xadc_get_dclk_rate(xadc);
> +	unsigned long clk_rate;
>  	unsigned int div;
>  
> +	if (!xadc->ops->get_dclk_rate)
> +		return -EOPNOTSUPP;

> +	clk_rate = xadc_get_dclk_rate(xadc);
> +

Unneeded blank line.

Also, don't you asked for options?

>  	if (!clk_rate)
>  		return -EINVAL;

...

> +	i2c_set_clientdata(client, indio_dev);

Is it used?

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support
  2026-03-23 10:52   ` Andy Shevchenko
@ 2026-03-23 11:31     ` Sai Krishna Potthuri
  2026-03-23 11:46       ` Andy Shevchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Sai Krishna Potthuri @ 2026-03-23 11:31 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, David Lechner, Nuno Sa, Andy Shevchenko,
	Simek, Michal, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, saikrishna12468@gmail.com,
	git (AMD-Xilinx)

Hi Andy Shevchenko,

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
> Sent: Monday, March 23, 2026 4:22 PM
> To: Potthuri, Sai Krishna <sai.krishna.potthuri@amd.com>
> Cc: Jonathan Cameron <jic23@kernel.org>; David Lechner
> <dlechner@baylibre.com>; Nuno Sa <nuno.sa@analog.com>; Andy
> Shevchenko <andy@kernel.org>; Simek, Michal <michal.simek@amd.com>;
> Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
> Conor Dooley <conor+dt@kernel.org>; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; saikrishna12468@gmail.com; git (AMD-Xilinx)
> <git@amd.com>
> Subject: Re: [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support
> 
> On Mon, Mar 23, 2026 at 01:15:04PM +0530, Sai Krishna Potthuri wrote:
> > Add I2C interface support for Xilinx System Management Wizard IP along
> > with the existing AXI memory-mapped interface. This support enables
> > monitoring the voltage and temperature on UltraScale+ devices where
> > the System Management Wizard is connected via I2C.
> >
> > Key changes:
> > - Implement 32-bit DRP(Dynamic Reconfiguration Port) packet format as per
> >   Xilinx PG185 specification.
> > - Add separate I2C probe with xadc_i2c_of_match_table to handle same
> >   compatible string("xlnx,system-management-wiz-1.3") on I2C bus.
> > - Implement delayed version of hardware initialization for I2C interface
> >   to handle the case where System Management Wizard IP is not ready
> during
> >   the I2C probe.
> > - Add NULL checks for get_dclk_rate callback function in sampling rate
> >   functions to support interfaces without clock control
> > - Create separate iio_info structure(xadc_i2c_info) without event
> >   callbacks for I2C devices
> > - Add xadc_i2c_transaction() function to handle I2C read/write
> > operations
> > - Add XADC_TYPE_US_I2C type to distinguish I2C interface from AXI
> 
> ...
> 
> > -	if (xadc->ops->type == XADC_TYPE_US)
> > +	if (xadc->ops->type == XADC_TYPE_US ||
> > +	    xadc->ops->type == XADC_TYPE_US_I2C)
> 
> Can be one line.
> 
> >  		return 0;
> 
> ...
> 
> >  	/* UltraScale has only one ADC and supports only continuous mode
> */
> > -	if (xadc->ops->type == XADC_TYPE_US)
> > +	if (xadc->ops->type == XADC_TYPE_US ||
> > +	    xadc->ops->type == XADC_TYPE_US_I2C)
> 
> 
> Ditto.
> 
> >  		return XADC_CONF1_SEQ_CONTINUOUS;
> 
> ...
> 
> >  int xadc_write_samplerate(struct xadc *xadc, int val)  {
> > -	unsigned long clk_rate = xadc_get_dclk_rate(xadc);
> > +	unsigned long clk_rate;
> >  	unsigned int div;
> >
> > +	if (!xadc->ops->get_dclk_rate)
> > +		return -EOPNOTSUPP;
> 
> > +	clk_rate = xadc_get_dclk_rate(xadc);
> > +
> 
> Unneeded blank line.
> 
> Also, don't you asked for options?
This callback is defined for all other platforms except I2C.
Currently for I2c interface we are not supporting any configuration, it
is used only to read the channels.

> 
> >  	if (!clk_rate)
> >  		return -EINVAL;
> 
> ...
> 
> > +	i2c_set_clientdata(client, indio_dev);
> 
> Is it used?
No, will remove this one.

Regards
Sai Krishna


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support
  2026-03-23 11:31     ` Sai Krishna Potthuri
@ 2026-03-23 11:46       ` Andy Shevchenko
  2026-03-23 12:16         ` Sai Krishna Potthuri
  0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2026-03-23 11:46 UTC (permalink / raw)
  To: Sai Krishna Potthuri
  Cc: Andy Shevchenko, Jonathan Cameron, David Lechner, Nuno Sa,
	Andy Shevchenko, Simek, Michal, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, saikrishna12468@gmail.com,
	git (AMD-Xilinx)

On Mon, Mar 23, 2026 at 1:32 PM Sai Krishna Potthuri
<sai.krishna.potthuri@amd.com> wrote:
> > -----Original Message-----
> > From: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Sent: Monday, March 23, 2026 4:22 PM
> > On Mon, Mar 23, 2026 at 01:15:04PM +0530, Sai Krishna Potthuri wrote:

...

> > >  int xadc_write_samplerate(struct xadc *xadc, int val)  {
> > > -   unsigned long clk_rate = xadc_get_dclk_rate(xadc);
> > > +   unsigned long clk_rate;
> > >     unsigned int div;
> > >
> > > +   if (!xadc->ops->get_dclk_rate)
> > > +           return -EOPNOTSUPP;
> >
> > > +   clk_rate = xadc_get_dclk_rate(xadc);
> > > +
> >
> > Unneeded blank line.
> >
> > Also, don't you asked for options?
> This callback is defined for all other platforms except I2C.
> Currently for I2c interface we are not supporting any configuration, it
> is used only to read the channels.

If the callback defined you should use its value and not hardcoded one, right?

> > >     if (!clk_rate)
> > >             return -EINVAL;

-- 
With Best Regards,
Andy Shevchenko


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support
  2026-03-23 11:46       ` Andy Shevchenko
@ 2026-03-23 12:16         ` Sai Krishna Potthuri
  0 siblings, 0 replies; 14+ messages in thread
From: Sai Krishna Potthuri @ 2026-03-23 12:16 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Jonathan Cameron, David Lechner, Nuno Sa,
	Andy Shevchenko, Simek, Michal, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, saikrishna12468@gmail.com,
	git (AMD-Xilinx)

Hi Andy Shevchenko,

On 3/23/2026 5:16 PM, Andy Shevchenko wrote:
> On Mon, Mar 23, 2026 at 1:32 PM Sai Krishna Potthuri
> <sai.krishna.potthuri@amd.com> wrote:
>>> -----Original Message-----
>>> From: Andy Shevchenko <andriy.shevchenko@intel.com>
>>> Sent: Monday, March 23, 2026 4:22 PM
>>> On Mon, Mar 23, 2026 at 01:15:04PM +0530, Sai Krishna Potthuri wrote:
> 
> ...
> 
>>>>   int xadc_write_samplerate(struct xadc *xadc, int val)  {
>>>> -   unsigned long clk_rate = xadc_get_dclk_rate(xadc);
>>>> +   unsigned long clk_rate;
>>>>      unsigned int div;
>>>>
>>>> +   if (!xadc->ops->get_dclk_rate)
>>>> +           return -EOPNOTSUPP;
>>>
>>>> +   clk_rate = xadc_get_dclk_rate(xadc);
>>>> +
>>>
>>> Unneeded blank line.
>>>
>>> Also, don't you asked for options?
>> This callback is defined for all other platforms except I2C.
>> Currently for I2c interface we are not supporting any configuration, it
>> is used only to read the channels.
> 
> If the callback defined you should use its value and not hardcoded one, right?

Yes, that's the intention.
xadc_get_dclk_rate() - this is a wrapper function to call
xadc->ops->get_dclk_rate(). May be i will remove the wrapper function or
move the if (!xadc->ops->get_dclk_rate) check inside wrapper.

Regards
Sai Krishna
> 
>>>>      if (!clk_rate)
>>>>              return -EINVAL;
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/4] dt-bindings: iio: adc: xlnx,axi-xadc: convert to DT schema
  2026-03-23  7:45 ` [PATCH v2 4/4] dt-bindings: iio: adc: xlnx,axi-xadc: convert to DT schema Sai Krishna Potthuri
@ 2026-03-23 13:24   ` Rob Herring (Arm)
  0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring (Arm) @ 2026-03-23 13:24 UTC (permalink / raw)
  To: Sai Krishna Potthuri
  Cc: linux-arm-kernel, linux-iio, Michal Simek, Nuno Sa, Conor Dooley,
	linux-kernel, saikrishna12468, Krzysztof Kozlowski, git,
	David Lechner, Jonathan Cameron, Andy Shevchenko, devicetree


On Mon, 23 Mar 2026 13:15:05 +0530, Sai Krishna Potthuri wrote:
> Convert the xilinx-xadc.txt to DT schema and remove the old text binding.
> Update xilinx-xadc binding path to YAML format in MAINTAINERS file.
> 
> The 'xlnx,channels' property is a container node(type: object) with
> a vendor prefix, which triggers the below warning:
>   "properties:xlnx,channels:type: 'boolean' was expected"
> 
> This cannot be changed because the original text binding specified
> 'xlnx,channels' since 2014, driver explicitly looks up "xlnx,channels"
> using device_get_named_child_node() and removing the vendor prefix
> would break backward compatibility with existing device trees.
> Also, keep all vendor-prefixed properties from the original binding to
> maintain backward compatibility with existing devicetrees and driver code.
> 
> Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
> ---
>  .../bindings/iio/adc/xilinx-xadc.txt          | 141 ----------------
>  .../bindings/iio/adc/xlnx,axi-xadc.yaml       | 154 ++++++++++++++++++
>  2 files changed, 154 insertions(+), 141 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,axi-xadc.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/xlnx,axi-xadc.yaml: properties:xlnx,channels:type: 'boolean' was expected
	hint: A vendor boolean property can use "type: boolean"
	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260323074505.3853353-5-sai.krishna.potthuri@amd.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support
  2026-03-23  7:45 ` [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support Sai Krishna Potthuri
  2026-03-23 10:52   ` Andy Shevchenko
@ 2026-03-23 20:26   ` Jonathan Cameron
  1 sibling, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2026-03-23 20:26 UTC (permalink / raw)
  To: Sai Krishna Potthuri
  Cc: David Lechner, Nuno Sa, Andy Shevchenko, Michal Simek,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
	devicetree, linux-arm-kernel, linux-kernel, saikrishna12468, git

On Mon, 23 Mar 2026 13:15:04 +0530
Sai Krishna Potthuri <sai.krishna.potthuri@amd.com> wrote:

> Add I2C interface support for Xilinx System Management Wizard IP along
> with the existing AXI memory-mapped interface. This support enables
> monitoring the voltage and temperature on UltraScale+ devices where the
> System Management Wizard is connected via I2C.
> 
> Key changes:
> - Implement 32-bit DRP(Dynamic Reconfiguration Port) packet format as per
>   Xilinx PG185 specification.
> - Add separate I2C probe with xadc_i2c_of_match_table to handle same
>   compatible string("xlnx,system-management-wiz-1.3") on I2C bus.
> - Implement delayed version of hardware initialization for I2C interface
>   to handle the case where System Management Wizard IP is not ready during
>   the I2C probe.
> - Add NULL checks for get_dclk_rate callback function in sampling rate
>   functions to support interfaces without clock control
> - Create separate iio_info structure(xadc_i2c_info) without event
>   callbacks for I2C devices
> - Add xadc_i2c_transaction() function to handle I2C read/write operations
> - Add XADC_TYPE_US_I2C type to distinguish I2C interface from AXI
> 
> Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
Hi.
A few minor things inline.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/Kconfig            |  15 ++
>  drivers/iio/adc/Makefile           |   1 +
>  drivers/iio/adc/xilinx-xadc-core.c |  28 +++-
>  drivers/iio/adc/xilinx-xadc-i2c.c  | 215 +++++++++++++++++++++++++++++
>  drivers/iio/adc/xilinx-xadc.h      |   1 +
>  5 files changed, 256 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/iio/adc/xilinx-xadc-i2c.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index a4a7556f4016..5a3956a5c086 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1767,6 +1767,21 @@ config XILINX_XADC
>  	  The driver can also be build as a module. If so, the module will be called
>  	  xilinx-xadc.
>  
> +config XILINX_XADC_I2C
> +	tristate "Xilinx System Management Wizard I2C Interface support"
> +	depends on I2C
> +	select XILINX_XADC_CORE
> +	help
> +	  Say yes here to allow accessing the System Management
> +	  Wizard on UltraScale+ devices via I2C.
> +
> +	  This provides voltage and temperature monitoring capabilities
> +	  through the same IIO sysfs interface, but using I2C communication
> +	  protocol.
> +
> +	  The driver can also be build as a module. If so, the module will be called

line is a little too long for Kconfig.

> +	  xilinx-xadc-i2c.
> +
>
> diff --git a/drivers/iio/adc/xilinx-xadc-i2c.c b/drivers/iio/adc/xilinx-xadc-i2c.c
> new file mode 100644
> index 000000000000..3d802b907260
> --- /dev/null
> +++ b/drivers/iio/adc/xilinx-xadc-i2c.c
> @@ -0,0 +1,215 @@

> +static int xadc_i2c_read_transaction(struct xadc *xadc, unsigned int reg, u16 *val)
> +{
> +	struct xadc_i2c *xadc_i2c = container_of(xadc, struct xadc_i2c, xadc);
> +	char write_buffer[XADC_I2C_WRITE_DATA_SIZE] = { 0 };
> +	struct i2c_client *client = xadc_i2c->client;
> +	char read_buffer[XADC_I2C_READ_DATA_SIZE];
> +	int ret;
> +
> +	write_buffer[2] = FIELD_GET(XADC_I2C_DRP_ADDR_MASK, reg);
> +	write_buffer[3] = XADC_I2C_INSTR_READ;
> +
> +	ret = i2c_master_send(client, write_buffer, XADC_I2C_WRITE_DATA_SIZE);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_master_recv(client, read_buffer, XADC_I2C_READ_DATA_SIZE);
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = FIELD_PREP(XADC_I2C_DRP_DATA0_MASK, read_buffer[0]) |
> +	       FIELD_PREP(XADC_I2C_DRP_DATA1_MASK, read_buffer[1]);
> +
> +	return 0;
> +}
> +
> +static int xadc_i2c_write_transaction(struct xadc *xadc, unsigned int reg, u16 val)
> +{
> +	struct xadc_i2c *xadc_i2c = container_of(xadc, struct xadc_i2c, xadc);
> +	struct i2c_client *client = xadc_i2c->client;
> +	char write_buffer[XADC_I2C_WRITE_DATA_SIZE];
> +	int ret;
> +
> +	write_buffer[0] = FIELD_GET(XADC_I2C_DRP_DATA0_MASK, val);
> +	write_buffer[1] = FIELD_GET(XADC_I2C_DRP_DATA1_MASK, val);

This is odd enough it might be useful to have some comments.  Why do
we need to write the value to two places for instance?

> +	write_buffer[2] = FIELD_GET(XADC_I2C_DRP_ADDR_MASK, reg);
> +	write_buffer[3] = XADC_I2C_INSTR_WRITE;
> +
> +	ret = i2c_master_send(client, write_buffer, XADC_I2C_WRITE_DATA_SIZE);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int xadc_hardware_init(struct xadc *xadc)
> +{
> +	struct xadc_i2c *xadc_i2c = container_of(xadc, struct xadc_i2c, xadc);
> +	int ret;
> +	u32 i;
> +
> +	for (i = 0; i < ARRAY_SIZE(xadc->threshold); i++) {
	for (u32 i = 0;

Though why a u32?  If size doesn't matter, convention is pretty much always
use a unsigned int for the iterator.

> +		ret = xadc_i2c_read_transaction(xadc, XADC_REG_THRESHOLD(i),
> +						&xadc->threshold[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = xadc_i2c_write_transaction(xadc, XADC_REG_CONF0, xadc_i2c->conf0);
> +	if (ret)
> +		return ret;
> +
> +	ret = xadc_i2c_write_transaction(xadc, XADC_REG_INPUT_MODE(0),
> +					 xadc_i2c->bipolar_mask);
> +	if (ret)
> +		return ret;
> +
> +	ret = xadc_i2c_write_transaction(xadc, XADC_REG_INPUT_MODE(1),
> +					 xadc_i2c->bipolar_mask >> XADC_INPUT_MODE_BITS);
> +	if (ret)
> +		return ret;
> +
> +	xadc_i2c->hw_initialized = true;
> +
> +	return 0;
> +}
> +
> +static int xadc_i2c_read_reg(struct xadc *xadc, unsigned int reg, u16 *val)
> +{
> +	struct xadc_i2c *xadc_i2c = container_of(xadc, struct xadc_i2c, xadc);
> +
> +	if (!xadc_i2c->hw_initialized) {
> +		int ret;
> +
> +		ret = xadc_hardware_init(xadc);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return xadc_i2c_read_transaction(xadc, reg, val);
> +}
> +
> +static int xadc_i2c_write_reg(struct xadc *xadc, unsigned int reg, u16 val)
> +{
> +	struct xadc_i2c *xadc_i2c = container_of(xadc, struct xadc_i2c, xadc);
> +
> +	if (!xadc_i2c->hw_initialized) {

Seems like this is always called once on first access?
If so just do it form probe and simplify the read/ write_reg() functions.

> +		int ret;
> +
> +		ret = xadc_hardware_init(xadc);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return xadc_i2c_write_transaction(xadc, reg, val);
> +}

> +static int xadc_i2c_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	unsigned int conf0, bipolar_mask;
> +	const struct xadc_ops *ops;
> +	struct iio_dev *indio_dev;
> +	struct xadc_i2c *xadc_i2c;
> +	struct xadc *xadc;
> +	int ret;
> +
> +	indio_dev = xadc_device_setup(dev, sizeof(*xadc_i2c), &ops);
> +	if (IS_ERR(indio_dev))
> +		return PTR_ERR(indio_dev);
> +
> +	xadc_i2c = iio_priv(indio_dev);
> +	xadc_i2c->client = client;
> +	xadc = &xadc_i2c->xadc;
> +	xadc->clk = NULL;
> +	xadc->ops = ops;
> +	mutex_init(&xadc->mutex);
For new code (feel free to update the other code in a separate patch).
	ret = devm_mutex_init(xadc->mutex);
	if (ret)
		return ret;

As gives a small amount of lock debugging infrastructure and is now
cheap to do.  The devm form didn't used to exist.

> +	spin_lock_init(&xadc->lock);
> +
> +	ret = xadc_device_configure(dev, indio_dev, 0, &conf0, &bipolar_mask);
> +	if (ret) {
> +		dev_err(dev, "Failed to setup the device: %d\n", ret);
> +		return ret;
		return dev_err_probe(dev, "Failed to setup the device.\n");
Which will pretty print the error and hide it if -EPROBEDEFER (because that should
be silent) or -ENOMEM (because memory allocation errors are very noisy anyway!)

> +	}
> +
> +	i2c_set_clientdata(client, indio_dev);
> +	xadc_i2c->conf0 = conf0;
> +	xadc_i2c->bipolar_mask = bipolar_mask;
> +	xadc_i2c->hw_initialized = false;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files
  2026-03-23  7:45 ` [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files Sai Krishna Potthuri
  2026-03-23 10:47   ` Andy Shevchenko
@ 2026-03-26  5:39   ` kernel test robot
  2026-03-27 18:58   ` kernel test robot
  2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2026-03-26  5:39 UTC (permalink / raw)
  To: Sai Krishna Potthuri, Jonathan Cameron, David Lechner, Nuno Sa,
	Andy Shevchenko, Michal Simek, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: llvm, oe-kbuild-all, linux-iio, devicetree, linux-arm-kernel,
	linux-kernel, saikrishna12468, git, Sai Krishna Potthuri

Hi Sai,

kernel test robot noticed the following build errors:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on robh/for-next linus/master v7.0-rc5 next-20260325]
[cannot apply to xilinx-xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sai-Krishna-Potthuri/iio-adc-xilinx-xadc-Split-driver-into-core-and-platform-files/20260326-024045
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20260323074505.3853353-2-sai.krishna.potthuri%40amd.com
patch subject: [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files
config: i386-buildonly-randconfig-002-20260326 (https://download.01.org/0day-ci/archive/20260326/202603261323.1EonYM3W-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260326/202603261323.1EonYM3W-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603261323.1EonYM3W-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "xadc_handle_events" [drivers/iio/adc/xilinx-xadc-platform.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files
  2026-03-23  7:45 ` [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files Sai Krishna Potthuri
  2026-03-23 10:47   ` Andy Shevchenko
  2026-03-26  5:39   ` kernel test robot
@ 2026-03-27 18:58   ` kernel test robot
  2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2026-03-27 18:58 UTC (permalink / raw)
  To: Sai Krishna Potthuri, Jonathan Cameron, David Lechner, Nuno Sa,
	Andy Shevchenko, Michal Simek, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: oe-kbuild-all, linux-iio, devicetree, linux-arm-kernel,
	linux-kernel, saikrishna12468, git, Sai Krishna Potthuri

Hi Sai,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on robh/for-next linus/master v7.0-rc5 next-20260326]
[cannot apply to xilinx-xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sai-Krishna-Potthuri/iio-adc-xilinx-xadc-Split-driver-into-core-and-platform-files/20260326-024045
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20260323074505.3853353-2-sai.krishna.potthuri%40amd.com
patch subject: [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files
config: hexagon-randconfig-r133-20260326 (https://download.01.org/0day-ci/archive/20260328/202603280238.N4wp7jaC-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 054e11d1a17e5ba88bb1a8ef32fad3346e80b186)
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260328/202603280238.N4wp7jaC-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603280238.N4wp7jaC-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/iio/adc/xilinx-xadc-core.c:764:12: sparse: sparse: symbol 'xadc_type_names' was not declared. Should it be static?

vim +/xadc_type_names +764 drivers/iio/adc/xilinx-xadc-core.c

   763	
 > 764	const char * const xadc_type_names[] = {
   765		[XADC_TYPE_S7] = "xadc",
   766		[XADC_TYPE_US] = "xilinx-system-monitor",
   767	};
   768	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-03-27 18:59 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-23  7:45 [PATCH v2 0/4] iio: adc: xilinx-xadc: Add I2C interface support for System Management Wizard Sai Krishna Potthuri
2026-03-23  7:45 ` [PATCH v2 1/4] iio: adc: xilinx-xadc: Split driver into core and platform files Sai Krishna Potthuri
2026-03-23 10:47   ` Andy Shevchenko
2026-03-26  5:39   ` kernel test robot
2026-03-27 18:58   ` kernel test robot
2026-03-23  7:45 ` [PATCH v2 2/4] iio: adc: xilinx-xadc: Add .setup_channels() to struct xadc_ops Sai Krishna Potthuri
2026-03-23  7:45 ` [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support Sai Krishna Potthuri
2026-03-23 10:52   ` Andy Shevchenko
2026-03-23 11:31     ` Sai Krishna Potthuri
2026-03-23 11:46       ` Andy Shevchenko
2026-03-23 12:16         ` Sai Krishna Potthuri
2026-03-23 20:26   ` Jonathan Cameron
2026-03-23  7:45 ` [PATCH v2 4/4] dt-bindings: iio: adc: xlnx,axi-xadc: convert to DT schema Sai Krishna Potthuri
2026-03-23 13:24   ` Rob Herring (Arm)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox