Re: [Patch v6 1/7] slimbus: Device management on SLIMbus

[Bjorn Andersson]

On Wed 18 Oct 09:38 PDT 2017, Srinivas Kandagatla wrote:
> On 17/10/17 07:23, Bjorn Andersson wrote:
> > On Fri 06 Oct 08:51 PDT 2017, srinivas.kandagatla@xxxxxxxxxx wrote:
[..]
> > > +static int slim_device_remove(struct device *dev)
> > > +{
> > > +	struct slim_device *sbdev;
> > > +	struct slim_driver *sbdrv;
> > > +	int status = 0;
> > > +
> > > +	sbdev = to_slim_device(dev);
> > > +	if (!dev->driver)
> > > +		return 0;
> > > +
> > > +	sbdrv = to_slim_driver(dev->driver);
> > > +	if (sbdrv->remove)
> > > +		status = sbdrv->remove(sbdev);
> > > +
> > > +	mutex_lock(&sbdev->report_lock);
> > > +	sbdev->notified = false;
> > > +	if (status == 0)
> > > +		sbdev->driver = NULL;
> > > +	mutex_unlock(&sbdev->report_lock);
> > > +	return status;
> > 
> > device_unregister() will call device_del() which will end up in
> > __device_release_driver() which will call this function. Upon returning
> > from this function the core expect the bus to have cleaned up after the
> > dev (normally by calling drv->remove(dev)).
> > 
> > It will completely ignore the return value and continue tearing down the
> > rest of the core resources, e.g. three lines down it will
> > devres_release_all().
> > 
> > 
> > So you have the option of sleeping, while waiting for stuff to be
> > aborted/finished and then you need to clean things up.
> > 
> > The slim_device object itself will stick around until all references are
> > dropped though.
> 
> So you are suggesting that we make slim_driver remove not return anything?
> 

Yes.

Having the opportunity of failing remove() causes driver writers to
expect that they can fail, with the result of things not being torn
down properly.

Note that right after remove() returns devm_* resources will be
released, so anything that is not torn down and for some reason later
access allocated resources would cause issues.

Regards,
Bjorn