[PATCH v1 0/4] x86: Clean up and fix to avoid compiler warnings

[PATCH v1 0/4] x86: Clean up and fix to avoid compiler warnings

[Andy Shevchenko]

Some fixes to avoid compiler warnings with `make W=1`. The idea to
continue this for at least some of the x86 related code.

Andy Shevchenko (4):
  x86: cpu: Use default print_cpuinfo() for all
  x86: cpu: Mark a few functions static
  x86: cpu: Add a few prototypes to the header file
  x86: cpu: Add missing header inclusion

 arch/x86/cpu/broadwell/cpu.c                 | 12 ------------
 arch/x86/cpu/coreboot/coreboot.c             |  5 -----
 arch/x86/cpu/cpu.c                           |  8 ++++++--
 arch/x86/cpu/cpu_x86.c                       |  1 +
 arch/x86/cpu/efi/app.c                       |  5 -----
 arch/x86/cpu/efi/payload.c                   |  5 -----
 arch/x86/cpu/i386/interrupt.c                |  2 +-
 arch/x86/cpu/ivybridge/cpu.c                 | 14 --------------
 arch/x86/cpu/mtrr.c                          |  2 +-
 arch/x86/cpu/qemu/qemu.c                     |  6 ------
 arch/x86/cpu/quark/quark.c                   |  6 ------
 arch/x86/cpu/slimbootloader/slimbootloader.c |  5 -----
 arch/x86/cpu/tangier/tangier.c               |  5 -----
 arch/x86/cpu/x86_64/misc.c                   |  5 -----
 arch/x86/include/asm/cpu.h                   |  7 +++++++
 arch/x86/include/asm/u-boot-x86.h            |  1 -
 arch/x86/lib/fsp/fsp_common.c                |  6 ------
 17 files changed, 16 insertions(+), 79 deletions(-)

-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 1/4] x86: cpu: Use default print_cpuinfo() for all

[Andy Shevchenko]

Most of the copies of the print_cpuinfo() call the default method.
Remove all of those in order to have only the default one when
no `cpu` command is compiled.

This also helps avoiding compiler warning, e.g.:

  arch/x86/cpu/tangier/tangier.c:23:5: warning: no previous prototype for ‘print_cpuinfo’ [-Wmissing-prototypes]

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/cpu/broadwell/cpu.c                 | 12 ------------
 arch/x86/cpu/coreboot/coreboot.c             |  5 -----
 arch/x86/cpu/cpu.c                           |  6 +++++-
 arch/x86/cpu/efi/app.c                       |  5 -----
 arch/x86/cpu/efi/payload.c                   |  5 -----
 arch/x86/cpu/ivybridge/cpu.c                 | 14 --------------
 arch/x86/cpu/qemu/qemu.c                     |  6 ------
 arch/x86/cpu/quark/quark.c                   |  6 ------
 arch/x86/cpu/slimbootloader/slimbootloader.c |  5 -----
 arch/x86/cpu/tangier/tangier.c               |  5 -----
 arch/x86/cpu/x86_64/misc.c                   |  5 -----
 arch/x86/include/asm/u-boot-x86.h            |  1 -
 arch/x86/lib/fsp/fsp_common.c                |  6 ------
 13 files changed, 5 insertions(+), 76 deletions(-)

diff --git a/arch/x86/cpu/broadwell/cpu.c b/arch/x86/cpu/broadwell/cpu.c
index dc6717eca40e..63698362e420 100644
--- a/arch/x86/cpu/broadwell/cpu.c
+++ b/arch/x86/cpu/broadwell/cpu.c
@@ -88,18 +88,6 @@ int checkcpu(void)
 	return 0;
 }
 
-int print_cpuinfo(void)
-{
-	char processor_name[CPU_MAX_NAME_LEN];
-	const char *name;
-
-	/* Print processor name */
-	name = cpu_get_name(processor_name);
-	printf("CPU:   %s\n", name);
-
-	return 0;
-}
-
 void board_debug_uart_init(void)
 {
 	/* com1 / com2 decode range */
diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c
index d474c79e25ef..8ed4483b24b2 100644
--- a/arch/x86/cpu/coreboot/coreboot.c
+++ b/arch/x86/cpu/coreboot/coreboot.c
@@ -43,11 +43,6 @@ int checkcpu(void)
 	return 0;
 }
 
