Re: make localmodconfig doesn't work for thunderbolt

[Steven Rostedt]

On Fri, 12 May 2023 01:15:49 +0900
Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:

> +CC Steven Rostedt, author of streamline_config.pl

Thanks!

> 
> 
> 
> On Wed, May 10, 2023 at 8:01 PM Jiri Slaby <jirislaby@xxxxxxxxxx> wrote:
> >
> > Hi,
> >
> > if I use localmodconfig for example like this:
> > mkdir /tmp/tb/
> > echo thunderbolt >/tmp/tb/lsmod
> > make O=/tmp/tb LSMOD=/tmp/tb/lsmod localmodconfig
> >
> > I get:
> > using config: '.config'
> > thunderbolt config not found!!
> >
> > $ grep 'USB4\>' /tmp/tb/.config
> > # CONFIG_USB4 is not set
> >
> > I believe it's due to:
> >    obj-${CONFIG_USB4} := thunderbolt.o
> > in drivers/thunderbolt/Makefile. I.e. ${} used instead of more common $().
> >
> > But even if I change the parser:
> >
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -317,7 +317,7 @@ foreach my $makefile (@makefiles) {
> >          $_ = convert_vars($_, %make_vars);
> >
> >          # collect objects after obj-$(CONFIG_FOO_BAR)
> > -       if (/obj-\$\((CONFIG_[^\)]*)\)\s*[+:]?=\s*(.*)/) {
> > +       if (/obj-\$[({](CONFIG_[^})]*)[)}]\s*[+:]?=\s*(.*)/) {
> >              $var = $1;
> >              $objs = $2;
> >
> >
> > I see:
> > module thunderbolt did not have configs CONFIG_USB4
> >
> > and:
> > $ grep 'USB4\>' /tmp/tb/.config
> > # CONFIG_USB4 is not set
> >
> > So two questions:
> > 1) is ${} supported and should be the above change sent as a patch? Or
> > should be drivers/thunderbolt/Makefile fixed to use $(). (And maybe
> > other Makefiles too.)  
> 
> 
> I believe streamline_config.pl should be fixed.

So do I.

> 
> There is no good reason to support only $(), but not ${}.

Agreed.

> 
> 
> In fact, the comment line around line 395 of this script
> uses the curly brace style.
> 
>   # a loaded module. This is a direct obj-${CONFIG_FOO} += bar.o
> 
> 
> 
> 
> 
> 
> > 2) how to fix that 'thunderbolt did not have configs'?  
> 
> That warning is CONFIG_USB4 was unset in the resulting .config.
> 
> Was CONFIG_USB4 enabled before running localmodconfig?
> 
> In my understanding, the purpose of localmodconfig
> is to slim down the .config file.

Correct.

> 
> It disables unneeded CONFIG options, but
> it does not enable needed CONFIG options.

Also correct.

> 
> 
> 
> 
> If I start from allnoconfig (i.e. CONFIG_USB4 is unset), I see the warning.
> 
> $ echo thunderbolt >/tmp/tb/lsmod
> $ make -s O=/tmp/tb allnoconfig
> $ make O=/tmp/tb LSMOD=/tmp/tb/lsmod  localmodconfig
>   GEN     Makefile
> using config: '.config'
> module thunderbolt did not have configs CONFIG_USB4
> 
> 
> 
> If I start from allmodconfig (i.e. CONFIG_USB4 is m), I do not see the warning.
> 
> $ echo thunderbolt >/tmp/tb/lsmod
> $ make -s O=/tmp/tb allmodconfig
> $ make O=/tmp/tb LSMOD=/tmp/tb/lsmod  localmodconfig
>   GEN     Makefile
> using config: '.config'
> 

I hate the inconsistency in the Makefile, but localmodconfig should be
able to adapt to it.

Acked-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>

Thanks Jiri and Masamhiro!

-- Steve