Message ID | 20210503171001.12.Iea4abe7ae3455f42385e0d1c2b5776485b1ab88b@changeid |
---|---|
State | Changes Requested |
Delegated to: | Tom Rini |
Headers | show |
Series | image: Reduce #ifdefs and ad-hoc defines in image code | expand |
On 04/05/2021 01.10, Simon Glass wrote: > Rather than adding an #ifdef and open-coding this calculation, add a > helper function to handle it. Use this in the image code. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > (no changes since v1) > > common/image.c | 33 +++++++-------------------------- > include/relocate.h | 24 +++++++++++++++++++++++- > 2 files changed, 30 insertions(+), 27 deletions(-) > > diff --git a/common/image.c b/common/image.c > index 0ef8f30fcfa..086ae609f29 100644 > --- a/common/image.c > +++ b/common/image.c > @@ -63,6 +63,7 @@ DECLARE_GLOBAL_DATA_PTR; > #include <image.h> > #include <lz4.h> > #include <imximage.h> > +#include <relocate.h> > #include <linux/lzo.h> > #include <linux/zstd.h> > #include <linux/kconfig.h> > @@ -563,11 +564,7 @@ const char *genimg_get_cat_name(enum ih_category category, uint id) > entry = get_table_entry(table_info[category].table, id); > if (!entry) > return unknown_msg(category); > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) > - return entry->lname; > -#else > - return entry->lname + gd->reloc_off; > -#endif > + return manual_reloc(entry->lname); > } > > /** > @@ -587,11 +584,7 @@ const char *genimg_get_cat_short_name(enum ih_category category, uint id) > entry = get_table_entry(table_info[category].table, id); > if (!entry) > return unknown_msg(category); > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) > - return entry->sname; > -#else > - return entry->sname + gd->reloc_off; > -#endif > + return manual_reloc(entry->sname); > } > > int genimg_get_cat_count(enum ih_category category) > @@ -641,11 +634,7 @@ char *get_table_entry_name(const table_entry_t *table, char *msg, int id) > table = get_table_entry(table, id); > if (!table) > return msg; > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) > - return table->lname; > -#else > - return table->lname + gd->reloc_off; > -#endif > + return manual_reloc(table->lname); > } > > const char *genimg_get_os_name(uint8_t os) > @@ -675,11 +664,7 @@ static const char *genimg_get_short_name(const table_entry_t *table, int val) > table = get_table_entry(table, val); > if (!table) > return "unknown"; > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) > - return table->sname; > -#else > - return table->sname + gd->reloc_off; > -#endif > + return manual_reloc(table->sname); > } > > const char *genimg_get_type_short_name(uint8_t type) > @@ -722,12 +707,8 @@ int get_table_entry_id(const table_entry_t *table, > const table_entry_t *t; > > for (t = table; t->id >= 0; ++t) { > -#if !defined(USE_HOSTCC) && defined(CONFIG_NEEDS_MANUAL_RELOC) > - if (t->sname && strcasecmp(t->sname + gd->reloc_off, name) == 0) > -#else > - if (t->sname && strcasecmp(t->sname, name) == 0) > -#endif > - return (t->id); > + if (t->sname && !strcasecmp(manual_reloc(t->sname), name)) > + return t->id; > } > debug("Invalid %s Type: %s\n", table_name, name); > > diff --git a/include/relocate.h b/include/relocate.h > index 9ceeecdbe71..c4fad336128 100644 > --- a/include/relocate.h > +++ b/include/relocate.h > @@ -7,7 +7,11 @@ > #ifndef _RELOCATE_H_ > #define _RELOCATE_H_ > > -#include <common.h> > +#ifndef USE_HOSTCC > +#include <asm/global_data.h> > + > +DECLARE_GLOBAL_DATA_PTR; > +#endif > > /** > * copy_uboot_to_ram() - Copy U-Boot to its new relocated position > @@ -35,4 +39,22 @@ int clear_bss(void); > */ > int do_elf_reloc_fixups(void); > > +/** > + * manual_reloc() - Manually relocate a pointer if needed > + * > + * This is a nop in almost all cases, except for the systems with a broken gcc > + * which need to manually relocate some things. > + * > + * @ptr: Pointer to relocate > + * @return new pointer value > + */ > +static inline void *manual_reloc(void *ptr) > +{ > +#ifndef USE_HOSTCC > + if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC)) > + return ptr + gd->reloc_off; > +#endif > + return ptr; > +} static inlines are preferred in most cases, but you could make this const-correct if it was a macro. Dunno, maybe there are no const-qualified pointers where this might be used. The function also has the advantage of ensuring that the arithmetic is done on a void* pointer (i.e. effectively sizeof==1), though if the pointer is actually already a u32*, say, the current manual relocation code wouldn't be doing a bare ptr+gd->reloc_off anyway. Rasmus
Hi Rasmus, On Tue, 4 May 2021 at 01:17, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > On 04/05/2021 01.10, Simon Glass wrote: > > Rather than adding an #ifdef and open-coding this calculation, add a > > helper function to handle it. Use this in the image code. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > (no changes since v1) > > > > common/image.c | 33 +++++++-------------------------- > > include/relocate.h | 24 +++++++++++++++++++++++- > > 2 files changed, 30 insertions(+), 27 deletions(-) > > > > diff --git a/common/image.c b/common/image.c > > index 0ef8f30fcfa..086ae609f29 100644 > > --- a/common/image.c > > +++ b/common/image.c > > @@ -63,6 +63,7 @@ DECLARE_GLOBAL_DATA_PTR; > > #include <image.h> > > #include <lz4.h> > > #include <imximage.h> > > +#include <relocate.h> > > #include <linux/lzo.h> > > #include <linux/zstd.h> > > #include <linux/kconfig.h> > > @@ -563,11 +564,7 @@ const char *genimg_get_cat_name(enum ih_category category, uint id) > > entry = get_table_entry(table_info[category].table, id); > > if (!entry) > > return unknown_msg(category); > > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) > > - return entry->lname; > > -#else > > - return entry->lname + gd->reloc_off; > > -#endif > > + return manual_reloc(entry->lname); > > } > > > > /** > > @@ -587,11 +584,7 @@ const char *genimg_get_cat_short_name(enum ih_category category, uint id) > > entry = get_table_entry(table_info[category].table, id); > > if (!entry) > > return unknown_msg(category); > > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) > > - return entry->sname; > > -#else > > - return entry->sname + gd->reloc_off; > > -#endif > > + return manual_reloc(entry->sname); > > } > > > > int genimg_get_cat_count(enum ih_category category) > > @@ -641,11 +634,7 @@ char *get_table_entry_name(const table_entry_t *table, char *msg, int id) > > table = get_table_entry(table, id); > > if (!table) > > return msg; > > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) > > - return table->lname; > > -#else > > - return table->lname + gd->reloc_off; > > -#endif > > + return manual_reloc(table->lname); > > } > > > > const char *genimg_get_os_name(uint8_t os) > > @@ -675,11 +664,7 @@ static const char *genimg_get_short_name(const table_entry_t *table, int val) > > table = get_table_entry(table, val); > > if (!table) > > return "unknown"; > > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) > > - return table->sname; > > -#else > > - return table->sname + gd->reloc_off; > > -#endif > > + return manual_reloc(table->sname); > > } > > > > const char *genimg_get_type_short_name(uint8_t type) > > @@ -722,12 +707,8 @@ int get_table_entry_id(const table_entry_t *table, > > const table_entry_t *t; > > > > for (t = table; t->id >= 0; ++t) { > > -#if !defined(USE_HOSTCC) && defined(CONFIG_NEEDS_MANUAL_RELOC) > > - if (t->sname && strcasecmp(t->sname + gd->reloc_off, name) == 0) > > -#else > > - if (t->sname && strcasecmp(t->sname, name) == 0) > > -#endif > > - return (t->id); > > + if (t->sname && !strcasecmp(manual_reloc(t->sname), name)) > > + return t->id; > > } > > debug("Invalid %s Type: %s\n", table_name, name); > > > > diff --git a/include/relocate.h b/include/relocate.h > > index 9ceeecdbe71..c4fad336128 100644 > > --- a/include/relocate.h > > +++ b/include/relocate.h > > @@ -7,7 +7,11 @@ > > #ifndef _RELOCATE_H_ > > #define _RELOCATE_H_ > > > > -#include <common.h> > > +#ifndef USE_HOSTCC > > +#include <asm/global_data.h> > > + > > +DECLARE_GLOBAL_DATA_PTR; > > +#endif > > > > /** > > * copy_uboot_to_ram() - Copy U-Boot to its new relocated position > > @@ -35,4 +39,22 @@ int clear_bss(void); > > */ > > int do_elf_reloc_fixups(void); > > > > +/** > > + * manual_reloc() - Manually relocate a pointer if needed > > + * > > + * This is a nop in almost all cases, except for the systems with a broken gcc > > + * which need to manually relocate some things. > > + * > > + * @ptr: Pointer to relocate > > + * @return new pointer value > > + */ > > +static inline void *manual_reloc(void *ptr) > > +{ > > +#ifndef USE_HOSTCC > > + if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC)) > > + return ptr + gd->reloc_off; > > +#endif > > + return ptr; > > +} > > static inlines are preferred in most cases, but you could make this > const-correct if it was a macro. Dunno, maybe there are no > const-qualified pointers where this might be used. I can see what we would want that, but I haven't seen those used yet. > > The function also has the advantage of ensuring that the arithmetic is > done on a void* pointer (i.e. effectively sizeof==1), though if the > pointer is actually already a u32*, say, the current manual relocation > code wouldn't be doing a bare ptr+gd->reloc_off anyway. Right. Regards, SImon
diff --git a/common/image.c b/common/image.c index 0ef8f30fcfa..086ae609f29 100644 --- a/common/image.c +++ b/common/image.c @@ -63,6 +63,7 @@ DECLARE_GLOBAL_DATA_PTR; #include <image.h> #include <lz4.h> #include <imximage.h> +#include <relocate.h> #include <linux/lzo.h> #include <linux/zstd.h> #include <linux/kconfig.h> @@ -563,11 +564,7 @@ const char *genimg_get_cat_name(enum ih_category category, uint id) entry = get_table_entry(table_info[category].table, id); if (!entry) return unknown_msg(category); -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) - return entry->lname; -#else - return entry->lname + gd->reloc_off; -#endif + return manual_reloc(entry->lname); } /** @@ -587,11 +584,7 @@ const char *genimg_get_cat_short_name(enum ih_category category, uint id) entry = get_table_entry(table_info[category].table, id); if (!entry) return unknown_msg(category); -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) - return entry->sname; -#else - return entry->sname + gd->reloc_off; -#endif + return manual_reloc(entry->sname); } int genimg_get_cat_count(enum ih_category category) @@ -641,11 +634,7 @@ char *get_table_entry_name(const table_entry_t *table, char *msg, int id) table = get_table_entry(table, id); if (!table) return msg; -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) - return table->lname; -#else - return table->lname + gd->reloc_off; -#endif + return manual_reloc(table->lname); } const char *genimg_get_os_name(uint8_t os) @@ -675,11 +664,7 @@ static const char *genimg_get_short_name(const table_entry_t *table, int val) table = get_table_entry(table, val); if (!table) return "unknown"; -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC) - return table->sname; -#else - return table->sname + gd->reloc_off; -#endif + return manual_reloc(table->sname); } const char *genimg_get_type_short_name(uint8_t type) @@ -722,12 +707,8 @@ int get_table_entry_id(const table_entry_t *table, const table_entry_t *t; for (t = table; t->id >= 0; ++t) { -#if !defined(USE_HOSTCC) && defined(CONFIG_NEEDS_MANUAL_RELOC) - if (t->sname && strcasecmp(t->sname + gd->reloc_off, name) == 0) -#else - if (t->sname && strcasecmp(t->sname, name) == 0) -#endif - return (t->id); + if (t->sname && !strcasecmp(manual_reloc(t->sname), name)) + return t->id; } debug("Invalid %s Type: %s\n", table_name, name); diff --git a/include/relocate.h b/include/relocate.h index 9ceeecdbe71..c4fad336128 100644 --- a/include/relocate.h +++ b/include/relocate.h @@ -7,7 +7,11 @@ #ifndef _RELOCATE_H_ #define _RELOCATE_H_ -#include <common.h> +#ifndef USE_HOSTCC +#include <asm/global_data.h> + +DECLARE_GLOBAL_DATA_PTR; +#endif /** * copy_uboot_to_ram() - Copy U-Boot to its new relocated position @@ -35,4 +39,22 @@ int clear_bss(void); */ int do_elf_reloc_fixups(void); +/** + * manual_reloc() - Manually relocate a pointer if needed + * + * This is a nop in almost all cases, except for the systems with a broken gcc + * which need to manually relocate some things. + * + * @ptr: Pointer to relocate + * @return new pointer value + */ +static inline void *manual_reloc(void *ptr) +{ +#ifndef USE_HOSTCC + if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC)) + return ptr + gd->reloc_off; +#endif + return ptr; +} + #endif /* _RELOCATE_H_ */
Rather than adding an #ifdef and open-coding this calculation, add a helper function to handle it. Use this in the image code. Signed-off-by: Simon Glass <sjg@chromium.org> --- (no changes since v1) common/image.c | 33 +++++++-------------------------- include/relocate.h | 24 +++++++++++++++++++++++- 2 files changed, 30 insertions(+), 27 deletions(-)