[U-Boot] [RFC PATCH 00/12] Devres (Managed Device Resource) for U-Boot

[U-Boot] [RFC PATCH 00/12] Devres (Managed Device Resource) for U-Boot

[Masahiro Yamada]

Please refer to the commit message of 06/12
for what this series wants to do.



Masahiro Yamada (12):
  x86: delete unneeded declarations of disable_irq() and enable_irq()
  linux_compat: remove cpu_relax() define
  linux_compat: move vzalloc() to header file as an inline function
  linux_compat: handle __GFP_ZERO in kmalloc()
  dm: add DM_FLAG_BOUND flag
  devres: introduce Devres (Managed Device Resource) framework
  devres: add devm_kmalloc() and friends (managed memory allocation)
  dm: refactor device_bind() and device_unbind() with devm_kzalloc()
  dm: merge fail_alloc labels
  linux_compat: introduce GFP_DMA flag for kmalloc()
  dm: refactor device_probe() and device_remove() with devm_kzalloc()
  devres: add debug command to dump devres

 arch/x86/include/asm/interrupt.h       |   4 -
 drivers/core/Kconfig                   |  16 +++
 drivers/core/Makefile                  |   2 +-
 drivers/core/device-remove.c           |  41 ++-----
 drivers/core/device.c                  |  64 ++++-------
 drivers/core/devres.c                  | 195 +++++++++++++++++++++++++++++++++
 drivers/usb/musb-new/musb_gadget_ep0.c |   1 +
 include/dm/device.h                    |  52 +++++++--
 include/linux/compat.h                 |  29 +++--
 lib/linux_compat.c                     |  19 ++--
 10 files changed, 310 insertions(+), 113 deletions(-)
 create mode 100644 drivers/core/devres.c

-- 
1.9.1

[U-Boot] [RFC PATCH 01/12] x86: delete unneeded declarations of disable_irq() and enable_irq()

[Masahiro Yamada]

These two declarations in arch/x86/include/asm/interrupt.h conflict
with ones in include/linux/compat.h, so x86 boards cannot include
<linux/compat.h>.

The comment /* arch/x86/lib/interrupts.c */ is bogus now, and we do
not see any definitions of disable_irq() and enable_irq() in there.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/x86/include/asm/interrupt.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/include/asm/interrupt.h b/arch/x86/include/asm/interrupt.h
index 0a75f89..00cbe07 100644
--- a/arch/x86/include/asm/interrupt.h
+++ b/arch/x86/include/asm/interrupt.h
@@ -16,10 +16,6 @@
 /* arch/x86/cpu/interrupts.c */
 void set_vector(u8 intnum, void *routine);
 
-/* arch/x86/lib/interrupts.c */
-void disable_irq(int irq);
-void enable_irq(int irq);
-
 /* Architecture specific functions */
 void mask_irq(int irq);
 void unmask_irq(int irq);
-- 
1.9.1

[U-Boot] [RFC PATCH 01/12] x86: delete unneeded declarations of disable_irq() and enable_irq()

[Bin Meng]

On Wed, Jul 8, 2015 at 12:29 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> These two declarations in arch/x86/include/asm/interrupt.h conflict
> with ones in include/linux/compat.h, so x86 boards cannot include
> <linux/compat.h>.
>
> The comment /* arch/x86/lib/interrupts.c */ is bogus now, and we do
> not see any definitions of disable_irq() and enable_irq() in there.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  arch/x86/include/asm/interrupt.h | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/interrupt.h b/arch/x86/include/asm/interrupt.h
> index 0a75f89..00cbe07 100644
> --- a/arch/x86/include/asm/interrupt.h
> +++ b/arch/x86/include/asm/interrupt.h
> @@ -16,10 +16,6 @@
>  /* arch/x86/cpu/interrupts.c */
>  void set_vector(u8 intnum, void *routine);
>
> -/* arch/x86/lib/interrupts.c */
> -void disable_irq(int irq);
> -void enable_irq(int irq);
> -
>  /* Architecture specific functions */
>  void mask_irq(int irq);
>  void unmask_irq(int irq);
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [RFC PATCH 01/12] x86: delete unneeded declarations of disable_irq() and enable_irq()

[Simon Glass]

On 7 July 2015 at 22:46, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Wed, Jul 8, 2015 at 12:29 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> These two declarations in arch/x86/include/asm/interrupt.h conflict
>> with ones in include/linux/compat.h, so x86 boards cannot include
>> <linux/compat.h>.
>>
>> The comment /* arch/x86/lib/interrupts.c */ is bogus now, and we do
>> not see any definitions of disable_irq() and enable_irq() in there.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  arch/x86/include/asm/interrupt.h | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/interrupt.h b/arch/x86/include/asm/interrupt.h
>> index 0a75f89..00cbe07 100644
>> --- a/arch/x86/include/asm/interrupt.h
>> +++ b/arch/x86/include/asm/interrupt.h
>> @@ -16,10 +16,6 @@
>>  /* arch/x86/cpu/interrupts.c */
>>  void set_vector(u8 intnum, void *routine);
>>
>> -/* arch/x86/lib/interrupts.c */
>> -void disable_irq(int irq);
>> -void enable_irq(int irq);
>> -
>>  /* Architecture specific functions */
>>  void mask_irq(int irq);
>>  void unmask_irq(int irq);
>> --
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [RFC PATCH 02/12] linux_compat: remove cpu_relax() define

[Masahiro Yamada]

The macro cpu_relax() is defined by several headers in different
ways.

arch/{arm,avr32,mips}/include/asm/processor.h defines it as follows:
  #define cpu_relax()	barrier()

On the other hand, include/linux/compat.h defines it as follows:
  #define cpu_relax() do {} while (0)

If both headers are included from the same source file, the warning
  warning: "cpu_relax" redefined [enabled by default]
is displayed.

It effectively makes it impossible to include <linux/compat.h>
from some sources.  Drop the latter.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/usb/musb-new/musb_gadget_ep0.c | 1 +
 include/linux/compat.h                 | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/musb-new/musb_gadget_ep0.c b/drivers/usb/musb-new/musb_gadget_ep0.c
index 5a71501..415a9f2 100644
--- a/drivers/usb/musb-new/musb_gadget_ep0.c
+++ b/drivers/usb/musb-new/musb_gadget_ep0.c
@@ -43,6 +43,7 @@
 #else
 #include <common.h>
 #include "linux-compat.h"
+#include <asm/processor.h>
 #endif
 
 #include "musb_core.h"
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 6ff3915..da1420f 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -315,8 +315,6 @@ struct notifier_block {};
 
 typedef unsigned long dmaaddr_t;
 
-#define cpu_relax() do {} while (0)
-
 #define pm_runtime_get_sync(dev) do {} while (0)
 #define pm_runtime_put(dev) do {} while (0)
 #define pm_runtime_put_sync(dev) do {} while (0)
-- 
1.9.1

[U-Boot] [RFC PATCH 02/12] linux_compat: remove cpu_relax() define

[Heiko Schocher]

Hello Masahiro,

Am 08.07.2015 um 06:29 schrieb Masahiro Yamada:
> The macro cpu_relax() is defined by several headers in different
> ways.
>
> arch/{arm,avr32,mips}/include/asm/processor.h defines it as follows:
>    #define cpu_relax()	barrier()
>
> On the other hand, include/linux/compat.h defines it as follows:
>    #define cpu_relax() do {} while (0)
>
> If both headers are included from the same source file, the warning
>    warning: "cpu_relax" redefined [enabled by default]
> is displayed.
>
> It effectively makes it impossible to include <linux/compat.h>
> from some sources.  Drop the latter.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>   drivers/usb/musb-new/musb_gadget_ep0.c | 1 +
>   include/linux/compat.h                 | 2 --
>   2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/usb/musb-new/musb_gadget_ep0.c b/drivers/usb/musb-new/musb_gadget_ep0.c
> index 5a71501..415a9f2 100644
> --- a/drivers/usb/musb-new/musb_gadget_ep0.c
> +++ b/drivers/usb/musb-new/musb_gadget_ep0.c
> @@ -43,6 +43,7 @@
>   #else
>   #include <common.h>
>   #include "linux-compat.h"
> +#include <asm/processor.h>
>   #endif
>
>   #include "musb_core.h"
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index 6ff3915..da1420f 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -315,8 +315,6 @@ struct notifier_block {};
>
>   typedef unsigned long dmaaddr_t;
>
> -#define cpu_relax() do {} while (0)
> -

doesn;t this lead to compie errors on archs, which does not define
this?

If so, we should add this in arch/{xxx}/include/asm/processor.h

beside of that.
Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
>   #define pm_runtime_get_sync(dev) do {} while (0)
>   #define pm_runtime_put(dev) do {} while (0)
>   #define pm_runtime_put_sync(dev) do {} while (0)
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [RFC PATCH 02/12] linux_compat: remove cpu_relax() define

