[PATCH v3 0/4] i2c: designware: Improve device disable handling

[PATCH v3 0/4] i2c: designware: Improve device disable handling

[William A. Kennington III]

When the I2C master attempts a new transaction while the slave
controller is shutting down or restarting, it can lead to bus lockups
and system bootloops if the hardware enters an inconsistent state.

This patch series addresses this by ensuring that the internal state
machines are properly cleared when disabling the controller if slave
activity is detected.

Additionally, it adds a shutdown hook that gracefully sets the slave
disable bit before disabling the controller. This guarantees that any
incoming requests from the master are immediately NACKed during
shutdown, preventing the bus from hanging.

--- 
Changes in v3:
- Split the monolithic patch into 4 logical patches
- Reverted stray formatting change in the PCI driver's dw_i2c_driver struct.
Changes in v2:
- Fix description footers
- Fix emails

Signed-off-by: William A. Kennington III <william@wkennington.com>
---
William A. Kennington III (4):
      i2c: designware: Introduce shutdown exported function
      i2c: designware: Convert PCI driver to use shutdown hook
      i2c: designware: Convert platform driver to use shutdown hook
      i2c: designware: Handle active slave cleanly

 drivers/i2c/busses/i2c-designware-common.c  | 32 +++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-designware-core.h    |  5 +++--
 drivers/i2c/busses/i2c-designware-master.c  | 31 +++++++++++++++++-----------
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 15 +++++++++++++-
 drivers/i2c/busses/i2c-designware-platdrv.c | 13 ++++++++++++
 5 files changed, 81 insertions(+), 15 deletions(-)
---
base-commit: 6d35786de28116ecf78797a62b84e6bf3c45aa5a
change-id: 20260504-dw-i2c-d5a1b1a0036a

Best regards,
-- 
William A. Kennington III <william@wkennington.com>

[PATCH v3 1/4] i2c: designware: Introduce shutdown exported function

[William A. Kennington III]

Introduce an exported shutdown function to safely shutdown the
DesignWare I2C controller.

This shutdown hook gracefully sets the slave disable bit before disabling
the controller. This guarantees that any incoming requests from the master
are immediately NACKed during shutdown, preventing the bus from hanging.

Signed-off-by: William A. Kennington III <william@wkennington.com>
---
 drivers/i2c/busses/i2c-designware-common.c | 24 ++++++++++++++++++++++++
 drivers/i2c/busses/i2c-designware-core.h   |  1 +
 2 files changed, 25 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4dc57fd56170..0703fb29038c 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -1028,5 +1028,29 @@ EXPORT_GPL_DEV_PM_OPS(i2c_dw_dev_pm_ops) = {
 	RUNTIME_PM_OPS(i2c_dw_runtime_suspend, i2c_dw_runtime_resume, NULL)
 };
 
