All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] menu: Make some parts of the menu available to other components
@ 2019-12-10 15:47 Schrempf Frieder
  2019-12-10 15:47 ` [PATCH 2/2] pxe: Get default selection from board type if label matches Schrempf Frieder
  2019-12-30  1:21 ` [PATCH 1/2] menu: Make some parts of the menu available to other components Simon Glass
  0 siblings, 2 replies; 6+ messages in thread
From: Schrempf Frieder @ 2019-12-10 15:47 UTC (permalink / raw)
  To: u-boot

From: Frieder Schrempf <frieder.schrempf@kontron.de>

In order to iterate over the menu entries and match for a specific
name in the pxe boot, we need to properly export the needed structs
and functions.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 common/menu.c  | 31 +------------------------------
 include/menu.h | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/common/menu.c b/common/menu.c
index 7b66d199a9..82b03f17f7 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -12,36 +12,7 @@
 
 #include "menu.h"
 
-/*
- * Internally, each item in a menu is represented by a struct menu_item.
- *
- * These items will be alloc'd and initialized by menu_item_add and destroyed
- * by menu_item_destroy, and the consumer of the interface never sees that
- * this struct is used at all.
- */
-struct menu_item {
-	char *key;
-	void *data;
-	struct list_head list;
-};
 
-/*
- * The menu is composed of a list of items along with settings and callbacks
- * provided by the user. An incomplete definition of this struct is available
- * in menu.h, but the full definition is here to prevent consumers from
- * relying on its contents.
- */
-struct menu {
-	struct menu_item *default_item;
-	int timeout;
-	char *title;
-	int prompt;
-	void (*item_data_print)(void *);
-	char *(*item_choice)(void *);
-	void *item_choice_data;
-	struct list_head items;
-	int item_cnt;
-};
 
 /*
  * An iterator function for menu items. callback will be called for each item
@@ -51,7 +22,7 @@ struct menu {
  * used for search type operations. It is also safe for callback to remove the
  * item from the list of items.
  */
