Re: [PATCH] linux-module/autoload: allow defining multiple modules with custom name

[Henning Schild <henning.schild@siemens.com>]

Am Wed, 12 Sep 2018 14:22:37 +0200
schrieb "[ext] Henning Schild" <henning.schild@siemens.com>:

> Am Wed, 12 Sep 2018 13:42:58 +0200
> schrieb "[ext] claudius.heine.ext@siemens.com"
> <claudius.heine.ext@siemens.com>:
> 
> > From: Claudius Heine <ch@denx.de>
> > 
> > Currently `AUTOLOAD` only supports binary operation that defines if
> > a module (with the same name as the recipe) should be loaded or not.
> > This is very cumbersome to work with if one recipe allows deployment
> > of multiple modules or if the module name differs from the recipe
> > name.
> > 
> > With this patch `AUTOLOAD` will have a new meaning, it revers to a
> > bitbake array that contains the module names.
> > 
> > This commit breaks the usage of `AUTOLOAD`. Recipes will need to
> > replace all their:
> > 
> >     AUTOLOAD = "1"
> > 
> > lines with:
> > 
> >     AUTOLOAD = "${PN}"
> > 
> > And their:
> > 
> >     AUTOLOAD = "0"
> > 
> > lines with:
> > 
> >     AUTOLOAD = ""
> > 
> > in order to migrate to the new interface.
> > 
> > Signed-off-by: Claudius Heine <ch@denx.de>
> > ---
> >  meta-isar/recipes-kernel/example-module/example-module.bb | 2 +-
> >  meta/recipes-kernel/linux-module/module.inc               | 8
> > ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git
> > a/meta-isar/recipes-kernel/example-module/example-module.bb
> > b/meta-isar/recipes-kernel/example-module/example-module.bb index
> > 6b63ae3..1a4554a 100644 ---
> > a/meta-isar/recipes-kernel/example-module/example-module.bb +++
> > b/meta-isar/recipes-kernel/example-module/example-module.bb @@ -11,7
> > +11,7 @@ SRC_URI += "file://src" S = "${WORKDIR}/src"
> >  
> > -AUTOLOAD = "1"
> > +AUTOLOAD = "${PN}"
> >  
> >  # Cross-compilation is not supported for the default Debian
> > kernels. # For example, package with kernel headers for ARM:
> > diff --git a/meta/recipes-kernel/linux-module/module.inc
> > b/meta/recipes-kernel/linux-module/module.inc index 7bc29a8..af7526f
> > 100644 --- a/meta/recipes-kernel/linux-module/module.inc
> > +++ b/meta/recipes-kernel/linux-module/module.inc
> > @@ -13,7 +13,7 @@ DEPENDS += "linux-headers-${KERNEL_NAME}"
> >  
> >  SRC_URI += "file://debian/"
> >  
> > -AUTOLOAD ?= "0"
> > +AUTOLOAD ?= ""
> >  
> >  inherit dpkg
> >  
> > @@ -24,7 +24,7 @@ do_prepare_prepend() {
> >             -e 's/@DESCRIPTION@/${DESCRIPTION}/g' \
> >          ${S}/debian/changelog ${S}/debian/control
> >  
> > -    if [ ${AUTOLOAD} = "1" ]; then
> > -        echo "echo ${PN} >> /etc/modules" >> ${S}/debian/postinst
> > -    fi
> > +    for module in "${AUTOLOAD}"; do
> > +        echo "echo $module >> /etc/modules" >> ${S}/debian/postinst
> > +    done  
> 
> I know that is not the scope of the patch, but here we have an
> update/uninstall problem here.
> 
> In fact we should probably package /etc/modules-load.d/${PN}.conf
> with a "\n" separated list of modules to autoload.
>
> Now ... thinking about it this way, the debianization of the package
> should do that and the whole AUTOLOAD thingy should be dropped from
> Isar.

Ok, that code is actually a debianization helper so it has to deal with
that.
In that case it is all fine, except that we might want to enable
generation of /etc/modules-load.d/ files some day. And do the postinst
steps only if we are not running on systemd. And think about postrm
steps as well.

Henning

> Henning 
> 
> >  }  
>