[PATCH V4 00/14] i3c: Support IBI-based system wakeup

[PATCH V4 00/14] i3c: Support IBI-based system wakeup

[Adrian Hunter]

Hi

Intel LPSS I3C controllers support up to two I3C busses and can wake the
system from an In-Band Interrupt (IBI) via a PCI PME.  Today that wakeup
capability lives only at the PCI function, with no way to express which
I3C device is actually responsible for waking the system, and no way for
user space to enable or disable wakeup on a per-device basis.

This series pushes the wakeup capability down to the individual I3C
devices and then aggregates the resulting wakeup state back up to the PCI
device.

An IBI-capable I3C device on a bus whose controller can wake the system
is marked as wakeup capable, so it can be managed through the standard
device wakeup framework (e.g. via power/wakeup in sysfs).  When such a
device is enabled for wakeup, a wakeup event is reported each time it
queues an IBI.

At suspend time, the mipi-i3c-hci PCI driver aggregates the wakeup
configuration of the I3C devices across its HCI instances (up to two I3C
busses) and enables PCI wakeup (PME) only when at least one attached I3C
device is enabled as a wakeup source and has IBI enabled.  This keeps
the PCI wakeup state in sync with the actual requirements of the devices
on the busses.

The series is organised as follows:

 - Patch 1 fixes a pre-existing recursive acquisition of the bus rwsem
   in i3c_master_register_new_i3c_devs().  The remaining patches add
   work to that same function, so the locking is corrected first.

 - Patches 2-7 fix use-after-free and unlocked accesses of the i3c_device
   desc pointer.  Moving device registration out from under the bus lock
   in patch 1 is what makes it possible to take the bus lock in the
   helpers that run during registration.

 - Patches 8-11 add the generic I3C core support: an ibi_wakeup flag for
   controllers, marking IBI-capable devices as wakeup capable, reporting
   wakeup events on IBIs, a helper to query whether any device on a bus
   has both wakeup and IBI enabled, and a fix to reject IBI requests
   from devices that do not advertise IBI capability.

 - Patches 12-14 wire this up for the mipi-i3c-hci driver: propagate the
   aggregated I3C wakeup requirements to the PCI function, factor out
   i3c_hci_sysdev() for the shared device lookup, and advertise IBI
   wakeup capability when the underlying system device can wake the
   system.

Note, since the PCI wakeup state is now derived from the wakeup
configuration of the attached I3C devices, the PCI device's
power/wakeup sysfs attribute no longer provides independent wakeup
control.


Changes in V4:

    i3c: master: Fix recursive locking during device registration
	Expanded the kernel-doc for the new struct i3c_device @node
	member to note that it is only for use by
	i3c_master_register_new_i3c_devs() and is not protected by a
	lock.

    i3c: master: Fix use-after-free of master->this
	Also reset master->this and bus.cur_master to NULL on the
	i3c_master_set_info() error path before freeing the allocated
	device.  Tidied up the commit message wording.

    i3c: master: Support IBI-based wakeup capability
	Shortened the comment about IBI-capable devices being wakeup
	capable.

    i3c: master: Add helper to query bus wakeup requirements
	Renamed i3c_master_any_wakeup_enabled() to
	i3c_master_has_wakeup_enabled_devs().

    i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI
	Updated for the rename of i3c_master_any_wakeup_enabled() to
	i3c_master_has_wakeup_enabled_devs().

    i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()
	Reworked the comment that moves with the code into kernel-doc,
	and mentioned that in the commit message.

    i3c: mipi-i3c-hci: Advertise IBI wakeup capability
	Updated the i3c_hci_sysdev() kernel-doc to mention system PM and
	wakeup, in place of the old comment tweak.

    Added Frank Li's Reviewed-by and Mukesh Savaliya's Acked-by tags.


Changes in V3:

    Added 6 patches (2-7) that fix use-after-free and unlocked accesses
    of the i3c_device desc pointer.  Moving device registration out from
    under the bus lock in patch 1 is what allows that pointer to be
    protected in the helpers that run during registration.

    i3c: master: Fix recursive locking during device registration
	Added Cc: stable@vger.kernel.org

    i3c: master: Add helper to query bus wakeup requirements
	Skip the master device explicitly.  Noted in the kernel-doc that
	wakeup enablement is user space policy, so the helper is meant to
	be called from a system suspend callback.

    Added Frank Li's Reviewed-by tags.

    Rebased onto i3c/next.


Changes in V2:

    Dropped the RFC tag.

    i3c: master: Fix recursive locking during device registration
	New patch

    i3c: master: Support IBI-based wakeup capability
	Dropped the redundant #include <linux/pm_wakeup.h>.  That header
	must not be included directly, and linux/device.h, which is
	already included, provides device_set_wakeup_capable().

    i3c: master: Add helper to query bus wakeup requirements
	i3c_master_any_wakeup_enabled() now also requires the device to
	have IBI enabled, not just system wakeup enabled, so that a
	device with no active IBI request does not keep PCI PME enabled.
	desc->ibi_lock is taken while checking.  The commit message and
	kernel-doc are updated to match.

    Rebased onto i3c/next.


Adrian Hunter (14):
      i3c: master: Fix recursive locking during device registration
      i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()
      i3c: master: Do not treat master device as a duplicate target
      i3c: master: Fix use-after-free of master->this
      i3c: Make dev->desc locking assumptions explicit
      i3c: master: Fix potential UAF in i3c_device_uevent()
      i3c: master: Fix potential UAF in i3c_device_match()
      i3c: master: Support IBI-based wakeup capability
      i3c: master: Report wakeup events for IBIs
      i3c: master: Add helper to query bus wakeup requirements
      i3c: master: Reject IBI requests from non-IBI-capable devices
      i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI
      i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()
      i3c: mipi-i3c-hci: Advertise IBI wakeup capability

 drivers/i3c/device.c                               |  13 +--
 drivers/i3c/internals.h                            |   5 +
 drivers/i3c/master.c                               | 119 ++++++++++++++++-----
 drivers/i3c/master/mipi-i3c-hci/core.c             |  20 ++++
 drivers/i3c/master/mipi-i3c-hci/dma.c              |  15 +--
 drivers/i3c/master/mipi-i3c-hci/hci.h              |   2 +
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c |  23 +++-
 include/linux/i3c/master.h                         |   6 ++
 8 files changed, 155 insertions(+), 48 deletions(-)


Regards
Adrian

[PATCH V4 01/14] i3c: master: Fix recursive locking during device registration

[Adrian Hunter]

i3c_master_register_new_i3c_devs() registers newly discovered devices
while holding i3c_bus_normaluse_lock(), a down_read().  device_register()
can immediately probe the device, and probe callbacks typically invoke
I3C helpers that take i3c_bus_normaluse_lock() again, leading to a
recursive acquisition of the same rwsem.  rwsems do not support recursive
read locking and can deadlock when a writer is waiting.  See the
"Recursive read locks" section of Documentation/locking/lockdep-design.rst.

For example, with Intel LPSS I3C, LOCKDEP generates a WARNING like:
  # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/unbind
  # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/bind
  WARNING: possible recursive locking detected
  kworker/5:1/94 is trying to acquire lock:
  ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_device_match_id+0x45/0x370
  but task is already holding lock:
  ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_master_reg_work_fn+0x21/0x5f0

Fix this by separating device creation from device registration.
Populate desc->dev under the maintenance lock, collect the devices that
still need registration into a local list, then release the lock before
calling device_register().  Finally retake the lock and clean up any
devices that failed to register.

Use the maintenance lock rather than the normal-use lock while adding
device objects.  A write-side maintenance lock prevents readers from
observing a partially initialized desc->dev during initial device
population, or desc->dev disappearing if registration fails.

The local list requires a list node, so add a list node member to struct
i3c_device.

Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V4:

	Expanded the kernel-doc for the new struct i3c_device @node
	member to note that it is only for use by
	i3c_master_register_new_i3c_devs() and is not protected by a
	lock.

Changes in V3:

	Added Cc: stable@vger.kernel.org

Changes in V2:

	New patch


 drivers/i3c/master.c       | 45 ++++++++++++++++++++++++++++----------
 include/linux/i3c/master.h |  3 +++
 2 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index f485b98805cf..d2fb1a110521 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2069,12 +2069,21 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
 static void
 i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 {
+	struct i3c_device *i3cdev, *tmp;
 	struct i3c_dev_desc *desc;
+	LIST_HEAD(i3c_unreg_devs);
 	int ret;
 
 	if (!master->init_done)
 		return;
 
+	i3c_bus_maintenance_lock(&master->bus);
+
+	if (master->shutting_down) {
+		i3c_bus_maintenance_unlock(&master->bus);
+		return;
+	}
+
 	i3c_bus_for_each_i3cdev(&master->bus, desc) {
 		if (desc->dev || !desc->info.dyn_addr || desc == master->this)
 			continue;
@@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 		if (desc->boardinfo)
 			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
 
-		ret = device_register(&desc->dev->dev);
-		if (ret) {
-			dev_err(&master->dev,
-				"Failed to add I3C device (err = %d)\n", ret);
-			desc->dev->desc = NULL;
-			put_device(&desc->dev->dev);
-			desc->dev = NULL;
-		}
+		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
+	}
+
+	i3c_bus_maintenance_unlock(&master->bus);
+
+	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
+		ret = device_register(&i3cdev->dev);
+		if (ret)
+			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
+		else
+			list_del_init(&i3cdev->node);
+	}
+
+	i3c_bus_maintenance_lock(&master->bus);
+
+	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
+		list_del(&i3cdev->node);
+		desc = i3cdev->desc;
+		i3cdev->desc = NULL;
+		put_device(&i3cdev->dev);
+		desc->dev = NULL;
 	}
+
+	i3c_bus_maintenance_unlock(&master->bus);
 }
 
 static void i3c_master_reg_work_fn(struct work_struct *work)
 {
 	struct i3c_master_controller *master = container_of(work, typeof(*master), reg_work);
 
-	i3c_bus_normaluse_lock(&master->bus);
-	if (!master->shutting_down)
-		i3c_master_register_new_i3c_devs(master);
-	i3c_bus_normaluse_unlock(&master->bus);
+	i3c_master_register_new_i3c_devs(master);
 }
 
 /**
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 2dc139a217bf..26535beb1e77 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -238,6 +238,8 @@ struct i3c_dev_desc {
  *	  every time the I3C device is rediscovered with a different dynamic
  *	  address assigned
  * @bus: I3C bus this device is attached to
+ * @node: unregistered device list node, only for use by
+ *	  i3c_master_register_new_i3c_devs(), it is not protected by a lock
  *
  * I3C device object exposed to I3C device drivers. The takes care of linking
  * this object to the relevant &struct_i3c_dev_desc one.
@@ -248,6 +250,7 @@ struct i3c_device {
 	struct device dev;
 	struct i3c_dev_desc *desc;
 	struct i3c_bus *bus;
+	struct list_head node;
 };
 
 /*
-- 
2.53.0

[PATCH V4 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()

[Adrian Hunter]

i3c_device_get_supported_xfer_mode() uses dev->desc to obtain the
master controller.  However, dev->desc must not be dereferenced unless
bus->lock is held, and this function does not take that lock.

The function only needs access to the master controller associated with
the device's bus.  Use dev->bus instead, which is always valid for the
lifetime of the device and does not require dereferencing dev->desc.

Fixes: 256a21743d91 ("i3c: Add HDR API support")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V4:

	None

Changes in V3:

	New patch


 drivers/i3c/device.c    | 2 +-
 drivers/i3c/internals.h | 5 +++++
 drivers/i3c/master.c    | 6 ------
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
index 101eaa77de68..a3778282e84c 100644
--- a/drivers/i3c/device.c
+++ b/drivers/i3c/device.c
@@ -309,7 +309,7 @@ EXPORT_SYMBOL_GPL(i3c_device_match_id);
  */
 u32 i3c_device_get_supported_xfer_mode(struct i3c_device *dev)
 {
-	return i3c_dev_get_master(dev->desc)->this->info.hdr_cap | BIT(I3C_SDR);
+	return i3c_bus_to_i3c_master(dev->bus)->this->info.hdr_cap | BIT(I3C_SDR);
 }
 EXPORT_SYMBOL_GPL(i3c_device_get_supported_xfer_mode);
 
diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
index 0f1f3f766623..86a36b951e0d 100644
--- a/drivers/i3c/internals.h
+++ b/drivers/i3c/internals.h
@@ -72,4 +72,9 @@ static inline void i3c_readl_fifo(const void __iomem *addr, void *buf,
 	}
 }
 
+static inline struct i3c_master_controller *i3c_bus_to_i3c_master(struct i3c_bus *i3cbus)
+{
+	return container_of(i3cbus, struct i3c_master_controller, bus);
+}
+
 #endif /* I3C_INTERNAL_H */
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index d2fb1a110521..c7bb52b71d88 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -102,12 +102,6 @@ void i3c_bus_normaluse_unlock(struct i3c_bus *bus)
 	up_read(&bus->lock);
 }
 
