[PATCH 00/12] drop volatile from global data

[PATCH 00/12] drop volatile from global data

[Rasmus Villemoes]

There's really no reason for the gd pointer to have the volatile
qualifier.

In fact, I claim that it's completely unnecessary and just pessimizes
code generation and forces ugly casts in lots of places. For example,
see the casts in drivers/core/tag.c where elements are added to
gd->dm_taglist, or a helper such as cyclic_get_list() that should not
be needed.

Also, it is what ends up standing in the way of an otherwise
innocent code cleanup of list.h:
https://lore.kernel.org/u-boot/20250522165656.GB2179216@bill-the-cat/

Note that riscv, x86 as well as arm64 with LTO enabled has not had
this volatile qualifier, so it's unlikely there's any generic code
that depends on it.

CI seems mostly happy:

  https://github.com/u-boot/u-boot/pull/775/checks?check_run_id=43463083829

The one failing test seems to be an infrastructure thing unrelated to
these patches ("docker: filesystem layer verification failed for
digest
sha256:379cc6cd06dc3fc80e31d5170204c0cfcc25cb3b3ad457ced23bed2b38ec9f36")


Rasmus Villemoes (12):
  arm: drop volatile qualifier from gd pointer
  powerpc: drop volatile qualifier from gd pointer
  mips: drop volatile qualifier from gd pointer
  microblaze: drop volatile qualifier from gd pointer
  m68k: drop volatile qualifier from gd pointer
  m68k: drop pointless #if 0 block
  arc: drop volatile qualifier from gd pointer
  arm: imx: remove unnecessary volatile qualifiers from "save gd"
    variables
  efi: drop volatile qualifier from "save gd" variables
  lib/trace: drop volatile qualifier from "save gd" variables
  arm: remove volatile from set_gd prototype
  riscv: remove volatile from set_gd prototype

 arch/arc/include/asm/global_data.h        | 2 +-
 arch/arm/include/asm/global_data.h        | 6 +++---
 arch/arm/mach-imx/hab.c                   | 2 +-
 arch/arm/mach-imx/imx9/scmi/soc.c         | 4 ++--
 arch/arm/mach-imx/romapi.c                | 6 +++---
 arch/m68k/include/asm/global_data.h       | 7 +------
 arch/microblaze/include/asm/global_data.h | 2 +-
 arch/mips/include/asm/global_data.h       | 2 +-
 arch/powerpc/include/asm/global_data.h    | 2 +-
 arch/riscv/include/asm/global_data.h      | 2 +-
 lib/efi_loader/efi_boottime.c             | 2 +-
 lib/trace.c                               | 4 ++--
 12 files changed, 18 insertions(+), 23 deletions(-)

-- 
2.49.0

[PATCH 01/12] arm: drop volatile qualifier from gd pointer

[Rasmus Villemoes]

There's a bunch of other places where this qualifier should be
dropped, e.g. in the set_gd() prototype and for various variables used
for stashing the value in the mach-imx/ directory and elsewhere. But
that will be done in follow-up patches.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/arm/include/asm/global_data.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index 45401d5e3c8..f7a47204b7c 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -133,9 +133,9 @@ static inline gd_t *get_gd(void)
 #else
 
 #ifdef CONFIG_ARM64
-#define DECLARE_GLOBAL_DATA_PTR		register volatile gd_t *gd asm ("x18")
+#define DECLARE_GLOBAL_DATA_PTR		register gd_t *gd asm ("x18")
 #else
-#define DECLARE_GLOBAL_DATA_PTR		register volatile gd_t *gd asm ("r9")
+#define DECLARE_GLOBAL_DATA_PTR		register gd_t *gd asm ("r9")
 #endif
 #endif
 
-- 
2.49.0

[PATCH 02/12] powerpc: drop volatile qualifier from gd pointer

[Rasmus Villemoes]

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/powerpc/include/asm/global_data.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/global_data.h b/arch/powerpc/include/asm/global_data.h
index cc2ce617350..26cbc7854d9 100644
--- a/arch/powerpc/include/asm/global_data.h
+++ b/arch/powerpc/include/asm/global_data.h
@@ -93,7 +93,7 @@ struct arch_global_data {
 
 #include <asm-generic/global_data.h>
 
-#define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm ("r2")
+#define DECLARE_GLOBAL_DATA_PTR     register gd_t *gd asm ("r2")
 
 #include <asm/u-boot.h>
 
-- 
2.49.0

[PATCH 03/12] mips: drop volatile qualifier from gd pointer

[Rasmus Villemoes]

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/mips/include/asm/global_data.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/global_data.h b/arch/mips/include/asm/global_data.h
index 147a95ecea8..265dd2a3ec4 100644
--- a/arch/mips/include/asm/global_data.h
+++ b/arch/mips/include/asm/global_data.h
@@ -44,6 +44,6 @@ struct arch_global_data {
 
 #include <asm-generic/global_data.h>
 
-#define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm ("k0")
+#define DECLARE_GLOBAL_DATA_PTR     register gd_t *gd asm ("k0")
 
 #endif /* __ASM_GBL_DATA_H */
-- 
2.49.0

[PATCH 04/12] microblaze: drop volatile qualifier from gd pointer

[Rasmus Villemoes]

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/microblaze/include/asm/global_data.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/microblaze/include/asm/global_data.h b/arch/microblaze/include/asm/global_data.h
index bb4112f22a3..f7922fac41c 100644
--- a/arch/microblaze/include/asm/global_data.h
+++ b/arch/microblaze/include/asm/global_data.h
@@ -18,7 +18,7 @@ struct arch_global_data {
 
 #include <asm-generic/global_data.h>
 
-#define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm ("r31")
+#define DECLARE_GLOBAL_DATA_PTR     register gd_t *gd asm ("r31")
 
 #define gd_cpuinfo()	((struct microblaze_cpuinfo *)&gd->arch.cpuinfo)
 
-- 
2.49.0

[PATCH 05/12] m68k: drop volatile qualifier from gd pointer

[Rasmus Villemoes]

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/m68k/include/asm/global_data.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/m68k/include/asm/global_data.h b/arch/m68k/include/asm/global_data.h
index 4ac886933c6..29b46645676 100644
--- a/arch/m68k/include/asm/global_data.h
+++ b/arch/m68k/include/asm/global_data.h
@@ -36,7 +36,7 @@ struct arch_global_data {
 extern gd_t *global_data;
 #define DECLARE_GLOBAL_DATA_PTR     gd_t *gd = global_data
 #else
-#define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm ("d7")
+#define DECLARE_GLOBAL_DATA_PTR     register gd_t *gd asm ("d7")
 #endif
 
 #endif /* __ASM_GBL_DATA_H */
-- 
2.49.0

[PATCH 06/12] m68k: drop pointless #if 0 block

[Rasmus Villemoes]

The way DECLARE_GLOBAL_DATA_PTR is used, the stuff under #if 0 can
never compile as you cannot have a non-constant initializer at global
scope (and one would get linker errors as well because the 'gd' symbol
would be defined in multiple TUs).

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/m68k/include/asm/global_data.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/m68k/include/asm/global_data.h b/arch/m68k/include/asm/global_data.h
index 29b46645676..aea2ccabe08 100644
--- a/arch/m68k/include/asm/global_data.h
+++ b/arch/m68k/include/asm/global_data.h
@@ -32,11 +32,6 @@ struct arch_global_data {
 
 #include <asm-generic/global_data.h>
 
-#if 0
-extern gd_t *global_data;
-#define DECLARE_GLOBAL_DATA_PTR     gd_t *gd = global_data
-#else
 #define DECLARE_GLOBAL_DATA_PTR     register gd_t *gd asm ("d7")
-#endif
 
 #endif /* __ASM_GBL_DATA_H */
-- 
2.49.0

[PATCH 07/12] arc: drop volatile qualifier from gd pointer

[Rasmus Villemoes]

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/arc/include/asm/global_data.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arc/include/asm/global_data.h b/arch/arc/include/asm/global_data.h
index fd9b7fb5f8d..4c3a25996fc 100644
--- a/arch/arc/include/asm/global_data.h
+++ b/arch/arc/include/asm/global_data.h
@@ -20,6 +20,6 @@ struct arch_global_data {
 
 #include <asm-generic/global_data.h>
 
-#define DECLARE_GLOBAL_DATA_PTR		register volatile gd_t *gd asm ("r25")
+#define DECLARE_GLOBAL_DATA_PTR		register gd_t *gd asm ("r25")
 
 #endif /* __ASM_ARC_GLOBAL_DATA_H */
-- 
2.49.0

[PATCH 08/12] arm: imx: remove unnecessary volatile qualifiers from "save gd" variables

[Rasmus Villemoes]

Now that the global gd pointer is no longer volatile-qualified,
there's no reason for the temporary variables used for
saving/restoring it to have that qualifier.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/arm/mach-imx/hab.c           | 2 +-
 arch/arm/mach-imx/imx9/scmi/soc.c | 4 ++--
 arch/arm/mach-imx/romapi.c        | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index 600092389a3..ab5861578e5 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -74,7 +74,7 @@ static int verify_ivt_header(struct ivt_header *ivt_hdr)
 #define FSL_SIP_HAB_REPORT_STATUS	0x04
 #define FSL_SIP_HAB_FAILSAFE		0x05
 #define FSL_SIP_HAB_CHECK_TARGET	0x06
-static volatile gd_t *gd_save;
+static gd_t *gd_save;
 #endif
 
 static inline void save_gd(void)
diff --git a/arch/arm/mach-imx/imx9/scmi/soc.c b/arch/arm/mach-imx/imx9/scmi/soc.c
index d2b0455bff9..d68a1166deb 100644
--- a/arch/arm/mach-imx/imx9/scmi/soc.c
+++ b/arch/arm/mach-imx/imx9/scmi/soc.c
@@ -67,7 +67,7 @@ int mmc_get_env_dev(void)
 	u16 boot_type;
 	u8 boot_instance;
 
-	volatile gd_t *pgd = gd;
+	gd_t *pgd = gd;
 	rom_passover_t *rdata;
 
 #if IS_ENABLED(CONFIG_XPL_BUILD)
@@ -675,7 +675,7 @@ enum imx9_soc_voltage_mode soc_target_voltage_mode(void)
 #if IS_ENABLED(CONFIG_SCMI_FIRMWARE)
 enum boot_device get_boot_device(void)
 {
-	volatile gd_t *pgd = gd;
+	gd_t *pgd = gd;
 	int ret;
 	u16 boot_type;
 	u8 boot_instance;
diff --git a/arch/arm/mach-imx/romapi.c b/arch/arm/mach-imx/romapi.c
index ff0522c2d11..c6fe4d8858e 100644
--- a/arch/arm/mach-imx/romapi.c
+++ b/arch/arm/mach-imx/romapi.c
@@ -9,7 +9,7 @@ DECLARE_GLOBAL_DATA_PTR;
 u32 rom_api_download_image(u8 *dest, u32 offset, u32 size)
 {
 	u32 xor = (uintptr_t)dest ^ offset ^ size;
-	volatile gd_t *sgd = gd;
+	gd_t *sgd = gd;
 	u32 ret;
 
 	ret = g_rom_api->download_image(dest, offset, size, xor);
@@ -21,7 +21,7 @@ u32 rom_api_download_image(u8 *dest, u32 offset, u32 size)
 u32 rom_api_query_boot_infor(u32 info_type, u32 *info)
 {
 	u32 xor = info_type ^ (uintptr_t)info;
-	volatile gd_t *sgd = gd;
+	gd_t *sgd = gd;
 	u32 ret;
 
 	ret = g_rom_api->query_boot_infor(info_type, info, xor);
@@ -34,7 +34,7 @@ extern struct rom_api *g_rom_api;
 
 enum boot_device get_boot_device(void)
 {
-	volatile gd_t *pgd = gd;
+	gd_t *pgd = gd;
 	int ret;
 	u32 boot;
 	u16 boot_type;
-- 
2.49.0

[PATCH 09/12] efi: drop volatile qualifier from "save gd" variables

[Rasmus Villemoes]

The global gd pointer is no longer volatile-qualified, so drop that
qualifier from these bookkeeping variables.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 lib/efi_loader/efi_boottime.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 24b0e52a2a2..754bc6a6519 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -58,7 +58,7 @@ static efi_handle_t current_image;
  * restriction so we need to manually swap its and our view of that register on
  * EFI callback entry/exit.
  */
-static volatile gd_t *efi_gd, *app_gd;
+static gd_t *efi_gd, *app_gd;
 #endif
 
 efi_status_t efi_uninstall_protocol
-- 
2.49.0

[PATCH 10/12] lib/trace: drop volatile qualifier from "save gd" variables

[Rasmus Villemoes]

The global gd pointer is no longer volatile-qualified, so drop that
qualifier from these bookkeeping variables.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 lib/trace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/trace.c b/lib/trace.c
index 1d5f7dec979..3d54dfaddc0 100644
--- a/lib/trace.c
+++ b/lib/trace.c
@@ -66,7 +66,7 @@ static inline uintptr_t __attribute__((no_instrument_function))
 /**
  * trace_gd - the value of the gd register
  */
-static volatile gd_t *trace_gd;
+static gd_t *trace_gd;
 
 /**
  * trace_save_gd() - save the value of the gd register
@@ -86,7 +86,7 @@ static void notrace trace_save_gd(void)
  */
 static void notrace trace_swap_gd(void)
 {
-	volatile gd_t *temp_gd = trace_gd;
+	gd_t *temp_gd = trace_gd;
 
 	trace_gd = gd;
 	set_gd(temp_gd);
-- 
2.49.0

[PATCH 11/12] arm: remove volatile from set_gd prototype

[Rasmus Villemoes]

The global gd pointer is no longer volatile-qualified. Callers of this
helper function have been updated to no longer use volatile-qualifed
temporary variables, so update the prototype accordingly.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/arm/include/asm/global_data.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index f7a47204b7c..b2ec450f900 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -139,7 +139,7 @@ static inline gd_t *get_gd(void)
 #endif
 #endif
 
-static inline void set_gd(volatile gd_t *gd_ptr)
+static inline void set_gd(gd_t *gd_ptr)
 {
 #ifdef CONFIG_ARM64
 	__asm__ volatile("ldr x18, %0\n" : : "m"(gd_ptr));
-- 
2.49.0

[PATCH 12/12] riscv: remove volatile from set_gd prototype

[Rasmus Villemoes]

It's slightly ironic that riscv at very first had the gd pointer
volatile qualified [6020faf62c3 ("riscv: nx25: include: Add header
files to support RISC-V")], removed that back in 2018
[40717eb849c ("riscv: checkpatch: Fix use of volatile")], and then in
2020 this helper was added [6b9966e1aa3 ("riscv: define function
set_gd()")] which needlessly had volatile in the prototype.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/riscv/include/asm/global_data.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
index 47b5e2cfc8f..33f2b5ec5c8 100644
--- a/arch/riscv/include/asm/global_data.h
+++ b/arch/riscv/include/asm/global_data.h
@@ -68,7 +68,7 @@ static inline gd_t *get_gd(void)
 
 #endif
 
-static inline void set_gd(volatile gd_t *gd_ptr)
+static inline void set_gd(gd_t *gd_ptr)
 {
 #ifdef CONFIG_64BIT
 	asm volatile("ld gp, %0\n" : : "m"(gd_ptr));
-- 
2.49.0

Re: [PATCH 00/12] drop volatile from global data

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]

On Wed, Jun 04, 2025 at 09:56:00PM +0200, Rasmus Villemoes wrote:
> There's really no reason for the gd pointer to have the volatile
> qualifier.
> 
> In fact, I claim that it's completely unnecessary and just pessimizes
> code generation and forces ugly casts in lots of places. For example,
> see the casts in drivers/core/tag.c where elements are added to
> gd->dm_taglist, or a helper such as cyclic_get_list() that should not
> be needed.
> 
> Also, it is what ends up standing in the way of an otherwise
> innocent code cleanup of list.h:
> https://lore.kernel.org/u-boot/20250522165656.GB2179216@bill-the-cat/
> 
> Note that riscv, x86 as well as arm64 with LTO enabled has not had
> this volatile qualifier, so it's unlikely there's any generic code
> that depends on it.
> 
> CI seems mostly happy:
> 
>   https://github.com/u-boot/u-boot/pull/775/checks?check_run_id=43463083829
> 
> The one failing test seems to be an infrastructure thing unrelated to
> these patches ("docker: filesystem layer verification failed for
> digest
> sha256:379cc6cd06dc3fc80e31d5170204c0cfcc25cb3b3ad457ced23bed2b38ec9f36")

Azure seems to be having some hiccups, sadly. I know from my end I can't
hit "rerun failed jobs", is there an option visible to you like that?
Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 00/12] drop volatile from global data

[Rasmus Villemoes]

On Wed, Jun 04 2025, Tom Rini <trini@konsulko.com> wrote:

> On Wed, Jun 04, 2025 at 09:56:00PM +0200, Rasmus Villemoes wrote:
>> There's really no reason for the gd pointer to have the volatile
>> qualifier.
>> 
>> In fact, I claim that it's completely unnecessary and just pessimizes
>> code generation and forces ugly casts in lots of places. For example,
>> see the casts in drivers/core/tag.c where elements are added to
>> gd->dm_taglist, or a helper such as cyclic_get_list() that should not
>> be needed.
>> 
>> Also, it is what ends up standing in the way of an otherwise
>> innocent code cleanup of list.h:
>> https://lore.kernel.org/u-boot/20250522165656.GB2179216@bill-the-cat/
>> 
>> Note that riscv, x86 as well as arm64 with LTO enabled has not had
>> this volatile qualifier, so it's unlikely there's any generic code
>> that depends on it.
>> 
>> CI seems mostly happy:
>> 
>>   https://github.com/u-boot/u-boot/pull/775/checks?check_run_id=43463083829
>> 
>> The one failing test seems to be an infrastructure thing unrelated to
>> these patches ("docker: filesystem layer verification failed for
>> digest
>> sha256:379cc6cd06dc3fc80e31d5170204c0cfcc25cb3b3ad457ced23bed2b38ec9f36")
>
> Azure seems to be having some hiccups, sadly. I know from my end I can't
> hit "rerun failed jobs", is there an option visible to you like that?

Technically yes, practically no.

On
https://dev.azure.com/u-boot/u-boot/_build/results?buildId=11303&view=results,
I see a "Rerun failed jobs" button. Clicking that, I get a popup saying

  Rerun Stage

  Are you sure you want to rerun failed jobs in the stage 'world_build'?

and after clicking yes, a banner with red(dish) background appears at
the top of the page with text

  TF400813: The user 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' is not authorized to access this resource.

and after that the "Rerun" button is greyed out.

I can do a trivial change the commit log for the first patch and
force-push, which triggers a new round of tests, but I don't know if
it's worth it. I think the current revision is the third time I've had
CI chew on it before sending the series to the list, and each time
there's been at least one test failing, every time for reasons I've
deemed unrelated to the patches.

Rasmus

Re: [PATCH 00/12] drop volatile from global data

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]

On Wed, Jun 04, 2025 at 02:07:02PM -0600, Tom Rini wrote:
> On Wed, Jun 04, 2025 at 09:56:00PM +0200, Rasmus Villemoes wrote:
> > There's really no reason for the gd pointer to have the volatile
> > qualifier.
> > 
> > In fact, I claim that it's completely unnecessary and just pessimizes
> > code generation and forces ugly casts in lots of places. For example,
> > see the casts in drivers/core/tag.c where elements are added to
> > gd->dm_taglist, or a helper such as cyclic_get_list() that should not
> > be needed.
> > 
> > Also, it is what ends up standing in the way of an otherwise
> > innocent code cleanup of list.h:
> > https://lore.kernel.org/u-boot/20250522165656.GB2179216@bill-the-cat/
> > 
> > Note that riscv, x86 as well as arm64 with LTO enabled has not had
> > this volatile qualifier, so it's unlikely there's any generic code
> > that depends on it.
> > 
> > CI seems mostly happy:
> > 
> >   https://github.com/u-boot/u-boot/pull/775/checks?check_run_id=43463083829
> > 
> > The one failing test seems to be an infrastructure thing unrelated to
> > these patches ("docker: filesystem layer verification failed for
> > digest
> > sha256:379cc6cd06dc3fc80e31d5170204c0cfcc25cb3b3ad457ced23bed2b38ec9f36")
> 
> Azure seems to be having some hiccups, sadly. I know from my end I can't
> hit "rerun failed jobs", is there an option visible to you like that?

Ah, this was a different Azure problem (I was looking like I was logged
in and authorized, but was in fact not authorized) so I hit the retry
failed jobs button now.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 09/12] efi: drop volatile qualifier from "save gd" variables

[Heinrich Schuchardt]

Am 4. Juni 2025 21:56:09 MESZ schrieb Rasmus Villemoes <ravi@prevas.dk>:
>The global gd pointer is no longer volatile-qualified, so drop that
>qualifier from these bookkeeping variables.
>
>Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


>---
> lib/efi_loader/efi_boottime.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>index 24b0e52a2a2..754bc6a6519 100644
>--- a/lib/efi_loader/efi_boottime.c
>+++ b/lib/efi_loader/efi_boottime.c
>@@ -58,7 +58,7 @@ static efi_handle_t current_image;
>  * restriction so we need to manually swap its and our view of that register on
>  * EFI callback entry/exit.
>  */
>-static volatile gd_t *efi_gd, *app_gd;
>+static gd_t *efi_gd, *app_gd;
> #endif
> 
> efi_status_t efi_uninstall_protocol

Re: [PATCH 06/12] m68k: drop pointless #if 0 block

[Angelo Dureghello]

thanks,

Acked-by: Angelo Dureghello <angelo@kernel-space.org>

On Wed,  4 Jun 2025 21:56:06 +0200

Rasmus Villemoes <ravi@prevas.dk> wrote:

> The way DECLARE_GLOBAL_DATA_PTR is used, the stuff under #if 0 can
> never compile as you cannot have a non-constant initializer at global
> scope (and one would get linker errors as well because the 'gd' symbol
> would be defined in multiple TUs).
> 
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> ---
>  arch/m68k/include/asm/global_data.h | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/arch/m68k/include/asm/global_data.h b/arch/m68k/include/asm/global_data.h
> index 29b46645676..aea2ccabe08 100644
> --- a/arch/m68k/include/asm/global_data.h
> +++ b/arch/m68k/include/asm/global_data.h
> @@ -32,11 +32,6 @@ struct arch_global_data {
>  
>  #include <asm-generic/global_data.h>
>  
> -#if 0
> -extern gd_t *global_data;
> -#define DECLARE_GLOBAL_DATA_PTR     gd_t *gd = global_data
> -#else
>  #define DECLARE_GLOBAL_DATA_PTR     register gd_t *gd asm ("d7")
> -#endif
>  
>  #endif /* __ASM_GBL_DATA_H */
> -- 
> 2.49.0
> 

Re: [PATCH 00/12] drop volatile from global data

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2613 bytes --]

On Wed, Jun 04, 2025 at 10:40:58PM +0200, Rasmus Villemoes wrote:
> On Wed, Jun 04 2025, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Wed, Jun 04, 2025 at 09:56:00PM +0200, Rasmus Villemoes wrote:
> >> There's really no reason for the gd pointer to have the volatile
> >> qualifier.
> >> 
> >> In fact, I claim that it's completely unnecessary and just pessimizes
> >> code generation and forces ugly casts in lots of places. For example,
> >> see the casts in drivers/core/tag.c where elements are added to
> >> gd->dm_taglist, or a helper such as cyclic_get_list() that should not
> >> be needed.
> >> 
> >> Also, it is what ends up standing in the way of an otherwise
> >> innocent code cleanup of list.h:
> >> https://lore.kernel.org/u-boot/20250522165656.GB2179216@bill-the-cat/
> >> 
> >> Note that riscv, x86 as well as arm64 with LTO enabled has not had
> >> this volatile qualifier, so it's unlikely there's any generic code
> >> that depends on it.
> >> 
> >> CI seems mostly happy:
> >> 
> >>   https://github.com/u-boot/u-boot/pull/775/checks?check_run_id=43463083829
> >> 
> >> The one failing test seems to be an infrastructure thing unrelated to
> >> these patches ("docker: filesystem layer verification failed for
> >> digest
> >> sha256:379cc6cd06dc3fc80e31d5170204c0cfcc25cb3b3ad457ced23bed2b38ec9f36")
> >
> > Azure seems to be having some hiccups, sadly. I know from my end I can't
> > hit "rerun failed jobs", is there an option visible to you like that?
> 
> Technically yes, practically no.
> 
> On
> https://dev.azure.com/u-boot/u-boot/_build/results?buildId=11303&view=results,
> I see a "Rerun failed jobs" button. Clicking that, I get a popup saying
> 
>   Rerun Stage
> 
>   Are you sure you want to rerun failed jobs in the stage 'world_build'?
> 
> and after clicking yes, a banner with red(dish) background appears at
> the top of the page with text
> 
>   TF400813: The user 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' is not authorized to access this resource.
> 
> and after that the "Rerun" button is greyed out.
> 
> I can do a trivial change the commit log for the first patch and
> force-push, which triggers a new round of tests, but I don't know if
> it's worth it. I think the current revision is the third time I've had
> CI chew on it before sending the series to the list, and each time
> there's been at least one test failing, every time for reasons I've
> deemed unrelated to the patches.

Azure is annoying at times, sigh. I re-ran the failed part and the
build is green now. Thanks for looking.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 01/12] arm: drop volatile qualifier from gd pointer

[Anshul Dalal]

On Thu Jun 5, 2025 at 1:26 AM IST, Rasmus Villemoes wrote:
> There's a bunch of other places where this qualifier should be
> dropped, e.g. in the set_gd() prototype and for various variables used
> for stashing the value in the mach-imx/ directory and elsewhere. But
> that will be done in follow-up patches.
>
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>

Tested-by: Anshul Dalal <anshuld@ti.com>

Re: [PATCH 08/12] arm: imx: remove unnecessary volatile qualifiers from "save gd" variables

[Anshul Dalal]

On Thu Jun 5, 2025 at 1:26 AM IST, Rasmus Villemoes wrote:
> Now that the global gd pointer is no longer volatile-qualified,
> there's no reason for the temporary variables used for
> saving/restoring it to have that qualifier.
>
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>

Tested-by: Anshul Dalal <anshuld@ti.com>

Re: [PATCH 00/12] drop volatile from global data

[Tom Rini]

On Wed, 04 Jun 2025 21:56:00 +0200, Rasmus Villemoes wrote:

> There's really no reason for the gd pointer to have the volatile
> qualifier.
> 
> In fact, I claim that it's completely unnecessary and just pessimizes
> code generation and forces ugly casts in lots of places. For example,
> see the casts in drivers/core/tag.c where elements are added to
> gd->dm_taglist, or a helper such as cyclic_get_list() that should not
> be needed.
> 
> [...]

Applied to u-boot/next, thanks!

[01/12] arm: drop volatile qualifier from gd pointer
        commit: 45fa0014ba56fc52765d66edda489933ce0e9871
[02/12] powerpc: drop volatile qualifier from gd pointer
        commit: aa53d9bb7d7b470eaa62e53550d66546f80d2446
[03/12] mips: drop volatile qualifier from gd pointer
        commit: 344c7e091c2033be4a5cb8bf73c08d8cf020b77d
[04/12] microblaze: drop volatile qualifier from gd pointer
        commit: fe0fa2bb6635ce3826b38204e68b284ada3a79fe
[05/12] m68k: drop volatile qualifier from gd pointer
        commit: e66d7fc089912a8f84bbbfa4a69eb5c0ebaa1e1d
[06/12] m68k: drop pointless #if 0 block
        commit: a6f3d70a6bc2f99ac86f47bc5697d7e6e36ea7b8
[07/12] arc: drop volatile qualifier from gd pointer
        commit: 30b97f58f20e58ea5bb1d5dce7253c58d03629d5
[08/12] arm: imx: remove unnecessary volatile qualifiers from "save gd" variables
        commit: 01869368c08278d19a0b7ec744013bc88afcf09a
[09/12] efi: drop volatile qualifier from "save gd" variables
        commit: 83b040983c92446c284c7b4ee5f97c60c6ab5afc
[10/12] lib/trace: drop volatile qualifier from "save gd" variables
        commit: abe705be4751dfd5ae2ed0c24971cd4417f645ad
[11/12] arm: remove volatile from set_gd prototype
        commit: ce55643546a59c05d6c4b181c2b490b6db7ba1f5
[12/12] riscv: remove volatile from set_gd prototype
        commit: ae86cd8c59fe08c9a37d14dbd6e42190dda2a0d4
-- 
Tom