[OE-core] [PATCH v2 1/2] classes/uboot-extlinux-config: Add class
Ulf Magnusson
ulfalizer at gmail.com
Mon Oct 3 22:24:11 UTC 2016
On Tue, Oct 4, 2016 at 12:08 AM, Ulf Magnusson <ulfalizer at gmail.com> wrote:
> Hello,
>
> On Mon, Oct 3, 2016 at 2:17 PM, Fabio Berton
> <fabio.berton at ossystems.com.br> wrote:
>> This class allow the extlinux.conf generation for U-Boot use.
>> The U-Boot support for it is given to allow the Generic Distribution
>> Configuration specification use by OpenEmbedded-based products.
>>
>> This class can be inherited by u-boot recipes to create extlinux.conf
>> and boot using menu options.
>>
>> U-boot with extlinux support is machine dependent, so to use this class
>> you need to set UBOOT_EXTLINUX to 1 in machine configuration file and
>> also set root= kernel cmdline UBOOT_EXTLINUX_ROOT. This variable is used
>> to pass root kernel cmdline, e.g:
>>
>> UBOOT_EXTLINUX_ROOT = "root=/dev/mmcblk2p2"
>>
>> Signed-off-by: Fabio Berton <fabio.berton at ossystems.com.br>
>> Signed-off-by: Otavio Salvador <otavio at ossystems.com.br>
>> ---
>> meta/classes/uboot-extlinux-config.bbclass | 130 +++++++++++++++++++++++++++++
>> 1 file changed, 130 insertions(+)
>> create mode 100644 meta/classes/uboot-extlinux-config.bbclass
>>
>> diff --git a/meta/classes/uboot-extlinux-config.bbclass b/meta/classes/uboot-extlinux-config.bbclass
>> new file mode 100644
>> index 0000000..a4dc0c0
>> --- /dev/null
>> +++ b/meta/classes/uboot-extlinux-config.bbclass
>> @@ -0,0 +1,130 @@
>> +# uboot-extlinux-config.bbclass
>> +#
>> +# This class allow the extlinux.conf generation for U-Boot use. The
>> +# U-Boot support for it is given to allow the Generic Distribution
>> +# Configuration specification use by OpenEmbedded-based products.
>> +#
>> +# External variables:
>> +#
>> +# UBOOT_EXTLINUX_CONFIG - Configuration file default set to source dir.
>> +# UBOOT_EXTLINUX_CONSOLE - Set to "console=ttyX" to change kernel boot
>> +# default console.
>> +# UBOOT_EXTLINUX_LABELS - A list of targets for the automatic config.
>> +# UBOOT_EXTLINUX_KERNEL_ARGS - Add additional kernel arguments.
>> +# UBOOT_EXTLINUX_KERNEL_IMAGE - Kernel image name.
>> +# UBOOT_EXTLINUX_FDTDIR - Device tree directory.
>> +# UBOOT_EXTLINUX_INITRD - Indicates a list of filesystem images to
>> +# concatenate and use as an initrd (optional).
>> +# UBOOT_EXTLINUX_MENU_DESCRIPTION - Name to use as description.
>> +# UBOOT_EXTLINUX_ROOT - Root kernel cmdline.
>> +#
>> +# If there's only one label system will boot automatically and menu won't be
>> +# created. If you want to use more than one labels, e.g linux and alternate,
>> +# use overrides to set menu description, console and others variables.
>> +#
>> +# Ex:
>> +#
>> +# UBOOT_EXTLINUX_LABELS ??= "default fallback"
>> +#
>> +# UBOOT_EXTLINUX_KERNEL_IMAGE_default ??= "../zImage"
>> +# UBOOT_EXTLINUX_MENU_DESCRIPTION_default ??= "Linux Default"
>> +#
>> +# UBOOT_EXTLINUX_KERNEL_IMAGE_fallback ??= "../zImage-fallback"
>> +# UBOOT_EXTLINUX_MENU_DESCRIPTION_fallback ??= "Linux Fallback"
>> +#
>> +# Results:
>> +#
>> +# menu title Select the boot mode
>> +# LABEL Linux Default
>> +# KERNEL ../zImage
>> +# FDTDIR ../
>> +# APPEND root=/dev/mmcblk2p2 rootwait rw console=${console}
>> +# LABEL Linux Fallback
>> +# KERNEL ../zImage-fallback
>> +# FDTDIR ../
>> +# APPEND root=/dev/mmcblk2p2 rootwait rw console=${console}
>> +#
>> +# Copyright (C) 2016, O.S. Systems Software LTDA. All Rights Reserved
>> +# Released under the MIT license (see packages/COPYING)
>> +#
>> +# The kernel has an internal default console, which you can override with
>> +# a console=...some_tty...
>> +UBOOT_EXTLINUX_CONSOLE ??= "console=${console}"
>> +UBOOT_EXTLINUX_CONFIG ??= "${S}/extlinux.conf"
>> +UBOOT_EXTLINUX_LABELS ??= "linux"
>> +UBOOT_EXTLINUX_FDTDIR ??= "../"
>> +UBOOT_EXTLINUX_KERNEL_IMAGE ??= "../${KERNEL_IMAGETYPE}"
>> +UBOOT_EXTLINUX_KERNEL_ARGS ??= "rootwait rw"
>> +UBOOT_EXTLINUX_MENU_DESCRIPTION_linux ??= "${DISTRO_NAME}"
>> +
>> +python create_extlinux_config() {
>> + if d.getVar("UBOOT_EXTLINUX", False) == "1":
>
> Is there a reason why False is passed here? Passing True would make
> the code more general, as the value of the flag could come from
> another variable, e.g.
>
> UBOOT_EXTLINUX = "${SOME_FLAG_HELPER_VAR}"
>
> , or from a function, e.g.
>
> UBOOT_EXTLINUX = "${@needs_uboot_extlinux()}"
>
> As a minor style note, the indentation of the function could be
> decreased by doing
>
> if d.getVar("UBOOT_EXTLINUX", True) != "1":
> return
> ...
>
> That might avoid excessive indentation if you use 'with' as mentioned below.
>
>> + workdir = d.getVar('WORKDIR', True)
>> + if not workdir:
>> + bb.error("WORKDIR not defined, unable to package")
>> + return
>
> Could be simplified to the following to make it clear that 'workdir'
> is never referenced afterwards (I know there's some existing code like
> the above though):
>
> if not d.getVar('WORKDIR', True):
> ...
>
>> +
>> + labels = d.getVar('UBOOT_EXTLINUX_LABELS', True)
>> + if not labels:
>> + bb.fatal(1, "UBOOT_EXTLINUX_LABELS not defined, nothing to do")
>> + return
>
> 'return' is redundant after bb.fatal(). bb.fatal() throws an exception
> (BBHandledException, though that's internalish), so the 'return' will
> never be reached.
>
>> +
>> + if labels == []:
>> + bb.fatal(1, "No labels, nothing to do")
>> + return
>
> Redundant 'return'.
>
>> +
>> + cfile = d.getVar('UBOOT_EXTLINUX_CONFIG', True)
>> + if not cfile:
>> + bb.fatal('Unable to read UBOOT_EXTLINUX_CONFIG')
>> +
>> + try:
>> + cfgfile = open(cfile, 'w')
>> + except OSError:
>> + bb.fatal('Unable to open %s' % (cfile))
>> +
>> + cfgfile.write('# Generic Distro Configuration file generated by OpenEmbedded\n')
>> +
>> + if len(labels.split()) > 1:
>> + cfgfile.write('menu title Select the boot mode\n')
>> +
>> + for label in labels.split():
>> + localdata = bb.data.createCopy(d)
>> +
>> + overrides = localdata.getVar('OVERRIDES', True)
>> + if not overrides:
>> + bb.fatal('OVERRIDES not defined')
>> +
>> + localdata.setVar('OVERRIDES', label + ':' + overrides)
>> + bb.data.update_data(localdata)
>> +
>> + extlinux_console = localdata.getVar('UBOOT_EXTLINUX_CONSOLE', True)
>> +
>> + menu_description = localdata.getVar('UBOOT_EXTLINUX_MENU_DESCRIPTION', True)
>> + if not menu_description:
>> + menu_description = label
>> +
>> + root = localdata.getVar('UBOOT_EXTLINUX_ROOT', True)
>> + if not root:
>> + bb.fatal('UBOOT_EXTLINUX_ROOT not defined')
>> +
>> + kernel_image = localdata.getVar('UBOOT_EXTLINUX_KERNEL_IMAGE', True)
>> + fdtdir = localdata.getVar('UBOOT_EXTLINUX_FDTDIR', True)
>> + if fdtdir:
>> + cfgfile.write('LABEL %s\n\tKERNEL %s\n\tFDTDIR %s\n' %
>> + (menu_description, kernel_image, fdtdir))
>> + else:
>> + cfgfile.write('LABEL %s\n\tKERNEL %s\n' % (menu_description, kernel_image))
>> +
>> + kernel_args = localdata.getVar('UBOOT_EXTLINUX_KERNEL_ARGS', True)
>> +
>> + initrd = localdata.getVar('UBOOT_EXTLINUX_INITRD', True)
>> + if initrd:
>> + cfgfile.write('\tINITRD %s\n'% initrd)
>> +
>> + kernel_args = root + " " + kernel_args
>> + cfgfile.write('\tAPPEND %s %s\n' % (kernel_args, extlinux_console))
>> +
>> + cfgfile.close()
>
> Using Python's 'with' syntax would be a better way to handle the file.
> That way, the file is guaranteed to be close()d immediately even if
> the function exits via bb.fatal().
>
> It is possible to use 'with' even after opening the file by the way.
> Just do the following:
>
> with cfgfile:
> ...
>
>> +}
>> +
>> +do_install[prefuncs] += "create_extlinux_config"
>> --
>> 2.1.4
>>
>> --
>
> I'm not familiar with the functionality itself, so hard to say
> anything constructive there.
>
> Cheers,
> Ulf
The manual doesn't make it perfectly clear that bb.fatal() raises an
exception. I opened a documentation bug for it:
https://bugzilla.yoctoproject.org/show_bug.cgi?id=10363
Cheers,
Ulf
More information about the Openembedded-core
mailing list