diff mbox

[U-Boot,v3,2/8] board_init_f_mem(): Don't require memset()

Message ID 1445116021-27296-3-git-send-email-sjg@chromium.org
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Simon Glass Oct. 17, 2015, 9:06 p.m. UTC
Unfortunately memset() is not always available, so provide a substitute when
needed.

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

Changes in v3: None
Changes in v2:
- Add comments as to why this is needed, deal with arch-specific memset()

 common/init/board_init.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Albert ARIBAUD Oct. 18, 2015, 4:28 p.m. UTC | #1
Hello Simon,

On Sat, 17 Oct 2015 15:06:55 -0600, Simon Glass <sjg@chromium.org>
wrote:
> Unfortunately memset() is not always available, so provide a substitute when
> needed.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v3: None
> Changes in v2:
> - Add comments as to why this is needed, deal with arch-specific memset()
> 
>  common/init/board_init.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/common/init/board_init.c b/common/init/board_init.c
> index e7ebca7..c113a80 100644
> --- a/common/init/board_init.c
> +++ b/common/init/board_init.c
> @@ -11,6 +11,16 @@
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> +/*
> + * It isn't trivial to figure out whether memcpy() exists. The arch-specific
> + * memcpy() is not normally available in SPL due to code size.
> + */
> +#if !defined(CONFIG_SPL_BUILD) || \
> +		(defined(CONFIG_SPL_LIBGENERIC_SUPPORT) && \
> +		!defined(CONFIG_USE_ARCH_MEMSET))
> +#define _USE_MEMCPY
> +#endif
> +
>  /* Unfortunately x86 can't compile this code as gd cannot be assigned */
>  #ifndef CONFIG_X86
>  __weak void arch_setup_gd(struct global_data *gd_ptr)
> @@ -22,6 +32,9 @@ __weak void arch_setup_gd(struct global_data *gd_ptr)
>  ulong board_init_f_mem(ulong top)
>  {
>  	struct global_data *gd_ptr;
> +#ifndef _USE_MEMCPY
> +	int *ptr, *end;
> +#endif
>  
>  	/* Leave space for the stack we are running with now */
>  	top -= 0x40;
> @@ -29,7 +42,12 @@ ulong board_init_f_mem(ulong top)
>  	top -= sizeof(struct global_data);
>  	top = ALIGN(top, 16);
>  	gd_ptr = (struct global_data *)top;
> +#ifdef _USE_MEMCPY
>  	memset(gd_ptr, '\0', sizeof(*gd));
> +#else
> +	for (ptr = (int *)gd_ptr, end = (int *)(gd_ptr + 1); ptr < end; )

Nitpick here: There is little point in naming a variable just for
it to be set and used once. Without 'end', the compiler will be just as
fine if ptr is directly tested against (int *)(gd_ptr + 1), and human
readers won't wonder why 'end' was created.

> +		*ptr++ = 0;
> +#endif
>  	arch_setup_gd(gd_ptr);
>  
>  #if defined(CONFIG_SYS_MALLOC_F)
> -- 
> 2.6.0.rc2.230.g3dd15c0

Amicalement,
Simon Glass Oct. 18, 2015, 8:38 p.m. UTC | #2
Hi Albert,

On 18 October 2015 at 10:28, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
> Hello Simon,
>
> On Sat, 17 Oct 2015 15:06:55 -0600, Simon Glass <sjg@chromium.org>
> wrote:
>> Unfortunately memset() is not always available, so provide a substitute when
>> needed.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v3: None
>> Changes in v2:
>> - Add comments as to why this is needed, deal with arch-specific memset()
>>
>>  common/init/board_init.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/common/init/board_init.c b/common/init/board_init.c
>> index e7ebca7..c113a80 100644
>> --- a/common/init/board_init.c
>> +++ b/common/init/board_init.c
>> @@ -11,6 +11,16 @@
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> +/*
>> + * It isn't trivial to figure out whether memcpy() exists. The arch-specific
>> + * memcpy() is not normally available in SPL due to code size.
>> + */
>> +#if !defined(CONFIG_SPL_BUILD) || \
>> +             (defined(CONFIG_SPL_LIBGENERIC_SUPPORT) && \
>> +             !defined(CONFIG_USE_ARCH_MEMSET))
>> +#define _USE_MEMCPY
>> +#endif
>> +
>>  /* Unfortunately x86 can't compile this code as gd cannot be assigned */
>>  #ifndef CONFIG_X86
>>  __weak void arch_setup_gd(struct global_data *gd_ptr)
>> @@ -22,6 +32,9 @@ __weak void arch_setup_gd(struct global_data *gd_ptr)
>>  ulong board_init_f_mem(ulong top)
>>  {
>>       struct global_data *gd_ptr;
>> +#ifndef _USE_MEMCPY
>> +     int *ptr, *end;
>> +#endif
>>
>>       /* Leave space for the stack we are running with now */
>>       top -= 0x40;
>> @@ -29,7 +42,12 @@ ulong board_init_f_mem(ulong top)
>>       top -= sizeof(struct global_data);
>>       top = ALIGN(top, 16);
>>       gd_ptr = (struct global_data *)top;
>> +#ifdef _USE_MEMCPY
>>       memset(gd_ptr, '\0', sizeof(*gd));
>> +#else
>> +     for (ptr = (int *)gd_ptr, end = (int *)(gd_ptr + 1); ptr < end; )
>
> Nitpick here: There is little point in naming a variable just for
> it to be set and used once. Without 'end', the compiler will be just as
> fine if ptr is directly tested against (int *)(gd_ptr + 1), and human
> readers won't wonder why 'end' was created.

Well it makes it clear that the ptr goes from the start to the end.
But it's probably clear enough just doing what you suggest, so I can
change it.

>
>> +             *ptr++ = 0;
>> +#endif
>>       arch_setup_gd(gd_ptr);
>>
>>  #if defined(CONFIG_SYS_MALLOC_F)
>> --
>> 2.6.0.rc2.230.g3dd15c0
>
> Amicalement,
> --
> Albert.

Regards,
Simon
Albert ARIBAUD Oct. 19, 2015, 5:21 a.m. UTC | #3
Hello Simon,

On Sun, 18 Oct 2015 14:38:06 -0600, Simon Glass <sjg@chromium.org>
wrote:
> Hi Albert,
> 
> On 18 October 2015 at 10:28, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
> > Hello Simon,
> >
> > On Sat, 17 Oct 2015 15:06:55 -0600, Simon Glass <sjg@chromium.org>
> > wrote:
> >> Unfortunately memset() is not always available, so provide a substitute when
> >> needed.
> >>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> ---
> >>
> >> Changes in v3: None
> >> Changes in v2:
> >> - Add comments as to why this is needed, deal with arch-specific memset()
> >>
> >>  common/init/board_init.c | 18 ++++++++++++++++++
> >>  1 file changed, 18 insertions(+)
> >>
> >> diff --git a/common/init/board_init.c b/common/init/board_init.c
> >> index e7ebca7..c113a80 100644
> >> --- a/common/init/board_init.c
> >> +++ b/common/init/board_init.c
> >> @@ -11,6 +11,16 @@
> >>
> >>  DECLARE_GLOBAL_DATA_PTR;
> >>
> >> +/*
> >> + * It isn't trivial to figure out whether memcpy() exists. The arch-specific
> >> + * memcpy() is not normally available in SPL due to code size.
> >> + */
> >> +#if !defined(CONFIG_SPL_BUILD) || \
> >> +             (defined(CONFIG_SPL_LIBGENERIC_SUPPORT) && \
> >> +             !defined(CONFIG_USE_ARCH_MEMSET))
> >> +#define _USE_MEMCPY
> >> +#endif
> >> +
> >>  /* Unfortunately x86 can't compile this code as gd cannot be assigned */
> >>  #ifndef CONFIG_X86
> >>  __weak void arch_setup_gd(struct global_data *gd_ptr)
> >> @@ -22,6 +32,9 @@ __weak void arch_setup_gd(struct global_data *gd_ptr)
> >>  ulong board_init_f_mem(ulong top)
> >>  {
> >>       struct global_data *gd_ptr;
> >> +#ifndef _USE_MEMCPY
> >> +     int *ptr, *end;
> >> +#endif
> >>
> >>       /* Leave space for the stack we are running with now */
> >>       top -= 0x40;
> >> @@ -29,7 +42,12 @@ ulong board_init_f_mem(ulong top)
> >>       top -= sizeof(struct global_data);
> >>       top = ALIGN(top, 16);
> >>       gd_ptr = (struct global_data *)top;
> >> +#ifdef _USE_MEMCPY
> >>       memset(gd_ptr, '\0', sizeof(*gd));
> >> +#else
> >> +     for (ptr = (int *)gd_ptr, end = (int *)(gd_ptr + 1); ptr < end; )
> >
> > Nitpick here: There is little point in naming a variable just for
> > it to be set and used once. Without 'end', the compiler will be just as
> > fine if ptr is directly tested against (int *)(gd_ptr + 1), and human
> > readers won't wonder why 'end' was created.
> 
> Well it makes it clear that the ptr goes from the start to the end.
> But it's probably clear enough just doing what you suggest, so I can
> change it.

Please do, thanks!

Amicalement,
diff mbox

Patch

diff --git a/common/init/board_init.c b/common/init/board_init.c
index e7ebca7..c113a80 100644
--- a/common/init/board_init.c
+++ b/common/init/board_init.c
@@ -11,6 +11,16 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
+/*
+ * It isn't trivial to figure out whether memcpy() exists. The arch-specific
+ * memcpy() is not normally available in SPL due to code size.
+ */
+#if !defined(CONFIG_SPL_BUILD) || \
+		(defined(CONFIG_SPL_LIBGENERIC_SUPPORT) && \
+		!defined(CONFIG_USE_ARCH_MEMSET))
+#define _USE_MEMCPY
+#endif
+
 /* Unfortunately x86 can't compile this code as gd cannot be assigned */
 #ifndef CONFIG_X86
 __weak void arch_setup_gd(struct global_data *gd_ptr)
@@ -22,6 +32,9 @@  __weak void arch_setup_gd(struct global_data *gd_ptr)
 ulong board_init_f_mem(ulong top)
 {
 	struct global_data *gd_ptr;
+#ifndef _USE_MEMCPY
+	int *ptr, *end;
+#endif
 
 	/* Leave space for the stack we are running with now */
 	top -= 0x40;
@@ -29,7 +42,12 @@  ulong board_init_f_mem(ulong top)
 	top -= sizeof(struct global_data);
 	top = ALIGN(top, 16);
 	gd_ptr = (struct global_data *)top;
+#ifdef _USE_MEMCPY
 	memset(gd_ptr, '\0', sizeof(*gd));
+#else
+	for (ptr = (int *)gd_ptr, end = (int *)(gd_ptr + 1); ptr < end; )
+		*ptr++ = 0;
+#endif
 	arch_setup_gd(gd_ptr);
 
 #if defined(CONFIG_SYS_MALLOC_F)