-int print_cpuinfo(void)
-{
-	return default_print_cpuinfo();
-}
-
 static void board_final_init(void)
 {
 	/*
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index ad21fdb457a3..3aa35b811b36 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -163,8 +163,11 @@ char *cpu_get_name(char *name)
 	return ptr;
 }
 
-int default_print_cpuinfo(void)
+#if !CONFIG_IS_ENABLED(CPU)
+int print_cpuinfo(void)
 {
+	post_code(POST_CPU_INFO);
+
 	printf("CPU: %s, vendor %s, device %xh\n",
 	       cpu_has_64bit() ? "x86_64" : "x86",
 	       cpu_vendor_name(gd->arch.x86_vendor), gd->arch.x86_device);
@@ -176,6 +179,7 @@ int default_print_cpuinfo(void)
 
 	return 0;
 }
+#endif
 
 #if CONFIG_IS_ENABLED(SHOW_BOOT_PROGRESS)
 void show_boot_progress(int val)
diff --git a/arch/x86/cpu/efi/app.c b/arch/x86/cpu/efi/app.c
index 218a68c4642d..84fe50e2f2f8 100644
--- a/arch/x86/cpu/efi/app.c
+++ b/arch/x86/cpu/efi/app.c
@@ -19,11 +19,6 @@ int checkcpu(void)
 	return 0;
 }
 
-int print_cpuinfo(void)
-{
-	return default_print_cpuinfo();
-}
-
 void board_final_init(void)
 {
 }
diff --git a/arch/x86/cpu/efi/payload.c b/arch/x86/cpu/efi/payload.c
index 642a87a37d8b..6845ce72ff94 100644
--- a/arch/x86/cpu/efi/payload.c
+++ b/arch/x86/cpu/efi/payload.c
@@ -144,11 +144,6 @@ int checkcpu(void)
 	return 0;
 }
 
-int print_cpuinfo(void)
-{
-	return default_print_cpuinfo();
-}
-
 /* Find any available tables and copy them to a safe place */
 int reserve_arch(void)
 {
diff --git a/arch/x86/cpu/ivybridge/cpu.c b/arch/x86/cpu/ivybridge/cpu.c
index d71ab0a6385e..c2ec52b9a912 100644
--- a/arch/x86/cpu/ivybridge/cpu.c
+++ b/arch/x86/cpu/ivybridge/cpu.c
@@ -187,20 +187,6 @@ int checkcpu(void)
 	return 0;
 }
 
-int print_cpuinfo(void)
-{
-	char processor_name[CPU_MAX_NAME_LEN];
-	const char *name;
-
-	/* Print processor name */
-	name = cpu_get_name(processor_name);
-	printf("CPU:   %s\n", name);
-
-	post_code(POST_CPU_INFO);
-
-	return 0;
-}
-
 void board_debug_uart_init(void)
 {
 	/* This enables the debug UART */
diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
index 262584d01f0f..563f63e2bc81 100644
--- a/arch/x86/cpu/qemu/qemu.c
+++ b/arch/x86/cpu/qemu/qemu.c
@@ -109,12 +109,6 @@ int checkcpu(void)
 {
 	return 0;
 }
-
-int print_cpuinfo(void)
-{
-	post_code(POST_CPU_INFO);
-	return default_print_cpuinfo();
-}
 #endif
 
 int arch_early_init_r(void)
diff --git a/arch/x86/cpu/quark/quark.c b/arch/x86/cpu/quark/quark.c
index fdf92b2c0c3a..07504faaffb7 100644
--- a/arch/x86/cpu/quark/quark.c
+++ b/arch/x86/cpu/quark/quark.c
@@ -266,12 +266,6 @@ int checkcpu(void)
 	return 0;
 }
 
-int print_cpuinfo(void)
-{
-	post_code(POST_CPU_INFO);
-	return default_print_cpuinfo();
-}
-
 static void quark_pcie_init(void)
 {
 	u32 val;
diff --git a/arch/x86/cpu/slimbootloader/slimbootloader.c b/arch/x86/cpu/slimbootloader/slimbootloader.c
index 142c9341cf86..8a5c78595aa6 100644
--- a/arch/x86/cpu/slimbootloader/slimbootloader.c
+++ b/arch/x86/cpu/slimbootloader/slimbootloader.c
@@ -54,8 +54,3 @@ int checkcpu(void)
 {
 	return 0;
 }
-
-int print_cpuinfo(void)
-{
-	return default_print_cpuinfo();
-}
diff --git a/arch/x86/cpu/tangier/tangier.c b/arch/x86/cpu/tangier/tangier.c
index 8a8f7d27a9d1..b005bc7d9a0e 100644
--- a/arch/x86/cpu/tangier/tangier.c
+++ b/arch/x86/cpu/tangier/tangier.c
@@ -19,8 +19,3 @@ int checkcpu(void)
 {
 	return 0;
 }
-
-int print_cpuinfo(void)
-{
-	return default_print_cpuinfo();
-}
diff --git a/arch/x86/cpu/x86_64/misc.c b/arch/x86/cpu/x86_64/misc.c
index 294511e6ebab..fc449ca4ed67 100644
--- a/arch/x86/cpu/x86_64/misc.c
+++ b/arch/x86/cpu/x86_64/misc.c
@@ -32,9 +32,4 @@ int checkcpu(void)
 {
 	return 0;
 }
-
-int print_cpuinfo(void)
-{
-	return 0;
-}
 #endif
diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h
index 3acc58ad74b4..9418ef9742a4 100644
--- a/arch/x86/include/asm/u-boot-x86.h
+++ b/arch/x86/include/asm/u-boot-x86.h
@@ -78,7 +78,6 @@ void x86_enable_caches(void);
 void x86_disable_caches(void);
 int x86_init_cache(void);
 phys_addr_t board_get_usable_ram_top(phys_size_t total_size);
-int default_print_cpuinfo(void);
 
 /* Set up a UART which can be used with printch(), printhex8(), etc. */
 int setup_internal_uart(int enable);
diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c
index c47e6ca47388..7e4c14766347 100644
--- a/arch/x86/lib/fsp/fsp_common.c
+++ b/arch/x86/lib/fsp/fsp_common.c
@@ -26,12 +26,6 @@ int checkcpu(void)
 	return 0;
 }
 
-int print_cpuinfo(void)
-{
-	post_code(POST_CPU_INFO);
-	return default_print_cpuinfo();
-}
-
 int fsp_init_phase_pci(void)
 {
 	u32 status;
-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 2/4] x86: cpu: Mark a few functions static

[Andy Shevchenko]

Some functions are not used anywhere except the same file
where they are defined. Mark them static. This helps avoiding
the compiler warnings:

  arch/x86/cpu/cpu.c:343:6: warning: no previous prototype for ‘detect_coreboot_table_at’ [-Wmissing-prototypes]
  arch/x86/cpu/mtrr.c:90:6: warning: no previous prototype for ‘mtrr_write_all’ [-Wmissing-prototypes]
  arch/x86/cpu/i386/interrupt.c:240:6: warning: no previous prototype for ‘__do_irq’ [-Wmissing-prototypes]

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/cpu/cpu.c            | 2 +-
 arch/x86/cpu/i386/interrupt.c | 2 +-
 arch/x86/cpu/mtrr.c           | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index 3aa35b811b36..e9300709347e 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -340,7 +340,7 @@ int reserve_arch(void)
 }
 #endif
 