[Masahiro Yamada]

2015-07-08 14:01 GMT+09:00 Heiko Schocher <hs@denx.de>:
> Hello Masahiro,
>
>
> Am 08.07.2015 um 06:29 schrieb Masahiro Yamada:
>>
>> The macro cpu_relax() is defined by several headers in different
>> ways.
>>
>> arch/{arm,avr32,mips}/include/asm/processor.h defines it as follows:
>>    #define cpu_relax()  barrier()
>>
>> On the other hand, include/linux/compat.h defines it as follows:
>>    #define cpu_relax() do {} while (0)
>>
>> If both headers are included from the same source file, the warning
>>    warning: "cpu_relax" redefined [enabled by default]
>> is displayed.
>>
>> It effectively makes it impossible to include <linux/compat.h>
>> from some sources.  Drop the latter.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>   drivers/usb/musb-new/musb_gadget_ep0.c | 1 +
>>   include/linux/compat.h                 | 2 --
>>   2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/musb-new/musb_gadget_ep0.c
>> b/drivers/usb/musb-new/musb_gadget_ep0.c
>> index 5a71501..415a9f2 100644
>> --- a/drivers/usb/musb-new/musb_gadget_ep0.c
>> +++ b/drivers/usb/musb-new/musb_gadget_ep0.c
>> @@ -43,6 +43,7 @@
>>   #else
>>   #include <common.h>
>>   #include "linux-compat.h"
>> +#include <asm/processor.h>
>>   #endif
>>
>>   #include "musb_core.h"
>> diff --git a/include/linux/compat.h b/include/linux/compat.h
>> index 6ff3915..da1420f 100644
>> --- a/include/linux/compat.h
>> +++ b/include/linux/compat.h
>> @@ -315,8 +315,6 @@ struct notifier_block {};
>>
>>   typedef unsigned long dmaaddr_t;
>>
>> -#define cpu_relax() do {} while (0)
>> -
>
>
> doesn;t this lead to compie errors on archs, which does not define
> this?
>
> If so, we should add this in arch/{xxx}/include/asm/processor.h
>
> beside of that.
> Reviewed-by: Heiko Schocher <hs@denx.de>


No worry.

Buildman assured this series did not break any board, at least for compile test.

Run test was done only on some boards I have, of course...




-- 
Best Regards
Masahiro Yamada

[U-Boot] [RFC PATCH 02/12] linux_compat: remove cpu_relax() define

[Simon Glass]

On 7 July 2015 at 22:29, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> The macro cpu_relax() is defined by several headers in different
> ways.
>
> arch/{arm,avr32,mips}/include/asm/processor.h defines it as follows:
>   #define cpu_relax()   barrier()
>
> On the other hand, include/linux/compat.h defines it as follows:
>   #define cpu_relax() do {} while (0)
>
> If both headers are included from the same source file, the warning
>   warning: "cpu_relax" redefined [enabled by default]
> is displayed.
>
> It effectively makes it impossible to include <linux/compat.h>
> from some sources.  Drop the latter.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  drivers/usb/musb-new/musb_gadget_ep0.c | 1 +
>  include/linux/compat.h                 | 2 --
>  2 files changed, 1 insertion(+), 2 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [RFC PATCH 03/12] linux_compat: move vzalloc() to header file as an inline function

[Masahiro Yamada]

The vzalloc(size) is equivalent to kzalloc(size, 0).  Move it to
include/linux/compat.h as an inline function in order to avoid the
function call overhead.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 include/linux/compat.h | 6 ++++--
 lib/linux_compat.c     | 5 -----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index da1420f..a3d136b 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -40,6 +40,10 @@ void *kmalloc(size_t size, int flags);
 void *kzalloc(size_t size, int flags);
 #define vmalloc(size)	kmalloc(size, 0)
 #define __vmalloc(size, flags, pgsz)	kmalloc(size, flags)
+static inline void *vzalloc(unsigned long size)
+{
+	return kzalloc(size, 0);
+}
 #define kfree(ptr)	free(ptr)
 #define vfree(ptr)	free(ptr)
 
@@ -189,8 +193,6 @@ struct work_struct {};
 unsigned long copy_from_user(void *dest, const void *src,
 			     unsigned long count);
 
-void *vzalloc(unsigned long size);
-
 typedef unused_t spinlock_t;
 typedef int	wait_queue_head_t;
 
diff --git a/lib/linux_compat.c b/lib/linux_compat.c
index a3d4675..8c7a7b5 100644
--- a/lib/linux_compat.c
+++ b/lib/linux_compat.c
@@ -26,11 +26,6 @@ void *kzalloc(size_t size, int flags)
 	return ptr;
 }
 
