i3c: Support IBI-based system wakeup

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 i3c_device_uevent() and driver probe functions to a Use-After-Free if a concurrent DAA replaces the device.

Pre-existing issues:
- [High] i3c_device_uevent() leaks uninitialized stack memory to userspace when i3cdev->desc is NULL. --

--- Patch [1]: [PATCH V2 1/8] i3c: master: Fix recursive locking during device registration --- commit 33a4dcd6c5626faef95bc7cd02ec4c56c6710c78
Author: Adrian Hunter <[email protected]>

i3c: master: Fix recursive locking during device registration

This commit separates device creation from registration to fix a recursive locking issue. It drops the bus maintenance lock around device_register() to prevent deadlocks when probe callbacks invoke I3C helpers.

drivers/i3c/master.c +0 -0

index fd3e79d10c840..874f6e11b296d 100644
[ ... ]

Quoted 15 lines

(quoted diff) +3 -8

@@ -1999,25 +2008,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 		if (desc->boardinfo)
 			desc->dev->dev.of_node = desc->boardinfo->of_node;
 
-		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);
+	}
+

Review i3c_master_register_new_i3c_devs(struct i3c_master_controller *master) · line 2017

Quoted 4 lines

(quoted diff) +4 -0

+	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 maintenance lock around device_register() expose i3c_device_uevent() and driver probes to a use-after-free?

If a concurrent I3C hot-join event or a sysfs write to do_daa occurs while device_register() is running without the lock, __i3c_master_add_i3c_dev_locked() could find a duplicate PID:

__i3c_master_add_i3c_dev_locked() {
        olddev = i3c_master_search_i3c_dev_duplicate(newdev);
        if (olddev) {
                newdev->dev = olddev->dev;
                if (newdev->dev)
                        newdev->dev->desc = newdev;
                ...
                i3c_master_detach_i3c_dev(olddev);
                i3c_master_free_i3c_dev(olddev);
        }
}

Meanwhile, device_register() triggers a KOBJ_ADD uevent. i3c_device_uevent() reads i3cdev->desc locklessly:

i3c_device_uevent() {
        if (i3cdev->desc)
                devinfo = i3cdev->desc->info;
}

Could this lead to i3c_device_uevent() or driver probe functions dereferencing the old descriptor while it is being freed?

Review i3c_master_register_new_i3c_devs(struct i3c_master_controller *master) · line 2029

Quoted 12 lines

(quoted diff) +12 -0

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

[Severity: High]
This is a pre-existing issue, but does setting i3cdev->desc to NULL before unregistration or cleanup lead to leaking uninitialized stack memory in i3c_device_uevent()?

When a device is unregistered (or if registration fails and triggers a uevent), i3cdev->desc is set to NULL. This bypasses the initialization of devinfo in i3c_device_uevent():

i3c_device_uevent() {
        struct i3c_device_info devinfo;
        u16 manuf, part, ext;

        if (i3cdev->desc)
                devinfo = i3cdev->desc->info;

        manuf = I3C_PID_MANUF_ID(devinfo.pid);
        ...
        return add_uevent_var(env, "MODALIAS=i3c:dcr%02Xmanuf%04X",
                              devinfo.dcr, manuf);
}

Since devinfo is uninitialized on the stack when desc is NULL, could this broadcast uninitialized kernel stack data to userspace processes listening to netlink uevents?

Review i3c_master_register_new_i3c_devs(struct i3c_master_controller *master) · line 2035

Quoted 6 lines

(quoted diff) +4 -0

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

--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Quoted 39 lines · On Tue, Jul 28, 2026 at 04:19:35PM +0000, [email protected] 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 i3c_device_uevent() and driver probe functions to a Use-After-Free if a concurrent DAA replaces the device.

Pre-existing issues:
- [High] i3c_device_uevent() leaks uninitialized stack memory to userspace when i3cdev->desc is NULL. --

--- Patch [1]: [PATCH V2 1/8] i3c: master: Fix recursive locking during device registration --- commit 33a4dcd6c5626faef95bc7cd02ec4c56c6710c78
Author: Adrian Hunter <[email protected]>

i3c: master: Fix recursive locking during device registration

This commit separates device creation from registration to fix a recursive locking issue. It drops the bus maintenance lock around device_register() to prevent deadlocks when probe callbacks invoke I3C helpers.

drivers/i3c/master.c +0 -0

index fd3e79d10c840..874f6e11b296d 100644
[ ... ]

Quoted 15 lines

(quoted diff) +3 -8

@@ -1999,25 +2008,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 		if (desc->boardinfo)
 			desc->dev->dev.of_node = desc->boardinfo->of_node;
 
