choice =y selection becomes lost after having multiple entries =m with depends on

Hi,

in USB gadget menu (that is Device Drivers ---> USB support ---> USB Gadget Support ---> USB Gadget Drivers) I can create a configuration which is "lost". Here is how to reproduce it:

- first config two gadgets as M:
<M> USB Gadget Drivers

 <M>     Audio Gadget
 <M>     Ethernet Gadget
 <M>     MIDI Gadget

save config & leave

- now start menu config again and go to the same menu, now select built-in:
<*> USB Gadget Drivers (Ethernet Gadget
the ethernet gadget is chosen automatically because we can have only one gadget selected.
save config & leave

- step three, go back to the menu and you will see that everything is as it was (the <*> is ignored).

With only Audio & Ethernet =m and Midi =n then the switch to =y behaves differently: The menu looks correct (=y) but the .config keeps the Audio gadget as =M and it is built. Having multiple entries =m which have no "depends on" behaves correctly.

So the problem exists only if the item itself has a "depends on" line. Anyone aware of such kconfig limitation?

Sebastian, All,

So, I've tried your tests here, and indeed it does not behave as expected. Yet, I can observe a slight deviation from your observations:
the third time I enter the "USB Gadget Support" sub-menu, the "USB Gadget Drivers" choice is back to 'M', not 'Y'. But that's still considered an issue.

Quoted 2 lines

So the problem exists only if the item itself has a "depends on" line. Anyone aware of such kconfig limitation?

I'll try to come up with a smaller test-case to investigate, but since I'm at the LinuxCon Europe and ELCE this week (hey! Party tonight!), I won't have much time to look at it before the start of next week. Be sure to ping me later next week if you do not see any reply (and in case no one else solves the issue in the meantime, that is).

Thanks for this bug report.

Regards,
Yann E. MORIN.

Quoted 1 line · Yann E. MORIN

Sebastian, All,

Hi Yann,

Quoted 5 lines

So, I've tried your tests here, and indeed it does not behave as expected. Yet, I can observe a slight deviation from your observations:
the third time I enter the "USB Gadget Support" sub-menu, the "USB Gadget Drivers" choice is back to 'M', not 'Y'. But that's still considered an issue.

Yes, that is what I meant. Everything goes back M.

Quoted 8 lines

Quoted 2 lines

So the problem exists only if the item itself has a "depends on" line. Anyone aware of such kconfig limitation?

I'll try to come up with a smaller test-case to investigate, but since I'm at the LinuxCon Europe and ELCE this week (hey! Party tonight!), I won't have much time to look at it before the start of next week. Be sure to ping me later next week if you do not see any reply (and in case no one else solves the issue in the meantime, that is).

Okay, thanks.

Quoted 6 lines

Thanks for this bug report.

Regards,
Yann E. MORIN.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Sebastian,

I was looking at what you described and initially had a hard time to reproduce the problem, probably because I tried it after `make mrproper'. I am only able to reproduce the problem with an existing .config of my running kernel, for example.

Just to make sure that I am correctly trying to reproduce the problem:
did you already try to do what you described after a `make mrproper' and do you then also notice the described problems? If not, could you please try that?

Thanks,

Dirk

Quoted 1 line · Dirk Gouders

Hi Sebastian,

Hi Dirk,

Quoted 9 lines

I was looking at what you described and initially had a hard time to reproduce the problem, probably because I tried it after `make mrproper'. I am only able to reproduce the problem with an existing .config of my running kernel, for example.

Just to make sure that I am correctly trying to reproduce the problem:
did you already try to do what you described after a `make mrproper' and do you then also notice the described problems? If not, could you please try that?

What is the purpose behind mrproper?
The key to reproduce it is to have a .config with atleast two items within a choice statement which also have a "depends on" statement and those set to =m. If you don't have this after mrproper (with your fresh .config) then you don't see it.

Quoted 4 lines

Thanks,

Dirk

Sebastian

Quoted 1 line · Dirk Gouders

Hi Sebastian,

Hi Dirk,

Quoted 9 lines

I was looking at what you described and initially had a hard time to reproduce the problem, probably because I tried it after `make mrproper'. I am only able to reproduce the problem with an existing .config of my running kernel, for example.

Just to make sure that I am correctly trying to reproduce the problem:
did you already try to do what you described after a `make mrproper' and do you then also notice the described problems? If not, could you please try that?

What is the purpose behind mrproper?
The key to reproduce it is to have a .config with atleast two items within a choice statement which also have a "depends on" statement and those set to =m. If you don't have this after mrproper (with your fresh .config) then you don't see it.

Those values are also written to the config file causing modules when they should not.

2013/10/30 Dirk Gouders:

Quoted 2 lines

Those values are also written to the config file causing modules when they should not.

this sentence of the commit message is missing something, I think you mean:
s/causing modules/causing modules to be built/

Quoted 15 lines · Sebastian Andrzej Siewior

Quoted 1 line · Dirk Gouders

Hi Sebastian,

Hi Dirk,

Quoted 9 lines

I was looking at what you described and initially had a hard time to reproduce the problem, probably because I tried it after `make mrproper'. I am only able to reproduce the problem with an existing .config of my running kernel, for example.

Just to make sure that I am correctly trying to reproduce the problem:
did you already try to do what you described after a `make mrproper' and do you then also notice the described problems? If not, could you please try that?

Quoted 5 lines

What is the purpose behind mrproper?
The key to reproduce it is to have a .config with atleast two items within a choice statement which also have a "depends on" statement and those set to =m. If you don't have this after mrproper (with your fresh .config) then you don't see it.

Hi Sebastian, Yann, all

apologies for my previous misunderstanding.

I hope I now understood the problem and I tried to fix it with the attached patch. Could you please test the patch if it fixes the problem you noticed?

Another problem that I noticed is that if a choice is set to 'y', then I think the choice list should not include symbols that depend on symbols set to 'm', because they cannot be chosen, anyway. Well, this is rather confusing but does not produce real errors; I will see if I can fix that, too.

Yann, if you could have a look at and comment on my patch, I would be glad.

Dirk

From 3e555d59dd651366e14d6d6a47668acec3039fff Mon Sep 17 00:00:00 2001
From: Dirk Gouders <[email protected]>
Date: Wed, 30 Oct 2013 10:44:54 +0100
Subject: [PATCH] kconfig/symbol.c: handle choice_values that depend on 'm'
symbols

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules when they should not.

The following config can be used to reproduce and examine the problem:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch handles choice_values that depend on symbols set to 'm' if the corresponding choice is set to 'y'.

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

---
 scripts/kconfig/symbol.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

scripts/kconfig/symbol.c +22 -4

index c9a6775..043c041 100644
@@ -338,10 +338,27 @@ void sym_calc_value(struct symbol *sym)
 
 	switch (sym_get_type(sym)) {
 	case S_BOOLEAN:
-	case S_TRISTATE:
-		if (sym_is_choice_value(sym) && sym->visible == yes) {
-			prop = sym_get_choice_prop(sym);
-			newval.tri = (prop_get_symbol(prop)->curr.val == sym) ? yes : no;
+	case S_TRISTATE: {
+		struct symbol *choice_sym = NULL;
+
+		if (sym_is_choice_value(sym))
+			choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
+		/*
+		 * If this is a visible choice_value we want to check
+		 * if it is the currently selected, in two cases:
+		 *
+		 * 1) If it's visibility is 'yes'.
+		 * 2) If it's visibility is 'mod' and the correspondig
+		 *    choice symbols' value is 'yes'.
+		 *
+		 *    If a choice symbol is 'yes', only the selected
+		 *    choice_value may be 'yes' and all others (also
+		 *    those currently set to 'mod') must be set to 'no'.
+		 */
+		if (choice_sym &&
+		    (sym->visible == yes || (sym->visible == mod && choice_sym->curr.tri == yes))) {
+			    newval.tri = choice_sym->curr.val == sym ? yes : no;
 		} else {
 			if (sym->visible != no) {
 				/* if the symbol is visible use the user value

@@ -382,6 +399,7 @@ void sym_calc_value(struct symbol *sym)
 		if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN)
 			newval.tri = yes;
 		break;
+	}
 	case S_STRING:
 	case S_HEX:
 	case S_INT:

Hi Sebastian, Yann, all,

below is a patch that prevents choice_values to appear in the list if they depend on 'm' symbols and the choice symbol is set to 'y'. I would be glad if you could have a look at it.

Dirk

Dirk Gouders <[email protected]> writes:

below is a patch that prevents choice_values to appear in the list if they depend on 'm' symbols and the choice symbol is set to 'y'. I would be glad if you could have a look at it.

Yann, in this patch I wrote " if (choice_sym != NULL..." -- I guess that is one point that you will probably remark in the previous patch.

Another point is that all the conditionals in both patches could be

limited to only those choice_values that are of type tristate but I think that makes the code even harder to read...

From 677f5830588749cbb0bdb0568cbdaba271937c8d Mon Sep 17 00:00:00 2001
From: Dirk Gouders <[email protected]>
Date: Wed, 30 Oct 2013 15:06:05 +0100
Subject: [PATCH 2/2] kconfig/symbol.c: handle visibility of choice_values that
depend on 'm' symbols

If choice_values depend on symbols that are set to 'm' then these choice_values should not be visible in choice lists if the choice symbol is set to 'y'. See USB Gadget Drivers, for example.

Dirk, All,

On 2013-10-30 15:26 +0100, Dirk Gouders spake thusly:

Quoted 1 line

Dirk Gouders <[email protected]> writes:

[--SNIP--]

Quoted 3 lines

below is a patch that prevents choice_values to appear in the list if they depend on 'm' symbols and the choice symbol is set to 'y'. I would be glad if you could have a look at it.

Next time, could you use 'git send-email' to send your patches, it makes reviewing and commenting much simpler.

Also, it does not mangle the patch commit, and thus makes it easier to apply with 'git am'.

Quoted 2 lines

Yann, in this patch I wrote " if (choice_sym != NULL..." -- I guess that is one point that you will probably remark in the previous patch.

When we check that a pointer is valid, there's no reason to check it against NULL, just:

    if (choice_sym && choice_sym->...)

Quoted 1 line

Another point is that all the conditionals in both patches could be

I am not sure I understand what issue this patch(es) are supposed to fix.

What bothers me is that they touch two different places in the code, yet have the same subject, so it seems both are tryiong to fix the same issue.

Do you intend that both patches should be applied, or that this second one supersedes the previous one?

Quoted 2 lines

limited to only those choice_values that are of type tristate but I think that makes the code even harder to read...

Yes, and it does not matter since non-trisates are necessarily booleans, and those are covered since they will never be '== mod', so their visibility was, and still is, properly handled.

Quoted 9 lines

From 677f5830588749cbb0bdb0568cbdaba271937c8d Mon Sep 17 00:00:00 2001
From: Dirk Gouders <[email protected]>
Date: Wed, 30 Oct 2013 15:06:05 +0100
Subject: [PATCH 2/2] kconfig/symbol.c: handle visibility of choice_values that
depend on 'm' symbols

If choice_values depend on symbols that are set to 'm' then these choice_values should not be visible in choice lists if the choice symbol is set to 'y'. See USB Gadget Drivers, for example.

I liked your previous commit message better, because it had a test-case that did exhibit the problem.

Can you please:
- confirm which patch/es is/are needed
- update the commit log/s back with the test-case/s
- resend needed patch/es

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Quoted 8 lines

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Reported-by Sebastian Andrzej Siewior <[email protected]>

Still solves the issue for me :)

Sebastian, All,

Quoted 1 line

Still solves the issue for me :)

Should I consider this as:
Tested-by: <you>
?

Regards,
Yann E. MORIN.

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

scripts/kconfig/symbol.c +11 -0

index 22a3c40..32bbaa3 100644
@@ -188,12 +188,23 @@ static void sym_validate_range(struct symbol *sym)
 static void sym_calc_visibility(struct symbol *sym)
 {
 	struct property *prop;
+	struct symbol *choice_sym = NULL;
 	tristate tri;
 
 	/* any prompt visible? */
 	tri = no;
+
+	if (sym_is_choice_value(sym))
+		choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
 	for_all_prompts(sym, prop) {
 		prop->visible.tri = expr_calc_value(prop->visible.expr);
+		/*
+		 * choice_values with visibility 'mod' are not visible if the
+		 * corresponding choice's value is 'yes'.
+		 */
+		if (prop->visible.tri == mod && (choice_sym && choice_sym->curr.tri == yes))
+			prop->visible.tri = no;
 		tri = EXPR_OR(tri, prop->visible.tri);
 	}
 	if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))

Signature · 1 line

1.8.4

Dirk, All,

On 2013-11-01 00:39 +0100, Dirk Gouders spake thusly:

Quoted 28 lines

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

Quoted 7 lines

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

It seems I'm missing something here.

I just copy-pasted your example (test.in. below) and used it with current master (cset #be408cd) without your patch, and then ran:

    $ git clean -dX         # clean the tree
    $ make menuconfig       # Generate the frontend
      --> exit without saving
    $ ./scripts/kconfig/mconf test.in
      --> change the choice to 'y'
      --> do not change anything else
      --> exit and save

    $ cat .config
    CONFIG_modules=y
    CONFIG_dependency=m
    CONFIG_c0=y
    # CONFIG_c1 is not set

(.config header elided on purpose)
This looks like the expected output to me.

So I did further tests (still without your patch):

    $ git clean -dX         # clean the tree
    $ make menuconfig       # Generate the frontend
      --> exit without saving
    $ ./scripts/kconfig/mconf test.in
      --> change nothing
      --> exit and save

    $ cat .config
    CONFIG_modules=y
    CONFIG_dependency=m
    # CONFIG_c0 is not set
    # CONFIG_c1 is not set

    $ ./scripts/kconfig/mconf test.in
      --> change the choice to 'y'
      --> do not change anything else
      --> exit and save

    $ cat .config
    CONFIG_modules=y
    CONFIG_dependency=m
    CONFIG_c0=y
    # CONFIG_c1 is not set

Still the expected output, as far as I can see.

I can observe the exact same result with your patch applied. Ditto with kbuild/for-next from Michal's tree, with or without your patch.

So while I understand and can reproduce the original issue, and this patch indeed solves this original issue, the test-case above does not seem to completely illustrate the issue.

Are you sure this test-case exhibits the problem for you?

Anyway, I'm taking that in my tree locally, but that won't be part of for-next, since I'd like that we:
- either find a real reduced test-case,
- or just repeat the description from the original bug report

Needless to say that I'd prefer the former over the latter. Then I'll queue it as a post-rc1 fix.

Regards,
Yann E. MORIN.

Quoted 37 lines

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

scripts/kconfig/symbol.c +11 -0

index 22a3c40..32bbaa3 100644
@@ -188,12 +188,23 @@ static void sym_validate_range(struct symbol *sym)
 static void sym_calc_visibility(struct symbol *sym)
 {
 	struct property *prop;
+	struct symbol *choice_sym = NULL;
 	tristate tri;
 
 	/* any prompt visible? */
 	tri = no;
+
+	if (sym_is_choice_value(sym))
+		choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
 	for_all_prompts(sym, prop) {
 		prop->visible.tri = expr_calc_value(prop->visible.expr);
+		/*
+		 * choice_values with visibility 'mod' are not visible if the
+		 * corresponding choice's value is 'yes'.
+		 */
+		if (prop->visible.tri == mod && (choice_sym && choice_sym->curr.tri == yes))
+			prop->visible.tri = no;
 		tri = EXPR_OR(tri, prop->visible.tri);
 	}
 	if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))

Signature · 1 line

1.8.4

"Yann E. MORIN" <[email protected]> writes:

Quoted 1 line

It seems I'm missing something here.

Yes, but obviously, I did not describe it very clearly. The steps to reproduce the problem are:

     $ ./scripts/kconfig/mconf test.in
       --> change c0 and c1 to 'm'        # This is the missing part!

I spontaneously planned to answer with a modified config file with default values 'm' specified for 'c0' and 'c1' (complete file below) but I noticed that my latest patch does not help in that case. The first patch that modifies sym_calc_value() would handle it nicely but the latter one that modifies sym_calc_visibility() does not. The combination also does not work, because sym_calc_visibility() influences sym_calc_value().

So, I have to say that I am no longer really satisfied with the patch. It fixes the reported problem but I think it should fix related obvious problems as well (see config below). I'd prefer I take some more time and try to find a more sensible fix.

Dirk, All,

On 2013-11-06 15:43 +0100, Dirk Gouders spake thusly:

Quoted 1 line

"Yann E. MORIN" <[email protected]> writes:

[--SNIP--]

Quoted 1 line

Quoted 1 line

It seems I'm missing something here.

[--SNIP--]

Quoted 5 lines

Yes, but obviously, I did not describe it very clearly. The steps to reproduce the problem are:

     $ ./scripts/kconfig/mconf test.in
       --> change c0 and c1 to 'm'        # This is the missing part!

Aha! Gotcha. Thanks.

Quoted 8 lines

I spontaneously planned to answer with a modified config file with default values 'm' specified for 'c0' and 'c1' (complete file below) but I noticed that my latest patch does not help in that case. The first patch that modifies sym_calc_value() would handle it nicely but the latter one that modifies sym_calc_visibility() does not. The combination also does not work, because sym_calc_visibility() influences sym_calc_value().

Quoted 4 lines

So, I have to say that I am no longer really satisfied with the patch. It fixes the reported problem but I think it should fix related obvious problems as well (see config below). I'd prefer I take some more time and try to find a more sensible fix.

Please, one patch to fix one bug.

It does not matter if you need to touch the same part of the code, but please keep fixes for different bugs, separate (unless of course, the bugs are just different manifestations of the same deficiency in the code).

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

scripts/kconfig/symbol.c +11 -0

index c9a6775..06d96c9 100644
@@ -189,12 +189,23 @@ static void sym_validate_range(struct symbol *sym)
 static void sym_calc_visibility(struct symbol *sym)
 {
 	struct property *prop;
+	struct symbol *choice_sym = NULL;
 	tristate tri;
 
 	/* any prompt visible? */
 	tri = no;
+
+	if (sym_is_choice_value(sym))
+		choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
 	for_all_prompts(sym, prop) {
 		prop->visible.tri = expr_calc_value(prop->visible.expr);
+		/*
+		 * choice_values with visibility 'mod' are not visible if the
+		 * corresponding choice's value is 'yes'.
+		 */
+		if (prop->visible.tri == mod && (choice_sym && choice_sym->curr.tri == yes))
+			prop->visible.tri = no;
 		tri = EXPR_OR(tri, prop->visible.tri);
 	}
 	if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))

Signature · 1 line

1.8.4

Dirk, All,

Acked-by "Yann E. MORIN" <[email protected]>

It will be in my tree soon. Thanks!

Dirk, All,

On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:

Quoted 38 lines

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Quoted 3 lines

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

Acked-by "Yann E. MORIN" <[email protected]>

It will be in my tree soon. Thanks!

Regards,
Yann E. MORIN.

Quoted 33 lines

---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

scripts/kconfig/symbol.c +11 -1

index c9a6775..06d96c9 100644
@@ -189,12 +189,23 @@ static void sym_validate_range(struct symbol *sym)
 static void sym_calc_visibility(struct symbol *sym)
 {
      struct property *prop;
+     struct symbol *choice_sym = NULL;
      tristate tri;
 
      /* any prompt visible? */
      tri = no;
+
+     if (sym_is_choice_value(sym))
+             choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
      for_all_prompts(sym, prop) {
              prop->visible.tri = expr_calc_value(prop->visible.expr);
+             /*
+              * choice_values with visibility 'mod' are not visible if the
+              * corresponding choice's value is 'yes'.
+              */
+             if (prop->visible.tri == mod && (choice_sym && choice_sym->curr.tri == yes))
+                     prop->visible.tri = no;
              tri = EXPR_OR(tri, prop->visible.tri);
      }
      if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))
--

Quoted 2 lines

1.8.4

--
.-----------------.--------------------.------------------.--------------------.

|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |

'------------------------------^-------^------------------^--------------------' --
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

All,

Quoted 45 lines · Yann E. MORIN

Dirk, All,

On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:

Quoted 41 lines

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

Quoted 3 lines

Acked-by "Yann E. MORIN" <[email protected]>

It will be in my tree soon. Thanks!

I don't see this patch in 3.16 but 3.16 does not have the issue any more. Anyone has an idea how the issue got fixed? I am trying to find the right patch to backport.

Thanks,
-Bin.

Quoted 51 lines

Regards,
Yann E. MORIN.

Quoted 33 lines

---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

scripts/kconfig/symbol.c +11 -1

index c9a6775..06d96c9 100644
@@ -189,12 +189,23 @@ static void sym_validate_range(struct symbol *sym)
 static void sym_calc_visibility(struct symbol *sym)
 {
      struct property *prop;
+     struct symbol *choice_sym = NULL;
      tristate tri;
 
      /* any prompt visible? */
      tri = no;
+
+     if (sym_is_choice_value(sym))
+             choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
      for_all_prompts(sym, prop) {
              prop->visible.tri = expr_calc_value(prop->visible.expr);
+             /*
+              * choice_values with visibility 'mod' are not visible if the
+              * corresponding choice's value is 'yes'.
+              */
+             if (prop->visible.tri == mod && (choice_sym && choice_sym->curr.tri == yes))
+                     prop->visible.tri = no;
              tri = EXPR_OR(tri, prop->visible.tri);
      }
      if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))
--

Quoted 2 lines

1.8.4

--
.-----------------.--------------------.------------------.--------------------.

|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |

'------------------------------^-------^------------------^--------------------' --
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 50 lines · Bin Liu

All,

Quoted 47 lines · Yann E. MORIN

Dirk, All,

On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:

Quoted 38 lines

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Quoted 3 lines

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

Acked-by "Yann E. MORIN" <[email protected]>

Quoted 5 lines

Quoted 1 line

It will be in my tree soon. Thanks!

I don't see this patch in 3.16 but 3.16 does not have the issue any more. Anyone has an idea how the issue got fixed? I am trying to find the right patch to backport.

With the above sample kconfig I still see the issue. How did you notice the issue got fixed?

Dirk

Quoted 55 lines

Thanks,
-Bin.

Quoted 51 lines

Regards,
Yann E. MORIN.

Quoted 33 lines

---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

scripts/kconfig/symbol.c +11 -1

index c9a6775..06d96c9 100644
@@ -189,12 +189,23 @@ static void sym_validate_range(struct symbol *sym)
 static void sym_calc_visibility(struct symbol *sym)
 {
      struct property *prop;
+     struct symbol *choice_sym = NULL;
      tristate tri;
 
      /* any prompt visible? */
      tri = no;
+
+     if (sym_is_choice_value(sym))
+             choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
      for_all_prompts(sym, prop) {
              prop->visible.tri = expr_calc_value(prop->visible.expr);
+             /*
+              * choice_values with visibility 'mod' are not visible if the
+              * corresponding choice's value is 'yes'.
+              */
+             if (prop->visible.tri == mod && (choice_sym && choice_sym->curr.tri == yes))
+                     prop->visible.tri = no;
              tri = EXPR_OR(tri, prop->visible.tri);
      }
      if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))
