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

[Adrian Hunter <adrian.hunter@intel.com>]

On 21/05/2026 21:32, Frank Li wrote:
> On Fri, May 15, 2026 at 07:42:20PM +0300, Adrian Hunter wrote:
>> On 13/05/2026 22:03, Frank Li wrote:
>>> On Wed, May 13, 2026 at 08:45:55AM +0300, Adrian Hunter wrote:
>>>> On 12/05/2026 19:39, Frank Li wrote:
>>>>> On Tue, May 12, 2026 at 03:17:30PM +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.
>>>>>
>>>>> why not direct use hj_work?
>>>>
>>>> i3c_master_register_new_i3c_devs() use of desc->dev is racy, so
>>>> i3c_master_register_new_i3c_devs() must not be allowed to race
>>>> with itself.  Having it only ever run via reg_work achieves that.
>>>
>>> Sorry, I have not understand these, Can provide some detail?
>>
>> >From i3c_master_register_new_i3c_devs():
>>
>> 	i3c_bus_for_each_i3cdev(&master->bus, desc) {
>> 		if (desc->dev || !desc->info.dyn_addr || desc == master->this)
>> 			continue;
>>
>> 		desc->dev = kzalloc_obj(*desc->dev);
>> 		...
>> 		ret = device_register(&desc->dev->dev);
>>
>> This is done under the shared i3c_bus_normaluse_lock(), so there can
> 
> i3c_bus_normaluse_lock() may is wrong, suppose it should be
> i3c_bus_maintenance_lock(), register new devices change i3c bus's
> hierarchical structure.

If device_register() probes the device and the probe tries to
access the device, won't it deadlock if i3c_bus_maintenance_lock()
is held.

> 
> Frank
> 
>> be 2 or more instances of i3c_master_register_new_i3c_devs() running
>> at the same time.  They might all see desc->dev is NULL and then all
>> of them try to initialize and register a dev for the same I3C device.
>>