-static struct i3c_master_controller *
-i3c_bus_to_i3c_master(struct i3c_bus *i3cbus)
-{
-	return container_of(i3cbus, struct i3c_master_controller, bus);
-}
-
 static struct i3c_master_controller *dev_to_i3cmaster(struct device *dev)
 {
 	return container_of(dev, struct i3c_master_controller, dev);
-- 
2.53.0

[PATCH V4 03/14] i3c: master: Do not treat master device as a duplicate target

[Adrian Hunter]

i3c_master_search_i3c_dev_duplicate() searches the bus for another I3C
device with the same PID as the reference device.  The search can match
master->this, causing the controller itself to be returned as a
duplicate.

Since the controller is not a target device, it cannot be a duplicate of
one.  Exclude master->this from matching so that the function only
returns real duplicate target devices.

Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
---


Changes in V4:

	Added Frank's Rev-by and Mukesh's Acked-by tags

Changes in V3:

	New patch


 drivers/i3c/master.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index c7bb52b71d88..abb582645a2e 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2545,7 +2545,8 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
 
 	i3c_bus_for_each_i3cdev(&master->bus, i3cdev) {
 		if (i3cdev != refdev && i3cdev->info.pid &&
-		    i3cdev->info.pid == refdev->info.pid)
+		    i3cdev->info.pid == refdev->info.pid &&
+		    i3cdev != master->this)
 			return i3cdev;
 	}
 
-- 
2.53.0

[PATCH V4 04/14] i3c: master: Fix use-after-free of master->this

[Adrian Hunter]

sysfs attribute callbacks for the master controller device dereference
master->this.  However, master->this is freed in
i3c_master_detach_free_devs() before the master device itself is
released.

As a result, sysfs accesses can dereference a freed master->this
pointer, leading to a use-after-free.

Keep master->this alive until i3c_masterdev_release(), which is called
after the master device and its sysfs state are being torn down. Do not
free master->this as part of the normal device detach path.

On the error path in i3c_master_set_info(), reset master->this and
bus.cur_master to NULL before freeing the allocated device.

Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V4:

	Also reset master->this and bus.cur_master to NULL on the
	i3c_master_set_info() error path before freeing the allocated
	device.  Tidied up the commit message wording.

Changes in V3:

	New patch


 drivers/i3c/master.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index abb582645a2e..2357874bb9d6 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -842,6 +842,11 @@ static struct attribute *i3c_masterdev_attrs[] = {
 };
 ATTRIBUTE_GROUPS(i3c_masterdev);
 
+static void i3c_master_free_i3c_dev(struct i3c_dev_desc *dev)
+{
+	kfree(dev);
+}
+
 static void i3c_masterdev_release(struct device *dev)
 {
 	struct i3c_master_controller *master = dev_to_i3cmaster(dev);
@@ -854,6 +859,8 @@ static void i3c_masterdev_release(struct device *dev)
 	i3c_bus_cleanup(bus);
 
 	fwnode_handle_put(dev->fwnode);
+
+	i3c_master_free_i3c_dev(master->this);
 }
 
 static const struct device_type i3c_masterdev_type = {
@@ -1125,11 +1132,6 @@ static void i3c_device_release(struct device *dev)
 	kfree(i3cdev);
 }
 
-static void i3c_master_free_i3c_dev(struct i3c_dev_desc *dev)
-{
-	kfree(dev);
-}
-
 static struct i3c_dev_desc *
 i3c_master_alloc_i3c_dev(struct i3c_master_controller *master,
 			 const struct i3c_device_info *info)
@@ -2266,6 +2268,8 @@ int i3c_master_set_info(struct i3c_master_controller *master,
 	return 0;
 
 err_free_dev:
+	master->bus.cur_master = NULL;
+	master->this = NULL;
 	i3c_master_free_i3c_dev(i3cdev);
 
 	return ret;
@@ -2286,7 +2290,8 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
 					i3cdev->boardinfo->init_dyn_addr,
 					I3C_ADDR_SLOT_FREE);
 
