From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC PATCH 06/12] devres: introduce Devres (Managed Device Resource) framework
Date: Thu, 9 Jul 2015 07:41:04 +0200 [thread overview]
Message-ID: <20150709074104.78d2dd25@lilith> (raw)
In-Reply-To: <CAK7LNAQNyJqAddZxXFPWP7D43vf9C-J0yrFnVfb8k54PX3W6zQ@mail.gmail.com>
Hello Masahiro,
On Thu, 9 Jul 2015 14:16:33 +0900, Masahiro Yamada
<yamada.masahiro@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?
> > 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.
> > 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.
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).
> --
> Best Regards
> Masahiro Yamada
Amicalement,
--
Albert.
next prev parent reply other threads:[~2015-07-09 5:41 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-08 4:29 [U-Boot] [RFC PATCH 00/12] Devres (Managed Device Resource) for U-Boot Masahiro Yamada
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 01/12] x86: delete unneeded declarations of disable_irq() and enable_irq() Masahiro Yamada
2015-07-08 4:46 ` Bin Meng
2015-07-09 0:22 ` Simon Glass
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 02/12] linux_compat: remove cpu_relax() define Masahiro Yamada
2015-07-08 5:01 ` Heiko Schocher
2015-07-08 5:19 ` Masahiro Yamada
2015-07-09 0:22 ` Simon Glass
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 03/12] linux_compat: move vzalloc() to header file as an inline function Masahiro Yamada
2015-07-08 5:01 ` Heiko Schocher
2015-07-09 0:22 ` Simon Glass
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 04/12] linux_compat: handle __GFP_ZERO in kmalloc() Masahiro Yamada
2015-07-08 5:03 ` Heiko Schocher
2015-07-09 0:22 ` Simon Glass
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 05/12] dm: add DM_FLAG_BOUND flag Masahiro Yamada
2015-07-09 0:22 ` Simon Glass
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 06/12] devres: introduce Devres (Managed Device Resource) framework Masahiro Yamada
2015-07-08 5:07 ` Heiko Schocher
2015-07-09 0:22 ` Simon Glass
2015-07-09 5:16 ` Masahiro Yamada
2015-07-09 5:41 ` Albert ARIBAUD [this message]
2015-07-09 13:31 ` Simon Glass
2015-07-09 15:03 ` Albert ARIBAUD
2015-07-09 15:12 ` Simon Glass
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 07/12] devres: add devm_kmalloc() and friends (managed memory allocation) Masahiro Yamada
2015-07-09 0:23 ` Simon Glass
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 08/12] dm: refactor device_bind() and device_unbind() with devm_kzalloc() Masahiro Yamada
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 09/12] dm: merge fail_alloc labels Masahiro Yamada
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 10/12] linux_compat: introduce GFP_DMA flag for kmalloc() Masahiro Yamada
2015-07-08 5:10 ` Heiko Schocher
2015-07-08 5:15 ` Masahiro Yamada
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 11/12] dm: refactor device_probe() and device_remove() with devm_kzalloc() Masahiro Yamada
2015-07-08 5:14 ` Heiko Schocher
2015-07-08 4:29 ` [U-Boot] [RFC PATCH 12/12] devres: add debug command to dump devres Masahiro Yamada
2015-07-08 7:37 ` Masahiro Yamada
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150709074104.78d2dd25@lilith \
--to=albert.u.boot@aribaud.net \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox