Re: [PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF

["Hart, Darren" <darren.hart@intel.com>]

On 6/13/14, 12:13, "Denys Dmytriyenko" <denis@denix.org> wrote:

>On Fri, Jun 13, 2014 at 03:02:05PM -0400, Bruce Ashfield wrote:
>> On Fri, Jun 13, 2014 at 11:46 AM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>> > The current module_autoload_* and module_conf_* variables are error
>> > both ugly and error prone. They aren't registered in the task
>>checksums
>> > so changes to them aren't reflected in the build. This turns out to
>> > be near impossible to fix with the current variable format in any
>> > sensible way :(.
>> >
>> > This patch replace module_autoload with the list of variables in
>> > KERNEL_MODULE_AUTOLOAD which is a much simpler and usable API. An
>> > error is printed if an old style variable is encountered. It should
>> > be simple to convert to this.
>> >
>> > module_conf_* are harder to deal with since there is data associated
>> > with it, it isn't simply a flag. We need a list of variables that are
>>set
>> > in order to be able to correctly handle the task checksum so we add
>> > KERNEL_MODULE_PROBECONF for this purpose and error if the user hasn't
>> > added a module to it when they should have.
>> 
>> Looks reasonable to me. In my experience, there aren't a lot of users of
>> the module_* variables, so the transition to the new names and semantics
>> shouldn't be a big issue.
>
>Right, only BSP/Distro maintainers should care... :)
>
>BTW, how about a documentation patch? :)

Adding Scott R.

I was just looking into that. It appears the ref-manual.html is the place
to update. The glossary has a module_autoload definition, which I suppose
needs to be replaced with KERNEL_MODULE_AUTOLOAD, which  will have
slightly different semantics.

If I understand this correctly, the old model was:

	module_autoload_foo = "foo"
	module_autoload_bar = "bar"

Although the following line in the docs confuses me:

	module_autoload_<modname> = "modname1 modname2 modname3"


And now, if I interpreted the commit comment correctly, it should look
like:

	KERNEL_MODULE_AUTOLOAD = "foo"
	...
	KERNEL_MODULE_AUTOLOAD += "bar"

I'm not sure how KERNEL_MODULE_PROBECONF is involved, or what value it
brings beyond module_conf. From what I can tell, the changes now require:

	KERNEL_MODULE_PROBECONF = "foo"

	module_conf_foo = "options foo baz=1"

(/me notes the order of operations is non-obvious here "if modconf and
basename in modconflist")

Do I have this correct?

--
Darren

>
>-- 
>Denys
>
>
>> > [YOCTO #5786]
>> >
>> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>> >
>> > diff --git a/meta/classes/kernel-module-split.bbclass
>>b/meta/classes/kernel-module-split.bbclass
>> > index d43f743..e38a6f6 100644
>> > --- a/meta/classes/kernel-module-split.bbclass
>> > +++ b/meta/classes/kernel-module-split.bbclass
>> > @@ -130,8 +130,11 @@ python split_kernel_module_packages () {
>> >
>> >          # If autoloading is requested, output
>>/etc/modules-load.d/<name>.conf and append
>> >          # appropriate modprobe commands to the postinst
>> > +        autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD", True) or
>>"").split()
>> >          autoload = d.getVar('module_autoload_%s' % basename, True)
>> >          if autoload:
>> > +            bb.error("KERNEL_MODULE_AUTOLOAD has replaced
>>module_autoload_%s, please replace it!" % basename)
>> > +        if basename in autoloadlist:
>> >              name = '%s/etc/modules-load.d/%s.conf' % (dvar, basename)
>> >              f = open(name, 'w')
>> >              for m in autoload.split():
>> > @@ -144,12 +147,15 @@ python split_kernel_module_packages () {
>> >              d.setVar('pkg_postinst_%s' % pkg, postinst)
>> >
>> >          # Write out any modconf fragment
>> > +        modconflist = (d.getVar("KERNEL_MODULE_PROBECONF", True) or
>>"").split()
>> >          modconf = d.getVar('module_conf_%s' % basename, True)
>> > -        if modconf:
>> > +        if modconf and basename in modconflist:
>> >              name = '%s/etc/modprobe.d/%s.conf' % (dvar, basename)
>> >              f = open(name, 'w')
>> >              f.write("%s\n" % modconf)
>> >              f.close()
>> > +        elif modconf:
>> > +            bb.error("Please ensure module %s is listed in
>>KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename,
>>basename))
>> >
>> >          files = d.getVar('FILES_%s' % pkg, True)
>> >          files = "%s /etc/modules-load.d/%s.conf
>>/etc/modprobe.d/%s.conf" % (files, basename, basename)
>> > @@ -185,3 +191,5 @@ python split_kernel_module_packages () {
>> >          if len(os.listdir(dir)) == 0:
>> >              os.rmdir(dir)
>> >  }
>> > +
>> > +do_package[vardeps] += '${@" ".join(map(lambda s: "module_conf_" +
>>s, (d.getVar("KERNEL_MODULE_PROBECONF", True) or "").split()))}'
>> >
>> >
>> 
>> 
>> 
>> -- 
>> "Thou shalt not follow the NULL pointer, for chaos and madness await
>> thee at its end"
>> -- 
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>


-- 
Darren Hart					Open Source Technology Center
darren.hart@intel.com				            Intel Corporation