-long detect_coreboot_table_at(ulong start, ulong size)
+static long detect_coreboot_table_at(ulong start, ulong size)
 {
 	u32 *ptr, *end;
 
diff --git a/arch/x86/cpu/i386/interrupt.c b/arch/x86/cpu/i386/interrupt.c
index b3f4214acdb7..6f78b072cde5 100644
--- a/arch/x86/cpu/i386/interrupt.c
+++ b/arch/x86/cpu/i386/interrupt.c
@@ -237,7 +237,7 @@ void *x86_get_idt(void)
 	return &idt_ptr;
 }
 
-void __do_irq(int irq)
+static void __do_irq(int irq)
 {
 	printf("Unhandled IRQ : %d\n", irq);
 }
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c
index 50cba5fb88dc..07ea89162dee 100644
--- a/arch/x86/cpu/mtrr.c
+++ b/arch/x86/cpu/mtrr.c
@@ -87,7 +87,7 @@ void mtrr_read_all(struct mtrr_info *info)
 	}
 }
 
-void mtrr_write_all(struct mtrr_info *info)
+static void mtrr_write_all(struct mtrr_info *info)
 {
 	int reg_count = mtrr_get_var_count();
 	struct mtrr_state state;
-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 3/4] x86: cpu: Add a few prototypes to the header file

[Andy Shevchenko]

The compiler is not happy to have no prototypes for the functions that
are not defined static. Add them. This helps avoiding the compiler warnings:

  arch/x86/cpu/cpu.c:197:13: warning: no previous prototype for ‘board_final_init’ [-Wmissing-prototypes]
  arch/x86/cpu/cpu.c:205:13: warning: no previous prototype for ‘board_final_cleanup’ [-Wmissing-prototypes]
  arch/x86/cpu/cpu.c:307:5: warning: no previous prototype for ‘reserve_arch’ [-Wmissing-prototypes]

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/include/asm/cpu.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 073f80b07f10..d71bc1b80c05 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -298,4 +298,11 @@ u32 cpu_get_stepping(void);
  */
 int cpu_phys_address_size(void);
 
+void board_final_init(void);
+void board_final_cleanup(void);
+
+#ifndef CONFIG_EFI_STUB
+int reserve_arch(void);
+#endif
+
 #endif
