Re: [PATCH 2/8] i3c: master: Serialize i3c_set_hotjoin() with the maintenance lock

[David Nyström]

On Wed, 13 May 2026, Adrian Hunter wrote:

> On 12/05/2026 22:42, David Nyström wrote:
> > On Tue, 12 May 2026, Frank Li wrote:
> >
> > > On Tue, May 12, 2026 at 03:17:26PM +0300, 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.
> > >
> > > It should be bug fix, add fix tag here.
> >
> > Agreed, Fixes: "i3c: master: Add sysfs option to rescan bus via entdaa"
> > Is this series headed for 7.1-rc3 ? if not, its probably wise to revert the sysfs addition from 7.1-rc
>
> "i3c: master: Add sysfs option to rescan bus via entdaa" added "do_daa".
> "hotjoin" is a different sysfs attribute.

Replied to wrong patch, correct patch commented now.

> > > Frank
> > > > Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> > > > ---
> > > >  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);
> > > > --
> > > > 2.51.0
> > >
> > > -- 
> > > linux-i3c mailing list
> > > linux-i3c@xxxxxxxxxxxxxxxxxxx
> > > http://lists.infradead.org/mailman/listinfo/linux-i3c