[yocto] [meta-raspberrypi][PATCH v6 3/4] linux-raspberrypi-base.bbclass: support for .dtbo files for dtb overlays

[Francois Muller]

Hi Hervé,

Thank you for giving us up and running .dtbo overlays functionality
through this patch, it is running fine for me.

Nevertheless, I have a concern about the processing applied to overlay
filenames.

According to this post https://www.raspberrypi.org/forums/viewtopic.php
?f=107&t=139732, '-overlay.dtb' suffixes have been superseded by
'.dtbo' suffix to reflect an improvement in overlay functionalities and
thus overlay binary format (or simply content, I don't know).
Moreover, changing the suffix gives a chance for older and newer
overlay formats to coexist in a boot folder.

So, I'm not very fond of a build system changing the inputs I give to
it (i.e. KERNEL_DEVICETREE) depending on another input from my side
(kernel version) without at least notifying me the change occurred.
I would highly prefer a bbwarn (or should it even fail?) if the
requested overlay filenames don't suit the kernel version being chosen
(btw device trees are theoretically aimed to be kernel version and even
OS independent as it should only contain pure hardware platform
description).
I understand and appreciate the intention of offering a kind of
compatibility for kernels prior to 4.4.6 but I don't think it's needed
here. 

If KERNEL_DEVICETREE isn't empty for kernels prior to 3.18 => let's
disable device trees (or simply remove this test, this layer doesn't
provide any)
If KERNEL_DEVICETREE asks for -overlay.dtb and kernel >= 3.18 => let's
compile it, it has been requested
If KERNEL_DEVICETREE asks for -overlay.dtb and/or .dtbo and kernel >=
4.4.6 => let's compile it, it has been requested

The tough thing will be to keep consistency between KERNEL_DEVICETREE
content and kernel version to build.
So as long as 4.1 is the layer's kernel preferred version,
KERNEL_DEVICETREE should only contain '-overlay.dtb' overlays, along
with the next to be used KERNEL_DEVICETREE variable (containing .dtbo)
that is commented out.
As soon as 4.4 is chosen, let's default to .dtbo overlays and comment
out KERNEL_DEVICETREE variable containing '-overlay.dtb' overlays.

What do you think about this behavior ?

As side notes, you may use some existing utility function to compare
software versions like bb.utils.vercmp_string_op and overlays filtering
may be done in a single pass : I've attached a proposal.

BR,
François

Le jeudi 21 juillet 2016 à 06:00 +0800, Herve Jourdain a écrit :
> > Kernel 4.4.6+ on RaspberryPi support .dtbo files for overlays,
instead of .dtb.
> > Add support for both variants of overlays ("-overlay.dtb" and
".dtbo")
> > Change which variant needs to be supported based on the kernel
version
> 
> > > CAUTION: when called from IMAGE_CMD_rpi-sdimg, 'TMPDIR' is not set,
causing 'STAGING_KERNEL_BUILDDIR' to not be expanded, causing
get_kernelversion_file() to fail!
> > > To avoid this problem, get_dts() and split_overlays() MUST be called
with the kernel version parameter set, when called from
IMAGE_CMD_rpi-sdimg!
> 
> > Signed-off-by: Herve Jourdain <herve.jourdain at neuf.fr>
> ---
>  classes/linux-raspberrypi-base.bbclass | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> > diff --git a/classes/linux-raspberrypi-base.bbclass b/classes/linux-
raspberrypi-base.bbclass
> index 40beef1..930fc44 100644
> --- a/classes/linux-raspberrypi-base.bbclass
> +++ b/classes/linux-raspberrypi-base.bbclass
> @@ -1,7 +1,8 @@
>  inherit linux-kernel-base
>  
> -
>  def get_dts(d, ver):
> +    import re
> +
>      staging_dir = d.getVar("STAGING_KERNEL_BUILDDIR", True)
>      dts = d.getVar("KERNEL_DEVICETREE", True)
>  
> @@ -20,20 +21,24 @@ def get_dts(d, ver):
>  
>      # Always turn off device tree support for kernel's < 3.18
>      try:
> -        if int(min_ver[0]) <= 3:
> -            if int(min_ver[1]) < 18:
> -                dts = ""
> +        if int(min_ver[0]) >= 4:
> > +            if (int(min_ver[1]) < 4) or (int(min_ver[1]) == 4 and
int(min_ver[2]) < 6):
> > +                dts = ' '.join([(re.sub(r'(.*)\.dtbo$', r'\1-
overlay.dtb', x)) for x in dts.split()])
> +        elif int(min_ver[1]) < 18:
> +            dts = ""
>      except IndexError:
>          min_ver = None
>  
>      return dts
>  
>  
> -def split_overlays(d, out):
> -    dts = get_dts(d, None)
> +def split_overlays(d, ver, out):
> +    dts = get_dts(d, ver)
>      if out:
> >          overlays = oe.utils.str_filter_out('\S+\-overlay\.dtb$',
dts, d)
> > +        overlays = oe.utils.str_filter_out('\S+\.dtbo$', overlays,
d)
>      else:
> -        overlays = oe.utils.str_filter('\S+\-overlay\.dtb$', dts, d)
> > +        overlays = oe.utils.str_filter('\S+\-overlay\.dtb$', dts, d)
+ \
> +                   " " + oe.utils.str_filter('\S+\.dtbo$', dts, d)
>  
>      return overlays
> -- 
> 2.7.4
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/yocto/attachments/20160725/f0919020/attachment.html>
-------------- next part --------------
inherit linux-kernel-base

def get_dts(d, ver):
    import re

    staging_dir = d.getVar("STAGING_KERNEL_BUILDDIR", True)
    dts = d.getVar("KERNEL_DEVICETREE", True)

    # d.getVar() might return 'None' as a normal string
    # leading to 'is None' check isn't enough.
    # TODO: Investigate if this is a bug in bitbake
    if ver is None or ver == "None":
        ''' if 'ver' isn't set try to grab the kernel version
        from the kernel staging '''
        ver = get_kernelversion_file(staging_dir)

    if ver is None:
        return dts

    # Always turn off device tree support for kernel's < 3.18
    if bb.utils.vercmp_string_op(ver, '3.18', '<'):
        dts = ""
    elif bb.utils.vercmp_string_op(ver, '4.4.6', '<'):
        dts = ' '.join([(re.sub(r'(.*)\.dtbo$', r'\1-overlay.dtb', x)) for x in dts.split()])
    else:
        pass # no change to dts variable

    return dts


def split_overlays(d, ver, out):
    dts = get_dts(d, ver)
    if out:
        overlays = oe.utils.str_filter_out('\S+(\-overlay\.dtb|\.dtbo)$', dts, d)
    else:
        overlays = oe.utils.str_filter('\S+(\-overlay\.dtb|\.dtbo)$', dts, d)

    return overlays