[PATCH v1 0/2] drm/xe/i2c: alerts and controller enabling modifications

[PATCH v1 0/2] drm/xe/i2c: alerts and controller enabling modifications

[Heikki Krogerus]

Hi,

This includes support for the SMBus alerts, and special handling for the
IC_ENABLE register.

Thanks,

Heikki Krogerus (2):
  drm/xe/i2c: Handler for SMBus Alerts
  drm/xe/mcu_i2c: Take over control of the controller enabling

 drivers/gpu/drm/xe/Makefile       |  1 +
 drivers/gpu/drm/xe/xe_i2c.c       | 86 ++++++++++++++++++++++++++++---
 drivers/gpu/drm/xe/xe_i2c.h       | 16 ++++++
 drivers/gpu/drm/xe/xe_i2c_smbus.c | 59 +++++++++++++++++++++
 4 files changed, 154 insertions(+), 8 deletions(-)
 create mode 100644 drivers/gpu/drm/xe/xe_i2c_smbus.c

-- 
2.50.1

[PATCH v1 1/2] drm/xe/i2c: Handler for SMBus Alerts

[Heikki Krogerus]

Some devices that are attached to the I2C controller use the
SMBus Alert signal for example to inform the host about
thermal events, so registering the default SMBus Alert
device device for them. The alert device makes sure that
the alert is processed and passed to the correct I2C client
driver.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/gpu/drm/xe/Makefile       |  1 +
 drivers/gpu/drm/xe/xe_i2c.c       | 21 +++++++----
 drivers/gpu/drm/xe/xe_i2c.h       | 15 ++++++++
 drivers/gpu/drm/xe/xe_i2c_smbus.c | 59 +++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/xe/xe_i2c_smbus.c

diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 8e7b146880f46..9c8af00cf37fd 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -153,6 +153,7 @@ xe-y += xe_bb.o \
 	xe_wopcm.o
 
 xe-$(CONFIG_I2C)	+= xe_i2c.o
+xe-$(CONFIG_I2C_SMBUS)	+= xe_i2c_smbus.o
 xe-$(CONFIG_DRM_XE_GPUSVM) += xe_svm.o
 xe-$(CONFIG_DRM_GPUSVM) += xe_userptr.o
 
diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
index 706783863d07d..732ad6ee05e9c 100644
--- a/drivers/gpu/drm/xe/xe_i2c.c
+++ b/drivers/gpu/drm/xe/xe_i2c.c
@@ -30,6 +30,7 @@
 #include "xe_device.h"
 #include "xe_i2c.h"
 #include "xe_mmio.h"
+#include "xe_platform_types.h"
 #include "xe_sriov.h"
 #include "xe_survivability_mode.h"
 
@@ -163,7 +164,7 @@ bool xe_i2c_present(struct xe_device *xe)
 
 static bool xe_i2c_irq_present(struct xe_device *xe)
 {
-	return xe->i2c && xe->i2c->adapter_irq;
+	return xe->i2c && (xe->i2c->adapter_irq || xe->i2c->smbus_alert);
 }
 
 /**
@@ -182,7 +183,10 @@ void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
 		return;
 
 	/* Forward interrupt to I2C adapter */
-	generic_handle_irq_safe(xe->i2c->adapter_irq);
+	if (xe->i2c->smbus_alert)
+		xe_i2c_handle_smbus_alert(xe->i2c);
+	else
+		generic_handle_irq_safe(xe->i2c->adapter_irq);
 
 	/* Deassert after I2C adapter clears the interrupt */
 	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_INTX_DISABLE);
@@ -232,6 +236,9 @@ static int xe_i2c_create_irq(struct xe_device *xe)
 	    xe_survivability_mode_is_boot_enabled(xe))
 		return 0;
 
+	if (xe->info.platform == XE_CRESCENTISLAND)
+		return xe_i2c_register_smbus_alert(i2c);
+
 	domain = irq_domain_create_linear(dev_fwnode(i2c->drm_dev), 1, &xe_i2c_irq_ops, NULL);
 	if (!domain)
 		return -ENOMEM;