--

Quoted 2 lines

1.8.4

--
.-----------------.--------------------.------------------.--------------------.

|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |

'------------------------------^-------^------------------^--------------------' --
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Dirk,

Quoted 58 lines · Dirk Gouders

Quoted 55 lines · Bin Liu

All,

Quoted 41 lines · Yann E. MORIN

Dirk, All,

On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:

Quoted 38 lines

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Quoted 7 lines

Quoted 3 lines

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

Acked-by "Yann E. MORIN" <[email protected]>

It will be in my tree soon. Thanks!

I don't see this patch in 3.16 but 3.16 does not have the issue any more. Anyone has an idea how the issue got fixed? I am trying to find the right patch to backport.

Quoted 2 lines

With the above sample kconfig I still see the issue. How did you notice the issue got fixed?

I did not pay much attention on the above sample kconfig, but just focused on the USB gadget driver kconfig issue initially reported by Sebastian. I saw the issue exists in 3.14, but does not in 3.16, unless I messed up with my test. I will test 3.16 again some time next week.

in USB gadget menu (that is Device Drivers ---> USB support ---> USB Gadget Support ---> USB Gadget Drivers) I can create a configuration which is "lost". Here is how to reproduce it:

- first config two gadgets as M:
<M> USB Gadget Drivers

 <M>     Audio Gadget
 <M>     Ethernet Gadget
 <M>     MIDI Gadget

