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

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);

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

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

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().
participants (2)
-
Hans de Goede
-
Stephen Warren