-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 4/4] x86: cpu: Add missing header inclusion

[Andy Shevchenko]

Without asm/cpu_x86.h inclusion a compiler is not happy:

  arch/x86/cpu/cpu_x86.c:14:5: warning: no previous prototype for ‘cpu_x86_bind’ [-Wmissing-prototypes]
  arch/x86/cpu/cpu_x86.c:29:5: warning: no previous prototype for ‘cpu_x86_get_vendor’ [-Wmissing-prototypes]
  arch/x86/cpu/cpu_x86.c:41:5: warning: no previous prototype for ‘cpu_x86_get_desc’ [-Wmissing-prototypes]
  arch/x86/cpu/cpu_x86.c:55:5: warning: no previous prototype for ‘cpu_x86_get_count’ [-Wmissing-prototypes]

Add missing header inclusion.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/cpu/cpu_x86.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/cpu/cpu_x86.c b/arch/x86/cpu/cpu_x86.c
index 6c53f0ea821f..6c32ae499df3 100644
--- a/arch/x86/cpu/cpu_x86.c
+++ b/arch/x86/cpu/cpu_x86.c
@@ -7,6 +7,7 @@
 #include <dm.h>
 #include <errno.h>
 #include <asm/cpu.h>
+#include <asm/cpu_x86.h>
 #include <asm/global_data.h>
 
 DECLARE_GLOBAL_DATA_PTR;
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 4/4] x86: cpu: Add missing header inclusion

[Simon Glass]

On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Without asm/cpu_x86.h inclusion a compiler is not happy:
>
>   arch/x86/cpu/cpu_x86.c:14:5: warning: no previous prototype for ‘cpu_x86_bind’ [-Wmissing-prototypes]
>   arch/x86/cpu/cpu_x86.c:29:5: warning: no previous prototype for ‘cpu_x86_get_vendor’ [-Wmissing-prototypes]
>   arch/x86/cpu/cpu_x86.c:41:5: warning: no previous prototype for ‘cpu_x86_get_desc’ [-Wmissing-prototypes]
>   arch/x86/cpu/cpu_x86.c:55:5: warning: no previous prototype for ‘cpu_x86_get_count’ [-Wmissing-prototypes]
>
> Add missing header inclusion.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/cpu/cpu_x86.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Simon Glass <sjg@chromium.org>


>
> diff --git a/arch/x86/cpu/cpu_x86.c b/arch/x86/cpu/cpu_x86.c
> index 6c53f0ea821f..6c32ae499df3 100644
> --- a/arch/x86/cpu/cpu_x86.c
> +++ b/arch/x86/cpu/cpu_x86.c
> @@ -7,6 +7,7 @@
>  #include <dm.h>
>  #include <errno.h>
>  #include <asm/cpu.h>
> +#include <asm/cpu_x86.h>
>  #include <asm/global_data.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>

Re: [PATCH v1 3/4] x86: cpu: Add a few prototypes to the header file

[Simon Glass]

On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> The compiler is not happy to have no prototypes for the functions that
> are not defined static. Add them. This helps avoiding the compiler warnings:
>
>   arch/x86/cpu/cpu.c:197:13: warning: no previous prototype for ‘board_final_init’ [-Wmissing-prototypes]
>   arch/x86/cpu/cpu.c:205:13: warning: no previous prototype for ‘board_final_cleanup’ [-Wmissing-prototypes]
>   arch/x86/cpu/cpu.c:307:5: warning: no previous prototype for ‘reserve_arch’ [-Wmissing-prototypes]
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/include/asm/cpu.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

OK, but could you add proper comments for these?


> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index 073f80b07f10..d71bc1b80c05 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -298,4 +298,11 @@ u32 cpu_get_stepping(void);
>   */
>  int cpu_phys_address_size(void);
>
> +void board_final_init(void);
> +void board_final_cleanup(void);
> +
> +#ifndef CONFIG_EFI_STUB
> +int reserve_arch(void);
> +#endif
> +
>  #endif
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>

Re: [PATCH v1 1/4] x86: cpu: Use default print_cpuinfo() for all

[Simon Glass]

