Re: [PATCH 5/5] i3c: dw: Resume the controller before unregistering the bus

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] If pm_runtime_resume_and_get() fails, the driver logs an error but proceeds to unregister the bus, which accesses hardware registers while clocks are gated, causing a system hang.
--

--- Patch [5]: [PATCH 5/5] i3c: dw: Resume the controller before unregistering the bus ---
commit 505f2f2b67429286e85c86a3aafae3b5a8a3a876
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: dw: Resume the controller before unregistering the bus

Unregistering the I3C bus without resuming the device accesses registers
with gated clocks, causing system hangs. This commit resumes the device
around unregistration to ensure the clocks are running for register accesses.

Closes: https://lore.kernel.org/all/20260728071757.5B3CD1F000E9@smtp.kernel.org/

--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1723,8 +1723,20 @@ EXPORT_SYMBOL_GPL(dw_i3c_common_probe);
 
 void dw_i3c_common_remove(struct dw_i3c_master *master)
 {
+	int ret;
+
+	/* Unregistering the bus accesses the controller registers */
+	ret = pm_runtime_resume_and_get(master->dev);
+	if (ret < 0)
+		dev_err(master->dev,
+			"<%s> cannot resume i3c bus master, err: %d\n",
+			__func__, ret);
+
 	i3c_master_unregister(&master->base);

[Severity: High]
This is a pre-existing issue, but if pm_runtime_resume_and_get() fails here,
will the driver still proceed to call i3c_master_unregister()?

If the resume fails (for example, due to a power domain or clock enablement
failure), the clocks remain gated. Calling i3c_master_unregister() will
unconditionally invoke the cleanup path:

dw_i3c_common_remove()
   i3c_master_unregister()
      i3c_master_bus_cleanup()
         dw_i3c_master_disable()

Inside dw_i3c_master_disable(), the code will attempt to read the DEVICE_CTRL
register while the hardware is still unclocked. 

Could this lead to the same system crash (such as a Data Abort) that this
patch is trying to prevent by accessing memory-mapped registers of the
power-gated hardware block?

> +	if (ret >= 0)
> +		pm_runtime_put_noidle(master->dev);
> +
>  	/* Balance pm_runtime_get_noresume() from probe() */
>  	if (master->quirks & DW_I3C_DISABLE_RUNTIME_PM_QUIRK)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804101301.2975911-1-akhilrajeev@nvidia.com?part=5

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c