[PATCH V2 18/20] i3c: master: Introduce optional Runtime PM support
Adrian Hunter
adrian.hunter at intel.com
Fri Jan 9 03:42:06 PST 2026
On 08/01/2026 17:09, Frank Li wrote:
> On Thu, Jan 08, 2026 at 10:05:56AM +0200, Adrian Hunter wrote:
>> Master drivers currently manage Runtime PM individually, but all require
>> runtime resume for bus operations. This can be centralized in common code.
>>
>> Add optional Runtime PM support to ensure the parent device is runtime
>> resumed before bus operations and auto-suspended afterward.
>>
>> Notably, do not call ->bus_cleanup() if runtime resume fails. Master
>> drivers that opt-in to core runtime PM support must take that into account.
>>
>> Also provide an option to allow IBIs and hot-joins while runtime suspended.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter at intel.com>
>> ---
>>
>>
>> Changes in V2:
>>
>> None
>>
>> Frank suggested dispensing with rpm_allowed. That would be OK for
>> drivers that do not enable runtime PM, but the drivers that do
>> enable runtime PM (i.e. dw-i3c-master.c and svc-i3c-master.c) might
>> be affected. rpm_allowed can be removed when they are converted to
>> the new approach.
>>
>>
>> drivers/i3c/device.c | 46 +++++++++++++++++--
>> drivers/i3c/internals.h | 4 ++
>> drivers/i3c/master.c | 93 +++++++++++++++++++++++++++++++++++---
>> include/linux/i3c/master.h | 4 ++
>> 4 files changed, 138 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
>> index 8a156f5ad692..101eaa77de68 100644
>> --- a/drivers/i3c/device.c
>> +++ b/drivers/i3c/device.c
>> @@ -46,10 +46,16 @@ int i3c_device_do_xfers(struct i3c_device *dev, struct i3c_xfer *xfers,
>> return -EINVAL;
>> }
>>
>> + ret = i3c_bus_rpm_get(dev->bus);
>> + if (ret)
>> + return ret;
>> +
>
> Can you similar method
>
> commit ef8057b07c72a817537856b98d6e7493b9404eaf
> Author: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
> Date: Thu Nov 13 20:33:33 2025 +0100
>
> PM: runtime: Wrapper macros for ACQUIRE()/ACQUIRE_ERR()
>
> Add wrapper macros for ACQUIRE()/ACQUIRE_ERR() and runtime PM
> usage counter guards introduced recently: pm_runtime_active_try,
> pm_runtime_active_auto_try, pm_runtime_active_try_enabled, and
> pm_runtime_active_auto_try_enabled.
>
> The new macros should be more straightforward to use.
>
> For example, they can be used for rewriting a piece of code like below:
>
> ACQUIRE(pm_runtime_active_try, pm)(dev);
> if ((ret = ACQUIRE_ERR(pm_runtime_active_try, &pm)))
> return ret;
In this case there is little benefit, and it introduces some
ugliness.
PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND() cannot be used
because the runtime pm is conditional on master->rpm_allowed.
New guard definitions could be added e.g.
static void i3c_master_rpm(struct i3c_master_controller *master)
{
BUG();
}
DEFINE_GUARD(i3c_master_rpm_active, struct i3c_master_controller *, i3c_master_rpm(_T), i3c_master_rpm_put(_T))
DEFINE_GUARD_COND(i3c_master_rpm_active, _auto, i3c_master_rpm_get(_T))
And then used like:
- ret = i3c_master_rpm_get(master);
- if (ret)
+ ACQUIRE(i3c_master_rpm_active_auto, pm)(master);
+ if ((ret = ACQUIRE_ERR(i3c_master_rpm_active_auto, &pm)))
return ret;
But the result is ugly and there is only one place that it
simplifies the error path, and then only slightly:
int i3c_master_do_daa(struct i3c_master_controller *master)
{
int ret;
- ret = i3c_master_rpm_get(master);
- if (ret)
+ ACQUIRE(i3c_master_rpm_active_auto, pm)(master);
+ if ((ret = ACQUIRE_ERR(i3c_master_rpm_active_auto, &pm)))
return ret;
i3c_bus_maintenance_lock(&master->bus);
ret = master->ops->do_daa(master);
i3c_bus_maintenance_unlock(&master->bus);
if (ret)
- goto out;
+ return ret;
i3c_bus_normaluse_lock(&master->bus);
i3c_master_register_new_i3c_devs(master);
i3c_bus_normaluse_unlock(&master->bus);
-out:
- i3c_master_rpm_put(master);
+
return ret;
}
EXPORT_SYMBOL_GPL(i3c_master_do_daa);
>
> Frank
>
>> i3c_bus_normaluse_lock(dev->bus);
>> ret = i3c_dev_do_xfers_locked(dev->desc, xfers, nxfers, mode);
>> i3c_bus_normaluse_unlock(dev->bus);
>>
>> + i3c_bus_rpm_put(dev->bus);
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(i3c_device_do_xfers);
>> @@ -66,10 +72,16 @@ int i3c_device_do_setdasa(struct i3c_device *dev)
>> {
>> int ret;
>>
>> + ret = i3c_bus_rpm_get(dev->bus);
>> + if (ret)
>> + return ret;
>> +
>> i3c_bus_normaluse_lock(dev->bus);
>> ret = i3c_dev_setdasa_locked(dev->desc);
>> i3c_bus_normaluse_unlock(dev->bus);
>>
>> + i3c_bus_rpm_put(dev->bus);
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(i3c_device_do_setdasa);
>> @@ -106,16 +118,27 @@ EXPORT_SYMBOL_GPL(i3c_device_get_info);
>> */
>> int i3c_device_disable_ibi(struct i3c_device *dev)
>> {
>> - int ret = -ENOENT;
>> + int ret;
>> +
>> + if (i3c_bus_rpm_ibi_allowed(dev->bus)) {
>> + ret = i3c_bus_rpm_get(dev->bus);
>> + if (ret)
>> + return ret;
>> + }
>>
>> i3c_bus_normaluse_lock(dev->bus);
>> if (dev->desc) {
>> mutex_lock(&dev->desc->ibi_lock);
>> ret = i3c_dev_disable_ibi_locked(dev->desc);
>> mutex_unlock(&dev->desc->ibi_lock);
>> + } else {
>> + ret = -ENOENT;
>> }
>> i3c_bus_normaluse_unlock(dev->bus);
>>
>> + if (!ret || i3c_bus_rpm_ibi_allowed(dev->bus))
>> + i3c_bus_rpm_put(dev->bus);
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(i3c_device_disable_ibi);
>> @@ -135,16 +158,25 @@ EXPORT_SYMBOL_GPL(i3c_device_disable_ibi);
>> */
>> int i3c_device_enable_ibi(struct i3c_device *dev)
>> {
>> - int ret = -ENOENT;
>> + int ret;
>> +
>> + ret = i3c_bus_rpm_get(dev->bus);
>> + if (ret)
>> + return ret;
>>
>> i3c_bus_normaluse_lock(dev->bus);
>> if (dev->desc) {
>> mutex_lock(&dev->desc->ibi_lock);
>> ret = i3c_dev_enable_ibi_locked(dev->desc);
>> mutex_unlock(&dev->desc->ibi_lock);
>> + } else {
>> + ret = -ENOENT;
>> }
>> i3c_bus_normaluse_unlock(dev->bus);
>>
>> + if (ret || i3c_bus_rpm_ibi_allowed(dev->bus))
>> + i3c_bus_rpm_put(dev->bus);
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(i3c_device_enable_ibi);
>> @@ -163,19 +195,27 @@ EXPORT_SYMBOL_GPL(i3c_device_enable_ibi);
>> int i3c_device_request_ibi(struct i3c_device *dev,
>> const struct i3c_ibi_setup *req)
>> {
>> - int ret = -ENOENT;
>> + int ret;
>>
>> if (!req->handler || !req->num_slots)
>> return -EINVAL;
>>
>> + ret = i3c_bus_rpm_get(dev->bus);
>> + if (ret)
>> + return ret;
>> +
>> i3c_bus_normaluse_lock(dev->bus);
>> if (dev->desc) {
>> 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);
>>
>> + i3c_bus_rpm_put(dev->bus);
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(i3c_device_request_ibi);
>> diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
>> index f609e5098137..0f1f3f766623 100644
>> --- a/drivers/i3c/internals.h
>> +++ b/drivers/i3c/internals.h
>> @@ -11,6 +11,10 @@
>> #include <linux/i3c/master.h>
>> #include <linux/io.h>
>>
>> +int __must_check i3c_bus_rpm_get(struct i3c_bus *bus);
>> +void i3c_bus_rpm_put(struct i3c_bus *bus);
>> +bool i3c_bus_rpm_ibi_allowed(struct i3c_bus *bus);
>> +
>> void i3c_bus_normaluse_lock(struct i3c_bus *bus);
>> void i3c_bus_normaluse_unlock(struct i3c_bus *bus);
>>
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index ff6cbc044787..594d61edcef4 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -106,6 +106,38 @@ static struct i3c_master_controller *dev_to_i3cmaster(struct device *dev)
>> return container_of(dev, struct i3c_master_controller, dev);
>> }
>>
>> +static int __must_check i3c_master_rpm_get(struct i3c_master_controller *master)
>> +{
>> + int ret = master->rpm_allowed ? pm_runtime_resume_and_get(master->dev.parent) : 0;
>> +
>> + if (ret < 0) {
>> + dev_err(master->dev.parent, "runtime resume failed, error %d\n", ret);
>> + return ret;
>> + }
>> + return 0;
>> +}
>> +
>> +static void i3c_master_rpm_put(struct i3c_master_controller *master)
>> +{
>> + if (master->rpm_allowed)
>> + pm_runtime_put_autosuspend(master->dev.parent);
>> +}
>> +
>> +int i3c_bus_rpm_get(struct i3c_bus *bus)
>> +{
>> + return i3c_master_rpm_get(i3c_bus_to_i3c_master(bus));
>> +}
>> +
>> +void i3c_bus_rpm_put(struct i3c_bus *bus)
>> +{
>> + i3c_master_rpm_put(i3c_bus_to_i3c_master(bus));
>> +}
>> +
>> +bool i3c_bus_rpm_ibi_allowed(struct i3c_bus *bus)
>> +{
>> + return i3c_bus_to_i3c_master(bus)->rpm_ibi_allowed;
>> +}
>> +
>> static const struct device_type i3c_device_type;
>>
>> static struct i3c_bus *dev_to_i3cbus(struct device *dev)
>> @@ -611,6 +643,12 @@ static int i3c_set_hotjoin(struct i3c_master_controller *master, bool enable)
>> if (!master->ops->enable_hotjoin || !master->ops->disable_hotjoin)
>> return -EINVAL;
>>
>> + if (enable || master->rpm_ibi_allowed) {
>> + ret = i3c_master_rpm_get(master);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> i3c_bus_normaluse_lock(&master->bus);
>>
>> if (enable)
>> @@ -623,6 +661,9 @@ static int i3c_set_hotjoin(struct i3c_master_controller *master, bool enable)
>>
>> i3c_bus_normaluse_unlock(&master->bus);
>>
>> + if ((enable && ret) || (!enable && !ret) || master->rpm_ibi_allowed)
>> + i3c_master_rpm_put(master);
>> +
>> return ret;
>> }
>>
>> @@ -1712,18 +1753,23 @@ int i3c_master_do_daa(struct i3c_master_controller *master)
>> {
>> int ret;
>>
>> + ret = i3c_master_rpm_get(master);
>> + if (ret)
>> + return ret;
>> +
>> i3c_bus_maintenance_lock(&master->bus);
>> ret = master->ops->do_daa(master);
>> i3c_bus_maintenance_unlock(&master->bus);
>>
>> if (ret)
>> - return ret;
>> + goto out;
>>
>> i3c_bus_normaluse_lock(&master->bus);
>> i3c_master_register_new_i3c_devs(master);
>> i3c_bus_normaluse_unlock(&master->bus);
>> -
>> - return 0;
>> +out:
>> + i3c_master_rpm_put(master);
>> + return ret;
>> }
>> EXPORT_SYMBOL_GPL(i3c_master_do_daa);
>>
>> @@ -2065,8 +2111,17 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
>>
>> static void i3c_master_bus_cleanup(struct i3c_master_controller *master)
>> {
>> - if (master->ops->bus_cleanup)
>> - master->ops->bus_cleanup(master);
>> + if (master->ops->bus_cleanup) {
>> + int ret = i3c_master_rpm_get(master);
>> +
>> + if (ret) {
>> + dev_err(&master->dev,
>> + "runtime resume error: master bus_cleanup() not done\n");
>> + } else {
>> + master->ops->bus_cleanup(master);
>> + i3c_master_rpm_put(master);
>> + }
>> + }
>>
>> i3c_master_detach_free_devs(master);
>> }
>> @@ -2421,6 +2476,10 @@ static int i3c_master_i2c_adapter_xfer(struct i2c_adapter *adap,
>> return -EOPNOTSUPP;
>> }
>>
>> + ret = i3c_master_rpm_get(master);
>> + if (ret)
>> + return ret;
>> +
>> i3c_bus_normaluse_lock(&master->bus);
>> dev = i3c_master_find_i2c_dev_by_addr(master, addr);
>> if (!dev)
>> @@ -2429,6 +2488,8 @@ static int i3c_master_i2c_adapter_xfer(struct i2c_adapter *adap,
>> ret = master->ops->i2c_xfers(dev, xfers, nxfers);
>> i3c_bus_normaluse_unlock(&master->bus);
>>
>> + i3c_master_rpm_put(master);
>> +
>> return ret ? ret : nxfers;
>> }
>>
>> @@ -2531,6 +2592,10 @@ static int i3c_i2c_notifier_call(struct notifier_block *nb, unsigned long action
>>
>> master = i2c_adapter_to_i3c_master(adap);
>>
>> + ret = i3c_master_rpm_get(master);
>> + if (ret)
>> + return ret;
>> +
>> i3c_bus_maintenance_lock(&master->bus);
>> switch (action) {
>> case BUS_NOTIFY_ADD_DEVICE:
>> @@ -2544,6 +2609,8 @@ static int i3c_i2c_notifier_call(struct notifier_block *nb, unsigned long action
>> }
>> i3c_bus_maintenance_unlock(&master->bus);
>>
>> + i3c_master_rpm_put(master);
>> +
>> return ret;
>> }
>>
>> @@ -2881,6 +2948,10 @@ int i3c_master_register(struct i3c_master_controller *master,
>> INIT_LIST_HEAD(&master->boardinfo.i2c);
>> INIT_LIST_HEAD(&master->boardinfo.i3c);
>>
>> + ret = i3c_master_rpm_get(master);
>> + if (ret)
>> + return ret;
>> +
>> device_initialize(&master->dev);
>> dev_set_name(&master->dev, "i3c-%d", i3cbus->id);
>>
>> @@ -2960,6 +3031,8 @@ int i3c_master_register(struct i3c_master_controller *master,
>> i3c_master_register_new_i3c_devs(master);
>> i3c_bus_normaluse_unlock(&master->bus);
>>
>> + i3c_master_rpm_put(master);
>> +
>> return 0;
>>
>> err_del_dev:
>> @@ -2969,6 +3042,7 @@ int i3c_master_register(struct i3c_master_controller *master,
>> i3c_master_bus_cleanup(master);
>>
>> err_put_dev:
>> + i3c_master_rpm_put(master);
>> put_device(&master->dev);
>>
>> return ret;
>> @@ -3114,8 +3188,15 @@ void i3c_dev_free_ibi_locked(struct i3c_dev_desc *dev)
>> return;
>>
>> if (dev->ibi->enabled) {
>> + int ret;
>> +
>> dev_err(&master->dev, "Freeing IBI that is still enabled\n");
>> - if (i3c_dev_disable_ibi_locked(dev))
>> + ret = i3c_master_rpm_get(master);
>> + if (!ret) {
>> + ret = i3c_dev_disable_ibi_locked(dev);
>> + i3c_master_rpm_put(master);
>> + }
>> + if (ret)
>> dev_err(&master->dev, "Failed to disable IBI before freeing\n");
>> }
>>
>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
>> index 6225ad28f210..c1ec597f655c 100644
>> --- a/include/linux/i3c/master.h
>> +++ b/include/linux/i3c/master.h
>> @@ -504,6 +504,8 @@ struct i3c_master_controller_ops {
>> * @secondary: true if the master is a secondary master
>> * @init_done: true when the bus initialization is done
>> * @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
>> * @boardinfo.i3c: list of I3C boardinfo objects
>> * @boardinfo.i2c: list of I2C boardinfo objects
>> * @boardinfo: board-level information attached to devices connected on the bus
>> @@ -527,6 +529,8 @@ struct i3c_master_controller {
>> unsigned int secondary : 1;
>> unsigned int init_done : 1;
>> unsigned int hotjoin: 1;
>> + unsigned int rpm_allowed: 1;
>> + unsigned int rpm_ibi_allowed: 1;
>> struct {
>> struct list_head i3c;
>> struct list_head i2c;
>> --
>> 2.51.0
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c
More information about the linux-i3c
mailing list