[PATCH 0/5] i3c: Fix firmware node refcounting and error paths

[PATCH 0/5] i3c: Fix firmware node refcounting and error paths

[Akhil R]

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


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

[PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo

[Akhil R]

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


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

Re: [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fixes a fwnode reference leak in `i3c_master_add_i2c_boardinfo()` but completely misses the identical leak in `i3c_master_add_i3c_boardinfo()`. Furthermore, there is a structural lifecycle flaw: `i3c_master_register_new_i3c_devs()` assigns the boardinfo's fwnode to the new I3C device without taking a reference, and `i3c_device_release()` unconditionally drops it. This means if one merely blindly copies the devm fix to `i3c_master_add_i3c_boardinfo()`, it would trigger a double-put UAF for instantiated devices. To fix the subsystem fully, both the devm action must be added and a `fwnode_handle_get()` must be taken in `i3c_master_register_new_i3c_devs()`.
--

--- Patch [1]: [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo ---
commit 9b7e7e5dd15b80d230fc0e52c4d7b8ef9dea16c9
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: master: Release the fwnode of i2c boardinfo

This commit fixes a fwnode reference leak in i3c_master_add_i2c_boardinfo().
It adds a devm action to automatically drop the firmware node reference taken
for every I2C child device when the controller device is unbound.

Closes: https://lore.kernel.org/all/97465893-3650-4f37-98b7-1db76fc6fd03@intel.com/

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index f485b98805cf6..08dc10f172aab 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -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);
>  }

[Severity: High]
This is a pre-existing issue, but while fixing the leak for I2C devices, it 
looks like the identical leak was left behind in the I3C counterpart, 
i3c_master_add_i3c_boardinfo().

However, would applying the same devm_add_action_or_reset() fix there
trigger a double-put use-after-free for instantiated I3C devices?

Looking at how the I3C devices are created, i3c_master_register_new_i3c_devs()
assigns the boardinfo fwnode to the new device without taking an additional
reference:

drivers/i3c/master.c:i3c_master_register_new_i3c_devs() {
    ...
    device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
    ...
}

And when the device is unregistered, i3c_device_release() unconditionally
drops it:

drivers/i3c/master.c:i3c_device_release() {
    ...
    fwnode_handle_put(dev->fwnode);
    ...
}

If a devm action is added to i3c_master_add_i3c_boardinfo() to drop the
reference on teardown, wouldn't we also need to add a fwnode_handle_get()
in i3c_master_register_new_i3c_devs() to ensure i3c_device_release()
doesn't consume the reference the devm action expects to drop?

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

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

Re: [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo

[Frank Li]

On Tue, Aug 04, 2026 at 10:33:05AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The patch fixes a fwnode reference leak in `i3c_master_add_i2c_boardinfo()` but completely misses the identical leak in `i3c_master_add_i3c_boardinfo()`. Furthermore, there is a structural lifecycle flaw: `i3c_master_register_new_i3c_devs()` assigns the boardinfo's fwnode to the new I3C device without taking a reference, and `i3c_device_release()` unconditionally drops it. This means if one merely blindly copies the devm fix to `i3c_master_add_i3c_boardinfo()`, it would trigger a double-put UAF for instantiated devices. To fix the subsystem fully, both the devm action must be added and a `fwnode_handle_get()` must be taken in `i3c_master_register_new_i3c_devs()`.
> --
>
> --- Patch [1]: [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo ---
> commit 9b7e7e5dd15b80d230fc0e52c4d7b8ef9dea16c9
> Author: Akhil R <akhilrajeev@nvidia.com>
>
> i3c: master: Release the fwnode of i2c boardinfo
>
> This commit fixes a fwnode reference leak in i3c_master_add_i2c_boardinfo().
> It adds a devm action to automatically drop the firmware node reference taken
> for every I2C child device when the controller device is unbound.
>
> Closes: https://lore.kernel.org/all/97465893-3650-4f37-98b7-1db76fc6fd03@intel.com/
>
> > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > index f485b98805cf6..08dc10f172aab 100644
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
> > @@ -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);
> >  }
>
> [Severity: High]
> This is a pre-existing issue, but while fixing the leak for I2C devices, it
> looks like the identical leak was left behind in the I3C counterpart,
> i3c_master_add_i3c_boardinfo().
>
> However, would applying the same devm_add_action_or_reset() fix there
> trigger a double-put use-after-free for instantiated I3C devices?
>
> Looking at how the I3C devices are created, i3c_master_register_new_i3c_devs()
> assigns the boardinfo fwnode to the new device without taking an additional
> reference:
>
> drivers/i3c/master.c:i3c_master_register_new_i3c_devs() {
>     ...
>     device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
>     ...
> }
>
> And when the device is unregistered, i3c_device_release() unconditionally
> drops it:
>
> drivers/i3c/master.c:i3c_device_release() {
>     ...
>     fwnode_handle_put(dev->fwnode);
>     ...
> }
>
> If a devm action is added to i3c_master_add_i3c_boardinfo() to drop the
> reference on teardown, wouldn't we also need to add a fwnode_handle_get()
> in i3c_master_register_new_i3c_devs() to ensure i3c_device_release()
> doesn't consume the reference the devm action expects to drop?

Make sense, please make i2c/i3c handle fwnode consisent.

Frank

>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260804101301.2975911-1-akhilrajeev@nvidia.com?part=1
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

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

Re: [PATCH 1/5] i3c: master: Release the fwnode of i2c boardinfo

[Adrian Hunter]

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


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

[PATCH 2/5] i3c: master: Fix refcount of i3c fwnode

[Akhil R]

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


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

Re: [PATCH 2/5] i3c: master: Fix refcount of i3c fwnode

[Adrian Hunter]

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,


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

[PATCH 3/5] i3c: master: Do not release the addresses when reattach fails

[Akhil R]

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


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

Re: [PATCH 3/5] i3c: master: Do not release the addresses when reattach fails

[Adrian Hunter]

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;


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

[PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask

[Akhil R]

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


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

Re: [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing error check for platform_get_irq() which breaks probe deferral.
--

--- Patch [4]: [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask ---
commit b46fa3977d02ec78544c1f987ffbf99b39c47f0d
Author: Akhil R <akhilrajeev@nvidia.com>

i3c: dw: Do not use OF match data as a quirk bitmask

This commit updates dw_i3c_common_probe() to explicitly check for a match
in the driver's ACPI table instead of merely testing for an ACPI companion.
This prevents OF match data from being incorrectly cast to a quirk bitmask.

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

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 3816a50a52cc5..17e1dd4fb5f3d 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c

[ ... ]

> @@ -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;

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing
dw_i3c_common_probe(), I noticed an existing issue with how the IRQ is
fetched later in the same function:

	irq = platform_get_irq(pdev, 0);
	ret = devm_request_irq(&pdev->dev, irq,
			       dw_i3c_master_irq_handler, 0,
			       dev_name(&pdev->dev), master);

Does this code fail to handle deferred probing properly?

If platform_get_irq() returns a negative error like -EPROBE_DEFER, it is
passed directly as the irq parameter to devm_request_irq(). Because the irq
parameter is an unsigned integer, the negative value is cast to a large
number. This causes request_threaded_irq() to reject it as out of bounds
and return -EINVAL.

Could this permanently break initialization on systems that require probe
deferral by masking the original deferral error code?

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

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

Re: [PATCH 4/5] i3c: dw: Do not use OF match data as a quirk bitmask

[Frank Li]

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
>

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

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

[Akhil R]

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


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

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

[sashiko-bot]

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

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

[Frank Li]

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
>

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