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

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

[Sebastian Andrzej Siewior]

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

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

[Yann E. MORIN]

Sebastian, All,

On 2013-10-23 12:51 +0200, Sebastian Andrzej Siewior spake thusly:
> 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, 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.

> 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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

[Sebastian Andrzej Siewior]

On 10/23/2013 01:23 PM, Yann E. MORIN wrote:
> Sebastian, All,

Hi Yann,

> 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.

>> 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.

> 
> Thanks for this bug report.
> 
> Regards,
> Yann E. MORIN.
> 

Sebastian

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

[Dirk Gouders]

Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes:

> On 10/23/2013 01:23 PM, Yann E. MORIN wrote:
>> Sebastian, All,
>
> Hi Yann,
>
>> 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.

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

>>> 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.
>
>> 
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Sebastian Andrzej Siewior]

On 10/24/2013 05:30 PM, Dirk Gouders wrote:
> Hi Sebastian,

Hi Dirk,

> 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.

> 
> Thanks,
> 
> Dirk

Sebastian

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

[Dirk Gouders]

Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes:

> On 10/24/2013 05:30 PM, Dirk Gouders wrote:
>> Hi Sebastian,
>
> Hi Dirk,
>
>> 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.

Hi Sebastian,

what I meant was that you start all steps that you previously described
with one initial mrproper, i.e.:

1) make mrproper
2) make menuconfig and select the gadgets with 'm', then save and exit
3) make menuconfig and select 'y' for "USB Gadget Drivers", then save
   and exit.
4) Check for problems either with menuconfig or directly in .config

With that initial mrproper I was not able to reproduce the problem
and therefore I suspect some content of an existing .config causing
the problems.

Dirk

PS: Probably `make mrproper' can be replaced by `rm .config' but I tested
    it only once and cannot say for sure.)

>> 
>> Thanks,
>> 
>> Dirk
>
> Sebastian

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

[Dirk Gouders]

[-- Attachment #1: Type: text/plain, Size: 1535 bytes --]

Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes:

> On 10/24/2013 05:30 PM, Dirk Gouders wrote:
>> Hi Sebastian,
>
> Hi Dirk,
>
>> 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.

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


[-- Attachment #2: Patch --]
[-- Type: text/plain, Size: 2737 bytes --]

From 3e555d59dd651366e14d6d6a47668acec3039fff Mon Sep 17 00:00:00 2001
From: Dirk Gouders <dirk@gouders.net>
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 <bigeasy@linutronix.de>
Signed-off-by: Dirk Gouders <dirk@gouders.net>
---
 scripts/kconfig/symbol.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index c9a6775..043c041 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -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:
-- 
1.8.3.2

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

[Daniele Forsi]

2013/10/30 Dirk Gouders:

> 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/

-- 
Daniele Forsi

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

[Dirk Gouders]

Daniele Forsi <dforsi@gmail.com> writes:

> 2013/10/30 Dirk Gouders:
>
>> 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/

Yes, thanks, that is correct.

I will fix that in a v2 or perhaps Yann will do if he does not notice
other problems with the patch.

Dirk

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

[Dirk Gouders]

[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]

Dirk Gouders <dirk@gouders.net> writes:

> Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes:
>
>> On 10/24/2013 05:30 PM, Dirk Gouders wrote:
>>> Hi Sebastian,
>>
>> Hi Dirk,
>>
>>> 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.
>
> 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.

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.

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...

Dirk


[-- Attachment #2: Visibility Patch --]
[-- Type: text/plain, Size: 1591 bytes --]

From 677f5830588749cbb0bdb0568cbdaba271937c8d Mon Sep 17 00:00:00 2001
From: Dirk Gouders <dirk@gouders.net>
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.

Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-of-by: Dirk Gouders <dirk@gouders.net>
---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 043c041..fbabb80 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -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 != NULL && 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))
-- 
1.8.3.2


[-- Attachment #3: Type: text/plain, Size: 2984 bytes --]



> 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 <dirk@gouders.net>
> 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 <bigeasy@linutronix.de>
> Signed-off-by: Dirk Gouders <dirk@gouders.net>
> ---
>  scripts/kconfig/symbol.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index c9a6775..043c041 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -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:

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

[Sebastian Andrzej Siewior]

* Dirk Gouders | 2013-10-30 15:26:30 [+0100]:

>Hi Sebastian, Yann, all,
Hi Dirk,

>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.

I applied this patch (and ignored the other one in this thread) and the
problem Tomi Valkeinen noticed and I reported here seems to be gone.
Thank you very much.

>Dirk

Sebastian

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

[Yann E. MORIN]

Dirk, All,

On 2013-10-30 15:26 +0100, Dirk Gouders spake thusly:
> Dirk Gouders <dirk@gouders.net> writes:
[--SNIP--]
> 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'.

> 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->...)

> 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?

> 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.

> From 677f5830588749cbb0bdb0568cbdaba271937c8d Mon Sep 17 00:00:00 2001
> From: Dirk Gouders <dirk@gouders.net>
> 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

Thank you!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

[Dirk Gouders]

"Yann E. MORIN" <yann.morin.1998@free.fr> writes:

> Dirk, All,
>
> On 2013-10-30 15:26 +0100, Dirk Gouders spake thusly:
>> Dirk Gouders <dirk@gouders.net> writes:
> [--SNIP--]
>> 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'.
>
>> 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->...)
>
>> 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?
>
>> 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.
>
>> From 677f5830588749cbb0bdb0568cbdaba271937c8d Mon Sep 17 00:00:00 2001
>> From: Dirk Gouders <dirk@gouders.net>
>> 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

