[2/2] default-distrovars: Have KERNEL_CONSOLE reference SERIAL_CONSOLES

Currently, KERNEL_CONSOLE has a default value of "ttyS0".  However, Arm
machines and those using virtio serial prefer to use "ttyAMA0" or "hvc0"
(or something else).  These are usually defined by the machine config
file as SERIAL_CONSOLES, which has one or more entries.  Take the first
one of those instead of ttyS0, but default back to ttyS0 if nothing is
set.

Also, use this variable in the efi wic file instead of "ttyS0".

Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
 meta/conf/distro/include/default-distrovars.inc | 3 ++-
 scripts/lib/wic/canned-wks/mkefidisk.wks        | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

On Wed, 2024-08-14 at 15:59 -0400, Jon Mason via lists.openembedded.org wrote:
> Currently, KERNEL_CONSOLE has a default value of "ttyS0".  However, Arm
> machines and those using virtio serial prefer to use "ttyAMA0" or "hvc0"
> (or something else).  These are usually defined by the machine config
> file as SERIAL_CONSOLES, which has one or more entries.  Take the first
> one of those instead of ttyS0, but default back to ttyS0 if nothing is
> set.
> 
> Also, use this variable in the efi wic file instead of "ttyS0".
> 
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
> ---
>  meta/conf/distro/include/default-distrovars.inc | 3 ++-
>  scripts/lib/wic/canned-wks/mkefidisk.wks        | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
> index 7554081e8b1a..26fa26f80c31 100644
> --- a/meta/conf/distro/include/default-distrovars.inc
> +++ b/meta/conf/distro/include/default-distrovars.inc
> @@ -1,7 +1,8 @@
>  QA_LOGFILE = "${TMPDIR}/qa.log"
>  
>  OEINCLUDELOGS ?= "yes"
> -KERNEL_CONSOLE ?= "ttyS0"
> +# if SERIAL_CONSOLES is set, take the first device entry.  Otherwise use ttyS0 as the default
> +KERNEL_CONSOLE ?= "${@d.getVar('SERIAL_CONSOLES').split(' ')[0].split(';')[1] or 'ttyS0'}"
>  KEEPUIMAGE ??= "yes"
>  
>  DEFAULT_IMAGE_LINGUAS = "en-us en-gb"
> diff --git a/scripts/lib/wic/canned-wks/mkefidisk.wks b/scripts/lib/wic/canned-wks/mkefidisk.wks
> index 9f534fe18471..1ab870805ed1 100644
> --- a/scripts/lib/wic/canned-wks/mkefidisk.wks
> +++ b/scripts/lib/wic/canned-wks/mkefidisk.wks
> @@ -8,4 +8,4 @@ part / --source rootfs --ondisk sda --fstype=ext4 --label platform --align 1024
>  
>  part swap --ondisk sda --size 44 --label swap1 --fstype=swap
>  
> -bootloader --ptable gpt --timeout=5 --append="rootfstype=ext4 console=ttyS0,115200 console=tty0"
> +bootloader --ptable gpt --timeout=5 --append="rootfstype=ext4 console=${KERNEL_CONSOLE}"

This doesn't look the same and at least needs some explanation. 

a) Where did console=tty0 go?

b) KERNEL_CONSOLE doesn't have the speed in it (115200). Is that needed?

These changes might be intended but if so, the commit message should mention this.

Cheers,

Richard

On Thu, Aug 15, 2024 at 9:53 AM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Wed, 2024-08-14 at 15:59 -0400, Jon Mason via lists.openembedded.org wrote:
> > Currently, KERNEL_CONSOLE has a default value of "ttyS0".  However, Arm
> > machines and those using virtio serial prefer to use "ttyAMA0" or "hvc0"
> > (or something else).  These are usually defined by the machine config
> > file as SERIAL_CONSOLES, which has one or more entries.  Take the first
> > one of those instead of ttyS0, but default back to ttyS0 if nothing is
> > set.
> >
> > Also, use this variable in the efi wic file instead of "ttyS0".
> >
> > Signed-off-by: Jon Mason <jdmason@kudzu.us>
> > ---
> >  meta/conf/distro/include/default-distrovars.inc | 3 ++-
> >  scripts/lib/wic/canned-wks/mkefidisk.wks        | 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
> > index 7554081e8b1a..26fa26f80c31 100644
> > --- a/meta/conf/distro/include/default-distrovars.inc
> > +++ b/meta/conf/distro/include/default-distrovars.inc
> > @@ -1,7 +1,8 @@
> >  QA_LOGFILE = "${TMPDIR}/qa.log"
> >
> >  OEINCLUDELOGS ?= "yes"
> > -KERNEL_CONSOLE ?= "ttyS0"
> > +# if SERIAL_CONSOLES is set, take the first device entry.  Otherwise use ttyS0 as the default
> > +KERNEL_CONSOLE ?= "${@d.getVar('SERIAL_CONSOLES').split(' ')[0].split(';')[1] or 'ttyS0'}"
> >  KEEPUIMAGE ??= "yes"
> >
> >  DEFAULT_IMAGE_LINGUAS = "en-us en-gb"
> > diff --git a/scripts/lib/wic/canned-wks/mkefidisk.wks b/scripts/lib/wic/canned-wks/mkefidisk.wks
> > index 9f534fe18471..1ab870805ed1 100644
> > --- a/scripts/lib/wic/canned-wks/mkefidisk.wks
> > +++ b/scripts/lib/wic/canned-wks/mkefidisk.wks
> > @@ -8,4 +8,4 @@ part / --source rootfs --ondisk sda --fstype=ext4 --label platform --align 1024
> >
> >  part swap --ondisk sda --size 44 --label swap1 --fstype=swap
> >
> > -bootloader --ptable gpt --timeout=5 --append="rootfstype=ext4 console=ttyS0,115200 console=tty0"
> > +bootloader --ptable gpt --timeout=5 --append="rootfstype=ext4 console=${KERNEL_CONSOLE}"
>
> This doesn't look the same and at least needs some explanation.
>
> a) Where did console=tty0 go?

Oversight, adding it back

> b) KERNEL_CONSOLE doesn't have the speed in it (115200). Is that needed?

My logic was that this might make the line difficult to parse and that
these should probably be overridden (and currently my testing is only
on virtual machines), but I do think making this a more sane default
at the expense of being a little ugly is probably the right call.
I'll do a v2 with both of these changes.

Thanks,
Jon

> These changes might be intended but if so, the commit message should mention this.
>
> Cheers,
>
> Richard
>