@@ -244,6 +251,9 @@ static int xe_i2c_create_irq(struct xe_device *xe)
 
 static void xe_i2c_remove_irq(struct xe_i2c *i2c)
 {
+	if (i2c->smbus_alert)
+		i2c_unregister_device(i2c->smbus_alert);
+
 	if (!i2c->irqdomain)
 		return;
 
@@ -327,7 +337,6 @@ int xe_i2c_probe(struct xe_device *xe)
 {
 	struct device *drm_dev = xe->drm.dev;
 	struct xe_i2c_endpoint ep;
-	struct regmap *regmap;
 	struct xe_i2c *i2c;
 	int ret;
 
@@ -354,9 +363,9 @@ int xe_i2c_probe(struct xe_device *xe)
 	/* PCI PM isn't aware of this device, bring it up and match it with SGUnit state. */
 	xe_i2c_pm_resume(xe, true);
 
-	regmap = devm_regmap_init(drm_dev, NULL, i2c, &i2c_regmap_config);
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
+	i2c->regmap = devm_regmap_init(drm_dev, NULL, i2c, &i2c_regmap_config);
+	if (IS_ERR(i2c->regmap))
+		return PTR_ERR(i2c->regmap);
 
 	i2c->bus_notifier.notifier_call = xe_i2c_notifier;
 	ret = bus_register_notifier(&i2c_bus_type, &i2c->bus_notifier);
diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
index 425d8160835f4..fdbad51950423 100644
--- a/drivers/gpu/drm/xe/xe_i2c.h
+++ b/drivers/gpu/drm/xe/xe_i2c.h
@@ -3,6 +3,7 @@
 #define _XE_I2C_H_
 
 #include <linux/bits.h>
+#include <linux/errno.h>
 #include <linux/notifier.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
@@ -12,6 +13,7 @@ struct fwnode_handle;
 struct i2c_adapter;
 struct i2c_client;
 struct irq_domain;
+struct regmap;
 struct platform_device;
 struct xe_device;
 struct xe_mmio;
@@ -40,6 +42,8 @@ struct xe_i2c {
 
 	struct irq_domain *irqdomain;
 	int adapter_irq;
+	struct i2c_client *smbus_alert;
+	struct regmap *regmap;
 
 	struct xe_i2c_endpoint ep;
 	struct device *drm_dev;
@@ -65,4 +69,15 @@ static inline void xe_i2c_pm_suspend(struct xe_device *xe) { }
 static inline void xe_i2c_pm_resume(struct xe_device *xe, bool d3cold) { }
 #endif
 
+#if IS_ENABLED(CONFIG_I2C_SMBUS)
+void xe_i2c_handle_smbus_alert(struct xe_i2c *i2c);
+int xe_i2c_register_smbus_alert(struct xe_i2c *i2c);
+#else
+static inline void xe_i2c_handle_smbus_alert(struct xe_i2c *i2c) { }
+static inline int xe_i2c_register_smbus_alert(struct xe_i2c *i2c)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
 #endif
diff --git a/drivers/gpu/drm/xe/xe_i2c_smbus.c b/drivers/gpu/drm/xe/xe_i2c_smbus.c
new file mode 100644
index 0000000000000..185a6d2f0a508
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_i2c_smbus.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Intel Xe SMBus handling
+ *
+ * Copyright (C) 2026 Intel Corporation.
+ */
+
+#include <linux/i2c-smbus.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#include "xe_i2c.h"
+
+/**
+ * DOC: Handler for SMBus Alerts
+ *
+ * If the I2C controller on the platform supports the SMBus extensions, a
+ * dedicated SMBus alert device needs to be registered for it. The alert device
+ * handles the standard SMBus alert processing and forwarding of the alert to
+ * the correct I2C client driver. On most platforms the i2c client device that
+ * generates the alerts will be the AMC.
+ */
+
+#define DW_IC_SMBUS_INTR_STAT			0xc8
+#define DW_IC_CLR_SMBUS_INTR			0xd4
+
+#define DW_IC_SMBUS_INTR_ALERT			BIT(10)
+
+void xe_i2c_handle_smbus_alert(struct xe_i2c *i2c)
+{
+	u32 stat;
+
+	if (!i2c->smbus_alert)
+		return;
+
+	regmap_read(i2c->regmap, DW_IC_SMBUS_INTR_STAT, &stat);
+	if (!stat)
+		return;
+
+	regmap_write(i2c->regmap, DW_IC_CLR_SMBUS_INTR, stat);
+
+	if (stat & DW_IC_SMBUS_INTR_ALERT)
+		i2c_handle_smbus_alert(i2c->smbus_alert);
+}
+
+static struct i2c_smbus_alert_setup xe_i2c_smbus_setup;
+
+int xe_i2c_register_smbus_alert(struct xe_i2c *i2c)
+{
+	struct i2c_client *alert;
+
+	alert = i2c_new_smbus_alert_device(i2c->adapter, &xe_i2c_smbus_setup);
+	if (IS_ERR(alert))
+		return PTR_ERR(alert);
+
+	i2c->smbus_alert = alert;
+
+	return 0;
+}
-- 
2.50.1

Re: [PATCH v1 1/2] drm/xe/i2c: Handler for SMBus Alerts

[Andy Shevchenko]

On Mon, Jun 22, 2026 at 01:47:58PM +0200, Heikki Krogerus wrote:
> Some devices that are attached to the I2C controller use the
> SMBus Alert signal for example to inform the host about
> thermal events, so registering the default SMBus Alert
> device device for them. The alert device makes sure that
> the alert is processed and passed to the correct I2C client
> driver.

...

> +	if (i2c->smbus_alert)

Unneeded. Also the below incorporates error pointer cases.

> +		i2c_unregister_device(i2c->smbus_alert);
> +
>  	if (!i2c->irqdomain)
>  		return;

...

> +++ b/drivers/gpu/drm/xe/xe_i2c_smbus.c
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Intel Xe SMBus handling
> + *
> + * Copyright (C) 2026 Intel Corporation.
> + */

+ bits.h // BIT()
+ err.h // IS_ERR()

> +#include <linux/i2c-smbus.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>

...

> +static struct i2c_smbus_alert_setup xe_i2c_smbus_setup;
> +
> +int xe_i2c_register_smbus_alert(struct xe_i2c *i2c)
> +{
> +	struct i2c_client *alert;
> +
> +	alert = i2c_new_smbus_alert_device(i2c->adapter, &xe_i2c_smbus_setup);
> +	if (IS_ERR(alert))
> +		return PTR_ERR(alert);
> +
> +	i2c->smbus_alert = alert;
> +
> +	return 0;
> +}

As per above comment, this IS_ERR() is not need, as the removal API is optional-aware.
OTOH, the other NULL-check in the driver in that case needs to be IS_ERR_OR_NULL().

But I'm not sure if you want to have alert setup to always succeed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] drm/xe/i2c: Handler for SMBus Alerts

[Heikki Krogerus]

On Tue, Jun 23, 2026 at 01:54:21PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 22, 2026 at 01:47:58PM +0200, Heikki Krogerus wrote:
> > Some devices that are attached to the I2C controller use the
> > SMBus Alert signal for example to inform the host about
> > thermal events, so registering the default SMBus Alert
> > device device for them. The alert device makes sure that
> > the alert is processed and passed to the correct I2C client
> > driver.
> 
> ...
> 
> > +	if (i2c->smbus_alert)
> 
> Unneeded. Also the below incorporates error pointer cases.
> 
> > +		i2c_unregister_device(i2c->smbus_alert);
> > +
> >  	if (!i2c->irqdomain)
> >  		return;
> 
> ...
> 
> > +++ b/drivers/gpu/drm/xe/xe_i2c_smbus.c
> > @@ -0,0 +1,59 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Intel Xe SMBus handling
> > + *
> > + * Copyright (C) 2026 Intel Corporation.
> > + */
> 
> + bits.h // BIT()
> + err.h // IS_ERR()
> 
> > +#include <linux/i2c-smbus.h>
> > +#include <linux/regmap.h>
> > +#include <linux/types.h>
> 
> ...
> 
> > +static struct i2c_smbus_alert_setup xe_i2c_smbus_setup;
> > +
> > +int xe_i2c_register_smbus_alert(struct xe_i2c *i2c)
> > +{
> > +	struct i2c_client *alert;
> > +
> > +	alert = i2c_new_smbus_alert_device(i2c->adapter, &xe_i2c_smbus_setup);
> > +	if (IS_ERR(alert))
> > +		return PTR_ERR(alert);
> > +
> > +	i2c->smbus_alert = alert;
> > +
> > +	return 0;
> > +}
> 
> As per above comment, this IS_ERR() is not need, as the removal API is optional-aware.
> OTOH, the other NULL-check in the driver in that case needs to be IS_ERR_OR_NULL().
> 
> But I'm not sure if you want to have alert setup to always succeed.

Thanks Andy. I'll get these fixed.

I have to refactor the code a little. It does not seem to be possible
to try to conditionally include this file based on CONFIG_I2C_SMBUS
like I'm doing now, so I'm going to propose that CONFIG_XE selects
CONFIG_I2C_SMBUS in v2. Since there are only two functions here, I'll
just move them to xe_i2c.c

cheers,

-- 
heikki

Re: [PATCH v1 1/2] drm/xe/i2c: Handler for SMBus Alerts

[Raag Jadav]

On Mon, Jun 22, 2026 at 01:47:58PM +0200, Heikki Krogerus wrote:
> Some devices that are attached to the I2C controller use the
> SMBus Alert signal for example to inform the host about
> thermal events, so registering the default SMBus Alert
> device device for them. The alert device makes sure that

Nit: Duplicate 'device' ;)

> the alert is processed and passed to the correct I2C client
> driver.

...

> @@ -182,7 +183,10 @@ void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
>  		return;
>  
>  	/* Forward interrupt to I2C adapter */
> -	generic_handle_irq_safe(xe->i2c->adapter_irq);
> +	if (xe->i2c->smbus_alert)
> +		xe_i2c_handle_smbus_alert(xe->i2c);
> +	else
> +		generic_handle_irq_safe(xe->i2c->adapter_irq);

This looks like the else case will never hit since no other platform
supports irq. Is it on the cards at some point or can we make it obsolete?

Raag

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

[Heikki Krogerus]

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.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/gpu/drm/xe/xe_i2c.c | 65 +++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/xe/xe_i2c.h |  1 +
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
index 732ad6ee05e9c..45fa9094b6142 100644
--- a/drivers/gpu/drm/xe/xe_i2c.c
+++ b/drivers/gpu/drm/xe/xe_i2c.c
@@ -8,6 +8,7 @@
 #include <drm/drm_print.h>
 #include <linux/array_size.h>
 #include <linux/container_of.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
@@ -261,11 +262,49 @@ static void xe_i2c_remove_irq(struct xe_i2c *i2c)
 	irq_domain_remove(i2c->irqdomain);
 }
 
+#define IC_CON				0x00
+#define IC_TAR				0x04
+#define IC_SAR				0x08
+#define IC_ENABLE			0x6c
+#define IC_ENABLE_STATUS		0x9c
+
+/* 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);
+	} while (timeout--);
+
+	return -ETIMEDOUT;
+}
+
 static int xe_i2c_read(void *context, unsigned int reg, unsigned int *val)
 {
 	struct xe_i2c *i2c = context;
 
-	*val = xe_mmio_read32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET));
+	switch (reg) {
+	case IC_ENABLE:
+		*val = i2c->ic_enable;
+		break;
+	case IC_ENABLE_STATUS:
+		*val = i2c->ic_enable & 1; /* NOTE: Checking only the enable bit */
+		break;
+	default:
+		*val = xe_mmio_read32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET));
+		break;
+	}
 
 	return 0;
 }
