[U-Boot] [PATCH] pxe: automatically add console= to bootargs when not specified in append

[U-Boot] [PATCH] pxe: automatically add console= to bootargs when not specified in append

[Hans de Goede]

From: Dennis Gilmore <dennis@ausil.us>

if there is a console variable in the u-boot environment and not one on
the append line from syslinux config add what is in the environment to
the bootargs.

This is necessary to allow distros to have a single extlinux/extlinux.conf
file which will work on multiple boards, even if these boards have different
consoles (e.g. ttyS0 vs ttyAMA0).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 common/cmd_pxe.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
index ba48692..147d7d1 100644
--- a/common/cmd_pxe.c
+++ b/common/cmd_pxe.c
@@ -606,6 +606,7 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
 	char initrd_str[22];
 	char mac_str[29] = "";
 	char ip_str[68] = "";
+	char console[30] = "";
 	char *bootargs;
 	int bootm_argc = 3;
 	int len = 0;
@@ -665,8 +666,14 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
 	}
 #endif
 
-	if (label->append)
+	if (label->append) {
 		len += strlen(label->append);
+		/* If no console in append and $console is set, use it */
+		if (!strstr(label->append, "console=") && getenv("console")) {
+			sprintf(console, " console=%s", getenv("console"));
+			len += strlen(console);
+		}
+	}
 
 	if (len) {
 		bootargs = malloc(len + 1);
@@ -675,6 +682,7 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
 		bootargs[0] = '\0';
 		if (label->append)
 			strcpy(bootargs, label->append);
+		strcat(bootargs, console);
 		strcat(bootargs, ip_str);
 		strcat(bootargs, mac_str);
 
-- 
2.0.3

[U-Boot] [PATCH] pxe: automatically add console= to bootargs when not specified in append

[Hans de Goede]

Hi,

On 08/01/2014 09:46 AM, Hans de Goede wrote:
> From: Dennis Gilmore <dennis@ausil.us>
> 
> if there is a console variable in the u-boot environment and not one on
> the append line from syslinux config add what is in the environment to
> the bootargs.
> 
> This is necessary to allow distros to have a single extlinux/extlinux.conf
> file which will work on multiple boards, even if these boards have different
> consoles (e.g. ttyS0 vs ttyAMA0).
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

I asked Dennis for its Signed-off-by for this, and instead I got this:

> NAK, I was planning to drop the patch entirely, while its useful in
> some cases, it breaks things like plymouth working, as well it will
> force anaconda installs to always be text over the serial port.
> wandboard and cubox-i for instance boot happily without a console= line
> and you get plymouth on the screen when video is initialised. having it
> add the console line if u-boot has the console set to serial would be
> okay. I think it needs some more thought and careful planning.
>
> Dennis

So lets drop this one for now.

Regards,

Hans

[U-Boot] [PATCH] pxe: automatically add console= to bootargs when not specified in append

[Hans de Goede]

Hi,

On 08/01/2014 10:20 AM, Hans de Goede wrote:
> Hi,
> 
> On 08/01/2014 09:46 AM, Hans de Goede wrote:
>> From: Dennis Gilmore <dennis@ausil.us>
>>
>> if there is a console variable in the u-boot environment and not one on
>> the append line from syslinux config add what is in the environment to
>> the bootargs.
>>
>> This is necessary to allow distros to have a single extlinux/extlinux.conf
>> file which will work on multiple boards, even if these boards have different
>> consoles (e.g. ttyS0 vs ttyAMA0).
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> I asked Dennis for its Signed-off-by for this, and instead I got this:

s/its/his/ sorry.

>> NAK, I was planning to drop the patch entirely, while its useful in
>> some cases, it breaks things like plymouth working, as well it will
>> force anaconda installs to always be text over the serial port.
>> wandboard and cubox-i for instance boot happily without a console= line
>> and you get plymouth on the screen when video is initialised. having it
>> add the console line if u-boot has the console set to serial would be
>> okay. I think it needs some more thought and careful planning.
>>
>> Dennis
> 
> So lets drop this one for now.

So if we're not going to do this one we're going to need something else,
because without this sunxi is useless, as the last message the
user sees is "Starting kernel ..." and then nothing.

Sow how about allowing the use of u-boot environment variables in append,
so that extlinux.conf can have:

label Fedora-Minimal-armhfp-rawhide-20140731 (3.16.0-0.rc7.git1.1.fc22.armv7hl)
        kernel /vmlinuz-3.16.0-0.rc7.git1.1.fc22.armv7hl
        append ro root=UUID=a6446dcb-320d-421f-9b71-725b68e5a41b console=$console
        fdtdir /dtb-3.16.0-0.rc7.git1.1.fc22.armv7hl/
        initrd /initramfs-3.16.0-0.rc7.git1.1.fc22.armv7hl.img

And then put 2 entries in extlinux.conf one for booting with the console on
the framebuffer, and one for using the serial console ?  Even with systems
where we have video the user may not have the video hooked up and use the
serial console for initial setup.

Alternatively we could use something like this:

console=tty0 console=ttyS0,115200 that will give us boot messages on both
video out and ttyS0, dunno what firstboot will do in this case.

Anyways I think we need to separate this discussion in 2 things:

1) Mechanism: u-boot should offer a way for extlinux.conf to find out the right
device + baudrate for the serial console. Traditionally this has been done
through the console environment variable. If we teach u-boot's extlinux support
to interpret $foo variables in the append string, then that mechanism can be
used for extlinux.conf booting too, and I think this is the best way to do
this.

2) Policy: Once we have 1). the question becomes how to use it in generic ARM
distros like Fedora, that discussion is probably best held on the Fedora ARM
list and not here.

Regards,

Hans

[U-Boot] [PATCH] pxe: automatically add console= to bootargs when not specified in append

[Stephen Warren]

On 08/01/2014 01:46 AM, Hans de Goede wrote:
> From: Dennis Gilmore <dennis@ausil.us>
>
> if there is a console variable in the u-boot environment and not one on
> the append line from syslinux config add what is in the environment to
> the bootargs.
>
> This is necessary to allow distros to have a single extlinux/extlinux.conf
> file which will work on multiple boards, even if these boards have different
> consoles (e.g. ttyS0 vs ttyAMA0).

> diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c

I know you rejected this afterwards, but since I read the patch before 
the reply, a couple of comments for anyone who finds this code in the 
archived and wants to copy it:

> +	char console[30] = "";

> -	if (label->append)
> +	if (label->append) {
>   		len += strlen(label->append);
> +		/* If no console in append and $console is set, use it */
> +		if (!strstr(label->append, "console=") && getenv("console")) {

This would get a false positive match for the text fooconsole= in the 
existing command-line. It might be better to strtok() (or similar) the 
existing value, so it can guarantee to match only options that start 
with console= not that contain console=.

> +			sprintf(console, " console=%s", getenv("console"));

This should check for buffer overflows, e.g. use snprintf().