-void *vzalloc(unsigned long size)
-{
-	return kzalloc(size, 0);
-}
-
 struct kmem_cache *get_mem(int element_sz)
 {
 	struct kmem_cache *ret;
-- 
1.9.1

[U-Boot] [RFC PATCH 03/12] linux_compat: move vzalloc() to header file as an inline function

[Heiko Schocher]

Hello Masahiro,

Am 08.07.2015 um 06:29 schrieb Masahiro Yamada:
> The vzalloc(size) is equivalent to kzalloc(size, 0).  Move it to
> include/linux/compat.h as an inline function in order to avoid the
> function call overhead.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>   include/linux/compat.h | 6 ++++--
>   lib/linux_compat.c     | 5 -----
>   2 files changed, 4 insertions(+), 7 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
>
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index da1420f..a3d136b 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -40,6 +40,10 @@ void *kmalloc(size_t size, int flags);
>   void *kzalloc(size_t size, int flags);
>   #define vmalloc(size)	kmalloc(size, 0)
>   #define __vmalloc(size, flags, pgsz)	kmalloc(size, flags)
> +static inline void *vzalloc(unsigned long size)
> +{
> +	return kzalloc(size, 0);
> +}
>   #define kfree(ptr)	free(ptr)
>   #define vfree(ptr)	free(ptr)
>
> @@ -189,8 +193,6 @@ struct work_struct {};
>   unsigned long copy_from_user(void *dest, const void *src,
>   			     unsigned long count);
>
> -void *vzalloc(unsigned long size);
> -
>   typedef unused_t spinlock_t;
>   typedef int	wait_queue_head_t;
>
> diff --git a/lib/linux_compat.c b/lib/linux_compat.c
> index a3d4675..8c7a7b5 100644
> --- a/lib/linux_compat.c
> +++ b/lib/linux_compat.c
> @@ -26,11 +26,6 @@ void *kzalloc(size_t size, int flags)
>   	return ptr;
>   }
>
> -void *vzalloc(unsigned long size)
> -{
> -	return kzalloc(size, 0);
> -}
> -
>   struct kmem_cache *get_mem(int element_sz)
>   {
>   	struct kmem_cache *ret;
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [RFC PATCH 03/12] linux_compat: move vzalloc() to header file as an inline function

[Simon Glass]

On 7 July 2015 at 22:29, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> The vzalloc(size) is equivalent to kzalloc(size, 0).  Move it to
> include/linux/compat.h as an inline function in order to avoid the
> function call overhead.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  include/linux/compat.h | 6 ++++--
>  lib/linux_compat.c     | 5 -----
>  2 files changed, 4 insertions(+), 7 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [RFC PATCH 04/12] linux_compat: handle __GFP_ZERO in kmalloc()

[Masahiro Yamada]

Currently, kzalloc() returns zero-filled memory, while kmalloc()
simply ignores the second argument and never fills the memory
area with zeros.

I want kmalloc(size, __GFP_ZERO) to behave as kzalloc() does,
which will make it easier to add more memory allocator variants.

With the introduction of __GFP_ZERO flag, going forward, kzmalloc()
variants can fall back to kmalloc() enabling the __GFP_ZERO flag.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 include/linux/compat.h | 20 ++++++++++++--------
 lib/linux_compat.c     | 13 ++++++-------
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index a3d136b..64988bf 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -36,8 +36,19 @@ extern struct p_current *current;
 #define KERN_INFO
 #define KERN_DEBUG
 
+#define GFP_ATOMIC ((gfp_t) 0)
+#define GFP_KERNEL ((gfp_t) 0)
+#define GFP_NOFS ((gfp_t) 0)
+#define GFP_USER ((gfp_t) 0)
+#define __GFP_NOWARN ((gfp_t) 0)
+#define __GFP_ZERO	((__force gfp_t)0x8000u) /* Return zeroed page on success */
+
 void *kmalloc(size_t size, int flags);
-void *kzalloc(size_t size, int flags);
+
+static inline void *kzalloc(size_t size, gfp_t flags)
+{
+	return kmalloc(size, flags | __GFP_ZERO);
+}
 #define vmalloc(size)	kmalloc(size, 0)
 #define __vmalloc(size, flags, pgsz)	kmalloc(size, flags)
 static inline void *vzalloc(unsigned long size)
@@ -77,13 +88,6 @@ void *kmem_cache_alloc(struct kmem_cache *obj, int flag);
 /* drivers/char/random.c */
 #define get_random_bytes(...)
 
-/* idr.c */
-#define GFP_ATOMIC ((gfp_t) 0)
-#define GFP_KERNEL ((gfp_t) 0)
-#define GFP_NOFS ((gfp_t) 0)
-#define GFP_USER ((gfp_t) 0)
-#define __GFP_NOWARN ((gfp_t) 0)
-
 /* include/linux/leds.h */
 struct led_trigger {};
 
diff --git a/lib/linux_compat.c b/lib/linux_compat.c
index 8c7a7b5..a936a7e 100644
--- a/lib/linux_compat.c
+++ b/lib/linux_compat.c
@@ -16,14 +16,13 @@ unsigned long copy_from_user(void *dest, const void *src,
 
 void *kmalloc(size_t size, int flags)
 {
-	return memalign(ARCH_DMA_MINALIGN, size);
-}
+	void *p;
 
-void *kzalloc(size_t size, int flags)
-{
-	void *ptr = kmalloc(size, flags);
-	memset(ptr, 0, size);
-	return ptr;
+	p = memalign(ARCH_DMA_MINALIGN, size);
+	if (flags & __GFP_ZERO)
+		memset(p, 0, size);
+
+	return p;
 }
 
 struct kmem_cache *get_mem(int element_sz)
-- 
1.9.1

[U-Boot] [RFC PATCH 04/12] linux_compat: handle __GFP_ZERO in kmalloc()

[Heiko Schocher]

Hello Masahiro,

Am 08.07.2015 um 06:29 schrieb Masahiro Yamada:
> Currently, kzalloc() returns zero-filled memory, while kmalloc()
> simply ignores the second argument and never fills the memory
> area with zeros.
>
> I want kmalloc(size, __GFP_ZERO) to behave as kzalloc() does,
> which will make it easier to add more memory allocator variants.
>
> With the introduction of __GFP_ZERO flag, going forward, kzmalloc()
> variants can fall back to kmalloc() enabling the __GFP_ZERO flag.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>   include/linux/compat.h | 20 ++++++++++++--------
>   lib/linux_compat.c     | 13 ++++++-------
>   2 files changed, 18 insertions(+), 15 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
>
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index a3d136b..64988bf 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -36,8 +36,19 @@ extern struct p_current *current;
>   #define KERN_INFO
>   #define KERN_DEBUG
>
> +#define GFP_ATOMIC ((gfp_t) 0)
> +#define GFP_KERNEL ((gfp_t) 0)
> +#define GFP_NOFS ((gfp_t) 0)
> +#define GFP_USER ((gfp_t) 0)
> +#define __GFP_NOWARN ((gfp_t) 0)
> +#define __GFP_ZERO	((__force gfp_t)0x8000u) /* Return zeroed page on success */
> +
>   void *kmalloc(size_t size, int flags);
> -void *kzalloc(size_t size, int flags);
> +
> +static inline void *kzalloc(size_t size, gfp_t flags)
> +{
> +	return kmalloc(size, flags | __GFP_ZERO);
> +}
>   #define vmalloc(size)	kmalloc(size, 0)
>   #define __vmalloc(size, flags, pgsz)	kmalloc(size, flags)
>   static inline void *vzalloc(unsigned long size)
> @@ -77,13 +88,6 @@ void *kmem_cache_alloc(struct kmem_cache *obj, int flag);
>   /* drivers/char/random.c */
>   #define get_random_bytes(...)
>
> -/* idr.c */
> -#define GFP_ATOMIC ((gfp_t) 0)
> -#define GFP_KERNEL ((gfp_t) 0)
> -#define GFP_NOFS ((gfp_t) 0)
> -#define GFP_USER ((gfp_t) 0)
> -#define __GFP_NOWARN ((gfp_t) 0)
> -
>   /* include/linux/leds.h */
>   struct led_trigger {};
>
> diff --git a/lib/linux_compat.c b/lib/linux_compat.c
> index 8c7a7b5..a936a7e 100644
> --- a/lib/linux_compat.c
> +++ b/lib/linux_compat.c
> @@ -16,14 +16,13 @@ unsigned long copy_from_user(void *dest, const void *src,
>
>   void *kmalloc(size_t size, int flags)
>   {
> -	return memalign(ARCH_DMA_MINALIGN, size);
> -}
> +	void *p;
>
> -void *kzalloc(size_t size, int flags)
> -{
> -	void *ptr = kmalloc(size, flags);
> -	memset(ptr, 0, size);
> -	return ptr;
> +	p = memalign(ARCH_DMA_MINALIGN, size);
> +	if (flags & __GFP_ZERO)
> +		memset(p, 0, size);
> +
> +	return p;
>   }
>
>   struct kmem_cache *get_mem(int element_sz)
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [RFC PATCH 04/12] linux_compat: handle __GFP_ZERO in kmalloc()

[Simon Glass]

On 7 July 2015 at 22:29, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Currently, kzalloc() returns zero-filled memory, while kmalloc()
> simply ignores the second argument and never fills the memory
> area with zeros.
>
> I want kmalloc(size, __GFP_ZERO) to behave as kzalloc() does,
> which will make it easier to add more memory allocator variants.
>
> With the introduction of __GFP_ZERO flag, going forward, kzmalloc()
> variants can fall back to kmalloc() enabling the __GFP_ZERO flag.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  include/linux/compat.h | 20 ++++++++++++--------
>  lib/linux_compat.c     | 13 ++++++-------
>  2 files changed, 18 insertions(+), 15 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [RFC PATCH 05/12] dm: add DM_FLAG_BOUND flag

[Masahiro Yamada]

Currently, we only have DM_FLAG_ACTIVATED to indicate the device
status, but we still cannot know in which stage is in progress,
binding or probing.

This commit introduces a new flag, DM_FLAG_BOUND, which is set when
the device is really bound, and cleared when it is unbound.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/core/device-remove.c | 3 +++
 drivers/core/device.c        | 2 ++
 include/dm/device.h          | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 6a16b4f..20b56f9 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -75,6 +75,9 @@ int device_unbind(struct udevice *dev)
 	if (dev->flags & DM_FLAG_ACTIVATED)
 		return -EINVAL;
 
+	if (!(dev->flags & DM_FLAG_BOUND))
+		return -EINVAL;
+
 	drv = dev->driver;
 	assert(drv);
 
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 85fd1fc..b954974 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -132,6 +132,8 @@ int device_bind(struct udevice *parent, const struct driver *drv,
 		dm_dbg("Bound device %s to %s\n", dev->name, parent->name);
 	*devp = dev;
 
+	dev->flags |= DM_FLAG_BOUND;
+
 	return 0;
 
 fail_child_post_bind:
diff --git a/include/dm/device.h b/include/dm/device.h
index 18296bb..3674d19 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -36,6 +36,9 @@ struct driver_info;
 /* Allocate driver private data on a DMA boundary */
 #define DM_FLAG_ALLOC_PRIV_DMA	(1 << 5)
 
+/* Device is bound */
+#define DM_FLAG_BOUND	(1 << 6)
+
 /**
  * struct udevice - An instance of a driver
  *
-- 
1.9.1

[U-Boot] [RFC PATCH 05/12] dm: add DM_FLAG_BOUND flag

[Simon Glass]

Hi Masahiro,

On 7 July 2015 at 22:29, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Currently, we only have DM_FLAG_ACTIVATED to indicate the device
> status, but we still cannot know in which stage is in progress,
> binding or probing.
>
> This commit introduces a new flag, DM_FLAG_BOUND, which is set when
> the device is really bound, and cleared when it is unbound.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Under what situation does a device exist without being bound? Binding
is supposed to be the act of creating the device.

OK I see after reading the rest of the patches that we might use
devm_alloc() during the bind process and want to track that. I guess
this is OK.

Regards,
Simon

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

[Masahiro Yamada]

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

 drivers/core/Kconfig         |  10 ++++
 drivers/core/Makefile        |   2 +-
 drivers/core/device-remove.c |   5 ++
 drivers/core/device.c        |   3 +
 drivers/core/devres.c        | 137 +++++++++++++++++++++++++++++++++++++++++++
 include/dm/device.h          |  17 ++++++
 6 files changed, 173 insertions(+), 1 deletion(-)
 create mode 100644 drivers/core/devres.c

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 2861b43..5966801 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -55,3 +55,13 @@ config DM_SEQ_ALIAS
 	  Most boards will have a '/aliases' node containing the path to
 	  numbered devices (e.g. serial0 = &serial0). This feature can be
 	  disabled if it is not required, to save code space in SPL.
+
+config DEBUG_DEVRES
+	bool "Managed device resources verbose debug messages"
+	depends on DM
+	help
+	  If this option is enabled, devres debug messages are printed.
+	  Select this if you are having a problem with devres or want to
+	  debug resource management for a managed device.
+
+	  If you are unsure about this, Say N here.
diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index a3fec38..cd8c104 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -4,7 +4,7 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
-obj-$(CONFIG_DM)	+= device.o lists.o root.o uclass.o util.o
+obj-$(CONFIG_DM)	+= device.o lists.o root.o uclass.o util.o devres.o
 ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_OF_CONTROL) += simple-bus.o
 endif
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 20b56f9..e1714b2 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -109,6 +109,9 @@ int device_unbind(struct udevice *dev)
 
 	if (dev->parent)
 		list_del(&dev->sibling_node);
+
+	devres_release_all(dev);
+
 	free(dev);
 
 	return 0;
@@ -142,6 +145,8 @@ void device_free(struct udevice *dev)
 			dev->parent_priv = NULL;
 		}
 	}
+
+	devres_release_probe(dev);
 }
 
 int device_remove(struct udevice *dev)
diff --git a/drivers/core/device.c b/drivers/core/device.c
index b954974..ac2c4f8 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -47,6 +47,7 @@ int device_bind(struct udevice *parent, const struct driver *drv,
 	INIT_LIST_HEAD(&dev->sibling_node);
 	INIT_LIST_HEAD(&dev->child_head);
 	INIT_LIST_HEAD(&dev->uclass_node);
+	INIT_LIST_HEAD(&dev->devres_head);
 	dev->platdata = platdata;
 	dev->name = name;
 	dev->of_offset = of_offset;
@@ -170,6 +171,8 @@ fail_alloc2:
 		dev->platdata = NULL;
 	}
 fail_alloc1:
+	devres_release_all(dev);
+
 	free(dev);
 
 	return ret;
diff --git a/drivers/core/devres.c b/drivers/core/devres.c
new file mode 100644
index 0000000..2e967bf
--- /dev/null
+++ b/drivers/core/devres.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2015  Masahiro Yamada <yamada.masahiro@socionext.com>
+ *
+ * Based on the original work in Linux by
+ * Copyright (c) 2006  SUSE Linux Products GmbH
+ * Copyright (c) 2006  Tejun Heo <teheo@suse.de>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <linux/compat.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <dm/device.h>
+
+struct devres {
+	struct list_head		entry;
+	dr_release_t			release;
+	bool				probe;
+#ifdef CONFIG_DEBUG_DEVRES
+	const char			*name;
+	size_t				size;
+#endif
+	unsigned long long		data[];
+};
+
+#ifdef CONFIG_DEBUG_DEVRES
+
+static void set_node_dbginfo(struct devres *dr, const char *name, size_t size)
+{
+	dr->name = name;
+	dr->size = size;
+}
+
+static void devres_log(struct udevice *dev, struct devres *dr,
+		       const char *op)
+{
+	printf("%s: DEVRES %3s %p %s (%lu bytes)\n",
+	       dev->name, op, dr, dr->name, (unsigned long)dr->size);
+}
+#else /* CONFIG_DEBUG_DEVRES */
+#define set_node_dbginfo(dr, n, s)	do {} while (0)
+#define devres_log(dev, dr, op)		do {} while (0)
+#endif
+
+/**
+ * devres_alloc - Allocate device resource data
+ * @release: Release function devres will be associated with
+ * @size: Allocation size
+ * @gfp: Allocation flags
+ *
+ * Allocate devres of @size bytes.  The allocated area is zeroed, then
+ * associated with @release.  The returned pointer can be passed to
+ * other devres_*() functions.
+ *
+ * RETURNS:
+ * Pointer to allocated devres on success, NULL on failure.
+ */
+#if CONFIG_DEBUG_DEVRES
+void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp,
+		     const char *name)
+#else
+void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp)
+#endif
+{
+	size_t tot_size = sizeof(struct devres) + size;
+	struct devres *dr;
+
+	dr = kmalloc(tot_size, gfp);
+	if (unlikely(!dr))
+		return NULL;
+
+	INIT_LIST_HEAD(&dr->entry);
+	dr->release = release;
+	set_node_dbginfo(dr, name, size);
+
+	return dr->data;
+}
+
+/**
+ * devres_add - Register device resource
+ * @dev: Device to add resource to
+ * @res: Resource to register
+ *
+ * Register devres @res to @dev.  @res should have been allocated
+ * using devres_alloc().  On driver detach, the associated release
+ * function will be invoked and devres will be freed automatically.
+ */
+void devres_add(struct udevice *dev, void *res)
+{
+	struct devres *dr = container_of(res, struct devres, data);
+
+	devres_log(dev, dr, "ADD");
+	BUG_ON(!list_empty(&dr->entry));
+	dr->probe = dev->flags & DM_FLAG_BOUND ? true : false;
+	list_add_tail(&dr->entry, &dev->devres_head);
+}
+
+static void release_nodes(struct udevice *dev, struct list_head *head,
+			  bool probe_only)
+{
+	struct devres *dr, *tmp;
+
+	list_for_each_entry_safe_reverse(dr, tmp, head, entry)  {
+		if (probe_only && !dr->probe)
+			break;
+		devres_log(dev, dr, "REL");
+		dr->release(dev, dr->data);
+		list_del(&dr->entry);
+		kfree(dr);
+	}
+}
+
+/**
+ * devres_release_probe - Release managed resources allocated after probing
+ * @dev: Device to release resources for
+ *
+ * Release all resources allocated for @dev when it was probed or later.
+ * This function is called on driver removal.
+ */
+void devres_release_probe(struct udevice *dev)
+{
+	release_nodes(dev, &dev->devres_head, true);
+}
+
+/**
+ * devres_release_all - Release all managed resources
+ * @dev: Device to release resources for
+ *
+ * Release all resources associated with @dev.  This function is
+ * called on driver unbinding.
+ */
+void devres_release_all(struct udevice *dev)
+{
+	release_nodes(dev, &dev->devres_head, false);
+}
diff --git a/include/dm/device.h b/include/dm/device.h
index 3674d19..7b39659 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -96,6 +96,7 @@ struct udevice {
 	uint32_t flags;
 	int req_seq;
 	int seq;
+	struct list_head devres_head;
 };
 
 /* Maximum sequence number supported */
@@ -449,4 +450,20 @@ bool device_has_active_children(struct udevice *dev);
  */
 bool device_is_last_sibling(struct udevice *dev);
 
+/* device resource management */
+typedef void (*dr_release_t)(struct udevice *dev, void *res);
+
+#ifdef CONFIG_DEBUG_DEVRES
+void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp,
+		     const char *name);
+#define devres_alloc(release, size, gfp) \
+	__devres_alloc(release, size, gfp, #release)
+#else
+void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp);
+#endif
+void devres_free(void *res);
+void devres_add(struct udevice *dev, void *res);
+void devres_release_probe(struct udevice *dev);
+void devres_release_all(struct udevice *dev);
+
 #endif
-- 
1.9.1

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

[Heiko Schocher]

Hello Masahiro,

Am 08.07.2015 um 06:29 schrieb Masahiro Yamada:
> 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>
> ---
>
>   drivers/core/Kconfig         |  10 ++++
>   drivers/core/Makefile        |   2 +-
>   drivers/core/device-remove.c |   5 ++
>   drivers/core/device.c        |   3 +
>   drivers/core/devres.c        | 137 +++++++++++++++++++++++++++++++++++++++++++
>   include/dm/device.h          |  17 ++++++
>   6 files changed, 173 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/core/devres.c

sounds good, nice work. Do you have a statistic how the codesize
footprint is? In the long term it should save codesize I think.

bye,
Heiko
>
> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> index 2861b43..5966801 100644
> --- a/drivers/core/Kconfig
> +++ b/drivers/core/Kconfig
> @@ -55,3 +55,13 @@ config DM_SEQ_ALIAS
>   	  Most boards will have a '/aliases' node containing the path to
>   	  numbered devices (e.g. serial0 = &serial0). This feature can be
>   	  disabled if it is not required, to save code space in SPL.
> +
> +config DEBUG_DEVRES
> +	bool "Managed device resources verbose debug messages"
> +	depends on DM
> +	help
> +	  If this option is enabled, devres debug messages are printed.
> +	  Select this if you are having a problem with devres or want to
> +	  debug resource management for a managed device.
> +
> +	  If you are unsure about this, Say N here.
> diff --git a/drivers/core/Makefile b/drivers/core/Makefile
> index a3fec38..cd8c104 100644
> --- a/drivers/core/Makefile
> +++ b/drivers/core/Makefile
> @@ -4,7 +4,7 @@
>   # SPDX-License-Identifier:	GPL-2.0+
>   #
>
> -obj-$(CONFIG_DM)	+= device.o lists.o root.o uclass.o util.o
> +obj-$(CONFIG_DM)	+= device.o lists.o root.o uclass.o util.o devres.o
>   ifndef CONFIG_SPL_BUILD
>   obj-$(CONFIG_OF_CONTROL) += simple-bus.o
>   endif
> diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
> index 20b56f9..e1714b2 100644
> --- a/drivers/core/device-remove.c
> +++ b/drivers/core/device-remove.c
> @@ -109,6 +109,9 @@ int device_unbind(struct udevice *dev)
>
>   	if (dev->parent)
>   		list_del(&dev->sibling_node);
> +
> +	devres_release_all(dev);
> +
>   	free(dev);
>
>   	return 0;
> @@ -142,6 +145,8 @@ void device_free(struct udevice *dev)
>   			dev->parent_priv = NULL;
>   		}
>   	}
> +
> +	devres_release_probe(dev);
>   }
>
>   int device_remove(struct udevice *dev)
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index b954974..ac2c4f8 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -47,6 +47,7 @@ int device_bind(struct udevice *parent, const struct driver *drv,
>   	INIT_LIST_HEAD(&dev->sibling_node);
>   	INIT_LIST_HEAD(&dev->child_head);
>   	INIT_LIST_HEAD(&dev->uclass_node);
> +	INIT_LIST_HEAD(&dev->devres_head);
>   	dev->platdata = platdata;
>   	dev->name = name;
>   	dev->of_offset = of_offset;
> @@ -170,6 +171,8 @@ fail_alloc2:
>   		dev->platdata = NULL;
>   	}
>   fail_alloc1:
> +	devres_release_all(dev);
> +
>   	free(dev);
>
>   	return ret;
> diff --git a/drivers/core/devres.c b/drivers/core/devres.c
> new file mode 100644
> index 0000000..2e967bf
> --- /dev/null
> +++ b/drivers/core/devres.c
> @@ -0,0 +1,137 @@
> +/*
> + * Copyright (C) 2015  Masahiro Yamada <yamada.masahiro@socionext.com>
> + *
> + * Based on the original work in Linux by
> + * Copyright (c) 2006  SUSE Linux Products GmbH
> + * Copyright (c) 2006  Tejun Heo <teheo@suse.de>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <linux/compat.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <dm/device.h>
> +
> +struct devres {
> +	struct list_head		entry;
> +	dr_release_t			release;
> +	bool				probe;
> +#ifdef CONFIG_DEBUG_DEVRES
> +	const char			*name;
> +	size_t				size;
> +#endif
> +	unsigned long long		data[];
> +};
> +
> +#ifdef CONFIG_DEBUG_DEVRES
> +
> +static void set_node_dbginfo(struct devres *dr, const char *name, size_t size)
> +{
> +	dr->name = name;
> +	dr->size = size;
> +}
> +
> +static void devres_log(struct udevice *dev, struct devres *dr,
> +		       const char *op)
> +{
> +	printf("%s: DEVRES %3s %p %s (%lu bytes)\n",
> +	       dev->name, op, dr, dr->name, (unsigned long)dr->size);
> +}
> +#else /* CONFIG_DEBUG_DEVRES */
> +#define set_node_dbginfo(dr, n, s)	do {} while (0)
> +#define devres_log(dev, dr, op)		do {} while (0)
> +#endif
> +
> +/**
> + * devres_alloc - Allocate device resource data
> + * @release: Release function devres will be associated with
> + * @size: Allocation size
> + * @gfp: Allocation flags
> + *
> + * Allocate devres of @size bytes.  The allocated area is zeroed, then
> + * associated with @release.  The returned pointer can be passed to
> + * other devres_*() functions.
> + *
> + * RETURNS:
> + * Pointer to allocated devres on success, NULL on failure.
> + */
> +#if CONFIG_DEBUG_DEVRES
> +void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp,
> +		     const char *name)
> +#else
> +void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp)
> +#endif
> +{
> +	size_t tot_size = sizeof(struct devres) + size;
> +	struct devres *dr;
> +
> +	dr = kmalloc(tot_size, gfp);
> +	if (unlikely(!dr))
> +		return NULL;
> +
> +	INIT_LIST_HEAD(&dr->entry);
> +	dr->release = release;
> +	set_node_dbginfo(dr, name, size);
> +
> +	return dr->data;
> +}
> +
> +/**
> + * devres_add - Register device resource
> + * @dev: Device to add resource to
> + * @res: Resource to register
> + *
> + * Register devres @res to @dev.  @res should have been allocated
> + * using devres_alloc().  On driver detach, the associated release
> + * function will be invoked and devres will be freed automatically.
> + */
> +void devres_add(struct udevice *dev, void *res)
> +{
> +	struct devres *dr = container_of(res, struct devres, data);
> +
> +	devres_log(dev, dr, "ADD");
> +	BUG_ON(!list_empty(&dr->entry));
> +	dr->probe = dev->flags & DM_FLAG_BOUND ? true : false;
> +	list_add_tail(&dr->entry, &dev->devres_head);
> +}
> +
> +static void release_nodes(struct udevice *dev, struct list_head *head,
> +			  bool probe_only)
> +{
> +	struct devres *dr, *tmp;
> +
> +	list_for_each_entry_safe_reverse(dr, tmp, head, entry)  {
> +		if (probe_only && !dr->probe)
> +			break;
> +		devres_log(dev, dr, "REL");
> +		dr->release(dev, dr->data);
> +		list_del(&dr->entry);
> +		kfree(dr);
> +	}
> +}
> +
> +/**
> + * devres_release_probe - Release managed resources allocated after probing
> + * @dev: Device to release resources for
> + *
> + * Release all resources allocated for @dev when it was probed or later.
> + * This function is called on driver removal.
> + */
> +void devres_release_probe(struct udevice *dev)
> +{
> +	release_nodes(dev, &dev->devres_head, true);
> +}
> +
> +/**
> + * devres_release_all - Release all managed resources
> + * @dev: Device to release resources for
> + *
> + * Release all resources associated with @dev.  This function is
> + * called on driver unbinding.
> + */
> +void devres_release_all(struct udevice *dev)
> +{
> +	release_nodes(dev, &dev->devres_head, false);
> +}
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 3674d19..7b39659 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -96,6 +96,7 @@ struct udevice {
>   	uint32_t flags;
>   	int req_seq;
>   	int seq;
> +	struct list_head devres_head;
>   };
>
>   /* Maximum sequence number supported */
> @@ -449,4 +450,20 @@ bool device_has_active_children(struct udevice *dev);
>    */
>   bool device_is_last_sibling(struct udevice *dev);
>
> +/* device resource management */
> +typedef void (*dr_release_t)(struct udevice *dev, void *res);
> +
> +#ifdef CONFIG_DEBUG_DEVRES
> +void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp,
> +		     const char *name);
> +#define devres_alloc(release, size, gfp) \
> +	__devres_alloc(release, size, gfp, #release)
> +#else
> +void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp);
> +#endif
> +void devres_free(void *res);
> +void devres_add(struct udevice *dev, void *res);
> +void devres_release_probe(struct udevice *dev);
> +void devres_release_all(struct udevice *dev);
> +
>   #endif
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Simon Glass]

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]

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