-static inline void *menu_items_iter(struct menu *m,
+inline void *menu_items_iter(struct menu *m,
 		void *(*callback)(struct menu *, struct menu_item *, void *),
 		void *extra)
 {
diff --git a/include/menu.h b/include/menu.h
index 2d227c20bd..b3f8db87e4 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -6,8 +6,40 @@
 #ifndef __MENU_H__
 #define __MENU_H__
 
-struct menu;
+/*
+ * Internally, each item in a menu is represented by a struct menu_item.
+ *
+ * These items will be alloc'd and initialized by menu_item_add and destroyed
+ * by menu_item_destroy, and the consumer of the interface never sees that
+ * this struct is used at all.
+ */
+struct menu_item {
+	char *key;
+	void *data;
+	struct list_head list;
+};
+
+/*
+ * The menu is composed of a list of items along with settings and callbacks
+ * provided by the user. An incomplete definition of this struct is available
+ * in menu.h, but the full definition is here to prevent consumers from
+ * relying on its contents.
+ */
+struct menu {
+	struct menu_item *default_item;
+	int timeout;
+	char *title;
+	int prompt;
+	void (*item_data_print)(void *);
+	char *(*item_choice)(void *);
+	void *item_choice_data;
+	struct list_head items;
+	int item_cnt;
+};
 
+void *menu_items_iter(struct menu *m,
+		void *(*callback)(struct menu *, struct menu_item *, void *),
+		void *extra);
 struct menu *menu_create(char *title, int timeout, int prompt,
 				void (*item_data_print)(void *),
 				char *(*item_choice)(void *),
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] pxe: Get default selection from board type if label matches
  2019-12-10 15:47 [PATCH 1/2] menu: Make some parts of the menu available to other components Schrempf Frieder
@ 2019-12-10 15:47 ` Schrempf Frieder
  2019-12-11  9:40   ` Schrempf Frieder
  2019-12-30  1:21 ` [PATCH 1/2] menu: Make some parts of the menu available to other components Simon Glass
  1 sibling, 1 reply; 6+ messages in thread
From: Schrempf Frieder @ 2019-12-10 15:47 UTC (permalink / raw)
  To: u-boot

From: Frieder Schrempf <frieder.schrempf@kontron.de>

In order to auto-select an option from the pxe boot menu, that
matches the detected board, we check the board model string in the
devicetree and set the default menu selection, if it matches the
label of the menu entry and there is no default selection already
set.

This is useful in combination with SPL that loads a FIT image with
U-Boot and multiple DTBs. SPL can detect the board and choose the
matching configuration in the FIT by using
board_fit_config_name_match().

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 cmd/pxe_utils.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
index a636346bb5..510957e68f 100644
--- a/cmd/pxe_utils.c
+++ b/cmd/pxe_utils.c
@@ -1219,6 +1219,61 @@ struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, unsigned long menucfg)
 	return cfg;
 }
 
+#ifdef CONFIG_OF_CONTROL
+/*
+ * Check if an item's name matches a provided string, pointed to by extra.
+ *
+ * This is called via menu_items_iter, so it returns a pointer to the item if
+ * the name matches, and returns NULL otherwise.
+ */
+static inline void *pxe_item_name_match(struct menu *m, struct menu_item *item,
+					void *extra)
+{
+	char *name = extra;
+	struct pxe_label *label;
+
+	if (!name || !item->key)
+		return NULL;
+
+	label = (struct pxe_label *)item->data;
+
+	if (strcmp(label->name, name) == 0)
+		return item;
+
+	return NULL;
+}
+
+/*
+ * Find the first item with a name matching the given name, if any exists.
+ */
+inline struct menu_item *menu_item_by_pxe_name(struct menu *m, char *name)
+{
+	return menu_items_iter(m, pxe_item_name_match, name);
+}
+
+int pxe_runtime_select_menu_default(struct menu *m)
+{
+	DECLARE_GLOBAL_DATA_PTR;
+	struct menu_item *item = NULL;
+	const char *model;
+
+	model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
+
+	if (!model)
+		return 0;
+
+	item = menu_item_by_pxe_name(m, model);
+
+	if (item) {
+		printf("Menu entry %s fits detected board. " \
+		       "Use as default selection...\n", item->key);
+		m->default_item = item;
+	}
+
+	return 0;
+}
+#endif
+
 /*
  * Converts a pxe_menu struct into a menu struct for use with U-Boot's generic
  * menu code.
@@ -1257,6 +1312,8 @@ static struct menu *pxe_menu_to_menu(struct pxe_menu *cfg)
 	/*
 	 * After we've created items for each label in the menu, set the
 	 * menu's default label if one was specified.
+	 * If OF_CONTROL is enabled and we don't have a default specified,
+	 * we try to use an entry that matches the board/model name as default.
 	 */
 	if (default_num) {
 		err = menu_default_set(m, default_num);
@@ -1268,6 +1325,11 @@ static struct menu *pxe_menu_to_menu(struct pxe_menu *cfg)
 
 			printf("Missing default: %s\n", cfg->default_label);
 		}
+#ifdef CONFIG_OF_CONTROL
+	} else if (pxe_runtime_select_menu_default(m)) {
+		menu_destroy(m);
+		return NULL;
+#endif
 	}
 
 	return m;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] pxe: Get default selection from board type if label matches
  2019-12-10 15:47 ` [PATCH 2/2] pxe: Get default selection from board type if label matches Schrempf Frieder
@ 2019-12-11  9:40   ` Schrempf Frieder
  0 siblings, 0 replies; 6+ messages in thread
From: Schrempf Frieder @ 2019-12-11  9:40 UTC (permalink / raw)
  To: u-boot

On 10.12.19 16:45, Schrempf Frieder wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> In order to auto-select an option from the pxe boot menu, that
> matches the detected board, we check the board model string in the
> devicetree and set the default menu selection, if it matches the
> label of the menu entry and there is no default selection already
> set.
> 
> This is useful in combination with SPL that loads a FIT image with
> U-Boot and multiple DTBs. SPL can detect the board and choose the
> matching configuration in the FIT by using
> board_fit_config_name_match().
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>

I forgot to fix some compiler warnings in this code, so this is probably 
more like a RFC until I send a fixed v2.

> ---
>   cmd/pxe_utils.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 62 insertions(+)
> 
> diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
> index a636346bb5..510957e68f 100644
> --- a/cmd/pxe_utils.c
> +++ b/cmd/pxe_utils.c
> @@ -1219,6 +1219,61 @@ struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, unsigned long menucfg)
>   	return cfg;
>   }
>   
> +#ifdef CONFIG_OF_CONTROL
> +/*
> + * Check if an item's name matches a provided string, pointed to by extra.
> + *
> + * This is called via menu_items_iter, so it returns a pointer to the item if
> + * the name matches, and returns NULL otherwise.
> + */
> +static inline void *pxe_item_name_match(struct menu *m, struct menu_item *item,
> +					void *extra)
> +{
> +	char *name = extra;
> +	struct pxe_label *label;
> +
> +	if (!name || !item->key)
> +		return NULL;
> +
> +	label = (struct pxe_label *)item->data;
> +
> +	if (strcmp(label->name, name) == 0)
> +		return item;
> +
> +	return NULL;
> +}
> +
> +/*
> + * Find the first item with a name matching the given name, if any exists.
> + */
> +inline struct menu_item *menu_item_by_pxe_name(struct menu *m, char *name)
> +{
> +	return menu_items_iter(m, pxe_item_name_match, name);
> +}
> +
> +int pxe_runtime_select_menu_default(struct menu *m)
> +{
> +	DECLARE_GLOBAL_DATA_PTR;
> +	struct menu_item *item = NULL;
> +	const char *model;
> +
> +	model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
> +
> +	if (!model)
> +		return 0;
> +
> +	item = menu_item_by_pxe_name(m, model);
> +
> +	if (item) {
> +		printf("Menu entry %s fits detected board. " \
> +		       "Use as default selection...\n", item->key);
> +		m->default_item = item;
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
>   /*
>    * Converts a pxe_menu struct into a menu struct for use with U-Boot's generic
>    * menu code.
> @@ -1257,6 +1312,8 @@ static struct menu *pxe_menu_to_menu(struct pxe_menu *cfg)
>   	/*
>   	 * After we've created items for each label in the menu, set the
>   	 * menu's default label if one was specified.
> +	 * If OF_CONTROL is enabled and we don't have a default specified,
> +	 * we try to use an entry that matches the board/model name as default.
>   	 */
>   	if (default_num) {
>   		err = menu_default_set(m, default_num);
> @@ -1268,6 +1325,11 @@ static struct menu *pxe_menu_to_menu(struct pxe_menu *cfg)
>   
>   			printf("Missing default: %s\n", cfg->default_label);
>   		}
> +#ifdef CONFIG_OF_CONTROL
> +	} else if (pxe_runtime_select_menu_default(m)) {
> +		menu_destroy(m);
> +		return NULL;
> +#endif
>   	}
>   
>   	return m;
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] menu: Make some parts of the menu available to other components
  2019-12-10 15:47 [PATCH 1/2] menu: Make some parts of the menu available to other components Schrempf Frieder
  2019-12-10 15:47 ` [PATCH 2/2] pxe: Get default selection from board type if label matches Schrempf Frieder
@ 2019-12-30  1:21 ` Simon Glass
  2020-02-05 13:44   ` Schrempf Frieder
  1 sibling, 1 reply; 6+ messages in thread
From: Simon Glass @ 2019-12-30  1:21 UTC (permalink / raw)
  To: u-boot

Hi Schrempf,

On Tue, 10 Dec 2019 at 08:47, Schrempf Frieder
<frieder.schrempf@kontron.de> wrote:
>
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> In order to iterate over the menu entries and match for a specific
> name in the pxe boot, we need to properly export the needed structs
> and functions.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
>  common/menu.c  | 31 +------------------------------
>  include/menu.h | 34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 31 deletions(-)
>
> diff --git a/common/menu.c b/common/menu.c
> index 7b66d199a9..82b03f17f7 100644
> --- a/common/menu.c
> +++ b/common/menu.c
> @@ -12,36 +12,7 @@
>
>  #include "menu.h"
>
> -/*
> - * Internally, each item in a menu is represented by a struct menu_item.
> - *
> - * These items will be alloc'd and initialized by menu_item_add and destroyed
> - * by menu_item_destroy, and the consumer of the interface never sees that
> - * this struct is used at all.
> - */
> -struct menu_item {
> -       char *key;
> -       void *data;
> -       struct list_head list;
> -};
>
> -/*
> - * The menu is composed of a list of items along with settings and callbacks
> - * provided by the user. An incomplete definition of this struct is available
> - * in menu.h, but the full definition is here to prevent consumers from
> - * relying on its contents.
> - */
> -struct menu {
> -       struct menu_item *default_item;
> -       int timeout;
> -       char *title;
> -       int prompt;
> -       void (*item_data_print)(void *);
> -       char *(*item_choice)(void *);
> -       void *item_choice_data;
> -       struct list_head items;
> -       int item_cnt;
> -};
>
>  /*
>   * An iterator function for menu items. callback will be called for each item
> @@ -51,7 +22,7 @@ struct menu {
>   * used for search type operations. It is also safe for callback to remove the
>   * item from the list of items.
>   */
> -static inline void *menu_items_iter(struct menu *m,
> +inline void *menu_items_iter(struct menu *m,
>                 void *(*callback)(struct menu *, struct menu_item *, void *),
>                 void *extra)
>  {
> diff --git a/include/menu.h b/include/menu.h
> index 2d227c20bd..b3f8db87e4 100644
> --- a/include/menu.h
> +++ b/include/menu.h
> @@ -6,8 +6,40 @@
>  #ifndef __MENU_H__
>  #define __MENU_H__
>
> -struct menu;
> +/*
> + * Internally, each item in a menu is represented by a struct menu_item.
> + *
> + * These items will be alloc'd and initialized by menu_item_add and destroyed
> + * by menu_item_destroy, and the consumer of the interface never sees that
> + * this struct is used at all.
> + */
> +struct menu_item {
> +       char *key;
> +       void *data;
> +       struct list_head list;
> +};
> +
> +/*
> + * The menu is composed of a list of items along with settings and callbacks
> + * provided by the user. An incomplete definition of this struct is available
> + * in menu.h, but the full definition is here to prevent consumers from
> + * relying on its contents.

This comment doesn't seem to be true anymore.

Is it possible to add a function to get the info you need from the menu?

> + */
> +struct menu {
> +       struct menu_item *default_item;
> +       int timeout;
> +       char *title;
> +       int prompt;
> +       void (*item_data_print)(void *);
> +       char *(*item_choice)(void *);
> +       void *item_choice_data;
> +       struct list_head items;
> +       int item_cnt;
> +};
>
> +void *menu_items_iter(struct menu *m,
> +               void *(*callback)(struct menu *, struct menu_item *, void *),
> +               void *extra);
>  struct menu *menu_create(char *title, int timeout, int prompt,
>                                 void (*item_data_print)(void *),
>                                 char *(*item_choice)(void *),
> --
> 2.17.1

Regards,
Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] menu: Make some parts of the menu available to other components
  2019-12-30  1:21 ` [PATCH 1/2] menu: Make some parts of the menu available to other components Simon Glass
@ 2020-02-05 13:44   ` Schrempf Frieder
  2020-02-05 15:02     ` Schrempf Frieder
  0 siblings, 1 reply; 6+ messages in thread
From: Schrempf Frieder @ 2020-02-05 13:44 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 30.12.19 02:21, Simon Glass wrote:
> Hi Schrempf,
> 
> On Tue, 10 Dec 2019 at 08:47, Schrempf Frieder
> <frieder.schrempf@kontron.de> wrote:
>>
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> In order to iterate over the menu entries and match for a specific
>> name in the pxe boot, we need to properly export the needed structs
>> and functions.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>> ---
>>   common/menu.c  | 31 +------------------------------
>>   include/menu.h | 34 +++++++++++++++++++++++++++++++++-
>>   2 files changed, 34 insertions(+), 31 deletions(-)
>>
>> diff --git a/common/menu.c b/common/menu.c
>> index 7b66d199a9..82b03f17f7 100644
>> --- a/common/menu.c
>> +++ b/common/menu.c
>> @@ -12,36 +12,7 @@
>>
>>   #include "menu.h"
>>
>> -/*
>> - * Internally, each item in a menu is represented by a struct menu_item.
>> - *
>> - * These items will be alloc'd and initialized by menu_item_add and destroyed
>> - * by menu_item_destroy, and the consumer of the interface never sees that
>> - * this struct is used at all.
>> - */
>> -struct menu_item {
>> -       char *key;
>> -       void *data;
>> -       struct list_head list;
>> -};
>>
>> -/*
>> - * The menu is composed of a list of items along with settings and callbacks
>> - * provided by the user. An incomplete definition of this struct is available
>> - * in menu.h, but the full definition is here to prevent consumers from
>> - * relying on its contents.
>> - */
>> -struct menu {
>> -       struct menu_item *default_item;
>> -       int timeout;
>> -       char *title;
>> -       int prompt;
>> -       void (*item_data_print)(void *);
>> -       char *(*item_choice)(void *);
>> -       void *item_choice_data;
>> -       struct list_head items;
>> -       int item_cnt;
>> -};
>>
>>   /*
>>    * An iterator function for menu items. callback will be called for each item
>> @@ -51,7 +22,7 @@ struct menu {
>>    * used for search type operations. It is also safe for callback to remove the
>>    * item from the list of items.
>>    */
>> -static inline void *menu_items_iter(struct menu *m,
>> +inline void *menu_items_iter(struct menu *m,
>>                  void *(*callback)(struct menu *, struct menu_item *, void *),
>>                  void *extra)
>>   {
>> diff --git a/include/menu.h b/include/menu.h
>> index 2d227c20bd..b3f8db87e4 100644
>> --- a/include/menu.h
>> +++ b/include/menu.h
>> @@ -6,8 +6,40 @@
>>   #ifndef __MENU_H__
>>   #define __MENU_H__
>>
>> -struct menu;
>> +/*
>> + * Internally, each item in a menu is represented by a struct menu_item.
>> + *
>> + * These items will be alloc'd and initialized by menu_item_add and destroyed
>> + * by menu_item_destroy, and the consumer of the interface never sees that
>> + * this struct is used at all.
>> + */
>> +struct menu_item {
>> +       char *key;
>> +       void *data;
>> +       struct list_head list;
>> +};
>> +
>> +/*
>> + * The menu is composed of a list of items along with settings and callbacks
>> + * provided by the user. An incomplete definition of this struct is available
>> + * in menu.h, but the full definition is here to prevent consumers from
>> + * relying on its contents.
> 
> This comment doesn't seem to be true anymore.

Right.

> 
> Is it possible to add a function to get the info you need from the menu?

Unfortunately I didn't find a better solution, that doesn't expose the 
menu structures. The problem is that menu and pxe are not separated 
cleanly as the menu items contain references to struct pxe_label.

If I would add a function to iterate over the items to the menu code, I 
would need to make struct pxe_label known to the menu code, which 
doesn't seem nice, either.

Please let me know if you propose a different approach.

Thanks,
Frieder

> 
>> + */
>> +struct menu {
>> +       struct menu_item *default_item;
>> +       int timeout;
>> +       char *title;
>> +       int prompt;
>> +       void (*item_data_print)(void *);
>> +       char *(*item_choice)(void *);
>> +       void *item_choice_data;
>> +       struct list_head items;
>> +       int item_cnt;
>> +};
>>
>> +void *menu_items_iter(struct menu *m,
>> +               void *(*callback)(struct menu *, struct menu_item *, void *),
>> +               void *extra);
>>   struct menu *menu_create(char *title, int timeout, int prompt,
>>                                  void (*item_data_print)(void *),
>>                                  char *(*item_choice)(void *),
>> --
>> 2.17.1
> 
> Regards,
> Simon
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] menu: Make some parts of the menu available to other components
  2020-02-05 13:44   ` Schrempf Frieder
@ 2020-02-05 15:02     ` Schrempf Frieder
  0 siblings, 0 replies; 6+ messages in thread
From: Schrempf Frieder @ 2020-02-05 15:02 UTC (permalink / raw)
  To: u-boot

On 05.02.20 14:44, Frieder Schrempf wrote:
> Hi Simon,
> 
> On 30.12.19 02:21, Simon Glass wrote:
>> Hi Schrempf,
>>
>> On Tue, 10 Dec 2019 at 08:47, Schrempf Frieder
>> <frieder.schrempf@kontron.de> wrote:
>>>
>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>
>>> In order to iterate over the menu entries and match for a specific
>>> name in the pxe boot, we need to properly export the needed structs
>>> and functions.
>>>
>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>>> ---
>>>   common/menu.c  | 31 +------------------------------
>>>   include/menu.h | 34 +++++++++++++++++++++++++++++++++-
>>>   2 files changed, 34 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/common/menu.c b/common/menu.c
>>> index 7b66d199a9..82b03f17f7 100644
>>> --- a/common/menu.c
>>> +++ b/common/menu.c
>>> @@ -12,36 +12,7 @@
>>>
>>>   #include "menu.h"
>>>
>>> -/*
>>> - * Internally, each item in a menu is represented by a struct 
>>> menu_item.
>>> - *
>>> - * These items will be alloc'd and initialized by menu_item_add and 
>>> destroyed
>>> - * by menu_item_destroy, and the consumer of the interface never 
>>> sees that
>>> - * this struct is used at all.
>>> - */
>>> -struct menu_item {
>>> -       char *key;
>>> -       void *data;
>>> -       struct list_head list;
>>> -};
>>>
>>> -/*
>>> - * The menu is composed of a list of items along with settings and 
>>> callbacks
>>> - * provided by the user. An incomplete definition of this struct is 
>>> available
>>> - * in menu.h, but the full definition is here to prevent consumers from
>>> - * relying on its contents.
>>> - */
>>> -struct menu {
>>> -       struct menu_item *default_item;
>>> -       int timeout;
>>> -       char *title;
>>> -       int prompt;
>>> -       void (*item_data_print)(void *);
>>> -       char *(*item_choice)(void *);
>>> -       void *item_choice_data;
>>> -       struct list_head items;
>>> -       int item_cnt;
>>> -};
>>>
>>>   /*
>>>    * An iterator function for menu items. callback will be called for 
>>> each item
>>> @@ -51,7 +22,7 @@ struct menu {
>>>    * used for search type operations. It is also safe for callback to 
>>> remove the
>>>    * item from the list of items.
>>>    */
>>> -static inline void *menu_items_iter(struct menu *m,
>>> +inline void *menu_items_iter(struct menu *m,
>>>                  void *(*callback)(struct menu *, struct menu_item *, 
>>> void *),
>>>                  void *extra)
>>>   {
>>> diff --git a/include/menu.h b/include/menu.h
>>> index 2d227c20bd..b3f8db87e4 100644
>>> --- a/include/menu.h
>>> +++ b/include/menu.h
>>> @@ -6,8 +6,40 @@
>>>   #ifndef __MENU_H__
>>>   #define __MENU_H__
>>>
>>> -struct menu;
>>> +/*
>>> + * Internally, each item in a menu is represented by a struct 
>>> menu_item.
>>> + *
>>> + * These items will be alloc'd and initialized by menu_item_add and 
>>> destroyed
>>> + * by menu_item_destroy, and the consumer of the interface never 
>>> sees that
>>> + * this struct is used at all.
>>> + */
>>> +struct menu_item {
>>> +       char *key;
>>> +       void *data;
>>> +       struct list_head list;
>>> +};
>>> +
>>> +/*
>>> + * The menu is composed of a list of items along with settings and 
>>> callbacks
>>> + * provided by the user. An incomplete definition of this struct is 
>>> available
>>> + * in menu.h, but the full definition is here to prevent consumers from
>>> + * relying on its contents.
>>
>> This comment doesn't seem to be true anymore.
> 
> Right.
> 
>>
>> Is it possible to add a function to get the info you need from the menu?
> 
> Unfortunately I didn't find a better solution, that doesn't expose the 
> menu structures. The problem is that menu and pxe are not separated 
> cleanly as the menu items contain references to struct pxe_label.

Actually, on second thought, if I don't use the existing iterator 
function in the menu code, I could keep all the menu code in menu.c and 
expose just one additional function.

I'll send a v2 with this approach.

> 
> If I would add a function to iterate over the items to the menu code, I 
> would need to make struct pxe_label known to the menu code, which 
> doesn't seem nice, either.
> 
> Please let me know if you propose a different approach.
> 
> Thanks,
> Frieder
> 
>>
>>> + */
>>> +struct menu {
>>> +       struct menu_item *default_item;
>>> +       int timeout;
>>> +       char *title;
>>> +       int prompt;
>>> +       void (*item_data_print)(void *);
>>> +       char *(*item_choice)(void *);
>>> +       void *item_choice_data;
>>> +       struct list_head items;
>>> +       int item_cnt;
>>> +};
>>>
>>> +void *menu_items_iter(struct menu *m,
>>> +               void *(*callback)(struct menu *, struct menu_item *, 
>>> void *),
>>> +               void *extra);
>>>   struct menu *menu_create(char *title, int timeout, int prompt,
>>>                                  void (*item_data_print)(void *),
>>>                                  char *(*item_choice)(void *),
>>> -- 
>>> 2.17.1
>>
>> Regards,
>> Simon
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-05 15:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 15:47 [PATCH 1/2] menu: Make some parts of the menu available to other components Schrempf Frieder
2019-12-10 15:47 ` [PATCH 2/2] pxe: Get default selection from board type if label matches Schrempf Frieder
2019-12-11  9:40   ` Schrempf Frieder
2019-12-30  1:21 ` [PATCH 1/2] menu: Make some parts of the menu available to other components Simon Glass
2020-02-05 13:44   ` Schrempf Frieder
2020-02-05 15:02     ` Schrempf Frieder

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.