[PATCH v2 00/12] kconfig: fix choice value calculation with misc cleanups

[PATCH v2 00/12] kconfig: fix choice value calculation with misc cleanups

[Masahiro Yamada]

The main patch for this series is:
  "kconfig: refactor choice value calculation"
This rewrites the handling of user values for choices.

The others are misc cleanups.



Masahiro Yamada (12):
  kconfig: import list_move(_tail) and list_for_each_entry_reverse
    macros
  kconfig: refactor choice value calculation
  kconfig: remove sym_get_choice_value()
  kconfig: remove conf_unsaved in conf_read_simple()
  kconfig: change sym_choice_default() to take the choice menu
  kconfig: use menu_list_for_each_sym() in sym_choice_default()
  kconfig: remove expr_list_for_each_sym() macro
  kconfig: use sym_get_choice_menu() in sym_check_print_recursive()
  kconfig: use sym_get_choice_menu() in sym_check_choice_deps()
  kconfig: use sym_get_choice_menu() in sym_check_deps()
  kconfig: remove P_CHOICE property
  kconfig: remove E_LIST expression type

 scripts/kconfig/conf.c      | 137 +++++++++++-------------
 scripts/kconfig/confdata.c  |  61 +++--------
 scripts/kconfig/expr.c      |  15 ---
 scripts/kconfig/expr.h      |  21 ++--
 scripts/kconfig/gconf.c     |   2 +-
 scripts/kconfig/list.h      |  53 ++++++++++
 scripts/kconfig/lkc.h       |  10 +-
 scripts/kconfig/lkc_proto.h |   1 -
 scripts/kconfig/mconf.c     |   6 +-
 scripts/kconfig/menu.c      |  25 +----
 scripts/kconfig/nconf.c     |   6 +-
 scripts/kconfig/parser.y    |   8 +-
 scripts/kconfig/qconf.cc    |   8 --
 scripts/kconfig/symbol.c    | 206 ++++++++++++++++++++----------------
 14 files changed, 272 insertions(+), 287 deletions(-)

-- 
2.43.0

[PATCH v2 01/12] kconfig: import list_move(_tail) and list_for_each_entry_reverse macros

[Masahiro Yamada]

Import more macros from include/linux/list.h.

These will be used in the next commit.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - Import list_for_each_entry_reverse too

 scripts/kconfig/list.h | 53 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/scripts/kconfig/list.h b/scripts/kconfig/list.h
index 882859ddf9f4..409201cd495b 100644
--- a/scripts/kconfig/list.h
+++ b/scripts/kconfig/list.h
@@ -127,6 +127,29 @@ static inline void list_del(struct list_head *entry)
 	entry->prev = LIST_POISON2;
 }
 
