Re: [PATCH 15/17] i3c: master: Introduce optional Runtime PM support

Frank Li
2025-12-19T17:08:36Z

On Fri, Dec 19, 2025 at 04:45:32PM +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@intel.com>
> ---
> 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);

Is it i3c_bus_rpm_get(dev)? dev->bus is parent of dev. in case i3c
periphal need enable clock or other run time pm enable call back?

> + if (ret)
> + return ret;
> +
> 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;
> }
...
>
> +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;

I think rpm_allowed is not necessary. If don't allow rpm, i3c bus driver
should disable runtime_pm.

I remember pm_runtime_resume_and_get() do nothing if runtime_pm disabled
(need double check).

Frank

> +
> + 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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c