-		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);
+	}
+

Quoted 8 lines

Review i3c_master_register_new_i3c_devs(struct i3c_master_controller *master) · line 2017

Quoted 4 lines

(quoted diff) +4 -0

+	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 maintenance lock around device_register() expose i3c_device_uevent() and driver probes to a use-after-free?

look like make sense.

Frank

Quoted 76 lines

If a concurrent I3C hot-join event or a sysfs write to do_daa occurs while device_register() is running without the lock, __i3c_master_add_i3c_dev_locked() could find a duplicate PID:

__i3c_master_add_i3c_dev_locked() {
        olddev = i3c_master_search_i3c_dev_duplicate(newdev);
        if (olddev) {
                newdev->dev = olddev->dev;
                if (newdev->dev)
                        newdev->dev->desc = newdev;
                ...
                i3c_master_detach_i3c_dev(olddev);
                i3c_master_free_i3c_dev(olddev);
        }
}

Meanwhile, device_register() triggers a KOBJ_ADD uevent. i3c_device_uevent() reads i3cdev->desc locklessly:

i3c_device_uevent() {
        if (i3cdev->desc)
                devinfo = i3cdev->desc->info;
}

Could this lead to i3c_device_uevent() or driver probe functions dereferencing the old descriptor while it is being freed?

Review i3c_master_register_new_i3c_devs(struct i3c_master_controller *master) · line 2029

Quoted 12 lines

(quoted diff) +12 -0

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

[Severity: High]
This is a pre-existing issue, but does setting i3cdev->desc to NULL before unregistration or cleanup lead to leaking uninitialized stack memory in i3c_device_uevent()?

When a device is unregistered (or if registration fails and triggers a uevent), i3cdev->desc is set to NULL. This bypasses the initialization of devinfo in i3c_device_uevent():

i3c_device_uevent() {
        struct i3c_device_info devinfo;
        u16 manuf, part, ext;

        if (i3cdev->desc)
                devinfo = i3cdev->desc->info;

        manuf = I3C_PID_MANUF_ID(devinfo.pid);
        ...
        return add_uevent_var(env, "MODALIAS=i3c:dcr%02Xmanuf%04X",
                              devinfo.dcr, manuf);
}

Since devinfo is uninitialized on the stack when desc is NULL, could this broadcast uninitialized kernel stack data to userspace processes listening to netlink uevents?

Review i3c_master_register_new_i3c_devs(struct i3c_master_controller *master) · line 2035

Quoted 6 lines

(quoted diff) +4 -0

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

--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

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

New issues:
- [High] Concurrent execution of i3c_master_queue_ibi() and device unregistration leads to a NULL pointer dereference or Use-After-Free on dev->dev.

Pre-existing issues:
- [High] Concurrently accessing dev->ibi and its workqueue without synchronization leads to Use-After-Free. --

--- Patch [3]: [PATCH V2 3/8] i3c: master: Report wakeup events for IBIs --- commit 1c61ec6aa53c785eceea38530075c6fa6b7794f2
Author: Adrian Hunter <[email protected]>

i3c: master: Report wakeup events for IBIs

This commit records a wakeup event using pm_wakeup_event() when an In-Band Interrupt (IBI) is queued for processing and the device has wakeup enabled. It uses a 100 ms processing interval.

Quoted 4 lines

drivers/i3c/master.c +0 -0

index cf3345265ac0b..9d56ca08569a1 100644

[ ... ]

drivers/i3c/master.c void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) · line 3106

Quoted 7 lines

drivers/i3c/master.c +3 -0

@@ -3098,6 +3101,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]
Does this introduce a regression where a race condition with device unregistration could lead to a NULL pointer dereference or use-after-free on dev->dev?

If an IBI fires and i3c_master_queue_ibi() executes concurrently with device unregistration (for example, during driver module removal or hot-unplug), i3c_master_unregister_i3c_devs() can free dev->dev and set it to NULL:

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

If i3c_master_queue_ibi() passes the lockless if (!dev->ibi) check just before the unregistration runs, could the subsequent call to device_may_wakeup() dereference a freed or NULL pointer?

drivers/i3c/master.c void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) · line 3109

Quoted 3 lines

drivers/i3c/master.c +0 -0

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

[Severity: High]
This is a pre-existing issue, but can accessing dev->ibi and its workqueue here race with IBI teardown and cause a use-after-free?

If i3c_master_queue_ibi() passes the if (!dev->ibi) check, and concurrently the driver is unbound, i3c_dev_free_ibi_locked() will destroy the workqueue and free dev->ibi:

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

	kfree(dev->ibi);
	dev->ibi = NULL;
	...
}