+void i2c_dw_shutdown(struct dw_i2c_dev *dev)
+{
+	unsigned int con;
+
+	/*
+	 * We only need to handle shutdown for slave mode to ensure
+	 * we NACK any incoming master requests. Master mode cleanup
+	 * is handled after each transfer in i2c_dw_xfer.
+	 */
+	if (dev->mode != DW_IC_SLAVE)
+		return;
+
+	/*
+	 * To quickly NACK the master during shutdown, we set the slave
+	 * disable bit while the controller is still enabled.
+	 */
+	regmap_read(dev->map, DW_IC_CON, &con);
+	con |= DW_IC_CON_SLAVE_DISABLE;
+	regmap_write(dev->map, DW_IC_CON, con);
+
+	i2c_dw_disable(dev);
+}
+EXPORT_SYMBOL_GPL(i2c_dw_shutdown);
+
 MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
 MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 9d8d104cc391..8b422249acbd 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -393,6 +393,7 @@ static inline void __i2c_dw_read_intr_mask(struct dw_i2c_dev *dev,
 
 void __i2c_dw_disable(struct dw_i2c_dev *dev);
 void i2c_dw_disable(struct dw_i2c_dev *dev);
+void i2c_dw_shutdown(struct dw_i2c_dev *dev);
 
 extern void i2c_dw_configure_master(struct dw_i2c_dev *dev);
 extern int i2c_dw_probe_master(struct dw_i2c_dev *dev);

-- 
2.54.0.545.g6539524ca2-goog

[PATCH v3 2/4] i2c: designware: Convert PCI driver to use shutdown hook

[William A. Kennington III]

Convert the PCI driver to use the new i2c_dw_shutdown() hook, allowing
the controller to gracefully NACK master requests during system shutdown.

Signed-off-by: William A. Kennington III <william@wkennington.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index f21f9877c040..87074655c0e7 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -356,11 +356,24 @@ static const struct pci_device_id i2c_designware_pci_ids[] = {
 };
 MODULE_DEVICE_TABLE(pci, i2c_designware_pci_ids);
 
+static void i2c_dw_pci_shutdown(struct pci_dev *pdev)
+{
+	struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
+
+	if (!i_dev)
+		return;
+
+	pm_runtime_disable(&pdev->dev);
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		i2c_dw_shutdown(i_dev);
+}
+
 static struct pci_driver dw_i2c_driver = {
 	.name		= DRIVER_NAME,
 	.probe		= i2c_dw_pci_probe,
 	.remove		= i2c_dw_pci_remove,
-	.driver         = {
+	.shutdown	= i2c_dw_pci_shutdown,
+	.driver		= {
 		.pm	= pm_ptr(&i2c_dw_dev_pm_ops),
 	},
 	.id_table	= i2c_designware_pci_ids,

-- 
2.54.0.545.g6539524ca2-goog

[PATCH v3 3/4] i2c: designware: Convert platform driver to use shutdown hook

[William A. Kennington III]

Convert the platform driver to use the new i2c_dw_shutdown() hook,
allowing the controller to gracefully NACK master requests during
system shutdown.

Signed-off-by: William A. Kennington III <william@wkennington.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3351c4a9ef11..7ff7c1631e64 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -289,9 +289,22 @@ static const struct platform_device_id dw_i2c_platform_ids[] = {
 };
 MODULE_DEVICE_TABLE(platform, dw_i2c_platform_ids);
 
+static void dw_i2c_plat_shutdown(struct platform_device *pdev)
+{
+	struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
+
+	if (!i_dev)
+		return;
+
+	pm_runtime_disable(&pdev->dev);
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		i2c_dw_shutdown(i_dev);
+}
+
 static struct platform_driver dw_i2c_driver = {
 	.probe = dw_i2c_plat_probe,
 	.remove = dw_i2c_plat_remove,
+	.shutdown = dw_i2c_plat_shutdown,
 	.driver		= {
 		.name	= "i2c_designware",
 		.of_match_table = dw_i2c_of_match,

-- 
2.54.0.545.g6539524ca2-goog

[PATCH v3 4/4] i2c: designware: Handle active slave cleanly

[William A. Kennington III]

When the I2C master attempts a new transaction while the slave
controller is shutting down or restarting, it can lead to bus lockups
and system bootloops if the hardware enters an inconsistent state.

Address this by ensuring that the internal state machines are properly
cleared when disabling the controller if slave activity is detected.

If the controller remains active after disabling, perform a bus recovery
to reset it to a known good state.

Signed-off-by: William A. Kennington III <william@wkennington.com>
---
 drivers/i2c/busses/i2c-designware-common.c |  8 ++++++++
 drivers/i2c/busses/i2c-designware-core.h   |  4 ++--
 drivers/i2c/busses/i2c-designware-master.c | 31 ++++++++++++++++++------------
 3 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 0703fb29038c..31394d8fe612 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -633,6 +633,14 @@ void __i2c_dw_disable(struct dw_i2c_dev *dev)
 
 	abort_needed = (raw_intr_stats & DW_IC_INTR_MST_ON_HOLD) ||
 			(ic_stats & DW_IC_STATUS_MASTER_HOLD_TX_FIFO_EMPTY);
+
+	/*
+	 * If we are in slave mode and there is activity, we should also
+	 * trigger an abort to clear the internal state machines.
+	 */
+	if (dev->mode == DW_IC_SLAVE && (ic_stats & DW_IC_STATUS_SLAVE_ACTIVITY))
+		abort_needed = true;
+
 	if (abort_needed) {
 		if (!(enable & DW_IC_ENABLE_ENABLE)) {
 			regmap_write(dev->map, DW_IC_ENABLE, DW_IC_ENABLE_ENABLE);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 8b422249acbd..259b7ce16d0c 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -129,8 +129,8 @@
 #define DW_IC_STATUS_ACTIVITY			BIT(0)
 #define DW_IC_STATUS_TFE			BIT(2)
 #define DW_IC_STATUS_RFNE			BIT(3)
-#define DW_IC_STATUS_MASTER_ACTIVITY		BIT(5)
-#define DW_IC_STATUS_SLAVE_ACTIVITY		BIT(6)
+#define DW_IC_STATUS_MASTER_ACTIVITY	BIT(5)
+#define DW_IC_STATUS_SLAVE_ACTIVITY	BIT(6)
 #define DW_IC_STATUS_MASTER_HOLD_TX_FIFO_EMPTY	BIT(7)
 
 #define DW_IC_SDA_HOLD_RX_SHIFT			16
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index de929b91d5ea..7a301c8b604e 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -785,18 +785,25 @@ __i2c_dw_xfer_one_part(struct dw_i2c_dev *dev, struct i2c_msg *msgs, size_t num)
 	 * IC_RAW_INTR_STAT.MASTER_ON_HOLD holding SCL low. Check if
 	 * controller is still ACTIVE before disabling I2C.
 	 */
-	if (i2c_dw_is_controller_active(dev))
-		dev_err(dev->dev, "controller active\n");
-
-	/*
-	 * We must disable the adapter before returning and signaling the end
-	 * of the current transfer. Otherwise the hardware might continue
-	 * generating interrupts which in turn causes a race condition with
-	 * the following transfer. Needs some more investigation if the
-	 * additional interrupts are a hardware bug or this driver doesn't
-	 * handle them correctly yet.
-	 */
-	__i2c_dw_disable_nowait(dev);
+	if (i2c_dw_is_controller_active(dev)) {
+		/*
+		 * If the controller is still active after the timeout, attempt a
+		 * bus recovery to clear any potentially locked state.
+		 */
+		dev_err(dev->dev, "controller active after xfer, recovering\n");
+		i2c_recover_bus(&dev->adapter);
+		i2c_dw_init(dev);
+	} else {
+		/*
+		 * We must disable the adapter before returning and signaling the end
+		 * of the current transfer. Otherwise the hardware might continue
+		 * generating interrupts which in turn causes a race condition with
+		 * the following transfer. Needs some more investigation if the
+		 * additional interrupts are a hardware bug or this driver doesn't
+		 * handle them correctly yet.
+		 */
+		__i2c_dw_disable_nowait(dev);
+	}
 
 	if (dev->msg_err)
 		return dev->msg_err;

-- 
2.54.0.545.g6539524ca2-goog

Re: [PATCH v3 1/4] i2c: designware: Introduce shutdown exported function

[Andy Shevchenko]

On Mon, May 04, 2026 at 08:15:02PM +0000, William A. Kennington III wrote:
> Introduce an exported shutdown function to safely shutdown the
> DesignWare I2C controller.
> 
> This shutdown hook gracefully sets the slave disable bit before disabling
> the controller. This guarantees that any incoming requests from the master
> are immediately NACKed during shutdown, preventing the bus from hanging.

...

> +void i2c_dw_shutdown(struct dw_i2c_dev *dev)
> +{
> +	unsigned int con;
> +
> +	/*
> +	 * We only need to handle shutdown for slave mode to ensure
> +	 * we NACK any incoming master requests. Master mode cleanup
> +	 * is handled after each transfer in i2c_dw_xfer.

i2c_dw_xfer()

> +	 */

Since it's a newly added comment, can you switch to use inclusive language?
Same for the whole series related to the commit messages, comments, and
documentation.

> +	if (dev->mode != DW_IC_SLAVE)
> +		return;
> +
> +	/*
> +	 * To quickly NACK the master during shutdown, we set the slave
> +	 * disable bit while the controller is still enabled.
> +	 */
> +	regmap_read(dev->map, DW_IC_CON, &con);
> +	con |= DW_IC_CON_SLAVE_DISABLE;
> +	regmap_write(dev->map, DW_IC_CON, con);
> +
> +	i2c_dw_disable(dev);
> +}

> +EXPORT_SYMBOL_GPL(i2c_dw_shutdown);

Can we use namespace?

...

>  void __i2c_dw_disable(struct dw_i2c_dev *dev);
>  void i2c_dw_disable(struct dw_i2c_dev *dev);

> +void i2c_dw_shutdown(struct dw_i2c_dev *dev);

Isn't more tighten to the probe/remove than this?

>  extern void i2c_dw_configure_master(struct dw_i2c_dev *dev);
>  extern int i2c_dw_probe_master(struct dw_i2c_dev *dev);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/4] i2c: designware: Convert PCI driver to use shutdown hook

[Andy Shevchenko]

On Mon, May 04, 2026 at 08:15:03PM +0000, William A. Kennington III wrote:
> Convert the PCI driver to use the new i2c_dw_shutdown() hook, allowing
> the controller to gracefully NACK master requests during system shutdown.

...

> +static void i2c_dw_pci_shutdown(struct pci_dev *pdev)
> +{
> +	struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
> +
> +	if (!i_dev)
> +		return;

In long term this is bad style from maintenance perspective. Use

	struct dw_i2c_dev *i_dev;

	i_dev = pci_get_drvdata(pdev);
	if (!i_dev)
		return;

> +	pm_runtime_disable(&pdev->dev);
> +	if (!pm_runtime_status_suspended(&pdev->dev))
> +		i2c_dw_shutdown(i_dev);
> +}

...

> -	.driver         = {

> +	.driver		= {

Stray change.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/4] i2c: designware: Convert platform driver to use shutdown hook

[Andy Shevchenko]

On Mon, May 04, 2026 at 08:15:04PM +0000, William A. Kennington III wrote:
> Convert the platform driver to use the new i2c_dw_shutdown() hook,
> allowing the controller to gracefully NACK master requests during
> system shutdown.

...

> +static void dw_i2c_plat_shutdown(struct platform_device *pdev)
> +{
> +	struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
> +
> +	if (!i_dev)
> +		return;

Follow better style.

> +	pm_runtime_disable(&pdev->dev);
> +	if (!pm_runtime_status_suspended(&pdev->dev))
> +		i2c_dw_shutdown(i_dev);
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/4] i2c: designware: Handle active slave cleanly

[Andy Shevchenko]

On Mon, May 04, 2026 at 08:15:05PM +0000, William A. Kennington III wrote:
> When the I2C master attempts a new transaction while the slave
> controller is shutting down or restarting, it can lead to bus lockups
> and system bootloops if the hardware enters an inconsistent state.
> 
> Address this by ensuring that the internal state machines are properly
> cleared when disabling the controller if slave activity is detected.
> 
> If the controller remains active after disabling, perform a bus recovery
> to reset it to a known good state.

...

>  #define DW_IC_STATUS_ACTIVITY			BIT(0)
>  #define DW_IC_STATUS_TFE			BIT(2)
>  #define DW_IC_STATUS_RFNE			BIT(3)
> -#define DW_IC_STATUS_MASTER_ACTIVITY		BIT(5)
> -#define DW_IC_STATUS_SLAVE_ACTIVITY		BIT(6)
> +#define DW_IC_STATUS_MASTER_ACTIVITY	BIT(5)
> +#define DW_IC_STATUS_SLAVE_ACTIVITY	BIT(6)

If you wish to fix some indentation issues, combine these with the one from the
second patch and make it a separate change (as a last patch in the series).

>  #define DW_IC_STATUS_MASTER_HOLD_TX_FIFO_EMPTY	BIT(7)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/4] i2c: designware: Improve device disable handling

[Andy Shevchenko]

On Mon, May 04, 2026 at 08:15:01PM +0000, William A. Kennington III wrote:
> When the I2C master attempts a new transaction while the slave
> controller is shutting down or restarting, it can lead to bus lockups
> and system bootloops if the hardware enters an inconsistent state.
> 
> This patch series addresses this by ensuring that the internal state
> machines are properly cleared when disabling the controller if slave
> activity is detected.
> 
> Additionally, it adds a shutdown hook that gracefully sets the slave
> disable bit before disabling the controller. This guarantees that any
> incoming requests from the master are immediately NACKed during
> shutdown, preventing the bus from hanging.

In general series looks good, just some style issues, see individual replies.
If you address them as suggested, I will give my tag against v4.

-- 
With Best Regards,
Andy Shevchenko