(no subject)

[David Hunter]

	
Date: Fri, 13 Sep 2024 11:52:16 -0400
Subject: [PATCH 0/7] linux-kbuild: fix: process configs set to "y"

An assumption made in this script is that the config options do not need
to be processed because they will simply be in the new config file. This
assumption is incorrect. 

Process the config entries set to "y" because those config entries might
have dependencies set to "m". If a config entry is set to "m" and is not
loaded directly into the machine, the script will currently turn off
that config entry; however, if that turned off config entry is a
dependency for a "y" option. that means the config entry set to "y"
will also be turned off later when the conf executive file is called. 

Here is a model of the problem (arrows show dependency): 

Original config file
Config_1 (m) <-- Config_2 (y) 

Config_1 is not loaded in this example, so it is turned off. 
After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf
Config_1 (n) <-- Config_2 (y) 

After  scripts/kconfig/conf
Config_1 (n) <-- Config_2 (n) 


It should also be noted that any module in the dependency chain will
also be turned off, even if that module is loaded directly onto the
computer. Here is an example: 

Original config file
Config_1 (m) <-- Config_2 (y) <-- Config_3 (m)

Config_3 will be loaded in this example.
After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf
Config_1 (n) <-- Config_2 (y) <-- Config_3 (m)

After scripts/kconfig/conf
Config_1 (n) <-- Config_2 (n) <-- Config_3 (n)


I discovered this problem when I ran "make localmodconfig" on a generic
Ubuntu config file. Many hardware devices were not recognized once the
kernel was installed and booted. Another way to reproduced the error I
had is to run "make localmodconfig" twice. The standard error might display
warnings that certain modules should be selected but no config files are
turned on that select that module. 

With the changes in this series patch, all modules are loaded properly
and all of the hardware is loaded when the kernel is installed and
booted.  


David Hunter (7):
  linux-kbuild: fix: config option can be bool
  linux-kbuild: fix: missing variable operator
  linux-kbuild: fix: ensure all defaults are tracked
  linux-kbuild: fix: ensure selected configs were turned on in original
  linux-kbuild: fix: implement choice for kconfigs
  linux-kbuild: fix: configs with defaults do not need a prompt
  linux-kbuild: fix: process config options set to "y"

 scripts/kconfig/streamline_config.pl | 77 ++++++++++++++++++++++++----
 1 file changed, 66 insertions(+), 11 deletions(-)

-- 
2.43.0

[PATCH 1/7] linux-kbuild: fix: config option can be bool

[David Hunter]

Select configs that do not have a prompt. Config options can be bool or
tristate. Ensure that bool options are also selected.

Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
---
 scripts/kconfig/streamline_config.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index d51cd7ac15d2..a828d7ab7e26 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -238,7 +238,7 @@ sub read_kconfig {
 	    }
 
 	# configs without prompts must be selected