Could the interrupt handler then continue execution and call atomic_inc() and queue_work() using the recently freed dev->ibi struct and destroyed workqueue?

Add i3c_master_any_wakeup_enabled(), 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 <[email protected]>

---

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       | 30 ++++++++++++++++++++++++++++++
 include/linux/i3c/master.h |  1 +
 2 files changed, 31 insertions(+)

drivers/i3c/master.c +1 -0

index 9d56ca08569a..dd49652425b4 100644
@@ -2048,6 +2048,36 @@ static void i3c_master_reg_work_fn(struct work_struct *work)
 	i3c_master_register_new_i3c_devs(master);
 }
 
+/**

drivers/i3c/master.c +19 -0

+	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 || !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_any_wakeup_enabled);
+
 /**
  * i3c_master_dma_map_single() - Map buffer for single DMA transfer
  * @dev: device object of a device doing DMA

include/linux/i3c/master.h +1 -1

index 99f89fef7653..d8cdc7ff2459 100644
@@ -748,6 +748,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_any_wakeup_enabled(struct i3c_master_controller *master);
 
 struct i3c_ibi_slot *i3c_master_get_free_ibi_slot(struct i3c_dev_desc *dev);
 
--

2.53.0

Quoted 33 lines · Adrian Hunter

Add i3c_master_any_wakeup_enabled(), 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 <[email protected]>

---

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       | 30 ++++++++++++++++++++++++++++++
 include/linux/i3c/master.h |  1 +
 2 files changed, 31 insertions(+)

drivers/i3c/master.c +1 -0

index 9d56ca08569a..dd49652425b4 100644
@@ -2048,6 +2048,36 @@ static void i3c_master_reg_work_fn(struct work_struct *work)
 	i3c_master_register_new_i3c_devs(master);
 }
 
+/**

drivers/i3c/master.c static void i3c_master_reg_work_fn(struct work_struct *work) · line 2061

Quoted 10 lines

drivers/i3c/master.c +10 -0

+ * i3c_master_any_wakeup_enabled() - 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.
+ *
+ * Return: true if any device may wake the system via IBI, false otherwise.
+ */
+bool i3c_master_any_wakeup_enabled(struct i3c_master_controller *master)
+{

This check most like introdouce rise condition.

if (i3c_master_any_wakeup_enabled())

	do_some_thing()

no lock protection, so between check i3c_master_any_wakeup_enabled() and do_some_thing(), condtion may be changed.

Frank

Quoted 37 lines

drivers/i3c/master.c +19 -0

+	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 || !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_any_wakeup_enabled);
+
 /**
  * i3c_master_dma_map_single() - Map buffer for single DMA transfer
  * @dev: device object of a device doing DMA

include/linux/i3c/master.h +1 -1

index 99f89fef7653..d8cdc7ff2459 100644
@@ -748,6 +748,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_any_wakeup_enabled(struct i3c_master_controller *master);
 
 struct i3c_ibi_slot *i3c_master_get_free_ibi_slot(struct i3c_dev_desc *dev);
 
--

2.53.0

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 <[email protected]>

---

Changes in V2:

None

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

drivers/i3c/device.c +5 -4

index 101eaa77de68..307ca6dc04ca 100644
@@ -205,12 +205,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

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

New issues:
- [High] Modifying device wakeup state dynamically during the suspend callback violates PM architecture, overrides sysfs UAPI, and triggers allocations and PM core warnings during system transitions.
- [Medium] The driver violates MFD architectural guidelines by dynamically allocating the mfd_cell array, calling MFD APIs outside drivers/mfd/, and the patch introduces a bespoke accessor that tightly couples parent and child drivers.

Pre-existing issues:
- [High] Unbound child devices cause a NULL pointer dereference and guaranteed kernel panic during the parent's suspend loop. --

--- Patch [6]: [PATCH V2 6/8] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI --- commit bd9a09f1c602f19882ed3fc13b554af06b2d2298
Author: Adrian Hunter <[email protected]>

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

This commit updates the PCI function's wakeup state based on the wakeup requirements of the I3C devices attached to the HCI instances during suspend. It checks if any attached device is enabled as a wakeup source and updates the PCI device's wakeup capability accordingly.

Quoted 5 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +0 -0

index 5a9e2a43eff8d..2b3bf6fa74f29 100644

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static bool mipi_i3c_hci_pci_is_mfd(struct device *dev) · line 282

Quoted 9 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +6 -0

@@ -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_any_wakeup_enabled(&hci->master);
+}

[Severity: High]
This is a pre-existing issue, but does this code handle unbound child devices safely?

If the child platform driver is unbound manually via sysfs, or if it fails to probe, dev_get_drvdata() will return NULL. If the hardware bus was left enabled

mipi_i3c_hci_pci_is_operational() can still return true. The parent's
suspend loop will then pass the device here, and to the existing i3c_hci_rpm_suspend() function, where both will unconditionally dereference the NULL pointer.

Could a check for a NULL hci pointer be added to prevent a kernel panic during suspend?

[Severity: Medium]
Does this custom accessor bypass MFD architectural guidelines?

The MFD subsystem rules specify that parent drivers should not use custom accessors to dig into the private driver data of child devices. Here, the parent driver is directly extracting the child's internal state (&hci->master) instead of using standard APIs.

Could this state propagation be handled through standard device PM wake APIs rather than directly coupling the parent and child drivers?

[ ... ]

Quoted 14 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +5 -1

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

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static int mipi_i3c_hci_pci_suspend(struct device *dev) · line 342

Quoted 3 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +2 -1

-	return ret;
+	if (device_may_wakeup(dev) != pm_data.may_wakeup)
+		device_set_wakeup_enable(dev, pm_data.may_wakeup);

[Severity: High]
Is it safe to call device_set_wakeup_enable() during the suspend execution path?

Calling this function during a system suspend triggers memory allocations via kzalloc(..., GFP_KERNEL) and registers sysfs nodes while the PM transition is actively in progress. This triggers the PM core warning "Suspicious device_wakeup_enable() during system transition!" and introduces memory deadlock risks.

Additionally, does this unconditionally override the user's explicit PM policy configured via the power/wakeup sysfs attribute?

Quoted 4 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +2 -0

+
+	return 0;
 }

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.

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:

None

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

drivers/i3c/master/mipi-i3c-hci/core.c +1 -0

index cfe9b5390b56..0212c9e984cf 100644
@@ -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>

drivers/i3c/master/mipi-i3c-hci/core.c +4 -0

+{
+	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,

drivers/i3c/master/mipi-i3c-hci/dma.c +1 -14

index 0672ed1132f8..7c2b20474130 100644
@@ -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));

drivers/i3c/master/mipi-i3c-hci/hci.h +2 -1

index b3d9803b1968..b8d2a3d680f8 100644
@@ -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

Quoted 37 lines · 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.

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:

None

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

drivers/i3c/master/mipi-i3c-hci/core.c +1 -0

index cfe9b5390b56..0212c9e984cf 100644
@@ -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,17 @@ static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
 	return container_of(m, struct i3c_hci, master);
 }
 

drivers/i3c/master/mipi-i3c-hci/core.c static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m) · line 127

Quoted 7 lines

drivers/i3c/master/mipi-i3c-hci/core.c +7 -0

+/*
+ * Determine the device that does PM / DMA and has IOMMU setup done for it in
+ * case of enabled IOMMU (for use with the DMA API).
+ * Such device is either "mipi-i3c-hci" platform device (OF/ACPI enumeration)
+ * parent or grandparent (PCI enumeration).
+ */
+struct device *i3c_hci_sysdev(struct device *dev)

