Re: [PATCH 1/2] soundwire: bus: Don't unassign dev_num before unregistering device

[Charles Keepax]

On Mon, Sep 14, 2026 at 08:21:34PM +0200, Pierre-Louis Bossart wrote:
> On 9/11/26 18:19, Charles Keepax wrote:
> > Don't mark dev_num as unassigned until after device_unregister()
> > has been called. The driver may want to communicate with the
> > device as part of the driver remove operation, so the dev_num
> > should remain assigned until that has completed.
> 
> That change looks fine.
> 
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxx>
> 
> But I am a bit worried about the dev_num handling now that I think of
> it. This is *only* at the manager level. At the peripheral level, the
> dev_num register has not been modified so the hardware still reports as
> ATTACHED with the dev_num whose mask was cleared on the host side.
> IOW we have a zombie peripheral on the link, and if we do a sequence
> rmmod/insmod things could be fun...

I think there is also a bit of a race here as well. I think we
can't access slave after we call device_unregister. Will think
about this a little more and prepare a v2. We can probably cache
the dev_num before but can't help wondering if we are clearing
the dev_num in the right place.

Thanks,
Charles

> > Signed-off-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
> > ---
> >  drivers/soundwire/bus.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> > index aeaae5a57c89d..b17718f4277ed 100644
> > --- a/drivers/soundwire/bus.c
> > +++ b/drivers/soundwire/bus.c
> > @@ -175,8 +175,9 @@ static int sdw_delete_slave(struct device *dev, void *data)
> >  
> >  	sdw_slave_debugfs_exit(slave);
> >  
> > -	mutex_lock(&bus->bus_lock);
> > +	device_unregister(dev);
> >  
> > +	mutex_lock(&bus->bus_lock);
> >  	if (slave->dev_num) { /* clear dev_num if assigned */
> >  		clear_bit(slave->dev_num, bus->assigned);
> >  		if (bus->ops && bus->ops->put_device_num)
> > @@ -185,7 +186,6 @@ static int sdw_delete_slave(struct device *dev, void *data)
> >  	list_del_init(&slave->node);
> >  	mutex_unlock(&bus->bus_lock);
> >  
> > -	device_unregister(dev);
> >  	return 0;
> >  }
> >  
>