Thanks for the comments, Yann, and apologies for the confusion.
Patch v3 comes in a minute and hopefully in a satisfactory format.

You are right, both patches that I sent fix the same (reported) problem
but v2 seems to be more sensible to me, because it causes non-selectable
choices to not even appear in choice lists.  However, it uses the
side-effect or "natural consequence" that a tristate choice_value's
value is set to no in sym_calc_value() if it's visibility is no.  So,
this seems to be a bit subtle and I tried to address it in the new commit
message.

I probably noticed another problem with those tristate choices: if
a non-optional tristate choice is set to 'm', then the default value is
not selected if no other is and I think that is not correct, but I'd
prefer to hear if others also think that this is a problem that should
be fixed.

Dirk

[PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

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 <bigeasy@linutronix.de>
Signed-off-by: Dirk Gouders <dirk@gouders.net>
---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 22a3c40..32bbaa3 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -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))
-- 
1.8.4

Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Sebastian Andrzej Siewior]

* Dirk Gouders | 2013-11-01 00:39:46 [+0100]:

>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 <bigeasy@linutronix.de>

Still solves the issue for me :)

>Signed-off-by: Dirk Gouders <dirk@gouders.net>

Sebastian

Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Yann E. MORIN]

Sebastian, All,

On 2013-11-04 18:27 +0100, Sebastian Andrzej Siewior spake thusly:
> * Dirk Gouders | 2013-11-01 00:39:46 [+0100]:
> 
> >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 <bigeasy@linutronix.de>
> 
> Still solves the issue for me :)

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

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Sebastian Andrzej Siewior]

On 11/04/2013 09:46 PM, Yann E. MORIN wrote:
> Sebastian, All,

Hi Yann,

>> Still solves the issue for me :)
> 
> Should I consider this as:
>     Tested-by: <you>
> ?

Yes, please.
Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> 
> Regards,
> Yann E. MORIN.
> 

Sebastian

Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Yann E. MORIN]

Dirk, All,

On 2013-11-01 00:39 +0100, Dirk Gouders spake thusly:
> 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.

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.

> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Dirk Gouders <dirk@gouders.net>
> ---
>  scripts/kconfig/symbol.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 22a3c40..32bbaa3 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -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))
> -- 
> 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.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

"Yann E. MORIN" <yann.morin.1998@free.fr> writes:

> Dirk, All,
>
> On 2013-11-01 00:39 +0100, Dirk Gouders spake thusly:
>> 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.
>
> 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

This, by the way, is the other problem I mentioned earlier.
There is a default value defined for "Tristate Choice" and the way I
understand the kconfig language, CONFIG_c0 should be 'm' here.

But that is another issue it is just a nice example for what I tried to
describe earlier.

>     $ ./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?

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().

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.

Thanks for your review and testing, Yann.

Dirk

PS: Sebastian, I also want to say thank you to you for the testing so
    far!

- 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

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

> 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.
>
>> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> Signed-off-by: Dirk Gouders <dirk@gouders.net>
>> ---
>>  scripts/kconfig/symbol.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>> 
>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>> index 22a3c40..32bbaa3 100644
>> --- a/scripts/kconfig/symbol.c
>> +++ b/scripts/kconfig/symbol.c
>> @@ -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))
>> -- 
>> 1.8.4
>> 

Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Yann E. MORIN]

Dirk, All,

