mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/5] i3c: Fix firmware node refcounting and error paths
@ 2026-08-04 10:12 Akhil R
  2026-08-04 10:12 ` [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo Akhil R
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Akhil R @ 2026-08-04 10:12 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Adrian Hunter, Akhil R, linux-i3c,
	linux-kernel

These are fixes for problems that were noticed while reviewing the ACPI
and SETAASA support series.

The core fixes balance the reference counting of the firmware nodes of
the boardinfo, and keep a failed reattach from releasing an address that
the target may still respond to. The rest fix the quirk selection and the
unbind path of the DesignWare driver.

All the patches are verified by inspection and compile testing. None of
the error paths that they address were reproduced on hardware.

Akhil R (5):
  i3c: master: Release the fwnode of i2c boardinfo
  i3c: master: Fix refcount of i3c fwnode
  i3c: master: Do not release the addresses when reattach fails
  i3c: dw: Do not use OF match data as a quirk bitmask
  i3c: dw: Resume the controller before unregistering the bus

 drivers/i3c/master.c               | 18 ++++++++++++------
 drivers/i3c/master/dw-i3c-master.c | 21 ++++++++++++++++++---
 2 files changed, 30 insertions(+), 9 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo
  2026-08-04 10:12 [PATCH 0/5] i3c: Fix firmware node refcounting and error paths Akhil R
@ 2026-08-04 10:12 ` Akhil R
  2026-08-04 16:53   ` Adrian Hunter
  2026-08-04 10:12 ` [PATCH 2/5] i3c: master: Fix refcount of i3c fwnode Akhil R
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Akhil R @ 2026-08-04 10:12 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Adrian Hunter, Akhil R, linux-i3c,
	linux-kernel

i3c_master_add_i2c_boardinfo() takes a reference on the firmware node of
every i2c child described for the bus, but nothing ever drops it. The i2c
core takes a reference of its own for the client that it creates, so this
one is never consumed. The nodes stay pinned for as long as the system
runs, and more references are leaked on every rebind of the controller
driver and on every failed probe.

Drop the reference with a device managed action on the controller device,
next to where it is taken.

Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Reported-by: Adrian Hunter <adrian.hunter@intel.com>
Closes: https://lore.kernel.org/all/97465893-3650-4f37-98b7-1db76fc6fd03@intel.com/
Assisted-by: Cursor:claude-opus-5
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index f485b98805cf..08dc10f172aa 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2871,6 +2871,11 @@ static inline int i3c_acpi_add_i2c_boardinfo(struct i2c_dev_boardinfo *boardinfo
 }
 #endif
 
+static void i3c_master_put_boardinfo_fwnode(void *fwnode)
+{
+	fwnode_handle_put(fwnode);
+}
+
 static int
 i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
 			     struct fwnode_handle *fwnode, u32 *reg)
@@ -2913,7 +2918,8 @@ i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
 	list_add_tail(&boardinfo->node, &master->boardinfo.i2c);
 	fwnode_handle_get(fwnode);
 
-	return 0;
+	return devm_add_action_or_reset(dev, i3c_master_put_boardinfo_fwnode,
+					fwnode);
 }
 
 static int
-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/5] i3c: master: Fix refcount of i3c fwnode
  2026-08-04 10:12 [PATCH 0/5] i3c: Fix firmware node refcounting and error paths Akhil R
  2026-08-04 10:12 ` [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo Akhil R
@ 2026-08-04 10:12 ` Akhil R
  2026-08-04 16:53   ` Adrian Hunter
  2026-08-04 10:12 ` [PATCH 3/5] i3c: master: Do not release the addresses when reattach fails Akhil R
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Akhil R @ 2026-08-04 10:12 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Adrian Hunter, Akhil R, linux-i3c,
	linux-kernel

i3c_master_add_i3c_boardinfo() takes a reference on the firmware node of
every i3c child described for the bus and never drops it, while
i3c_device_release() drops one that the device never took, as
i3c_master_register_new_i3c_devs() assigns the node of the boardinfo as
is.

The two mistakes cancel out for a device that is registered exactly once,
which is why the imbalance goes unnoticed. Register it again, as happens
when the bus is re-initialised, and the count underflows, so the node may
be freed while the boardinfo still refers to it. A child that never joins
the bus leaks its reference instead.

