| Message ID | 20190903075224.9404-1-scileont@gmail.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,v2] cmd: pxe: Use internal FDT if retrieving from FDTDIR fails | expand |
On 9/3/19 1:52 AM, Anton Leontiev wrote: > From: Anton Leontiev <aleontiev@elvees.com> > > As FDTDIR label doesn't specify exact file to be loaded, it should > not fail if no file exists in the directory. In this case try to boot > with internal FDT if it exists. > > Signed-off-by: Anton Leontiev <aleontiev@elvees.com> > --- > cmd/pxe.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/cmd/pxe.c b/cmd/pxe.c > index 2059975446..1eec6d29bf 100644 > --- a/cmd/pxe.c > +++ b/cmd/pxe.c > @@ -727,11 +727,14 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label) > > /* > * fdt usage is optional: > - * It handles the following scenarios. All scenarios are exclusive > + * It handles the following scenarios. > * > - * Scenario 1: If fdt_addr_r specified and "fdt" label is defined in > - * pxe file, retrieve fdt blob from server. Pass fdt_addr_r to bootm, > - * and adjust argc appropriately. > + * Scenario 1: If fdt_addr_r specified and "fdt" or "fdtdir" label is > + * defined in pxe file, retrieve fdt blob from server. Pass fdt_addr_r to > + * bootm, and adjust argc appropriately. > + * > + * If retrieve fails and no exact fdt blob is specified in pxe file with > + * "fdt" label, try Scenario 2. Is it possible/sensible to distinguish between "file not found" and "error during retrieval"? "File not found" indicates the case you care about, and continuing to use a built-in DT makes sense here. "Error during retrieval" indicates that the file was found, and hence really should be use, yet couldn't be due to download failure. In this case, I wonder if we shouldn't outright fail this boot option, just like the existing code does? But either way, I suppose this patch is reasonable. > * Scenario 2: If there is an fdt_addr specified, pass it along to > * bootm, and adjust argc appropriately. > @@ -795,9 +798,13 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label) > int err = get_relfile_envaddr(cmdtp, fdtfile, "fdt_addr_r"); > free(fdtfilefree); > if (err < 0) { > - printf("Skipping %s for failure retrieving fdt\n", > - label->name); > - goto cleanup; > + bootm_argv[3] = NULL; > + > + if (label->fdt) { > + printf("Skipping %s for failure retrieving FDT\n", > + label->name); > + goto cleanup; > + } > } > } else { > bootm_argv[3] = NULL; >
2019-09-03 19:18, Stephen Warren <swarren@wwwdotorg.org>: > Is it possible/sensible to distinguish between "file not found" and > "error during retrieval"? "File not found" indicates the case you care > about, and continuing to use a built-in DT makes sense here. "Error > during retrieval" indicates that the file was found, and hence really > should be use, yet couldn't be due to download failure. In this case, I > wonder if we shouldn't outright fail this boot option, just like the > existing code does? > > But either way, I suppose this patch is reasonable. As I see, for example from do_get_ext2(), currently it is not possible to distinguish between "file not found" and "error during retrieval". It seems possible to update all do_get_*() functions to check file existence before retrieving, but it will require extensive testing and I prefer this to be changed independently to current patch. Best regards,
2019-09-03 10:52, Anton Leontiev <scileont@gmail.com>: > From: Anton Leontiev <aleontiev@elvees.com> > > As FDTDIR label doesn't specify exact file to be loaded, it should > not fail if no file exists in the directory. In this case try to boot > with internal FDT if it exists. > > Signed-off-by: Anton Leontiev <aleontiev@elvees.com> Tom, could you take a look at my patch during the merge window? Or should I pass it through some custodian? Best regards, Anton Leontiev
On Tue, Sep 03, 2019 at 10:52:24AM +0300, Anton Leontiev wrote: > From: Anton Leontiev <aleontiev@elvees.com> > > As FDTDIR label doesn't specify exact file to be loaded, it should > not fail if no file exists in the directory. In this case try to boot > with internal FDT if it exists. > > Signed-off-by: Anton Leontiev <aleontiev@elvees.com> Sorry for the delay, applied to u-boot/next, thanks!
diff --git a/cmd/pxe.c b/cmd/pxe.c index 2059975446..1eec6d29bf 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -727,11 +727,14 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label) /* * fdt usage is optional: - * It handles the following scenarios. All scenarios are exclusive + * It handles the following scenarios. * - * Scenario 1: If fdt_addr_r specified and "fdt" label is defined in - * pxe file, retrieve fdt blob from server. Pass fdt_addr_r to bootm, - * and adjust argc appropriately. + * Scenario 1: If fdt_addr_r specified and "fdt" or "fdtdir" label is + * defined in pxe file, retrieve fdt blob from server. Pass fdt_addr_r to + * bootm, and adjust argc appropriately. + * + * If retrieve fails and no exact fdt blob is specified in pxe file with + * "fdt" label, try Scenario 2. * * Scenario 2: If there is an fdt_addr specified, pass it along to * bootm, and adjust argc appropriately. @@ -795,9 +798,13 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label) int err = get_relfile_envaddr(cmdtp, fdtfile, "fdt_addr_r"); free(fdtfilefree); if (err < 0) { - printf("Skipping %s for failure retrieving fdt\n", - label->name); - goto cleanup; + bootm_argv[3] = NULL; + + if (label->fdt) { + printf("Skipping %s for failure retrieving FDT\n", + label->name); + goto cleanup; + } } } else { bootm_argv[3] = NULL;