* choice =y selection becomes lost after having multiple entries =m with depends on
@ 2013-10-23 10:51 Sebastian Andrzej Siewior
2013-10-23 11:23 ` Yann E. MORIN
0 siblings, 1 reply; 42+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-23 10:51 UTC (permalink / raw)
To: Michal Marek
Cc: linux-kbuild, Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-23 10:51 choice =y selection becomes lost after having multiple entries =m with depends on Sebastian Andrzej Siewior
@ 2013-10-23 11:23 ` Yann E. MORIN
2013-10-23 11:28 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 42+ messages in thread
From: Yann E. MORIN @ 2013-10-23 11:23 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Michal Marek, linux-kbuild, Felipe Balbi, USB list,
Tomi Valkeinen, Roger Quadros
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-23 11:23 ` Yann E. MORIN
@ 2013-10-23 11:28 ` Sebastian Andrzej Siewior
2013-10-24 15:30 ` Dirk Gouders
0 siblings, 1 reply; 42+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-23 11:28 UTC (permalink / raw)
To: Yann E. MORIN
Cc: Michal Marek, linux-kbuild, Felipe Balbi, USB list,
Tomi Valkeinen, Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-23 11:28 ` Sebastian Andrzej Siewior
@ 2013-10-24 15:30 ` Dirk Gouders
2013-10-24 16:19 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 42+ messages in thread
From: Dirk Gouders @ 2013-10-24 15:30 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Yann E. MORIN, Michal Marek, linux-kbuild, Felipe Balbi,
USB list, Tomi Valkeinen, Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-24 15:30 ` Dirk Gouders
@ 2013-10-24 16:19 ` Sebastian Andrzej Siewior
2013-10-24 16:50 ` Dirk Gouders
2013-10-30 10:00 ` Dirk Gouders
0 siblings, 2 replies; 42+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-24 16:19 UTC (permalink / raw)
To: Dirk Gouders
Cc: Yann E. MORIN, Michal Marek, linux-kbuild, Felipe Balbi,
USB list, Tomi Valkeinen, Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-24 16:19 ` Sebastian Andrzej Siewior
@ 2013-10-24 16:50 ` Dirk Gouders
2013-10-30 10:00 ` Dirk Gouders
1 sibling, 0 replies; 42+ messages in thread
From: Dirk Gouders @ 2013-10-24 16:50 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Yann E. MORIN, Michal Marek, linux-kbuild, Felipe Balbi,
USB list, Tomi Valkeinen, Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-24 16:19 ` Sebastian Andrzej Siewior
2013-10-24 16:50 ` Dirk Gouders
@ 2013-10-30 10:00 ` Dirk Gouders
2013-10-30 10:30 ` Daniele Forsi
2013-10-30 14:26 ` Dirk Gouders
1 sibling, 2 replies; 42+ messages in thread
From: Dirk Gouders @ 2013-10-30 10:00 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Yann E. MORIN, Michal Marek, linux-kbuild, Felipe Balbi,
USB list, Tomi Valkeinen, Roger Quadros
[-- 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
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-30 10:00 ` Dirk Gouders
@ 2013-10-30 10:30 ` Daniele Forsi
2013-10-30 10:41 ` Dirk Gouders
2013-10-30 14:26 ` Dirk Gouders
1 sibling, 1 reply; 42+ messages in thread
From: Daniele Forsi @ 2013-10-30 10:30 UTC (permalink / raw)
To: Dirk Gouders
Cc: Sebastian Andrzej Siewior, Yann E. MORIN, Michal Marek,
linux-kbuild, Felipe Balbi, USB list, Tomi Valkeinen,
Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-30 10:30 ` Daniele Forsi
@ 2013-10-30 10:41 ` Dirk Gouders
0 siblings, 0 replies; 42+ messages in thread
From: Dirk Gouders @ 2013-10-30 10:41 UTC (permalink / raw)
To: Daniele Forsi
Cc: Sebastian Andrzej Siewior, Yann E. MORIN, Michal Marek,
linux-kbuild, Felipe Balbi, USB list, Tomi Valkeinen,
Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-30 10:00 ` Dirk Gouders
2013-10-30 10:30 ` Daniele Forsi
@ 2013-10-30 14:26 ` Dirk Gouders
2013-10-31 10:20 ` Sebastian Andrzej Siewior
2013-10-31 21:49 ` Yann E. MORIN
1 sibling, 2 replies; 42+ messages in thread
From: Dirk Gouders @ 2013-10-30 14:26 UTC (permalink / raw)
To: Dirk Gouders
Cc: Sebastian Andrzej Siewior, Yann E. MORIN, Michal Marek,
linux-kbuild, Felipe Balbi, USB list, Tomi Valkeinen,
Roger Quadros
[-- 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:
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-30 14:26 ` Dirk Gouders
@ 2013-10-31 10:20 ` Sebastian Andrzej Siewior
2013-10-31 21:49 ` Yann E. MORIN
1 sibling, 0 replies; 42+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-31 10:20 UTC (permalink / raw)
To: Dirk Gouders
Cc: Yann E. MORIN, Michal Marek, linux-kbuild, Felipe Balbi,
USB list, Tomi Valkeinen, Roger Quadros
* 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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-30 14:26 ` Dirk Gouders
2013-10-31 10:20 ` Sebastian Andrzej Siewior
@ 2013-10-31 21:49 ` Yann E. MORIN
2013-11-01 8:45 ` Dirk Gouders
1 sibling, 1 reply; 42+ messages in thread
From: Yann E. MORIN @ 2013-10-31 21:49 UTC (permalink / raw)
To: Dirk Gouders
Cc: Sebastian Andrzej Siewior, Michal Marek, linux-kbuild,
Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-01 8:45 ` Dirk Gouders
@ 2013-10-31 23:39 ` Dirk Gouders
2013-11-04 17:27 ` Sebastian Andrzej Siewior
2013-11-05 23:04 ` Yann E. MORIN
0 siblings, 2 replies; 42+ messages in thread
From: Dirk Gouders @ 2013-10-31 23:39 UTC (permalink / raw)
To: Yann E. MORIN
Cc: Sebastian Andrzej Siewior, Michal Marek, linux-kbuild,
Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
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
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: choice =y selection becomes lost after having multiple entries =m with depends on
2013-10-31 21:49 ` Yann E. MORIN
@ 2013-11-01 8:45 ` Dirk Gouders
2013-10-31 23:39 ` [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols Dirk Gouders
0 siblings, 1 reply; 42+ messages in thread
From: Dirk Gouders @ 2013-11-01 8:45 UTC (permalink / raw)
To: Yann E. MORIN
Cc: Sebastian Andrzej Siewior, Michal Marek, linux-kbuild,
Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
"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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-10-31 23:39 ` [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols Dirk Gouders
@ 2013-11-04 17:27 ` Sebastian Andrzej Siewior
2013-11-04 20:46 ` Yann E. MORIN
2013-11-05 23:04 ` Yann E. MORIN
1 sibling, 1 reply; 42+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-11-04 17:27 UTC (permalink / raw)
To: Dirk Gouders
Cc: Yann E. MORIN, Michal Marek, linux-kbuild, Felipe Balbi,
USB list, Tomi Valkeinen, Roger Quadros
* 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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-04 17:27 ` Sebastian Andrzej Siewior
@ 2013-11-04 20:46 ` Yann E. MORIN
2013-11-05 8:45 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 42+ messages in thread
From: Yann E. MORIN @ 2013-11-04 20:46 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Dirk Gouders, Michal Marek, linux-kbuild, Felipe Balbi, USB list,
Tomi Valkeinen, Roger Quadros
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-04 20:46 ` Yann E. MORIN
@ 2013-11-05 8:45 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 42+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-11-05 8:45 UTC (permalink / raw)
To: Yann E. MORIN
Cc: Dirk Gouders, Michal Marek, linux-kbuild, Felipe Balbi, USB list,
Tomi Valkeinen, Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-10-31 23:39 ` [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols Dirk Gouders
2013-11-04 17:27 ` Sebastian Andrzej Siewior
@ 2013-11-05 23:04 ` Yann E. MORIN
2013-11-06 14:43 ` Dirk Gouders
1 sibling, 1 reply; 42+ messages in thread
From: Yann E. MORIN @ 2013-11-05 23:04 UTC (permalink / raw)
To: Dirk Gouders
Cc: Sebastian Andrzej Siewior, Michal Marek, linux-kbuild,
Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-05 23:04 ` Yann E. MORIN
@ 2013-11-06 14:43 ` Dirk Gouders
2013-11-06 18:59 ` Yann E. MORIN
2013-11-08 9:46 ` [PATCH v3] " Dirk Gouders
0 siblings, 2 replies; 42+ messages in thread
From: Dirk Gouders @ 2013-11-06 14:43 UTC (permalink / raw)
To: Yann E. MORIN
Cc: Sebastian Andrzej Siewior, Michal Marek, linux-kbuild,
Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
"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
>>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-06 14:43 ` Dirk Gouders
@ 2013-11-06 18:59 ` Yann E. MORIN
2013-11-07 14:02 ` Dirk Gouders
2013-11-08 9:46 ` [PATCH v3] " Dirk Gouders
1 sibling, 1 reply; 42+ messages in thread
From: Yann E. MORIN @ 2013-11-06 18:59 UTC (permalink / raw)
To: Dirk Gouders
Cc: Sebastian Andrzej Siewior, Michal Marek, linux-kbuild,
Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-06 18:59 ` Yann E. MORIN
@ 2013-11-07 14:02 ` Dirk Gouders
2013-11-07 14:05 ` [PATCH v4] " Dirk Gouders
0 siblings, 1 reply; 42+ messages in thread
From: Dirk Gouders @ 2013-11-07 14:02 UTC (permalink / raw)
To: Yann E. MORIN
Cc: Sebastian Andrzej Siewior, Michal Marek, linux-kbuild,
Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
"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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-07 14:02 ` Dirk Gouders
@ 2013-11-07 14:05 ` Dirk Gouders
2013-11-18 18:08 ` Yann E. MORIN
0 siblings, 1 reply; 42+ messages in thread
From: Dirk Gouders @ 2013-11-07 14:05 UTC (permalink / raw)
To: Yann E. MORIN
Cc: Sebastian Andrzej Siewior, Michal Marek, linux-kbuild,
Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
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
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-06 14:43 ` Dirk Gouders
2013-11-06 18:59 ` Yann E. MORIN
@ 2013-11-08 9:46 ` Dirk Gouders
1 sibling, 0 replies; 42+ messages in thread
From: Dirk Gouders @ 2013-11-08 9:46 UTC (permalink / raw)
To: Yann E. MORIN
Cc: Sebastian Andrzej Siewior, Michal Marek, linux-kbuild,
Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
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
>
> ------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-07 14:05 ` [PATCH v4] " Dirk Gouders
@ 2013-11-18 18:08 ` Yann E. MORIN
2013-12-20 12:46 ` Sebastian Andrzej Siewior
2014-08-13 15:35 ` Bin Liu
0 siblings, 2 replies; 42+ messages in thread
From: Yann E. MORIN @ 2013-11-18 18:08 UTC (permalink / raw)
To: Dirk Gouders
Cc: Sebastian Andrzej Siewior, Michal Marek, linux-kbuild,
Felipe Balbi, USB list, Tomi Valkeinen, Roger Quadros
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-18 18:08 ` Yann E. MORIN
@ 2013-12-20 12:46 ` Sebastian Andrzej Siewior
2014-08-13 15:35 ` Bin Liu
1 sibling, 0 replies; 42+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-12-20 12:46 UTC (permalink / raw)
To: Yann E. MORIN
Cc: Dirk Gouders, Michal Marek, linux-kbuild, Felipe Balbi, USB list,
Tomi Valkeinen, Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2013-11-18 18:08 ` Yann E. MORIN
2013-12-20 12:46 ` Sebastian Andrzej Siewior
@ 2014-08-13 15:35 ` Bin Liu
2014-08-14 6:52 ` Dirk Gouders
1 sibling, 1 reply; 42+ messages in thread
From: Bin Liu @ 2014-08-13 15:35 UTC (permalink / raw)
To: Yann E. MORIN
Cc: Dirk Gouders, Sebastian Andrzej Siewior, Michal Marek,
linux-kbuild, Felipe Balbi, USB list, Tomi Valkeinen,
Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2014-08-13 15:35 ` Bin Liu
@ 2014-08-14 6:52 ` Dirk Gouders
2014-08-14 13:54 ` Bin Liu
0 siblings, 1 reply; 42+ messages in thread
From: Dirk Gouders @ 2014-08-14 6:52 UTC (permalink / raw)
To: Bin Liu
Cc: Yann E. MORIN, Sebastian Andrzej Siewior, Michal Marek,
linux-kbuild, Felipe Balbi, USB list, Tomi Valkeinen,
Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2014-08-14 6:52 ` Dirk Gouders
@ 2014-08-14 13:54 ` Bin Liu
2014-08-15 7:37 ` Dirk Gouders
0 siblings, 1 reply; 42+ messages in thread
From: Bin Liu @ 2014-08-14 13:54 UTC (permalink / raw)
To: Dirk Gouders
Cc: Yann E. MORIN, Sebastian Andrzej Siewior, Michal Marek,
linux-kbuild, Felipe Balbi, USB list, Tomi Valkeinen,
Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2014-08-14 13:54 ` Bin Liu
@ 2014-08-15 7:37 ` Dirk Gouders
2014-08-15 7:43 ` Sebastian Andrzej Siewior
2016-03-30 22:08 ` Bin Liu
0 siblings, 2 replies; 42+ messages in thread
From: Dirk Gouders @ 2014-08-15 7:37 UTC (permalink / raw)
To: Bin Liu
Cc: Yann E. MORIN, Sebastian Andrzej Siewior, Michal Marek,
linux-kbuild, Felipe Balbi, USB list, Tomi Valkeinen,
Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2014-08-15 7:37 ` Dirk Gouders
@ 2014-08-15 7:43 ` Sebastian Andrzej Siewior
2016-03-30 22:08 ` Bin Liu
1 sibling, 0 replies; 42+ messages in thread
From: Sebastian Andrzej Siewior @ 2014-08-15 7:43 UTC (permalink / raw)
To: Dirk Gouders, Bin Liu
Cc: Yann E. MORIN, Michal Marek, linux-kbuild, Felipe Balbi,
USB list, Tomi Valkeinen, Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2014-08-15 7:37 ` Dirk Gouders
2014-08-15 7:43 ` Sebastian Andrzej Siewior
@ 2016-03-30 22:08 ` Bin Liu
2016-03-30 22:16 ` Ruslan Bilovol
1 sibling, 1 reply; 42+ messages in thread
From: Bin Liu @ 2016-03-30 22:08 UTC (permalink / raw)
To: Dirk Gouders
Cc: Yann E. MORIN, Sebastian Andrzej Siewior, Michal Marek,
linux-kbuild, USB list, Tomi Valkeinen, Roger Quadros
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.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-03-30 22:08 ` Bin Liu
@ 2016-03-30 22:16 ` Ruslan Bilovol
2016-03-31 7:13 ` Roger Quadros
0 siblings, 1 reply; 42+ messages in thread
From: Ruslan Bilovol @ 2016-03-30 22:16 UTC (permalink / raw)
To: Bin Liu
Cc: Dirk Gouders, Yann E. MORIN, Sebastian Andrzej Siewior,
Michal Marek, linux-kbuild, USB list, Tomi Valkeinen,
Roger Quadros
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-03-30 22:16 ` Ruslan Bilovol
@ 2016-03-31 7:13 ` Roger Quadros
2016-03-31 9:38 ` Dirk Gouders
2016-04-20 10:19 ` [RESEND PATCH " Dirk Gouders
0 siblings, 2 replies; 42+ messages in thread
From: Roger Quadros @ 2016-03-31 7:13 UTC (permalink / raw)
To: Ruslan Bilovol, Bin Liu
Cc: Dirk Gouders, Yann E. MORIN, Sebastian Andrzej Siewior,
Michal Marek, linux-kbuild, USB list, Tomi Valkeinen
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-03-31 7:13 ` Roger Quadros
@ 2016-03-31 9:38 ` Dirk Gouders
2016-03-31 9:53 ` Dirk Gouders
2016-04-20 10:19 ` [RESEND PATCH " Dirk Gouders
1 sibling, 1 reply; 42+ messages in thread
From: Dirk Gouders @ 2016-03-31 9:38 UTC (permalink / raw)
To: Roger Quadros
Cc: Ruslan Bilovol, Bin Liu, Sebastian Andrzej Siewior, Michal Marek,
linux-kbuild, USB list, Tomi Valkeinen
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-03-31 9:38 ` Dirk Gouders
@ 2016-03-31 9:53 ` Dirk Gouders
0 siblings, 0 replies; 42+ messages in thread
From: Dirk Gouders @ 2016-03-31 9:53 UTC (permalink / raw)
To: Roger Quadros
Cc: Ruslan Bilovol, Bin Liu, Sebastian Andrzej Siewior, Michal Marek,
linux-kbuild, USB list, Tomi Valkeinen
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [RESEND PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-03-31 7:13 ` Roger Quadros
2016-03-31 9:38 ` Dirk Gouders
@ 2016-04-20 10:19 ` Dirk Gouders
2016-04-20 11:04 ` kbuild test robot
2016-04-20 12:12 ` [RESEND PATCH v4] " kbuild test robot
1 sibling, 2 replies; 42+ messages in thread
From: Dirk Gouders @ 2016-04-20 10:19 UTC (permalink / raw)
To: Michal Marek
Cc: Roger Quadros, Ruslan Bilovol, Bin Liu,
Sebastian Andrzej Siewior, linux-kbuild, USB list,
Tomi Valkeinen
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
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [RESEND PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-04-20 10:19 ` [RESEND PATCH " Dirk Gouders
@ 2016-04-20 11:04 ` kbuild test robot
2016-04-20 13:14 ` Dirk Gouders
2016-04-29 8:24 ` [PATCH v5] " Dirk Gouders
2016-04-20 12:12 ` [RESEND PATCH v4] " kbuild test robot
1 sibling, 2 replies; 42+ messages in thread
From: kbuild test robot @ 2016-04-20 11:04 UTC (permalink / raw)
To: Dirk Gouders
Cc: kbuild-all, Michal Marek, Roger Quadros, Ruslan Bilovol, Bin Liu,
Sebastian Andrzej Siewior, linux-kbuild, USB list,
Tomi Valkeinen
[-- 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 --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RESEND PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-04-20 10:19 ` [RESEND PATCH " Dirk Gouders
2016-04-20 11:04 ` kbuild test robot
@ 2016-04-20 12:12 ` kbuild test robot
1 sibling, 0 replies; 42+ messages in thread
From: kbuild test robot @ 2016-04-20 12:12 UTC (permalink / raw)
To: Dirk Gouders
Cc: kbuild-all, Michal Marek, Roger Quadros, Ruslan Bilovol, Bin Liu,
Sebastian Andrzej Siewior, linux-kbuild, USB list,
Tomi Valkeinen
[-- 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 --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RESEND PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-04-20 11:04 ` kbuild test robot
@ 2016-04-20 13:14 ` Dirk Gouders
2016-04-29 8:24 ` [PATCH v5] " Dirk Gouders
1 sibling, 0 replies; 42+ messages in thread
From: Dirk Gouders @ 2016-04-20 13:14 UTC (permalink / raw)
To: Michal Marek
Cc: kbuild-all, Roger Quadros, Ruslan Bilovol, Bin Liu,
Sebastian Andrzej Siewior, linux-kbuild, USB list,
Tomi Valkeinen
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
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v5] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-04-20 11:04 ` kbuild test robot
2016-04-20 13:14 ` Dirk Gouders
@ 2016-04-29 8:24 ` Dirk Gouders
2016-05-02 8:43 ` Roger Quadros
1 sibling, 1 reply; 42+ messages in thread
From: Dirk Gouders @ 2016-04-29 8:24 UTC (permalink / raw)
To: Michal Marek
Cc: kbuild-all, Roger Quadros, Ruslan Bilovol, Bin Liu,
Sebastian Andrzej Siewior, linux-kbuild, USB list,
Tomi Valkeinen
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
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v5] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-04-29 8:24 ` [PATCH v5] " Dirk Gouders
@ 2016-05-02 8:43 ` Roger Quadros
2016-05-10 19:15 ` Michal Marek
0 siblings, 1 reply; 42+ messages in thread
From: Roger Quadros @ 2016-05-02 8:43 UTC (permalink / raw)
To: Dirk Gouders, Michal Marek
Cc: kbuild-all, Ruslan Bilovol, Bin Liu, Sebastian Andrzej Siewior,
linux-kbuild, USB list, Tomi Valkeinen
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))
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v5] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
2016-05-02 8:43 ` Roger Quadros
@ 2016-05-10 19:15 ` Michal Marek
0 siblings, 0 replies; 42+ messages in thread
From: Michal Marek @ 2016-05-10 19:15 UTC (permalink / raw)
To: Roger Quadros
Cc: Dirk Gouders, Michal Marek, kbuild-all, Ruslan Bilovol, Bin Liu,
Sebastian Andrzej Siewior, linux-kbuild, USB list,
Tomi Valkeinen
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
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2016-05-10 19:15 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-23 10:51 choice =y selection becomes lost after having multiple entries =m with depends on Sebastian Andrzej Siewior
2013-10-23 11:23 ` Yann E. MORIN
2013-10-23 11:28 ` Sebastian Andrzej Siewior
2013-10-24 15:30 ` Dirk Gouders
2013-10-24 16:19 ` Sebastian Andrzej Siewior
2013-10-24 16:50 ` Dirk Gouders
2013-10-30 10:00 ` Dirk Gouders
2013-10-30 10:30 ` Daniele Forsi
2013-10-30 10:41 ` Dirk Gouders
2013-10-30 14:26 ` Dirk Gouders
2013-10-31 10:20 ` Sebastian Andrzej Siewior
2013-10-31 21:49 ` Yann E. MORIN
2013-11-01 8:45 ` Dirk Gouders
2013-10-31 23:39 ` [PATCH v3] kconfig/symbol.c: handle choice_values that depend on 'm' symbols Dirk Gouders
2013-11-04 17:27 ` Sebastian Andrzej Siewior
2013-11-04 20:46 ` Yann E. MORIN
2013-11-05 8:45 ` Sebastian Andrzej Siewior
2013-11-05 23:04 ` Yann E. MORIN
2013-11-06 14:43 ` Dirk Gouders
2013-11-06 18:59 ` Yann E. MORIN
2013-11-07 14:02 ` Dirk Gouders
2013-11-07 14:05 ` [PATCH v4] " Dirk Gouders
2013-11-18 18:08 ` Yann E. MORIN
2013-12-20 12:46 ` Sebastian Andrzej Siewior
2014-08-13 15:35 ` Bin Liu
2014-08-14 6:52 ` Dirk Gouders
2014-08-14 13:54 ` Bin Liu
2014-08-15 7:37 ` Dirk Gouders
2014-08-15 7:43 ` Sebastian Andrzej Siewior
2016-03-30 22:08 ` Bin Liu
2016-03-30 22:16 ` Ruslan Bilovol
2016-03-31 7:13 ` Roger Quadros
2016-03-31 9:38 ` Dirk Gouders
2016-03-31 9:53 ` Dirk Gouders
2016-04-20 10:19 ` [RESEND PATCH " Dirk Gouders
2016-04-20 11:04 ` kbuild test robot
2016-04-20 13:14 ` Dirk Gouders
2016-04-29 8:24 ` [PATCH v5] " Dirk Gouders
2016-05-02 8:43 ` Roger Quadros
2016-05-10 19:15 ` Michal Marek
2016-04-20 12:12 ` [RESEND PATCH v4] " kbuild test robot
2013-11-08 9:46 ` [PATCH v3] " Dirk Gouders
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.