[U-Boot] [PATCH] global_data: Move x86 specific GD flags into common flags
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@denx.de Cc: Bin Meng bmeng.cn@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Cc: Simon Goldschmidt simon.k.r.goldschmidt@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 */ + #endif /* __ASM_GENERIC_GBL_DATA_H */
On Thu, Aug 15, 2019 at 3:05 PM Stefan Roese sr@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@denx.de Cc: Bin Meng bmeng.cn@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Cc: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
arch/x86/include/asm/global_data.h | 6 ------ include/asm-generic/global_data.h | 6 +++++- 2 files changed, 5 insertions(+), 7 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Hi Stefan,
On Thu, Aug 15, 2019 at 9:05 AM Stefan Roese sr@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@denx.de Cc: Bin Meng bmeng.cn@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Cc: Simon Goldschmidt simon.k.r.goldschmidt@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
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@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@denx.de Cc: Bin Meng bmeng.cn@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Cc: Simon Goldschmidt simon.k.r.goldschmidt@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
*/ #define GD_FLG_RELOC 0x00001 /* Code was relocated to RAM */ #define GD_FLG_DEVINIT 0x00002 /* Devices have been initialized */
- Common Global Data Flags
@@ -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.
Thanks, Stefan
Hi Stefan,
On Thu, Aug 15, 2019 at 10:06 AM Stefan Roese sr@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@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@denx.de Cc: Bin Meng bmeng.cn@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com Cc: Simon Goldschmidt simon.k.r.goldschmidt@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
*/ #define GD_FLG_RELOC 0x00001 /* Code was relocated to RAM */ #define GD_FLG_DEVINIT 0x00002 /* Devices have been initialized */
- Common Global Data Flags
@@ -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@gmail.com
Regards, Simon
participants (3)
-
Bin Meng -
Simon Goldschmidt -
Stefan Roese