Is it simpler if argument is struct i3c_hci *hci

Frank

Quoted 68 lines

drivers/i3c/master/mipi-i3c-hci/core.c +4 -0

+{
+	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,

drivers/i3c/master/mipi-i3c-hci/dma.c +1 -14

index 0672ed1132f8..7c2b20474130 100644
@@ -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));

drivers/i3c/master/mipi-i3c-hci/hci.h +2 -1

index b3d9803b1968..b8d2a3d680f8 100644
@@ -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

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 <[email protected]>

---

Changes in V2:

None

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

drivers/i3c/master/mipi-i3c-hci/core.c +6 -4

index 0212c9e984cf..5d92cf0645ef 100644
@@ -120,9 +120,9 @@ static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
 
 /*
  * Determine the device that does PM / DMA and has IOMMU setup done for it in
- * case of enabled IOMMU (for use with the DMA API).
- * Such device is either "mipi-i3c-hci" platform device (OF/ACPI enumeration)
- * parent or grandparent (PCI enumeration).
+ * case of enabled IOMMU (for use with the DMA API). It is also used to check
+ * for wakeup capability. Such device is either "mipi-i3c-hci" platform device
+ * (OF/ACPI enumeration) parent or grandparent (PCI enumeration).
  */
 struct device *i3c_hci_sysdev(struct device *dev)
 {

@@ -1176,6 +1176,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