Re: [PATCH v2 3/4] iio: adc: xilinx-xadc: Add I2C interface support

[Andy Shevchenko]

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