Message ID | 1445116021-27296-3-git-send-email-sjg@chromium.org |
---|---|
State | Superseded |
Delegated to: | Tom Rini |
Headers | show |
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,
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
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 --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)
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(+)