On Tue, Oct 4, 2016 at 12:24 AM, Ulf Magnusson <ulfali...@gmail.com> wrote:
> On Tue, Oct 4, 2016 at 12:08 AM, Ulf Magnusson <ulfali...@gmail.com> wrote:
>> Hello,
>>
>> On Mon, Oct 3, 2016 at 2:17 PM, Fabio Berton
>> <fabio.ber...@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.ber...@ossystems.com.br>
>>> Signed-off-by: Otavio Salvador <ota...@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

It looks like the first two bb.fatal() calls accidentally ended up
with two arguments by the way. bb.fatal() does not take a 'level'
arguments like bb.debug() does.

Cheers,
Ulf
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Reply via email to