diff mbox series

[v3] default-distrovars: Have KERNEL_CONSOLE reference SERIAL_CONSOLES

Message ID 20240920184135.789227-1-jdmason@kudzu.us
State Under Review
Headers show
Series [v3] default-distrovars: Have KERNEL_CONSOLE reference SERIAL_CONSOLES | expand

Commit Message

Jon Mason Sept. 20, 2024, 6:41 p.m. UTC
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".

This includes revisions suggested by Quentin Schulz.

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

Comments

Ross Burton Sept. 24, 2024, 11:35 a.m. UTC | #1
> On 20 Sep 2024, at 19:41, Jon Mason via lists.openembedded.org <jdmason=kudzu.us@lists.openembedded.org> wrote:
> 
> --- 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} console=tty0”

My understanding was the .wks files had to be .wks.in for variable expansion to happen.  Can you confirm that this is actually just an idiom and the expansions happen always?

Ross
Jon Mason Sept. 26, 2024, 3:03 p.m. UTC | #2
On Tue, Sep 24, 2024 at 7:35 AM Ross Burton <Ross.Burton@arm.com> wrote:
>
>
>
> > On 20 Sep 2024, at 19:41, Jon Mason via lists.openembedded.org <jdmason=kudzu.us@lists.openembedded.org> wrote:
> >
> > --- 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} console=tty0”
>
> My understanding was the .wks files had to be .wks.in for variable expansion to happen.  Can you confirm that this is actually just an idiom and the expansions happen always?

Always?  No.  Can I confirm it works in my handful of tests?  Yes.

Thanks,
Jon

>
> Ross
Peter Kjellerstedt Sept. 27, 2024, 2:05 a.m. UTC | #3
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Jon Mason
> Sent: den 20 september 2024 20:42
> To: openembedded-core@lists.openembedded.org
> Subject: [OE-core] [PATCH v3] 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".
> 
> This includes revisions suggested by Quentin Schulz.
> 
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
> ---
>  meta/classes-recipe/image_types_wic.bbclass     | 1 +
>  meta/conf/distro/include/default-distrovars.inc | 3 ++-
>  scripts/lib/wic/canned-wks/mkefidisk.wks        | 2 +-
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes-recipe/image_types_wic.bbclass b/meta/classes- recipe/image_types_wic.bbclass
> index 86f40633ebcb..db598e874ad7 100644
> --- a/meta/classes-recipe/image_types_wic.bbclass
> +++ b/meta/classes-recipe/image_types_wic.bbclass
> @@ -26,6 +26,7 @@ WICVARS ?= "\
>  	INITRD \
>  	INITRD_LIVE \
>  	ISODIR \
> +        KERNEL_CONSOLE \

Inconsistent indentation (the other lines use a tab, while this uses spaces).

>  	KERNEL_IMAGETYPE \
>  	MACHINE \
>  	PSEUDO_IGNORE_PATHS \

//Peter
Quentin Schulz Sept. 30, 2024, 9:26 a.m. UTC | #4
Hi Ross,

On 9/24/24 1:35 PM, Ross Burton via lists.openembedded.org wrote:
> 
> 
>> On 20 Sep 2024, at 19:41, Jon Mason via lists.openembedded.org <jdmason=kudzu.us@lists.openembedded.org> wrote:
>>
>> --- 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} console=tty0”
> 
> My understanding was the .wks files had to be .wks.in for variable expansion to happen.  Can you confirm that this is actually just an idiom and the expansions happen always?
> 

meta-rockchip also does variable expansion for wic and only has 
rockchip.wks (and not wks.in), so that's one more that works with this 
(though if that is a missed corner case that we aren't supposed to 
support is another question).

Cheers,
Quentin
Quentin Schulz Sept. 30, 2024, 9:29 a.m. UTC | #5
Hi Jon,

On 9/20/24 8:41 PM, 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".
> 
> This includes revisions suggested by Quentin Schulz.
> 
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
> ---
>   meta/classes-recipe/image_types_wic.bbclass     | 1 +
>   meta/conf/distro/include/default-distrovars.inc | 3 ++-
>   scripts/lib/wic/canned-wks/mkefidisk.wks        | 2 +-
>   3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes-recipe/image_types_wic.bbclass b/meta/classes-recipe/image_types_wic.bbclass
> index 86f40633ebcb..db598e874ad7 100644
> --- a/meta/classes-recipe/image_types_wic.bbclass
> +++ b/meta/classes-recipe/image_types_wic.bbclass
> @@ -26,6 +26,7 @@ WICVARS ?= "\
>   	INITRD \
>   	INITRD_LIVE \
>   	ISODIR \
> +        KERNEL_CONSOLE \
>   	KERNEL_IMAGETYPE \
>   	MACHINE \
>   	PSEUDO_IGNORE_PATHS \
> diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
> index 7554081e8b1a..4d6ddc45b3de 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 ?= "${@','.join(d.getVar('SERIAL_CONSOLES').split(' ')[0].split(';')[::-1]) or 'ttyS0'}"

Here we removed ",115200" from the default value we used to have in the 
bootloader section of the mkefidisk WKS, is this expected? Are there 
possible side effects from that change?

Cheers,
Quentin
Ross Burton Oct. 1, 2024, 1:25 p.m. UTC | #6
On 30 Sep 2024, at 10:29, Quentin Schulz via lists.openembedded.org <quentin.schulz=cherry.de@lists.openembedded.org> wrote:
>> -KERNEL_CONSOLE ?= "ttyS0"
>> +# if SERIAL_CONSOLES is set, take the first device entry.  Otherwise use ttyS0 as the default
>> +KERNEL_CONSOLE ?= "${@','.join(d.getVar('SERIAL_CONSOLES').split(' ')[0].split(';')[::-1]) or 'ttyS0'}"
> 
> Here we removed ",115200" from the default value we used to have in the bootloader section of the mkefidisk WKS, is this expected? Are there possible side effects from that change?

I just checked the kernel documentation and the default is 9600, so I’d say yes there are side effects.

The default should remain ttyS0,115200.

Ross
diff mbox series

Patch

diff --git a/meta/classes-recipe/image_types_wic.bbclass b/meta/classes-recipe/image_types_wic.bbclass
index 86f40633ebcb..db598e874ad7 100644
--- a/meta/classes-recipe/image_types_wic.bbclass
+++ b/meta/classes-recipe/image_types_wic.bbclass
@@ -26,6 +26,7 @@  WICVARS ?= "\
 	INITRD \
 	INITRD_LIVE \
 	ISODIR \
+        KERNEL_CONSOLE \
 	KERNEL_IMAGETYPE \
 	MACHINE \
 	PSEUDO_IGNORE_PATHS \
diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
index 7554081e8b1a..4d6ddc45b3de 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 ?= "${@','.join(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..5fa6682a9e10 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} console=tty0"