All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/2] menu: Add a function to set the default by matching the item data
@ 2020-02-06  9:09 Schrempf Frieder
  2020-02-06  9:09 ` [PATCH v3 2/2] pxe: Get default selection from board type if label matches Schrempf Frieder
  2020-02-06 17:46 ` [PATCH v3 1/2] menu: Add a function to set the default by matching the item data Simon Glass
  0 siblings, 2 replies; 6+ messages in thread
From: Schrempf Frieder @ 2020-02-06  9:09 UTC (permalink / raw)
  To: u-boot

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

In order to make it possible to auto select a default entry by
matching the data of the menu entries by an external matching
function, we add some helpers and expose the
menu_set_default_by_item_data_match() function.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
Changes in v3:
* Add a full function comment to describe menu_set_default_by_item_data_match().

Changes in v2:
* Keep the menu structs private and instead only expose one additional
  function, that sets the default by calling an external matching
  function on each entry.
* Change the title and commit message to reflect the changes.
---
 common/menu.c  | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/menu.h |  3 +++
 2 files changed, 57 insertions(+)

diff --git a/common/menu.c b/common/menu.c
index 7b66d199a9..6110b2396c 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -160,6 +160,60 @@ static inline struct menu_item *menu_item_by_key(struct menu *m,
 	return menu_items_iter(m, menu_item_key_match, item_key);
 }
 