On 2013-11-06 15:43 +0100, Dirk Gouders spake thusly:
> "Yann E. MORIN" <yann.morin.1998@free.fr> writes:
[--SNIP--]
> > It seems I'm missing something here.
[--SNIP--]
> 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.

> 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.

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).

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

"Yann E. MORIN" <yann.morin.1998@free.fr> writes:

> Dirk, All,
>
> On 2013-11-06 15:43 +0100, Dirk Gouders spake thusly:
>> "Yann E. MORIN" <yann.morin.1998@free.fr> writes:
> [--SNIP--]
>> > It seems I'm missing something here.
> [--SNIP--]
>> 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.
>
>> 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.
>
> 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).

I understand.  I will send a v4 with a clearer description of the steps
needed to trigger the problem and also the added Tested-by: line, in
case you see a need for it.

The other two problems I mentioned are concerning default values of
tristate choices and I am quite confident that those fixes will touch
other parts of the code.

Dirk

[PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

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 <bigeasy@linutronix.de>
Signed-off-by: Dirk Gouders <dirk@gouders.net>
Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index c9a6775..06d96c9 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -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))
-- 
1.8.4

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Yann E. MORIN]

Dirk, All,

On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
> 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 <bigeasy@linutronix.de>
> Signed-off-by: Dirk Gouders <dirk@gouders.net>
> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

It will be in my tree soon. Thanks!

Regards,
Yann E. MORIN.

> ---
>  scripts/kconfig/symbol.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index c9a6775..06d96c9 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -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))
> -- 
> 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.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Sebastian Andrzej Siewior]

On 11/18/2013 07:08 PM, Yann E. MORIN wrote:
> Dirk, All,

Hi Yann,

> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> It will be in my tree soon. Thanks!

I've been looking at gitorious.org:linux-kconfig/linux-kconfig.git into
the yem/kconfig-for-next and kconfig-rc-fixes branch and haven't found
it. Didn't you push your tree out or do you use another git tree for
that?

Sebastian

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Bin Liu]

All,

On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Dirk, All,
>
> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>> 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 <bigeasy@linutronix.de>
>> Signed-off-by: Dirk Gouders <dirk@gouders.net>
>> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> 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.

>
> Regards,
> Yann E. MORIN.
>
>> ---
>>  scripts/kconfig/symbol.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>> index c9a6775..06d96c9 100644
>> --- a/scripts/kconfig/symbol.c
>> +++ b/scripts/kconfig/symbol.c
>> @@ -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))
>> --
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

Bin Liu <binmlist@gmail.com> writes:

> All,
>
> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>> Dirk, All,
>>
>> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>>> 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 <bigeasy@linutronix.de>
>>> Signed-off-by: Dirk Gouders <dirk@gouders.net>
>>> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>
>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>
>> 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

>
> Thanks,
> -Bin.
>
>>
>> Regards,
>> Yann E. MORIN.
>>
>>> ---
>>>  scripts/kconfig/symbol.c | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>> index c9a6775..06d96c9 100644
>>> --- a/scripts/kconfig/symbol.c
>>> +++ b/scripts/kconfig/symbol.c
>>> @@ -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))
>>> --
>>> 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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Bin Liu]

Dirk,

On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders <dirk@gouders.net> wrote:
> Bin Liu <binmlist@gmail.com> writes:
>
>> All,
>>
>> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>> Dirk, All,
>>>
>>> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>>>> 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 <bigeasy@linutronix.de>
>>>> Signed-off-by: Dirk Gouders <dirk@gouders.net>
>>>> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>>
>>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>
>>> 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?

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.

Regards,
-Bin.

>
> Dirk
>
>>
>> Thanks,
>> -Bin.
>>
>>>
>>> Regards,
>>> Yann E. MORIN.
>>>
>>>> ---
>>>>  scripts/kconfig/symbol.c | 11 +++++++++++
>>>>  1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>>> index c9a6775..06d96c9 100644
>>>> --- a/scripts/kconfig/symbol.c
>>>> +++ b/scripts/kconfig/symbol.c
>>>> @@ -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))
>>>> --
>>>> 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 majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

Bin Liu <binmlist@gmail.com> writes:

> Dirk,
>
> On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders <dirk@gouders.net> wrote:
>> Bin Liu <binmlist@gmail.com> writes:
>>
>>> All,
>>>
>>> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>> Dirk, All,
>>>>
>>>> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>>>>> 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 <bigeasy@linutronix.de>
>>>>> Signed-off-by: Dirk Gouders <dirk@gouders.net>
>>>>> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>>>
>>>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>>
>>>> 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?
>
> 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:

