Re: [PATCH v1 2/2] drm/xe/mcu_i2c: Take over control of the controller enabling

[Andy Shevchenko]

On Mon, Jun 22, 2026 at 01:47:59PM +0200, Heikki Krogerus wrote:
> Some platforms make an assumption that the i2c controller's
> enabled state indicates also the power state of the
> controller. This can create a problem when the controller is
> in disabled state, because the hardware may assume
> incorrectly that it is then also in low-power state.
> 
> To fix this, the controller is kept enabled by taking over
> the IC_ENABLE register. The controller has to be disabled
> when the configuration is updated and when the target
> address or the slave address are assigned, so disabling it
> when IC_CON, IC_TAR or IC_SAR registers are programmed, and
> then re-enabling it again.

...

> +#define IC_CON				0x00
> +#define IC_TAR				0x04
> +#define IC_SAR				0x08
> +#define IC_ENABLE			0x6c
> +#define IC_ENABLE_STATUS		0x9c

Heh, I would like to have a shared header with the registers, but dunno
if the prons will weight out the cons.

...

> +/* See "Disabling DW_apb_i2c" in the DesignWare DW_abp_i2c databook. */
> +static int xe_i2c_disable(struct xe_i2c *i2c)
> +{
> +	int timeout = 100;
> +	u32 status;
> +
> +	do {
> +		/* Disable.*/
> +		xe_mmio_rmw32(i2c->mmio, XE_REG(IC_ENABLE + I2C_MEM_SPACE_OFFSET), 1, 0);
> +
> +		/* Verify. */
> +		status = xe_mmio_read32(i2c->mmio, XE_REG(IC_ENABLE_STATUS + I2C_MEM_SPACE_OFFSET));
> +		if (!(status & 1))
> +			return 0;
> +
> +		/* Repeat. */
> +		usleep_range(25, 250);

fsleep().

> +	} while (timeout--);
> +
> +	return -ETIMEDOUT;
> +}

-- 
With Best Regards,
Andy Shevchenko