On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Most of the copies of the print_cpuinfo() call the default method.
> Remove all of those in order to have only the default one when
> no `cpu` command is compiled.
>
> This also helps avoiding compiler warning, e.g.:
>
>   arch/x86/cpu/tangier/tangier.c:23:5: warning: no previous prototype for ‘print_cpuinfo’ [-Wmissing-prototypes]
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/cpu/broadwell/cpu.c                 | 12 ------------
>  arch/x86/cpu/coreboot/coreboot.c             |  5 -----
>  arch/x86/cpu/cpu.c                           |  6 +++++-
>  arch/x86/cpu/efi/app.c                       |  5 -----
>  arch/x86/cpu/efi/payload.c                   |  5 -----
>  arch/x86/cpu/ivybridge/cpu.c                 | 14 --------------
>  arch/x86/cpu/qemu/qemu.c                     |  6 ------
>  arch/x86/cpu/quark/quark.c                   |  6 ------
>  arch/x86/cpu/slimbootloader/slimbootloader.c |  5 -----
>  arch/x86/cpu/tangier/tangier.c               |  5 -----
>  arch/x86/cpu/x86_64/misc.c                   |  5 -----
>  arch/x86/include/asm/u-boot-x86.h            |  1 -
>  arch/x86/lib/fsp/fsp_common.c                |  6 ------
>  13 files changed, 5 insertions(+), 76 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

BTW, this function is deprecated...since with CONFIG_CPU it is shown
by the CPU driver.

Regards,
Simon

Re: [PATCH v1 2/4] x86: cpu: Mark a few functions static

[Simon Glass]

On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Some functions are not used anywhere except the same file
> where they are defined. Mark them static. This helps avoiding
> the compiler warnings:
>
>   arch/x86/cpu/cpu.c:343:6: warning: no previous prototype for ‘detect_coreboot_table_at’ [-Wmissing-prototypes]
>   arch/x86/cpu/mtrr.c:90:6: warning: no previous prototype for ‘mtrr_write_all’ [-Wmissing-prototypes]
>   arch/x86/cpu/i386/interrupt.c:240:6: warning: no previous prototype for ‘__do_irq’ [-Wmissing-prototypes]
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/cpu/cpu.c            | 2 +-
>  arch/x86/cpu/i386/interrupt.c | 2 +-
>  arch/x86/cpu/mtrr.c           | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v1 1/4] x86: cpu: Use default print_cpuinfo() for all

[Andy Shevchenko]

On Tue, Oct 08, 2024 at 07:56:43PM -0600, Simon Glass wrote:
> On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Most of the copies of the print_cpuinfo() call the default method.
> > Remove all of those in order to have only the default one when
> > no `cpu` command is compiled.
> >
> > This also helps avoiding compiler warning, e.g.:
> >
> >   arch/x86/cpu/tangier/tangier.c:23:5: warning: no previous prototype for ‘print_cpuinfo’ [-Wmissing-prototypes]
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  arch/x86/cpu/broadwell/cpu.c                 | 12 ------------
> >  arch/x86/cpu/coreboot/coreboot.c             |  5 -----
> >  arch/x86/cpu/cpu.c                           |  6 +++++-
> >  arch/x86/cpu/efi/app.c                       |  5 -----
> >  arch/x86/cpu/efi/payload.c                   |  5 -----
> >  arch/x86/cpu/ivybridge/cpu.c                 | 14 --------------
> >  arch/x86/cpu/qemu/qemu.c                     |  6 ------
> >  arch/x86/cpu/quark/quark.c                   |  6 ------
> >  arch/x86/cpu/slimbootloader/slimbootloader.c |  5 -----
> >  arch/x86/cpu/tangier/tangier.c               |  5 -----
> >  arch/x86/cpu/x86_64/misc.c                   |  5 -----
> >  arch/x86/include/asm/u-boot-x86.h            |  1 -
> >  arch/x86/lib/fsp/fsp_common.c                |  6 ------
> >  13 files changed, 5 insertions(+), 76 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks!

> BTW, this function is deprecated...since with CONFIG_CPU it is shown
> by the CPU driver.

Is it docemented somewhere? In any case this patch I think is good on
its own as it drastically reduces the possible churn in the future.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 3/4] x86: cpu: Add a few prototypes to the header file

[Andy Shevchenko]

On Tue, Oct 08, 2024 at 07:55:33PM -0600, Simon Glass wrote:
> On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > The compiler is not happy to have no prototypes for the functions that
> > are not defined static. Add them. This helps avoiding the compiler warnings:
> >
> >   arch/x86/cpu/cpu.c:197:13: warning: no previous prototype for ‘board_final_init’ [-Wmissing-prototypes]
> >   arch/x86/cpu/cpu.c:205:13: warning: no previous prototype for ‘board_final_cleanup’ [-Wmissing-prototypes]
> >   arch/x86/cpu/cpu.c:307:5: warning: no previous prototype for ‘reserve_arch’ [-Wmissing-prototypes]
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks!

> OK, but could you add proper comments for these?

Any proposed texts for them? The comments were out of scope of my patch,
but I may add anything that is provided as a template.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/4] x86: cpu: Use default print_cpuinfo() for all

[Simon Glass]

Hi Andy,