------------------------------------------------------------------------
> 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).
------------------------------------------------------------------------

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

>
> Regards,
> -Bin.
>
>>
>> Dirk
>>
>>>
>>> Thanks,
>>> -Bin.
>>>
>>>>
>>>> Regards,
>>>> Yann E. MORIN.
>>>>
>>>>> ---
>>>>>  scripts/kconfig/symbol.c | 11 +++++++++++
>>>>>  1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>>>> index c9a6775..06d96c9 100644
>>>>> --- a/scripts/kconfig/symbol.c
>>>>> +++ b/scripts/kconfig/symbol.c
>>>>> @@ -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))
>>>>> --
>>>>> 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 majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Sebastian Andrzej Siewior]

On 08/15/2014 09:37 AM, Dirk Gouders wrote:
> 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...

I will re-test tonight and report. I don't see the patch in this thread
(and I remeber that it fixed the issue) so I doubt that anything will
change but lets see.

> 
> Dirk

Sebastian

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Bin Liu]

Hi,

On Fri, Aug 15, 2014 at 2:37 AM, Dirk Gouders <dirk@gouders.net> wrote:
> Bin Liu <binmlist@gmail.com> writes:
>
>> Dirk,
>>
>> On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders <dirk@gouders.net> wrote:
>>> Bin Liu <binmlist@gmail.com> writes:
>>>
>>>> All,
>>>>
>>>> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>>> Dirk, All,
>>>>>
>>>>> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>>>>>> 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 <bigeasy@linutronix.de>
>>>>>> Signed-off-by: Dirk Gouders <dirk@gouders.net>
>>>>>> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>>>>
>>>>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>>>
>>>>> 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?
>>
>> 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:
>
> ------------------------------------------------------------------------
>> 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).
> ------------------------------------------------------------------------
>
> 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.

Regards,
-Bin.

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Ruslan Bilovol]

Hi,

On Thu, Mar 31, 2016 at 1:08 AM, Bin Liu <binmlist@gmail.com> wrote:
> Hi,
>
> On Fri, Aug 15, 2014 at 2:37 AM, Dirk Gouders <dirk@gouders.net> wrote:
>> Bin Liu <binmlist@gmail.com> writes:
>>
>>> Dirk,
>>>
>>> On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders <dirk@gouders.net> wrote:
>>>> Bin Liu <binmlist@gmail.com> writes:
>>>>
>>>>> All,
>>>>>
>>>>> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>>>> Dirk, All,
>>>>>>
>>>>>> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>>>>>>> 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 <bigeasy@linutronix.de>
>>>>>>> Signed-off-by: Dirk Gouders <dirk@gouders.net>
>>>>>>> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>>>>>
>>>>>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>>>>
>>>>>> 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?
>>>
>>> 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:
>>
>> ------------------------------------------------------------------------
>>> 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).
>> ------------------------------------------------------------------------
>>
>> 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)

-- 
Best regards,
Ruslan Bilovol

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Roger Quadros]

On 31/03/16 01:16, Ruslan Bilovol wrote:
> Hi,
> 
> On Thu, Mar 31, 2016 at 1:08 AM, Bin Liu <binmlist@gmail.com> wrote:
>> Hi,
>>
>> On Fri, Aug 15, 2014 at 2:37 AM, Dirk Gouders <dirk@gouders.net> wrote:
>>> Bin Liu <binmlist@gmail.com> writes:
>>>
>>>> Dirk,
>>>>
>>>> On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders <dirk@gouders.net> wrote:
>>>>> Bin Liu <binmlist@gmail.com> writes:
>>>>>
>>>>>> All,
>>>>>>
>>>>>> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>>>>> Dirk, All,
>>>>>>>
>>>>>>> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>>>>>>>> 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 <bigeasy@linutronix.de>
>>>>>>>> Signed-off-by: Dirk Gouders <dirk@gouders.net>
>>>>>>>> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>>>>>>
>>>>>>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>>>>>
>>>>>>> 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?
>>>>
>>>> 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:
>>>
>>> ------------------------------------------------------------------------
>>>> 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).
>>> ------------------------------------------------------------------------
>>>
>>> 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)
> 

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

cheers,
-roger

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

Roger Quadros <rogerq@ti.com> writes:

> On 31/03/16 01:16, Ruslan Bilovol wrote:
>> Hi,
>> 
>> On Thu, Mar 31, 2016 at 1:08 AM, Bin Liu <binmlist@gmail.com> wrote:
>>> Hi,
>>>
>>> On Fri, Aug 15, 2014 at 2:37 AM, Dirk Gouders <dirk@gouders.net> wrote:
>>>> Bin Liu <binmlist@gmail.com> writes:
>>>>
>>>>> Dirk,
>>>>>
>>>>> On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders <dirk@gouders.net> wrote:
>>>>>> Bin Liu <binmlist@gmail.com> writes:
>>>>>>
>>>>>>> All,
>>>>>>>
>>>>>>> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>>>>>> Dirk, All,
>>>>>>>>
>>>>>>>> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>>>>>>>>> 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 <bigeasy@linutronix.de>
>>>>>>>>> Signed-off-by: Dirk Gouders <dirk@gouders.net>
>>>>>>>>> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>>>>>>>
>>>>>>>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>>>>>>
>>>>>>>> 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?
>>>>>
>>>>> 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:
>>>>
>>>> ------------------------------------------------------------------------
>>>>> 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).
>>>> ------------------------------------------------------------------------
>>>>
>>>> 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)
>> 
>
> 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.

Dirk

Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

Dirk Gouders <dirk@gouders.net> writes:

> Roger Quadros <rogerq@ti.com> writes:
>
>> On 31/03/16 01:16, Ruslan Bilovol wrote:
>>> Hi,
>>> 
>>> On Thu, Mar 31, 2016 at 1:08 AM, Bin Liu <binmlist@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> On Fri, Aug 15, 2014 at 2:37 AM, Dirk Gouders <dirk@gouders.net> wrote:
>>>>> Bin Liu <binmlist@gmail.com> writes:
>>>>>
>>>>>> Dirk,
>>>>>>
>>>>>> On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders <dirk@gouders.net> wrote:
>>>>>>> Bin Liu <binmlist@gmail.com> writes:
>>>>>>>
>>>>>>>> All,
>>>>>>>>
>>>>>>>> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>>>>>>> Dirk, All,
>>>>>>>>>
>>>>>>>>> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>>>>>>>>>> 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 <bigeasy@linutronix.de>
>>>>>>>>>> Signed-off-by: Dirk Gouders <dirk@gouders.net>
>>>>>>>>>> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>>>>>>>>
>>>>>>>>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>>>>>>>
>>>>>>>>> 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?
>>>>>>
>>>>>> 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:
>>>>>
>>>>> ------------------------------------------------------------------------
>>>>>> 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).
>>>>> ------------------------------------------------------------------------
>>>>>
>>>>> 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)
>>> 
>>
>> 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.

Apologies if this sounded rude (I am not a native speaker).  What I
had in mind was that as far as I remember this patch was posted at the
time when Yann changed his interests and therefore it probably now needs
Michals attention.

Dirk

[RESEND PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

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 <bigeasy@linutronix.de>
Signed-off-by: Dirk Gouders <dirk@gouders.net>
Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 scripts/kconfig/symbol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index c9a6775..06d96c9 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -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))
-- 
1.8.4

Re: [RESEND PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 19801 bytes --]

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':
>> 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);
                                ^
>> 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 <stephane.duverger@gmail.com>
:::::: CC: Greg Kroah-Hartman <gregkh@suse.de>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 32283 bytes --]

Re: [RESEND PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

kbuild test robot <lkp@intel.com> writes:

> 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 

This problem is because, with this patch we now get a Problem if choices
are all boolean and all of them depend on a symbol set to 'm'.  In this
case none of the choices can be other than 'n'.  I'm not sure if we want
such a behavior.

In the case of dbgp we could probably fix the Kconfig file by removing
the dependency from the default choice (the choices are surrounded by
"if USB_G_DBGP", anyway).

Suggestions are very welcome.

Dirk

> All errors (new ones prefixed by >>):
>
>    drivers/usb/gadget/legacy/dbgp.c: In function 'dbgp_disconnect':
>>> 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);
>                                 ^
>>> 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 <stephane.duverger@gmail.com>
> :::::: CC: Greg Kroah-Hartman <gregkh@suse.de>
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH v5] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[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 <bigeasy@linutronix.de>
Signed-off-by: Dirk Gouders <dirk@gouders.net>
Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
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(+)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 25cf0c2..2432298 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -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))
-- 
2.8.1

Re: [PATCH v5] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Roger Quadros]