@@ -273,8 +312,30 @@ static int xe_i2c_read(void *context, unsigned int reg, unsigned int *val)
 static int xe_i2c_write(void *context, unsigned int reg, unsigned int val)
 {
 	struct xe_i2c *i2c = context;
+	int ret;
 
-	xe_mmio_write32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET), val);
+	switch (reg) {
+	case IC_CON:
+	case IC_TAR:
+	case IC_SAR:
+		/* Disable the controller. */
+		ret = xe_i2c_disable(i2c);
+		if (ret)
+			return ret;
+
+		/* Write the register. */
+		xe_mmio_write32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET), val);
+
+		/* Enable the controller. */
+		xe_mmio_rmw32(i2c->mmio, XE_REG(IC_ENABLE + I2C_MEM_SPACE_OFFSET), 0, 1);
+		break;
+	case IC_ENABLE:
+		i2c->ic_enable = val;
+		break;
+	default:
+		xe_mmio_write32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET), val);
+		break;
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
index fdbad51950423..e28279f0ebec7 100644
--- a/drivers/gpu/drm/xe/xe_i2c.h
+++ b/drivers/gpu/drm/xe/xe_i2c.h
@@ -36,6 +36,7 @@ struct xe_i2c {
 	struct platform_device *pdev;
 	struct i2c_adapter *adapter;
 	struct i2c_client *client[XE_I2C_MAX_CLIENTS];
+	unsigned int ic_enable;
 
 	struct notifier_block bus_notifier;
 	struct work_struct work;
-- 
2.50.1

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

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

[Raag Jadav]

On Tue, Jun 23, 2026 at 01:56:53PM +0300, Andy Shevchenko wrote:
> 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.

Perhaps something like i2c-algo-pca.h?

Raag

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

[Andy Shevchenko]

On Tue, Jun 23, 2026 at 04:58:39PM +0200, Raag Jadav wrote:
> On Tue, Jun 23, 2026 at 01:56:53PM +0300, Andy Shevchenko wrote:
> > On Mon, Jun 22, 2026 at 01:47:59PM +0200, Heikki Krogerus wrote:

...

> > > +#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.
> 
> Perhaps something like i2c-algo-pca.h?

It's different. It's more device specific header and it doesn't look like
a platform data. For SPI PXA2xx we have them in pxa2xx_ssp.h. And we have
some device specific i2c headers, seems like the pattern is
*_i2c.h, exempli gratia, designware_i2c.h.

-- 
With Best Regards,
Andy Shevchenko

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

[Raag Jadav]

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.

...

>  static int xe_i2c_read(void *context, unsigned int reg, unsigned int *val)
>  {
>  	struct xe_i2c *i2c = context;
>  
> -	*val = xe_mmio_read32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET));
> +	switch (reg) {

Curious, should I expect DW_IC_INTR_MASK case here which skips the MMIO?

> +	case IC_ENABLE:
> +		*val = i2c->ic_enable;
> +		break;
> +	case IC_ENABLE_STATUS:
> +		*val = i2c->ic_enable & 1; /* NOTE: Checking only the enable bit */
> +		break;
> +	default:
> +		*val = xe_mmio_read32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET));
> +		break;
> +	}
>  
>  	return 0;
>  }
> @@ -273,8 +312,30 @@ static int xe_i2c_read(void *context, unsigned int reg, unsigned int *val)
>  static int xe_i2c_write(void *context, unsigned int reg, unsigned int val)
>  {
>  	struct xe_i2c *i2c = context;
> +	int ret;
>  
> -	xe_mmio_write32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET), val);
> +	switch (reg) {

Ditto.

Raag

> +	case IC_CON:
> +	case IC_TAR:
> +	case IC_SAR:
> +		/* Disable the controller. */
> +		ret = xe_i2c_disable(i2c);
> +		if (ret)
> +			return ret;
> +
> +		/* Write the register. */
> +		xe_mmio_write32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET), val);
> +
> +		/* Enable the controller. */
> +		xe_mmio_rmw32(i2c->mmio, XE_REG(IC_ENABLE + I2C_MEM_SPACE_OFFSET), 0, 1);
> +		break;
> +	case IC_ENABLE:
> +		i2c->ic_enable = val;
> +		break;
> +	default:
> +		xe_mmio_write32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET), val);
> +		break;
> +	}
>  
>  	return 0;
>  }

