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

From: "David Nyström" <david.nystrom@est.tech>
To: Adrian Hunter <adrian.hunter@intel.com>
Cc: Frank Li <Frank.li@nxp.com>,
	alexandre.belloni@bootlin.com,  linux-i3c@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/8] i3c: master: Defer new-device registration out of DAA caller context
Date: Wed, 13 May 2026 12:20:09 +0200 (CEST)	[thread overview]
Message-ID: <c179bea5-0efd-0691-92d4-66bcf50f9d8c@est.tech> (raw)
In-Reply-To: <01df8e0e-9041-401b-ab73-634701c4acdc@intel.com>

On Wed, 13 May 2026, 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.

This race was introduced in
3a379bbcea0a ("i3c: Add core I3C infrastructure")

But since this path was exposed via sysfs in latest 7.1-rc via:
8ea0b60bc00d ("i3c: master: Add sysfs option to rescan bus via entdaa")

I would argue that we should revert the sysfs addition from 7.1-rc until 
this fix is in place.

>
> -- 
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
>