bootz: fix silent console

PATCH New Markus Niebel 2 participants 4 messages Nov 18, 2014 – Nov 25, 2014

lore

Share & tools

lore

Markus Niebel

author

2014-11-18 12:52 UTC #0 lore raw

PATCH 1/1 bootz: fix silent console

From: Markus Niebel <[email protected]>

fixup was lost during split between command code and logic.

Signed-off-by Markus Niebel <[email protected]>

---
 common/bootm.c     | 2 +-
 common/cmd_bootm.c | 6 ++++++
 include/bootm.h    | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

common/bootm.c +1 -1

index 6b3ea8c..94b9503 100644
@@ -467,7 +467,7 @@ ulong bootm_disable_interrupts(void)
 #define CONSOLE_ARG     "console="
 #define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1)
 
-static void fixup_silent_linux(void)
+void fixup_silent_linux(void)
 {
 	char *buf;
 	const char *env_val;

common/cmd_bootm.c +6 -0

index 6723360..d3e410a 100644
@@ -596,6 +596,12 @@ int do_bootz(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	 * disable interrupts ourselves
 	 */
 	bootm_disable_interrupts();
+#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
+	/*
+	 * same goes for fixup_silent_linux
+	 */
+	fixup_silent_linux();
+#endif
 
 	images.os.os = IH_OS_LINUX;
 	ret = do_bootm_states(cmdtp, flag, argc, argv,

include/bootm.h +2 -0

index b3d1a62..8e094b3 100644
@@ -50,6 +50,8 @@ ulong bootm_disable_interrupts(void);
 
 /* This is a special function used by booti/bootz */
 int bootm_find_ramdisk_fdt(int flag, int argc, char * const argv[]);
+/* This function is used also used by bootz */
+void fixup_silent_linux(void);
 
 int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		    int states, bootm_headers_t *images, int boot_progress);

Signature · 1 line

2.1.1

Simon Glass

2014-11-20 19:15 UTC #1 lore raw

PATCH 1/1 bootz: fix silent console

Hi Markus,

Quoted 46 lines · Markus Niebel

From: Markus Niebel <[email protected]>

fixup was lost during split between command code and logic.

Signed-off-by Markus Niebel <[email protected]>

---
 common/bootm.c     | 2 +-
 common/cmd_bootm.c | 6 ++++++
 include/bootm.h    | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

common/bootm.c +1 -1

index 6b3ea8c..94b9503 100644
@@ -467,7 +467,7 @@ ulong bootm_disable_interrupts(void)
 #define CONSOLE_ARG     "console="
 #define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1)
 
-static void fixup_silent_linux(void)
+void fixup_silent_linux(void)
 {
        char *buf;
        const char *env_val;

common/cmd_bootm.c +6 -0

index 6723360..d3e410a 100644
@@ -596,6 +596,12 @@ int do_bootz(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
         * disable interrupts ourselves
         */
        bootm_disable_interrupts();
+#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
+       /*
+        * same goes for fixup_silent_linux
+        */
+       fixup_silent_linux();
+#endif
 
        images.os.os = IH_OS_LINUX;
        ret = do_bootm_states(cmdtp, flag, argc, argv,

include/bootm.h +0 -0

index b3d1a62..8e094b3 100644

include/bootm.h ulong bootm_disable_interrupts(void); · line 57

Quoted 8 lines

include/bootm.h +2 -0

@@ -50,6 +50,8 @@ ulong bootm_disable_interrupts(void);
 
 /* This is a special function used by booti/bootz */
 int bootm_find_ramdisk_fdt(int flag, int argc, char * const argv[]);
+/* This function is used also used by bootz */
+void fixup_silent_linux(void);
 
 int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
                    int states, bootm_headers_t *images, int boot_progress);

Quite a bit of effort was expended trying to join this code back together rather than having two separate code paths for bootm and bootz.

Since this is cmdline-related, I suggest something like this:

- Enable BOOTM_STATE_OS_CMDLINE for all archs in do_bootm()
- Call fixup_silent_linux() in do_bootm_states() when processing BOOTM_STATE_OS_CMDLINE
- Add BOOTM_STATE_OS_CMDLINE to the do_bootm_states() call in do_bootz()

Or similar...that way we keep bootm and bootz in sync. The separate call to bootm_disable_interrupts() is unfortunate but is a problem for another day.

Regards,
Simon

Markus Niebel

author

2014-11-25 08:31 UTC #2 lore raw

PATCH 1/1 bootz: fix silent console

Hello Simon,

Am 20.11.2014 um 20:15 schrieb Simon Glass:

Quoted 55 lines

Hi Markus,

Quoted 52 lines · Markus Niebel

From: Markus Niebel <[email protected]>

fixup was lost during split between command code and logic.

Signed-off-by Markus Niebel <[email protected]>

---
 common/bootm.c     | 2 +-
 common/cmd_bootm.c | 6 ++++++
 include/bootm.h    | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

common/bootm.c +1 -1

index 6b3ea8c..94b9503 100644
@@ -467,7 +467,7 @@ ulong bootm_disable_interrupts(void)
 #define CONSOLE_ARG     "console="
 #define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1)
 
-static void fixup_silent_linux(void)
+void fixup_silent_linux(void)
 {
        char *buf;
        const char *env_val;

common/cmd_bootm.c +6 -0

index 6723360..d3e410a 100644
@@ -596,6 +596,12 @@ int do_bootz(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
         * disable interrupts ourselves
         */
        bootm_disable_interrupts();
+#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
+       /*
+        * same goes for fixup_silent_linux
+        */
+       fixup_silent_linux();
+#endif
 
        images.os.os = IH_OS_LINUX;
        ret = do_bootm_states(cmdtp, flag, argc, argv,

include/bootm.h +2 -0

index b3d1a62..8e094b3 100644
@@ -50,6 +50,8 @@ ulong bootm_disable_interrupts(void);
 
 /* This is a special function used by booti/bootz */
 int bootm_find_ramdisk_fdt(int flag, int argc, char * const argv[]);
+/* This function is used also used by bootz */
+void fixup_silent_linux(void);

Quoted 6 lines

include/bootm.h ulong bootm_disable_interrupts(void); · line 57
Quoted 2 lines
include/bootm.h +0 -0
 int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
                    int states, bootm_headers_t *images, int boot_progress);
Quite a bit of effort was expended trying to join this code back together rather than having two separate code paths for bootm and bootz.

I Understand

Quoted 8 lines

Since this is cmdline-related, I suggest something like this:
- Enable BOOTM_STATE_OS_CMDLINE for all archs in do_bootm()
- Call fixup_silent_linux() in do_bootm_states() when processing BOOTM_STATE_OS_CMDLINE
- Add BOOTM_STATE_OS_CMDLINE to the do_bootm_states() call in do_bootz()

I read through the code to find out, what the implications of your suggestions were. Since I'm not very familiar with this piece of code, please correct me, If i'm wrong:

- rename the arch specific do_bootm_linux to arch_bootm_linux
- implement a generic version of do_bootm_linux inside bootm_os.c
- call arch_bootm_linux from generic do_bootm_linux
- move the call to fixup_silent_linux to new do_bootm_linux for the BOOTM_STATE_OS_CMDLINE case
- mask BOOTM_STATE_OS_CMDLINE before calling arch_bootm_linux for all but MIPS and PPC

because this seems a bit intrusive I'm not sure. But if this is the wy to go, I will prepare a patch.

Quoted 7 lines

Or similar...that way we keep bootm and bootz in sync. The separate call to bootm_disable_interrupts() is unfortunate but is a problem for another day.
Regards,
Simon

Regards,
Markus

Simon Glass

2014-11-25 14:03 UTC #3 lore raw

PATCH 1/1 bootz: fix silent console

Hi Markus,

Quoted 88 lines · Markus Niebel

Hello Simon,
Am 20.11.2014 um 20:15 schrieb Simon Glass:
Quoted 55 lines
Hi Markus,
Quoted 52 lines · Markus Niebel
From: Markus Niebel <[email protected]>
fixup was lost during split between command code and logic.
Signed-off-by Markus Niebel <[email protected]>
---
 common/bootm.c     | 2 +-
 common/cmd_bootm.c | 6 ++++++
 include/bootm.h    | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)
common/bootm.c +1 -1
index 6b3ea8c..94b9503 100644
@@ -467,7 +467,7 @@ ulong bootm_disable_interrupts(void)
 #define CONSOLE_ARG     "console="
 #define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1)
 
-static void fixup_silent_linux(void)
+void fixup_silent_linux(void)
 {
        char *buf;
        const char *env_val;
common/cmd_bootm.c +6 -0
index 6723360..d3e410a 100644
@@ -596,6 +596,12 @@ int do_bootz(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
         * disable interrupts ourselves
         */
        bootm_disable_interrupts();
+#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
+       /*
+        * same goes for fixup_silent_linux
+        */
+       fixup_silent_linux();
+#endif
 
        images.os.os = IH_OS_LINUX;
        ret = do_bootm_states(cmdtp, flag, argc, argv,
include/bootm.h +2 -0
index b3d1a62..8e094b3 100644
@@ -50,6 +50,8 @@ ulong bootm_disable_interrupts(void);
 
 /* This is a special function used by booti/bootz */
 int bootm_find_ramdisk_fdt(int flag, int argc, char * const argv[]);
+/* This function is used also used by bootz */
+void fixup_silent_linux(void);
 
Quoted 6 lines
include/bootm.h ulong bootm_disable_interrupts(void); · line 57
Quoted 2 lines
include/bootm.h +0 -0
 int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
                    int states, bootm_headers_t *images, int boot_progress);
Quite a bit of effort was expended trying to join this code back together rather than having two separate code paths for bootm and bootz.
I Understand
Quoted 8 lines
Since this is cmdline-related, I suggest something like this:
- Enable BOOTM_STATE_OS_CMDLINE for all archs in do_bootm()
- Call fixup_silent_linux() in do_bootm_states() when processing BOOTM_STATE_OS_CMDLINE
- Add BOOTM_STATE_OS_CMDLINE to the do_bootm_states() call in do_bootz()
I read through the code to find out, what the implications of your suggestions were. Since I'm not very familiar with this piece of code, please correct me, If i'm wrong:
- rename the arch specific do_bootm_linux to arch_bootm_linux
- implement a generic version of do_bootm_linux inside bootm_os.c
- call arch_bootm_linux from generic do_bootm_linux
- move the call to fixup_silent_linux to new do_bootm_linux for the BOOTM_STATE_OS_CMDLINE case
- mask BOOTM_STATE_OS_CMDLINE before calling arch_bootm_linux for all but MIPS and PPC

Quoted 2 lines

because this seems a bit intrusive I'm not sure. But if this is the wy to go, I will prepare a patch.

Yes that sounds complicated. But copying code in multiple places is how the image/bootm support got so ugly, and it was a big effort to fix it up. So I'd really like to avoid copying code.

But it should be a lot easier than that.

In do_boot() there is:

return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER | BOOTM_STATE_LOADOS |
#if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
BOOTM_STATE_OS_CMDLINE |
#endif
BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO, &images, 1);
}

You should be able to remove the #ifdef.

Then in do_bootm_states() there is:

/* Load the OS */
if (!ret && (states & BOOTM_STATE_LOADOS)) {
...
#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
if (images->os.os == IH_OS_LINUX)

      fixup_silent_linux();

#endif
}

And later:

if (!ret && (states & BOOTM_STATE_OS_CMDLINE))
ret = boot_fn(BOOTM_STATE_OS_CMDLINE, argc, argv, images);

You should be able to move the '#if defined' code to inside this if ()
- so that fixup_silent_linux() happens in the CMDLINE stage instead of the LOADOS stage.

Then in do_bootz(), add BOOTM_STATE_OS_CMDLINE to the call to do_bootm_states().

Then bootm and bootz will still go along exactly the same code path.

The tricky bit is that of course this does affect each arch - you need to make sure that each do_bootm_linux() function in arch/xxx/lib/bootm.c will handle BOOTM_STATE_OS_CMDLINE (and return 0, not -1). At a glance I can see that ARM returns -1 - it will need to return 0.

Also each function in boot_os[] needs to do the same. I looked at most of them and they check for BOOTM_STATE_OS_GO and return 0 for anything else, so should be OK. Maybe do_bootm_standalone() will need to do so too.

Quoted 10 lines

Quoted 8 lines
Or similar...that way we keep bootm and bootz in sync. The separate call to bootm_disable_interrupts() is unfortunate but is a problem for another day.
Regards,
Simon
Regards,
Markus

Regards,
Simon