Re: [PATCH V2 2/8] i3c: master: Serialize i3c_set_hotjoin() with the maintenance lock
Adrian Hunter <[email protected]>
| Newsgroups | org.infradead.lists.linux-i3c,org.kernel.vger.linux-kernel |
|---|---|
| Organization | Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki |
| Message-ID | <[email protected]> |
On 18/05/2026 14:55, Adrian Hunter wrote:
> i3c_set_hotjoin() dispatches the controller's enable_hotjoin() or
> disable_hotjoin() op and updates master->hotjoin under
> i3c_bus_normaluse_lock(). That lock is a read-side acquisition of
> bus->lock (down_read()), so it does not exclude concurrent callers.
>
> The hotjoin sysfs attribute can be opened multiple times, and writes
> through different opens are not serialized. Two concurrent writers
> to "hotjoin" can therefore race in i3c_set_hotjoin(), with the
> controller op and the master->hotjoin store from one call interleaving
> with the other. The hardware enable/disable state and the value reported
> by hotjoin_show() can end up out of sync.
>
> Take i3c_bus_maintenance_lock() instead. Toggling Hot Join enable
> changes bus state and is conceptually a maintenance operation, so the
> write-side acquisition of bus->lock is the appropriate lock and
> serializes concurrent callers against each other and against other
> maintenance operations.
>
> Fixes: 317bacf960a48 ("i3c: master: add enable(disable) hot join in sys entry")
> Signed-off-by: Adrian Hunter <[email protected]>
Any comments on this patch?
> ---
>
>
> Changes in V2:
>
> Add Fixes tag
>
>
> drivers/i3c/master.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index ab11e2d79aab..38ffc8713167 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -649,7 +649,7 @@ static int i3c_set_hotjoin(struct i3c_master_controller *master, bool enable)
> return ret;
> }
>
> - i3c_bus_normaluse_lock(&master->bus);
> + i3c_bus_maintenance_lock(&master->bus);
>
> if (enable)
> ret = master->ops->enable_hotjoin(master);
> @@ -659,7 +659,7 @@ static int i3c_set_hotjoin(struct i3c_master_controller *master, bool enable)
> if (!ret)
> master->hotjoin = enable;
>
> - i3c_bus_normaluse_unlock(&master->bus);
> + i3c_bus_maintenance_unlock(&master->bus);
>
> if ((enable && ret) || (!enable && !ret) || master->rpm_ibi_allowed)
> i3c_master_rpm_put(master);
--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c