On Wed, 9 Oct 2024 at 08:06, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Oct 08, 2024 at 07:56:43PM -0600, Simon Glass wrote:
> > On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > Most of the copies of the print_cpuinfo() call the default method.
> > > Remove all of those in order to have only the default one when
> > > no `cpu` command is compiled.
> > >
> > > This also helps avoiding compiler warning, e.g.:
> > >
> > >   arch/x86/cpu/tangier/tangier.c:23:5: warning: no previous prototype for ‘print_cpuinfo’ [-Wmissing-prototypes]
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  arch/x86/cpu/broadwell/cpu.c                 | 12 ------------
> > >  arch/x86/cpu/coreboot/coreboot.c             |  5 -----
> > >  arch/x86/cpu/cpu.c                           |  6 +++++-
> > >  arch/x86/cpu/efi/app.c                       |  5 -----
> > >  arch/x86/cpu/efi/payload.c                   |  5 -----
> > >  arch/x86/cpu/ivybridge/cpu.c                 | 14 --------------
> > >  arch/x86/cpu/qemu/qemu.c                     |  6 ------
> > >  arch/x86/cpu/quark/quark.c                   |  6 ------
> > >  arch/x86/cpu/slimbootloader/slimbootloader.c |  5 -----
> > >  arch/x86/cpu/tangier/tangier.c               |  5 -----
> > >  arch/x86/cpu/x86_64/misc.c                   |  5 -----
> > >  arch/x86/include/asm/u-boot-x86.h            |  1 -
> > >  arch/x86/lib/fsp/fsp_common.c                |  6 ------
> > >  13 files changed, 5 insertions(+), 76 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Thanks!
>
> > BTW, this function is deprecated...since with CONFIG_CPU it is shown
> > by the CPU driver.
>
> Is it docemented somewhere? In any case this patch I think is good on
> its own as it drastically reduces the possible churn in the future.

Documentation for uclasses is generally in the code. But there is
nothing much here. In this case the code is print_cpuinfo() in
board_f.c

Regards,
Simon

Re: [PATCH v1 3/4] x86: cpu: Add a few prototypes to the header file

[Simon Glass]

Hi Andy,

On Wed, 9 Oct 2024 at 08:08, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Oct 08, 2024 at 07:55:33PM -0600, Simon Glass wrote:
> > On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > The compiler is not happy to have no prototypes for the functions that
> > > are not defined static. Add them. This helps avoiding the compiler warnings:
> > >
> > >   arch/x86/cpu/cpu.c:197:13: warning: no previous prototype for ‘board_final_init’ [-Wmissing-prototypes]
> > >   arch/x86/cpu/cpu.c:205:13: warning: no previous prototype for ‘board_final_cleanup’ [-Wmissing-prototypes]
> > >   arch/x86/cpu/cpu.c:307:5: warning: no previous prototype for ‘reserve_arch’ [-Wmissing-prototypes]
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Thanks!
>
> > OK, but could you add proper comments for these?
>
> Any proposed texts for them? The comments were out of scope of my patch,
> but I may add anything that is provided as a template.

I found some comments in one of the files, so you could use these as a
base. Whenever you change code, you should make sure it has function
comments, follows style, etc. That way things slowly get better.

/*
 * Implement a weak default function for boards that need to do some final init
 * before the system is ready.
 */
__weak void board_final_init(void)
{
}

/*
 * Implement a weak default function for boards that need to do some final
 * processing before booting the OS.
 */
__weak void board_final_cleanup(void)
{
}

Regards,
Simon

Re: [PATCH v1 4/4] x86: cpu: Add missing header inclusion

[Simon Glass]

On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Without asm/cpu_x86.h inclusion a compiler is not happy:
>
>   arch/x86/cpu/cpu_x86.c:14:5: warning: no previous prototype for ‘cpu_x86_bind’ [-Wmissing-prototypes]
>   arch/x86/cpu/cpu_x86.c:29:5: warning: no previous prototype for ‘cpu_x86_get_vendor’ [-Wmissing-prototypes]
>   arch/x86/cpu/cpu_x86.c:41:5: warning: no previous prototype for ‘cpu_x86_get_desc’ [-Wmissing-prototypes]
>   arch/x86/cpu/cpu_x86.c:55:5: warning: no previous prototype for ‘cpu_x86_get_count’ [-Wmissing-prototypes]
>
> Add missing header inclusion.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/cpu/cpu_x86.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Simon Glass <sjg@chromium.org>


>
Applied to u-boot-dm, thanks!

Re: [PATCH v1 3/4] x86: cpu: Add a few prototypes to the header file

[Simon Glass]

Hi Andy,

