| Message ID | 20211022155526.54368-1-aouledameur@baylibre.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | cmd: pxe_utils: sysboot: add label override support | expand |
Hi Amjad, On Fri, 22 Oct 2021 at 09:55, Amjad Ouled-Ameur <aouledameur@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@baylibre.com> > > --- > > cmd/pxe_utils.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) Can we add this to the docs somewhere? > > 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() > + 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
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@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@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() > >> + 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
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@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@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() ? 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
Hi Amjad, On Fri, 12 Nov 2021 at 09:46, Amjad Ouled-Ameur <aouledameur@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@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@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
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)) + 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.
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@baylibre.com> --- cmd/pxe_utils.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)