-		i3c_master_free_i3c_dev(i3cdev);
+		if (i3cdev != master->this)
+			i3c_master_free_i3c_dev(i3cdev);
 	}
 
 	list_for_each_entry_safe(i2cdev, i2ctmp, &master->bus.devs.i2c,
-- 
2.53.0

[PATCH V4 05/14] i3c: Make dev->desc locking assumptions explicit

[Adrian Hunter]

i3c_device_get_info() takes the bus normal-use lock before accessing
dev->desc. Under that lock, the descriptor pointer is guaranteed to be
valid for the duration of the access.

Remove the unnecessary NULL check on dev->desc so the code more clearly
reflects the locking rules and expected descriptor lifetime.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
---


Changes in V4:

	Added Mukesh's Acked-by tag

Changes in V3:

	New patch


 drivers/i3c/device.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
index a3778282e84c..5e6df6de0283 100644
--- a/drivers/i3c/device.c
+++ b/drivers/i3c/device.c
@@ -101,8 +101,7 @@ void i3c_device_get_info(const struct i3c_device *dev,
 		return;
 
 	i3c_bus_normaluse_lock(dev->bus);
-	if (dev->desc)
-		*info = dev->desc->info;
+	*info = dev->desc->info;
 	i3c_bus_normaluse_unlock(dev->bus);
 }
 EXPORT_SYMBOL_GPL(i3c_device_get_info);
-- 
2.53.0

[PATCH V4 06/14] i3c: master: Fix potential UAF in i3c_device_uevent()

[Adrian Hunter]

i3c_device_uevent() dereferences i3cdev->desc without holding the bus
normal-use lock.  Since the descriptor pointer can be replaced
concurrently, including when a uevent is generated from sysfs, this can
result in dereferencing a stale descriptor and lead to a use-after-free.

Use i3c_device_get_info() instead, which protects access to the
descriptor with the normal-use lock.

Commit 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling
i3c_device_get_info() to avoid deadlock") replaced the accessor with a
direct descriptor dereference because i3c_device_get_info() would
recursively acquire bus->lock during device registration.

This change depends on "i3c: master: Fix recursive locking during device
registration", which moves device registration out from under bus->lock
and removes the possibility of that deadlock.  Without that change,
restoring the i3c_device_get_info() call would reintroduce the deadlock.

Fixes: 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling i3c_device_get_info() to avoid deadlock")
Cc: stable@vger.kernel.org # requires "i3c: master: Fix recursive locking during device registration"
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
---


Changes in V4:

	Added Mukesh's Acked-by tag

Changes in V3:

	New patch


 drivers/i3c/master.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 2357874bb9d6..ee771ac0b203 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -316,8 +316,7 @@ static int i3c_device_uevent(const struct device *dev, struct kobj_uevent_env *e
 	struct i3c_device_info devinfo;
 	u16 manuf, part, ext;
 
-	if (i3cdev->desc)
-		devinfo = i3cdev->desc->info;
+	i3c_device_get_info(i3cdev, &devinfo);
 	manuf = I3C_PID_MANUF_ID(devinfo.pid);
 	part = I3C_PID_PART_ID(devinfo.pid);
 	ext = I3C_PID_EXTRA_INFO(devinfo.pid);
-- 
2.53.0

[PATCH V4 07/14] i3c: master: Fix potential UAF in i3c_device_match()

[Adrian Hunter]

i3c_device_match() dereferences i3cdev->desc without holding the bus
normal-use lock.  Since the descriptor pointer can be replaced
concurrently, the dereference can race with descriptor replacement and
result in a use-after-free.

Protect access to i3cdev->desc with the normal-use lock.  While the lock
is held, the descriptor is guaranteed to remain valid, so the NULL check
is also unnecessary and can be removed.

This change depends on "i3c: master: Fix recursive locking during device
registration".  Prior to that change, taking the normal-use lock in
i3c_device_match() could recurse on bus->lock during device
registration.

This fixes "i3c: master: match I3C device through DT and ACPI".

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


Changes in V4:

	None

Changes in V3:

	New patch


 drivers/i3c/master.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index ee771ac0b203..83e04a2c202a 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -347,8 +347,10 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
 	i3cdev = dev_to_i3cdev(dev);
 	i3cdrv = drv_to_i3cdrv(drv);
 
-	if (i3cdev->desc && i3cdev->desc->boardinfo)
+	i3c_bus_normaluse_lock(i3cdev->bus);
+	if (i3cdev->desc->boardinfo)
 		static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
+	i3c_bus_normaluse_unlock(i3cdev->bus);
 
 	/*
 	 * SETAASA-based devices need not always have a matching ID since
-- 
2.53.0

[PATCH V4 08/14] i3c: master: Support IBI-based wakeup capability

[Adrian Hunter]

An I3C controller acts as a bus controller for one or more I3C devices.
If the controller can wake the system in response to an In-Band
Interrupt (IBI), then any device on that bus that is capable of
generating IBIs can potentially be used as a wakeup source.

Add an ibi_wakeup flag to struct i3c_master_controller so controller
drivers can advertise support for IBI-based wakeup.

If set, mark IBI-capable I3C devices as wakeup capable when they are
registered, allowing wakeup management through the standard device
wakeup framework.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---


Changes in V4:

	Shortened the comment about IBI-capable devices being wakeup
	capable.

Changes in V3:

	None

Changes in V2:

	Dropped the redundant #include <linux/pm_wakeup.h>.  That header
	must not be included directly, and linux/device.h, which is
	already included, provides device_set_wakeup_capable().


 drivers/i3c/master.c       | 4 ++++
 include/linux/i3c/master.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 83e04a2c202a..2616354dbe54 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2110,6 +2110,10 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 		if (desc->boardinfo)
 			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
 
+		/* If the device has IBI capability, set as wakeup capable */
+		if (master->ibi_wakeup && (desc->info.bcr & I3C_BCR_IBI_REQ_CAP))
+			device_set_wakeup_capable(&desc->dev->dev, true);
+
 		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
 	}
 
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 26535beb1e77..9d675d01522c 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -524,6 +524,7 @@ struct i3c_master_controller_ops {
  * @hotjoin: true if the master support hotjoin
  * @rpm_allowed: true if Runtime PM allowed
  * @rpm_ibi_allowed: true if IBI and Hot-Join allowed while runtime suspended
+ * @ibi_wakeup: IBI can wakeup the system
  * @shutting_down: set to true when master begins shutdown or unregister
  * @boardinfo.i3c: list of I3C  boardinfo objects
  * @boardinfo.i2c: list of I2C boardinfo objects
@@ -563,6 +564,7 @@ struct i3c_master_controller {
 	unsigned int hotjoin: 1;
 	unsigned int rpm_allowed: 1;
 	unsigned int rpm_ibi_allowed: 1;
+	unsigned int ibi_wakeup: 1;
 	bool shutting_down;
 	struct {
 		struct list_head i3c;
-- 
2.53.0

[PATCH V4 09/14] i3c: master: Report wakeup events for IBIs

[Adrian Hunter]

An I3C device configured as a wakeup source can wake the system by
generating an In-Band Interrupt (IBI).

When an IBI is queued for processing, record a wakeup event for the
device if wakeup is enabled.  Use a 100 ms processing interval to give
the I3C device driver time to process the IBI.

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


Changes in V4:

	None

Changes in V2 and V3:

	None


 drivers/i3c/master.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 2616354dbe54..6c5341491944 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -3384,6 +3384,9 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master)
 	}
 }
 
+/* Approximate time for IBI handler to run */
+#define I3C_WAKEUP_PROCESSING_TIME_MS 100
+
 /**
  * i3c_master_queue_ibi() - Queue an IBI
  * @dev: the device this IBI is coming from
@@ -3397,6 +3400,9 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
 	if (!dev->ibi || !slot)
 		return;
 
+	if (device_may_wakeup(&dev->dev->dev))
+		pm_wakeup_event(&dev->dev->dev, I3C_WAKEUP_PROCESSING_TIME_MS);
+
 	atomic_inc(&dev->ibi->pending_ibis);
 	queue_work(dev->ibi->wq, &slot->work);
 }
-- 
2.53.0

[PATCH V4 10/14] i3c: master: Add helper to query bus wakeup requirements

[Adrian Hunter]

Add i3c_master_has_wakeup_enabled_devs(), which iterates over the devices
on an I3C bus and reports whether any of them are enabled for system
wakeup and have IBI enabled.

Controller drivers can use this helper to determine whether wakeup
support must remain available while the system is suspended.

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


Changes in V4:

	Renamed i3c_master_any_wakeup_enabled() to
	i3c_master_has_wakeup_enabled_devs().

Changes in V3:

	Skip the master device explicitly.  Added kernel-doc noting
	that wakeup enablement is user space policy, so the helper is
	meant to be called from a system suspend callback.

Changes in V2:

	i3c_master_any_wakeup_enabled() now also requires the device to
	have IBI enabled, not just system wakeup enabled, so that a
	device with no active IBI request does not keep PCI PME enabled.
	desc->ibi_lock is taken while checking.  The commit message and
	kernel-doc are updated to match.


 drivers/i3c/master.c       | 35 +++++++++++++++++++++++++++++++++++
 include/linux/i3c/master.h |  1 +
 2 files changed, 36 insertions(+)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 6c5341491944..afcd7a21a3e6 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2147,6 +2147,41 @@ static void i3c_master_reg_work_fn(struct work_struct *work)
 	i3c_master_register_new_i3c_devs(master);
 }
 
+/**
+ * i3c_master_has_wakeup_enabled_devs() - check if any device can wake the system
+ * @master: I3C master controller
+ *
+ * Iterate over devices on the bus and return true if any device has
+ * system wakeup enabled and IBI enabled.
+ *
+ * Whether a device is enabled for system wakeup is user space policy,
+ * settable at any time through the device's power/wakeup sysfs attribute,
+ * so the answer is only stable once user space is frozen.  Call this from
+ * a system suspend callback.
+ *
+ * Return: true if any device may wake the system via IBI, false otherwise.
+ */
+bool i3c_master_has_wakeup_enabled_devs(struct i3c_master_controller *master)
+{
+	struct i3c_dev_desc *desc;
+	bool wakeup = false;
+
+	i3c_bus_normaluse_lock(&master->bus);
+	i3c_bus_for_each_i3cdev(&master->bus, desc) {
+		if (!desc->dev || desc == master->this || !device_may_wakeup(&desc->dev->dev))
+			continue;
+		guard(mutex)(&desc->ibi_lock);
+		if (desc->ibi && desc->ibi->enabled) {
+			wakeup = true;
+			break;
+		}
+	}
+	i3c_bus_normaluse_unlock(&master->bus);
+
+	return wakeup;
+}
+EXPORT_SYMBOL_GPL(i3c_master_has_wakeup_enabled_devs);
+
 /**
  * i3c_master_dma_map_single() - Map buffer for single DMA transfer
  * @dev: device object of a device doing DMA
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 9d675d01522c..82d9886e7f12 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -765,6 +765,7 @@ void i3c_generic_ibi_recycle_slot(struct i3c_generic_ibi_pool *pool,
 				  struct i3c_ibi_slot *slot);
 
 void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot);
+bool i3c_master_has_wakeup_enabled_devs(struct i3c_master_controller *master);
 
 struct i3c_ibi_slot *i3c_master_get_free_ibi_slot(struct i3c_dev_desc *dev);
 
-- 
2.53.0

[PATCH V4 11/14] i3c: master: Reject IBI requests from non-IBI-capable devices

[Adrian Hunter]

i3c_device_request_ibi() does not verify that a device advertises IBI
support before attempting to set up IBI handling.

Add a check for I3C_BCR_IBI_REQ_CAP and fail with -EOPNOTSUPP when IBI
support is not reported by the device.  This keeps IBI setup consistent
with other IBI-related functionality, such as exposing wakeup capability
only for IBI-capable devices.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---


Changes in V4:

	None

Changes in V3:

	Added Frank's Rev-by tag

Changes in V2:

	None


 drivers/i3c/device.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
index 5e6df6de0283..f1ba363b22a1 100644
--- a/drivers/i3c/device.c
+++ b/drivers/i3c/device.c
@@ -204,12 +204,14 @@ int i3c_device_request_ibi(struct i3c_device *dev,
 		return ret;
 
 	i3c_bus_normaluse_lock(dev->bus);
-	if (dev->desc) {
+	if (!dev->desc) {
+		ret = -ENOENT;
+	} else if (!(dev->desc->info.bcr & I3C_BCR_IBI_REQ_CAP)) {
+		ret = -EOPNOTSUPP;
+	} else {
 		mutex_lock(&dev->desc->ibi_lock);
 		ret = i3c_dev_request_ibi_locked(dev->desc, req);
 		mutex_unlock(&dev->desc->ibi_lock);
-	} else {
-		ret = -ENOENT;
 	}
 	i3c_bus_normaluse_unlock(dev->bus);
 
-- 
2.53.0

[PATCH V4 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

[Adrian Hunter]

Keep the PCI wakeup state aligned with the wakeup requirements of the
devices served by the controller(s).

The PCI function is the wakeup source for HCI instances exposed beneath
it.  However, wakeup is only needed when at least one attached I3C device
is enabled as a wakeup source.

During suspend, check whether any HCI instance has a wakeup-enabled I3C
device and enable wakeup for the PCI function only in that case.
Otherwise leave PCI wakeup disabled.

Note, the suspend callback is used for both system and runtime suspend.
Although this change may update the PCI wakeup state during runtime
suspend, it does so only when the required wakeup state changes.
Moreover, PCI wakeup-capable devices already have PME wakeup armed for
runtime suspend, so changing the wakeup-enabled state does not affect
runtime PM wakeup behavior.

Note also, since the PCI wakeup state is derived from the wakeup
configuration of the attached I3C devices, the PCI device power/wakeup
sysfs attribute no longer provides independent wakeup control.

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


Changes in V4:

	Updated for the rename of i3c_master_any_wakeup_enabled() to
	i3c_master_has_wakeup_enabled_devs().

Changes in V2 and V3:

	None


 .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 23 +++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 5a9e2a43eff8..42c172e82622 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -265,6 +265,8 @@ static bool mipi_i3c_hci_pci_is_operational(struct device *dev, bool update)
 struct mipi_i3c_hci_pci_pm_data {
 	struct device *dev[INST_MAX];
 	int dev_cnt;
+	bool can_wakeup;
+	bool may_wakeup;
 };
 
 static bool mipi_i3c_hci_pci_is_mfd(struct device *dev)
@@ -272,6 +274,13 @@ static bool mipi_i3c_hci_pci_is_mfd(struct device *dev)
 	return dev_is_platform(dev) && mfd_get_cell(to_platform_device(dev));
 }
 
+static bool mipi_i3c_hci_pci_any_wakeup_enabled(struct device *dev)
+{
+	struct i3c_hci *hci = dev_get_drvdata(dev);
+
+	return i3c_master_has_wakeup_enabled_devs(&hci->master);
+}
+
 static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *data)
 {
 	struct mipi_i3c_hci_pci_pm_data *pm_data = data;
@@ -287,6 +296,9 @@ static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *data)
 
 	pm_data->dev[pm_data->dev_cnt++] = dev;
 
+	if (pm_data->can_wakeup && mipi_i3c_hci_pci_any_wakeup_enabled(dev))
+		pm_data->may_wakeup = true;
+
 	return 0;
 }
 
@@ -317,12 +329,19 @@ static int mipi_i3c_hci_pci_suspend(struct device *dev)
 	if (!hci->info->control_instance_pm)
 		return 0;
 
+	pm_data.can_wakeup = device_can_wakeup(dev);
+
 	ret = device_for_each_child_reverse(dev, &pm_data, mipi_i3c_hci_pci_suspend_instance);
-	if (ret)
+	if (ret) {
 		for (int i = 0; i < pm_data.dev_cnt; i++)
 			i3c_hci_rpm_resume(pm_data.dev[i]);
+		return ret;
+	}
 
-	return ret;
+	if (device_may_wakeup(dev) != pm_data.may_wakeup)
+		device_set_wakeup_enable(dev, pm_data.may_wakeup);
+
+	return 0;
 }
 
 static int mipi_i3c_hci_pci_resume(struct device *dev)
-- 
2.53.0

[PATCH V4 13/14] i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()

[Adrian Hunter]

The MIPI I3C HCI driver needs to identify the underlying system device
used for DMA mapping and PM operations. The logic for determining that
device is currently embedded in the DMA implementation.

Factor this code out into i3c_hci_sysdev() so it can be shared by other
parts of the driver and keep the device-selection logic in one place.

The explanatory comment moves with the code, reworked as kernel-doc now
that it documents a function rather than an inline block.

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


Changes in V4:

	Reworked the comment that moves with the code into kernel-doc,
	and mentioned that in the commit message.

Changes in V2 and V3:

	None


 drivers/i3c/master/mipi-i3c-hci/core.c | 16 ++++++++++++++++
 drivers/i3c/master/mipi-i3c-hci/dma.c  | 15 +--------------
 drivers/i3c/master/mipi-i3c-hci/hci.h  |  2 ++
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index cfe9b5390b56..f95da427d7a9 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -15,6 +15,7 @@
 #include <linux/interrupt.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
+#include <linux/pci.h>
 #include <linux/platform_data/mipi-i3c-hci.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -117,6 +118,21 @@ static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
 	return container_of(m, struct i3c_hci, master);
 }
 
+/**
+ * i3c_hci_sysdev() - Get the device to use for DMA
+ * @dev: Device the HCI controller is bound to
+ *
+ * When an IOMMU is enabled, DMA API calls must use the device that IOMMU
+ * setup was done for.  Under PCI enumeration that is the PCI device, not
+ * the "mipi-i3c-hci" platform device below it.
+ *
+ * Return: @dev's parent if it is a PCI device, otherwise @dev.
+ */
+struct device *i3c_hci_sysdev(struct device *dev)
+{
+	return dev->parent && dev_is_pci(dev->parent) ? dev->parent : dev;
+}
+
 static void i3c_hci_set_master_dyn_addr(struct i3c_hci *hci)
 {
 	reg_write(MASTER_DEVICE_ADDR,
diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
index 0672ed1132f8..7c2b20474130 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dma.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
@@ -15,7 +15,6 @@
 #include <linux/errno.h>
 #include <linux/i3c/master.h>
 #include <linux/io.h>
-#include <linux/pci.h>
 
 #include "hci.h"
 #include "cmd.h"
@@ -301,23 +300,11 @@ static int hci_dma_init(struct i3c_hci *hci)
 {
 	struct hci_rings_data *rings;
 	struct hci_rh_data *rh;
-	struct device *sysdev;
 	u32 regval;
 	unsigned int i, nr_rings, xfers_sz, resps_sz;
 	unsigned int ibi_status_ring_sz, ibi_data_ring_sz;
 	int ret;
 
-	/*
-	 * Set pointer to a physical device that does DMA and has IOMMU setup
-	 * done for it in case of enabled IOMMU and use it with the DMA API.
-	 * Here such device is either
-	 * "mipi-i3c-hci" platform device (OF/ACPI enumeration) parent or
-	 * grandparent (PCI enumeration).
-	 */
-	sysdev = hci->master.dev.parent;
-	if (sysdev->parent && dev_is_pci(sysdev->parent))
-		sysdev = sysdev->parent;
-
 	regval = rhs_reg_read(CONTROL);
 	nr_rings = FIELD_GET(MAX_HEADER_COUNT_CAP, regval);
 	dev_dbg(&hci->master.dev, "%d DMA rings available\n", nr_rings);
@@ -332,7 +319,7 @@ static int hci_dma_init(struct i3c_hci *hci)
 		return -ENOMEM;
 	hci->io_data = rings;
 	rings->total = nr_rings;
-	rings->sysdev = sysdev;
+	rings->sysdev = i3c_hci_sysdev(hci->master.dev.parent);
 
 	for (i = 0; i < rings->total; i++) {
 		u32 offset = rhs_reg_read(RHn_OFFSET(i));
diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
index b3d9803b1968..b8d2a3d680f8 100644
--- a/drivers/i3c/master/mipi-i3c-hci/hci.h
+++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
@@ -184,6 +184,8 @@ void amd_set_resp_buf_thld(struct i3c_hci *hci);
 void i3c_hci_sync_irq_inactive(struct i3c_hci *hci);
 int i3c_hci_process_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
 
+struct device *i3c_hci_sysdev(struct device *dev);
+
 #define DEFAULT_AUTOSUSPEND_DELAY_MS 1000
 
 int i3c_hci_rpm_suspend(struct device *dev);
-- 
2.53.0

[PATCH V4 14/14] i3c: mipi-i3c-hci: Advertise IBI wakeup capability

[Adrian Hunter]

Set master->ibi_wakeup during probe when the associated system device
advertises wakeup capability, allowing the I3C core to mark IBI-capable
I3C devices as wakeup capable.

Tweak the comment for i3c_hci_sysdev() to mention the new usage.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---


Changes in V4:

	Updated the i3c_hci_sysdev() kernel-doc to mention system PM
	and wakeup, in place of the old comment tweak.

Changes in V3:

	Added Frank's Rev-by tag

Changes in V2:

	None


 drivers/i3c/master/mipi-i3c-hci/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index f95da427d7a9..06acf6cf5a0d 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -119,12 +119,13 @@ static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
 }
 
 /**
- * i3c_hci_sysdev() - Get the device to use for DMA
+ * i3c_hci_sysdev() - Get the device to use for DMA and system PM
  * @dev: Device the HCI controller is bound to
  *
  * When an IOMMU is enabled, DMA API calls must use the device that IOMMU
  * setup was done for.  Under PCI enumeration that is the PCI device, not
- * the "mipi-i3c-hci" platform device below it.
+ * the "mipi-i3c-hci" platform device below it.  The same device owns
+ * system PM and wakeup configuration.
  *
  * Return: @dev's parent if it is a PCI device, otherwise @dev.
  */
@@ -1180,6 +1181,9 @@ static int i3c_hci_probe(struct platform_device *pdev)
 	if (hci->quirks & HCI_QUIRK_RPM_IBI_ALLOWED)
 		hci->master.rpm_ibi_allowed = true;
 
+	if (device_can_wakeup(i3c_hci_sysdev(&pdev->dev)))
+		hci->master.ibi_wakeup = true;
+
 	return i3c_master_register(&hci->master, &pdev->dev, &i3c_hci_ops, false);
 }
 
-- 
2.53.0

Re: [PATCH V4 10/14] i3c: master: Add helper to query bus wakeup requirements

[sashiko-bot]

> Add i3c_master_has_wakeup_enabled_devs(), which iterates over the devices
> on an I3C bus and reports whether any of them are enabled for system
> wakeup and have IBI enabled.
> 
> Controller drivers can use this helper to determine whether wakeup
> support must remain available while the system is suspended.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=10

Re: [PATCH V4 08/14] i3c: master: Support IBI-based wakeup capability

[sashiko-bot]

> An I3C controller acts as a bus controller for one or more I3C devices.
> If the controller can wake the system in response to an In-Band
> Interrupt (IBI), then any device on that bus that is capable of
> generating IBIs can potentially be used as a wakeup source.
> 
> Add an ibi_wakeup flag to struct i3c_master_controller so controller
> drivers can advertise support for IBI-based wakeup.
> 
> If set, mark IBI-capable I3C devices as wakeup capable when they are
> registered, allowing wakeup management through the standard device
> wakeup framework.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=8

Re: [PATCH V4 06/14] i3c: master: Fix potential UAF in i3c_device_uevent()

[sashiko-bot]

> i3c_device_uevent() dereferences i3cdev->desc without holding the bus
> normal-use lock.  Since the descriptor pointer can be replaced
> concurrently, including when a uevent is generated from sysfs, this can
> result in dereferencing a stale descriptor and lead to a use-after-free.
> 
> Use i3c_device_get_info() instead, which protects access to the
> descriptor with the normal-use lock.
> 
> Commit 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling
> i3c_device_get_info() to avoid deadlock") replaced the accessor with a
> direct descriptor dereference because i3c_device_get_info() would
> recursively acquire bus->lock during device registration.
> 
> This change depends on "i3c: master: Fix recursive locking during device
> registration", which moves device registration out from under bus->lock
> and removes the possibility of that deadlock.  Without that change,
> restoring the i3c_device_get_info() call would reintroduce the deadlock.
> 
> Fixes: 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling i3c_device_get_info() to avoid deadlock")
> Cc: stable@vger.kernel.org # requires "i3c: master: Fix recursive locking during device registration"
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=6

Re: [PATCH V4 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()

[sashiko-bot]

> i3c_device_get_supported_xfer_mode() uses dev->desc to obtain the
> master controller.  However, dev->desc must not be dereferenced unless
> bus->lock is held, and this function does not take that lock.
> 
> The function only needs access to the master controller associated with
> the device's bus.  Use dev->bus instead, which is always valid for the
> lifetime of the device and does not require dereferencing dev->desc.
> 
> Fixes: 256a21743d91 ("i3c: Add HDR API support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=2

Re: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration

[sashiko-bot]

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

New issues:
- [High] Dropping the bus maintenance lock around device_register() exposes the I3C device descriptor to concurrent Use-After-Free.

Pre-existing issues:
- [High] Unbalanced fwnode reference count decrement in i3c_device_release() leads to Use-After-Free.
--

--- Patch [1]: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration ---
commit b31a8482aa2ea710ccc853d0c74b77edebb131ff
Author: Adrian Hunter <adrian.hunter@intel.com>
Date:   Thu Aug 6 13:18:44 2026 +0000

    i3c: master: Fix recursive locking during device registration

This commit addresses a recursive locking lockdep warning by decoupling
device creation from registration. It collects newly populated devices into a
temporary list under the maintenance lock, then releases the lock prior to
invoking device_register() to prevent probe deadlocks. Finally, the lock is
re-acquired to clean up any devices that failed registration.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)

[ ... ]

> +		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
> +	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		ret = device_register(&i3cdev->dev);

[Severity: High]
Does dropping the bus maintenance lock around device_register() expose the
I3C device descriptor to a concurrent use-after-free?

During registration, the driver core can call subsystem functions like
i3c_device_match(), which read i3cdev->desc locklessly:

drivers/i3c/master.c:i3c_device_match() {
    ...
    if (i3cdev->desc && i3cdev->desc->boardinfo)
        static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
    ...
}

Concurrently, an I3C Hot-Join or sysfs-triggered Dynamic Address Assignment
(DAA) could execute i3c_master_do_daa(), take the i3c_bus_maintenance_lock,
and call __i3c_master_add_i3c_dev_locked(). If DAA detects the device as a
duplicate, it replaces the descriptor and frees the old one:

drivers/i3c/master.c:__i3c_master_add_i3c_dev_locked() {
    ...
    i3c_master_detach_i3c_dev(olddev);
    i3c_master_free_i3c_dev(olddev);
    ...
}

If the registering thread is actively evaluating i3cdev->desc when this
happens, will it lead to memory corruption when reading fields like
desc->boardinfo?

> +		if (ret)
> +			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
> +		else
> +			list_del_init(&i3cdev->node);
> +	}
> +
> +	i3c_bus_maintenance_lock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		list_del(&i3cdev->node);
> +		desc = i3cdev->desc;
> +		i3cdev->desc = NULL;
> +		put_device(&i3cdev->dev);

[Severity: High]
This is a pre-existing issue, but does this error path cause an unbalanced
fwnode reference count decrement leading to a use-after-free?

During device initialization, device_set_node() assigns the fwnode without
incrementing its reference count:

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

However, when put_device() is called here on registration failure,
the release function unconditionally drops a reference:

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

Since the device object never acquired a reference to the boardinfo's fwnode,
will repeated registration failures or unbind operations drop the refcount
below zero, eventually prematurely freeing the fwnode?

> +		desc->dev = NULL;
>  	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=1

Re: [PATCH V4 05/14] i3c: Make dev->desc locking assumptions explicit

[sashiko-bot]

> i3c_device_get_info() takes the bus normal-use lock before accessing
> dev->desc. Under that lock, the descriptor pointer is guaranteed to be
> valid for the duration of the access.
> 
> Remove the unnecessary NULL check on dev->desc so the code more clearly
> reflects the locking rules and expected descriptor lifetime.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=5

Re: [PATCH V4 04/14] i3c: master: Fix use-after-free of master->this

[sashiko-bot]

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

Pre-existing issues:
- [High] Delayed device release triggers Use-After-Free of devm-allocated master structure.
- [High] If `idr_alloc()` fails in `i3c_bus_init()`, the error path will erroneously remove ID 0 from `i3c_bus_idr`, corrupting the state of another successfully registered I3C master.
--

--- Patch [4]: [PATCH V4 04/14] i3c: master: Fix use-after-free of master->this ---
commit 327e5dd17c108564121fb788d79fe1ed7a32a2b4
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Fix use-after-free of master->this

This commit aims to fix a use-after-free by keeping master->this alive until
i3c_masterdev_release() is called. It moves the freeing of master->this out
of the normal device detach path.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index abb582645a2ed..2357874bb9d6a 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -842,6 +842,11 @@ static struct attribute *i3c_masterdev_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(i3c_masterdev);
>  
> +static void i3c_master_free_i3c_dev(struct i3c_dev_desc *dev)
> +{
> +	kfree(dev);
> +}
> +
>  static void i3c_masterdev_release(struct device *dev)
>  {
>  	struct i3c_master_controller *master = dev_to_i3cmaster(dev);
> @@ -854,6 +859,8 @@ static void i3c_masterdev_release(struct device *dev)
>  	i3c_bus_cleanup(bus);

[Severity: High]
This is a pre-existing issue, but can this unconditional call to
i3c_bus_cleanup() corrupt the ID pool if idr_alloc() fails during probe?

If idr_alloc() fails in i3c_bus_init(), the error path in
i3c_master_register() will call put_device(&master->dev), which leads to this
release function.

Because the bus ID was not assigned on failure, i3cbus->id remains at its
default value of 0. Calling i3c_bus_cleanup() will then unconditionally call
idr_remove(&i3c_bus_idr, 0), removing ID 0 which might currently be in use
by another successfully registered I3C master.

>  	fwnode_handle_put(dev->fwnode);
> +
> +	i3c_master_free_i3c_dev(master->this);

[Severity: High]
This isn't a bug introduced by this patch, but does accessing master->this
here risk a use-after-free if the master structure was allocated with
devres?

Many I3C master drivers allocate their controller structures using
devm_kzalloc() and tie the memory lifetime to the platform device. If a
userspace process holds a sysfs attribute open for the master device, the
kobject refcount remains elevated.

When the master device is unbound (for example, if the module is unloaded),
devres will free the master structure before the final kobject reference is
dropped. When the sysfs file is eventually closed, this release callback
will execute, but the master structure it dereferences (for master->this and
master->wq) will have already been freed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=4

Re: [PATCH V4 03/14] i3c: master: Do not treat master device as a duplicate target

[sashiko-bot]

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

Pre-existing issues:
- [High] Concurrent sysfs access during controller unregistration causes Use-After-Free on master->this.
--

--- Patch [3]: [PATCH V4 03/14] i3c: master: Do not treat master device as a duplicate target ---
commit fc7734e8f0ca4a890942ede6fbf8e3880e33f59c
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Do not treat master device as a duplicate target

i3c_master_search_i3c_dev_duplicate() searches the bus for another I3C
device with the same PID as the reference device. The search can match
master->this, causing the controller itself to be returned as a duplicate.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index c7bb52b71d884..abb582645a2ed 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2545,7 +2545,8 @@ i3c_master_search_i3c_dev_duplicate(struct i3c_dev_desc *refdev)
>  
>  	i3c_bus_for_each_i3cdev(&master->bus, i3cdev) {
>  		if (i3cdev != refdev && i3cdev->info.pid &&
> -		    i3cdev->info.pid == refdev->info.pid)
> +		    i3cdev->info.pid == refdev->info.pid &&
> +		    i3cdev != master->this)
>  			return i3cdev;
>  	}
>  

[Severity: High]
This is a pre-existing issue, but since this patch modifies how master->this
is handled, I noticed a potential concurrent use-after-free with master->this
during unregistration.

When an I3C master controller is unregistered, i3c_master_unregister() calls
i3c_master_bus_cleanup() before device_unregister():

drivers/i3c/master.c:i3c_master_unregister() {
	...
	i3c_master_bus_cleanup(master);
	pm_runtime_disable(&master->dev);
	device_unregister(&master->dev);
	...
}

The cleanup function eventually calls i3c_master_detach_free_devs(), which
unconditionally frees all devices in the bus list, including master->this:

drivers/i3c/master.c:i3c_master_detach_free_devs() {
	...
		i3c_master_free_i3c_dev(i3cdev);
	...
}

Because device_unregister() has not yet executed, sysfs attributes attached
to the master device remain active. 

If userspace reads a sysfs attribute during this window, would bcr_show() 
dereference the freed master->this pointer?

drivers/i3c/master.c:bcr_show() {
	...
	desc = dev_to_i3cdesc(dev);
	ret = sysfs_emit(buf, "0x%02x\n", desc->info.bcr);
	...
}

The teardown path does not appear to hold the bus->lock, so the
i3c_bus_normaluse_lock() in the sysfs handler would still succeed. Could this
cause a memory corruption or information leak?

Should device_unregister() or sysfs attribute removal happen before the bus
is cleaned up and the devices are freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=3

Re: [PATCH V4 11/14] i3c: master: Reject IBI requests from non-IBI-capable devices

[sashiko-bot]

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

Pre-existing issues:
- [High] Workqueue and rescue thread leak in error path of i3c_dev_request_ibi_locked().
--

--- Patch [11]: [PATCH V4 11/14] i3c: master: Reject IBI requests from non-IBI-capable devices ---
commit cc9c665c421a1d987949a06269af6968afd5a89d
Author: Adrian Hunter <adrian.hunter@intel.com>
Subject: i3c: master: Reject IBI requests from non-IBI-capable devices

This commit adds a capability check before allowing devices to set up IBI
requests. It fails with -EOPNOTSUPP if the device does not advertise IBI
support, ensuring consistency with other IBI features.

> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index 5e6df6de02834..f1ba363b22a17 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -204,12 +204,14 @@ int i3c_device_request_ibi(struct i3c_device *dev,
>  		return ret;
>  
>  	i3c_bus_normaluse_lock(dev->bus);
> -	if (dev->desc) {
> +	if (!dev->desc) {
> +		ret = -ENOENT;
> +	} else if (!(dev->desc->info.bcr & I3C_BCR_IBI_REQ_CAP)) {
> +		ret = -EOPNOTSUPP;
> +	} else {
>  		mutex_lock(&dev->desc->ibi_lock);
>  		ret = i3c_dev_request_ibi_locked(dev->desc, req);

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
i3c_dev_request_ibi_locked() leak the workqueue?

When master->ops->request_ibi() fails, the previously allocated workqueue
appears to be orphaned:

drivers/i3c/master.c:i3c_dev_request_ibi_locked() {
    ...
	ibi->wq = alloc_ordered_workqueue(dev_name(...), WQ_MEM_RECLAIM);
	if (!ibi->wq) {
        ...
	}
    ...
	ret = master->ops->request_ibi(dev, req);
	if (ret) {
		kfree(ibi);
		dev->ibi = NULL;
	}
    ...
}

Can this leave the wq and its associated rescue worker thread permanently
leaked since destroy_workqueue() is never called before kfree(ibi)?

>  		mutex_unlock(&dev->desc->ibi_lock);
> -	} else {
> -		ret = -ENOENT;
>  	}
>  	i3c_bus_normaluse_unlock(dev->bus);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=11

Re: [PATCH V4 07/14] i3c: master: Fix potential UAF in i3c_device_match()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Removing the NULL check for `i3cdev->desc` introduces a deterministic NULL pointer dereference during device unregistration and registration error paths.
- [High] `i3c_device_get_info()` dereferences `dev->desc` without checking for NULL under the lock, causing a NULL pointer dereference.
--

--- Patch [7]: [PATCH V4 07/14] i3c: master: Fix potential UAF in i3c_device_match() ---
commit 4b5a3c82a1c4c5f9bf31e925977c2286520960fc
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Fix potential UAF in i3c_device_match()

This commit protects access to i3cdev->desc with the normal-use lock to
prevent a use-after-free when the descriptor pointer is concurrently
replaced. Since the descriptor is assumed to remain valid while the lock is
held, the NULL check is removed.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index ee771ac0b203e..83e04a2c202a9 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -347,8 +347,10 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
>  	i3cdev = dev_to_i3cdev(dev);
>  	i3cdrv = drv_to_i3cdrv(drv);
>  
> -	if (i3cdev->desc && i3cdev->desc->boardinfo)
> +	i3c_bus_normaluse_lock(i3cdev->bus);
> +	if (i3cdev->desc->boardinfo)

[Severity: High]
Is it safe to remove the NULL check for i3cdev->desc here?

If a userspace process writes to a driver's bind file via sysfs concurrently
with a device registration failure, bind_store() will call
driver_match_device() and eventually reach i3c_device_match().

In the error path of i3c_master_register_new_i3c_devs(), i3cdev->desc is set
to NULL under the write lock. When this code subsequently acquires the read
lock, i3cdev->desc will be NULL, which could cause a NULL pointer
dereference.

Similarly, i3c_master_unregister_i3c_devs() locklessly sets i3cdev->desc to
NULL during teardown, which can also race with this path.

>  		static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
> +	i3c_bus_normaluse_unlock(i3cdev->bus);
>  
>  	/*
>  	 * SETAASA-based devices need not always have a matching ID since
>  	 * it is not mandatory for such devices to implement deviceinfo
>  	 * CCC commands. Allow them to register through DT or ACPI.
>  	 */
>  	if (i3cdrv->id_table && i3c_device_match_id(i3cdev, i3cdrv->id_table))

[Severity: High]
Does this call to i3c_device_match_id() have a similar vulnerability?

i3c_device_match_id() eventually calls i3c_device_get_info(). While
i3c_device_get_info() does acquire the normal-use lock, it directly reads
dev->desc->info without checking if dev->desc is valid.

If a bind operation races with device unregistration or a registration error
path, dev->desc could be set to NULL before the lock is acquired, leading to
a crash when reading dev->desc->info.

>  		return 1;
>  
>  	if (static_addr_method & I3C_ADDR_METHOD_SETAASA) {

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=7

Re: [PATCH V4 13/14] i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()

[sashiko-bot]

> The MIPI I3C HCI driver needs to identify the underlying system device
> used for DMA mapping and PM operations. The logic for determining that
> device is currently embedded in the DMA implementation.
> 
> Factor this code out into i3c_hci_sysdev() so it can be shared by other
> parts of the driver and keep the device-selection logic in one place.
> 
> The explanatory comment moves with the code, reworked as kernel-doc now
> that it documents a function rather than an inline block.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=13

Re: [PATCH V4 09/14] i3c: master: Report wakeup events for IBIs

[sashiko-bot]

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

New issues:
- [High] NULL pointer dereference in `i3c_master_queue_ibi()` when accessing `dev->dev`.

Pre-existing issues:
- [High] Race condition in `i3c_master_queue_ibi()` leading to Use-After-Free of `dev->ibi`.
--

--- Patch [9]: [PATCH V4 09/14] i3c: master: Report wakeup events for IBIs ---
commit 461a49ac6bedf9467539aa8fac5583bf77ad5045
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: master: Report wakeup events for IBIs

An I3C device configured as a wakeup source can wake the system by
generating an In-Band Interrupt (IBI). When an IBI is queued for
processing, record a wakeup event for the device if wakeup is enabled.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 2616354dbe545..6c53414919445 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c

[ ... ]

> @@ -3397,6 +3400,9 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
>  	if (!dev->ibi || !slot)
>  		return;
>  
> +	if (device_may_wakeup(&dev->dev->dev))
> +		pm_wakeup_event(&dev->dev->dev, I3C_WAKEUP_PROCESSING_TIME_MS);
> +

[Severity: High]
Can dev->dev be NULL here?

Looking at i3c_master_unregister_i3c_devs(), it sets i3cdev->dev to NULL
during device teardown:

drivers/i3c/master.c:i3c_master_unregister_i3c_devs() {
    ...
		i3cdev->dev->desc = NULL;
		put_device(&i3cdev->dev->dev);
		i3cdev->dev = NULL;
	}
}

If a hardware IBI is received during or shortly after this teardown,
wouldn't accessing &dev->dev->dev cause a NULL pointer dereference when
evaluated inside device_may_wakeup()?

>  	atomic_inc(&dev->ibi->pending_ibis);

[Severity: High]
This is a pre-existing issue, but does the lack of synchronization here allow
a use-after-free of dev->ibi?

The teardown path in i3c_dev_disable_ibi_locked() checks pending_ibis
locklessly and bypasses waiting if it reads 0:

drivers/i3c/master.c:i3c_dev_disable_ibi_locked() {
    ...
	if (atomic_read(&dev->ibi->pending_ibis))
		wait_for_completion(&dev->ibi->all_ibis_handled);
    ...
}

If an ISR is executing concurrently and passes the check for !dev->ibi at
the top of i3c_master_queue_ibi(), but hasn't yet reached the atomic_inc()
here, the teardown thread could see pending_ibis == 0 and proceed to free
dev->ibi in i3c_dev_free_ibi_locked():

drivers/i3c/master.c:i3c_dev_free_ibi_locked() {
    ...
	kfree(dev->ibi);
	dev->ibi = NULL;
    ...
}

When the ISR resumes, wouldn't the subsequent accesses to
dev->ibi->pending_ibis and dev->ibi->wq cause a use-after-free? Adding
the wakeup event logic slightly widens this existing race window.

>  	queue_work(dev->ibi->wq, &slot->work);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=9

Re: [PATCH V4 14/14] i3c: mipi-i3c-hci: Advertise IBI wakeup capability

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The platform driver sets the I3C master `ibi_wakeup` flag but fails to propagate the wakeup state during system suspend for non-PCI platform devices, breaking IBI wakeup functionality.
--

--- Patch [14]: [PATCH V4 14/14] i3c: mipi-i3c-hci: Advertise IBI wakeup capability ---
commit 443a5c0d5859df8fbad91b6e456865274a7f7ca9
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: mipi-i3c-hci: Advertise IBI wakeup capability

This commit sets master->ibi_wakeup during probe when the associated system
device advertises wakeup capability. This allows the I3C core to mark
IBI-capable I3C devices as wakeup capable.

> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index f95da427d7a9e..06acf6cf5a0db 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
[ ... ]
> @@ -1180,6 +1181,9 @@ static int i3c_hci_probe(struct platform_device *pdev)
>  	if (hci->quirks & HCI_QUIRK_RPM_IBI_ALLOWED)
>  		hci->master.rpm_ibi_allowed = true;
>  
> +	if (device_can_wakeup(i3c_hci_sysdev(&pdev->dev)))
> +		hci->master.ibi_wakeup = true;
> +

[Severity: Medium]
If we advertise ibi_wakeup here for non-PCI platform devices, how does the
suspend path know to actually arm the system wakeup?

While the PCI glue driver aggregates wakeup states by checking
i3c_master_has_wakeup_enabled_devs() during suspend, the native platform
suspend routine i3c_hci_suspend() does not appear to check this or enable
wakeups:

drivers/i3c/master/mipi-i3c-hci/core.c:i3c_hci_suspend() {
	struct i3c_hci *hci = dev_get_drvdata(dev);

	if (!(hci->quirks & HCI_QUIRK_RPM_ALLOWED))
		return 0;

	return pm_runtime_force_suspend(dev);
}

Does this mean IBI wakeups will silently fail to wake the system for non-PCI
platform devices, or is there another mechanism handling the wakeup
propagation for them?

>  	return i3c_master_register(&hci->master, &pdev->dev, &i3c_hci_ops, false);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=14

Re: [PATCH V4 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The driver dynamically overwrites the user-controlled `dev->power.should_wakeup` policy during suspend.
--

--- Patch [12]: [PATCH V4 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI ---
commit 317489fcad21bddef17def19e22782f047f99dc7
Author: Adrian Hunter <adrian.hunter@intel.com>

i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

This commit updates the driver to keep the PCI wakeup state aligned with
the wakeup requirements of the attached I3C devices. During suspend, it
checks whether any HCI instance has a wakeup-enabled device and alters the
PCI function wakeup state accordingly.

> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 5a9e2a43eff8d..42c172e82622f 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c

[ ... ]

> @@ -317,12 +329,19 @@ static int mipi_i3c_hci_pci_suspend(struct device *dev)
>  	if (!hci->info->control_instance_pm)
>  		return 0;
>  
> +	pm_data.can_wakeup = device_can_wakeup(dev);
> +
>  	ret = device_for_each_child_reverse(dev, &pm_data, mipi_i3c_hci_pci_suspend_instance);
> -	if (ret)
> +	if (ret) {
>  		for (int i = 0; i < pm_data.dev_cnt; i++)
>  			i3c_hci_rpm_resume(pm_data.dev[i]);
> +		return ret;
> +	}
>  
> -	return ret;
> +	if (device_may_wakeup(dev) != pm_data.may_wakeup)
> +		device_set_wakeup_enable(dev, pm_data.may_wakeup);

[Severity: Medium]
The commit message notes that the PCI device power/wakeup sysfs attribute no
longer provides independent wakeup control. However, does this approach
conflict with the PM core architecture expectation that the should_wakeup
state and device_set_wakeup_enable() belong strictly to userspace?

By dynamically altering the core device PM state here in
mipi_i3c_hci_pci_suspend(), the driver clobbers any user-configured wakeup
policy on every system or runtime suspend. Is there an alternative way to
handle the I3C wakeup propagation without overriding the sysfs UAPI contract?

> +
> +	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=12

Re: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration

[Adrian Hunter]

On 06/08/2026 16:39, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Dropping the bus maintenance lock around device_register() exposes the I3C device descriptor to concurrent Use-After-Free.
> 
> Pre-existing issues:
> - [High] Unbalanced fwnode reference count decrement in i3c_device_release() leads to Use-After-Free.
> --
> 
> --- Patch [1]: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration ---
> commit b31a8482aa2ea710ccc853d0c74b77edebb131ff
> Author: Adrian Hunter <adrian.hunter@intel.com>
> Date:   Thu Aug 6 13:18:44 2026 +0000
> 
>     i3c: master: Fix recursive locking during device registration
> 
> This commit addresses a recursive locking lockdep warning by decoupling
> device creation from registration. It collects newly populated devices into a
> temporary list under the maintenance lock, then releases the lock prior to
> invoking device_register() to prevent probe deadlocks. Finally, the lock is
> re-acquired to clean up any devices that failed registration.
> 
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
> 
> [ ... ]
> 
>> +		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
>> +	}
>> +
>> +	i3c_bus_maintenance_unlock(&master->bus);
>> +
>> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
>> +		ret = device_register(&i3cdev->dev);
> 
> [Severity: High]
> Does dropping the bus maintenance lock around device_register() expose the
> I3C device descriptor to a concurrent use-after-free?
> 
> During registration, the driver core can call subsystem functions like
> i3c_device_match(), which read i3cdev->desc locklessly:
> 
> drivers/i3c/master.c:i3c_device_match() {
>     ...
>     if (i3cdev->desc && i3cdev->desc->boardinfo)
>         static_addr_method = i3cdev->desc->boardinfo->static_addr_method;

The fix is in patch 7 "i3c: master: Fix potential UAF in i3c_device_match()"

>     ...
> }
> 
> Concurrently, an I3C Hot-Join or sysfs-triggered Dynamic Address Assignment
> (DAA) could execute i3c_master_do_daa(), take the i3c_bus_maintenance_lock,
> and call __i3c_master_add_i3c_dev_locked(). If DAA detects the device as a
> duplicate, it replaces the descriptor and frees the old one:
> 
> drivers/i3c/master.c:__i3c_master_add_i3c_dev_locked() {
>     ...
>     i3c_master_detach_i3c_dev(olddev);
>     i3c_master_free_i3c_dev(olddev);
>     ...
> }
> 
> If the registering thread is actively evaluating i3cdev->desc when this
> happens, will it lead to memory corruption when reading fields like
> desc->boardinfo?
> 
>> +		if (ret)
>> +			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
>> +		else
>> +			list_del_init(&i3cdev->node);
>> +	}
>> +
>> +	i3c_bus_maintenance_lock(&master->bus);
>> +
>> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
>> +		list_del(&i3cdev->node);
>> +		desc = i3cdev->desc;
>> +		i3cdev->desc = NULL;
>> +		put_device(&i3cdev->dev);
> 
> [Severity: High]
> This is a pre-existing issue, but does this error path cause an unbalanced
> fwnode reference count decrement leading to a use-after-free?
> 
> During device initialization, device_set_node() assigns the fwnode without
> incrementing its reference count:
> 
> drivers/i3c/master.c:i3c_master_register_new_i3c_devs() {
>     ...
>     if (desc->boardinfo)
>         device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
>     ...
> }
> 
> However, when put_device() is called here on registration failure,
> the release function unconditionally drops a reference:
> 
> drivers/i3c/master.c:i3c_device_release() {
>     ...
>     fwnode_handle_put(dev->fwnode);
>     ...
> }
> 
> Since the device object never acquired a reference to the boardinfo's fwnode,
> will repeated registration failures or unbind operations drop the refcount
> below zero, eventually prematurely freeing the fwnode?
> 
>> +		desc->dev = NULL;
>>  	}
>> +
>> +	i3c_bus_maintenance_unlock(&master->bus);
>>  }
> 

Re: [PATCH V4 07/14] i3c: master: Fix potential UAF in i3c_device_match()

[Adrian Hunter]

On 06/08/2026 16:45, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Removing the NULL check for `i3cdev->desc` introduces a deterministic NULL pointer dereference during device unregistration and registration error paths.
> - [High] `i3c_device_get_info()` dereferences `dev->desc` without checking for NULL under the lock, causing a NULL pointer dereference.
> --
> 
> --- Patch [7]: [PATCH V4 07/14] i3c: master: Fix potential UAF in i3c_device_match() ---
> commit 4b5a3c82a1c4c5f9bf31e925977c2286520960fc
> Author: Adrian Hunter <adrian.hunter@intel.com>
> 
> i3c: master: Fix potential UAF in i3c_device_match()
> 
> This commit protects access to i3cdev->desc with the normal-use lock to
> prevent a use-after-free when the descriptor pointer is concurrently
> replaced. Since the descriptor is assumed to remain valid while the lock is
> held, the NULL check is removed.
> 
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index ee771ac0b203e..83e04a2c202a9 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -347,8 +347,10 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
>>  	i3cdev = dev_to_i3cdev(dev);
>>  	i3cdrv = drv_to_i3cdrv(drv);
>>  
>> -	if (i3cdev->desc && i3cdev->desc->boardinfo)
>> +	i3c_bus_normaluse_lock(i3cdev->bus);
>> +	if (i3cdev->desc->boardinfo)
> 
> [Severity: High]
> Is it safe to remove the NULL check for i3cdev->desc here?
> 
> If a userspace process writes to a driver's bind file via sysfs concurrently
> with a device registration failure, bind_store() will call
> driver_match_device() and eventually reach i3c_device_match().
> 
> In the error path of i3c_master_register_new_i3c_devs(), i3cdev->desc is set

i3c_master_register_new_i3c_devs() adds the device, so the error path
means the device was not added, so it cannot concurrently be being bound.

> to NULL under the write lock. When this code subsequently acquires the read
> lock, i3cdev->desc will be NULL, which could cause a NULL pointer
> dereference.
> 
> Similarly, i3c_master_unregister_i3c_devs() locklessly sets i3cdev->desc to
> NULL during teardown, which can also race with this path.
> 
>>  		static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
>> +	i3c_bus_normaluse_unlock(i3cdev->bus);
>>  
>>  	/*
>>  	 * SETAASA-based devices need not always have a matching ID since
>>  	 * it is not mandatory for such devices to implement deviceinfo
>>  	 * CCC commands. Allow them to register through DT or ACPI.
>>  	 */
>>  	if (i3cdrv->id_table && i3c_device_match_id(i3cdev, i3cdrv->id_table))
> 
> [Severity: High]
> Does this call to i3c_device_match_id() have a similar vulnerability?
> 
> i3c_device_match_id() eventually calls i3c_device_get_info(). While
> i3c_device_get_info() does acquire the normal-use lock, it directly reads
> dev->desc->info without checking if dev->desc is valid.
> 
> If a bind operation races with device unregistration or a registration error

As above bind operation races with registration error is nonsense.

Unregistration path was fixed by "i3c: master: Fix info leak and UAF in
device unregister path" currently in i3c-next

> path, dev->desc could be set to NULL before the lock is acquired, leading to
> a crash when reading dev->desc->info.
> 
>>  		return 1;
>>  
>>  	if (static_addr_method & I3C_ADDR_METHOD_SETAASA) {
> 
> [ ... ]
> 

Re: [PATCH V4 09/14] i3c: master: Report wakeup events for IBIs

[Adrian Hunter]

On 06/08/2026 16:49, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] NULL pointer dereference in `i3c_master_queue_ibi()` when accessing `dev->dev`.

Refer:

	https://lore.kernel.org/all/59d05b05-37a6-498e-8dc4-3d4ba1c83d93@intel.com/

> 
> Pre-existing issues:
> - [High] Race condition in `i3c_master_queue_ibi()` leading to Use-After-Free of `dev->ibi`.
> --
> 
> --- Patch [9]: [PATCH V4 09/14] i3c: master: Report wakeup events for IBIs ---
> commit 461a49ac6bedf9467539aa8fac5583bf77ad5045
> Author: Adrian Hunter <adrian.hunter@intel.com>
> 
> i3c: master: Report wakeup events for IBIs
> 
> An I3C device configured as a wakeup source can wake the system by
> generating an In-Band Interrupt (IBI). When an IBI is queued for
> processing, record a wakeup event for the device if wakeup is enabled.
> 
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index 2616354dbe545..6c53414919445 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
> 
> [ ... ]
> 
>> @@ -3397,6 +3400,9 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
>>  	if (!dev->ibi || !slot)
>>  		return;
>>  
>> +	if (device_may_wakeup(&dev->dev->dev))
>> +		pm_wakeup_event(&dev->dev->dev, I3C_WAKEUP_PROCESSING_TIME_MS);
>> +
> 
> [Severity: High]
> Can dev->dev be NULL here?
> 
> Looking at i3c_master_unregister_i3c_devs(), it sets i3cdev->dev to NULL
> during device teardown:
> 
> drivers/i3c/master.c:i3c_master_unregister_i3c_devs() {
>     ...
> 		i3cdev->dev->desc = NULL;
> 		put_device(&i3cdev->dev->dev);
> 		i3cdev->dev = NULL;
> 	}
> }
> 
> If a hardware IBI is received during or shortly after this teardown,
> wouldn't accessing &dev->dev->dev cause a NULL pointer dereference when
> evaluated inside device_may_wakeup()?
> 
>>  	atomic_inc(&dev->ibi->pending_ibis);
> 
> [Severity: High]
> This is a pre-existing issue, but does the lack of synchronization here allow
> a use-after-free of dev->ibi?
> 
> The teardown path in i3c_dev_disable_ibi_locked() checks pending_ibis
> locklessly and bypasses waiting if it reads 0:
> 
> drivers/i3c/master.c:i3c_dev_disable_ibi_locked() {
>     ...
> 	if (atomic_read(&dev->ibi->pending_ibis))
> 		wait_for_completion(&dev->ibi->all_ibis_handled);
>     ...
> }
> 
> If an ISR is executing concurrently and passes the check for !dev->ibi at
> the top of i3c_master_queue_ibi(), but hasn't yet reached the atomic_inc()
> here, the teardown thread could see pending_ibis == 0 and proceed to free
> dev->ibi in i3c_dev_free_ibi_locked():
> 
> drivers/i3c/master.c:i3c_dev_free_ibi_locked() {
>     ...
> 	kfree(dev->ibi);
> 	dev->ibi = NULL;
>     ...
> }
> 
> When the ISR resumes, wouldn't the subsequent accesses to
> dev->ibi->pending_ibis and dev->ibi->wq cause a use-after-free? Adding
> the wakeup event logic slightly widens this existing race window.
> 
>>  	queue_work(dev->ibi->wq, &slot->work);
>>  }
> 

Re: [PATCH V4 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

[Adrian Hunter]

On 06/08/2026 16:56, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The driver dynamically overwrites the user-controlled `dev->power.should_wakeup` policy during suspend.
> --
> 
> --- Patch [12]: [PATCH V4 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI ---
> commit 317489fcad21bddef17def19e22782f047f99dc7
> Author: Adrian Hunter <adrian.hunter@intel.com>
> 
> i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI
> 
> This commit updates the driver to keep the PCI wakeup state aligned with
> the wakeup requirements of the attached I3C devices. During suspend, it
> checks whether any HCI instance has a wakeup-enabled device and alters the
> PCI function wakeup state accordingly.
> 
>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> index 5a9e2a43eff8d..42c172e82622f 100644
>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> 
> [ ... ]
> 
>> @@ -317,12 +329,19 @@ static int mipi_i3c_hci_pci_suspend(struct device *dev)
>>  	if (!hci->info->control_instance_pm)
>>  		return 0;
>>  
>> +	pm_data.can_wakeup = device_can_wakeup(dev);
>> +
>>  	ret = device_for_each_child_reverse(dev, &pm_data, mipi_i3c_hci_pci_suspend_instance);
>> -	if (ret)
>> +	if (ret) {
>>  		for (int i = 0; i < pm_data.dev_cnt; i++)
>>  			i3c_hci_rpm_resume(pm_data.dev[i]);
>> +		return ret;
>> +	}
>>  
>> -	return ret;
>> +	if (device_may_wakeup(dev) != pm_data.may_wakeup)
>> +		device_set_wakeup_enable(dev, pm_data.may_wakeup);
> 
> [Severity: Medium]
> The commit message notes that the PCI device power/wakeup sysfs attribute no
> longer provides independent wakeup control. However, does this approach
> conflict with the PM core architecture expectation that the should_wakeup
> state and device_set_wakeup_enable() belong strictly to userspace?

Not really.  The user sets policy via I3C devices' wakeup attributes not the
bus controller's wakeup attribute.

> 
> By dynamically altering the core device PM state here in
> mipi_i3c_hci_pci_suspend(), the driver clobbers any user-configured wakeup
> policy on every system or runtime suspend. Is there an alternative way to
> handle the I3C wakeup propagation without overriding the sysfs UAPI contract?

Not really overriding, and no alternative.

> 
>> +
>> +	return 0;
>>  }
> 

Re: [PATCH V4 14/14] i3c: mipi-i3c-hci: Advertise IBI wakeup capability

[Adrian Hunter]

On 06/08/2026 16:54, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The platform driver sets the I3C master `ibi_wakeup` flag but fails to propagate the wakeup state during system suspend for non-PCI platform devices, breaking IBI wakeup functionality.
> --
> 
> --- Patch [14]: [PATCH V4 14/14] i3c: mipi-i3c-hci: Advertise IBI wakeup capability ---
> commit 443a5c0d5859df8fbad91b6e456865274a7f7ca9
> Author: Adrian Hunter <adrian.hunter@intel.com>
> 
> i3c: mipi-i3c-hci: Advertise IBI wakeup capability
> 
> This commit sets master->ibi_wakeup during probe when the associated system
> device advertises wakeup capability. This allows the I3C core to mark
> IBI-capable I3C devices as wakeup capable.
> 
>> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
>> index f95da427d7a9e..06acf6cf5a0db 100644
>> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
>> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> [ ... ]
>> @@ -1180,6 +1181,9 @@ static int i3c_hci_probe(struct platform_device *pdev)
>>  	if (hci->quirks & HCI_QUIRK_RPM_IBI_ALLOWED)
>>  		hci->master.rpm_ibi_allowed = true;
>>  
>> +	if (device_can_wakeup(i3c_hci_sysdev(&pdev->dev)))
>> +		hci->master.ibi_wakeup = true;
>> +
> 
> [Severity: Medium]
> If we advertise ibi_wakeup here for non-PCI platform devices, how does the
> suspend path know to actually arm the system wakeup?
> 
> While the PCI glue driver aggregates wakeup states by checking
> i3c_master_has_wakeup_enabled_devs() during suspend, the native platform
> suspend routine i3c_hci_suspend() does not appear to check this or enable
> wakeups:
> 
> drivers/i3c/master/mipi-i3c-hci/core.c:i3c_hci_suspend() {
> 	struct i3c_hci *hci = dev_get_drvdata(dev);
> 
> 	if (!(hci->quirks & HCI_QUIRK_RPM_ALLOWED))
> 		return 0;
> 
> 	return pm_runtime_force_suspend(dev);
> }
> 
> Does this mean IBI wakeups will silently fail to wake the system for non-PCI
> platform devices, or is there another mechanism handling the wakeup
> propagation for them?

It is assumed that platform firmware will not be amended to advertise
controller wakeup capability before that is supported.

> 
>>  	return i3c_master_register(&hci->master, &pdev->dev, &i3c_hci_ops, false);
>>  }
> 

Re: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration

[Frank Li]

On Thu, Aug 06, 2026 at 04:18:44PM +0300, Adrian Hunter wrote:
> i3c_master_register_new_i3c_devs() registers newly discovered devices
> while holding i3c_bus_normaluse_lock(), a down_read().  device_register()
> can immediately probe the device, and probe callbacks typically invoke
> I3C helpers that take i3c_bus_normaluse_lock() again, leading to a
> recursive acquisition of the same rwsem.  rwsems do not support recursive
> read locking and can deadlock when a writer is waiting.  See the
> "Recursive read locks" section of Documentation/locking/lockdep-design.rst.
>
> For example, with Intel LPSS I3C, LOCKDEP generates a WARNING like:
>   # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/unbind
>   # echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/bind
>   WARNING: possible recursive locking detected
>   kworker/5:1/94 is trying to acquire lock:
>   ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_device_match_id+0x45/0x370
>   but task is already holding lock:
>   ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_master_reg_work_fn+0x21/0x5f0
>
> Fix this by separating device creation from device registration.
> Populate desc->dev under the maintenance lock, collect the devices that
> still need registration into a local list, then release the lock before
> calling device_register().  Finally retake the lock and clean up any
> devices that failed to register.
>
> Use the maintenance lock rather than the normal-use lock while adding
> device objects.  A write-side maintenance lock prevents readers from
> observing a partially initialized desc->dev during initial device
> population, or desc->dev disappearing if registration fails.
>
> The local list requires a list node, so add a list node member to struct
> i3c_device.
>
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V4:
>
> 	Expanded the kernel-doc for the new struct i3c_device @node
> 	member to note that it is only for use by
> 	i3c_master_register_new_i3c_devs() and is not protected by a
> 	lock.
>
> Changes in V3:
>
> 	Added Cc: stable@vger.kernel.org
>
> Changes in V2:
>
> 	New patch
>
>
>  drivers/i3c/master.c       | 45 ++++++++++++++++++++++++++++----------
>  include/linux/i3c/master.h |  3 +++
>  2 files changed, 36 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index f485b98805cf..d2fb1a110521 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2069,12 +2069,21 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
>  static void
>  i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>  {
> +	struct i3c_device *i3cdev, *tmp;
>  	struct i3c_dev_desc *desc;
> +	LIST_HEAD(i3c_unreg_devs);
>  	int ret;
>
>  	if (!master->init_done)
>  		return;
>
> +	i3c_bus_maintenance_lock(&master->bus);
> +
> +	if (master->shutting_down) {
> +		i3c_bus_maintenance_unlock(&master->bus);
> +		return;
> +	}
> +
>  	i3c_bus_for_each_i3cdev(&master->bus, desc) {
>  		if (desc->dev || !desc->info.dyn_addr || desc == master->this)
>  			continue;
> @@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>  		if (desc->boardinfo)
>  			device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
>
> -		ret = device_register(&desc->dev->dev);
> -		if (ret) {
> -			dev_err(&master->dev,
> -				"Failed to add I3C device (err = %d)\n", ret);
> -			desc->dev->desc = NULL;
> -			put_device(&desc->dev->dev);
> -			desc->dev = NULL;
> -		}
> +		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
> +	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		ret = device_register(&i3cdev->dev);
> +		if (ret)
> +			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
> +		else
> +			list_del_init(&i3cdev->node);
> +	}
> +
> +	i3c_bus_maintenance_lock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		list_del(&i3cdev->node);
> +		desc = i3cdev->desc;
> +		i3cdev->desc = NULL;
> +		put_device(&i3cdev->dev);
> +		desc->dev = NULL;
>  	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
>  }
>
>  static void i3c_master_reg_work_fn(struct work_struct *work)
>  {
>  	struct i3c_master_controller *master = container_of(work, typeof(*master), reg_work);
>
> -	i3c_bus_normaluse_lock(&master->bus);
> -	if (!master->shutting_down)
> -		i3c_master_register_new_i3c_devs(master);
> -	i3c_bus_normaluse_unlock(&master->bus);
> +	i3c_master_register_new_i3c_devs(master);
>  }
>
>  /**
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index 2dc139a217bf..26535beb1e77 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -238,6 +238,8 @@ struct i3c_dev_desc {
>   *	  every time the I3C device is rediscovered with a different dynamic
>   *	  address assigned
>   * @bus: I3C bus this device is attached to
> + * @node: unregistered device list node, only for use by
> + *	  i3c_master_register_new_i3c_devs(), it is not protected by a lock
>   *
>   * I3C device object exposed to I3C device drivers. The takes care of linking
>   * this object to the relevant &struct_i3c_dev_desc one.
> @@ -248,6 +250,7 @@ struct i3c_device {
>  	struct device dev;
>  	struct i3c_dev_desc *desc;
>  	struct i3c_bus *bus;
> +	struct list_head node;
>  };
>
>  /*
> --
> 2.53.0
>

Re: [PATCH V4 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode()

[Frank Li]

On Thu, Aug 06, 2026 at 04:18:45PM +0300, Adrian Hunter wrote:
> i3c_device_get_supported_xfer_mode() uses dev->desc to obtain the
> master controller.  However, dev->desc must not be dereferenced unless
> bus->lock is held, and this function does not take that lock.
>
> The function only needs access to the master controller associated with
> the device's bus.  Use dev->bus instead, which is always valid for the
> lifetime of the device and does not require dereferencing dev->desc.
>
> Fixes: 256a21743d91 ("i3c: Add HDR API support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V4:
>
> 	None
>
> Changes in V3:
>
> 	New patch
>
>
>  drivers/i3c/device.c    | 2 +-
>  drivers/i3c/internals.h | 5 +++++
>  drivers/i3c/master.c    | 6 ------
>  3 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index 101eaa77de68..a3778282e84c 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -309,7 +309,7 @@ EXPORT_SYMBOL_GPL(i3c_device_match_id);
>   */
>  u32 i3c_device_get_supported_xfer_mode(struct i3c_device *dev)
>  {
> -	return i3c_dev_get_master(dev->desc)->this->info.hdr_cap | BIT(I3C_SDR);
> +	return i3c_bus_to_i3c_master(dev->bus)->this->info.hdr_cap | BIT(I3C_SDR);
>  }
>  EXPORT_SYMBOL_GPL(i3c_device_get_supported_xfer_mode);
>
> diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
> index 0f1f3f766623..86a36b951e0d 100644
> --- a/drivers/i3c/internals.h
> +++ b/drivers/i3c/internals.h
> @@ -72,4 +72,9 @@ static inline void i3c_readl_fifo(const void __iomem *addr, void *buf,
>  	}
>  }
>
> +static inline struct i3c_master_controller *i3c_bus_to_i3c_master(struct i3c_bus *i3cbus)
> +{
> +	return container_of(i3cbus, struct i3c_master_controller, bus);
> +}
> +
>  #endif /* I3C_INTERNAL_H */
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index d2fb1a110521..c7bb52b71d88 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -102,12 +102,6 @@ void i3c_bus_normaluse_unlock(struct i3c_bus *bus)
>  	up_read(&bus->lock);
>  }
>
> -static struct i3c_master_controller *
> -i3c_bus_to_i3c_master(struct i3c_bus *i3cbus)
> -{
> -	return container_of(i3cbus, struct i3c_master_controller, bus);
> -}
> -
>  static struct i3c_master_controller *dev_to_i3cmaster(struct device *dev)
>  {
>  	return container_of(dev, struct i3c_master_controller, dev);
> --
> 2.53.0
>

Re: [PATCH V4 04/14] i3c: master: Fix use-after-free of master->this

[Frank Li]

On Thu, Aug 06, 2026 at 04:18:47PM +0300, Adrian Hunter wrote:
> sysfs attribute callbacks for the master controller device dereference
> master->this.  However, master->this is freed in
> i3c_master_detach_free_devs() before the master device itself is
> released.
>
> As a result, sysfs accesses can dereference a freed master->this
> pointer, leading to a use-after-free.
>
> Keep master->this alive until i3c_masterdev_release(), which is called
> after the master device and its sysfs state are being torn down. Do not
> free master->this as part of the normal device detach path.
>
> On the error path in i3c_master_set_info(), reset master->this and
> bus.cur_master to NULL before freeing the allocated device.
>
> Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V4:
>
> 	Also reset master->this and bus.cur_master to NULL on the
> 	i3c_master_set_info() error path before freeing the allocated
> 	device.  Tidied up the commit message wording.
>
> Changes in V3:
>
> 	New patch
>
>
>  drivers/i3c/master.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index abb582645a2e..2357874bb9d6 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -842,6 +842,11 @@ static struct attribute *i3c_masterdev_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(i3c_masterdev);
>
> +static void i3c_master_free_i3c_dev(struct i3c_dev_desc *dev)
> +{
> +	kfree(dev);
> +}
> +
>  static void i3c_masterdev_release(struct device *dev)
>  {
>  	struct i3c_master_controller *master = dev_to_i3cmaster(dev);
> @@ -854,6 +859,8 @@ static void i3c_masterdev_release(struct device *dev)
>  	i3c_bus_cleanup(bus);
>
>  	fwnode_handle_put(dev->fwnode);
> +
> +	i3c_master_free_i3c_dev(master->this);
>  }
>
>  static const struct device_type i3c_masterdev_type = {
> @@ -1125,11 +1132,6 @@ static void i3c_device_release(struct device *dev)
>  	kfree(i3cdev);
>  }
>
> -static void i3c_master_free_i3c_dev(struct i3c_dev_desc *dev)
> -{
> -	kfree(dev);
> -}
> -
>  static struct i3c_dev_desc *
>  i3c_master_alloc_i3c_dev(struct i3c_master_controller *master,
>  			 const struct i3c_device_info *info)
> @@ -2266,6 +2268,8 @@ int i3c_master_set_info(struct i3c_master_controller *master,
>  	return 0;
>
>  err_free_dev:
> +	master->bus.cur_master = NULL;
> +	master->this = NULL;
>  	i3c_master_free_i3c_dev(i3cdev);
>
>  	return ret;
> @@ -2286,7 +2290,8 @@ static void i3c_master_detach_free_devs(struct i3c_master_controller *master)
>  					i3cdev->boardinfo->init_dyn_addr,
>  					I3C_ADDR_SLOT_FREE);
>
> -		i3c_master_free_i3c_dev(i3cdev);
> +		if (i3cdev != master->this)
> +			i3c_master_free_i3c_dev(i3cdev);
>  	}
>
>  	list_for_each_entry_safe(i2cdev, i2ctmp, &master->bus.devs.i2c,
> --
> 2.53.0
>

Re: [PATCH V4 05/14] i3c: Make dev->desc locking assumptions explicit

[Frank Li]

On Thu, Aug 06, 2026 at 04:18:48PM +0300, Adrian Hunter wrote:
> i3c_device_get_info() takes the bus normal-use lock before accessing
> dev->desc. Under that lock, the descriptor pointer is guaranteed to be
> valid for the duration of the access.
>
> Remove the unnecessary NULL check on dev->desc so the code more clearly
> reflects the locking rules and expected descriptor lifetime.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Acked-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V4:
>
> 	Added Mukesh's Acked-by tag
>
> Changes in V3:
>
> 	New patch
>
>
>  drivers/i3c/device.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index a3778282e84c..5e6df6de0283 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -101,8 +101,7 @@ void i3c_device_get_info(const struct i3c_device *dev,
>  		return;
>
>  	i3c_bus_normaluse_lock(dev->bus);
> -	if (dev->desc)
> -		*info = dev->desc->info;
> +	*info = dev->desc->info;
>  	i3c_bus_normaluse_unlock(dev->bus);
>  }
>  EXPORT_SYMBOL_GPL(i3c_device_get_info);
> --
> 2.53.0
>

Re: [PATCH V4 06/14] i3c: master: Fix potential UAF in i3c_device_uevent()

[Frank Li]

On Thu, Aug 06, 2026 at 04:18:49PM +0300, Adrian Hunter wrote:
> i3c_device_uevent() dereferences i3cdev->desc without holding the bus
> normal-use lock.  Since the descriptor pointer can be replaced
> concurrently, including when a uevent is generated from sysfs, this can
> result in dereferencing a stale descriptor and lead to a use-after-free.
>
> Use i3c_device_get_info() instead, which protects access to the
> descriptor with the normal-use lock.
>
> Commit 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling
> i3c_device_get_info() to avoid deadlock") replaced the accessor with a
> direct descriptor dereference because i3c_device_get_info() would
> recursively acquire bus->lock during device registration.
>
> This change depends on "i3c: master: Fix recursive locking during device
> registration", which moves device registration out from under bus->lock
> and removes the possibility of that deadlock.  Without that change,
> restoring the i3c_device_get_info() call would reintroduce the deadlock.
>
> Fixes: 6cf7b65f7029 ("i3c: Use i3cdev->desc->info instead of calling i3c_device_get_info() to avoid deadlock")
> Cc: stable@vger.kernel.org # requires "i3c: master: Fix recursive locking during device registration"
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Acked-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V4:
>
> 	Added Mukesh's Acked-by tag
>
> Changes in V3:
>
> 	New patch
>
>
>  drivers/i3c/master.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 2357874bb9d6..ee771ac0b203 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -316,8 +316,7 @@ static int i3c_device_uevent(const struct device *dev, struct kobj_uevent_env *e
>  	struct i3c_device_info devinfo;
>  	u16 manuf, part, ext;
>
> -	if (i3cdev->desc)
> -		devinfo = i3cdev->desc->info;
> +	i3c_device_get_info(i3cdev, &devinfo);
>  	manuf = I3C_PID_MANUF_ID(devinfo.pid);
>  	part = I3C_PID_PART_ID(devinfo.pid);
>  	ext = I3C_PID_EXTRA_INFO(devinfo.pid);
> --
> 2.53.0
>

Re: [PATCH V4 09/14] i3c: master: Report wakeup events for IBIs

[Frank Li]

On Thu, Aug 06, 2026 at 04:18:52PM +0300, Adrian Hunter wrote:
> An I3C device configured as a wakeup source can wake the system by
> generating an In-Band Interrupt (IBI).
>
> When an IBI is queued for processing, record a wakeup event for the
> device if wakeup is enabled.  Use a 100 ms processing interval to give
> the I3C device driver time to process the IBI.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V4:
>
> 	None
>
> Changes in V2 and V3:
>
> 	None
>
>
>  drivers/i3c/master.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 2616354dbe54..6c5341491944 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -3384,6 +3384,9 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master)
>  	}
>  }
>
> +/* Approximate time for IBI handler to run */
> +#define I3C_WAKEUP_PROCESSING_TIME_MS 100
> +
>  /**
>   * i3c_master_queue_ibi() - Queue an IBI
>   * @dev: the device this IBI is coming from
> @@ -3397,6 +3400,9 @@ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot)
>  	if (!dev->ibi || !slot)
>  		return;
>
> +	if (device_may_wakeup(&dev->dev->dev))
> +		pm_wakeup_event(&dev->dev->dev, I3C_WAKEUP_PROCESSING_TIME_MS);
> +
>  	atomic_inc(&dev->ibi->pending_ibis);
>  	queue_work(dev->ibi->wq, &slot->work);
>  }
> --
> 2.53.0
>

Re: [PATCH V4 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI

[Frank Li]

On Thu, Aug 06, 2026 at 04:18:55PM +0300, Adrian Hunter wrote:
> Keep the PCI wakeup state aligned with the wakeup requirements of the
> devices served by the controller(s).
>
> The PCI function is the wakeup source for HCI instances exposed beneath
> it.  However, wakeup is only needed when at least one attached I3C device
> is enabled as a wakeup source.
>
> During suspend, check whether any HCI instance has a wakeup-enabled I3C
> device and enable wakeup for the PCI function only in that case.
> Otherwise leave PCI wakeup disabled.
>
> Note, the suspend callback is used for both system and runtime suspend.
> Although this change may update the PCI wakeup state during runtime
> suspend, it does so only when the required wakeup state changes.
> Moreover, PCI wakeup-capable devices already have PME wakeup armed for
> runtime suspend, so changing the wakeup-enabled state does not affect
> runtime PM wakeup behavior.
>
> Note also, since the PCI wakeup state is derived from the wakeup
> configuration of the attached I3C devices, the PCI device power/wakeup
> sysfs attribute no longer provides independent wakeup control.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>
>
> Changes in V4:
>
> 	Updated for the rename of i3c_master_any_wakeup_enabled() to
> 	i3c_master_has_wakeup_enabled_devs().
>
> Changes in V2 and V3:
>
> 	None
>
>
>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 23 +++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 5a9e2a43eff8..42c172e82622 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -265,6 +265,8 @@ static bool mipi_i3c_hci_pci_is_operational(struct device *dev, bool update)
>  struct mipi_i3c_hci_pci_pm_data {
>  	struct device *dev[INST_MAX];
>  	int dev_cnt;
> +	bool can_wakeup;
> +	bool may_wakeup;
>  };
>
>  static bool mipi_i3c_hci_pci_is_mfd(struct device *dev)
> @@ -272,6 +274,13 @@ static bool mipi_i3c_hci_pci_is_mfd(struct device *dev)
>  	return dev_is_platform(dev) && mfd_get_cell(to_platform_device(dev));
>  }
>
> +static bool mipi_i3c_hci_pci_any_wakeup_enabled(struct device *dev)
> +{
> +	struct i3c_hci *hci = dev_get_drvdata(dev);
> +
> +	return i3c_master_has_wakeup_enabled_devs(&hci->master);
> +}
> +
>  static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *data)
>  {
>  	struct mipi_i3c_hci_pci_pm_data *pm_data = data;
> @@ -287,6 +296,9 @@ static int mipi_i3c_hci_pci_suspend_instance(struct device *dev, void *data)
>
>  	pm_data->dev[pm_data->dev_cnt++] = dev;
>
> +	if (pm_data->can_wakeup && mipi_i3c_hci_pci_any_wakeup_enabled(dev))
> +		pm_data->may_wakeup = true;
> +
>  	return 0;
>  }
>
> @@ -317,12 +329,19 @@ static int mipi_i3c_hci_pci_suspend(struct device *dev)
>  	if (!hci->info->control_instance_pm)
>  		return 0;
>
> +	pm_data.can_wakeup = device_can_wakeup(dev);
> +
>  	ret = device_for_each_child_reverse(dev, &pm_data, mipi_i3c_hci_pci_suspend_instance);
> -	if (ret)
> +	if (ret) {
>  		for (int i = 0; i < pm_data.dev_cnt; i++)
>  			i3c_hci_rpm_resume(pm_data.dev[i]);
> +		return ret;
> +	}
>
> -	return ret;
> +	if (device_may_wakeup(dev) != pm_data.may_wakeup)
> +		device_set_wakeup_enable(dev, pm_data.may_wakeup);
> +
> +	return 0;
>  }
>
>  static int mipi_i3c_hci_pci_resume(struct device *dev)
> --
> 2.53.0
>

Re: [PATCH V4 13/14] i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()

[Frank Li]

On Thu, Aug 06, 2026 at 04:18:56PM +0300, Adrian Hunter wrote:
> The MIPI I3C HCI driver needs to identify the underlying system device
> used for DMA mapping and PM operations. The logic for determining that
> device is currently embedded in the DMA implementation.
>
> Factor this code out into i3c_hci_sysdev() so it can be shared by other
> parts of the driver and keep the device-selection logic in one place.
>
> The explanatory comment moves with the code, reworked as kernel-doc now
> that it documents a function rather than an inline block.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V4:
>
> 	Reworked the comment that moves with the code into kernel-doc,
> 	and mentioned that in the commit message.
>
> Changes in V2 and V3:
>
> 	None
>
>
>  drivers/i3c/master/mipi-i3c-hci/core.c | 16 ++++++++++++++++
>  drivers/i3c/master/mipi-i3c-hci/dma.c  | 15 +--------------
>  drivers/i3c/master/mipi-i3c-hci/hci.h  |  2 ++
>  3 files changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index cfe9b5390b56..f95da427d7a9 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -15,6 +15,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/iopoll.h>
>  #include <linux/module.h>
> +#include <linux/pci.h>
>  #include <linux/platform_data/mipi-i3c-hci.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> @@ -117,6 +118,21 @@ static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
>  	return container_of(m, struct i3c_hci, master);
>  }
>
> +/**
> + * i3c_hci_sysdev() - Get the device to use for DMA
> + * @dev: Device the HCI controller is bound to
> + *
> + * When an IOMMU is enabled, DMA API calls must use the device that IOMMU
> + * setup was done for.  Under PCI enumeration that is the PCI device, not
> + * the "mipi-i3c-hci" platform device below it.
> + *
> + * Return: @dev's parent if it is a PCI device, otherwise @dev.
> + */
> +struct device *i3c_hci_sysdev(struct device *dev)
> +{
> +	return dev->parent && dev_is_pci(dev->parent) ? dev->parent : dev;
> +}
> +
>  static void i3c_hci_set_master_dyn_addr(struct i3c_hci *hci)
>  {
>  	reg_write(MASTER_DEVICE_ADDR,
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 0672ed1132f8..7c2b20474130 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -15,7 +15,6 @@
>  #include <linux/errno.h>
>  #include <linux/i3c/master.h>
>  #include <linux/io.h>
> -#include <linux/pci.h>
>
>  #include "hci.h"
>  #include "cmd.h"
> @@ -301,23 +300,11 @@ static int hci_dma_init(struct i3c_hci *hci)
>  {
>  	struct hci_rings_data *rings;
>  	struct hci_rh_data *rh;
> -	struct device *sysdev;
>  	u32 regval;
>  	unsigned int i, nr_rings, xfers_sz, resps_sz;
>  	unsigned int ibi_status_ring_sz, ibi_data_ring_sz;
>  	int ret;
>
> -	/*
> -	 * Set pointer to a physical device that does DMA and has IOMMU setup
> -	 * done for it in case of enabled IOMMU and use it with the DMA API.
> -	 * Here such device is either
> -	 * "mipi-i3c-hci" platform device (OF/ACPI enumeration) parent or
> -	 * grandparent (PCI enumeration).
> -	 */
> -	sysdev = hci->master.dev.parent;
> -	if (sysdev->parent && dev_is_pci(sysdev->parent))
> -		sysdev = sysdev->parent;
> -
>  	regval = rhs_reg_read(CONTROL);
>  	nr_rings = FIELD_GET(MAX_HEADER_COUNT_CAP, regval);
>  	dev_dbg(&hci->master.dev, "%d DMA rings available\n", nr_rings);
> @@ -332,7 +319,7 @@ static int hci_dma_init(struct i3c_hci *hci)
>  		return -ENOMEM;
>  	hci->io_data = rings;
>  	rings->total = nr_rings;
> -	rings->sysdev = sysdev;
> +	rings->sysdev = i3c_hci_sysdev(hci->master.dev.parent);
>
>  	for (i = 0; i < rings->total; i++) {
>  		u32 offset = rhs_reg_read(RHn_OFFSET(i));
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index b3d9803b1968..b8d2a3d680f8 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -184,6 +184,8 @@ void amd_set_resp_buf_thld(struct i3c_hci *hci);
>  void i3c_hci_sync_irq_inactive(struct i3c_hci *hci);
>  int i3c_hci_process_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
>
> +struct device *i3c_hci_sysdev(struct device *dev);
> +
>  #define DEFAULT_AUTOSUSPEND_DELAY_MS 1000
>
>  int i3c_hci_rpm_suspend(struct device *dev);
> --
> 2.53.0
>

Re: [PATCH V4 10/14] i3c: master: Add helper to query bus wakeup requirements

[Frank Li]

On Thu, Aug 06, 2026 at 04:18:53PM +0300, Adrian Hunter wrote:
> Add i3c_master_has_wakeup_enabled_devs(), which iterates over the devices
> on an I3C bus and reports whether any of them are enabled for system
> wakeup and have IBI enabled.
>
> Controller drivers can use this helper to determine whether wakeup
> support must remain available while the system is suspended.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V4:
>
> 	Renamed i3c_master_any_wakeup_enabled() to
> 	i3c_master_has_wakeup_enabled_devs().
>
> Changes in V3:
>
> 	Skip the master device explicitly.  Added kernel-doc noting
> 	that wakeup enablement is user space policy, so the helper is
> 	meant to be called from a system suspend callback.
>
> Changes in V2:
>
> 	i3c_master_any_wakeup_enabled() now also requires the device to
> 	have IBI enabled, not just system wakeup enabled, so that a
> 	device with no active IBI request does not keep PCI PME enabled.
> 	desc->ibi_lock is taken while checking.  The commit message and
> 	kernel-doc are updated to match.
>
>
>  drivers/i3c/master.c       | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/i3c/master.h |  1 +
>  2 files changed, 36 insertions(+)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 6c5341491944..afcd7a21a3e6 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2147,6 +2147,41 @@ static void i3c_master_reg_work_fn(struct work_struct *work)
>  	i3c_master_register_new_i3c_devs(master);
>  }
>
> +/**
> + * i3c_master_has_wakeup_enabled_devs() - check if any device can wake the system
> + * @master: I3C master controller
> + *
> + * Iterate over devices on the bus and return true if any device has
> + * system wakeup enabled and IBI enabled.
> + *
> + * Whether a device is enabled for system wakeup is user space policy,
> + * settable at any time through the device's power/wakeup sysfs attribute,
> + * so the answer is only stable once user space is frozen.  Call this from
> + * a system suspend callback.
> + *
> + * Return: true if any device may wake the system via IBI, false otherwise.
> + */
> +bool i3c_master_has_wakeup_enabled_devs(struct i3c_master_controller *master)
> +{
> +	struct i3c_dev_desc *desc;
> +	bool wakeup = false;
> +
> +	i3c_bus_normaluse_lock(&master->bus);
> +	i3c_bus_for_each_i3cdev(&master->bus, desc) {
> +		if (!desc->dev || desc == master->this || !device_may_wakeup(&desc->dev->dev))
> +			continue;
> +		guard(mutex)(&desc->ibi_lock);
> +		if (desc->ibi && desc->ibi->enabled) {
> +			wakeup = true;
> +			break;
> +		}
> +	}
> +	i3c_bus_normaluse_unlock(&master->bus);
> +
> +	return wakeup;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_has_wakeup_enabled_devs);
> +
>  /**
>   * i3c_master_dma_map_single() - Map buffer for single DMA transfer
>   * @dev: device object of a device doing DMA
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index 9d675d01522c..82d9886e7f12 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -765,6 +765,7 @@ void i3c_generic_ibi_recycle_slot(struct i3c_generic_ibi_pool *pool,
>  				  struct i3c_ibi_slot *slot);
>
>  void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot);
> +bool i3c_master_has_wakeup_enabled_devs(struct i3c_master_controller *master);
>
>  struct i3c_ibi_slot *i3c_master_get_free_ibi_slot(struct i3c_dev_desc *dev);
>
> --
> 2.53.0
>

Re: [PATCH V4 08/14] i3c: master: Support IBI-based wakeup capability

[Mukesh Savaliya]

On 8/6/2026 6:48 PM, Adrian Hunter wrote:
> An I3C controller acts as a bus controller for one or more I3C devices.
> If the controller can wake the system in response to an In-Band
> Interrupt (IBI), then any device on that bus that is capable of
> generating IBIs can potentially be used as a wakeup source.
> 
> Add an ibi_wakeup flag to struct i3c_master_controller so controller
> drivers can advertise support for IBI-based wakeup.
> 
> If set, mark IBI-capable I3C devices as wakeup capable when they are
> registered, allowing wakeup management through the standard device
> wakeup framework.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
Acked-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>

Re: [PATCH V4 10/14] i3c: master: Add helper to query bus wakeup requirements

[Mukesh Savaliya]

On 8/6/2026 6:48 PM, Adrian Hunter wrote:
> Add i3c_master_has_wakeup_enabled_devs(), which iterates over the devices
> on an I3C bus and reports whether any of them are enabled for system
> wakeup and have IBI enabled.
> 
> Controller drivers can use this helper to determine whether wakeup
> support must remain available while the system is suspended.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Acked-by : Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>

Re: [PATCH V4 13/14] i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev()

[Mukesh Savaliya]

On 8/6/2026 6:48 PM, Adrian Hunter wrote:
> The MIPI I3C HCI driver needs to identify the underlying system device
> used for DMA mapping and PM operations. The logic for determining that
> device is currently embedded in the DMA implementation.
> 
> Factor this code out into i3c_hci_sysdev() so it can be shared by other
> parts of the driver and keep the device-selection logic in one place.
> 
> The explanatory comment moves with the code, reworked as kernel-doc now
> that it documents a function rather than an inline block.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Acked-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>