On Wed, 9 Oct 2024 at 08:08, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Oct 08, 2024 at 07:55:33PM -0600, Simon Glass wrote:
> > On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > The compiler is not happy to have no prototypes for the functions that
> > > are not defined static. Add them. This helps avoiding the compiler warnings:
> > >
> > >   arch/x86/cpu/cpu.c:197:13: warning: no previous prototype for ‘board_final_init’ [-Wmissing-prototypes]
> > >   arch/x86/cpu/cpu.c:205:13: warning: no previous prototype for ‘board_final_cleanup’ [-Wmissing-prototypes]
> > >   arch/x86/cpu/cpu.c:307:5: warning: no previous prototype for ‘reserve_arch’ [-Wmissing-prototypes]
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Thanks!
>
> > OK, but could you add proper comments for these?
>
> Any proposed texts for them? The comments were out of scope of my patch,
> but I may add anything that is provided as a template.

I found some comments in one of the files, so you could use these as a
base. Whenever you change code, you should make sure it has function
comments, follows style, etc. That way things slowly get better.

/*
 * Implement a weak default function for boards that need to do some final init
 * before the system is ready.
 */
__weak void board_final_init(void)
{
}

/*
 * Implement a weak default function for boards that need to do some final
 * processing before booting the OS.
 */
__weak void board_final_cleanup(void)
{
}

Regards,
Simon

Applied to u-boot-dm, thanks!

Re: [PATCH v1 2/4] x86: cpu: Mark a few functions static

[Simon Glass]

On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Some functions are not used anywhere except the same file
> where they are defined. Mark them static. This helps avoiding
> the compiler warnings:
>
>   arch/x86/cpu/cpu.c:343:6: warning: no previous prototype for ‘detect_coreboot_table_at’ [-Wmissing-prototypes]
>   arch/x86/cpu/mtrr.c:90:6: warning: no previous prototype for ‘mtrr_write_all’ [-Wmissing-prototypes]
>   arch/x86/cpu/i386/interrupt.c:240:6: warning: no previous prototype for ‘__do_irq’ [-Wmissing-prototypes]
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/cpu/cpu.c            | 2 +-
>  arch/x86/cpu/i386/interrupt.c | 2 +-
>  arch/x86/cpu/mtrr.c           | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm, thanks!

Re: [PATCH v1 1/4] x86: cpu: Use default print_cpuinfo() for all

[Simon Glass]

Hi Andy,

On Wed, 9 Oct 2024 at 08:06, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Oct 08, 2024 at 07:56:43PM -0600, Simon Glass wrote:
> > On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > Most of the copies of the print_cpuinfo() call the default method.
> > > Remove all of those in order to have only the default one when
> > > no `cpu` command is compiled.
> > >
> > > This also helps avoiding compiler warning, e.g.:
> > >
> > >   arch/x86/cpu/tangier/tangier.c:23:5: warning: no previous prototype for ‘print_cpuinfo’ [-Wmissing-prototypes]
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  arch/x86/cpu/broadwell/cpu.c                 | 12 ------------
> > >  arch/x86/cpu/coreboot/coreboot.c             |  5 -----
> > >  arch/x86/cpu/cpu.c                           |  6 +++++-
> > >  arch/x86/cpu/efi/app.c                       |  5 -----
> > >  arch/x86/cpu/efi/payload.c                   |  5 -----
> > >  arch/x86/cpu/ivybridge/cpu.c                 | 14 --------------
> > >  arch/x86/cpu/qemu/qemu.c                     |  6 ------
> > >  arch/x86/cpu/quark/quark.c                   |  6 ------
> > >  arch/x86/cpu/slimbootloader/slimbootloader.c |  5 -----
> > >  arch/x86/cpu/tangier/tangier.c               |  5 -----
> > >  arch/x86/cpu/x86_64/misc.c                   |  5 -----
> > >  arch/x86/include/asm/u-boot-x86.h            |  1 -
> > >  arch/x86/lib/fsp/fsp_common.c                |  6 ------
> > >  13 files changed, 5 insertions(+), 76 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Thanks!
>
> > BTW, this function is deprecated...since with CONFIG_CPU it is shown
> > by the CPU driver.
>
> Is it docemented somewhere? In any case this patch I think is good on
> its own as it drastically reduces the possible churn in the future.

Documentation for uclasses is generally in the code. But there is
nothing much here. In this case the code is print_cpuinfo() in
board_f.c

Regards,
Simon

Applied to u-boot-dm, thanks!

Re: [PATCH v1 3/4] x86: cpu: Add a few prototypes to the header file

[Andy Shevchenko]