[Masahiro Yamada]

Hi Simon,


2015-07-09 9:22 GMT+09:00 Simon Glass <sjg@chromium.org>:
> 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.


Right.


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


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


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



-- 
Best Regards
Masahiro Yamada

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

[Albert ARIBAUD]

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.

[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@aribaud.net> wrote:
> 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?

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

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

[Albert ARIBAUD]

Hello Simon,

On Thu, 9 Jul 2015 07:31:05 -0600, Simon Glass <sjg@chromium.org> wrote:
> 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,
> >>
> >> > 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.

Then I'll be the Devil's advocate and question the interest of adding
code in U-Boot to fix a leak which, when it happens at all, does not
substantially affect U-Boot's functionality.

Amicalement,
-- 
Albert.

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

[Simon Glass]

Hi Albert,

On 9 July 2015 at 09:03, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
> Hello Simon,
>
> On Thu, 9 Jul 2015 07:31:05 -0600, Simon Glass <sjg@chromium.org> wrote:
>> 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,
>> >>
>> >> > 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.
>
> Then I'll be the Devil's advocate and question the interest of adding
> code in U-Boot to fix a leak which, when it happens at all, does not
> substantially affect U-Boot's functionality.

Yes, I don't think it matters in practice. I could fix it in the
current code too. To be complete we also need a way to 'allocate'
driver names such that they are freed when unbound. Not all names are
static strings.

Regards
Simon

[U-Boot] [RFC PATCH 07/12] devres: add devm_kmalloc() and friends (managed memory allocation)

[Masahiro Yamada]

devm_kmalloc() is identical to kmalloc() except that the memory
allocated with it is managed and will be automatically released
when the device is removed/unbound.

Likewise for the other variants.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/core/devres.c | 21 +++++++++++++++++++++
 include/dm/device.h   | 23 +++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/core/devres.c b/drivers/core/devres.c
index 2e967bf..f24bcac 100644
--- a/drivers/core/devres.c
+++ b/drivers/core/devres.c
@@ -135,3 +135,24 @@ void devres_release_all(struct udevice *dev)
 {
 	release_nodes(dev, &dev->devres_head, false);
 }
+
+/*
+ * Managed kmalloc/kfree
+ */
+static void devm_kmalloc_release(struct udevice *dev, void *res)
+{
+	/* noop */
+}
+
+void *devm_kmalloc(struct udevice *dev, size_t size, gfp_t gfp)
+{
+	void *data;
+
+	data = devres_alloc(devm_kmalloc_release, size, gfp);
+	if (unlikely(!data))
+		return NULL;
+
+	devres_add(dev, data);
+
+	return data;
+}
diff --git a/include/dm/device.h b/include/dm/device.h
index 7b39659..fac0868 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -14,6 +14,8 @@
 #include <dm/uclass-id.h>
 #include <fdtdec.h>
 #include <linker_lists.h>
+#include <linux/compat.h>
+#include <linux/kernel.h>
 #include <linux/list.h>
 
 struct driver_info;
@@ -466,4 +468,25 @@ void devres_add(struct udevice *dev, void *res);
 void devres_release_probe(struct udevice *dev);
 void devres_release_all(struct udevice *dev);
 
+/* managed devm_k.alloc/kfree for device drivers */
+void *devm_kmalloc(struct udevice *dev, size_t size, gfp_t gfp);
+static inline void *devm_kzalloc(struct udevice *dev, size_t size, gfp_t gfp)
+{
+	return devm_kmalloc(dev, size, gfp | __GFP_ZERO);
+}
+static inline void *devm_kmalloc_array(struct udevice *dev,
+				       size_t n, size_t size, gfp_t flags)
+{
+	if (size != 0 && n > SIZE_MAX / size)
+		return NULL;
+	return devm_kmalloc(dev, n * size, flags);
+}
+static inline void *devm_kcalloc(struct udevice *dev,
+				 size_t n, size_t size, gfp_t flags)
+{
+	return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
+}
+void devm_kfree(struct udevice *dev, void *p);
+
+
 #endif
-- 
1.9.1

[U-Boot] [RFC PATCH 07/12] devres: add devm_kmalloc() and friends (managed memory allocation)

[Simon Glass]

On 7 July 2015 at 22:29, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> devm_kmalloc() is identical to kmalloc() except that the memory
> allocated with it is managed and will be automatically released
> when the device is removed/unbound.
>
> Likewise for the other variants.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  drivers/core/devres.c | 21 +++++++++++++++++++++
>  include/dm/device.h   | 23 +++++++++++++++++++++++
>  2 files changed, 44 insertions(+)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [RFC PATCH 08/12] dm: refactor device_bind() and device_unbind() with devm_kzalloc()

[Masahiro Yamada]

With devm_kzalloc(), device_unbind() and the failure path in
device_bind() can be much cleaner.

We no longer need such flags as DM_FLAG_ALLOC_PDATA etc. because
we do not have to remember if the memory has been really allocated.

The memory is freed free when the device is unbind.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/core/device-remove.c | 12 ------------
 drivers/core/device.c        | 24 ++++++------------------
 include/dm/device.h          |  9 ---------
 3 files changed, 6 insertions(+), 39 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index e1714b2..0417535 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -91,18 +91,6 @@ int device_unbind(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	if (dev->flags & DM_FLAG_ALLOC_PDATA) {
-		free(dev->platdata);
-		dev->platdata = NULL;
-	}
-	if (dev->flags & DM_FLAG_ALLOC_UCLASS_PDATA) {
-		free(dev->uclass_platdata);
-		dev->uclass_platdata = NULL;
-	}
-	if (dev->flags & DM_FLAG_ALLOC_PARENT_PDATA) {
-		free(dev->parent_platdata);
-		dev->parent_platdata = NULL;
-	}
 	ret = uclass_unbind_device(dev);
 	if (ret)
 		return ret;
diff --git a/drivers/core/device.c b/drivers/core/device.c
index ac2c4f8..f024aa2 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -75,8 +75,9 @@ int device_bind(struct udevice *parent, const struct driver *drv,
 	}
 
 	if (!dev->platdata && drv->platdata_auto_alloc_size) {
-		dev->flags |= DM_FLAG_ALLOC_PDATA;
-		dev->platdata = calloc(1, drv->platdata_auto_alloc_size);
+		dev->platdata = devm_kzalloc(dev,
+					     drv->platdata_auto_alloc_size,
+					     GFP_KERNEL);
 		if (!dev->platdata) {
 			ret = -ENOMEM;
 			goto fail_alloc1;
@@ -85,8 +86,7 @@ int device_bind(struct udevice *parent, const struct driver *drv,
 
 	size = uc->uc_drv->per_device_platdata_auto_alloc_size;
 	if (size) {
-		dev->flags |= DM_FLAG_ALLOC_UCLASS_PDATA;
-		dev->uclass_platdata = calloc(1, size);
+		dev->uclass_platdata = devm_kzalloc(dev, size, GFP_KERNEL);
 		if (!dev->uclass_platdata) {
 			ret = -ENOMEM;
 			goto fail_alloc2;
@@ -100,8 +100,8 @@ int device_bind(struct udevice *parent, const struct driver *drv,
 					per_child_platdata_auto_alloc_size;
 		}
 		if (size) {
-			dev->flags |= DM_FLAG_ALLOC_PARENT_PDATA;
-			dev->parent_platdata = calloc(1, size);
+			dev->parent_platdata = devm_kzalloc(dev, size,
+							    GFP_KERNEL);
 			if (!dev->parent_platdata) {
 				ret = -ENOMEM;
 				goto fail_alloc3;
@@ -155,21 +155,9 @@ fail_bind:
 fail_uclass_bind:
 	if (IS_ENABLED(CONFIG_DM_DEVICE_REMOVE)) {
 		list_del(&dev->sibling_node);
-		if (dev->flags & DM_FLAG_ALLOC_PARENT_PDATA) {
-			free(dev->parent_platdata);
-			dev->parent_platdata = NULL;
-		}
 	}
 fail_alloc3:
-	if (dev->flags & DM_FLAG_ALLOC_UCLASS_PDATA) {
-		free(dev->uclass_platdata);
-		dev->uclass_platdata = NULL;
-	}
 fail_alloc2:
-	if (dev->flags & DM_FLAG_ALLOC_PDATA) {
-		free(dev->platdata);
-		dev->platdata = NULL;
-	}
 fail_alloc1:
 	devres_release_all(dev);
 
diff --git a/include/dm/device.h b/include/dm/device.h
index fac0868..9525581 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -23,18 +23,9 @@ struct driver_info;
 /* Driver is active (probed). Cleared when it is removed */
 #define DM_FLAG_ACTIVATED	(1 << 0)
 
-/* DM is responsible for allocating and freeing platdata */
-#define DM_FLAG_ALLOC_PDATA	(1 << 1)
-
 /* DM should init this device prior to relocation */
 #define DM_FLAG_PRE_RELOC	(1 << 2)
 
-/* DM is responsible for allocating and freeing parent_platdata */
-#define DM_FLAG_ALLOC_PARENT_PDATA	(1 << 3)
-
-/* DM is responsible for allocating and freeing uclass_platdata */
-#define DM_FLAG_ALLOC_UCLASS_PDATA	(1 << 4)
-
 /* Allocate driver private data on a DMA boundary */
 #define DM_FLAG_ALLOC_PRIV_DMA	(1 << 5)
 
-- 
1.9.1

[U-Boot] [RFC PATCH 09/12] dm: merge fail_alloc labels

[Masahiro Yamada]

A little more clean up made possible by devm_kzalloc().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/core/device.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index f024aa2..25b9b63 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -80,7 +80,7 @@ int device_bind(struct udevice *parent, const struct driver *drv,
 					     GFP_KERNEL);
 		if (!dev->platdata) {
 			ret = -ENOMEM;
-			goto fail_alloc1;
+			goto fail_alloc;
 		}
 	}
 
@@ -89,7 +89,7 @@ int device_bind(struct udevice *parent, const struct driver *drv,
 		dev->uclass_platdata = devm_kzalloc(dev, size, GFP_KERNEL);
 		if (!dev->uclass_platdata) {
 			ret = -ENOMEM;
-			goto fail_alloc2;
+			goto fail_alloc;
 		}
 	}
 
@@ -104,7 +104,7 @@ int device_bind(struct udevice *parent, const struct driver *drv,
 							    GFP_KERNEL);
 			if (!dev->parent_platdata) {
 				ret = -ENOMEM;
-				goto fail_alloc3;
+				goto fail_alloc;
 			}
 		}
 	}
@@ -156,9 +156,7 @@ fail_uclass_bind:
 	if (IS_ENABLED(CONFIG_DM_DEVICE_REMOVE)) {
 		list_del(&dev->sibling_node);
 	}
-fail_alloc3:
-fail_alloc2:
-fail_alloc1:
+fail_alloc:
 	devres_release_all(dev);
 
 	free(dev);
-- 
1.9.1

[U-Boot] [RFC PATCH 10/12] linux_compat: introduce GFP_DMA flag for kmalloc()

[Masahiro Yamada]

It does not seem efficient to always return cache-aligned memory.
Return aligned memory only when GFP_DMA flag is given.

My main motivation for this commit is to refactor device_probe()
and device_free() in the next commit.  DM_FLAG_ALLOC_PRIV_DMA
should be handled more easily.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 include/linux/compat.h | 1 +
 lib/linux_compat.c     | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index 64988bf..8b7f8ef 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -41,6 +41,7 @@ extern struct p_current *current;
 #define GFP_NOFS ((gfp_t) 0)
 #define GFP_USER ((gfp_t) 0)
 #define __GFP_NOWARN ((gfp_t) 0)
+#define GFP_DMA		((__force gfp_t)0x01u)
 #define __GFP_ZERO	((__force gfp_t)0x8000u) /* Return zeroed page on success */
 
 void *kmalloc(size_t size, int flags);
diff --git a/lib/linux_compat.c b/lib/linux_compat.c
index a936a7e..6da0cfa 100644
--- a/lib/linux_compat.c
+++ b/lib/linux_compat.c
@@ -18,7 +18,10 @@ void *kmalloc(size_t size, int flags)
 {
 	void *p;
 
-	p = memalign(ARCH_DMA_MINALIGN, size);
+	if (flags & GFP_DMA)
+		p = memalign(ARCH_DMA_MINALIGN, size);
+	else
+		p = malloc(size);
 	if (flags & __GFP_ZERO)
 		memset(p, 0, size);
 
-- 
1.9.1

[U-Boot] [RFC PATCH 10/12] linux_compat: introduce GFP_DMA flag for kmalloc()

[Heiko Schocher]

Hello Masahiro,

Am 08.07.2015 um 06:29 schrieb Masahiro Yamada:
> It does not seem efficient to always return cache-aligned memory.
> Return aligned memory only when GFP_DMA flag is given.
>
> My main motivation for this commit is to refactor device_probe()
> and device_free() in the next commit.  DM_FLAG_ALLOC_PRIV_DMA
> should be handled more easily.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>   include/linux/compat.h | 1 +
>   lib/linux_compat.c     | 5 ++++-
>   2 files changed, 5 insertions(+), 1 deletion(-)

Hmm... I think there are a lot of places in code, where implicit
is supposed, that kmalloc returns aligned memory ... so this
patch needs testing (USB, DFU and ethernet coming in my mind)

bye,
Heiko
>
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index 64988bf..8b7f8ef 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -41,6 +41,7 @@ extern struct p_current *current;
>   #define GFP_NOFS ((gfp_t) 0)
>   #define GFP_USER ((gfp_t) 0)
>   #define __GFP_NOWARN ((gfp_t) 0)
> +#define GFP_DMA		((__force gfp_t)0x01u)
>   #define __GFP_ZERO	((__force gfp_t)0x8000u) /* Return zeroed page on success */
>
>   void *kmalloc(size_t size, int flags);
> diff --git a/lib/linux_compat.c b/lib/linux_compat.c
> index a936a7e..6da0cfa 100644
> --- a/lib/linux_compat.c
> +++ b/lib/linux_compat.c
> @@ -18,7 +18,10 @@ void *kmalloc(size_t size, int flags)
>   {
>   	void *p;
>
> -	p = memalign(ARCH_DMA_MINALIGN, size);
> +	if (flags & GFP_DMA)
> +		p = memalign(ARCH_DMA_MINALIGN, size);
> +	else
> +		p = malloc(size);
>   	if (flags & __GFP_ZERO)
>   		memset(p, 0, size);
>
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [RFC PATCH 10/12] linux_compat: introduce GFP_DMA flag for kmalloc()

[Masahiro Yamada]

Hi Heiko,


2015-07-08 14:10 GMT+09:00 Heiko Schocher <hs@denx.de>:
> Hello Masahiro,
>
> Am 08.07.2015 um 06:29 schrieb Masahiro Yamada:
>>
>> It does not seem efficient to always return cache-aligned memory.
>> Return aligned memory only when GFP_DMA flag is given.
>>
>> My main motivation for this commit is to refactor device_probe()
>> and device_free() in the next commit.  DM_FLAG_ALLOC_PRIV_DMA
>> should be handled more easily.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>   include/linux/compat.h | 1 +
>>   lib/linux_compat.c     | 5 ++++-
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>
>
> Hmm... I think there are a lot of places in code, where implicit
> is supposed, that kmalloc returns aligned memory ... so this
> patch needs testing (USB, DFU and ethernet coming in my mind)


Right.

I wonder why cache-alignment is enabled by default.
Anybody who knows if it is also true for Linux?



-- 
Best Regards
Masahiro Yamada

[U-Boot] [RFC PATCH 11/12] dm: refactor device_probe() and device_remove() with devm_kzalloc()

[Masahiro Yamada]

The memory is automatically released on driver removal, so we do not
need to do so explicitly in device_free().

The helper function alloc_priv() is no longer needed because
devm_kzalloc() now understands the GFP_DMA flag.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/core/device-remove.c | 23 -----------------------
 drivers/core/device.c        | 25 +++++++------------------
 2 files changed, 7 insertions(+), 41 deletions(-)

diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 0417535..78551e4 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -111,29 +111,6 @@ int device_unbind(struct udevice *dev)
  */
 void device_free(struct udevice *dev)
 {
-	int size;
-
-	if (dev->driver->priv_auto_alloc_size) {
-		free(dev->priv);
-		dev->priv = NULL;
-	}
-	size = dev->uclass->uc_drv->per_device_auto_alloc_size;
-	if (size) {
-		free(dev->uclass_priv);
-		dev->uclass_priv = NULL;
-	}
-	if (dev->parent) {
-		size = dev->parent->driver->per_child_auto_alloc_size;
-		if (!size) {
-			size = dev->parent->uclass->uc_drv->
-					per_child_auto_alloc_size;
-		}
-		if (size) {
-			free(dev->parent_priv);
-			dev->parent_priv = NULL;
-		}
-	}
-
 	devres_release_probe(dev);
 }
 
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 25b9b63..e5291e2 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -179,27 +179,13 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only,
 			   -1, devp);
 }
 
-static void *alloc_priv(int size, uint flags)
-{
-	void *priv;
-
-	if (flags & DM_FLAG_ALLOC_PRIV_DMA) {
-		priv = memalign(ARCH_DMA_MINALIGN, size);
-		if (priv)
-			memset(priv, '\0', size);
-	} else {
-		priv = calloc(1, size);
-	}
-
-	return priv;
-}
-
 int device_probe_child(struct udevice *dev, void *parent_priv)
 {
 	const struct driver *drv;
 	int size = 0;
 	int ret;
 	int seq;
+	gfp_t flags = GFP_KERNEL;
 
 	if (!dev)
 		return -EINVAL;
@@ -210,9 +196,12 @@ int device_probe_child(struct udevice *dev, void *parent_priv)
 	drv = dev->driver;
 	assert(drv);
 
+	if (drv->flags & DM_FLAG_ALLOC_PRIV_DMA)
+		flags |= GFP_DMA;
+
 	/* Allocate private data if requested */
 	if (drv->priv_auto_alloc_size) {
-		dev->priv = alloc_priv(drv->priv_auto_alloc_size, drv->flags);
+		dev->priv = devm_kzalloc(dev, drv->priv_auto_alloc_size, flags);
 		if (!dev->priv) {
 			ret = -ENOMEM;
 			goto fail;
@@ -221,7 +210,7 @@ int device_probe_child(struct udevice *dev, void *parent_priv)
 	/* Allocate private data if requested */
 	size = dev->uclass->uc_drv->per_device_auto_alloc_size;
 	if (size) {
-		dev->uclass_priv = calloc(1, size);
+		dev->uclass_priv = devm_kzalloc(dev, size, GFP_KERNEL);
 		if (!dev->uclass_priv) {
 			ret = -ENOMEM;
 			goto fail;
@@ -236,7 +225,7 @@ int device_probe_child(struct udevice *dev, void *parent_priv)
 					per_child_auto_alloc_size;
 		}
 		if (size) {
-			dev->parent_priv = alloc_priv(size, drv->flags);
+			dev->parent_priv = devm_kzalloc(dev, size, flags);
 			if (!dev->parent_priv) {
 				ret = -ENOMEM;
 				goto fail;
-- 
1.9.1

[U-Boot] [RFC PATCH 11/12] dm: refactor device_probe() and device_remove() with devm_kzalloc()

[Heiko Schocher]

Hello Masahiro,

Am 08.07.2015 um 06:29 schrieb Masahiro Yamada:
> The memory is automatically released on driver removal, so we do not
> need to do so explicitly in device_free().
>
> The helper function alloc_priv() is no longer needed because
> devm_kzalloc() now understands the GFP_DMA flag.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>   drivers/core/device-remove.c | 23 -----------------------
>   drivers/core/device.c        | 25 +++++++------------------
>   2 files changed, 7 insertions(+), 41 deletions(-)

Nice statistic ... this effect should also popup in drivers code.

Reviewed-by: Heiko Schocher<hs@denx.de>

bye,
Heiko
>
> diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
> index 0417535..78551e4 100644
> --- a/drivers/core/device-remove.c
> +++ b/drivers/core/device-remove.c
> @@ -111,29 +111,6 @@ int device_unbind(struct udevice *dev)
>    */
>   void device_free(struct udevice *dev)
>   {
> -	int size;
> -
> -	if (dev->driver->priv_auto_alloc_size) {
> -		free(dev->priv);
> -		dev->priv = NULL;
> -	}
> -	size = dev->uclass->uc_drv->per_device_auto_alloc_size;
> -	if (size) {
> -		free(dev->uclass_priv);
> -		dev->uclass_priv = NULL;
> -	}
> -	if (dev->parent) {
> -		size = dev->parent->driver->per_child_auto_alloc_size;
> -		if (!size) {
> -			size = dev->parent->uclass->uc_drv->
> -					per_child_auto_alloc_size;
> -		}
> -		if (size) {
> -			free(dev->parent_priv);
> -			dev->parent_priv = NULL;
> -		}
> -	}
> -
>   	devres_release_probe(dev);
>   }
>
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 25b9b63..e5291e2 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -179,27 +179,13 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only,
>   			   -1, devp);
>   }
>
> -static void *alloc_priv(int size, uint flags)
> -{
> -	void *priv;
> -
> -	if (flags & DM_FLAG_ALLOC_PRIV_DMA) {
> -		priv = memalign(ARCH_DMA_MINALIGN, size);
> -		if (priv)
> -			memset(priv, '\0', size);
> -	} else {
> -		priv = calloc(1, size);
> -	}
> -
> -	return priv;
> -}
> -
>   int device_probe_child(struct udevice *dev, void *parent_priv)
>   {
>   	const struct driver *drv;
>   	int size = 0;
>   	int ret;
>   	int seq;
> +	gfp_t flags = GFP_KERNEL;
>
>   	if (!dev)
>   		return -EINVAL;
> @@ -210,9 +196,12 @@ int device_probe_child(struct udevice *dev, void *parent_priv)
>   	drv = dev->driver;
>   	assert(drv);
>
> +	if (drv->flags & DM_FLAG_ALLOC_PRIV_DMA)
> +		flags |= GFP_DMA;
> +
>   	/* Allocate private data if requested */
>   	if (drv->priv_auto_alloc_size) {
> -		dev->priv = alloc_priv(drv->priv_auto_alloc_size, drv->flags);
> +		dev->priv = devm_kzalloc(dev, drv->priv_auto_alloc_size, flags);
>   		if (!dev->priv) {
>   			ret = -ENOMEM;
>   			goto fail;
> @@ -221,7 +210,7 @@ int device_probe_child(struct udevice *dev, void *parent_priv)
>   	/* Allocate private data if requested */
>   	size = dev->uclass->uc_drv->per_device_auto_alloc_size;
>   	if (size) {
> -		dev->uclass_priv = calloc(1, size);
> +		dev->uclass_priv = devm_kzalloc(dev, size, GFP_KERNEL);
>   		if (!dev->uclass_priv) {
>   			ret = -ENOMEM;
>   			goto fail;
> @@ -236,7 +225,7 @@ int device_probe_child(struct udevice *dev, void *parent_priv)
>   					per_child_auto_alloc_size;
>   		}
>   		if (size) {
> -			dev->parent_priv = alloc_priv(size, drv->flags);
> +			dev->parent_priv = devm_kzalloc(dev, size, flags);
>   			if (!dev->parent_priv) {
>   				ret = -ENOMEM;
>   				goto fail;
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [RFC PATCH 12/12] devres: add debug command to dump devres

[Masahiro Yamada]

This new command can dump all device resources associated to
each device.  The fields in every line shows:
  - The address of the resource
  - The size of the resource
  - The name of the release function
  - The stage in which the resource has been acquired (BIND/PROBE)

The output looks like this:

=> devres
- root_driver
- soc
- extbus
- serial at 54006800
    0xbfb541e8 (8 byte) devm_kmalloc_release  BIND
    0xbfb54440 (4 byte) devm_kmalloc_release  PROBE
    0xbfb54460 (4 byte) devm_kmalloc_release  PROBE
- serial at 54006900
    0xbfb54270 (8 byte) devm_kmalloc_release  BIND
- gpio at 55000000
- i2c at 58780000
    0xbfb5bce8 (12 byte) devm_kmalloc_release  PROBE
    0xbfb5bd10 (4 byte) devm_kmalloc_release  PROBE
- eeprom
    0xbfb54418 (12 byte) devm_kmalloc_release  BIND

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/core/Kconfig  |  6 ++++++
 drivers/core/devres.c | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 5966801..9249ffd 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -65,3 +65,9 @@ config DEBUG_DEVRES
 	  debug resource management for a managed device.
 
 	  If you are unsure about this, Say N here.
+
+config CMD_DEVRES
+	bool "Debug command to dump device resources"
+	depends on DEBUG_DEVRES
+	help
+	  This command displays all resources allociated to each device.
diff --git a/drivers/core/devres.c b/drivers/core/devres.c
index f24bcac..71f2f67 100644
--- a/drivers/core/devres.c
+++ b/drivers/core/devres.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <dm/device.h>
+#include <dm/root.h>
 
 struct devres {
 	struct list_head		entry;
@@ -136,6 +137,42 @@ void devres_release_all(struct udevice *dev)
 	release_nodes(dev, &dev->devres_head, false);
 }
 
+#ifdef CONFIG_CMD_DEVRES
+void dump_resources(struct udevice *dev, int depth)
+{
+	struct devres *dr;
+	struct udevice *child;
+
+	printf("- %s\n", dev->name);
+
+	list_for_each_entry(dr, &dev->devres_head, entry)
+		printf("    0x%p (%lu byte) %s  %s\n", dr,
+		       (unsigned long)dr->size, dr->name,
+		       dr->probe ? "PROBE" : "BIND");
+
+	list_for_each_entry(child, &dev->child_head, sibling_node)
+		dump_resources(child, depth + 1);
+}
+
+static int do_devres(cmd_tbl_t *cmdtp, int flag, int argc,
+		     char * const argv[])
+{
+	struct udevice *root;
+
+	root = dm_root();
+	if (root)
+		dump_resources(root, 0);
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	devres,	1,	1,	do_devres,
+	"show device resources",
+	""
+);
+#endif
+
 /*
  * Managed kmalloc/kfree
  */
-- 
1.9.1

[U-Boot] [RFC PATCH 12/12] devres: add debug command to dump devres

[Masahiro Yamada]

2015-07-08 13:29 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:

> diff --git a/drivers/core/devres.c b/drivers/core/devres.c
> index f24bcac..71f2f67 100644
> --- a/drivers/core/devres.c
> +++ b/drivers/core/devres.c
> @@ -13,6 +13,7 @@
>  #include <linux/kernel.h>
>  #include <linux/list.h>
>  #include <dm/device.h>
> +#include <dm/root.h>
>
>  struct devres {
>         struct list_head                entry;
> @@ -136,6 +137,42 @@ void devres_release_all(struct udevice *dev)
>         release_nodes(dev, &dev->devres_head, false);
>  }
>
> +#ifdef CONFIG_CMD_DEVRES
> +void dump_resources(struct udevice *dev, int depth)
> +{


I forgot to add "static".

I will fix it in v2 if the basic idea of this series is OK.



-- 
Best Regards
Masahiro Yamada