Steven,
A few remarks follow. (Mainly suggesting to add a some comments.)
On Fri, 2012-01-13 at 18:12 -0500, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@xxxxxxxxxx>
>
> Thomas Lange reported that when he did a 'make localmodconfig', his
> config was missing the brcmsmac driver, even though he had the module
> loaded.
>
> Looking into this, I found the file:
> drivers/net/wireless/brcm80211/brcmsmac/Makefile
> had the following in the Makefile:
>
> MODULEPFX := brcmsmac
>
> obj-$(CONFIG_BRCMSMAC) += $(MODULEPFX).o
>
> The way streamline-config.pl works, is parsing all the
> obj-$(CONFIG_FOO) += foo.o
> lines to find that CONFIG_FOO belongs to the module foo.ko.
>
> But in this case, the brcmsmac.o was not used, but a variable in its place.
>
> By changing streamline-config.pl to remember defined variables in Makefiles
> and substituting them when they are used in the obj-X lines, allows
> Thomas (and others) to have their brcmsmac module stay configured
> when it is loaded and running "make localmodconfig".
>
> Reported-by: Thomas Lange <thomas-lange2@xxxxxx>
> Tested-by: Thomas Lange <thomas-lange2@xxxxxx>
> Cc: Arend van Spriel <arend@xxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
> ---
> scripts/kconfig/streamline_config.pl | 29 +++++++++++++++++++++++++++++
> 1 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 42ef5ea..bccf07d 100644
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -250,10 +250,33 @@ if ($kconfig) {
> read_kconfig($kconfig);
> }
>
> +sub convert_vars {
> + my ($line, %vars) = @_;
> +
> + my $process = "";
> +
> + while ($line =~ s/^(.*?)(\$\((.*?)\))//) {
I know just enough perl to be dangerous. But I'm guessing that even
people more familiar with perl (and regexes) wouldn't mind a short
comment here. Perhaps something like
# handle all occurrences of "$(VAR)" in this line
> + my $start = $1;
> + my $variable = $2;
> + my $var = $3;
Using both "$variable" and "$var" is a bit awkward. Perhaps "$name"
instead of "$var"?
> +
> + if (defined($vars{$var})) {
> + $process .= $start . $vars{$var};
> + } else {
Add
# Unknown variable, just put it back
here (or something similar)?
> + $process .= $start . $variable;
> + }
> + }
> +
> + $process .= $line;
> +
> + return $process;
> +}
> +
> # Read all Makefiles to map the configs to the objects
> foreach my $makefile (@makefiles) {
>
> my $line = "";
> + my %make_vars;
>
> open(MIN,$makefile) || die "Can't open $makefile";
> while (<MIN>) {
> @@ -270,10 +293,16 @@ foreach my $makefile (@makefiles) {
>
> my $objs;
>
> + $_ = convert_vars($_, %make_vars);
> +
> # collect objects after obj-$(CONFIG_FOO_BAR)
> if (/obj-\$\((CONFIG_[^\)]*)\)\s*[+:]?=\s*(.*)/) {
This is unrelated to this patch, but anyhow. Doing
git grep -n "obj-" $(git ls-files "*Makefile*") | grep CONFIG | grep -v "obj-\$(CONFIG"
generated three types of possible corner cases here. Some examples:
0) "obj-pci-$(CONFIG_ARCH_IXDP4XX) += ixdp425-pci.o"
(there are about a dozen of these)
and
"obj-pvrusb2-sysfs-$(CONFIG_VIDEO_PVRUSB2_SYSFS) := pvrusb2-sysfs.o"
(three times)
Are these relevant for this script?
1) "obj-${CONFIG_EISA_PCI_EISA} += pci_eisa.o"
(three times)
The braces look a bit odd. Shouldn't that be parentheses, at least to be
consistent? Michal?
2) "obj-$(subst m,y,$(CONFIG_VLAN_8021Q)) += vlan_core.o"
(about a dozen times)
I do not know what subst actually does here, but this might be missed by
this script too. Relevant?
Do these corner cases merit further attention?
> $var = $1;
> $objs = $2;
> +
> + # check if variables are set
"[...] and safe them for future substitution"?
> + } elsif (/^\s*(\S+)\s*[:]?=\s*(.*\S)/) {
> + $make_vars{$1} = $2;
> }
> if (defined($objs)) {
> foreach my $obj (split /\s+/,$objs) {
Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