Re: [PATCH v1 0/2] drm/xe/i2c: alerts and controller enabling modifications

[Heikki Krogerus]

Hi Raag,

I completely forgot the enum for the clients that you proposed
off-list. Sorry about that. I'll add it in v2.

On Mon, Jun 22, 2026 at 01:47:57PM +0200, Heikki Krogerus wrote:
> Hi,
> 
> This includes support for the SMBus alerts, and special handling for the
> IC_ENABLE register.
> 
> Thanks,
> 
> Heikki Krogerus (2):
>   drm/xe/i2c: Handler for SMBus Alerts
>   drm/xe/mcu_i2c: Take over control of the controller enabling
> 
>  drivers/gpu/drm/xe/Makefile       |  1 +
>  drivers/gpu/drm/xe/xe_i2c.c       | 86 ++++++++++++++++++++++++++++---
>  drivers/gpu/drm/xe/xe_i2c.h       | 16 ++++++
>  drivers/gpu/drm/xe/xe_i2c_smbus.c | 59 +++++++++++++++++++++
>  4 files changed, 154 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/gpu/drm/xe/xe_i2c_smbus.c

thanks,

-- 
heikki

Re: [PATCH v1 0/2] drm/xe/i2c: alerts and controller enabling modifications

[Raag Jadav]

On Tue, Jun 23, 2026 at 01:15:37PM +0300, Heikki Krogerus wrote:
> Hi Raag,
> 
> I completely forgot the enum for the clients that you proposed
> off-list. Sorry about that. I'll add it in v2.

Yeah, definitely need a vacation :D

Raag