[U-Boot] [PATCH] global_data: Move x86 specific GD flags into common flags

[Simon Goldschmidt]

Hi Stefan,

On Thu, Aug 15, 2019 at 10:06 AM Stefan Roese <sr at denx.de> wrote:
>
> Hi Simon,
>
> On 15.08.19 09:35, Simon Goldschmidt wrote:
> > Hi Stefan,
> >
> > On Thu, Aug 15, 2019 at 9:05 AM Stefan Roese <sr at denx.de> wrote:
> >>
> >> Currently the upper 16 bits of the GD flags are reserved for
> >> architecture specific flags. But only x86 uses 2 bits of these 16 bits
> >> and sprinkling those flags in multiple headers is confusing and does not
> >> scale.
> >>
> >> This patch now moves the x86 flags into the common header and removes
> >> the comment about the reservation of the upper 16 bits.
> >>
> >> Signed-off-by: Stefan Roese <sr at denx.de>
> >> Cc: Bin Meng <bmeng.cn at gmail.com>
> >> Cc: Simon Glass <sjg at chromium.org>
> >> Cc: Tom Rini <trini at konsulko.com>
> >> Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> >> ---
> >>   arch/x86/include/asm/global_data.h | 6 ------
> >>   include/asm-generic/global_data.h  | 6 +++++-
> >>   2 files changed, 5 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
> >> index 797397e697..17a4d34491 100644
> >> --- a/arch/x86/include/asm/global_data.h
> >> +++ b/arch/x86/include/asm/global_data.h
> >> @@ -137,10 +137,4 @@ static inline __attribute__((no_instrument_function)) gd_t *get_fs_gd_ptr(void)
> >>
> >>   #endif
> >>
> >> -/*
> >> - * Our private Global Data Flags
> >> - */
> >> -#define GD_FLG_COLD_BOOT       0x10000 /* Cold Boot */
> >> -#define GD_FLG_WARM_BOOT       0x20000 /* Warm Boot */
> >> -
> >>   #endif /* __ASM_GBL_DATA_H */
> >> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> >> index 5372d5d8cd..42ae61c781 100644
> >> --- a/include/asm-generic/global_data.h
> >> +++ b/include/asm-generic/global_data.h
> >> @@ -150,7 +150,7 @@ typedef struct global_data {
> >>   #endif
> >>
> >>   /*
> >> - * Global Data Flags - the top 16 bits are reserved for arch-specific flags
> >> + * Common Global Data Flags
> >>    */
> >>   #define GD_FLG_RELOC           0x00001 /* Code was relocated to RAM       */
> >>   #define GD_FLG_DEVINIT         0x00002 /* Devices have been initialized   */
> >> @@ -170,4 +170,8 @@ typedef struct global_data {
> >>   #define GD_FLG_LOG_READY       0x08000 /* Log system is ready for use     */
> >>   #define GD_FLG_WDT_READY       0x10000 /* Watchdog is ready for use       */
> >>
> >> +/* x86 specific GD flags */
> >> +#define GD_FLG_COLD_BOOT       0x20000 /* Cold Boot */
> >> +#define GD_FLG_WARM_BOOT       0x40000 /* Warm Boot */
> >
> > I think this change is generally good, but I failed to find code using the flag
> > GD_FLG_WARM_BOOT (it's only set, not checked). Only the cold-boot flag is used
> > (in coreboot's last_stage_init).
> >
> > So, since we don't have too many bits free here, instead of wasting 2 for x86,
> > could we probably remove one of these. Especially since you can either have
> > warm OR cold, but not both (so the absence of 'WARM' could mean 'COLD')?
>
> Yes. I already commented on those 2 x86 bits and their (non-)usage:
>
> https://lists.denx.de/pipermail/u-boot/2019-August/380634.html
> &
> https://lists.denx.de/pipermail/u-boot/2019-August/380800.html
>
> My suggestion is to apply this patch now and re-work the x86 flags usage
> in a later patch (most likely remove one of those bits). The main reason
> being that this double usage of 0x10000 needs to get fixed quickly.

You're right. In that case:
Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>

Regards,
Simon