On Fri, Feb 20, 2026 at 11:09:40AM +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 xadc_i2c_transaction() function to handle I2C read/write operations
- Add XADC_TYPE_US_I2C type to distinguish I2C interface from AXI.

...

 #include <linux/device.h>
 #include <linux/err.h>
+#include <linux/i2c.h>

No, split driver to a few files:
_core.c
_platform.c
_i2c.c

The last two will be just a glue code.

...

+#ifdef CONFIG_XILINX_XADC_I2C

No way for ugly ifdeffery!

...

+static int xadc_i2c_transaction(struct xadc *xadc, unsigned int reg, uint16_t write_data,
+				bool is_write, uint16_t *read_data)

Should be simply u16.

...

+	char write_buffer[XADC_I2C_WRITE_DATA_SIZE] = { 0 };

No '0' is needed.

...

+	if (is_write) {

Bad design. Instead make two functions, one for read, one for write and drop
the boolean parameter.

+	} else {
+	}

+	/* Read response for read operations */
+	if (!is_write) {

+	}

...

+static int xadc_hardware_init(struct xadc *xadc)
+{
+	int ret, i;

Use unsigned iterator inside the loop.

+	for (i = 0; i < 16; i++) {

Magic 16.

+		ret = xadc_i2c_transaction(xadc, XADC_REG_THRESHOLD(i), 0, false,
+					   &xadc->threshold[i]);
+		if (ret)
+			return ret;
+	}
+
+	ret = xadc_i2c_transaction(xadc, XADC_REG_CONF0, xadc->conf0, true, NULL);
+	if (ret)
+		return ret;
+
+	ret = xadc_i2c_transaction(xadc, XADC_REG_INPUT_MODE(0), xadc->bipolar_mask, true, NULL);
+	if (ret)
+		return ret;
+
+	ret = xadc_i2c_transaction(xadc, XADC_REG_INPUT_MODE(1), xadc->bipolar_mask >> 16, true,
+				   NULL);
+	if (ret)
+		return ret;
+
+	xadc->hw_initialized = true;
+
+	return 0;
+}

...

+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

Leave trailing comma as it's not a sentinel.

+	},
+	{ },

The opposite. IIO has established way besides the fact that sentinel by
definition must be the last, trailing comma is confusing for a bare minimum.

+};

-- 
With Best Regards,
Andy Shevchenko