Re: [U-Boot] [PATCH] global_data: Move x86 specific GD flags into common flags
Hi Stefan,
On Thu, Aug 15, 2019 at 10:06 AM Stefan Roese 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 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
> >> Cc: Bin Meng
> >> Cc: Simon Glass
> >> Cc: Tom Rini
> >> Cc: Simon Goldschmidt
> >> ---
> >> 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 0x1 /* Cold Boot */
> >> -#define GD_FLG_WARM_BOOT 0x2 /* 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 0x1 /* Code was relocated to RAM
> >> */
> >> #define GD_FLG_DEVINIT 0x2 /* 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 0x1 /* Watchdog is ready for use
> >> */
> >>
> >> +/* x86 specific GD flags */
> >> +#define GD_FLG_COLD_BOOT 0x2 /* Cold Boot */
> >> +#define GD_FLG_WARM_BOOT 0x4 /* 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 0x1 needs to get fixed quickly.
You're right. In that case:
Reviewed-by: Simon Goldschmidt
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] global_data: Move x86 specific GD flags into common flags
Hi Simon,
On 15.08.19 09:35, Simon Goldschmidt wrote:
Hi Stefan,
On Thu, Aug 15, 2019 at 9:05 AM Stefan Roese 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
Cc: Bin Meng
Cc: Simon Glass
Cc: Tom Rini
Cc: Simon Goldschmidt
---
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 0x1 /* Cold Boot */
-#define GD_FLG_WARM_BOOT 0x2 /* 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 0x1 /* Code was relocated to RAM */
#define GD_FLG_DEVINIT 0x2 /* 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 0x1 /* Watchdog is ready for use */
+/* x86 specific GD flags */
+#define GD_FLG_COLD_BOOT 0x2 /* Cold Boot */
+#define GD_FLG_WARM_BOOT 0x4 /* 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 0x1 needs to get fixed quickly.
Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] global_data: Move x86 specific GD flags into common flags
Hi Stefan,
On Thu, Aug 15, 2019 at 9:05 AM Stefan Roese 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
> Cc: Bin Meng
> Cc: Simon Glass
> Cc: Tom Rini
> Cc: Simon Goldschmidt
> ---
> 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 0x1 /* Cold Boot */
> -#define GD_FLG_WARM_BOOT 0x2 /* 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 0x1 /* Code was relocated to RAM */
> #define GD_FLG_DEVINIT 0x2 /* 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 0x1 /* Watchdog is ready for use */
>
> +/* x86 specific GD flags */
> +#define GD_FLG_COLD_BOOT 0x2 /* Cold Boot */
> +#define GD_FLG_WARM_BOOT 0x4 /* 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
Re: [U-Boot] [PATCH] global_data: Move x86 specific GD flags into common flags
On Thu, Aug 15, 2019 at 3:05 PM Stefan Roese 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 > Cc: Bin Meng > Cc: Simon Glass > Cc: Tom Rini > Cc: Simon Goldschmidt > --- > 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 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[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
Cc: Bin Meng
Cc: Simon Glass
Cc: Tom Rini
Cc: Simon Goldschmidt
---
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 0x1 /* Cold Boot */
-#define GD_FLG_WARM_BOOT 0x2 /* 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 0x1 /* Code was relocated to RAM */
#define GD_FLG_DEVINIT 0x2 /* 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 0x1 /* Watchdog is ready for use */
+/* x86 specific GD flags */
+#define GD_FLG_COLD_BOOT 0x2 /* Cold Boot */
+#define GD_FLG_WARM_BOOT 0x4 /* Warm Boot */
+
#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
