diff mbox

[U-Boot] generic-board: show model name in board_init_f() too

Message ID 1417107792-15953-1-git-send-email-yamada.m@jp.panasonic.com
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Masahiro Yamada Nov. 27, 2014, 5:03 p.m. UTC 
The common/board_r.c has show_model_r() to display the model name
if the DTB has a "model" property.  It sounds useful to have a similar
function in common/board_f.c too because most of the boards show
their board name before relocation.

Instead of implementing the same function in both common/board_f.c
and common/board_r.c, let's split it up into common/show_board_info.c.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

 common/Makefile     |  2 ++
 common/board_f.c    |  2 +-
 common/board_info.c | 34 ++++++++++++++++++++++++++++++++++
 common/board_r.c    | 18 +-----------------
 include/common.h    | 13 +++++++------
 5 files changed, 45 insertions(+), 24 deletions(-)
 create mode 100644 common/board_info.c

Comments

Simon Glass Nov. 27, 2014, 6:14 p.m. UTC | #1 
Hi Masahiro,

On 27 November 2014 at 10:03, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> The common/board_r.c has show_model_r() to display the model name
> if the DTB has a "model" property.  It sounds useful to have a similar
> function in common/board_f.c too because most of the boards show
> their board name before relocation.
>
> Instead of implementing the same function in both common/board_f.c
> and common/board_r.c, let's split it up into common/show_board_info.c.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> ---
>
>  common/Makefile     |  2 ++
>  common/board_f.c    |  2 +-
>  common/board_info.c | 34 ++++++++++++++++++++++++++++++++++
>  common/board_r.c    | 18 +-----------------
>  include/common.h    | 13 +++++++------
>  5 files changed, 45 insertions(+), 24 deletions(-)
>  create mode 100644 common/board_info.c

This is a change of behaviour in show_model_r() but I think it is OK.

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

I'd also suggest adding a comment to checkboard() in the header file -
it is only allowed to print the board name, and cannot do any init,
etc. That way we can retain the flexibility to drop it one day.

Regards,
Simon
Simon Glass Jan. 13, 2015, 2:45 p.m. UTC | #2 
Hi Masahiro,

On 27 November 2014 at 10:14, Simon Glass <sjg@chromium.org> wrote:
> Hi Masahiro,
>
> On 27 November 2014 at 10:03, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
>> The common/board_r.c has show_model_r() to display the model name
>> if the DTB has a "model" property.  It sounds useful to have a similar
>> function in common/board_f.c too because most of the boards show
>> their board name before relocation.
>>
>> Instead of implementing the same function in both common/board_f.c
>> and common/board_r.c, let's split it up into common/show_board_info.c.
>>
>> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
>> ---
>>
>>  common/Makefile     |  2 ++
>>  common/board_f.c    |  2 +-
>>  common/board_info.c | 34 ++++++++++++++++++++++++++++++++++
>>  common/board_r.c    | 18 +-----------------
>>  include/common.h    | 13 +++++++------
>>  5 files changed, 45 insertions(+), 24 deletions(-)
>>  create mode 100644 common/board_info.c
>
> This is a change of behaviour in show_model_r() but I think it is OK.
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
> I'd also suggest adding a comment to checkboard() in the header file -
> it is only allowed to print the board name, and cannot do any init,
> etc. That way we can retain the flexibility to drop it one day.

This breaks these boards:

02: generic-board: show model name in board_init_f() too
       arm: +   apalis_t30 venice2 ventana whistler s5pc210_universal
smdkc100 smdkv310 s5p_goni seaboard trimslice paz00 origen beaver
colibri_t20_iris dalmore colibri_t30 trats jetson-tk1 harmony cardhu
trats2

e.g.:

01: Prepare v2015.01
02: generic-board: show model name in board_init_f() too
       arm: +   whistler
w+../common/board_info.c: In function ‘show_board_info’:
w+../common/board_info.c:25:2: warning: implicit declaration of
function ‘fdt_getprop’ [-Wimplicit-function-declaration]
w+../common/board_info.c:25:8: warning: assignment makes pointer from
integer without a cast [enabled by default]

Regards,
Simon
Masahiro Yamada Jan. 15, 2015, 2:17 a.m. UTC | #3 
Hi Simon,



On Tue, 13 Jan 2015 06:45:07 -0800
Simon Glass <sjg@chromium.org> wrote:

> Hi Masahiro,
> 
> On 27 November 2014 at 10:14, Simon Glass <sjg@chromium.org> wrote:
> > Hi Masahiro,
> >
> > On 27 November 2014 at 10:03, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> >> The common/board_r.c has show_model_r() to display the model name
> >> if the DTB has a "model" property.  It sounds useful to have a similar
> >> function in common/board_f.c too because most of the boards show
> >> their board name before relocation.
> >>
> >> Instead of implementing the same function in both common/board_f.c
> >> and common/board_r.c, let's split it up into common/show_board_info.c.
> >>
> >> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> >> ---
> >>
> >>  common/Makefile     |  2 ++
> >>  common/board_f.c    |  2 +-
> >>  common/board_info.c | 34 ++++++++++++++++++++++++++++++++++
> >>  common/board_r.c    | 18 +-----------------
> >>  include/common.h    | 13 +++++++------
> >>  5 files changed, 45 insertions(+), 24 deletions(-)
> >>  create mode 100644 common/board_info.c
> >
> > This is a change of behaviour in show_model_r() but I think it is OK.
> >
> > Acked-by: Simon Glass <sjg@chromium.org>
> >
> > I'd also suggest adding a comment to checkboard() in the header file -
> > it is only allowed to print the board name, and cannot do any init,
> > etc. That way we can retain the flexibility to drop it one day.
> 
> This breaks these boards:
> 
> 02: generic-board: show model name in board_init_f() too
>        arm: +   apalis_t30 venice2 ventana whistler s5pc210_universal
> smdkc100 smdkv310 s5p_goni seaboard trimslice paz00 origen beaver
> colibri_t20_iris dalmore colibri_t30 trats jetson-tk1 harmony cardhu
> trats2
> 
> e.g.:
> 
> 01: Prepare v2015.01
> 02: generic-board: show model name in board_init_f() too
>        arm: +   whistler
> w+../common/board_info.c: In function ‘show_board_info’:
> w+../common/board_info.c:25:2: warning: implicit declaration of
> function ‘fdt_getprop’ [-Wimplicit-function-declaration]
> w+../common/board_info.c:25:8: warning: assignment makes pointer from
> integer without a cast [enabled by default]
> 

Oops, I have sent v2.

Thanks,

Masahiro Yamada
diff mbox

Patch

diff --git a/common/Makefile b/common/Makefile
index 9c47e20..8de1ac7 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -37,6 +37,8 @@  endif
 # boards
 obj-$(CONFIG_SYS_GENERIC_BOARD) += board_f.o
 obj-$(CONFIG_SYS_GENERIC_BOARD) += board_r.o
+obj-$(CONFIG_DISPLAY_BOARDINFO) += board_info.o
+obj-$(CONFIG_DISPLAY_BOARDINFO_LATE) += board_info.o
 
 # core command
 obj-y += cmd_boot.o
diff --git a/common/board_f.c b/common/board_f.c
index 98c9c72..48acc7d 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -886,7 +886,7 @@  static init_fnc_t init_sequence_f[] = {
 	prt_mpc5xxx_clks,
 #endif /* CONFIG_MPC5xxx */
 #if defined(CONFIG_DISPLAY_BOARDINFO)
-	checkboard,		/* display board info */
+	show_board_info,
 #endif
 	INIT_FUNC_WATCHDOG_INIT
 #if defined(CONFIG_MISC_INIT_F)
diff --git a/common/board_info.c b/common/board_info.c
new file mode 100644
index 0000000..e0d5cef
--- /dev/null
+++ b/common/board_info.c
@@ -0,0 +1,34 @@ 
+/*
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <linux/compiler.h>
+
+int __weak checkboard(void)
+{
+	printf("Board: Unknown\n");
+	return 0;
+}
+
+/*
+ * If the root node of the DTB has a "model" property, show it.
+ * If CONFIG_OF_CONTROL is disabled or the "model" property is missing,
+ * fall back to checkboard().
+ */
+int show_board_info(void)
+{
+#ifdef CONFIG_OF_CONTROL
+	DECLARE_GLOBAL_DATA_PTR;
+	const char *model;
+
+	model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
+
+	if (model) {
+		printf("Model: %s\n", model);
+		return 0;
+	}
+#endif
+
+	return checkboard();
+}
diff --git a/common/board_r.c b/common/board_r.c
index 19c6427..61fbbc8 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -498,22 +498,6 @@  static int initr_api(void)
 }
 #endif
 
-#ifdef CONFIG_DISPLAY_BOARDINFO_LATE
-static int show_model_r(void)
-{
-	/* Put this here so it appears on the LCD, now it is ready */
-# ifdef CONFIG_OF_CONTROL
-	const char *model;
-
-	model = (char *)fdt_getprop(gd->fdt_blob, 0, "model", NULL);
-	printf("Model: %s\n", model ? model : "<unknown>");
-# else
-	checkboard();
-# endif
-	return 0;
-}
-#endif
-
 /* enable exceptions */
 #ifdef CONFIG_ARM
 static int initr_enable_interrupts(void)
@@ -823,7 +807,7 @@  init_fnc_t init_sequence_r[] = {
 #endif
 	console_init_r,		/* fully init console as a device */
 #ifdef CONFIG_DISPLAY_BOARDINFO_LATE
-	show_model_r,
+	show_board_info,
 #endif
 #ifdef CONFIG_ARCH_MISC_INIT
 	arch_misc_init,		/* miscellaneous arch-dependent init */
diff --git a/include/common.h b/include/common.h
index f1ab2cf..c82336f 100644
--- a/include/common.h
+++ b/include/common.h
@@ -231,12 +231,13 @@  int run_command_list(const char *cmd, int len, int flag);
 extern char console_buffer[];
 
 /* arch/$(ARCH)/lib/board.c */
-void	board_init_f(ulong);
-void	board_init_r  (gd_t *, ulong) __attribute__ ((noreturn));
-int	checkboard    (void);
-int	checkflash    (void);
-int	checkdram     (void);
-int	last_stage_init(void);
+void board_init_f(ulong);
+void board_init_r(gd_t *, ulong) __attribute__ ((noreturn));
+int checkboard(void);
+int show_board_info(void);
+int checkflash(void);
+int checkdram(void);
+int last_stage_init(void);
 extern ulong monitor_flash_len;
 int mac_read_from_eeprom(void);
 extern u8 __dtb_dt_begin[];	/* embedded device tree blob */