Re: [PATCH V2 6/8] i3c: master: Defer new-device registration out of DAA caller context

On Mon, May 18, 2026 at 02:55:17PM +0300, Adrian Hunter wrote:
> Master drivers may invoke i3c_master_do_daa_ext() during resume to
> re-run Dynamic Address Assignment.  As well as assigning addresses to
> any newly arrived devices, this restores the dynamic address of devices
> that lost it across system suspend, so it has to run as part of the
> controller's resume path.
>
> A side effect of i3c_master_do_daa_ext() today is that it also
> registers any newly discovered I3C devices with the driver model
> inline, via i3c_master_register_new_i3c_devs().  Doing that from the
> resume path is problematic: a hot-join-capable device may join the bus
> during this same DAA, and registering it immediately would push driver
> model work (probing, sysfs, etc.) into the controller's resume context,
> where the rest of the system is not yet fully resumed and the
> controller driver is still partway through its own resume sequence.
>
> Decouple discovery from registration: add a reg_work work item to
> struct i3c_master_controller and have i3c_master_do_daa_ext() queue it
> on master->wq (the freezable workqueue) instead of calling
> i3c_master_register_new_i3c_devs() directly.  The worker performs the
> registration only when the controller is not shutting_down, and is
> cancelled alongside hj_work in i3c_master_shutdown().  Because wq is
> freezable, any newly observed devices end up being registered after
> the system has finished resuming.
>
> i3c_master_register() also routes its initial post-bus-init registration
> through reg_work, using flush_work() to keep probe-time behavior
> synchronous.  This keeps a single registration code path and ensures the
> worker is the only writer of desc->dev.
>
> Fixes: 3a379bbcea0af ("i3c: Add core I3C infrastructure")
> Signed-off-by: Adrian Hunter <[email protected]>
> ---

Reviewed-by: Frank Li <[email protected]>

>
>
> Changes in V2:
>
> 	Add comment about reg_work use
> 	Add Fixes tag
>
>
>  drivers/i3c/master.c       | 27 ++++++++++++++++++++-------
>  include/linux/i3c/master.h |  6 ++++++
>  2 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index a59c4b744b36..c9685379e868 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -839,6 +839,7 @@ static void i3c_master_shutdown(struct i3c_master_controller *master)
>  	i3c_bus_maintenance_unlock(&master->bus);
>
>  	cancel_work_sync(&master->hj_work);
> +	cancel_work_sync(&master->reg_work);
>  }
>
>  static void i3c_device_shutdown(struct device *dev)
> @@ -1838,6 +1839,16 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
>  	}
>  }
>
> +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_do_daa_ext() - Dynamic Address Assignment (extended version)
>   * @master: controller
> @@ -1878,9 +1889,7 @@ int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool rstdaa)
>  	if (ret)
>  		goto out;
>
> -	i3c_bus_normaluse_lock(&master->bus);
> -	i3c_master_register_new_i3c_devs(master);
> -	i3c_bus_normaluse_unlock(&master->bus);
> +	queue_work(master->wq, &master->reg_work);
>  out:
>  	i3c_master_rpm_put(master);
>
> @@ -3126,6 +3135,7 @@ int i3c_master_register(struct i3c_master_controller *master,
>  		goto err_put_dev;
>  	}
>  	INIT_WORK(&master->hj_work, i3c_master_hj_work_fn);
> +	INIT_WORK(&master->reg_work, i3c_master_reg_work_fn);
>
>  	ret = i3c_master_bus_init(master);
>  	if (ret)
> @@ -3151,12 +3161,15 @@ int i3c_master_register(struct i3c_master_controller *master,
>
>  	/*
>  	 * We're done initializing the bus and the controller, we can now
> -	 * register I3C devices discovered during the initial DAA.
> +	 * register I3C devices discovered during the initial DAA. Device
> +	 * registration is done via reg_work because that keeps a single
> +	 * registration code path and ensures the worker is the only writer
> +	 * of desc->dev. Flush the work to preserve synchronous probe-time
> +	 * behavior.
>  	 */
>  	master->init_done = true;
> -	i3c_bus_normaluse_lock(&master->bus);
> -	i3c_master_register_new_i3c_devs(master);
> -	i3c_bus_normaluse_unlock(&master->bus);
> +	queue_work(master->wq, &master->reg_work);
> +	flush_work(&master->reg_work);
>
>  	if (master->ops->set_dev_nack_retry)
>  		device_create_file(&master->dev, &dev_attr_dev_nack_retry_count);
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index 77e63082b06e..8cdd7be505d3 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -523,6 +523,11 @@ struct i3c_master_controller_ops {
>   *	be done from a sleep-able context
>   * @hj_work: work item used to run DAA after a Hot-Join event is detected.
>   *           Queued to @wq by i3c_master_queue_hotjoin()
> + * @reg_work: work item used to register newly discovered I3C devices with
> + *            the driver model. Queued to @wq by i3c_master_do_daa_ext() so
> + *            that device registration is deferred out of the DAA caller's
> + *            context (notably the resume path), and is skipped if the
> + *            controller is shutting down
>   * @dev_nack_retry_count: retry count when slave device nack
>   *
>   * A &struct i3c_master_controller has to be registered to the I3C subsystem
> @@ -548,6 +553,7 @@ struct i3c_master_controller {
>  	struct i3c_bus bus;
>  	struct workqueue_struct *wq;
>  	struct work_struct hj_work;
> +	struct work_struct reg_work;
>  	unsigned int dev_nack_retry_count;
>  };
>
> --
> 2.51.0
>

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c