Take a reference where the node is handed to the device, so that the one
the release callback drops is balanced, and drop the reference of the
boardinfo with a device managed action, as it is done for the i2c
boardinfo.

Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Reported-by: Adrian Hunter <adrian.hunter@intel.com>
Closes: https://lore.kernel.org/all/97465893-3650-4f37-98b7-1db76fc6fd03@intel.com/
Assisted-by: Cursor:claude-opus-5
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 08dc10f172aa..568788e4cdb5 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2102,7 +2102,8 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 				     desc->info.static_addr);
 
 		if (desc->boardinfo)
-			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
+			device_set_node(&desc->dev->dev,
+					fwnode_handle_get(desc->boardinfo->fwnode));
 
 		ret = device_register(&desc->dev->dev);
 		if (ret) {
@@ -2994,7 +2995,8 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
 	boardinfo->fwnode = fwnode_handle_get(fwnode);
 	list_add_tail(&boardinfo->node, &master->boardinfo.i3c);
 
-	return 0;
+	return devm_add_action_or_reset(dev, i3c_master_put_boardinfo_fwnode,
+					fwnode);
 }
 
 static int i3c_master_add_of_dev(struct i3c_master_controller *master,
-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/5] i3c: master: Do not release the addresses when reattach fails
  2026-08-04 10:12 [PATCH 0/5] i3c: Fix firmware node refcounting and error paths Akhil R
  2026-08-04 10:12 ` [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo Akhil R
  2026-08-04 10:12 ` [PATCH 2/5] i3c: master: Fix refcount of i3c fwnode Akhil R
@ 2026-08-04 10:12 ` Akhil R
  2026-08-04 17:56   ` Adrian Hunter
  2026-08-04 10:13 ` [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask Akhil R
  2026-08-04 10:13 ` [PATCH 5/5] i3c: dw: Resume the controller before unregistering the bus Akhil R
  4 siblings, 1 reply; 11+ messages in thread
From: Akhil R @ 2026-08-04 10:12 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Adrian Hunter, Akhil R, linux-i3c,
	linux-kernel
  Cc: Sashiko AI review

i3c_master_reattach_i3c_dev_locked() releases the address slots of the
device when the controller callback fails, even though the target keeps
responding to its dynamic address on the bus.

i3c_master_early_i3c_dev_add() cannot take that address back afterwards.
It jumps to err_rstdaa to reset the target, but i3c_master_rstdaa_locked()
only accepts an address that is marked as assigned. It rejects the request
with -EINVAL and sends no CCC, leaving the target responding to an address
that the core considers free and hands out during dynamic address
assignment.

i3c_master_reconcile_dyn_addrs() ignores the return value, so the device
stays attached and in the bus list at the address that SETNEWDA just
assigned to it, while the core is free to give the same address to another
device.

Leave the address slots to the callers. Those that cannot use the device
any longer detach it, which releases the slots in
i3c_master_detach_i3c_dev().

