[U-Boot] [RFC PATCH 06/12] devres: introduce Devres (Managed Device Resource) framework

[Simon Glass]

Hi,

On 8 July 2015 at 23:41, Albert ARIBAUD <albert.u.boot at aribaud.net> wrote:
> Hello Masahiro,
>
> On Thu, 9 Jul 2015 14:16:33 +0900, Masahiro Yamada
> <yamada.masahiro at socionext.com> wrote:
>> Hi Simon,
>>
>> > 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.
>>
>> 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.

>
>> > 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?
>>
>> 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