linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: Cristina-Gabriela Moraru <cristina.moraru09@gmail.com>
Cc: "Luis R. Rodriguez" <mcgrof@kernel.org>,
	linux-kernel@vger.kernel.org, Tom Gundersen <teg@jklm.no>,
	Kay Sievers <kay@vrfy.org>, Rusty Russell <rusty@rustcorp.com.au>,
	akpm@linux-foundation.org, backports@vger.kernel.org
Subject: Re: [RFC PATCH 2/5] Add CONFIG symbol to module as compilation parameter
Date: Tue, 23 Aug 2016 21:07:53 +0200	[thread overview]
Message-ID: <20160823190753.GF3296@wotan.suse.de> (raw)
In-Reply-To: <CAGZ2q2yt1NYshNpm5ZCmPXfYxPynttEU7KiiL3RALegGD04nug@mail.gmail.com>

On Sat, Aug 20, 2016 at 05:11:37PM +0200, Cristina-Gabriela Moraru wrote:
> 2016-08-18 20:10 GMT+02:00 Luis R. Rodriguez <mcgrof@kernel.org>:
> > On Wed, Aug 17, 2016 at 09:27:00PM +0200, Cristina Moraru wrote:
> >> Add  CONFIG symbol to kernel modules as a define via -D
> >
> > Perhaps better worded as:
> >
> > When modules have a direct Kconfig CONFIG_ symbol associated with
> > we want to be able to make it available to the build system when we
> > are building the module.
> >
> > You can then describe you do this with -D.
> >
> >> compilation parameter. The CONFIG_FOO symbol for each
> >> module is determined by the module name, using the
> >> associations from Module.ksymb. This file is loaded
> >> using the 'include' directive, thus run like a regular
> >> makefile. The format of the content of the file is the
> >> following:
> >>
> >> foo_KCONF=CONFIG_FOO
> >>
> >> creating a set of Makefile variables foo_KCONF with the
> >> CONFIG_FOO as values.
> >>
> >> The Makefile then adds this value in the compilation
> >> command with -DKBUILD_KCONF='"CONFIG_FOO"'.
> >
> > Very nice. What would it mean if it lacks this ?
> > This should be explained on the commit log.
> >
> 
> If we lack it then KBUILD_KCONF="" and in /sys the module has the
> kconfig_symbol attribute but with the empty string as content:
> 
> prompt:/sys$ cat ./module/mptbase/kconfig_symbol
> 
> prompt:/sys$
> 
> I will add it into the commit log.

OK -- I do wonder if instead of an empty string leaving the kconfig_symbol
out is better. If its empty then it can be confusing, and so perhaps better
an "unknown" is better. But skipping the attribute then seems best as then
we can focus on just addressing what it *does mean* when we do have a mapping.
This would allow addressing the semantic gap of modules that lack this and
trying to fix those step by step. To fix those we first need to identify
*why* we can't get an attribute pegged to these - document this perhaps on
kernelnewbies.org/KernelProjects/<pick-a-topic-name-for-your-project>
and then your commit log can reference this for a list of known reasons
and pending work.

> >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> >> index e7df0f5..8ae9b7f 100644
> >> --- a/scripts/Makefile.lib
> >> +++ b/scripts/Makefile.lib
> >> @@ -89,6 +89,10 @@ multi-objs-m       := $(addprefix $(obj)/,$(multi-objs-m))
> >>  subdir-ym    := $(addprefix $(obj)/,$(subdir-ym))
> >>  obj-dirs     := $(addprefix $(obj)/,$(obj-dirs))
> >>
> >> +# Include Module.ksymb which contains the associations of modules' names
> >> +# and corresponding CONFIG_* options
> >> +include Module.ksymb
> >
> > Is this file always generated? If not prefixing the include call with - would
> > be better. If we are going to make this optional, then an ifdef wrapper would
> > suffice as we know it'd be expected only if the new feature was enabled.
> 
> Yes. This file is always generated. In case a 'git pull' happened
> between two 'make' commands, the associations should be updated. Ok. I
> will add a ifdef.
> 
> >
> >> +
> >>  # These flags are needed for modversions and compiling, so we define them here
> >>  # already
> >>  # $(modname_flags) #defines KBUILD_MODNAME as the name of the module it will
> >> @@ -100,6 +104,9 @@ name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst -,_,$1))$(quote)$(squote)
> >>  basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
> >>  modname_flags  = $(if $(filter 1,$(words $(modname))),\
> >>                   -DKBUILD_MODNAME=$(call name-fix,$(modname)))
> >> +ksym-fix = $(squote)$(quote)$($(subst $(comma),_,$(subst -,_,$1))_KCONF)$(quote)$(squote)
> >> +ksymb_flags = $(if $(filter 1,$(words $(modname))),\
> >> +                 -DKBUILD_KSYMB=$(call ksym-fix, $(modname)))
> >
> > Are clashes possible with this formula? Can we end up with two results for instance?
> > If not what prevents current konfig logic and namespace from a clash ? If we do
> > not have anything to prevent a clash, what can we do to help make such clash not
> > possible ?
> 
> I think the fact that modname is unique prevents from having clashes.
> KBUILD_KSYMB is found according to modname.
> Currently there is no clash because I enforced the Module.ksymb to
> have 1-to-1 mapping.
> The only potential clash I can imagine right now it having the same
> module name but architecture specific CONFIG_* symbol. If there is a
> 1-to-1 mapping in Module.ksymb there should be no clash in the
> makefile.