No in-tree controller fails its ->reattach_i3c_dev callback today, so
there is no known trigger for this. It is a robustness fix for the error
path only.

Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260721043058.C02B31F000E9@smtp.kernel.org/
Assisted-by: Cursor:claude-opus-5
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 568788e4cdb5..23557ca2df68 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1944,10 +1944,8 @@ int i3c_master_reattach_i3c_dev_locked(struct i3c_dev_desc *dev,
 
 	if (master->ops->reattach_i3c_dev) {
 		ret = master->ops->reattach_i3c_dev(dev, old_dyn_addr);
-		if (ret) {
-			i3c_master_put_i3c_addrs(dev);
+		if (ret)
 			return ret;
-		}
 	}
 
 	return 0;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask
  2026-08-04 10:12 [PATCH 0/5] i3c: Fix firmware node refcounting and error paths Akhil R
                   ` (2 preceding siblings ...)
  2026-08-04 10:12 ` [PATCH 3/5] i3c: master: Do not release the addresses when reattach fails Akhil R
@ 2026-08-04 10:13 ` Akhil R
  2026-08-05 19:53   ` Frank Li
  2026-08-04 10:13 ` [PATCH 5/5] i3c: dw: Resume the controller before unregistering the bus Akhil R
  4 siblings, 1 reply; 11+ messages in thread
From: Akhil R @ 2026-08-04 10:13 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Adrian Hunter, Akhil R, linux-i3c,
	linux-kernel
  Cc: Sashiko AI review, stable

dw_i3c_common_probe() takes the match data as a plain bitmask of quirks
whenever the device has an ACPI companion, and as a pointer to struct
dw_i3c_drvdata otherwise. The two are not interchangeable.

A device that is enumerated from ACPI through the PRP0001 device ID
matches the OF table, so device_get_match_data() returns the drvdata
pointer of the matched entry. Casting that pointer to unsigned long
enables whatever quirks happen to line up with its address bits, for
instance disabling runtime PM or skipping the clock and reset setup.

Look for a match in the ACPI table of the driver instead of merely
testing for an ACPI companion, so that the match data is only read as a
bitmask when it really came from that table. acpi_match_device() returns
NULL when the driver has no ACPI table and when CONFIG_ACPI is disabled,
which keeps the ast2600 driver that shares this probe on the device tree
path.

Fixes: fba0e56ee752 ("i3c: dw: Disable runtime PM on Agilex5 to avoid bus hang on IBI")
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260728071757.5B3CD1F000E9@smtp.kernel.org/
Cc: stable@vger.kernel.org
Assisted-by: Cursor:claude-opus-5
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master/dw-i3c-master.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 3816a50a52cc..17e1dd4fb5f3 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -5,6 +5,7 @@
  * Author: Vitor Soares <vitor.soares@synopsys.com>
  */
 
+#include <linux/acpi.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/cleanup.h>
@@ -1610,6 +1611,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 {
 	int ret, irq;
 	u32 thld_ctrl;
+	const struct acpi_device_id *acpi_id;
 	const struct dw_i3c_drvdata *drvdata;
 	unsigned long quirks = 0;
 
@@ -1618,9 +1620,10 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 
 	master->dev = &pdev->dev;
 
-	if (has_acpi_companion(&pdev->dev)) {
-		quirks = (unsigned long)device_get_match_data(&pdev->dev);
-	} else if (pdev->dev.of_node) {
+	acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
+	if (acpi_id) {
+		quirks = acpi_id->driver_data;
+	} else {
 		drvdata = device_get_match_data(&pdev->dev);
 		if (drvdata)
 			quirks = drvdata->flags;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 5/5] i3c: dw: Resume the controller before unregistering the bus
  2026-08-04 10:12 [PATCH 0/5] i3c: Fix firmware node refcounting and error paths Akhil R
                   ` (3 preceding siblings ...)
  2026-08-04 10:13 ` [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask Akhil R
@ 2026-08-04 10:13 ` Akhil R
  2026-08-05 19:57   ` Frank Li
  4 siblings, 1 reply; 11+ messages in thread
From: Akhil R @ 2026-08-04 10:13 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Adrian Hunter, Akhil R, linux-i3c,
	linux-kernel
  Cc: Sashiko AI review, stable

dw_i3c_common_remove() unregisters the I3C bus without resuming the
device first. i3c_master_unregister() ends up in
dw_i3c_master_bus_cleanup(), which writes DEVICE_CTRL, so unbinding the
driver while the controller is runtime suspended accesses registers with
the clocks gated and the reset asserted, which hangs or aborts on most
platforms.

Resume the device around the unregistration and drop the reference
before runtime PM is disabled, so the clocks are running for the
register accesses and the state stays balanced.

Fixes: 62fe9d06f570 ("i3c: dw: Add power management support")
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260728071757.5B3CD1F000E9@smtp.kernel.org/
Cc: stable@vger.kernel.org
Assisted-by: Cursor:claude-opus-5
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i3c/master/dw-i3c-master.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 17e1dd4fb5f3..df542ab7744b 100644
--- 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);
 
+	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)
 		pm_runtime_put_noidle(master->dev);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo
  2026-08-04 10:12 ` [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo Akhil R
@ 2026-08-04 16:53   ` Adrian Hunter
  0 siblings, 0 replies; 11+ messages in thread
From: Adrian Hunter @ 2026-08-04 16:53 UTC (permalink / raw)
  To: Akhil R, Alexandre Belloni, Frank Li, linux-i3c, linux-kernel

On 04/08/2026 13:12, Akhil R wrote:
> i3c_master_add_i2c_boardinfo() takes a reference on the firmware node of
> every i2c child described for the bus, but nothing ever drops it. The i2c
> core takes a reference of its own for the client that it creates, so this
> one is never consumed. The nodes stay pinned for as long as the system
> runs, and more references are leaked on every rebind of the controller
> driver and on every failed probe.
> 
> Drop the reference with a device managed action on the controller device,
> next to where it is taken.
> 
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Reported-by: Adrian Hunter <adrian.hunter@intel.com>
> Closes: https://lore.kernel.org/all/97465893-3650-4f37-98b7-1db76fc6fd03@intel.com/
> Assisted-by: Cursor:claude-opus-5
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>

Cosmetic suggestion below, nevertheless:

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/i3c/master.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index f485b98805cf..08dc10f172aa 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2871,6 +2871,11 @@ static inline int i3c_acpi_add_i2c_boardinfo(struct i2c_dev_boardinfo *boardinfo
>  }
>  #endif
>  
> +static void i3c_master_put_boardinfo_fwnode(void *fwnode)
> +{
> +	fwnode_handle_put(fwnode);
> +}
> +
>  static int
>  i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
>  			     struct fwnode_handle *fwnode, u32 *reg)
> @@ -2913,7 +2918,8 @@ i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
>  	list_add_tail(&boardinfo->node, &master->boardinfo.i2c);
>  	fwnode_handle_get(fwnode);
>  
> -	return 0;
> +	return devm_add_action_or_reset(dev, i3c_master_put_boardinfo_fwnode,
> +					fwnode);

I'd suggest wrapping at 100 cols rather than 80.

>  }
>  
>  static int


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/5] i3c: master: Fix refcount of i3c fwnode
  2026-08-04 10:12 ` [PATCH 2/5] i3c: master: Fix refcount of i3c fwnode Akhil R
@ 2026-08-04 16:53   ` Adrian Hunter
  0 siblings, 0 replies; 11+ messages in thread
From: Adrian Hunter @ 2026-08-04 16:53 UTC (permalink / raw)
  To: Akhil R, Alexandre Belloni, Frank Li, linux-i3c, linux-kernel

On 04/08/2026 13:12, Akhil R wrote:
> i3c_master_add_i3c_boardinfo() takes a reference on the firmware node of
> every i3c child described for the bus and never drops it, while
> i3c_device_release() drops one that the device never took, as
> i3c_master_register_new_i3c_devs() assigns the node of the boardinfo as
> is.
> 
> The two mistakes cancel out for a device that is registered exactly once,
> which is why the imbalance goes unnoticed. Register it again, as happens

I think it is (potentially repeated) registration failure path

> when the bus is re-initialised, and the count underflows, so the node may
> be freed while the boardinfo still refers to it. A child that never joins
> the bus leaks its reference instead.
> 
> Take a reference where the node is handed to the device, so that the one
> the release callback drops is balanced, and drop the reference of the
> boardinfo with a device managed action, as it is done for the i2c
> boardinfo.
> 
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Reported-by: Adrian Hunter <adrian.hunter@intel.com>
> Closes: https://lore.kernel.org/all/97465893-3650-4f37-98b7-1db76fc6fd03@intel.com/
> Assisted-by: Cursor:claude-opus-5
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>

Cosmetic suggestion below, nevertheless:

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/i3c/master.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 08dc10f172aa..568788e4cdb5 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2102,7 +2102,8 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>  				     desc->info.static_addr);
>  
>  		if (desc->boardinfo)
> -			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
> +			device_set_node(&desc->dev->dev,
> +					fwnode_handle_get(desc->boardinfo->fwnode));
>  
>  		ret = device_register(&desc->dev->dev);
>  		if (ret) {
> @@ -2994,7 +2995,8 @@ i3c_master_add_i3c_boardinfo(struct i3c_master_controller *master,
>  	boardinfo->fwnode = fwnode_handle_get(fwnode);
>  	list_add_tail(&boardinfo->node, &master->boardinfo.i3c);
>  
> -	return 0;
> +	return devm_add_action_or_reset(dev, i3c_master_put_boardinfo_fwnode,
> +					fwnode);

I'd suggest wrapping at 100 cols rather than 80.

>  }
>  
>  static int i3c_master_add_of_dev(struct i3c_master_controller *master,


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/5] i3c: master: Do not release the addresses when reattach fails
  2026-08-04 10:12 ` [PATCH 3/5] i3c: master: Do not release the addresses when reattach fails Akhil R
@ 2026-08-04 17:56   ` Adrian Hunter
  0 siblings, 0 replies; 11+ messages in thread
From: Adrian Hunter @ 2026-08-04 17:56 UTC (permalink / raw)
  To: Akhil R, Alexandre Belloni, Frank Li, linux-i3c, linux-kernel
  Cc: Sashiko AI review

On 04/08/2026 13:12, Akhil R wrote:
> i3c_master_reattach_i3c_dev_locked() releases the address slots of the
> device when the controller callback fails, even though the target keeps
> responding to its dynamic address on the bus.
> 
> i3c_master_early_i3c_dev_add() cannot take that address back afterwards.
> It jumps to err_rstdaa to reset the target, but i3c_master_rstdaa_locked()
> only accepts an address that is marked as assigned. It rejects the request
> with -EINVAL and sends no CCC, leaving the target responding to an address
> that the core considers free and hands out during dynamic address
> assignment.
> 
> i3c_master_reconcile_dyn_addrs() ignores the return value, so the device

i3c_master_reconcile_dyn_addrs() doesn't call i3c_master_reattach_i3c_dev_locked()
but __i3c_master_add_i3c_dev_locked() does.

> stays attached and in the bus list at the address that SETNEWDA just
> assigned to it, while the core is free to give the same address to another
> device.
> 
> Leave the address slots to the callers. Those that cannot use the device
> any longer detach it, which releases the slots in
> i3c_master_detach_i3c_dev().
> 
> No in-tree controller fails its ->reattach_i3c_dev callback today, so
> there is no known trigger for this. It is a robustness fix for the error
> path only.
> 
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/20260721043058.C02B31F000E9@smtp.kernel.org/
> Assisted-by: Cursor:claude-opus-5
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  drivers/i3c/master.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 568788e4cdb5..23557ca2df68 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1944,10 +1944,8 @@ int i3c_master_reattach_i3c_dev_locked(struct i3c_dev_desc *dev,
>  
>  	if (master->ops->reattach_i3c_dev) {
>  		ret = master->ops->reattach_i3c_dev(dev, old_dyn_addr);
> -		if (ret) {
> -			i3c_master_put_i3c_addrs(dev);
> +		if (ret)
>  			return ret;
> -		}
>  	}

That becomes just:

	if (master->ops->reattach_i3c_dev)
		return master->ops->reattach_i3c_dev(dev, old_dyn_addr);

and 'ret' goes away.

>  
>  	return 0;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask
  2026-08-04 10:13 ` [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask Akhil R
@ 2026-08-05 19:53   ` Frank Li
  0 siblings, 0 replies; 11+ messages in thread
From: Frank Li @ 2026-08-05 19:53 UTC (permalink / raw)
  To: Akhil R
  Cc: Alexandre Belloni, Frank Li, Adrian Hunter, linux-i3c,
	linux-kernel, Sashiko AI review, stable

On Tue, Aug 04, 2026 at 10:13:00AM +0000, Akhil R wrote:
> dw_i3c_common_probe() takes the match data as a plain bitmask of quirks
> whenever the device has an ACPI companion, and as a pointer to struct
> dw_i3c_drvdata otherwise. The two are not interchangeable.
> A device that is enumerated from ACPI through the PRP0001 device ID
> matches the OF table, so device_get_match_data() returns the drvdata
> pointer of the matched entry. Casting that pointer to unsigned long
> enables whatever quirks happen to line up with its address bits, for
> instance disabling runtime PM or skipping the clock and reset setup.
>
> Look for a match in the ACPI table of the driver instead of merely
> testing for an ACPI companion, so that the match data is only read as a
> bitmask when it really came from that table. acpi_match_device() returns
> NULL when the driver has no ACPI table and when CONFIG_ACPI is disabled,
> which keeps the ast2600 driver that shares this probe on the device tree
> path.
Suggest commit message

i3c: dw: use acpi_match_device() for ACPI-specific match data

The driver uses has_acpi_companion() to decide whether to obtain quirks
from ACPI match data. However, the presence of an ACPI companion does not
guarantee that the device was matched through the ACPI ID table.
 
When an ACPI device binds through the PRP0001 OF-compatibility mechanism,
device_get_match_data() returns a pointer to struct dw_i3c_drvdata. Since
has_acpi_companion() still returns true, the driver incorrectly casts that
pointer to an unsigned long quirk bitmask, corrupting the driver state.
 
Use acpi_match_device() instead of has_acpi_companion() to obtain ACPI
match data. Ensure quirk bits are only retrieved when the device actually
matches an ACPI entry. acpi_match_device() also returns NULL when the
driver has no ACPI match table or when CONFIG_ACPI is disabled, preserving
the existing device tree probe path used by the AST2600 driver.

Frank
>
> Fixes: fba0e56ee752 ("i3c: dw: Disable runtime PM on Agilex5 to avoid bus hang on IBI")
> Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/20260728071757.5B3CD1F000E9@smtp.kernel.org/
> Cc: stable@vger.kernel.org
> Assisted-by: Cursor:claude-opus-5
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  drivers/i3c/master/dw-i3c-master.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 3816a50a52cc..17e1dd4fb5f3 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -5,6 +5,7 @@
>   * Author: Vitor Soares <vitor.soares@synopsys.com>
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
>  #include <linux/cleanup.h>
> @@ -1610,6 +1611,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
>  {
>  	int ret, irq;
>  	u32 thld_ctrl;
> +	const struct acpi_device_id *acpi_id;
>  	const struct dw_i3c_drvdata *drvdata;
>  	unsigned long quirks = 0;
>
> @@ -1618,9 +1620,10 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
>
>  	master->dev = &pdev->dev;
>
> -	if (has_acpi_companion(&pdev->dev)) {
> -		quirks = (unsigned long)device_get_match_data(&pdev->dev);
> -	} else if (pdev->dev.of_node) {
> +	acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
> +	if (acpi_id) {
> +		quirks = acpi_id->driver_data;
> +	} else {
>  		drvdata = device_get_match_data(&pdev->dev);
>  		if (drvdata)
>  			quirks = drvdata->flags;
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 5/5] i3c: dw: Resume the controller before unregistering the bus
  2026-08-04 10:13 ` [PATCH 5/5] i3c: dw: Resume the controller before unregistering the bus Akhil R
@ 2026-08-05 19:57   ` Frank Li
  0 siblings, 0 replies; 11+ messages in thread
From: Frank Li @ 2026-08-05 19:57 UTC (permalink / raw)
  To: Akhil R
  Cc: Alexandre Belloni, Frank Li, Adrian Hunter, linux-i3c,
	linux-kernel, Sashiko AI review, stable

On Tue, Aug 04, 2026 at 10:13:01AM +0000, Akhil R wrote:
> dw_i3c_common_remove() unregisters the I3C bus without resuming the
> device first. i3c_master_unregister() ends up in
> dw_i3c_master_bus_cleanup(), which writes DEVICE_CTRL, so unbinding the
> driver while the controller is runtime suspended accesses registers with
> the clocks gated and the reset asserted, which hangs or aborts on most
> platforms.
>
> Resume the device around the unregistration and drop the reference
> before runtime PM is disabled, so the clocks are running for the
> register accesses and the state stays balanced.

common driver already support i3c_master_rpm_get()

Does it fix this problem by set rpm_allowed true?

Frank
>
> Fixes: 62fe9d06f570 ("i3c: dw: Add power management support")
> Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/20260728071757.5B3CD1F000E9@smtp.kernel.org/
> Cc: stable@vger.kernel.org
> Assisted-by: Cursor:claude-opus-5
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  drivers/i3c/master/dw-i3c-master.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 17e1dd4fb5f3..df542ab7744b 100644
> --- 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);
>
> +	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)
>  		pm_runtime_put_noidle(master->dev);
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-08-05 19:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-04 10:12 [PATCH 0/5] i3c: Fix firmware node refcounting and error paths Akhil R
2026-08-04 10:12 ` [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo Akhil R
2026-08-04 16:53   ` Adrian Hunter
2026-08-04 10:12 ` [PATCH 2/5] i3c: master: Fix refcount of i3c fwnode Akhil R
2026-08-04 16:53   ` Adrian Hunter
2026-08-04 10:12 ` [PATCH 3/5] i3c: master: Do not release the addresses when reattach fails Akhil R
2026-08-04 17:56   ` Adrian Hunter
2026-08-04 10:13 ` [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask Akhil R
2026-08-05 19:53   ` Frank Li
2026-08-04 10:13 ` [PATCH 5/5] i3c: dw: Resume the controller before unregistering the bus Akhil R
2026-08-05 19:57   ` Frank Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®