-	} elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {
+	} elsif ($state ne "NONE" && /^\s*((bool|tristate)\s+\S|prompt\b)/) {
 	    # note if the config has a prompt
 	    $prompts{$config} = 1;
 
-- 
2.43.0

Re: [PATCH 1/7] linux-kbuild: fix: config option can be bool

[Masahiro Yamada]

"linux-kbuild: fix:" is not an appropriate subject prefix.

Just try "git log scripts/kconfig/streamline_config.pl".


You will see  "streamline_config.pl:" is consistently used
for the commit subject.


For example,

"streamline_config.pl: check prompt for bool options"




On Sat, Sep 14, 2024 at 2:12 AM David Hunter
<david.hunter.linux@gmail.com> wrote:
>
> Select configs that do not have a prompt. Config options can be bool or
> tristate. Ensure that bool options are also selected.


I do not see an immediate benefit from this patch.



Boolean CONFIG options are skipped due to the following code:

if (defined($orig_configs{$config}) && $orig_configs{$config} ne "m") {
    next forloop;
}


So, I do not understand why this patch is necessary
until I see 7/7.




>
> Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
> ---
>  scripts/kconfig/streamline_config.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index d51cd7ac15d2..a828d7ab7e26 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -238,7 +238,7 @@ sub read_kconfig {
>             }
>
>         # configs without prompts must be selected
> -       } elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {
> +       } elsif ($state ne "NONE" && /^\s*((bool|tristate)\s+\S|prompt\b)/) {
>             # note if the config has a prompt
>             $prompts{$config} = 1;
>




--
Best Regards

Masahiro Yamada

Re: [PATCH 1/7] linux-kbuild: fix: config option can be bool

[David Hunter]

On 9/23/24 22:55, Masahiro Yamada wrote:
> 
> I do not see an immediate benefit from this patch.
> 
> 
> 
> Boolean CONFIG options are skipped due to the following code:
> 
> if (defined($orig_configs{$config}) && $orig_configs{$config} ne "m") {
>      next forloop;
> }
> 
> 
> So, I do not understand why this patch is necessary
> until I see 7/7.
> 

Thank you for the feedback. I have been working on the second version 
for all of the patches, and I will resend the series patch soon. In the 
meantime, I have a few things I need cleared up, so I will reply to each 
email where appropriate.

For this email, I was a bit unsure of what my takeaway should be from 
this message. Are you saying one of the following:

1) The patch should be resent, but resent after the patch that is 
currently 7/7
2) The patch should be combined with the patch that is currently 7/7
3) The patch message should be improved so that people can see the need 
for the patch

As of now, my version 2 will be made with option 1, the patch will be 
after the current 7/7. If you had something different in mind, let me 
know. I would be happy to change it as needed.

Thanks,
David Hunter

Re: [PATCH 1/7] linux-kbuild: fix: config option can be bool

[David Hunter]

version 2: 
https://lore.kernel.org/all/20241014141345.5680-8-david.hunter.linux@gmail.com/

[PATCH 2/7] linux-kbuild: fix: missing variable operator

[David Hunter]

Put in the dollar sign for the variable '$config'. That way, the debug
message has more meaning.

Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
---
 scripts/kconfig/streamline_config.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index a828d7ab7e26..ddc630f2264a 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -503,7 +503,7 @@ sub parse_config_selects
 
 	    # Check if something other than a module selects this config
 	    if (defined($orig_configs{$conf}) && $orig_configs{$conf} ne "m") {
-		dprint "$conf (non module) selects config, we are good\n";
+		dprint "$conf (non module) selects $config, we are good\n";
 		# we are good with this
 		return;
 	    }
-- 
2.43.0

Re: [PATCH 2/7] linux-kbuild: fix: missing variable operator

[Masahiro Yamada]

Good catch, but please fix the subject prefix.

For example,

streamline_config.pl: fix missing variable operator in debug print





On Sat, Sep 14, 2024 at 2:12 AM David Hunter
<david.hunter.linux@gmail.com> wrote:
>
> Put in the dollar sign for the variable '$config'. That way, the debug
> message has more meaning.
>
> Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
> ---
>  scripts/kconfig/streamline_config.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index a828d7ab7e26..ddc630f2264a 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -503,7 +503,7 @@ sub parse_config_selects
>
>             # Check if something other than a module selects this config
>             if (defined($orig_configs{$conf}) && $orig_configs{$conf} ne "m") {
> -               dprint "$conf (non module) selects config, we are good\n";
> +               dprint "$conf (non module) selects $config, we are good\n";
>                 # we are good with this
>                 return;
>             }
> --
> 2.43.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/7] linux-kbuild: fix: missing variable operator

[David Hunter]

https://lore.kernel.org/all/20241014141345.5680-2-david.hunter.linux@gmail.com/

[PATCH 3/7] linux-kbuild: fix: ensure all defaults are tracked

[David Hunter]

