> From: Leon Romanovsky <leon@xxxxxxxxxx>
> Sent: Thursday, October 8, 2020 12:56 AM
>
> > > This API is partially obscures low level driver-core code and needs
> > > to provide clear and proper abstractions without need to remember
> > > about put_device. There is already _add() interface why don't you do
> > > put_device() in it?
> > >
> >
> > The pushback Pierre is referring to was during our mid-tier internal
> > review. It was primarily a concern of Parav as I recall, so he can speak to his
> reasoning.
> >
> > What we originally had was a single API call
> > (ancillary_device_register) that started with a call to
> > device_initialize(), and every error path out of the function performed a
> put_device().
> >
> > Is this the model you have in mind?
>
> I don't like this flow:
> ancillary_device_initialize()
> if (ancillary_ancillary_device_add()) {
> put_device(....)
> ancillary_device_unregister()
Calling device_unregister() is incorrect, because add() wasn't successful.
Only put_device() or a wrapper ancillary_device_put() is necessary.
> return err;
> }
>
> And prefer this flow:
> ancillary_device_initialize()
> if (ancillary_device_add()) {
> ancillary_device_unregister()
This is incorrect and a clear deviation from the current core APIs that adds the confusion.
> return err;
> }
>
> In this way, the ancillary users won't need to do non-intuitive put_device();
Below is most simple, intuitive and matching with core APIs for name and design pattern wise.
init()
{
err = ancillary_device_initialize();
if (err)
return ret;
err = ancillary_device_add();
if (ret)
goto err_unwind;
err = some_foo();
if (err)
goto err_foo;
return 0;
err_foo:
ancillary_device_del(adev);
err_unwind:
ancillary_device_put(adev->dev);
return err;
}
cleanup()
{
ancillary_device_de(adev);
ancillary_device_put(adev);
/* It is common to have a one wrapper for this as ancillary_device_unregister().
* This will match with core device_unregister() that has precise documentation.
* but given fact that init() code need proper error unwinding, like above,
* it make sense to have two APIs, and no need to export another symbol for unregister().
* This pattern is very easy to audit and code.
*/
}
