Hi Stefan,

On Thu, Aug 15, 2019 at 9:05 AM Stefan Roese <s...@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 <s...@denx.de>
> Cc: Bin Meng <bmeng...@gmail.com>
> Cc: Simon Glass <s...@chromium.org>
> Cc: Tom Rini <tr...@konsulko.com>
> Cc: Simon Goldschmidt <simon.k.r.goldschm...@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')?

Regards,
Simon

> +
>  #endif /* __ASM_GENERIC_GBL_DATA_H */
> --
> 2.22.1
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to