Track default options on the second line. On the second line of some
config entries, default and depndency options sometimes appear. In those
instances, the state will be "NEW" and not "DEP".

Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
---
 scripts/kconfig/streamline_config.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index ddc630f2264a..bb1f19a1ab5e 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -220,7 +220,7 @@ sub read_kconfig {
 	    $depends{$config} = $1;
 	} elsif ($state eq "DEP" && /^\s*depends\s+on\s+(.*)$/) {
 	    $depends{$config} .= " " . $1;
-	} elsif ($state eq "DEP" && /^\s*def(_(bool|tristate)|ault)\s+(\S.*)$/) {
+	} elsif (($state eq "DEP" || $state eq "NEW") && /^\s*def(_(bool|tristate)|ault)\s+(\S.*)$/) {
 	    my $dep = $3;
 	    if ($dep !~ /^\s*(y|m|n)\s*$/) {
 		$dep =~ s/.*\sif\s+//;
-- 
2.43.0

Re: [PATCH 3/7] linux-kbuild: fix: ensure all defaults are tracked

[Masahiro Yamada]

Same for the commit subject.



On Sat, Sep 14, 2024 at 2:12 AM David Hunter
<david.hunter.linux@gmail.com> wrote:
>
> Track default options on the second line. On the second line of some
> config entries, default and depndency options sometimes appear. In those
> instances, the state will be "NEW" and not "DEP".
>
> Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
> ---
>  scripts/kconfig/streamline_config.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index ddc630f2264a..bb1f19a1ab5e 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -220,7 +220,7 @@ sub read_kconfig {
>             $depends{$config} = $1;
>         } elsif ($state eq "DEP" && /^\s*depends\s+on\s+(.*)$/) {
>             $depends{$config} .= " " . $1;
> -       } elsif ($state eq "DEP" && /^\s*def(_(bool|tristate)|ault)\s+(\S.*)$/) {
> +       } elsif (($state eq "DEP" || $state eq "NEW") && /^\s*def(_(bool|tristate)|ault)\s+(\S.*)$/) {



I agree this is correct, but you can also fix it to
  $state ne "NONE"
This is more consistent with the existing code.


A few lines below, we already have this:

} elsif ($state ne "NONE" && /^\s*((bool|tristate)\s+\S|prompt\b)/) {



This will work unless you introduce a new
state, "CHOICE".

I am not sure if it is necessary.







>             my $dep = $3;
>             if ($dep !~ /^\s*(y|m|n)\s*$/) {
>                 $dep =~ s/.*\sif\s+//;
> --
> 2.43.0
>


--
Best Regards
Masahiro Yamada

Re: [PATCH 3/7] linux-kbuild: fix: ensure all defaults are tracked

[David Hunter]

https://lore.kernel.org/all/20241014141345.5680-3-david.hunter.linux@gmail.com/

[PATCH 4/7] linux-kbuild: fix: ensure selected configs were turned on in original

[David Hunter]

Ensure that only modules that were turned on in the original config are
turned on in the new config file. When ensuring that the config
dependencies are met, turning on the config options in the new config
leads to warnings and errors later in this script, especially for badly
constructed original config files.

One example could be a config option that is depended on by a module
needed in the new config but is not turned on in the original config
file. If this config needs to be selected, warnings will show up in the
standard output.

Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
---
 scripts/kconfig/streamline_config.pl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index bb1f19a1ab5e..26e544744579 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -459,7 +459,9 @@ sub parse_config_depends
 		next;
 	    }
 
-	    if (!defined($configs{$conf})) {
+	    # This script does not turn on any modules, so make sure the config
+	    # options are on in the original.
+	    if (!defined($configs{$conf}) && defined($orig_configs{$conf})) {
 		# We must make sure that this config has its
 		# dependencies met.
 		$repeat = 1; # do again
-- 
2.43.0

Re: [PATCH 4/7] linux-kbuild: fix: ensure selected configs were turned on in original

[Masahiro Yamada]

On Sat, Sep 14, 2024 at 2:12 AM David Hunter
<david.hunter.linux@gmail.com> wrote:
>
> Ensure that only modules that were turned on in the original config are
> turned on in the new config file. When ensuring that the config
> dependencies are met, turning on the config options in the new config
> leads to warnings and errors later in this script, especially for badly
> constructed original config files.
>
> One example could be a config option that is depended on by a module
> needed in the new config but is not turned on in the original config
> file. If this config needs to be selected, warnings will show up in the
> standard output.
>
> Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
> ---
>  scripts/kconfig/streamline_config.pl | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index bb1f19a1ab5e..26e544744579 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -459,7 +459,9 @@ sub parse_config_depends
>                 next;
>             }
>
> -           if (!defined($configs{$conf})) {
> +           # This script does not turn on any modules, so make sure the config
> +           # options are on in the original.
> +           if (!defined($configs{$conf}) && defined($orig_configs{$conf})) {
>                 # We must make sure that this config has its
>                 # dependencies met.
>                 $repeat = 1; # do again



I believe defined($orig_configs{$conf} is always true here
because it was already checked a few lines above.


    # We only need to process if the depend config is a module
    if (!defined($orig_configs{$conf}) || $orig_configs{$conf} eq "y") {
        next;
    }


If $conf is not present in the original .config,
the 'next' statement skips the current iteration.






--
Best Regards
Masahiro Yamada

Re: [PATCH 4/7] linux-kbuild: fix: ensure selected configs were turned on in original

[David Hunter]

On 9/23/24 23:45, Masahiro Yamada wrote:

> I believe defined($orig_configs{$conf} is always true here
> because it was already checked a few lines above.
> 
> 
>      # We only need to process if the depend config is a module
>      if (!defined($orig_configs{$conf}) || $orig_configs{$conf} eq "y") {
>          next;
>      }
> 
> 
> If $conf is not present in the original .config,
> the 'next' statement skips the current iteration.
> 


Good point, I will remove this portion for the series patch.

[PATCH 5/7] linux-kbuild: fix: implement choice for kconfigs

[David Hunter]

Properly implement the config entries that are within the choice keyword
for kconfig. Currently, the script only stops the previous config entry
when a choice keyword is encountered.

When the keyword "choice" is encountered, do the following:
	- distribute the lines immediately following the "choice"
	  keyword to each config entry inside the "choice" section.
	- process the config entries with the distributed lines.

Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
---
 scripts/kconfig/streamline_config.pl | 40 ++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 26e544744579..593df824ead7 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -162,6 +162,10 @@ sub read_kconfig {
 
     my $source = "$ksource/$kconfig";
     my $last_source = "";
+    my $choice_activated = 0;
+    my $distribute = 0;
+    my $dist_string;
+
 
     # Check for any environment variables used
     while ($source =~ /\$\((\w+)\)/ && $last_source ne $source) {
@@ -214,6 +218,19 @@ sub read_kconfig {
 		$state = "DEP";
 	    }
 
+           if($choice_activated) {
+               $distribute = 0;
+               my $config_lines = "$_\n" . "$dist_string";
+               my $tmpconfig = ".choice.kconfig";
+               open (my $FH, '>', $tmpconfig);
+               print $FH $config_lines;
+               close($FH);
+
+               read_kconfig($tmpconfig);
+               unlink($tmpconfig) or die "Can't delete $tmpconfig: $!\n";
+           }
+
+
 	# collect the depends for the config
 	} elsif ($state eq "NEW" && /^\s*depends\s+on\s+(.*)$/) {
 	    $state = "DEP";
@@ -258,8 +275,27 @@ sub read_kconfig {
 	    $iflevel-- if ($iflevel);
 
 	# stop on "help" and keywords that end a menu entry
-	} elsif (/^\s*(---)?help(---)?\s*$/ || /^(comment|choice|menu)\b/) {
-	    $state = "NONE";
+	} elsif (/^\s*(---)?help(---)?\s*$/ || /^(comment|menu)\b/) {
+            $state = "NONE";
+
+	# for choice, distribute the lines before each config entry
+	# to each config entry
+	} elsif (/^\s*choice\b/) {
+	    $state = "CHOICE";
+	    $choice_activated = 1;
+	    $distribute = 1;
+	} elsif(/^\s*endchoice/) {
+	    $choice_activated = 0;
+	    $dist_string = "";
+	}
+
+	if($choice_activated && $distribute) {
+	    # do not put 'choice' inside of string to distribute
+	    if($state eq "CHOICE") {
+		    $state = "NONE";
+	    } else {
+		$dist_string .= "$_\n";
+	    }
 	}
     }
     close($kinfile);
-- 
2.43.0

Re: [PATCH 5/7] linux-kbuild: fix: implement choice for kconfigs

[Masahiro Yamada]

On Sat, Sep 14, 2024 at 2:12 AM David Hunter
<david.hunter.linux@gmail.com> wrote:
>
> Properly implement the config entries that are within the choice keyword
> for kconfig. Currently, the script only stops the previous config entry
> when a choice keyword is encountered.
>
> When the keyword "choice" is encountered, do the following:
>         - distribute the lines immediately following the "choice"
>           keyword to each config entry inside the "choice" section.
>         - process the config entries with the distributed lines.
>
> Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
> ---
>  scripts/kconfig/streamline_config.pl | 40 ++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 26e544744579..593df824ead7 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -162,6 +162,10 @@ sub read_kconfig {
>
>      my $source = "$ksource/$kconfig";
>      my $last_source = "";
> +    my $choice_activated = 0;
> +    my $distribute = 0;
> +    my $dist_string;
> +
>
>      # Check for any environment variables used
>      while ($source =~ /\$\((\w+)\)/ && $last_source ne $source) {
> @@ -214,6 +218,19 @@ sub read_kconfig {
>                 $state = "DEP";
>             }
>
> +           if($choice_activated) {
> +               $distribute = 0;
> +               my $config_lines = "$_\n" . "$dist_string";
> +               my $tmpconfig = ".choice.kconfig";
> +               open (my $FH, '>', $tmpconfig);
> +               print $FH $config_lines;
> +               close($FH);
> +
> +               read_kconfig($tmpconfig);
> +               unlink($tmpconfig) or die "Can't delete $tmpconfig: $!\n";


This is ugly.
Please do not use the temp file.

I believe the only benefit to parse 'choice' block
is to propagate its 'depends on' down to member configs.

See how the 'if' statement is handled.











--
Best Regards
Masahiro Yamada

Re: [PATCH 5/7] linux-kbuild: fix: implement choice for kconfigs

[David Hunter]

On 9/23/24 23:46, Masahiro Yamada wrote:
> 
> This is ugly.
> Please do not use the temp file.
> 


Understood. I found a way to do it that is much more in line with the 
style of the script and it avoids using the temp file. Oddly enough it 
involves changing less lines of code as well. I will be submitting the 
changes soon. Right now, I am just finishing up the change logs for all 
the patches.

My question here is "Is there a general rule as to why the first version 
was bad?" For example, is it considered bad practice to make temporary 
files?

Thanks,
David Hunter

Re: [PATCH 5/7] linux-kbuild: fix: implement choice for kconfigs

[David Hunter]

Apologies, I was a little too quick with the send button on my previous 
email. I forgot that I also wanted to respond to this part as well.

On 9/23/24 23:46, Masahiro Yamada wrote:

> I believe the only benefit to parse 'choice' block
> is to propagate its 'depends on' down to member configs.

I am not quite sure this statement is correct. I definitely agree that 
the main reason to parse the "choice" block is to propagate the 
"depends"; however, I believe that there is also information for 
defaults as well as for prompts inside the "choice" blocks.


I made a shell script that reads all of the choice blocks in the various 
Kconfig files. I wanted to know how many of each type of information is 
in there. Here are the results:

  - select: 0
  - prompts: 152
  - defaults: 156
  - depends: 72

Also, I should note that while there are no selects inside of a choice, 
I am unaware if this will always be the case in the future.

Here is a link to the shell script that I made:

https://github.com/dshunter107/linux-tools/blob/main/check_propagation.sh

Thanks,
David Hunter

Re: [PATCH 5/7] linux-kbuild: fix: implement choice for kconfigs

[David Hunter]

version 2: 
https://lore.kernel.org/all/20241014141345.5680-6-david.hunter.linux@gmail.com/

[PATCH 6/7] linux-kbuild: fix: configs with defaults do not need a prompt

[David Hunter]

Ignore process select warnings for config entries that have a default
option. Some config entries have no prompt and nothing selects them, but
these config options are okay because they have a default option.

Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
---
 scripts/kconfig/streamline_config.pl | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 593df824ead7..948437aac535 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -144,6 +144,7 @@ my %selects;
 my %prompts;
 my %objects;
 my %config2kfile;
+my %defaults;
 my $var;
 my $iflevel = 0;
 my @ifdeps;
@@ -239,6 +240,7 @@ sub read_kconfig {
 	    $depends{$config} .= " " . $1;
 	} elsif (($state eq "DEP" || $state eq "NEW") && /^\s*def(_(bool|tristate)|ault)\s+(\S.*)$/) {
 	    my $dep = $3;
+            $defaults{$config} = 1 ;
 	    if ($dep !~ /^\s*(y|m|n)\s*$/) {
 		$dep =~ s/.*\sif\s+//;
 		$depends{$config} .= " " . $dep;
@@ -561,8 +563,16 @@ sub parse_config_selects
 
     # If no possible config selected this, then something happened.
     if (!defined($next_config)) {
-	print STDERR "WARNING: $config is required, but nothing in the\n";
-	print STDERR "  current config selects it.\n";
+
+	# Some config options have no prompt, and nothing selects them, but
+	# they stay turned on once the final checks for the configs
+	# are done. These configs have a default option, so turn off the
+	# warnings for configs with default options.
+	if(!defined($defaults{$config})) {
+	    print STDERR "WARNING: $config is required, but nothing in the\n";
+	    print STDERR "  current config selects it.\n";
+	}
+
 	return;
     }
 
-- 
2.43.0

Re: [PATCH 6/7] linux-kbuild: fix: configs with defaults do not need a prompt

[Masahiro Yamada]

Seems fine if you fix the subject prefix
and reword it in imperative mood.



On Sat, Sep 14, 2024 at 2:12 AM David Hunter
<david.hunter.linux@gmail.com> wrote:
>
> Ignore process select warnings for config entries that have a default
> option. Some config entries have no prompt and nothing selects them, but
> these config options are okay because they have a default option.
>
> Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
> ---
>  scripts/kconfig/streamline_config.pl | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 593df824ead7..948437aac535 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -144,6 +144,7 @@ my %selects;
>  my %prompts;
>  my %objects;
>  my %config2kfile;
> +my %defaults;
>  my $var;
>  my $iflevel = 0;
>  my @ifdeps;
> @@ -239,6 +240,7 @@ sub read_kconfig {
>             $depends{$config} .= " " . $1;
>         } elsif (($state eq "DEP" || $state eq "NEW") && /^\s*def(_(bool|tristate)|ault)\s+(\S.*)$/) {
>             my $dep = $3;
> +            $defaults{$config} = 1 ;
>             if ($dep !~ /^\s*(y|m|n)\s*$/) {
>                 $dep =~ s/.*\sif\s+//;
>                 $depends{$config} .= " " . $dep;
> @@ -561,8 +563,16 @@ sub parse_config_selects
>
>      # If no possible config selected this, then something happened.
>      if (!defined($next_config)) {
> -       print STDERR "WARNING: $config is required, but nothing in the\n";
> -       print STDERR "  current config selects it.\n";
> +
> +       # Some config options have no prompt, and nothing selects them, but
> +       # they stay turned on once the final checks for the configs
> +       # are done. These configs have a default option, so turn off the
> +       # warnings for configs with default options.
> +       if(!defined($defaults{$config})) {
> +           print STDERR "WARNING: $config is required, but nothing in the\n";
> +           print STDERR "  current config selects it.\n";
> +       }
> +
>         return;
>      }
>
> --
> 2.43.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 6/7] linux-kbuild: fix: configs with defaults do not need a prompt

[David Hunter]

version 2: 
https://lore.kernel.org/all/20241014141345.5680-4-david.hunter.linux@gmail.com/

[PATCH 7/7] linux-kbuild: fix: process config options set to "y"

[David Hunter]

The goal of "make localmodconfig" is to turn off modules that are not
necessary. Some modules are necessary because they are depended on by
config options set with a "y."

Process configs set to "y" so that the modules that are depended on
will not be turned off later.

Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
---
 scripts/kconfig/streamline_config.pl | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 948437aac535..762bf80408c7 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -466,6 +466,11 @@ foreach my $line (@config_file) {
 
     if (/(CONFIG_[$valid]*)=(m|y)/) {
 	$orig_configs{$1} = $2;
+	# all configs options set to 'y' need to be processed
+	if($2 eq "y") {
+            $configs{$1}= $2;
+        }
+
     }
 }
 
@@ -596,9 +601,11 @@ sub loop_depend {
       forloop:
 	foreach my $config (keys %configs) {
 
-	    # If this config is not a module, we do not need to process it
-	    if (defined($orig_configs{$config}) && $orig_configs{$config} ne "m") {
-		next forloop;
+           # If this config is not set in the original config,
+	    # we do not need to process it
+           if (defined($orig_configs{$config}) && $orig_configs{$config} ne "m"
+		    	&& $orig_configs{$config} ne "y")  {
+		    next forloop;
 	    }
 
 	    $config =~ s/^CONFIG_//;
-- 
2.43.0

Re: [PATCH 7/7] linux-kbuild: fix: process config options set to "y"

[Masahiro Yamada]

On Sat, Sep 14, 2024 at 2:13 AM David Hunter
<david.hunter.linux@gmail.com> wrote:
>
> The goal of "make localmodconfig" is to turn off modules that are not
> necessary. Some modules are necessary because they are depended on by
> config options set with a "y."
>
> Process configs set to "y" so that the modules that are depended on
> will not be turned off later.


You need to put the explanation in the cover letter.

Without reading the cover-letter, I do not understand
why this change is needed.





> Signed-off-by: David Hunter <david.hunter.linux@gmail.com>
> ---
>  scripts/kconfig/streamline_config.pl | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 948437aac535..762bf80408c7 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -466,6 +466,11 @@ foreach my $line (@config_file) {
>
>      if (/(CONFIG_[$valid]*)=(m|y)/) {
>         $orig_configs{$1} = $2;
> +       # all configs options set to 'y' need to be processed
> +       if($2 eq "y") {
> +            $configs{$1}= $2;
> +        }
> +


You are breaking the indentation style.

Check how the current code is indented.
(tab and spaces).


A space is needed after 'if'
for coding style consistency.




>      }
>  }
>
> @@ -596,9 +601,11 @@ sub loop_depend {
>        forloop:
>         foreach my $config (keys %configs) {
>
> -           # If this config is not a module, we do not need to process it
> -           if (defined($orig_configs{$config}) && $orig_configs{$config} ne "m") {
> -               next forloop;
> +           # If this config is not set in the original config,
> +           # we do not need to process it
> +           if (defined($orig_configs{$config}) && $orig_configs{$config} ne "m"
> +                       && $orig_configs{$config} ne "y")  {
> +                   next forloop;
>             }



I do not understand the condition:


defined($orig_configs{$config}) && $orig_configs{$config} ne "m"
      && $orig_configs{$config} ne "y"


Is there any case when this condition is met?











>             $config =~ s/^CONFIG_//;
> --
> 2.43.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 7/7] linux-kbuild: fix: process config options set to "y"

[David Hunter]

> Is there any case when this condition is met?
> 
> 
On my computer, there are no cases where this condition is met. I am 
aware of config options that are numbers or are strings; however, I am 
unsure if any of them are dependencies for another config.

If you would like me to, I can make a script that can figure this out. 
Even if there are config options like this, though, I am not sure that 
skipping them would be the right thing to do. I think the best thing to 
do would be to remove this condition.

Let me know if you would like me to make a script to find out how many 
config entries are simultaneously not tristate and dependencies for 
other config entries. Also let me know if you think that I should leave 
the condition in.

My version 2 of this patch will not have the condtion in it.
Thanks,
David Hunter

Re: [PATCH 7/7] linux-kbuild: fix: process config options set to "y"

[David Hunter]

Version 2: 
https://lore.kernel.org/all/20241014141345.5680-7-david.hunter.linux@gmail.com/

Re:

[Shuah Khan]

On 9/13/24 11:11, David Hunter wrote:

Missing subject line for the cover-letter?
> 	
> Date: Fri, 13 Sep 2024 11:52:16 -0400
> Subject: [PATCH 0/7] linux-kbuild: fix: process configs set to "y"
> 
> An assumption made in this script is that the config options do not need
> to be processed because they will simply be in the new config file. This
> assumption is incorrect.
> 
> Process the config entries set to "y" because those config entries might
> have dependencies set to "m". If a config entry is set to "m" and is not
> loaded directly into the machine, the script will currently turn off
> that config entry; however, if that turned off config entry is a
> dependency for a "y" option. that means the config entry set to "y"
> will also be turned off later when the conf executive file is called.
> 
> Here is a model of the problem (arrows show dependency):
> 
> Original config file
> Config_1 (m) <-- Config_2 (y)
> 
> Config_1 is not loaded in this example, so it is turned off.
> After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf
> Config_1 (n) <-- Config_2 (y)
> 
> After  scripts/kconfig/conf
> Config_1 (n) <-- Config_2 (n)
> 
> 
> It should also be noted that any module in the dependency chain will
> also be turned off, even if that module is loaded directly onto the
> computer. Here is an example:
> 
> Original config file
> Config_1 (m) <-- Config_2 (y) <-- Config_3 (m)
> 
> Config_3 will be loaded in this example.
> After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf
> Config_1 (n) <-- Config_2 (y) <-- Config_3 (m)
> 
> After scripts/kconfig/conf
> Config_1 (n) <-- Config_2 (n) <-- Config_3 (n)
> 
> 
> I discovered this problem when I ran "make localmodconfig" on a generic
> Ubuntu config file. Many hardware devices were not recognized once the
> kernel was installed and booted. Another way to reproduced the error I
> had is to run "make localmodconfig" twice. The standard error might display
> warnings that certain modules should be selected but no config files are
> turned on that select that module.
> 
> With the changes in this series patch, all modules are loaded properly
> and all of the hardware is loaded when the kernel is installed and
> booted.
> 
> 
> David Hunter (7):
>    linux-kbuild: fix: config option can be bool
>    linux-kbuild: fix: missing variable operator
>    linux-kbuild: fix: ensure all defaults are tracked
>    linux-kbuild: fix: ensure selected configs were turned on in original
>    linux-kbuild: fix: implement choice for kconfigs
>    linux-kbuild: fix: configs with defaults do not need a prompt
>    linux-kbuild: fix: process config options set to "y"
> 
>   scripts/kconfig/streamline_config.pl | 77 ++++++++++++++++++++++++----
>   1 file changed, 66 insertions(+), 11 deletions(-)
>