save config & leave

- now start menu config again and go to the same menu, now select built-in:
<*> USB Gadget Drivers (Ethernet Gadget
the ethernet gadget is chosen automatically because we can have only one gadget selected.
save config & leave

- step three, go back to the menu and you will see that everything is as it was (the <*> is ignored).

Regards,
-Bin.

Quoted 58 lines

Dirk

Quoted 55 lines

Thanks,
-Bin.

Quoted 51 lines

Regards,
Yann E. MORIN.

Quoted 33 lines

---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

scripts/kconfig/symbol.c +11 -1

index c9a6775..06d96c9 100644
@@ -189,12 +189,23 @@ static void sym_validate_range(struct symbol *sym)
 static void sym_calc_visibility(struct symbol *sym)
 {
      struct property *prop;
+     struct symbol *choice_sym = NULL;
      tristate tri;
 
      /* any prompt visible? */
      tri = no;
+
+     if (sym_is_choice_value(sym))
+             choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
      for_all_prompts(sym, prop) {
              prop->visible.tri = expr_calc_value(prop->visible.expr);
+             /*
+              * choice_values with visibility 'mod' are not visible if the
+              * corresponding choice's value is 'yes'.
+              */
+             if (prop->visible.tri == mod && (choice_sym && choice_sym->curr.tri == yes))
+                     prop->visible.tri = no;
              tri = EXPR_OR(tri, prop->visible.tri);
      }
      if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))
