| Message ID | 20221213204631.175285-1-marex@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [RFC] cmd: pxe_utils: Limit fdtcontroladdr usage to non-fitImage | expand |
On Tue, 13 Dec 2022 at 12:46, Marek Vasut <marex@denx.de> wrote: > > Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") > forces '$fdtcontroladdr' DT address as a third parameter of bootm command > even if the PXE transfer pulls in a fitImage which contains configuration > node with its own DT that is preferrable to be passed to Linux. Limit the > $fdtcontroladdr fallback utilization to non-fitImages, since it is highly > likely a fitImage would come with its own DT, while single-file images do > need a separate DT. > > Fixes: d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Peter Hoyes <Peter.Hoyes@arm.com> > Cc: Ramon Fried <rfried.dev@gmail.com> > Cc: Simon Glass <sjg@chromium.org> > --- > boot/pxe_utils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass <sjg@chromium.org> I'd suggest adding an explicit comment in the code too, so it is easier to understand when this gets refactored later.
On 12/14/22 05:39, Simon Glass wrote: > On Tue, 13 Dec 2022 at 12:46, Marek Vasut <marex@denx.de> wrote: >> >> Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") >> forces '$fdtcontroladdr' DT address as a third parameter of bootm command >> even if the PXE transfer pulls in a fitImage which contains configuration >> node with its own DT that is preferrable to be passed to Linux. Limit the >> $fdtcontroladdr fallback utilization to non-fitImages, since it is highly >> likely a fitImage would come with its own DT, while single-file images do >> need a separate DT. >> >> Fixes: d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") >> Signed-off-by: Marek Vasut <marex@denx.de> >> --- >> Cc: Peter Hoyes <Peter.Hoyes@arm.com> >> Cc: Ramon Fried <rfried.dev@gmail.com> >> Cc: Simon Glass <sjg@chromium.org> >> --- >> boot/pxe_utils.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > I'd suggest adding an explicit comment in the code too, so it is > easier to understand when this gets refactored later. I sent a fixed version of this patch, can you add this RB to it too ? Regarding comment in the code, there is already one massive comment explaining it just above, see the pxe_utils.c content in the sources.
Hi Marek, On Tue, 13 Dec 2022 at 20:50, Marek Vasut <marex@denx.de> wrote: > > On 12/14/22 05:39, Simon Glass wrote: > > On Tue, 13 Dec 2022 at 12:46, Marek Vasut <marex@denx.de> wrote: > >> > >> Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") > >> forces '$fdtcontroladdr' DT address as a third parameter of bootm command > >> even if the PXE transfer pulls in a fitImage which contains configuration > >> node with its own DT that is preferrable to be passed to Linux. Limit the > >> $fdtcontroladdr fallback utilization to non-fitImages, since it is highly > >> likely a fitImage would come with its own DT, while single-file images do > >> need a separate DT. > >> > >> Fixes: d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") > >> Signed-off-by: Marek Vasut <marex@denx.de> > >> --- > >> Cc: Peter Hoyes <Peter.Hoyes@arm.com> > >> Cc: Ramon Fried <rfried.dev@gmail.com> > >> Cc: Simon Glass <sjg@chromium.org> > >> --- > >> boot/pxe_utils.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > I'd suggest adding an explicit comment in the code too, so it is > > easier to understand when this gets refactored later. > > I sent a fixed version of this patch, can you add this RB to it too ? > > Regarding comment in the code, there is already one massive comment > explaining it just above, see the pxe_utils.c content in the sources. " * Scenario 3: If there is an fdtcontroladdr specified, pass it along to * bootm, and adjust argc appropriately." But are you not changing that? So update the comment? Regards, Simon
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 8133006875f..a25f0c22888 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -724,7 +724,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (!bootm_argv[3]) bootm_argv[3] = env_get("fdt_addr"); - if (!bootm_argv[3]) + if (!bootm_argv[3] && genimg_get_format(buf) != IMAGE_FORMAT_FIT) bootm_argv[3] = env_get("fdtcontroladdr"); if (bootm_argv[3]) {
Commit d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") forces '$fdtcontroladdr' DT address as a third parameter of bootm command even if the PXE transfer pulls in a fitImage which contains configuration node with its own DT that is preferrable to be passed to Linux. Limit the $fdtcontroladdr fallback utilization to non-fitImages, since it is highly likely a fitImage would come with its own DT, while single-file images do need a separate DT. Fixes: d5ba6188dfb ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Peter Hoyes <Peter.Hoyes@arm.com> Cc: Ramon Fried <rfried.dev@gmail.com> Cc: Simon Glass <sjg@chromium.org> --- boot/pxe_utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)