On 29/04/16 11:24, Dirk Gouders wrote:
> 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 <bigeasy@linutronix.de>
> Signed-off-by: Dirk Gouders <dirk@gouders.net>
> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> 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.

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

> 
> ---

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

Tested-by: Roger Quadros <rogerq@ti.com>

cheers,
-roger

>  scripts/kconfig/symbol.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 25cf0c2..2432298 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -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))
> 

Re: [PATCH v5] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Michal Marek]

On Mon, May 02, 2016 at 11:43:30AM +0300, Roger Quadros wrote:
> On 29/04/16 11:24, Dirk Gouders wrote:
> > 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 <bigeasy@linutronix.de>
> > Signed-off-by: Dirk Gouders <dirk@gouders.net>
> > Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> > 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.
> 
> If you made a non cosmetic change old Tested-By tags aren't valid.
> 
> > 
> > ---
> 
> The USB gadget case works fine for me. Thanks :)
> 
> Tested-by: Roger Quadros <rogerq@ti.com>

Applied to kbuild.git#kconfig.

Michal

Re: [RESEND PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2933 bytes --]

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: x86_64-randconfig-s0-04201943 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> fs/romfs/storage.c:18:2: error: #error no ROMFS backing store interface configured
    #error no ROMFS backing store interface configured
     ^

vim +18 fs/romfs/storage.c

da4458bd David Howells    2009-02-12   2   *
da4458bd David Howells    2009-02-12   3   * Copyright © 2007 Red Hat, Inc. All Rights Reserved.
da4458bd David Howells    2009-02-12   4   * Written by David Howells (dhowells@redhat.com)
da4458bd David Howells    2009-02-12   5   *
da4458bd David Howells    2009-02-12   6   * This program is free software; you can redistribute it and/or
da4458bd David Howells    2009-02-12   7   * modify it under the terms of the GNU General Public License
da4458bd David Howells    2009-02-12   8   * as published by the Free Software Foundation; either version
da4458bd David Howells    2009-02-12   9   * 2 of the License, or (at your option) any later version.
da4458bd David Howells    2009-02-12  10   */
da4458bd David Howells    2009-02-12  11  
da4458bd David Howells    2009-02-12  12  #include <linux/fs.h>
da4458bd David Howells    2009-02-12  13  #include <linux/mtd/super.h>
da4458bd David Howells    2009-02-12  14  #include <linux/buffer_head.h>
da4458bd David Howells    2009-02-12  15  #include "internal.h"
da4458bd David Howells    2009-02-12  16  
da4458bd David Howells    2009-02-12  17  #if !defined(CONFIG_ROMFS_ON_MTD) && !defined(CONFIG_ROMFS_ON_BLOCK)
da4458bd David Howells    2009-02-12 @18  #error no ROMFS backing store interface configured
da4458bd David Howells    2009-02-12  19  #endif
da4458bd David Howells    2009-02-12  20  
da4458bd David Howells    2009-02-12  21  #ifdef CONFIG_ROMFS_ON_MTD
c382fb43 Artem Bityutskiy 2012-01-30  22  #define ROMFS_MTD_READ(sb, ...) mtd_read((sb)->s_mtd, ##__VA_ARGS__)
da4458bd David Howells    2009-02-12  23  
da4458bd David Howells    2009-02-12  24  /*
da4458bd David Howells    2009-02-12  25   * read data from an romfs image on an MTD device
da4458bd David Howells    2009-02-12  26   */

:::::: The code at line 18 was first introduced by commit
:::::: da4458bda237aa0cb1688f6c359477f203788f6a NOMMU: Make it possible for RomFS to use MTD devices directly

:::::: TO: David Howells <dhowells@redhat.com>
:::::: CC: David Woodhouse <David.Woodhouse@intel.com>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 27393 bytes --]

Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Dirk Gouders]

Dirk Gouders <dirk@gouders.net> writes:

[SNIP]

>> 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().

[SNIP]

Hi Yann, all,

seems that I was a bit misleaded, here.  While looking at how to
possibly fix what I described, I realized that default values for
choice values are not supported and therfore there is no issue:

choices_kconfig:17:warning: defaults for choice values not supported
choices_kconfig:22:warning: defaults for choice values not supported

I noticed these warnings only accidently, when I was using an assert()
that caused an abort and prevented the output to stderr being hidden by
the ncurses output.  Perhaps I should redirect stderr to a file and
inspect it, in the future...

So, my concerns with my own patch were unsubstantiated.

Dirk


> - 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
>
> ------------------------------------------------------------------------