[PATCH] cmd: pxe_utils: sysboot: add label override support

[Simon Glass]

Hi Amjad,

On Fri, 12 Nov 2021 at 09:46, Amjad Ouled-Ameur
<aouledameur at baylibre.com> wrote:
>
> Hi Simon,
>
> On 06/11/2021 01:18, Amjad Ouled-Ameur wrote:
> > Hi Simon,
> >
> > On 05/11/2021 03:02, Simon Glass wrote:
> >> Hi Amjad,
> >>
> >> On Fri, 22 Oct 2021 at 09:55, Amjad Ouled-Ameur
> >> <aouledameur at baylibre.com> wrote:
> >>> This will allow consumers to choose a pxe label at runtime instead of
> >>> having to prompt the user. One good use-case for this, is choosing
> >>> whether or not to apply a dtbo depending on the hardware configuration.
> >>> e.g: for TI's AM335x EVM, it would be convenient to apply a particular
> >>> dtbo only when the J9 jumper is on PRUSS mode. To achieve this, the
> >>> pxe menu should have 2 labels, one with the dtbo and the other without,
> >>> then the "pxe_label_override" env variable should point to the label
> >>> with
> >>> the dtbo at runtime only when the jumper is on PRUSS mode.
> >>>
> >>> This change can be used for different use-cases and bring more
> >>> flexibilty to consumers who use sysboot/pxe_utils.
> >>>
> >>> if "pxe_label_override" is set but does not exist in the pxe menu,
> >>> the code should fallback to the default label if given, and no failure
> >>> is returned but rather a warning message.
> >>>
> >>> Signed-off-by: Amjad Ouled-Ameur <aouledameur at baylibre.com>
> >>>
> >>> ---
> >>>
> >>>   cmd/pxe_utils.c | 15 +++++++++++++++
> >>>   1 file changed, 15 insertions(+)
> >> Can we add this to the docs somewhere?
> >
> > This would be great, I think doc/README.pxe is the best place for it.
> >
> > Will send a V2 with the doc change, thank you for the suggestion.
> >
> >
> > Regards,
> >
> > Amjad
> >
> >>> diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
> >>> index 067c24e5ff4b..b1009f9c7547 100644
> >>> --- a/cmd/pxe_utils.c
> >>> +++ b/cmd/pxe_utils.c
> >>> @@ -1354,9 +1354,11 @@ static struct menu *pxe_menu_to_menu(struct
> >>> pxe_menu *cfg)
> >>>          struct pxe_label *label;
> >>>          struct list_head *pos;
> >>>          struct menu *m;
> >>> +       char *label_override;
> >>>          int err;
> >>>          int i = 1;
> >>>          char *default_num = NULL;
> >>> +       char *override_num = NULL;
> >>>
> >>>          /*
> >>>           * Create a menu and add items for all the labels.
> >>> @@ -1367,6 +1369,8 @@ static struct menu *pxe_menu_to_menu(struct
> >>> pxe_menu *cfg)
> >>>          if (!m)
> >>>                  return NULL;
> >>>
> >>> +       label_override = env_get("pxe_label_override");
> >>> +
> >>>          list_for_each(pos, &cfg->labels) {
> >>>                  label = list_entry(pos, struct pxe_label, list);
> >>>
> >>> @@ -1375,11 +1379,22 @@ static struct menu *pxe_menu_to_menu(struct
> >>> pxe_menu *cfg)
> >>>                          menu_destroy(m);
> >>>                          return NULL;
> >>>                  }
> >>> +               if (label_override &&
> >>> +                   (strcmp(label->name, label_override) == 0))
> >> !strcmp()
> >>
> Why !strcmp() and not strcmp() ?

I just mean that instead of

   strcmp(a, b) == 0

you should do:

   !strcmp(a, b)

since it is a bit easier to read.

Regards,
Simon


>
>
> Regards,
>
> Amjad
>
> >>> +                       override_num = label->num;
> >>>                  if (cfg->default_label &&
> >>>                      (strcmp(label->name, cfg->default_label) == 0))
> >>>                          default_num = label->num;
> >>>          }
> >>>
> >>> +       if (label_override) {
> >>> +               if (override_num)
> >>> +                       default_num = override_num;
> >>> +               else
> >>> +                       printf("Missing override pxe label: %s\n",
> >>> +                                       label_override);
> >>> +       }
> >>> +
> >>>          /*
> >>>           * After we've created items for each label in the menu,
> >>> set the
> >>>           * menu's default label if one was specified.
> >>> --
> >>> 2.25.1
> >>>
> >> Regards,
> >> Simon