--

Quoted 2 lines

1.8.4

--
.-----------------.--------------------.------------------.--------------------.

|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |

'------------------------------^-------^------------------^--------------------' --
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Here, I still see the problem (I was wondering if the issue has been solved/gone by a kconfig-file modification).

Perhaps, Sebastian could check if the problem, he reported still exists...

Dirk

Quoted 61 lines · Bin Liu

Dirk,

Quoted 58 lines · Dirk Gouders

Quoted 55 lines · Bin Liu

All,

Quoted 41 lines · Yann E. MORIN

Dirk, All,

On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:

Quoted 38 lines

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Quoted 7 lines

Quoted 3 lines

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

Acked-by "Yann E. MORIN" <[email protected]>

It will be in my tree soon. Thanks!

I don't see this patch in 3.16 but 3.16 does not have the issue any more. Anyone has an idea how the issue got fixed? I am trying to find the right patch to backport.

Quoted 8 lines

Quoted 2 lines

With the above sample kconfig I still see the issue. How did you notice the issue got fixed?

I did not pay much attention on the above sample kconfig, but just focused on the USB gadget driver kconfig issue initially reported by Sebastian. I saw the issue exists in 3.14, but does not in 3.16, unless I messed up with my test. I will test 3.16 again some time next week.

Hi Bin,

