But ... since the init() function is performing both device_init and
device_add - it should probably be called ancillary_device_register,
and we are back to a single exported API for both register and
unregister.
Kind reminder that we introduced the two functions to allow the
caller to know if it needed to free memory when initialize() fails,
and it didn't need to free memory when add() failed since
put_device() takes care of it. If you have a single init() function
it's impossible to know which behavior to select on error.
I also have a case with SoundWire where it's nice to first
initialize, then set some data and then add.
The flow as outlined by Parav above does an initialize as the first
step, so every error path out of the function has to do a
put_device(), so you would never need to manually free the memory in
the setup function.
It would be freed in the release call.
err = ancillary_device_initialize();
if (err)
return ret;
where is the put_device() here? if the release function does any sort of
kfree, then you'd need to do it manually in this case.
Since device_initialize() failed, put_device() cannot be done here.
So yes, pseudo code should have shown,
if (err) {
kfree(adev);
return err;
}
This doesn't work if the adev is part of a larger structure allocated by
the parent, which is pretty much the intent to extent the basic bus and
pass additional information which can be accessed with container_of().
Only the parent can do the kfree() explicitly in that case. If the
parent relies on devm_kzalloc, this also can make the .release callback
with no memory free required at all.
See e.g. the code I cooked for the transition of SoundWire away from
platform devices at
https://github.com/thesofproject/linux/pull/2484/commits/d0540ae3744f3a748d49c5fe61469d82ed816981#diff-ac8eb3d3951c024f52b1d463b5317f70R305
The allocation is done on an 'ldev' which contains 'adev'.
I really don't seen how an ancillary_device_register() could model the
different ways to allocate memory, for maximum flexibility across
different domains it seems more relevant to keep the initialize() and
add() APIs separate. I will accept the argument that this puts more
responsibility on the parent, but it also provides more flexibility to
the parent.
If we go with the suggested solution above, that already prevents
SoundWire from using this bus. Not so good.
