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
>
WARNING: multiple messages have this Message-ID (diff)
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
>
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
next prev parent reply other threads:[~2026-05-13 10:20 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-12 12:17 [PATCH 0/8] i3c: Hot-Join improvements and MIPI HCI Hot-Join support Adrian Hunter
2026-05-12 12:17 ` Adrian Hunter
2026-05-12 12:17 ` [PATCH 1/8] i3c: master: Make hot-join workqueue freezable to block hot-join during suspend Adrian Hunter
2026-05-12 12:17 ` Adrian Hunter
2026-05-12 16:09 ` Frank Li
2026-05-12 16:09 ` Frank Li
2026-05-12 12:17 ` [PATCH 2/8] i3c: master: Serialize i3c_set_hotjoin() with the maintenance lock Adrian Hunter
2026-05-12 12:17 ` Adrian Hunter
2026-05-12 16:11 ` Frank Li
2026-05-12 16:11 ` Frank Li
2026-05-12 19:42 ` David Nyström
2026-05-12 19:42 ` David Nyström
2026-05-13 5:01 ` Adrian Hunter
2026-05-13 5:01 ` Adrian Hunter
2026-05-13 10:21 ` David Nyström
2026-05-13 10:21 ` David Nyström
2026-05-13 5:09 ` Adrian Hunter
2026-05-13 5:09 ` Adrian Hunter
2026-05-12 12:17 ` [PATCH 3/8] i3c: master: Consolidate Hot-Join DAA work in the core Adrian Hunter
2026-05-12 12:17 ` Adrian Hunter
2026-05-12 16:16 ` Frank Li
2026-05-12 16:16 ` Frank Li
2026-05-12 12:17 ` [PATCH 4/8] i3c: master: Ensure Hot-Join operations are stopped on shutdown Adrian Hunter
2026-05-12 12:17 ` Adrian Hunter
2026-05-12 16:27 ` Frank Li
2026-05-12 16:27 ` Frank Li
2026-05-13 5:31 ` Adrian Hunter
2026-05-13 5:31 ` Adrian Hunter
2026-05-13 19:04 ` Frank Li
2026-05-13 19:04 ` Frank Li
2026-05-12 12:17 ` [PATCH 5/8] i3c: dw: Drop redundant Hot-Join cancel_work_sync() in shutdown Adrian Hunter
2026-05-12 12:17 ` Adrian Hunter
2026-05-12 16:30 ` Frank Li
2026-05-12 16:30 ` Frank Li
2026-05-12 12:17 ` [PATCH 6/8] i3c: master: Defer new-device registration out of DAA caller context Adrian Hunter
2026-05-12 12:17 ` Adrian Hunter
2026-05-12 16:39 ` Frank Li
2026-05-12 16:39 ` Frank Li
2026-05-13 5:45 ` Adrian Hunter
2026-05-13 5:45 ` Adrian Hunter
2026-05-13 10:20 ` David Nyström [this message]
2026-05-13 10:20 ` David Nyström
2026-05-13 19:03 ` Frank Li
2026-05-13 19:03 ` Frank Li
2026-05-15 16:42 ` Adrian Hunter
2026-05-15 16:42 ` Adrian Hunter
2026-05-12 12:17 ` [PATCH 7/8] i3c: master: Export i3c_master_enec_disec_locked() Adrian Hunter
2026-05-12 12:17 ` Adrian Hunter
2026-05-12 16:31 ` Frank Li
2026-05-12 16:31 ` Frank Li
2026-05-12 12:17 ` [PATCH 8/8] i3c: mipi-i3c-hci: Add Hot-Join support Adrian Hunter
2026-05-12 12:17 ` Adrian Hunter
2026-05-12 16:34 ` Frank Li
2026-05-12 16:34 ` Frank Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c179bea5-0efd-0691-92d4-66bcf50f9d8c@est.tech \
--to=david.nystrom@est.tech \
--cc=Frank.li@nxp.com \
--cc=adrian.hunter@intel.com \
--cc=alexandre.belloni@bootlin.com \
--cc=linux-i3c@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.