I now also re-tested the initially reported steps to reproduce the issue:

------------------------------------------------------------------------

Quoted 19 lines

in USB gadget menu (that is Device Drivers ---> USB support ---> USB Gadget Support ---> USB Gadget Drivers) I can create a configuration which is "lost". Here is how to reproduce it:

- first config two gadgets as M:
<M> USB Gadget Drivers

 <M>     Audio Gadget
 <M>     Ethernet Gadget
 <M>     MIDI Gadget

save config & leave

- now start menu config again and go to the same menu, now select built-in:
<*> USB Gadget Drivers (Ethernet Gadget
the ethernet gadget is chosen automatically because we can have only one gadget selected.
save config & leave

Quoted 2 lines

- step three, go back to the menu and you will see that everything is as it was (the <*> is ignored).

------------------------------------------------------------------------

Here, I still see the problem (I was wondering if the issue has been solved/gone by a kconfig-file modification).

Hi,

Quoted 99 lines · Dirk Gouders

Quoted 61 lines · Bin Liu

Dirk,

Quoted 58 lines · Dirk Gouders

Quoted 55 lines · Bin Liu

All,

Quoted 41 lines · Yann E. MORIN

Dirk, All,

On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:

Quoted 38 lines

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Quoted 7 lines

Quoted 3 lines

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

Acked-by "Yann E. MORIN" <[email protected]>

It will be in my tree soon. Thanks!

I don't see this patch in 3.16 but 3.16 does not have the issue any more. Anyone has an idea how the issue got fixed? I am trying to find the right patch to backport.

Quoted 8 lines

Quoted 2 lines

With the above sample kconfig I still see the issue. How did you notice the issue got fixed?

I did not pay much attention on the above sample kconfig, but just focused on the USB gadget driver kconfig issue initially reported by Sebastian. I saw the issue exists in 3.14, but does not in 3.16, unless I messed up with my test. I will test 3.16 again some time next week.

Hi Bin,

I now also re-tested the initially reported steps to reproduce the issue:

------------------------------------------------------------------------

Quoted 21 lines

in USB gadget menu (that is Device Drivers ---> USB support ---> USB Gadget Support ---> USB Gadget Drivers) I can create a configuration which is "lost". Here is how to reproduce it:

- first config two gadgets as M:
<M> USB Gadget Drivers

 <M>     Audio Gadget
 <M>     Ethernet Gadget
 <M>     MIDI Gadget

save config & leave

- now start menu config again and go to the same menu, now select built-in:
<*> USB Gadget Drivers (Ethernet Gadget
the ethernet gadget is chosen automatically because we can have only one gadget selected.
save config & leave

- step three, go back to the menu and you will see that everything is as it was (the <*> is ignored).

Quoted 4 lines

------------------------------------------------------------------------

Here, I still see the problem (I was wondering if the issue has been solved/gone by a kconfig-file modification).

This issue was gone since 3.16, but came back again due to commit 1fd6d08 ARM: omap2plus_defconfig: Enable n900 modem as loadable modules.

Hi,

Quoted 104 lines · Bin Liu

Hi,

Quoted 101 lines · Dirk Gouders

Quoted 61 lines · Bin Liu

Dirk,

Quoted 58 lines · Dirk Gouders

Quoted 55 lines · Bin Liu

All,

Quoted 41 lines · Yann E. MORIN

Dirk, All,

On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:

Quoted 38 lines

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Quoted 7 lines

Quoted 3 lines

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

Acked-by "Yann E. MORIN" <[email protected]>

It will be in my tree soon. Thanks!

I don't see this patch in 3.16 but 3.16 does not have the issue any more. Anyone has an idea how the issue got fixed? I am trying to find the right patch to backport.

Quoted 8 lines

Quoted 2 lines

With the above sample kconfig I still see the issue. How did you notice the issue got fixed?

I did not pay much attention on the above sample kconfig, but just focused on the USB gadget driver kconfig issue initially reported by Sebastian. I saw the issue exists in 3.14, but does not in 3.16, unless I messed up with my test. I will test 3.16 again some time next week.

Hi Bin,

I now also re-tested the initially reported steps to reproduce the issue:

------------------------------------------------------------------------

Quoted 19 lines

in USB gadget menu (that is Device Drivers ---> USB support ---> USB Gadget Support ---> USB Gadget Drivers) I can create a configuration which is "lost". Here is how to reproduce it:

- first config two gadgets as M:
<M> USB Gadget Drivers

 <M>     Audio Gadget
 <M>     Ethernet Gadget
 <M>     MIDI Gadget

save config & leave

- now start menu config again and go to the same menu, now select built-in:
<*> USB Gadget Drivers (Ethernet Gadget
the ethernet gadget is chosen automatically because we can have only one gadget selected.
save config & leave

Quoted 2 lines

- step three, go back to the menu and you will see that everything is as it was (the <*> is ignored).

------------------------------------------------------------------------

Quoted 6 lines

Quoted 2 lines

Here, I still see the problem (I was wondering if the issue has been solved/gone by a kconfig-file modification).

This issue was gone since 3.16, but came back again due to commit 1fd6d08 ARM: omap2plus_defconfig: Enable n900 modem as loadable modules.

I can confirm this issue too, faced it on v4.5 (but didn't try v4.6-rc1 yet)

Quoted 110 lines · Ruslan Bilovol

Hi,

Quoted 107 lines · Bin Liu

Hi,

Quoted 103 lines · Dirk Gouders

Quoted 61 lines · Bin Liu

Dirk,

Quoted 58 lines · Dirk Gouders

Quoted 55 lines · Bin Liu

All,

Quoted 41 lines · Yann E. MORIN

Dirk, All,

On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:

Quoted 38 lines

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Quoted 7 lines

Quoted 3 lines

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

Acked-by "Yann E. MORIN" <[email protected]>

It will be in my tree soon. Thanks!

I don't see this patch in 3.16 but 3.16 does not have the issue any more. Anyone has an idea how the issue got fixed? I am trying to find the right patch to backport.

Quoted 8 lines

Quoted 2 lines

With the above sample kconfig I still see the issue. How did you notice the issue got fixed?

I did not pay much attention on the above sample kconfig, but just focused on the USB gadget driver kconfig issue initially reported by Sebastian. I saw the issue exists in 3.14, but does not in 3.16, unless I messed up with my test. I will test 3.16 again some time next week.

Hi Bin,

I now also re-tested the initially reported steps to reproduce the issue:

------------------------------------------------------------------------

Quoted 19 lines

in USB gadget menu (that is Device Drivers ---> USB support ---> USB Gadget Support ---> USB Gadget Drivers) I can create a configuration which is "lost". Here is how to reproduce it:

- first config two gadgets as M:
<M> USB Gadget Drivers

 <M>     Audio Gadget
 <M>     Ethernet Gadget
 <M>     MIDI Gadget

save config & leave

- now start menu config again and go to the same menu, now select built-in:
<*> USB Gadget Drivers (Ethernet Gadget
the ethernet gadget is chosen automatically because we can have only one gadget selected.
save config & leave

Quoted 2 lines

- step three, go back to the menu and you will see that everything is as it was (the <*> is ignored).

------------------------------------------------------------------------

Here, I still see the problem (I was wondering if the issue has been solved/gone by a kconfig-file modification).

Quoted 6 lines

Quoted 3 lines

This issue was gone since 3.16, but came back again due to commit 1fd6d08 ARM: omap2plus_defconfig: Enable n900 modem as loadable modules.

I can confirm this issue too, faced it on v4.5 (but didn't try v4.6-rc1 yet)

Issue is present on v4.6-rc1 as well and the $subject patch fixes the issue.

Quoted 118 lines · Roger Quadros

Quoted 116 lines · Ruslan Bilovol

Hi,

Quoted 102 lines · Bin Liu

Hi,

Quoted 99 lines · Dirk Gouders

Quoted 61 lines · Bin Liu

Dirk,

Quoted 58 lines · Dirk Gouders

Quoted 55 lines · Bin Liu

All,

Quoted 41 lines · Yann E. MORIN

Dirk, All,

On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:

Quoted 38 lines

If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Quoted 7 lines

Quoted 3 lines

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

Acked-by "Yann E. MORIN" <[email protected]>

It will be in my tree soon. Thanks!

I don't see this patch in 3.16 but 3.16 does not have the issue any more. Anyone has an idea how the issue got fixed? I am trying to find the right patch to backport.

Quoted 8 lines

Quoted 2 lines

With the above sample kconfig I still see the issue. How did you notice the issue got fixed?

I did not pay much attention on the above sample kconfig, but just focused on the USB gadget driver kconfig issue initially reported by Sebastian. I saw the issue exists in 3.14, but does not in 3.16, unless I messed up with my test. I will test 3.16 again some time next week.

Hi Bin,

I now also re-tested the initially reported steps to reproduce the issue:

------------------------------------------------------------------------

Quoted 21 lines

in USB gadget menu (that is Device Drivers ---> USB support ---> USB Gadget Support ---> USB Gadget Drivers) I can create a configuration which is "lost". Here is how to reproduce it:

- first config two gadgets as M:
<M> USB Gadget Drivers

 <M>     Audio Gadget
 <M>     Ethernet Gadget
 <M>     MIDI Gadget

save config & leave

- now start menu config again and go to the same menu, now select built-in:
<*> USB Gadget Drivers (Ethernet Gadget
the ethernet gadget is chosen automatically because we can have only one gadget selected.
save config & leave

- step three, go back to the menu and you will see that everything is as it was (the <*> is ignored).

Quoted 8 lines

Quoted 4 lines

------------------------------------------------------------------------

Here, I still see the problem (I was wondering if the issue has been solved/gone by a kconfig-file modification).

This issue was gone since 3.16, but came back again due to commit 1fd6d08 ARM: omap2plus_defconfig: Enable n900 modem as loadable modules.

I can confirm this issue too, faced it on v4.5 (but didn't try v4.6-rc1 yet)

Quoted 1 line

Issue is present on v4.6-rc1 as well and the $subject patch fixes the issue.

I would say the issue is present until the code is fixed. Whether it is triggered depends on the Kconfig used.

Perhaps, Michal could take a look at the patch that Yann ack'ed, some time ago.

Hi,

[auto build test ERROR on v4.6-rc4]
[also build test ERROR on next-20160420]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Dirk-Gouders/kconfig-symbol-c-handle-choice_values-that-depend-on-m-symbols/20160420-182514
config: i386-randconfig-i0-201616 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

drivers/usb/gadget/legacy/dbgp.c: In function 'dbgp_disconnect':

Quoted 1 line

Quoted 1 line

drivers/usb/gadget/legacy/dbgp.c:213:25: error: 'struct dbgp' has no member named 'serial'

     gserial_disconnect(dbgp.serial);
                            ^
   drivers/usb/gadget/legacy/dbgp.c: In function 'dbgp_setup':
   drivers/usb/gadget/legacy/dbgp.c:374:29: error: 'struct dbgp' has no member named 'serial'
      err = gserial_connect(dbgp.serial, tty_line);
                                ^

Quoted 1 line

Quoted 1 line

drivers/usb/gadget/legacy/dbgp.c:374:38: error: 'tty_line' undeclared (first use in this function)

      err = gserial_connect(dbgp.serial, tty_line);
                                         ^

drivers/usb/gadget/legacy/dbgp.c:374:38: note: each undeclared identifier is reported only once for each function it appears in

vim +213 drivers/usb/gadget/legacy/dbgp.c

f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  207  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  208  static void dbgp_disconnect(struct usb_gadget *gadget)
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  209  {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  210  #ifdef CONFIG_USB_G_DBGP_PRINTK
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  211  	dbgp_disable_ep();
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  212  #else
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12 @213  	gserial_disconnect(dbgp.serial);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  214  #endif
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  215  }
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  216  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  217  static void dbgp_unbind(struct usb_gadget *gadget)
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  218  {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  219  #ifdef CONFIG_USB_G_DBGP_SERIAL
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  220  	kfree(dbgp.serial);
4958cf32 drivers/usb/gadget/legacy/dbgp.c Alexey Khoroshilov        2014-08-10  221  	dbgp.serial = NULL;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  222  #endif
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  223  	if (dbgp.req) {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  224  		kfree(dbgp.req->buf);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  225  		usb_ep_free_request(gadget->ep0, dbgp.req);
4958cf32 drivers/usb/gadget/legacy/dbgp.c Alexey Khoroshilov        2014-08-10  226  		dbgp.req = NULL;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  227  	}
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  228  }
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  229  
19b10a88 drivers/usb/gadget/dbgp.c        Sebastian Andrzej Siewior 2012-12-23  230  #ifdef CONFIG_USB_G_DBGP_SERIAL
19b10a88 drivers/usb/gadget/dbgp.c        Sebastian Andrzej Siewior 2012-12-23  231  static unsigned char tty_line;
19b10a88 drivers/usb/gadget/dbgp.c        Sebastian Andrzej Siewior 2012-12-23  232  #endif
19b10a88 drivers/usb/gadget/dbgp.c        Sebastian Andrzej Siewior 2012-12-23  233  
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  234  static int dbgp_configure_endpoints(struct usb_gadget *gadget)
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  235  {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  236  	int stp;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  237  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  238  	usb_ep_autoconfig_reset(gadget);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  239  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  240  	dbgp.i_ep = usb_ep_autoconfig(gadget, &i_desc);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  241  	if (!dbgp.i_ep) {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  242  		stp = 1;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  243  		goto fail_1;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  244  	}
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  245  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  246  	i_desc.wMaxPacketSize =
b8464bcf drivers/usb/gadget/legacy/dbgp.c Vaishali Thakkar          2015-06-06  247  		cpu_to_le16(USB_DEBUG_MAX_PACKET_SIZE);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  248  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  249  	dbgp.o_ep = usb_ep_autoconfig(gadget, &o_desc);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  250  	if (!dbgp.o_ep) {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  251  		stp = 2;
4ce86bfa drivers/usb/gadget/legacy/dbgp.c Robert Baldyga            2015-09-16  252  		goto fail_1;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  253  	}
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  254  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  255  	o_desc.wMaxPacketSize =
b8464bcf drivers/usb/gadget/legacy/dbgp.c Vaishali Thakkar          2015-06-06  256  		cpu_to_le16(USB_DEBUG_MAX_PACKET_SIZE);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  257  
a8779ee9 drivers/usb/gadget/dbgp.c        Sven Schnelle             2011-03-23  258  	dbg_desc.bDebugInEndpoint = i_desc.bEndpointAddress;
a8779ee9 drivers/usb/gadget/dbgp.c        Sven Schnelle             2011-03-23  259  	dbg_desc.bDebugOutEndpoint = o_desc.bEndpointAddress;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  260  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  261  #ifdef CONFIG_USB_G_DBGP_SERIAL
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  262  	dbgp.serial->in = dbgp.i_ep;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  263  	dbgp.serial->out = dbgp.o_ep;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  264  
72c973dd drivers/usb/gadget/dbgp.c        Tatyana Brokhman          2011-06-28  265  	dbgp.serial->in->desc = &i_desc;
72c973dd drivers/usb/gadget/dbgp.c        Tatyana Brokhman          2011-06-28  266  	dbgp.serial->out->desc = &o_desc;
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  267  #endif
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  268  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  269  	return 0;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  270  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  271  fail_1:
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  272  	dev_dbg(&dbgp.gadget->dev, "ep config: failure (%d)\n", stp);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  273  	return -ENODEV;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  274  }
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  275  
c94e289f drivers/usb/gadget/legacy/dbgp.c Arnd Bergmann             2015-04-11  276  static int dbgp_bind(struct usb_gadget *gadget,
ffe0b335 drivers/usb/gadget/dbgp.c        Sebastian Andrzej Siewior 2012-09-07  277  		struct usb_gadget_driver *driver)
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  278  {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  279  	int err, stp;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  280  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  281  	dbgp.gadget = gadget;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  282  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  283  	dbgp.req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  284  	if (!dbgp.req) {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  285  		err = -ENOMEM;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  286  		stp = 1;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  287  		goto fail;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  288  	}
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  289  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  290  	dbgp.req->buf = kmalloc(DBGP_REQ_EP0_LEN, GFP_KERNEL);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  291  	if (!dbgp.req->buf) {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  292  		err = -ENOMEM;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  293  		stp = 2;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  294  		goto fail;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  295  	}
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  296  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  297  	dbgp.req->length = DBGP_REQ_EP0_LEN;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  298  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  299  #ifdef CONFIG_USB_G_DBGP_SERIAL
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  300  	dbgp.serial = kzalloc(sizeof(struct gserial), GFP_KERNEL);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  301  	if (!dbgp.serial) {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  302  		stp = 3;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  303  		err = -ENOMEM;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  304  		goto fail;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  305  	}
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  306  
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  307  	if (gserial_alloc_line(&tty_line)) {
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  308  		stp = 4;
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  309  		err = -ENODEV;
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  310  		goto fail;
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  311  	}
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  312  #endif
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  313  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  314  	err = dbgp_configure_endpoints(gadget);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  315  	if (err < 0) {
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  316  		stp = 5;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  317  		goto fail;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  318  	}
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  319  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  320  	dev_dbg(&dbgp.gadget->dev, "bind: success\n");
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  321  	return 0;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  322  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  323  fail:
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  324  	dev_dbg(&gadget->dev, "bind: failure (%d:%d)\n", stp, err);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  325  	dbgp_unbind(gadget);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  326  	return err;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  327  }
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  328  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  329  static void dbgp_setup_complete(struct usb_ep *ep,
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  330  				struct usb_request *req)
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  331  {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  332  	dev_dbg(&dbgp.gadget->dev, "setup complete: %d, %d/%d\n",
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  333  		req->status, req->actual, req->length);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  334  }
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  335  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  336  static int dbgp_setup(struct usb_gadget *gadget,
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  337  		      const struct usb_ctrlrequest *ctrl)
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  338  {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  339  	struct usb_request *req = dbgp.req;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  340  	u8 request = ctrl->bRequest;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  341  	u16 value = le16_to_cpu(ctrl->wValue);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  342  	u16 length = le16_to_cpu(ctrl->wLength);
1744020c drivers/usb/gadget/dbgp.c        Sven Schnelle             2011-03-23  343  	int err = -EOPNOTSUPP;
1744020c drivers/usb/gadget/dbgp.c        Sven Schnelle             2011-03-23  344  	void *data = NULL;
1744020c drivers/usb/gadget/dbgp.c        Sven Schnelle             2011-03-23  345  	u16 len = 0;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  346  
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  347  	if (request == USB_REQ_GET_DESCRIPTOR) {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  348  		switch (value>>8) {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  349  		case USB_DT_DEVICE:
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  350  			dev_dbg(&dbgp.gadget->dev, "setup: desc device\n");
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  351  			len = sizeof device_desc;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  352  			data = &device_desc;
765f5b83 drivers/usb/gadget/dbgp.c        Sebastian Andrzej Siewior 2011-06-23  353  			device_desc.bMaxPacketSize0 = gadget->ep0->maxpacket;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  354  			break;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  355  		case USB_DT_DEBUG:
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  356  			dev_dbg(&dbgp.gadget->dev, "setup: desc debug\n");
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  357  			len = sizeof dbg_desc;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  358  			data = &dbg_desc;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  359  			break;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  360  		default:
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  361  			goto fail;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  362  		}
1744020c drivers/usb/gadget/dbgp.c        Sven Schnelle             2011-03-23  363  		err = 0;
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  364  	} else if (request == USB_REQ_SET_FEATURE &&
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  365  		   value == USB_DEVICE_DEBUG_MODE) {
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  366  		dev_dbg(&dbgp.gadget->dev, "setup: feat debug\n");
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  367  #ifdef CONFIG_USB_G_DBGP_PRINTK
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  368  		err = dbgp_enable_ep();
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  369  #else
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  370  		err = dbgp_configure_endpoints(gadget);
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  371  		if (err < 0) {
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  372  			goto fail;
6876d58f drivers/usb/gadget/legacy/dbgp.c Kyösti Mälkki             2014-11-03  373  		}
19b10a88 drivers/usb/gadget/dbgp.c        Sebastian Andrzej Siewior 2012-12-23 @374  		err = gserial_connect(dbgp.serial, tty_line);
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  375  #endif
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  376  		if (err < 0)
f6c826a9 drivers/usb/gadget/dbgp.c        stephane duverger         2010-07-12  377  			goto fail;

:::::: The code at line 213 was first introduced by commit
:::::: f6c826a90055dd05905982f7a3f60e0bcaa0434e USB: EHCI Debug Port Device Gadget

:::::: TO: stephane duverger <[email protected]> :::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

If choices consist of choice_values of type tristate that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets tristate choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'.

This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

---
v5: This patch should handle tristate choice-values, only.

I assumed that only tristate choice-values can have visibility 'm', which was wrong: tristate dependencies can result in 'm' visibility.

So, add an explicit test if a symbol is of type tristate.

I am a bit unsure how to handle Tested-By credits when patches change substantially and left the credits untouched but new test reports are welcome.

---

 scripts/kconfig/symbol.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

scripts/kconfig/symbol.c +14 -0

index 25cf0c2..2432298 100644
@@ -209,12 +209,26 @@ static void sym_set_all_changed(void)
 static void sym_calc_visibility(struct symbol *sym)
 {
 	struct property *prop;
+	struct symbol *choice_sym = NULL;
 	tristate tri;
 
 	/* any prompt visible? */
 	tri = no;
+
+	if (sym_is_choice_value(sym))
+		choice_sym = prop_get_symbol(sym_get_choice_prop(sym));
+
 	for_all_prompts(sym, prop) {
 		prop->visible.tri = expr_calc_value(prop->visible.expr);
+		/*
+		 * Tristate choice_values with visibility 'mod' are
+		 * not visible if the corresponding choice's value is
+		 * 'yes'.
+		 */
+		if (choice_sym && sym->type == S_TRISTATE &&
+		    prop->visible.tri == mod && choice_sym->curr.tri == yes)
+			prop->visible.tri = no;
+
 		tri = EXPR_OR(tri, prop->visible.tri);
 	}
 	if (tri == mod && (sym->type != S_TRISTATE || modules_val == no))

Quoted 49 lines · Dirk Gouders

If choices consist of choice_values of type tristate that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not.

The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration:

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
prompt "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"

config choice1
tristate "Choice 1"
depends on dependency

endchoice

This patch sets tristate choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'.

This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file.

Reported-by Sebastian Andrzej Siewior <[email protected]>

Signed-off-by Dirk Gouders <[email protected]>

Tested-by Sebastian Andrzej Siewior <[email protected]>

---
v5: This patch should handle tristate choice-values, only.

I assumed that only tristate choice-values can have visibility 'm', which was wrong: tristate dependencies can result in 'm' visibility.

Quoted 5 lines

So, add an explicit test if a symbol is of type tristate.

I am a bit unsure how to handle Tested-By credits when patches change substantially and left the credits untouched but new test reports are welcome.

If you made a non cosmetic change old Tested-By tags aren't valid.

Quoted 2 lines

---

The USB gadget case works fine for me. Thanks :)

Tested-by Roger Quadros <[email protected]>

Quoted 1 line

Are you sure this test-case exhibits the problem for you?

Yes, but obviously, I did not describe it very clearly. The steps to reproduce the problem are:

     $ ./scripts/kconfig/mconf test.in
       --> change c0 and c1 to 'm'        # This is the missing part!
       --> change the choice to 'y'
       --> do not change anything else
       --> exit and save

I spontaneously planned to answer with a modified config file with default values 'm' specified for 'c0' and 'c1' (complete file below) but I noticed that my latest patch does not help in that case. The first patch that modifies sym_calc_value() would handle it nicely but the latter one that modifies sym_calc_visibility() does not. The combination also does not work, because sym_calc_visibility() influences sym_calc_value().

- Sample Kconfig -------------------------------------------------------

config modules
boolean modules
default y
option modules

config dependency
tristate "Dependency"
default m

choice
tristate "Tristate Choice"
default choice0

config choice0
tristate "Choice 0"
default m

config choice1
tristate "Choice 1"
depends on dependency
default m

endchoice

------------------------------------------------------------------------