OK thanks.

  Luis

  reply	other threads:[~2016-08-23 19:10 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-17 19:26 [RFC PATCH 0/5] Add CONFIG symbol as module attribute Cristina Moraru
2016-08-17 19:26 ` [RFC PATCH 1/5] Add generation of Module.symb in streamline_config Cristina Moraru
2016-08-18 18:22   ` Luis R. Rodriguez
2016-08-18 18:32     ` Luis R. Rodriguez
     [not found]     ` <CAGZ2q2xi9Uy-ye387=mWhy_fOEJBC593Nos7fH027m-_ZdoOXA@mail.gmail.com>
2016-08-20 14:49       ` Cristina-Gabriela Moraru
2016-08-23 19:00       ` Luis R. Rodriguez
2016-08-17 19:27 ` [RFC PATCH 2/5] Add CONFIG symbol to module as compilation parameter Cristina Moraru
2016-08-18 18:10   ` Luis R. Rodriguez
2016-08-18 18:55     ` Luis R. Rodriguez
2016-08-20 15:11     ` Cristina-Gabriela Moraru
2016-08-23 19:07       ` Luis R. Rodriguez [this message]
2016-08-17 19:27 ` [RFC PATCH 3/5] Trigger Module.ksymb generation in Makefile Cristina Moraru
2016-08-18 18:30   ` Luis R. Rodriguez
2016-08-17 19:27 ` [RFC PATCH 4/5] Set KCONFIG_KSYMB as value for kconfig_ksymb module attribute Cristina Moraru
2016-08-18 18:59   ` Luis R. Rodriguez
2016-08-20 15:16     ` Cristina-Gabriela Moraru
2016-08-23 19:10       ` Luis R. Rodriguez
2016-08-17 19:27 ` [RFC PATCH 5/5] Add kconf_symb as kernel " Cristina Moraru
2016-08-18 19:02   ` Luis R. Rodriguez
2016-08-18 17:55 ` [RFC PATCH 0/5] Add CONFIG symbol as " Luis R. Rodriguez
2016-08-19  9:07   ` Michal Marek
2016-08-22 19:48     ` Cristina-Gabriela Moraru
2016-08-23 21:32     ` Luis R. Rodriguez
2016-08-24 11:05       ` Michal Marek
2016-08-24 16:33         ` Luis R. Rodriguez
2016-08-24 17:31           ` Naveen Kumar
2016-08-22 19:35   ` Cristina-Gabriela Moraru
2016-08-23 19:17     ` Luis R. Rodriguez
2016-08-25  7:43   ` Christoph Hellwig
2016-08-25  8:00     ` Johannes Berg
2016-08-25 19:51       ` Luis R. Rodriguez
2016-08-25  8:41     ` Michal Marek
2016-08-25 20:19     ` Luis R. Rodriguez
2019-02-05 22:07       ` Luis Chamberlain
2019-06-26 22:21         ` Luis Chamberlain
2019-06-27  4:50           ` Christoph Hellwig
2019-06-28 18:40             ` Luis Chamberlain
2019-06-29  8:42               ` Greg Kroah-Hartman
2019-07-02 20:51                 ` Luis Chamberlain
2019-07-03  7:40                   ` Greg Kroah-Hartman
2019-07-03 16:50                     ` Luis Chamberlain
2019-07-03 18:57                       ` Greg Kroah-Hartman
2019-07-03 22:25                         ` Luis Chamberlain
2019-07-11 23:07                           ` Brendan Higgins
2019-07-11 23:22                             ` Luis Chamberlain
2019-07-03 12:16               ` Enrico Weigelt, metux IT consult
2019-07-03 17:35                 ` Luis Chamberlain
2019-07-03 19:31                   ` Enrico Weigelt, metux IT consult
2019-07-03 22:42                     ` Luis Chamberlain
2019-07-11 23:27                       ` Brendan Higgins
2019-07-13 14:44                         ` Enrico Weigelt, metux IT consult

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160823190753.GF3296@wotan.suse.de \
    --to=mcgrof@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=backports@vger.kernel.org \
    --cc=cristina.moraru09@gmail.com \
    --cc=kay@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rusty@rustcorp.com.au \
    --cc=teg@jklm.no \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).