Hi,
On 8 July 2015 at 23:41, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hello Masahiro,
On Thu, 9 Jul 2015 14:16:33 +0900, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Hi Simon,
- 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.
So, currently this driver is leaking the memory, isn't it?
If we use devm_kzalloc() here, the platdata for GPIOs will be released when the parent pinctrl is unbound.
Does gpio_exynos_bind() get called enough between entry and exit from U-boot that the memory leaks prevent U-Boot from doing its job properly?
No we only bind devices once in U-Boot, except for USB which recently changed.
- 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?
I think I can do this.
devres.c can be built (and makes sense) only when CONFIG_DM_DEVICE_REMOVE is enabled.
Agreed.
In fact perhaps we need two options here - one that controls the inclusion of the remove() code and one that controls unbind().
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?
IIRC, when we discussed this before, we could not reach agreement which should be documented, headers or sources.
I know you prefer comments in headers, while I prefer in sources (like Linux).
I can move them to dm/device.h if you think it is important to be consistent in the DM core portion.
My .02EUR: I prefer comments in both, targeting different people.
In .h files, for the benefit of users of the function, describe what it does, what its arguments mean and what its return value means.
Yes, I like to see the file's API documented in the header so you can understand how to use it without reading through all the .c code.
In .c files, for the benefit of maintainers (in a loose sense) of the function, describe how it does its job (if and where the code does not make it reasonably obvious).
Sounds good.
Regards, Simon