On Thu, Oct 17, 2024 at 04:11:48PM -0700, Simon Glass wrote:
> On Wed, 9 Oct 2024 at 08:08, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Oct 08, 2024 at 07:55:33PM -0600, Simon Glass wrote:
> > > On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > The compiler is not happy to have no prototypes for the functions that
> > > > are not defined static. Add them. This helps avoiding the compiler warnings:
> > > >
> > > >   arch/x86/cpu/cpu.c:197:13: warning: no previous prototype for ‘board_final_init’ [-Wmissing-prototypes]
> > > >   arch/x86/cpu/cpu.c:205:13: warning: no previous prototype for ‘board_final_cleanup’ [-Wmissing-prototypes]
> > > >   arch/x86/cpu/cpu.c:307:5: warning: no previous prototype for ‘reserve_arch’ [-Wmissing-prototypes]
> > >
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > Thanks!
> >
> > > OK, but could you add proper comments for these?
> >
> > Any proposed texts for them? The comments were out of scope of my patch,
> > but I may add anything that is provided as a template.
> 
> I found some comments in one of the files, so you could use these as a
> base. Whenever you change code, you should make sure it has function
> comments, follows style, etc. That way things slowly get better.
> 
> /*
>  * Implement a weak default function for boards that need to do some final init
>  * before the system is ready.
>  */
> __weak void board_final_init(void)
> {
> }
> 
> /*
>  * Implement a weak default function for boards that need to do some final
>  * processing before booting the OS.
>  */
> __weak void board_final_cleanup(void)
> {
> }
> 
> Applied to u-boot-dm, thanks!

Thanks, but what does it mean to me? Should I send an update to add the comments?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 3/4] x86: cpu: Add a few prototypes to the header file

[Simon Glass]

Hi Andy,

On Fri, 18 Oct 2024 at 05:43, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Oct 17, 2024 at 04:11:48PM -0700, Simon Glass wrote:
> > On Wed, 9 Oct 2024 at 08:08, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, Oct 08, 2024 at 07:55:33PM -0600, Simon Glass wrote:
> > > > On Sat, 5 Oct 2024 at 13:14, Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > >
> > > > > The compiler is not happy to have no prototypes for the functions that
> > > > > are not defined static. Add them. This helps avoiding the compiler warnings:
> > > > >
> > > > >   arch/x86/cpu/cpu.c:197:13: warning: no previous prototype for ‘board_final_init’ [-Wmissing-prototypes]
> > > > >   arch/x86/cpu/cpu.c:205:13: warning: no previous prototype for ‘board_final_cleanup’ [-Wmissing-prototypes]
> > > > >   arch/x86/cpu/cpu.c:307:5: warning: no previous prototype for ‘reserve_arch’ [-Wmissing-prototypes]
> > > >
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > >
> > > Thanks!
> > >
> > > > OK, but could you add proper comments for these?
> > >
> > > Any proposed texts for them? The comments were out of scope of my patch,
> > > but I may add anything that is provided as a template.
> >
> > I found some comments in one of the files, so you could use these as a
> > base. Whenever you change code, you should make sure it has function
> > comments, follows style, etc. That way things slowly get better.
> >
> > /*
> >  * Implement a weak default function for boards that need to do some final init
> >  * before the system is ready.
> >  */
> > __weak void board_final_init(void)
> > {
> > }
> >
> > /*
> >  * Implement a weak default function for boards that need to do some final
> >  * processing before booting the OS.
> >  */
> > __weak void board_final_cleanup(void)
> > {
> > }
> >
> > Applied to u-boot-dm, thanks!
>
> Thanks, but what does it mean to me? Should I send an update to add the comments?

Yes, please. I'm just tidying up some x86 patches while Bin is away.

Regards,
Simon

Re: [PATCH v1 3/4] x86: cpu: Add a few prototypes to the header file

[Andy Shevchenko]

On Fri, Oct 18, 2024 at 08:55:00AM -0600, Simon Glass wrote:
> On Fri, 18 Oct 2024 at 05:43, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Oct 17, 2024 at 04:11:48PM -0700, Simon Glass wrote:
> > > On Wed, 9 Oct 2024 at 08:08, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:

...

> > > /*
> > >  * Implement a weak default function for boards that need to do some final init
> > >  * before the system is ready.
> > >  */
> > > __weak void board_final_init(void)
> > > {
> > > }
> > >
> > > /*
> > >  * Implement a weak default function for boards that need to do some final
> > >  * processing before booting the OS.
> > >  */
> > > __weak void board_final_cleanup(void)
> > > {
> > > }
> > >
> > > Applied to u-boot-dm, thanks!
> >
> > Thanks, but what does it mean to me? Should I send an update to add the comments?
> 
> Yes, please. I'm just tidying up some x86 patches while Bin is away.

Patch has just been sent.

-- 
With Best Regards,
Andy Shevchenko