Hi Masahiro,
On 7 July 2015 at 22:29, Masahiro Yamada yamada.masahiro@socionext.com wrote:
In U-Boot's driver model, memory is basically allocated and freed in the core framework. So, low level drivers generally only have to specify the size of needed memory with .priv_auto_alloc_size, .platdata_auto_alloc_size, etc. Nevertheless, some drivers still need to allocate memory on their own in case they cannot statically know how much memory is needed. Moreover, I am afraid the failure paths of driver model core parts are getting messier as more and more memory size members are supported, .per_child_auto_alloc_size, .per_child_platdata_auto_alloc_size... So, I believe it is reasonable enough to port Devres into U-boot.
As you know, Devres, which originates in Linux, manages device resources for each device and automatically releases them on driver detach. With devres, device resources are guaranteed to be freed whether initialization fails half-way or the device gets detached.
The basic idea is totally the same to that of Linux, but I tweaked it a bit so that it fits in U-Boot's driver model.
In U-Boot, drivers are activated in two steps: binding and probing. Binding puts a driver and a device together. It is just data manipulation on the system memory, so nothing has happened on the hardware device at this moment. When the device is really used, it is probed. Probing initializes the real hardware device to make it really ready for use.
So, the resources acquired during the probing process must be freed when the device is removed. Likewise, what has been allocated in binding should be released when the device is unbound. The struct devres has a member "probe" to remember when the resource was allocated.
CONFIG_DEBUG_DEVRES is also supported for easier debugging. If enabled, debug messages are printed each time a resource is allocated/freed.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
A few points to make:
1. I don't think we will be adding a lot more types of memory attached to devices. We have:
- device private data - uclass' private data for the device - parent's private data for the device
That is all set up in device_probe() and friends. Then we have platform data for the above which is set up in device_bind(). Within the driver model architecture it's hard to see another type of data coming along although of course I would not rule it out.
2. The auto-allocation feature of driver model has actually been very successful at removing the need for drivers to do their own memory allocation.
git grep -l U_BOOT_DRIVER |wc 105
grep '[mc]alloc(' `git grep -l U_BOOT_DRIVER ` |wc 20
and a quick check suggests that half of those 10 are bogus (could be redone to avoid a special malloc()).
So I don't think the devm functions will be used much.
3. How do we handle things like gpio_exynos_bind() which allocs some data and passes it to a device it creates, as platform data? At present we don't free it.
4. There is a data size overhead to this which is not insignificant. As I read it we add 16 bytes to each memory allocation, which for most devices will amount to 32 or 48 bytes. Currently struct udevice is 84 bytes so increasing the overhead by 50% for no real improvement in functionality. This does matter in SPL in some cases.
With all that said I think overall this is a good and useful change. I can see it saving hassle later.
So, can we reduce the overhead / turn it off for SPL? Perhaps it could dissolve to nothing if CONFIG_DM_DEVICE_REMOVE is not defined?
As it happens I started yesterday on a change to check driver model pointers. I've been talking about it for a while but there are enough drivers out there that I think it is worth doing now. I hope to have something next week. However it doesn't look like it will interfere with your change much.
BTW can we please have exported functions documented in the header file?
Regards, Simon
[snip]