+/*
+ * Find the first matching item, if any exists by calling a matching function
+ * on the items data field.
+ */
+static inline struct menu_item *menu_item_by_matching_fn(struct menu *m,
+			int match(void *, void *), void * extra)
+{
+	struct list_head *pos, *n;
+	struct menu_item *item;
+	int ret;
+
+	list_for_each_safe(pos, n, &m->items) {
+		item = list_entry(pos, struct menu_item, list);
+		if (item->key) {
+			ret = match(item->data, extra);
+			if (ret == 1)
+				return item;
+		}
+	}
+
+	return NULL;
+}
+
+/*
+ * menu_set_default_by_item_data_match() - sets a menu default option by calling
+ * a matching function on each of the menu items data field.
+ *
+ * m - Points to a menu created by menu_create().
+ *
+ * match - Points to a function that is passed a pointer to the items data field
+ *         and a pointer to extra data to compare with. It should return 1 on a
+ *         match.
+ *
+ * extra - Points to some data that is passed as a second parameter to the
+ *         matching function.
+ *
+ * key - Points to a char array that will be set to hold the key of the matched
+ *       menu item.
+ *
+ * Returns 0 if successful, or -ENOENT if no matching item was found.
+ */
+int menu_set_default_by_item_data_match(struct menu *m,
+			int match(void *, void *), void *extra, char **key)
+{
+	struct menu_item *item = menu_item_by_matching_fn(m, match, extra);
+
+	if (!item)
+		return -ENOENT;
+
+	*key = item->key;
+	m->default_item = item;
+	return 0;
+}
+
 /*
  * Set *choice to point to the default item's data, if any default item was
  * set, and returns 1. If no default item was set, returns -ENOENT.
diff --git a/include/menu.h b/include/menu.h
index 2d227c20bd..0a8b41a905 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -18,6 +18,9 @@ int menu_item_add(struct menu *m, char *item_key, void *item_data);
 int menu_destroy(struct menu *m);
 void menu_display_statusline(struct menu *m);
 int menu_default_choice(struct menu *m, void **choice);
+int menu_set_default_by_item_data_match(struct menu *m,
+					int match(void *, void *), void *extra,
+					char **key);
 
 /**
  * menu_show() Show a boot menu
-- 
2.17.1

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

* [PATCH v3 2/2] pxe: Get default selection from board type if label matches
  2020-02-06  9:09 [PATCH v3 1/2] menu: Add a function to set the default by matching the item data Schrempf Frieder
@ 2020-02-06  9:09 ` Schrempf Frieder
  2020-02-06 17:46   ` Simon Glass
  2020-02-06 17:46 ` [PATCH v3 1/2] menu: Add a function to set the default by matching the item data Simon Glass
  1 sibling, 1 reply; 6+ messages in thread
From: Schrempf Frieder @ 2020-02-06  9:09 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>
---
Changes in v3:
* Get rid of #ifdef by using IS_ENABLED() in else branch.

Changes in v2:
* Don't use internal structs of menu, but instead call
  menu_set_default_by_item_data_match() that does the iteration work.
---
 cmd/pxe_utils.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
index 53af04d7dc..253df468af 100644
--- a/cmd/pxe_utils.c
+++ b/cmd/pxe_utils.c
@@ -1220,6 +1220,46 @@ struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, unsigned long menucfg)
 	return cfg;
 }
 
+#ifdef CONFIG_OF_CONTROL
+int pxe_match_menu_label_with_str(void *data, void *str)
+{
+	struct pxe_label *label;
+
+	if (!data || !str)
+		return 0;
+
+	label = (struct pxe_label *)data;
+
+	if (strcmp(label->name, str) == 0)
+		return 1;
+
+	return 0;
+}
+
+int pxe_runtime_select_menu_default(struct menu *m)
+{
+	DECLARE_GLOBAL_DATA_PTR;
+	const char *model;
+	char *key;
+	int ret;
+
+	model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
+
+	if (!model)
+		return 0;
+
+	ret = menu_set_default_by_item_data_match(m,
+			pxe_match_menu_label_with_str, (void *)model, &key);
+	if (ret)
+		return ret;
+
+	printf("Menu entry %s fits detected board. " \
+	       "Use as default selection...\n", key);
+
+	return 0;
+}
+#endif
+
 /*
  * Converts a pxe_menu struct into a menu struct for use with U-Boot's generic
  * menu code.
@@ -1258,6 +1298,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);
@@ -1269,6 +1311,10 @@ static struct menu *pxe_menu_to_menu(struct pxe_menu *cfg)
 
 			printf("Missing default: %s\n", cfg->default_label);
 		}
+	} else if (IS_ENABLED(CONFIG_OF_CONTROL) &&
+		   pxe_runtime_select_menu_default(m)) {
+		menu_destroy(m);
+		return NULL;
 	}
 
 	return m;
-- 
2.17.1

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

* [PATCH v3 2/2] pxe: Get default selection from board type if label matches
  2020-02-06  9:09 ` [PATCH v3 2/2] pxe: Get default selection from board type if label matches Schrempf Frieder
@ 2020-02-06 17:46   ` Simon Glass
  2020-02-12 10:35     ` Schrempf Frieder
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2020-02-06 17:46 UTC (permalink / raw)
  To: u-boot

Hi Schrempf,

On Thu, 6 Feb 2020 at 02:09, Schrempf Frieder
<frieder.schrempf@kontron.de> 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>
> ---
> Changes in v3:
> * Get rid of #ifdef by using IS_ENABLED() in else branch.
>
> Changes in v2:
> * Don't use internal structs of menu, but instead call
>   menu_set_default_by_item_data_match() that does the iteration work.
> ---
>  cmd/pxe_utils.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>
> diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
> index 53af04d7dc..253df468af 100644
> --- a/cmd/pxe_utils.c
> +++ b/cmd/pxe_utils.c
> @@ -1220,6 +1220,46 @@ struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, unsigned long menucfg)
>         return cfg;
>  }
>
> +#ifdef CONFIG_OF_CONTROL

I think you need to remove this #ifdef, otherwise you'll get a build
error on boards without OF_CONTROL enabled.

> +int pxe_match_menu_label_with_str(void *data, void *str)
> +{
> +       struct pxe_label *label;
> +
> +       if (!data || !str)
> +               return 0;
> +
> +       label = (struct pxe_label *)data;
> +
> +       if (strcmp(label->name, str) == 0)
> +               return 1;
> +
> +       return 0;
> +}
> +
> +int pxe_runtime_select_menu_default(struct menu *m)
> +{
> +       DECLARE_GLOBAL_DATA_PTR;
> +       const char *model;
> +       char *key;
> +       int ret;
> +
> +       model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
> +
> +       if (!model)
> +               return 0;
> +
> +       ret = menu_set_default_by_item_data_match(m,
> +                       pxe_match_menu_label_with_str, (void *)model, &key);
> +       if (ret)
> +               return ret;
> +
> +       printf("Menu entry %s fits detected board. " \
> +              "Use as default selection...\n", key);
> +
> +       return 0;
> +}
> +#endif
> +
>  /*
>   * Converts a pxe_menu struct into a menu struct for use with U-Boot's generic
>   * menu code.
> @@ -1258,6 +1298,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);
> @@ -1269,6 +1311,10 @@ static struct menu *pxe_menu_to_menu(struct pxe_menu *cfg)
>
>                         printf("Missing default: %s\n", cfg->default_label);
>                 }
> +       } else if (IS_ENABLED(CONFIG_OF_CONTROL) &&
> +                  pxe_runtime_select_menu_default(m)) {
> +               menu_destroy(m);
> +               return NULL;
>         }
>
>         return m;
> --
> 2.17.1

Regards,
Simon

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

* [PATCH v3 1/2] menu: Add a function to set the default by matching the item data
  2020-02-06  9:09 [PATCH v3 1/2] menu: Add a function to set the default by matching the item data Schrempf Frieder
  2020-02-06  9:09 ` [PATCH v3 2/2] pxe: Get default selection from board type if label matches Schrempf Frieder
@ 2020-02-06 17:46 ` Simon Glass
  2020-02-12 10:36   ` Schrempf Frieder
  1 sibling, 1 reply; 6+ messages in thread
From: Simon Glass @ 2020-02-06 17:46 UTC (permalink / raw)
  To: u-boot

Hi Schrempf,

On Thu, 6 Feb 2020 at 02:09, Schrempf Frieder
<frieder.schrempf@kontron.de> wrote:
>
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> In order to make it possible to auto select a default entry by
> matching the data of the menu entries by an external matching
> function, we add some helpers and expose the
> menu_set_default_by_item_data_match() function.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
> Changes in v3:
> * Add a full function comment to describe menu_set_default_by_item_data_match().
>
> Changes in v2:
> * Keep the menu structs private and instead only expose one additional
>   function, that sets the default by calling an external matching
>   function on each entry.
> * Change the title and commit message to reflect the changes.
> ---
>  common/menu.c  | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/menu.h |  3 +++
>  2 files changed, 57 insertions(+)
>
> diff --git a/common/menu.c b/common/menu.c
> index 7b66d199a9..6110b2396c 100644
> --- a/common/menu.c
> +++ b/common/menu.c
> @@ -160,6 +160,60 @@ static inline struct menu_item *menu_item_by_key(struct menu *m,
>         return menu_items_iter(m, menu_item_key_match, item_key);
>  }
>
> +/*
> + * Find the first matching item, if any exists by calling a matching function
> + * on the items data field.
> + */
> +static inline struct menu_item *menu_item_by_matching_fn(struct menu *m,
> +                       int match(void *, void *), void * extra)
> +{
> +       struct list_head *pos, *n;
> +       struct menu_item *item;
> +       int ret;
> +
> +       list_for_each_safe(pos, n, &m->items) {
> +               item = list_entry(pos, struct menu_item, list);
> +               if (item->key) {
> +                       ret = match(item->data, extra);
> +                       if (ret == 1)
> +                               return item;
> +               }
> +       }
> +
> +       return NULL;
> +}
> +
> +/*
> + * menu_set_default_by_item_data_match() - sets a menu default option by calling
> + * a matching function on each of the menu items data field.
> + *
> + * m - Points to a menu created by menu_create().
> + *
> + * match - Points to a function that is passed a pointer to the items data field
> + *         and a pointer to extra data to compare with. It should return 1 on a
> + *         match.
> + *
> + * extra - Points to some data that is passed as a second parameter to the
> + *         matching function.
> + *
> + * key - Points to a char array that will be set to hold the key of the matched
> + *       menu item.
> + *
> + * Returns 0 if successful, or -ENOENT if no matching item was found.

Can you please update this to the correct comment style - e.g. see
cmd_process_error() in command.h for example. Also since this is an
exported function the comment should go in the header file.

Regards,
SImon

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

* [PATCH v3 2/2] pxe: Get default selection from board type if label matches
  2020-02-06 17:46   ` Simon Glass
@ 2020-02-12 10:35     ` Schrempf Frieder
  0 siblings, 0 replies; 6+ messages in thread
From: Schrempf Frieder @ 2020-02-12 10:35 UTC (permalink / raw)
  To: u-boot

On 06.02.20 18:46, Simon Glass wrote:
> Hi Schrempf,
> 
> On Thu, 6 Feb 2020 at 02:09, Schrempf Frieder
> <frieder.schrempf@kontron.de> 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>
>> ---
>> Changes in v3:
>> * Get rid of #ifdef by using IS_ENABLED() in else branch.
>>
>> Changes in v2:
>> * Don't use internal structs of menu, but instead call
>>    menu_set_default_by_item_data_match() that does the iteration work.
>> ---
>>   cmd/pxe_utils.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
>> index 53af04d7dc..253df468af 100644
>> --- a/cmd/pxe_utils.c
>> +++ b/cmd/pxe_utils.c
>> @@ -1220,6 +1220,46 @@ struct pxe_menu *parse_pxefile(cmd_tbl_t *cmdtp, unsigned long menucfg)
>>          return cfg;
>>   }
>>
>> +#ifdef CONFIG_OF_CONTROL
> 
> I think you need to remove this #ifdef, otherwise you'll get a build
> error on boards without OF_CONTROL enabled.

Right, I forgot about this.

> 
>> +int pxe_match_menu_label_with_str(void *data, void *str)
>> +{
>> +       struct pxe_label *label;
>> +
>> +       if (!data || !str)
>> +               return 0;
>> +
>> +       label = (struct pxe_label *)data;
>> +
>> +       if (strcmp(label->name, str) == 0)
>> +               return 1;
>> +
>> +       return 0;
>> +}
>> +
>> +int pxe_runtime_select_menu_default(struct menu *m)
>> +{
>> +       DECLARE_GLOBAL_DATA_PTR;
>> +       const char *model;
>> +       char *key;
>> +       int ret;
>> +
>> +       model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
>> +
>> +       if (!model)
>> +               return 0;
>> +
>> +       ret = menu_set_default_by_item_data_match(m,
>> +                       pxe_match_menu_label_with_str, (void *)model, &key);
>> +       if (ret)
>> +               return ret;
>> +
>> +       printf("Menu entry %s fits detected board. " \
>> +              "Use as default selection...\n", key);
>> +
>> +       return 0;
>> +}
>> +#endif
>> +
>>   /*
>>    * Converts a pxe_menu struct into a menu struct for use with U-Boot's generic
>>    * menu code.
>> @@ -1258,6 +1298,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);
>> @@ -1269,6 +1311,10 @@ static struct menu *pxe_menu_to_menu(struct pxe_menu *cfg)
>>
>>                          printf("Missing default: %s\n", cfg->default_label);
>>                  }
>> +       } else if (IS_ENABLED(CONFIG_OF_CONTROL) &&
>> +                  pxe_runtime_select_menu_default(m)) {
>> +               menu_destroy(m);
>> +               return NULL;
>>          }
>>
>>          return m;
>> --
>> 2.17.1
> 
> Regards,
> Simon
> 

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

* [PATCH v3 1/2] menu: Add a function to set the default by matching the item data
  2020-02-06 17:46 ` [PATCH v3 1/2] menu: Add a function to set the default by matching the item data Simon Glass
@ 2020-02-12 10:36   ` Schrempf Frieder
  0 siblings, 0 replies; 6+ messages in thread
From: Schrempf Frieder @ 2020-02-12 10:36 UTC (permalink / raw)
  To: u-boot

On 06.02.20 18:46, Simon Glass wrote:
> Hi Schrempf,
> 
> On Thu, 6 Feb 2020 at 02:09, Schrempf Frieder
> <frieder.schrempf@kontron.de> wrote:
>>
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> In order to make it possible to auto select a default entry by
>> matching the data of the menu entries by an external matching
>> function, we add some helpers and expose the
>> menu_set_default_by_item_data_match() function.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>> ---
>> Changes in v3:
>> * Add a full function comment to describe menu_set_default_by_item_data_match().
>>
>> Changes in v2:
>> * Keep the menu structs private and instead only expose one additional
>>    function, that sets the default by calling an external matching
>>    function on each entry.
>> * Change the title and commit message to reflect the changes.
>> ---
>>   common/menu.c  | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/menu.h |  3 +++
>>   2 files changed, 57 insertions(+)
>>
>> diff --git a/common/menu.c b/common/menu.c
>> index 7b66d199a9..6110b2396c 100644
>> --- a/common/menu.c
>> +++ b/common/menu.c
>> @@ -160,6 +160,60 @@ static inline struct menu_item *menu_item_by_key(struct menu *m,
>>          return menu_items_iter(m, menu_item_key_match, item_key);
>>   }
>>
>> +/*
>> + * Find the first matching item, if any exists by calling a matching function
>> + * on the items data field.
>> + */
>> +static inline struct menu_item *menu_item_by_matching_fn(struct menu *m,
>> +                       int match(void *, void *), void * extra)
>> +{
>> +       struct list_head *pos, *n;
>> +       struct menu_item *item;
>> +       int ret;
>> +
>> +       list_for_each_safe(pos, n, &m->items) {
>> +               item = list_entry(pos, struct menu_item, list);
>> +               if (item->key) {
>> +                       ret = match(item->data, extra);
>> +                       if (ret == 1)
>> +                               return item;
>> +               }
>> +       }
>> +
>> +       return NULL;
>> +}
>> +
>> +/*
>> + * menu_set_default_by_item_data_match() - sets a menu default option by calling
>> + * a matching function on each of the menu items data field.
>> + *
>> + * m - Points to a menu created by menu_create().
>> + *
>> + * match - Points to a function that is passed a pointer to the items data field
>> + *         and a pointer to extra data to compare with. It should return 1 on a
>> + *         match.
>> + *
>> + * extra - Points to some data that is passed as a second parameter to the
>> + *         matching function.
>> + *
>> + * key - Points to a char array that will be set to hold the key of the matched
>> + *       menu item.
>> + *
>> + * Returns 0 if successful, or -ENOENT if no matching item was found.
> 
> Can you please update this to the correct comment style - e.g. see
> cmd_process_error() in command.h for example. Also since this is an
> exported function the comment should go in the header file.

Sure. Thanks for pointing me to the correct style and for reviewing in 
general!

> 
> Regards,
> SImon
> 

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

end of thread, other threads:[~2020-02-12 10:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-06  9:09 [PATCH v3 1/2] menu: Add a function to set the default by matching the item data Schrempf Frieder
2020-02-06  9:09 ` [PATCH v3 2/2] pxe: Get default selection from board type if label matches Schrempf Frieder
2020-02-06 17:46   ` Simon Glass
2020-02-12 10:35     ` Schrempf Frieder
2020-02-06 17:46 ` [PATCH v3 1/2] menu: Add a function to set the default by matching the item data Simon Glass
2020-02-12 10:36   ` 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.