+/**
+ * list_move - delete from one list and add as another's head
+ * @list: the entry to move
+ * @head: the head that will precede our entry
+ */
+static inline void list_move(struct list_head *list, struct list_head *head)
+{
+	__list_del_entry(list);
+	list_add(list, head);
+}
+
+/**
+ * list_move_tail - delete from one list and add as another's tail
+ * @list: the entry to move
+ * @head: the head that will follow our entry
+ */
+static inline void list_move_tail(struct list_head *list,
+				  struct list_head *head)
+{
+	__list_del_entry(list);
+	list_add_tail(list, head);
+}
+
 /**
  * list_is_head - tests whether @list is the list @head
  * @list: the entry to test
@@ -166,6 +189,17 @@ static inline int list_empty(const struct list_head *head)
 #define list_first_entry(ptr, type, member) \
 	list_entry((ptr)->next, type, member)
 
+/**
+ * list_last_entry - get the last element from a list
+ * @ptr:	the list head to take the element from.
+ * @type:	the type of the struct this is embedded in.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Note, that list is expected to be not empty.
+ */
+#define list_last_entry(ptr, type, member) \
+	list_entry((ptr)->prev, type, member)
+
 /**
  * list_next_entry - get the next element in list
  * @pos:	the type * to cursor
@@ -174,6 +208,14 @@ static inline int list_empty(const struct list_head *head)
 #define list_next_entry(pos, member) \
 	list_entry((pos)->member.next, typeof(*(pos)), member)
 
+/**
+ * list_prev_entry - get the prev element in list
+ * @pos:	the type * to cursor
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_prev_entry(pos, member) \
+	list_entry((pos)->member.prev, typeof(*(pos)), member)
+
 /**
  * list_entry_is_head - test if the entry points to the head of the list
  * @pos:	the type * to cursor
@@ -194,6 +236,17 @@ static inline int list_empty(const struct list_head *head)
 	     !list_entry_is_head(pos, head, member);			\
 	     pos = list_next_entry(pos, member))
 
+/**
+ * list_for_each_entry_reverse - iterate backwards over list of given type.
+ * @pos:	the type * to use as a loop cursor.
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_for_each_entry_reverse(pos, head, member)			\
+	for (pos = list_last_entry(head, typeof(*pos), member);		\
+	     !list_entry_is_head(pos, head, member); 			\
+	     pos = list_prev_entry(pos, member))
+
 /**
  * list_for_each_entry_safe - iterate over list of given type. Safe against removal of list entry
  * @pos:	the type * to use as a loop cursor.
-- 
2.43.0

[PATCH v2 02/12] kconfig: refactor choice value calculation

[Masahiro Yamada]

Handling choices has always been in a PITA in Kconfig.

For example, fixes and reverts were repeated for randconfig with
KCONFIG_ALLCONFIG:

 - 422c809f03f0 ("kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG")
 - 23a5dfdad22a ("Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"")
 - 8357b48549e1 ("kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG")
 - 490f16171119 ("Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"")

As these commits pointed out, randconfig does not randomize choices when
KCONFIG_ALLCONFIG is used. This issue still remains.

[Test Case]

    choice
            prompt "choose"

    config A
            bool "A"

    config B
            bool "B"

    endchoice

    $ echo > all.config
    $ make KCONFIG_ALLCONFIG=1 randconfig

The output is always as follows:

    CONFIG_A=y
    # CONFIG_B is not set

Not only randconfig, but other all*config variants are also broken with
KCONFIG_ALLCONFIG.

With the same Kconfig,

    $ echo '# CONFIG_A is not set' > all.config
    $ make KCONFIG_ALLCONFIG=1 allyesconfig

You will get this:

    CONFIG_A=y
    # CONFIG_B is not set

This is incorrect because it does not respect all.config.

The correct output should be:

    # CONFIG_A is not set
    CONFIG_B=y

To handle user inputs more accurately, this commit refactors the code
based on the following principles:

 - When a user value is given, Kconfig must set it immediately.
   Do not defer it by setting SYMBOL_NEED_SET_CHOICE_VALUES.

 - The SYMBOL_DEF_USER flag must not be cleared, unless a new config
   file is loaded. Kconfig must not forget user inputs.

In addition, user values for choices must be managed with priority.
If user inputs conflict within a choice block, the newest value wins.
The values given by randconfig have lower priority than explicit user
inputs.

This commit implements it by using a linked list. Every time a choice
block gets a new input, it is moved to the top of the list.

Let me explain how it works.

Let's say, we have a choice block that consists of five symbols:
A, B, C, D, and E.

Initially, the linked list looks like this:

    A(=?) --> B(=?) --> C(=?) --> D(=?) --> E(=?)

Suppose randconfig is executed with the following KCONFIG_ALLCONFIG:

    CONFIG_C=y
    # CONFIG_A is not set
    CONFIG_D=y

First, CONFIG_C=y is read. C is set to 'y' and moved to the top.

    C(=y) --> A(=?) --> B(=?) --> D(=?) --> E(=?)

Next, '# CONFIG_A is not set' is read. A is set to 'n' and moved to
the top.

    A(=n) --> C(=y) --> B(=?) --> D(=?) --> E(=?)

Then, 'CONFIG_D=y' is read. D is set to 'y' and moved to the top.

    D(=y) --> A(=n) --> C(=y) --> B(=?) --> E(=?)

Lastly, randconfig shuffles the order of the remaining symbols,
resulting in:

    D(=y) --> A(=n) --> C(=y) --> B(=y) --> E(=y)
or
    D(=y) --> A(=n) --> C(=y) --> E(=y) --> B(=y)

When calculating the output, the linked list is traversed and the first
visible symbol with 'y' is taken. In this case, it is D if visible.

If D is hidden by 'depends on', the next node, A, is examined. Since
it is already specified as 'n', it is skipped. Next, C is checked, and
selected if it is visible.

If C is also invisible, either B or E is chosen as a result of the
randomization.

If B and E are also invisible, the linked list is traversed in the
reverse order, and the least prioritized 'n' symbol is chosen. It is
A in this case.

Now, Kconfig remembers all user values. This is a big difference from
the previous implementation, where Kconfig would forget CONFIG_C=y when
CONFIG_D=y appeared in the same input file.

The new appaorch respects user-specified values as much as possible.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - If all 'y' and '?' symbols are invisible, traverse the linked list
    in the reverse order to pick up the least prioritized 'n'.

 scripts/kconfig/conf.c     | 131 +++++++++++++++---------------
 scripts/kconfig/confdata.c |  54 +++----------
 scripts/kconfig/expr.h     |  12 ++-
 scripts/kconfig/lkc.h      |   7 +-
 scripts/kconfig/menu.c     |  17 +---
 scripts/kconfig/parser.y   |   4 +
 scripts/kconfig/symbol.c   | 159 +++++++++++++++++++++++--------------
 7 files changed, 187 insertions(+), 197 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 5dbdd9459f21..1c59998a62f7 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -114,41 +114,54 @@ static void set_randconfig_seed(void)
 	srand(seed);
 }
 
-static void randomize_choice_values(struct symbol *csym)
+/**
+ * randomize_choice_values - randomize choice block
+ *
+ * @choice: menu entry for the choice
+ */
+static void randomize_choice_values(struct menu *choice)
 {
-	struct property *prop;
-	struct symbol *sym;
-	struct expr *e;
-	int cnt, def;
-
-	prop = sym_get_choice_prop(csym);
-
-	/* count entries in choice block */
-	cnt = 0;
-	expr_list_for_each_sym(prop->expr, e, sym)
-		cnt++;
+	struct menu *menu;
+	int x;
+	int cnt = 0;
 
 	/*
-	 * find a random value and set it to yes,
-	 * set the rest to no so we have only one set
+	 * First, count the number of symbols to randomize. If sym_has_value()
+	 * is true, it was specified by KCONFIG_ALLCONFIG. It needs to be
+	 * respected.
 	 */
-	def = rand() % cnt;
+	menu_for_each_sub_entry(menu, choice) {
+		struct symbol *sym = menu->sym;
 
-	cnt = 0;
-	expr_list_for_each_sym(prop->expr, e, sym) {
-		if (def == cnt++) {
-			sym->def[S_DEF_USER].tri = yes;
-			csym->def[S_DEF_USER].val = sym;
-		} else {
-			sym->def[S_DEF_USER].tri = no;
-		}
-		sym->flags |= SYMBOL_DEF_USER;
-		/* clear VALID to get value calculated */
-		sym->flags &= ~SYMBOL_VALID;
+		if (sym && !sym_has_value(sym))
+			cnt++;
+	}
+
+	while (cnt > 0) {
+		x = rand() % cnt;
+
+		menu_for_each_sub_entry(menu, choice) {
+			struct symbol *sym = menu->sym;
+
+			if (sym && !sym_has_value(sym))
+				x--;
+
+			if (x < 0) {
+				sym->def[S_DEF_USER].tri = yes;
+				sym->flags |= SYMBOL_DEF_USER;
+				/*
+				 * Move the selected item to the _tail_ because
+				 * this needs to have a lower priority than the
+				 * user input from KCONFIG_ALLCONFIG.
+				 */
+				list_move_tail(&sym->choice_link,
+					       &choice->choice_members);
+
+				break;
+			}
+		}
+		cnt--;
 	}
-	csym->flags |= SYMBOL_DEF_USER;
-	/* clear VALID to get value calculated */
-	csym->flags &= ~SYMBOL_VALID;
 }
 
 enum conf_def_mode {
@@ -159,9 +172,9 @@ enum conf_def_mode {
 	def_random
 };
 
-static bool conf_set_all_new_symbols(enum conf_def_mode mode)
+static void conf_set_all_new_symbols(enum conf_def_mode mode)
 {
-	struct symbol *sym, *csym;
+	struct menu *menu;
 	int cnt;
 	/*
 	 * can't go as the default in switch-case below, otherwise gcc whines
@@ -170,7 +183,6 @@ static bool conf_set_all_new_symbols(enum conf_def_mode mode)
 	int pby = 50; /* probability of bool     = y */
 	int pty = 33; /* probability of tristate = y */
 	int ptm = 33; /* probability of tristate = m */
-	bool has_changed = false;
 
 	if (mode == def_random) {
 		int n, p[3];
@@ -217,14 +229,21 @@ static bool conf_set_all_new_symbols(enum conf_def_mode mode)
 		}
 	}
 
-	for_all_symbols(sym) {
+	menu_for_each_entry(menu) {
+		struct symbol *sym = menu->sym;
 		tristate val;
 
-		if (sym_has_value(sym) || sym->flags & SYMBOL_VALID ||
-		    (sym->type != S_BOOLEAN && sym->type != S_TRISTATE))
+		if (!sym || !menu->prompt || sym_has_value(sym) ||
+		    (sym->type != S_BOOLEAN && sym->type != S_TRISTATE) ||
+		    sym_is_choice_value(sym))
 			continue;
 
-		has_changed = true;
+		if (sym_is_choice(sym)) {
+			if (mode == def_random)
+				randomize_choice_values(menu);
+			continue;
+		}
+
 		switch (mode) {
 		case def_yes:
 			val = yes;
@@ -251,34 +270,10 @@ static bool conf_set_all_new_symbols(enum conf_def_mode mode)
 			continue;
 		}
 		sym->def[S_DEF_USER].tri = val;
-
-		if (!(sym_is_choice(sym) && mode == def_random))
-			sym->flags |= SYMBOL_DEF_USER;
+		sym->flags |= SYMBOL_DEF_USER;
 	}
 
 	sym_clear_all_valid();
-
-	if (mode != def_random) {
-		for_all_symbols(csym) {
-			if ((sym_is_choice(csym) && !sym_has_value(csym)) ||
-			    sym_is_choice_value(csym))
-				csym->flags |= SYMBOL_NEED_SET_CHOICE_VALUES;
-		}
-	}
-
-	for_all_symbols(csym) {
-		if (sym_has_value(csym) || !sym_is_choice(csym))
-			continue;
-
-		sym_calc_value(csym);
-		if (mode == def_random)
-			randomize_choice_values(csym);
-		else
-			set_all_choice_values(csym);
-		has_changed = true;
-	}
-
-	return has_changed;
 }
 
 static void conf_rewrite_tristates(tristate old_val, tristate new_val)
@@ -429,10 +424,9 @@ static void conf_choice(struct menu *menu)
 {
 	struct symbol *sym, *def_sym;
 	struct menu *child;
-	bool is_new;
+	bool is_new = false;
 
 	sym = menu->sym;
-	is_new = !sym_has_value(sym);
 
 	while (1) {
 		int cnt, def;
@@ -456,8 +450,10 @@ static void conf_choice(struct menu *menu)
 				printf("%*c", indent, ' ');
 			printf(" %d. %s (%s)", cnt, menu_get_prompt(child),
 			       child->sym->name);
-			if (!sym_has_value(child->sym))
+			if (!sym_has_value(child->sym)) {
+				is_new = true;
 				printf(" (NEW)");
+			}
 			printf("\n");
 		}
 		printf("%*schoice", indent - 1, "");
@@ -586,9 +582,7 @@ static void check_conf(struct menu *menu)
 		return;
 
 	sym = menu->sym;
-	if (sym && !sym_has_value(sym) &&
-	    (sym_is_changeable(sym) || sym_is_choice(sym))) {
-
+	if (sym && !sym_has_value(sym) && sym_is_changeable(sym)) {
 		switch (input_mode) {
 		case listnewconfig:
 			if (sym->name)
@@ -804,8 +798,7 @@ int main(int ac, char **av)
 		conf_set_all_new_symbols(def_default);
 		break;
 	case randconfig:
-		/* Really nothing to do in this loop */
-		while (conf_set_all_new_symbols(def_random)) ;
+		conf_set_all_new_symbols(def_random);
 		break;
 	case defconfig:
 		conf_set_all_new_symbols(def_default);
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 1ac7fc9ad756..05823f85402a 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -382,10 +382,7 @@ int conf_read_simple(const char *name, int def)
 
 	def_flags = SYMBOL_DEF << def;
 	for_all_symbols(sym) {
-		sym->flags |= SYMBOL_CHANGED;
 		sym->flags &= ~(def_flags|SYMBOL_VALID);
-		if (sym_is_choice(sym))
-			sym->flags |= def_flags;
 		switch (sym->type) {
 		case S_INT:
 		case S_HEX:
@@ -399,6 +396,8 @@ int conf_read_simple(const char *name, int def)
 	}
 
 	while (getline_stripped(&line, &line_asize, in) != -1) {
+		struct menu *choice;
+
 		conf_lineno++;
 
 		if (!line[0]) /* blank line */
@@ -460,15 +459,14 @@ int conf_read_simple(const char *name, int def)
 		if (conf_set_sym_val(sym, def, def_flags, val))
 			continue;
 
-		if (sym && sym_is_choice_value(sym)) {
-			struct symbol *cs = prop_get_symbol(sym_get_choice_prop(sym));
-			if (sym->def[def].tri == yes) {
-				if (cs->def[def].tri != no)
-					conf_warning("override: %s changes choice state", sym->name);
-				cs->def[def].val = sym;
-				cs->def[def].tri = yes;
-			}
-		}
+		/*
+		 * If this is a choice member, give it the highest priority.
+		 * If conflicting CONFIG options are given from an input file,
+		 * the last one wins.
+		 */
+		choice = sym_get_choice_menu(sym);
+		if (choice)
+			list_move(&sym->choice_link, &choice->choice_members);
 	}
 	free(line);
 	fclose(in);
@@ -514,18 +512,6 @@ int conf_read(const char *name)
 		/* maybe print value in verbose mode... */
 	}
 
-	for_all_symbols(sym) {
-		if (sym_has_value(sym) && !sym_is_choice_value(sym)) {
-			/* Reset values of generates values, so they'll appear
-			 * as new, if they should become visible, but that
-			 * doesn't quite work if the Kconfig and the saved
-			 * configuration disagree.
-			 */
-			if (sym->visible == no && !conf_unsaved)
-				sym->flags &= ~SYMBOL_DEF_USER;
-		}
-	}
-
 	if (conf_warnings || conf_unsaved)
 		conf_set_changed(true);
 
@@ -1146,23 +1132,3 @@ void conf_set_changed_callback(void (*fn)(bool))
 {
 	conf_changed_callback = fn;
 }
-
-void set_all_choice_values(struct symbol *csym)
-{
-	struct property *prop;
-	struct symbol *sym;
-	struct expr *e;
-
-	prop = sym_get_choice_prop(csym);
-
-	/*
-	 * Set all non-assinged choice values to no
-	 */
-	expr_list_for_each_sym(prop->expr, e, sym) {
-		if (!sym_has_value(sym))
-			sym->def[S_DEF_USER].tri = no;
-	}
-	csym->flags |= SYMBOL_DEF_USER;
-	/* clear VALID to get value calculated */
-	csym->flags &= ~(SYMBOL_VALID | SYMBOL_NEED_SET_CHOICE_VALUES);
-}
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 7c0c242318bc..7acf27a4f454 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -73,6 +73,8 @@ enum {
  * Represents a configuration symbol.
  *
  * Choices are represented as a special kind of symbol with null name.
+ *
+ * @choice_link: linked to menu::choice_members
  */
 struct symbol {
 	/* link node for the hash table */
@@ -110,6 +112,8 @@ struct symbol {
 	/* config entries associated with this symbol */
 	struct list_head menus;
 
+	struct list_head choice_link;
+
 	/* SYMBOL_* flags */
 	int flags;
 
@@ -133,7 +137,6 @@ struct symbol {
 #define SYMBOL_CHOICEVAL  0x0020  /* used as a value in a choice block */
 #define SYMBOL_VALID      0x0080  /* set when symbol.curr is calculated */
 #define SYMBOL_WRITE      0x0200  /* write symbol to file (KCONFIG_CONFIG) */
-#define SYMBOL_CHANGED    0x0400  /* ? */
 #define SYMBOL_WRITTEN    0x0800  /* track info to avoid double-write to .config */
 #define SYMBOL_CHECKED    0x2000  /* used during dependency checking */
 #define SYMBOL_WARNED     0x8000  /* warning has been issued */
@@ -145,9 +148,6 @@ struct symbol {
 #define SYMBOL_DEF3       0x40000  /* symbol.def[S_DEF_3] is valid */
 #define SYMBOL_DEF4       0x80000  /* symbol.def[S_DEF_4] is valid */
 
-/* choice values need to be set before calculating this symbol value */
-#define SYMBOL_NEED_SET_CHOICE_VALUES  0x100000
-
 #define SYMBOL_MAXLENGTH	256
 
 /* A property represent the config options that can be associated
@@ -204,6 +204,8 @@ struct property {
  * for all front ends). Each symbol, menu, etc. defined in the Kconfig files
  * gets a node. A symbol defined in multiple locations gets one node at each
  * location.
+ *
+ * @choice_members: list of choice members with priority.
  */
 struct menu {
 	/* The next menu node at the same level */
@@ -223,6 +225,8 @@ struct menu {
 
 	struct list_head link;	/* link to symbol::menus */
 
+	struct list_head choice_members;
+
 	/*
 	 * The prompt associated with the node. This holds the prompt for a
 	 * symbol as well as the text for a menu or comment, along with the
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index 64dfc354dd5c..bdd37a16b040 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -40,7 +40,6 @@ void zconf_nextfile(const char *name);
 /* confdata.c */
 extern struct gstr autoconf_cmd;
 const char *conf_get_configname(void);
-void set_all_choice_values(struct symbol *csym);
 
 /* confdata.c and expr.c */
 static inline void xfwrite(const void *str, size_t len, size_t count, FILE *out)
@@ -121,11 +120,7 @@ static inline tristate sym_get_tristate_value(struct symbol *sym)
 	return sym->curr.tri;
 }
 
-
-static inline struct symbol *sym_get_choice_value(struct symbol *sym)
-{
-	return (struct symbol *)sym->curr.val;
-}
+struct symbol *sym_get_choice_value(struct symbol *sym);
 
 static inline bool sym_is_choice(struct symbol *sym)
 {
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index bf5dcc05350b..170a269a8d7c 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -591,7 +591,6 @@ bool menu_is_empty(struct menu *menu)
 
 bool menu_is_visible(struct menu *menu)
 {
-	struct menu *child;
 	struct symbol *sym;
 	tristate visible;
 
@@ -610,21 +609,7 @@ bool menu_is_visible(struct menu *menu)
 	} else
 		visible = menu->prompt->visible.tri = expr_calc_value(menu->prompt->visible.expr);
 
-	if (visible != no)
-		return true;
-
-	if (!sym || sym_get_tristate_value(menu->sym) == no)
-		return false;
-
-	for (child = menu->list; child; child = child->next) {
-		if (menu_is_visible(child)) {
-			if (sym)
-				sym->flags |= SYMBOL_DEF_USER;
-			return true;
-		}
-	}
-
-	return false;
+	return visible != no;
 }
 
 const char *menu_get_prompt(struct menu *menu)
diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
index 20538e1d3788..9d58544b0255 100644
--- a/scripts/kconfig/parser.y
+++ b/scripts/kconfig/parser.y
@@ -157,6 +157,9 @@ config_stmt: config_entry_start config_option_list
 				current_entry->filename, current_entry->lineno);
 			yynerrs++;
 		}
+
+		list_add_tail(&current_entry->sym->choice_link,
+			      &current_choice->choice_members);
 	}
 
 	printd(DEBUG_PARSE, "%s:%d:endconfig\n", cur_filename, cur_lineno);
@@ -240,6 +243,7 @@ choice: T_CHOICE T_EOL
 	menu_add_entry(sym);
 	menu_add_expr(P_CHOICE, NULL, NULL);
 	menu_set_type(S_BOOLEAN);
+	INIT_LIST_HEAD(&current_entry->choice_members);
 
 	printd(DEBUG_PARSE, "%s:%d:choice\n", cur_filename, cur_lineno);
 };
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 8df0a75f40b9..329c7bd314cf 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -188,7 +188,6 @@ static void sym_set_changed(struct symbol *sym)
 {
 	struct menu *menu;
 
-	sym->flags |= SYMBOL_CHANGED;
 	list_for_each_entry(menu, &sym->menus, link)
 		menu->flags |= MENU_CHANGED;
 }
@@ -282,36 +281,95 @@ struct symbol *sym_choice_default(struct symbol *sym)
 	return NULL;
 }
 
-static struct symbol *sym_calc_choice(struct symbol *sym)
+/*
+ * sym_calc_choice - calculate symbol values in a choice
+ *
+ * @choice: a menu of the choice
+ *
+ * Return: a chosen symbol
+ */
+static struct symbol *sym_calc_choice(struct menu *choice)
 {
-	struct symbol *def_sym;
-	struct property *prop;
-	struct expr *e;
-	int flags;
+	struct symbol *res = NULL;
+	struct symbol *sym;
+	struct menu *menu;
 
-	/* first calculate all choice values' visibilities */
-	flags = sym->flags;
-	prop = sym_get_choice_prop(sym);
-	expr_list_for_each_sym(prop->expr, e, def_sym) {
-		sym_calc_visibility(def_sym);
-		if (def_sym->visible != no)
-			flags &= def_sym->flags;
+	/* Traverse the list of choice members in the priority order. */
+	list_for_each_entry(sym, &choice->choice_members, choice_link) {
+		sym_calc_visibility(sym);
+		if (sym->visible == no)
+			continue;
+
+		/* The first visible symble with the user value 'y'. */
+		if (sym_has_value(sym) && sym->def[S_DEF_USER].tri == yes) {
+			res = sym;
+			break;
+		}
 	}
 
-	sym->flags &= flags | ~SYMBOL_DEF_USER;
+	/*
+	 * If 'y' is not found in the user input, use the default, unless it is
+	 * explicitly set to 'n'.
+	 */
+	if (!res) {
+		res = sym_choice_default(choice->sym);
+		if (res && sym_has_value(res) && res->def[S_DEF_USER].tri == no)
+			res = NULL;
+	}
 
-	/* is the user choice visible? */
-	def_sym = sym->def[S_DEF_USER].val;
-	if (def_sym && def_sym->visible != no)
-		return def_sym;
+	/* Still not found. Pick up the first visible, user-unspecified symbol. */
+	if (!res) {
+		menu_for_each_sub_entry(menu, choice) {
+			sym = menu->sym;
 
-	def_sym = sym_choice_default(sym);
+			if (!sym || sym->visible == no || sym_has_value(sym))
+				continue;
 
-	if (def_sym == NULL)
-		/* no choice? reset tristate value */
-		sym->curr.tri = no;
+			res = sym;
+			break;
+		}
+	}
 
-	return def_sym;
+	/*
+	 * Still not found. Traverse the linked list in the _reverse_ order to
+	 * pick up the least prioritized 'n'.
+	 */
+	if (!res) {
+		list_for_each_entry_reverse(sym, &choice->choice_members,
+					    choice_link) {
+			if (sym->visible == no)
+				continue;
+
+			res = sym;
+			break;
+		}
+	}
+
+	menu_for_each_sub_entry(menu, choice) {
+		tristate val;
+
+		sym = menu->sym;
+
+		if (!sym || sym->visible == no)
+			continue;
+
+		val = sym == res ? yes : no;
+
+		if (sym->curr.tri != val)
+			sym_set_changed(sym);
+
+		sym->curr.tri = val;
+		sym->flags |= SYMBOL_VALID | SYMBOL_WRITE;
+	}
+
+	return res;
+}
+
+struct symbol *sym_get_choice_value(struct symbol *sym)
+{
+	struct menu *menu = list_first_entry(&sym->menus, struct menu, link);
+
+	return sym_calc_choice(menu);
 }
 
 static void sym_warn_unmet_dep(struct symbol *sym)
@@ -347,7 +405,7 @@ void sym_calc_value(struct symbol *sym)
 {
 	struct symbol_value newval, oldval;
 	struct property *prop;
-	struct expr *e;
+	struct menu *choice_menu;
 
 	if (!sym)
 		return;
@@ -355,13 +413,6 @@ void sym_calc_value(struct symbol *sym)
 	if (sym->flags & SYMBOL_VALID)
 		return;
 
-	if (sym_is_choice_value(sym) &&
-	    sym->flags & SYMBOL_NEED_SET_CHOICE_VALUES) {
-		sym->flags &= ~SYMBOL_NEED_SET_CHOICE_VALUES;
-		prop = sym_get_choice_prop(sym);
-		sym_calc_value(prop_get_symbol(prop));
-	}
-
 	sym->flags |= SYMBOL_VALID;
 
 	oldval = sym->curr;
@@ -400,9 +451,11 @@ 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;
+		choice_menu = sym_get_choice_menu(sym);
+
+		if (choice_menu) {
+			sym_calc_choice(choice_menu);
+			newval.tri = sym->curr.tri;
 		} else {
 			if (sym->visible != no) {
 				/* if the symbol is visible use the user value
@@ -461,8 +514,6 @@ void sym_calc_value(struct symbol *sym)
 	}
 
 	sym->curr = newval;
-	if (sym_is_choice(sym) && newval.tri == yes)
-		sym->curr.val = sym_calc_choice(sym);
 	sym_validate_range(sym);
 
 	if (memcmp(&oldval, &sym->curr, sizeof(oldval))) {
@@ -473,23 +524,8 @@ void sym_calc_value(struct symbol *sym)
 		}
 	}
 
-	if (sym_is_choice(sym)) {
-		struct symbol *choice_sym;
-
-		prop = sym_get_choice_prop(sym);
-		expr_list_for_each_sym(prop->expr, e, choice_sym) {
-			if ((sym->flags & SYMBOL_WRITE) &&
-			    choice_sym->visible != no)
-				choice_sym->flags |= SYMBOL_WRITE;
-			if (sym->flags & SYMBOL_CHANGED)
-				sym_set_changed(choice_sym);
-		}
-
+	if (sym_is_choice(sym))
 		sym->flags &= ~SYMBOL_WRITE;
-	}
-
-	if (sym->flags & SYMBOL_NEED_SET_CHOICE_VALUES)
-		set_all_choice_values(sym);
 }
 
 void sym_clear_all_valid(void)
@@ -523,15 +559,15 @@ bool sym_set_tristate_value(struct symbol *sym, tristate val)
 {
 	tristate oldval = sym_get_tristate_value(sym);
 
-	if (oldval != val && !sym_tristate_within_range(sym, val))
+	if (!sym_tristate_within_range(sym, val))
 		return false;
 
-	if (!(sym->flags & SYMBOL_DEF_USER)) {
+	if (!(sym->flags & SYMBOL_DEF_USER) || sym->def[S_DEF_USER].tri != val) {
+		sym->def[S_DEF_USER].tri = val;
 		sym->flags |= SYMBOL_DEF_USER;
 		sym_set_changed(sym);
 	}
 
-	sym->def[S_DEF_USER].tri = val;
 	if (oldval != val)
 		sym_clear_all_valid();
 
@@ -565,10 +601,17 @@ void choice_set_value(struct menu *choice, struct symbol *sym)
 
 		menu->sym->def[S_DEF_USER].tri = val;
 		menu->sym->flags |= SYMBOL_DEF_USER;
-	}
 
-	choice->sym->def[S_DEF_USER].val = sym;
-	choice->sym->flags |= SYMBOL_DEF_USER;
+		/*
+		 * Now, the user has explicitly enabled or disabled this symbol,
+		 * it should be given the highest priority. We are possibly
+		 * setting multiple symbols to 'n', where the first symbol is
+		 * given the least prioritized 'n'. This works well when the
+		 * choice block ends up with selecting 'n' symbol.
+		 * (see sym_calc_choice())
+		 */
+		list_move(&menu->sym->choice_link, &choice->choice_members);
+	}
 
 	if (changed)
 		sym_clear_all_valid();
-- 
2.43.0

Re: [PATCH v2 02/12] kconfig: refactor choice value calculation

[Niklas Söderlund]

Hello Yamada-san,

Thanks for your work.

I bisected a kconfig issue to this change, but I'm not sure how to 
resolve it and would appreciate your help.

Before this changes if I run menuconfig,

    $ ARCH=arm64 make menuconfig

The menu option for by SOC_RENESAS is visible at

    Device Drivers ->
        SOC (System On Chip) specific Drivers ->
            Renesas SoC driver support

However after this patch it is not.

Furthermore searching (/) for any config option protected by SOC_RENESAS 
in drivers/soc/renesas/Kconfig (e.g. ARCH_R8A77965) results in a search 
hit, but if I try to jump to it by pressing 1 all I get is a blank 
screen.

I'm not sure if a fix to the for mention Kconfig file is needed or if 
something else is wrong. This is still true for today's linux-next [1].

1. 985bf40edf43 ("Add linux-next specific files for 20240830")

On 2024-06-18 19:35:21 +0900, Masahiro Yamada wrote:
> Handling choices has always been in a PITA in Kconfig.
> 
> For example, fixes and reverts were repeated for randconfig with
> KCONFIG_ALLCONFIG:
> 
>  - 422c809f03f0 ("kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG")
>  - 23a5dfdad22a ("Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"")
>  - 8357b48549e1 ("kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG")
>  - 490f16171119 ("Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"")
> 
> As these commits pointed out, randconfig does not randomize choices when
> KCONFIG_ALLCONFIG is used. This issue still remains.
> 
> [Test Case]
> 
>     choice
>             prompt "choose"
> 
>     config A
>             bool "A"
> 
>     config B
>             bool "B"
> 
>     endchoice
> 
>     $ echo > all.config
>     $ make KCONFIG_ALLCONFIG=1 randconfig
> 
> The output is always as follows:
> 
>     CONFIG_A=y
>     # CONFIG_B is not set
> 
> Not only randconfig, but other all*config variants are also broken with
> KCONFIG_ALLCONFIG.
> 
> With the same Kconfig,
> 
>     $ echo '# CONFIG_A is not set' > all.config
>     $ make KCONFIG_ALLCONFIG=1 allyesconfig
> 
> You will get this:
> 
>     CONFIG_A=y
>     # CONFIG_B is not set
> 
> This is incorrect because it does not respect all.config.
> 
> The correct output should be:
> 
>     # CONFIG_A is not set
>     CONFIG_B=y
> 
> To handle user inputs more accurately, this commit refactors the code
> based on the following principles:
> 
>  - When a user value is given, Kconfig must set it immediately.
>    Do not defer it by setting SYMBOL_NEED_SET_CHOICE_VALUES.
> 
>  - The SYMBOL_DEF_USER flag must not be cleared, unless a new config
>    file is loaded. Kconfig must not forget user inputs.
> 
> In addition, user values for choices must be managed with priority.
> If user inputs conflict within a choice block, the newest value wins.
> The values given by randconfig have lower priority than explicit user
> inputs.
> 
> This commit implements it by using a linked list. Every time a choice
> block gets a new input, it is moved to the top of the list.
> 
> Let me explain how it works.
> 
> Let's say, we have a choice block that consists of five symbols:
> A, B, C, D, and E.
> 
> Initially, the linked list looks like this:
> 
>     A(=?) --> B(=?) --> C(=?) --> D(=?) --> E(=?)
> 
> Suppose randconfig is executed with the following KCONFIG_ALLCONFIG:
> 
>     CONFIG_C=y
>     # CONFIG_A is not set
>     CONFIG_D=y
> 
> First, CONFIG_C=y is read. C is set to 'y' and moved to the top.
> 
>     C(=y) --> A(=?) --> B(=?) --> D(=?) --> E(=?)
> 
> Next, '# CONFIG_A is not set' is read. A is set to 'n' and moved to
> the top.
> 
>     A(=n) --> C(=y) --> B(=?) --> D(=?) --> E(=?)
> 
> Then, 'CONFIG_D=y' is read. D is set to 'y' and moved to the top.
> 
>     D(=y) --> A(=n) --> C(=y) --> B(=?) --> E(=?)
> 
> Lastly, randconfig shuffles the order of the remaining symbols,
> resulting in:
> 
>     D(=y) --> A(=n) --> C(=y) --> B(=y) --> E(=y)
> or
>     D(=y) --> A(=n) --> C(=y) --> E(=y) --> B(=y)
> 
> When calculating the output, the linked list is traversed and the first
> visible symbol with 'y' is taken. In this case, it is D if visible.
> 
> If D is hidden by 'depends on', the next node, A, is examined. Since
> it is already specified as 'n', it is skipped. Next, C is checked, and
> selected if it is visible.
> 
> If C is also invisible, either B or E is chosen as a result of the
> randomization.
> 
> If B and E are also invisible, the linked list is traversed in the
> reverse order, and the least prioritized 'n' symbol is chosen. It is
> A in this case.
> 
> Now, Kconfig remembers all user values. This is a big difference from
> the previous implementation, where Kconfig would forget CONFIG_C=y when
> CONFIG_D=y appeared in the same input file.
> 
> The new appaorch respects user-specified values as much as possible.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
> Changes in v2:
>   - If all 'y' and '?' symbols are invisible, traverse the linked list
>     in the reverse order to pick up the least prioritized 'n'.
> 
>  scripts/kconfig/conf.c     | 131 +++++++++++++++---------------
>  scripts/kconfig/confdata.c |  54 +++----------
>  scripts/kconfig/expr.h     |  12 ++-
>  scripts/kconfig/lkc.h      |   7 +-
>  scripts/kconfig/menu.c     |  17 +---
>  scripts/kconfig/parser.y   |   4 +
>  scripts/kconfig/symbol.c   | 159 +++++++++++++++++++++++--------------
>  7 files changed, 187 insertions(+), 197 deletions(-)
> 
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 5dbdd9459f21..1c59998a62f7 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -114,41 +114,54 @@ static void set_randconfig_seed(void)
>  	srand(seed);
>  }
>  
> -static void randomize_choice_values(struct symbol *csym)
> +/**
> + * randomize_choice_values - randomize choice block
> + *
> + * @choice: menu entry for the choice
> + */
> +static void randomize_choice_values(struct menu *choice)
>  {
> -	struct property *prop;
> -	struct symbol *sym;
> -	struct expr *e;
> -	int cnt, def;
> -
> -	prop = sym_get_choice_prop(csym);
> -
> -	/* count entries in choice block */
> -	cnt = 0;
> -	expr_list_for_each_sym(prop->expr, e, sym)
> -		cnt++;
> +	struct menu *menu;
> +	int x;
> +	int cnt = 0;
>  
>  	/*
> -	 * find a random value and set it to yes,
> -	 * set the rest to no so we have only one set
> +	 * First, count the number of symbols to randomize. If sym_has_value()
> +	 * is true, it was specified by KCONFIG_ALLCONFIG. It needs to be
> +	 * respected.
>  	 */
> -	def = rand() % cnt;
> +	menu_for_each_sub_entry(menu, choice) {
> +		struct symbol *sym = menu->sym;
>  
> -	cnt = 0;
> -	expr_list_for_each_sym(prop->expr, e, sym) {
> -		if (def == cnt++) {
> -			sym->def[S_DEF_USER].tri = yes;
> -			csym->def[S_DEF_USER].val = sym;
> -		} else {
> -			sym->def[S_DEF_USER].tri = no;
> -		}
> -		sym->flags |= SYMBOL_DEF_USER;
> -		/* clear VALID to get value calculated */
> -		sym->flags &= ~SYMBOL_VALID;
> +		if (sym && !sym_has_value(sym))
> +			cnt++;
> +	}
> +
> +	while (cnt > 0) {
> +		x = rand() % cnt;
> +
> +		menu_for_each_sub_entry(menu, choice) {
> +			struct symbol *sym = menu->sym;
> +
> +			if (sym && !sym_has_value(sym))
> +				x--;
> +
> +			if (x < 0) {
> +				sym->def[S_DEF_USER].tri = yes;
> +				sym->flags |= SYMBOL_DEF_USER;
> +				/*
> +				 * Move the selected item to the _tail_ because
> +				 * this needs to have a lower priority than the
> +				 * user input from KCONFIG_ALLCONFIG.
> +				 */
> +				list_move_tail(&sym->choice_link,
> +					       &choice->choice_members);
> +
> +				break;
> +			}
> +		}
> +		cnt--;
>  	}
> -	csym->flags |= SYMBOL_DEF_USER;
> -	/* clear VALID to get value calculated */
> -	csym->flags &= ~SYMBOL_VALID;
>  }
>  
>  enum conf_def_mode {
> @@ -159,9 +172,9 @@ enum conf_def_mode {
>  	def_random
>  };
>  
> -static bool conf_set_all_new_symbols(enum conf_def_mode mode)
> +static void conf_set_all_new_symbols(enum conf_def_mode mode)
>  {
> -	struct symbol *sym, *csym;
> +	struct menu *menu;
>  	int cnt;
>  	/*
>  	 * can't go as the default in switch-case below, otherwise gcc whines
> @@ -170,7 +183,6 @@ static bool conf_set_all_new_symbols(enum conf_def_mode mode)
>  	int pby = 50; /* probability of bool     = y */
>  	int pty = 33; /* probability of tristate = y */
>  	int ptm = 33; /* probability of tristate = m */
> -	bool has_changed = false;
>  
>  	if (mode == def_random) {
>  		int n, p[3];
> @@ -217,14 +229,21 @@ static bool conf_set_all_new_symbols(enum conf_def_mode mode)
>  		}
>  	}
>  
> -	for_all_symbols(sym) {
> +	menu_for_each_entry(menu) {
> +		struct symbol *sym = menu->sym;
>  		tristate val;
>  
> -		if (sym_has_value(sym) || sym->flags & SYMBOL_VALID ||
> -		    (sym->type != S_BOOLEAN && sym->type != S_TRISTATE))
> +		if (!sym || !menu->prompt || sym_has_value(sym) ||
> +		    (sym->type != S_BOOLEAN && sym->type != S_TRISTATE) ||
> +		    sym_is_choice_value(sym))
>  			continue;
>  
> -		has_changed = true;
> +		if (sym_is_choice(sym)) {
> +			if (mode == def_random)
> +				randomize_choice_values(menu);
> +			continue;
> +		}
> +
>  		switch (mode) {
>  		case def_yes:
>  			val = yes;
> @@ -251,34 +270,10 @@ static bool conf_set_all_new_symbols(enum conf_def_mode mode)
>  			continue;
>  		}
>  		sym->def[S_DEF_USER].tri = val;
> -
> -		if (!(sym_is_choice(sym) && mode == def_random))
> -			sym->flags |= SYMBOL_DEF_USER;
> +		sym->flags |= SYMBOL_DEF_USER;
>  	}
>  
>  	sym_clear_all_valid();
> -
> -	if (mode != def_random) {
> -		for_all_symbols(csym) {
> -			if ((sym_is_choice(csym) && !sym_has_value(csym)) ||
> -			    sym_is_choice_value(csym))
> -				csym->flags |= SYMBOL_NEED_SET_CHOICE_VALUES;
> -		}
> -	}
> -
> -	for_all_symbols(csym) {
> -		if (sym_has_value(csym) || !sym_is_choice(csym))
> -			continue;
> -
> -		sym_calc_value(csym);
> -		if (mode == def_random)
> -			randomize_choice_values(csym);
> -		else
> -			set_all_choice_values(csym);
> -		has_changed = true;
> -	}
> -
> -	return has_changed;
>  }
>  
>  static void conf_rewrite_tristates(tristate old_val, tristate new_val)
> @@ -429,10 +424,9 @@ static void conf_choice(struct menu *menu)
>  {
>  	struct symbol *sym, *def_sym;
>  	struct menu *child;
> -	bool is_new;
> +	bool is_new = false;
>  
>  	sym = menu->sym;
> -	is_new = !sym_has_value(sym);
>  
>  	while (1) {
>  		int cnt, def;
> @@ -456,8 +450,10 @@ static void conf_choice(struct menu *menu)
>  				printf("%*c", indent, ' ');
>  			printf(" %d. %s (%s)", cnt, menu_get_prompt(child),
>  			       child->sym->name);
> -			if (!sym_has_value(child->sym))
> +			if (!sym_has_value(child->sym)) {
> +				is_new = true;
>  				printf(" (NEW)");
> +			}
>  			printf("\n");
>  		}
>  		printf("%*schoice", indent - 1, "");
> @@ -586,9 +582,7 @@ static void check_conf(struct menu *menu)
>  		return;
>  
>  	sym = menu->sym;
> -	if (sym && !sym_has_value(sym) &&
> -	    (sym_is_changeable(sym) || sym_is_choice(sym))) {
> -
> +	if (sym && !sym_has_value(sym) && sym_is_changeable(sym)) {
>  		switch (input_mode) {
>  		case listnewconfig:
>  			if (sym->name)
> @@ -804,8 +798,7 @@ int main(int ac, char **av)
>  		conf_set_all_new_symbols(def_default);
>  		break;
>  	case randconfig:
> -		/* Really nothing to do in this loop */
> -		while (conf_set_all_new_symbols(def_random)) ;
> +		conf_set_all_new_symbols(def_random);
>  		break;
>  	case defconfig:
>  		conf_set_all_new_symbols(def_default);
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 1ac7fc9ad756..05823f85402a 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -382,10 +382,7 @@ int conf_read_simple(const char *name, int def)
>  
>  	def_flags = SYMBOL_DEF << def;
>  	for_all_symbols(sym) {
> -		sym->flags |= SYMBOL_CHANGED;
>  		sym->flags &= ~(def_flags|SYMBOL_VALID);
> -		if (sym_is_choice(sym))
> -			sym->flags |= def_flags;
>  		switch (sym->type) {
>  		case S_INT:
>  		case S_HEX:
> @@ -399,6 +396,8 @@ int conf_read_simple(const char *name, int def)
>  	}
>  
>  	while (getline_stripped(&line, &line_asize, in) != -1) {
> +		struct menu *choice;
> +
>  		conf_lineno++;
>  
>  		if (!line[0]) /* blank line */
> @@ -460,15 +459,14 @@ int conf_read_simple(const char *name, int def)
>  		if (conf_set_sym_val(sym, def, def_flags, val))
>  			continue;
>  
> -		if (sym && sym_is_choice_value(sym)) {
> -			struct symbol *cs = prop_get_symbol(sym_get_choice_prop(sym));
> -			if (sym->def[def].tri == yes) {
> -				if (cs->def[def].tri != no)
> -					conf_warning("override: %s changes choice state", sym->name);
> -				cs->def[def].val = sym;
> -				cs->def[def].tri = yes;
> -			}
> -		}
> +		/*
> +		 * If this is a choice member, give it the highest priority.
> +		 * If conflicting CONFIG options are given from an input file,
> +		 * the last one wins.
> +		 */
> +		choice = sym_get_choice_menu(sym);
> +		if (choice)
> +			list_move(&sym->choice_link, &choice->choice_members);
>  	}
>  	free(line);
>  	fclose(in);
> @@ -514,18 +512,6 @@ int conf_read(const char *name)
>  		/* maybe print value in verbose mode... */
>  	}
>  
> -	for_all_symbols(sym) {
> -		if (sym_has_value(sym) && !sym_is_choice_value(sym)) {
> -			/* Reset values of generates values, so they'll appear
> -			 * as new, if they should become visible, but that
> -			 * doesn't quite work if the Kconfig and the saved
> -			 * configuration disagree.
> -			 */
> -			if (sym->visible == no && !conf_unsaved)
> -				sym->flags &= ~SYMBOL_DEF_USER;
> -		}
> -	}
> -
>  	if (conf_warnings || conf_unsaved)
>  		conf_set_changed(true);
>  
> @@ -1146,23 +1132,3 @@ void conf_set_changed_callback(void (*fn)(bool))
>  {
>  	conf_changed_callback = fn;
>  }
> -
> -void set_all_choice_values(struct symbol *csym)
> -{
> -	struct property *prop;
> -	struct symbol *sym;
> -	struct expr *e;
> -
> -	prop = sym_get_choice_prop(csym);
> -
> -	/*
> -	 * Set all non-assinged choice values to no
> -	 */
> -	expr_list_for_each_sym(prop->expr, e, sym) {
> -		if (!sym_has_value(sym))
> -			sym->def[S_DEF_USER].tri = no;
> -	}
> -	csym->flags |= SYMBOL_DEF_USER;
> -	/* clear VALID to get value calculated */
> -	csym->flags &= ~(SYMBOL_VALID | SYMBOL_NEED_SET_CHOICE_VALUES);
> -}
> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> index 7c0c242318bc..7acf27a4f454 100644
> --- a/scripts/kconfig/expr.h
> +++ b/scripts/kconfig/expr.h
> @@ -73,6 +73,8 @@ enum {
>   * Represents a configuration symbol.
>   *
>   * Choices are represented as a special kind of symbol with null name.
> + *
> + * @choice_link: linked to menu::choice_members
>   */
>  struct symbol {
>  	/* link node for the hash table */
> @@ -110,6 +112,8 @@ struct symbol {
>  	/* config entries associated with this symbol */
>  	struct list_head menus;
>  
> +	struct list_head choice_link;
> +
>  	/* SYMBOL_* flags */
>  	int flags;
>  
> @@ -133,7 +137,6 @@ struct symbol {
>  #define SYMBOL_CHOICEVAL  0x0020  /* used as a value in a choice block */
>  #define SYMBOL_VALID      0x0080  /* set when symbol.curr is calculated */
>  #define SYMBOL_WRITE      0x0200  /* write symbol to file (KCONFIG_CONFIG) */
> -#define SYMBOL_CHANGED    0x0400  /* ? */
>  #define SYMBOL_WRITTEN    0x0800  /* track info to avoid double-write to .config */
>  #define SYMBOL_CHECKED    0x2000  /* used during dependency checking */
>  #define SYMBOL_WARNED     0x8000  /* warning has been issued */
> @@ -145,9 +148,6 @@ struct symbol {
>  #define SYMBOL_DEF3       0x40000  /* symbol.def[S_DEF_3] is valid */
>  #define SYMBOL_DEF4       0x80000  /* symbol.def[S_DEF_4] is valid */
>  
> -/* choice values need to be set before calculating this symbol value */
> -#define SYMBOL_NEED_SET_CHOICE_VALUES  0x100000
> -
>  #define SYMBOL_MAXLENGTH	256
>  
>  /* A property represent the config options that can be associated
> @@ -204,6 +204,8 @@ struct property {
>   * for all front ends). Each symbol, menu, etc. defined in the Kconfig files
>   * gets a node. A symbol defined in multiple locations gets one node at each
>   * location.
> + *
> + * @choice_members: list of choice members with priority.
>   */
>  struct menu {
>  	/* The next menu node at the same level */
> @@ -223,6 +225,8 @@ struct menu {
>  
>  	struct list_head link;	/* link to symbol::menus */
>  
> +	struct list_head choice_members;
> +
>  	/*
>  	 * The prompt associated with the node. This holds the prompt for a
>  	 * symbol as well as the text for a menu or comment, along with the
> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> index 64dfc354dd5c..bdd37a16b040 100644
> --- a/scripts/kconfig/lkc.h
> +++ b/scripts/kconfig/lkc.h
> @@ -40,7 +40,6 @@ void zconf_nextfile(const char *name);
>  /* confdata.c */
>  extern struct gstr autoconf_cmd;
>  const char *conf_get_configname(void);
> -void set_all_choice_values(struct symbol *csym);
>  
>  /* confdata.c and expr.c */
>  static inline void xfwrite(const void *str, size_t len, size_t count, FILE *out)
> @@ -121,11 +120,7 @@ static inline tristate sym_get_tristate_value(struct symbol *sym)
>  	return sym->curr.tri;
>  }
>  
> -
> -static inline struct symbol *sym_get_choice_value(struct symbol *sym)
> -{
> -	return (struct symbol *)sym->curr.val;
> -}
> +struct symbol *sym_get_choice_value(struct symbol *sym);
>  
>  static inline bool sym_is_choice(struct symbol *sym)
>  {
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index bf5dcc05350b..170a269a8d7c 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -591,7 +591,6 @@ bool menu_is_empty(struct menu *menu)
>  
>  bool menu_is_visible(struct menu *menu)
>  {
> -	struct menu *child;
>  	struct symbol *sym;
>  	tristate visible;
>  
> @@ -610,21 +609,7 @@ bool menu_is_visible(struct menu *menu)
>  	} else
>  		visible = menu->prompt->visible.tri = expr_calc_value(menu->prompt->visible.expr);
>  
> -	if (visible != no)
> -		return true;
> -
> -	if (!sym || sym_get_tristate_value(menu->sym) == no)
> -		return false;
> -
> -	for (child = menu->list; child; child = child->next) {
> -		if (menu_is_visible(child)) {
> -			if (sym)
> -				sym->flags |= SYMBOL_DEF_USER;
> -			return true;
> -		}
> -	}
> -
> -	return false;
> +	return visible != no;
>  }
>  
>  const char *menu_get_prompt(struct menu *menu)
> diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
> index 20538e1d3788..9d58544b0255 100644
> --- a/scripts/kconfig/parser.y
> +++ b/scripts/kconfig/parser.y
> @@ -157,6 +157,9 @@ config_stmt: config_entry_start config_option_list
>  				current_entry->filename, current_entry->lineno);
>  			yynerrs++;
>  		}
> +
> +		list_add_tail(&current_entry->sym->choice_link,
> +			      &current_choice->choice_members);
>  	}
>  
>  	printd(DEBUG_PARSE, "%s:%d:endconfig\n", cur_filename, cur_lineno);
> @@ -240,6 +243,7 @@ choice: T_CHOICE T_EOL
>  	menu_add_entry(sym);
>  	menu_add_expr(P_CHOICE, NULL, NULL);
>  	menu_set_type(S_BOOLEAN);
> +	INIT_LIST_HEAD(&current_entry->choice_members);
>  
>  	printd(DEBUG_PARSE, "%s:%d:choice\n", cur_filename, cur_lineno);
>  };
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 8df0a75f40b9..329c7bd314cf 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -188,7 +188,6 @@ static void sym_set_changed(struct symbol *sym)
>  {
>  	struct menu *menu;
>  
> -	sym->flags |= SYMBOL_CHANGED;
>  	list_for_each_entry(menu, &sym->menus, link)
>  		menu->flags |= MENU_CHANGED;
>  }
> @@ -282,36 +281,95 @@ struct symbol *sym_choice_default(struct symbol *sym)
>  	return NULL;
>  }
>  
> -static struct symbol *sym_calc_choice(struct symbol *sym)
> +/*
> + * sym_calc_choice - calculate symbol values in a choice
> + *
> + * @choice: a menu of the choice
> + *
> + * Return: a chosen symbol
> + */
> +static struct symbol *sym_calc_choice(struct menu *choice)
>  {
> -	struct symbol *def_sym;
> -	struct property *prop;
> -	struct expr *e;
> -	int flags;
> +	struct symbol *res = NULL;
> +	struct symbol *sym;
> +	struct menu *menu;
>  
> -	/* first calculate all choice values' visibilities */
> -	flags = sym->flags;
> -	prop = sym_get_choice_prop(sym);
> -	expr_list_for_each_sym(prop->expr, e, def_sym) {
> -		sym_calc_visibility(def_sym);
> -		if (def_sym->visible != no)
> -			flags &= def_sym->flags;
> +	/* Traverse the list of choice members in the priority order. */
> +	list_for_each_entry(sym, &choice->choice_members, choice_link) {
> +		sym_calc_visibility(sym);
> +		if (sym->visible == no)
> +			continue;
> +
> +		/* The first visible symble with the user value 'y'. */
> +		if (sym_has_value(sym) && sym->def[S_DEF_USER].tri == yes) {
> +			res = sym;
> +			break;
> +		}
>  	}
>  
> -	sym->flags &= flags | ~SYMBOL_DEF_USER;
> +	/*
> +	 * If 'y' is not found in the user input, use the default, unless it is
> +	 * explicitly set to 'n'.
> +	 */
> +	if (!res) {
> +		res = sym_choice_default(choice->sym);
> +		if (res && sym_has_value(res) && res->def[S_DEF_USER].tri == no)
> +			res = NULL;
> +	}
>  
> -	/* is the user choice visible? */
> -	def_sym = sym->def[S_DEF_USER].val;
> -	if (def_sym && def_sym->visible != no)
> -		return def_sym;
> +	/* Still not found. Pick up the first visible, user-unspecified symbol. */
> +	if (!res) {
> +		menu_for_each_sub_entry(menu, choice) {
> +			sym = menu->sym;
>  
> -	def_sym = sym_choice_default(sym);
> +			if (!sym || sym->visible == no || sym_has_value(sym))
> +				continue;
>  
> -	if (def_sym == NULL)
> -		/* no choice? reset tristate value */
> -		sym->curr.tri = no;
> +			res = sym;
> +			break;
> +		}
> +	}
>  
> -	return def_sym;
> +	/*
> +	 * Still not found. Traverse the linked list in the _reverse_ order to
> +	 * pick up the least prioritized 'n'.
> +	 */
> +	if (!res) {
> +		list_for_each_entry_reverse(sym, &choice->choice_members,
> +					    choice_link) {
> +			if (sym->visible == no)
> +				continue;
> +
> +			res = sym;
> +			break;
> +		}
> +	}
> +
> +	menu_for_each_sub_entry(menu, choice) {
> +		tristate val;
> +
> +		sym = menu->sym;
> +
> +		if (!sym || sym->visible == no)
> +			continue;
> +
> +		val = sym == res ? yes : no;
> +
> +		if (sym->curr.tri != val)
> +			sym_set_changed(sym);
> +
> +		sym->curr.tri = val;
> +		sym->flags |= SYMBOL_VALID | SYMBOL_WRITE;
> +	}
> +
> +	return res;
> +}
> +
> +struct symbol *sym_get_choice_value(struct symbol *sym)
> +{
> +	struct menu *menu = list_first_entry(&sym->menus, struct menu, link);
> +
> +	return sym_calc_choice(menu);
>  }
>  
>  static void sym_warn_unmet_dep(struct symbol *sym)
> @@ -347,7 +405,7 @@ void sym_calc_value(struct symbol *sym)
>  {
>  	struct symbol_value newval, oldval;
>  	struct property *prop;
> -	struct expr *e;
> +	struct menu *choice_menu;
>  
>  	if (!sym)
>  		return;
> @@ -355,13 +413,6 @@ void sym_calc_value(struct symbol *sym)
>  	if (sym->flags & SYMBOL_VALID)
>  		return;
>  
> -	if (sym_is_choice_value(sym) &&
> -	    sym->flags & SYMBOL_NEED_SET_CHOICE_VALUES) {
> -		sym->flags &= ~SYMBOL_NEED_SET_CHOICE_VALUES;
> -		prop = sym_get_choice_prop(sym);
> -		sym_calc_value(prop_get_symbol(prop));
> -	}
> -
>  	sym->flags |= SYMBOL_VALID;
>  
>  	oldval = sym->curr;
> @@ -400,9 +451,11 @@ 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;
> +		choice_menu = sym_get_choice_menu(sym);
> +
> +		if (choice_menu) {
> +			sym_calc_choice(choice_menu);
> +			newval.tri = sym->curr.tri;
>  		} else {
>  			if (sym->visible != no) {
>  				/* if the symbol is visible use the user value
> @@ -461,8 +514,6 @@ void sym_calc_value(struct symbol *sym)
>  	}
>  
>  	sym->curr = newval;
> -	if (sym_is_choice(sym) && newval.tri == yes)
> -		sym->curr.val = sym_calc_choice(sym);
>  	sym_validate_range(sym);
>  
>  	if (memcmp(&oldval, &sym->curr, sizeof(oldval))) {
> @@ -473,23 +524,8 @@ void sym_calc_value(struct symbol *sym)
>  		}
>  	}
>  
> -	if (sym_is_choice(sym)) {
> -		struct symbol *choice_sym;
> -
> -		prop = sym_get_choice_prop(sym);
> -		expr_list_for_each_sym(prop->expr, e, choice_sym) {
> -			if ((sym->flags & SYMBOL_WRITE) &&
> -			    choice_sym->visible != no)
> -				choice_sym->flags |= SYMBOL_WRITE;
> -			if (sym->flags & SYMBOL_CHANGED)
> -				sym_set_changed(choice_sym);
> -		}
> -
> +	if (sym_is_choice(sym))
>  		sym->flags &= ~SYMBOL_WRITE;
> -	}
> -
> -	if (sym->flags & SYMBOL_NEED_SET_CHOICE_VALUES)
> -		set_all_choice_values(sym);
>  }
>  
>  void sym_clear_all_valid(void)
> @@ -523,15 +559,15 @@ bool sym_set_tristate_value(struct symbol *sym, tristate val)
>  {
>  	tristate oldval = sym_get_tristate_value(sym);
>  
> -	if (oldval != val && !sym_tristate_within_range(sym, val))
> +	if (!sym_tristate_within_range(sym, val))
>  		return false;
>  
> -	if (!(sym->flags & SYMBOL_DEF_USER)) {
> +	if (!(sym->flags & SYMBOL_DEF_USER) || sym->def[S_DEF_USER].tri != val) {
> +		sym->def[S_DEF_USER].tri = val;
>  		sym->flags |= SYMBOL_DEF_USER;
>  		sym_set_changed(sym);
>  	}
>  
> -	sym->def[S_DEF_USER].tri = val;
>  	if (oldval != val)
>  		sym_clear_all_valid();
>  
> @@ -565,10 +601,17 @@ void choice_set_value(struct menu *choice, struct symbol *sym)
>  
>  		menu->sym->def[S_DEF_USER].tri = val;
>  		menu->sym->flags |= SYMBOL_DEF_USER;
> -	}
>  
> -	choice->sym->def[S_DEF_USER].val = sym;
> -	choice->sym->flags |= SYMBOL_DEF_USER;
> +		/*
> +		 * Now, the user has explicitly enabled or disabled this symbol,
> +		 * it should be given the highest priority. We are possibly
> +		 * setting multiple symbols to 'n', where the first symbol is
> +		 * given the least prioritized 'n'. This works well when the
> +		 * choice block ends up with selecting 'n' symbol.
> +		 * (see sym_calc_choice())
> +		 */
> +		list_move(&menu->sym->choice_link, &choice->choice_members);
> +	}
>  
>  	if (changed)
>  		sym_clear_all_valid();
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v2 02/12] kconfig: refactor choice value calculation

[Masahiro Yamada]

On Sun, Sep 1, 2024 at 2:31 AM Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
>
> Hello Yamada-san,
>
> Thanks for your work.
>
> I bisected a kconfig issue to this change, but I'm not sure how to
> resolve it and would appreciate your help.
>
> Before this changes if I run menuconfig,
>
>     $ ARCH=arm64 make menuconfig
>
> The menu option for by SOC_RENESAS is visible at
>
>     Device Drivers ->
>         SOC (System On Chip) specific Drivers ->
>             Renesas SoC driver support
>
> However after this patch it is not.
>
> Furthermore searching (/) for any config option protected by SOC_RENESAS
> in drivers/soc/renesas/Kconfig (e.g. ARCH_R8A77965) results in a search
> hit, but if I try to jump to it by pressing 1 all I get is a blank
> screen.
>
> I'm not sure if a fix to the for mention Kconfig file is needed or if
> something else is wrong. This is still true for today's linux-next [1].
>
> 1. 985bf40edf43 ("Add linux-next specific files for 20240830")






The prompt of SOC_RENESAS depends on
COMPILE_TEST && !ARCH_RENESAS.
Hence, it is hidden by default.

Pressing (1) navigated to the nearest parent menu.

Setting COMPILE_TEST=y and ARCH_RENESAS=n made it visible.

All look quite normal to me.



 Symbol: SOC_RENESAS [=y]
 Type  : bool
 Defined at drivers/soc/renesas/Kconfig:2
   Prompt: Renesas SoC driver support
   Visible if: COMPILE_TEST [=n] && !ARCH_RENESAS [=y]
   Location:
     -> Device Drivers
 (1)   -> SOC (System On Chip) specific Drivers
         -> Renesas SoC driver support (SOC_RENESAS [=y])
 Selects: GPIOLIB [=y] && PINCTRL [=y] && SOC_BUS [=y]






-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 02/12] kconfig: refactor choice value calculation

[Niklas Söderlund]

On 2024-09-01 18:10:56 +0900, Masahiro Yamada wrote:
> On Sun, Sep 1, 2024 at 2:31 AM Niklas Söderlund
> <niklas.soderlund@ragnatech.se> wrote:
> >
> > Hello Yamada-san,
> >
> > Thanks for your work.
> >
> > I bisected a kconfig issue to this change, but I'm not sure how to
> > resolve it and would appreciate your help.
> >
> > Before this changes if I run menuconfig,
> >
> >     $ ARCH=arm64 make menuconfig
> >
> > The menu option for by SOC_RENESAS is visible at
> >
> >     Device Drivers ->
> >         SOC (System On Chip) specific Drivers ->
> >             Renesas SoC driver support
> >
> > However after this patch it is not.
> >
> > Furthermore searching (/) for any config option protected by SOC_RENESAS
> > in drivers/soc/renesas/Kconfig (e.g. ARCH_R8A77965) results in a search
> > hit, but if I try to jump to it by pressing 1 all I get is a blank
> > screen.
> >
> > I'm not sure if a fix to the for mention Kconfig file is needed or if
> > something else is wrong. This is still true for today's linux-next [1].
> >
> > 1. 985bf40edf43 ("Add linux-next specific files for 20240830")
> 
> 
> 
> 
> 
> 
> The prompt of SOC_RENESAS depends on
> COMPILE_TEST && !ARCH_RENESAS.
> Hence, it is hidden by default.
> 
> Pressing (1) navigated to the nearest parent menu.
> 
> Setting COMPILE_TEST=y and ARCH_RENESAS=n made it visible.

It might be that the Kconfig file needs an update, but the behavior 
changed with this commit.

Before this commit the entry was visible and symbols hidden under the 
SOC_RENESAS where selectable and I could for example select/deselect 
ARCH_R8A77965 in menuconfig. After this change I can't navigate to 
ARCH_R8A77965 at all.

But you are correct that both before and after the commit the 
SOC_RENESAS symbol is set to only be visible if 'COMPILE_TEST && 
!ARCH_RENESAS' so this might have been an issue that was fixed and that 
the Kconfig file needs to be updated.

Thanks for your help!

> 
> All look quite normal to me.
> 
> 
> 
>  Symbol: SOC_RENESAS [=y]
>  Type  : bool
>  Defined at drivers/soc/renesas/Kconfig:2
>    Prompt: Renesas SoC driver support
>    Visible if: COMPILE_TEST [=n] && !ARCH_RENESAS [=y]
>    Location:
>      -> Device Drivers
>  (1)   -> SOC (System On Chip) specific Drivers
>          -> Renesas SoC driver support (SOC_RENESAS [=y])
>  Selects: GPIOLIB [=y] && PINCTRL [=y] && SOC_BUS [=y]
> 
> 
> 
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada

-- 
Kind Regards,
Niklas Söderlund

[PATCH v2 03/12] kconfig: remove sym_get_choice_value()

[Masahiro Yamada]

sym_get_choice_value(menu->sym) is equivalent to sym_calc_choice(menu).

Convert all call sites of sym_get_choice_value() and then remove it.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

 scripts/kconfig/conf.c   | 6 ++----
 scripts/kconfig/gconf.c  | 2 +-
 scripts/kconfig/lkc.h    | 3 +--
 scripts/kconfig/mconf.c  | 6 +++---
 scripts/kconfig/nconf.c  | 6 +++---
 scripts/kconfig/symbol.c | 9 +--------
 6 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 1c59998a62f7..3d7d454c54da 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -422,17 +422,15 @@ static int conf_sym(struct menu *menu)
 
 static void conf_choice(struct menu *menu)
 {
-	struct symbol *sym, *def_sym;
+	struct symbol *def_sym;
 	struct menu *child;
 	bool is_new = false;
 
-	sym = menu->sym;
-
 	while (1) {
 		int cnt, def;
 
 		printf("%*s%s\n", indent - 1, "", menu_get_prompt(menu));
-		def_sym = sym_get_choice_value(sym);
+		def_sym = sym_calc_choice(menu);
 		cnt = def = 0;
 		line[0] = 0;
 		for (child = menu->list; child; child = child->next) {
diff --git a/scripts/kconfig/gconf.c b/scripts/kconfig/gconf.c
index 380421a5cfb2..6b50e25133e3 100644
--- a/scripts/kconfig/gconf.c
+++ b/scripts/kconfig/gconf.c
@@ -1054,7 +1054,7 @@ static gchar **fill_row(struct menu *menu)
 
 	if (sym_is_choice(sym)) {	// parse childs for getting final value
 		struct menu *child;
-		struct symbol *def_sym = sym_get_choice_value(sym);
+		struct symbol *def_sym = sym_calc_choice(menu);
 		struct menu *def_menu = NULL;
 
 		for (child = menu->list; child; child = child->next) {
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index bdd37a16b040..d820272a85fb 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -110,6 +110,7 @@ void menu_get_ext_help(struct menu *menu, struct gstr *help);
 /* symbol.c */
 void sym_clear_all_valid(void);
 struct symbol *sym_choice_default(struct symbol *sym);
+struct symbol *sym_calc_choice(struct menu *choice);
 struct property *sym_get_range_prop(struct symbol *sym);
 const char *sym_get_string_default(struct symbol *sym);
 struct symbol *sym_check_deps(struct symbol *sym);
@@ -120,8 +121,6 @@ static inline tristate sym_get_tristate_value(struct symbol *sym)
 	return sym->curr.tri;
 }
 
-struct symbol *sym_get_choice_value(struct symbol *sym);
-
 static inline bool sym_is_choice(struct symbol *sym)
 {
 	/* A choice is a symbol with no name */
diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index 03709eb734ae..4a0a97bb342f 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -514,7 +514,7 @@ static void build_conf(struct menu *menu)
 
 	type = sym_get_type(sym);
 	if (sym_is_choice(sym)) {
-		struct symbol *def_sym = sym_get_choice_value(sym);
+		struct symbol *def_sym = sym_calc_choice(menu);
 		struct menu *def_menu = NULL;
 
 		child_count++;
@@ -600,7 +600,7 @@ static void conf_choice(struct menu *menu)
 	struct menu *child;
 	struct symbol *active;
 
-	active = sym_get_choice_value(menu->sym);
+	active = sym_calc_choice(menu);
 	while (1) {
 		int res;
 		int selected;
@@ -619,7 +619,7 @@ static void conf_choice(struct menu *menu)
 			item_set_data(child);
 			if (child->sym == active)
 				item_set_selected(1);
-			if (child->sym == sym_get_choice_value(menu->sym))
+			if (child->sym == sym_calc_choice(menu))
 				item_set_tag('X');
 		}
 		dialog_clear();
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index eb5fc3ccaf9d..1456e24969aa 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -815,7 +815,7 @@ static void build_conf(struct menu *menu)
 
 	type = sym_get_type(sym);
 	if (sym_is_choice(sym)) {
-		struct symbol *def_sym = sym_get_choice_value(sym);
+		struct symbol *def_sym = sym_calc_choice(menu);
 		struct menu *def_menu = NULL;
 
 		child_count++;
@@ -1239,7 +1239,7 @@ static void conf_choice(struct menu *menu)
 		.pattern = "",
 	};
 
-	active = sym_get_choice_value(menu->sym);
+	active = sym_calc_choice(menu);
 	/* this is mostly duplicated from the conf() function. */
 	while (!global_exit) {
 		reset_menu();
@@ -1248,7 +1248,7 @@ static void conf_choice(struct menu *menu)
 			if (!show_all_items && !menu_is_visible(child))
 				continue;
 
-			if (child->sym == sym_get_choice_value(menu->sym))
+			if (child->sym == sym_calc_choice(menu))
 				item_make(child, ':', "<X> %s",
 						menu_get_prompt(child));
 			else if (child->sym)
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 329c7bd314cf..344a241e1e94 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -288,7 +288,7 @@ struct symbol *sym_choice_default(struct symbol *sym)
  *
  * Return: a chosen symbol
  */
-static struct symbol *sym_calc_choice(struct menu *choice)
+struct symbol *sym_calc_choice(struct menu *choice)
 {
 	struct symbol *res = NULL;
 	struct symbol *sym;
@@ -365,13 +365,6 @@ static struct symbol *sym_calc_choice(struct menu *choice)
 	return res;
 }
 
-struct symbol *sym_get_choice_value(struct symbol *sym)
-{
-	struct menu *menu = list_first_entry(&sym->menus, struct menu, link);
-
-	return sym_calc_choice(menu);
-}
-
 static void sym_warn_unmet_dep(struct symbol *sym)
 {
 	struct gstr gs = str_new();
-- 
2.43.0

[PATCH v2 04/12] kconfig: remove conf_unsaved in conf_read_simple()

[Masahiro Yamada]

This variable is unnecessary. Call conf_set_changed(true) directly.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

 scripts/kconfig/confdata.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 05823f85402a..4359fbc9255b 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -477,7 +477,6 @@ int conf_read_simple(const char *name, int def)
 int conf_read(const char *name)
 {
 	struct symbol *sym;
-	int conf_unsaved = 0;
 
 	conf_set_changed(false);
 
@@ -508,11 +507,11 @@ int conf_read(const char *name)
 		} else if (!sym_has_value(sym) && !(sym->flags & SYMBOL_WRITE))
 			/* no previous value and not saved */
 			continue;
-		conf_unsaved++;
+		conf_set_changed(true);
 		/* maybe print value in verbose mode... */
 	}
 
-	if (conf_warnings || conf_unsaved)
+	if (conf_warnings)
 		conf_set_changed(true);
 
 	return 0;
-- 
2.43.0

[PATCH v2 05/12] kconfig: change sym_choice_default() to take the choice menu

[Masahiro Yamada]

Change the argument of sym_choice_default() to ease further cleanups.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

 scripts/kconfig/confdata.c | 2 +-
 scripts/kconfig/lkc.h      | 2 +-
 scripts/kconfig/symbol.c   | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 4359fbc9255b..76193ce5a792 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -779,7 +779,7 @@ int conf_write_defconfig(const char *filename)
 		if (choice) {
 			struct symbol *ds;
 
-			ds = sym_choice_default(choice->sym);
+			ds = sym_choice_default(choice);
 			if (sym == ds && sym_get_tristate_value(sym) == yes)
 				continue;
 		}
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index d820272a85fb..586a5e11f51e 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -109,7 +109,7 @@ void menu_get_ext_help(struct menu *menu, struct gstr *help);
 
 /* symbol.c */
 void sym_clear_all_valid(void);
-struct symbol *sym_choice_default(struct symbol *sym);
+struct symbol *sym_choice_default(struct menu *choice);
 struct symbol *sym_calc_choice(struct menu *choice);
 struct property *sym_get_range_prop(struct symbol *sym);
 const char *sym_get_string_default(struct symbol *sym);
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 344a241e1e94..3d68ab8e1eb4 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -255,14 +255,14 @@ static void sym_calc_visibility(struct symbol *sym)
  * Next locate the first visible choice value
  * Return NULL if none was found
  */
-struct symbol *sym_choice_default(struct symbol *sym)
+struct symbol *sym_choice_default(struct menu *choice)
 {
 	struct symbol *def_sym;
 	struct property *prop;
 	struct expr *e;
 
 	/* any of the defaults visible? */
-	for_all_defaults(sym, prop) {
+	for_all_defaults(choice->sym, prop) {
 		prop->visible.tri = expr_calc_value(prop->visible.expr);
 		if (prop->visible.tri == no)
 			continue;
@@ -272,7 +272,7 @@ struct symbol *sym_choice_default(struct symbol *sym)
 	}
 
 	/* just get the first visible value */
-	prop = sym_get_choice_prop(sym);
+	prop = sym_get_choice_prop(choice->sym);
 	expr_list_for_each_sym(prop->expr, e, def_sym)
 		if (def_sym->visible != no)
 			return def_sym;
@@ -312,7 +312,7 @@ struct symbol *sym_calc_choice(struct menu *choice)
 	 * explicitly set to 'n'.
 	 */
 	if (!res) {
-		res = sym_choice_default(choice->sym);
+		res = sym_choice_default(choice);
 		if (res && sym_has_value(res) && res->def[S_DEF_USER].tri == no)
 			res = NULL;
 	}
-- 
2.43.0

[PATCH v2 06/12] kconfig: use menu_list_for_each_sym() in sym_choice_default()

[Masahiro Yamada]

Choices and their members are associated via the P_CHOICE property.

Currently, sym_get_choice_prop() and expr_list_for_each_sym() are
used to iterate on choice members.

Replace them with menu_for_each_sub_entry(), which achieves the same
without relying on P_CHOICE.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

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

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 3d68ab8e1eb4..56e7a76e7a77 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -257,9 +257,9 @@ static void sym_calc_visibility(struct symbol *sym)
  */
 struct symbol *sym_choice_default(struct menu *choice)
 {
+	struct menu *menu;
 	struct symbol *def_sym;
 	struct property *prop;
-	struct expr *e;
 
 	/* any of the defaults visible? */
 	for_all_defaults(choice->sym, prop) {
@@ -272,10 +272,9 @@ struct symbol *sym_choice_default(struct menu *choice)
 	}
 
 	/* just get the first visible value */
-	prop = sym_get_choice_prop(choice->sym);
-	expr_list_for_each_sym(prop->expr, e, def_sym)
-		if (def_sym->visible != no)
-			return def_sym;
+	menu_for_each_sub_entry(menu, choice)
+		if (menu->sym && menu->sym->visible != no)
+			return menu->sym;
 
 	/* failed to locate any defaults */
 	return NULL;
-- 
2.43.0

[PATCH v2 07/12] kconfig: remove expr_list_for_each_sym() macro

[Masahiro Yamada]

All users of this macro have been converted. Remove it.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

 scripts/kconfig/expr.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 7acf27a4f454..1d1c4442c941 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -43,9 +43,6 @@ struct expr {
 #define EXPR_AND(dep1, dep2)	(((dep1)<(dep2))?(dep1):(dep2))
 #define EXPR_NOT(dep)		(2-(dep))
 
-#define expr_list_for_each_sym(l, e, s) \
-	for (e = (l); e && (s = e->right.sym); e = e->left.expr)
-
 struct expr_value {
 	struct expr *expr;
 	tristate tri;
-- 
2.43.0

[PATCH v2 08/12] kconfig: use sym_get_choice_menu() in sym_check_print_recursive()

[Masahiro Yamada]

Choices and their members are associated via the P_CHOICE property.

Currently, prop_get_symbol(sym_get_choice_prop()) is used to obtain
the choice of the given choice member.

Replace it with sym_get_choice_menu(), which retrieves the choice
without relying on P_CHOICE.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

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

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 56e7a76e7a77..79f1b5e1cc9e 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1078,12 +1078,14 @@ static void sym_check_print_recursive(struct symbol *last_sym)
 	struct dep_stack *stack;
 	struct symbol *sym, *next_sym;
 	struct menu *menu = NULL;
+	struct menu *choice;
 	struct property *prop;
 	struct dep_stack cv_stack;
 
-	if (sym_is_choice_value(last_sym)) {
+	choice = sym_get_choice_menu(last_sym);
+	if (choice) {
 		dep_stack_insert(&cv_stack, last_sym);
-		last_sym = prop_get_symbol(sym_get_choice_prop(last_sym));
+		last_sym = choice->sym;
 	}
 
 	for (stack = check_top; stack != NULL; stack = stack->prev)
-- 
2.43.0

[PATCH v2 09/12] kconfig: use sym_get_choice_menu() in sym_check_choice_deps()

[Masahiro Yamada]

Choices and their members are associated via the P_CHOICE property.

Currently, prop_get_symbol(sym_get_choice_prop()) is used to obtain
the choice of the given choice member.

Replace it with sym_get_choice_menu(), which retrieves the choice
without relying on P_CHOICE.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

 scripts/kconfig/symbol.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 79f1b5e1cc9e..22c15a103371 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1280,9 +1280,13 @@ static struct symbol *sym_check_choice_deps(struct symbol *choice)
 		if (menu->sym)
 			menu->sym->flags &= ~SYMBOL_CHECK;
 
-	if (sym2 && sym_is_choice_value(sym2) &&
-	    prop_get_symbol(sym_get_choice_prop(sym2)) == choice)
-		sym2 = choice;
+	if (sym2) {
+		struct menu *choice_menu2;
+
+		choice_menu2 = sym_get_choice_menu(sym2);
+		if (choice_menu2 == choice_menu)
+			sym2 = choice;
+	}
 
 	dep_stack_remove();
 
-- 
2.43.0

[PATCH v2 10/12] kconfig: use sym_get_choice_menu() in sym_check_deps()

[Masahiro Yamada]

Choices and their members are associated via the P_CHOICE property.

Currently, prop_get_symbol(sym_get_choice_prop()) is used to obtain
the choice of the given choice member.

Replace it with sym_get_choice_menu(), which retrieves the choice
without relying on P_CHOICE.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

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

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 22c15a103371..b50911bcb08d 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1295,8 +1295,8 @@ static struct symbol *sym_check_choice_deps(struct symbol *choice)
 
 struct symbol *sym_check_deps(struct symbol *sym)
 {
+	struct menu *choice;
 	struct symbol *sym2;
-	struct property *prop;
 
 	if (sym->flags & SYMBOL_CHECK) {
 		sym_check_print_recursive(sym);
@@ -1305,13 +1305,13 @@ struct symbol *sym_check_deps(struct symbol *sym)
 	if (sym->flags & SYMBOL_CHECKED)
 		return NULL;
 
-	if (sym_is_choice_value(sym)) {
+	choice = sym_get_choice_menu(sym);
+	if (choice) {
 		struct dep_stack stack;
 
 		/* for choice groups start the check with main choice symbol */
 		dep_stack_insert(&stack, sym);
-		prop = sym_get_choice_prop(sym);
-		sym2 = sym_check_deps(prop_get_symbol(prop));
+		sym2 = sym_check_deps(choice->sym);
 		dep_stack_remove();
 	} else if (sym_is_choice(sym)) {
 		sym2 = sym_check_choice_deps(sym);
-- 
2.43.0

[PATCH v2 11/12] kconfig: remove P_CHOICE property

[Masahiro Yamada]

P_CHOICE is a pseudo property used to link a choice with its members.

There is no more code relying on this, except for some debug code.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

 scripts/kconfig/expr.h      |  4 +---
 scripts/kconfig/lkc_proto.h |  1 -
 scripts/kconfig/menu.c      |  8 +-------
 scripts/kconfig/parser.y    |  4 ----
 scripts/kconfig/qconf.cc    |  8 --------
 scripts/kconfig/symbol.c    | 14 +-------------
 6 files changed, 3 insertions(+), 36 deletions(-)

diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 1d1c4442c941..58fd4c8c3762 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -167,7 +167,6 @@ enum prop_type {
 	P_COMMENT,  /* text associated with a comment */
 	P_MENU,     /* prompt associated with a menu or menuconfig symbol */
 	P_DEFAULT,  /* default y */
-	P_CHOICE,   /* choice value */
 	P_SELECT,   /* select BAR */
 	P_IMPLY,    /* imply BAR */
 	P_RANGE,    /* range 7..100 (for a symbol) */
@@ -181,7 +180,7 @@ struct property {
 	struct expr_value visible;
 	struct expr *expr;         /* the optional conditional part of the property */
 	struct menu *menu;         /* the menu the property are associated with
-	                            * valid for: P_SELECT, P_RANGE, P_CHOICE,
+	                            * valid for: P_SELECT, P_RANGE,
 	                            * P_PROMPT, P_DEFAULT, P_MENU, P_COMMENT */
 	const char *filename;      /* what file was this property defined */
 	int lineno;                /* what lineno was this property defined */
@@ -191,7 +190,6 @@ struct property {
 	for (st = sym->prop; st; st = st->next) \
 		if (st->type == (tok))
 #define for_all_defaults(sym, st) for_all_properties(sym, st, P_DEFAULT)
-#define for_all_choices(sym, st) for_all_properties(sym, st, P_CHOICE)
 #define for_all_prompts(sym, st) \
 	for (st = sym->prop; st; st = st->next) \
 		if (st->text)
diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
index 1221709efac1..49cc649d2810 100644
--- a/scripts/kconfig/lkc_proto.h
+++ b/scripts/kconfig/lkc_proto.h
@@ -34,7 +34,6 @@ bool sym_string_valid(struct symbol *sym, const char *newval);
 bool sym_string_within_range(struct symbol *sym, const char *str);
 bool sym_set_string_value(struct symbol *sym, const char *newval);
 bool sym_is_changeable(struct symbol *sym);
-struct property * sym_get_choice_prop(struct symbol *sym);
 struct menu *sym_get_choice_menu(struct symbol *sym);
 const char * sym_get_string_value(struct symbol *sym);
 
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 170a269a8d7c..0353f621ecaa 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -306,7 +306,7 @@ static void _menu_finalize(struct menu *parent, bool inside_choice)
 	struct menu *menu, *last_menu;
 	struct symbol *sym;
 	struct property *prop;
-	struct expr *parentdep, *basedep, *dep, *dep2, **ep;
+	struct expr *parentdep, *basedep, *dep, *dep2;
 
 	sym = parent->sym;
 	if (parent->list) {
@@ -492,12 +492,6 @@ static void _menu_finalize(struct menu *parent, bool inside_choice)
 		    menu->sym && !sym_is_choice_value(menu->sym)) {
 			current_entry = menu;
 			menu->sym->flags |= SYMBOL_CHOICEVAL;
-			menu_add_symbol(P_CHOICE, sym, NULL);
-			prop = sym_get_choice_prop(sym);
-			for (ep = &prop->expr; *ep; ep = &(*ep)->left.expr)
-				;
-			*ep = expr_alloc_one(E_LIST, NULL);
-			(*ep)->right.sym = menu->sym;
 		}
 
 		/*
diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
index 9d58544b0255..745c82ee15d0 100644
--- a/scripts/kconfig/parser.y
+++ b/scripts/kconfig/parser.y
@@ -241,7 +241,6 @@ choice: T_CHOICE T_EOL
 	struct symbol *sym = sym_lookup(NULL, 0);
 
 	menu_add_entry(sym);
-	menu_add_expr(P_CHOICE, NULL, NULL);
 	menu_set_type(S_BOOLEAN);
 	INIT_LIST_HEAD(&current_entry->choice_members);
 
@@ -696,9 +695,6 @@ static void print_symbol(FILE *out, struct menu *menu)
 			}
 			fputc('\n', out);
 			break;
-		case P_CHOICE:
-			fputs("  #choice value\n", out);
-			break;
 		case P_SELECT:
 			fputs( "  select ", out);
 			expr_fprint(prop->expr, out);
diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index 30346e294d1a..7d239c032b3d 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -1101,14 +1101,6 @@ QString ConfigInfoView::debug_info(struct symbol *sym)
 				   &stream, E_NONE);
 			stream << "<br>";
 			break;
-		case P_CHOICE:
-			if (sym_is_choice(sym)) {
-				stream << "choice: ";
-				expr_print(prop->expr, expr_print_help,
-					   &stream, E_NONE);
-				stream << "<br>";
-			}
-			break;
 		default:
 			stream << "unknown property: ";
 			stream << prop_get_type_name(prop->type);
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index b50911bcb08d..cf682a8a3f1e 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -68,15 +68,6 @@ const char *sym_type_name(enum symbol_type type)
 	return "???";
 }
 
-struct property *sym_get_choice_prop(struct symbol *sym)
-{
-	struct property *prop;
-
-	for_all_choices(sym, prop)
-		return prop;
-	return NULL;
-}
-
 /**
  * sym_get_choice_menu - get the parent choice menu if present
  *
@@ -1225,8 +1216,7 @@ static struct symbol *sym_check_sym_deps(struct symbol *sym)
 	stack.expr = NULL;
 
 	for (prop = sym->prop; prop; prop = prop->next) {
-		if (prop->type == P_CHOICE || prop->type == P_SELECT ||
-		    prop->type == P_IMPLY)
+		if (prop->type == P_SELECT || prop->type == P_IMPLY)
 			continue;
 		stack.prop = prop;
 		sym2 = sym_check_expr_deps(prop->visible.expr);
@@ -1343,8 +1333,6 @@ const char *prop_get_type_name(enum prop_type type)
 		return "menu";
 	case P_DEFAULT:
 		return "default";
-	case P_CHOICE:
-		return "choice";
 	case P_SELECT:
 		return "select";
 	case P_IMPLY:
-- 
2.43.0

[PATCH v2 12/12] kconfig: remove E_LIST expression type

[Masahiro Yamada]

E_LIST was preveously used to form an expression tree consisting of
choice members.

It is no longer used.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

 scripts/kconfig/expr.c   | 15 ---------------
 scripts/kconfig/expr.h   |  2 +-
 scripts/kconfig/symbol.c |  3 +--
 3 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index bea82d5cac83..6d4b5a5a1e62 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -90,7 +90,6 @@ struct expr *expr_copy(const struct expr *org)
 		break;
 	case E_AND:
 	case E_OR:
-	case E_LIST:
 		e->left.expr = expr_copy(org->left.expr);
 		e->right.expr = expr_copy(org->right.expr);
 		break;
@@ -286,7 +285,6 @@ int expr_eq(struct expr *e1, struct expr *e2)
 		expr_free(e2);
 		trans_count = old_count;
 		return res;
-	case E_LIST:
 	case E_RANGE:
 	case E_NONE:
 		/* panic */;
@@ -676,7 +674,6 @@ struct expr *expr_transform(struct expr *e)
 	case E_LTH:
 	case E_UNEQUAL:
 	case E_SYMBOL:
-	case E_LIST:
 		break;
 	default:
 		e->left.expr = expr_transform(e->left.expr);
@@ -947,7 +944,6 @@ struct expr *expr_trans_compare(struct expr *e, enum expr_type type, struct symb
 		break;
 	case E_SYMBOL:
 		return expr_alloc_comp(type, e->left.sym, sym);
-	case E_LIST:
 	case E_RANGE:
 	case E_NONE:
 		/* panic */;
@@ -1097,10 +1093,6 @@ static int expr_compare_type(enum expr_type t1, enum expr_type t2)
 		if (t2 == E_OR)
 			return 1;
 		/* fallthrough */
-	case E_OR:
-		if (t2 == E_LIST)
-			return 1;
-		/* fallthrough */
 	default:
 		break;
 	}
@@ -1173,13 +1165,6 @@ void expr_print(struct expr *e,
 		fn(data, NULL, " && ");
 		expr_print(e->right.expr, fn, data, E_AND);
 		break;
-	case E_LIST:
-		fn(data, e->right.sym, e->right.sym->name);
-		if (e->left.expr) {
-			fn(data, NULL, " ^ ");
-			expr_print(e->left.expr, fn, data, E_LIST);
-		}
-		break;
 	case E_RANGE:
 		fn(data, NULL, "[");
 		fn(data, e->left.sym, e->left.sym->name);
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 58fd4c8c3762..8849a243b5e7 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -26,7 +26,7 @@ typedef enum tristate {
 enum expr_type {
 	E_NONE, E_OR, E_AND, E_NOT,
 	E_EQUAL, E_UNEQUAL, E_LTH, E_LEQ, E_GTH, E_GEQ,
-	E_LIST, E_SYMBOL, E_RANGE
+	E_SYMBOL, E_RANGE
 };
 
 union expr_data {
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index cf682a8a3f1e..e5441378c4b0 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1316,8 +1316,7 @@ struct symbol *sym_check_deps(struct symbol *sym)
 
 struct symbol *prop_get_symbol(struct property *prop)
 {
-	if (prop->expr && (prop->expr->type == E_SYMBOL ||
-			   prop->expr->type == E_LIST))
+	if (prop->expr && prop->expr->type == E_SYMBOL)
 		return prop->expr->left.sym;
 	return NULL;
 }
-- 
2.43.0