| Message ID | 20190823144043.26792-1-scileont@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] cmd: pxe: Use internal FDT if external one cannot be retrieved | expand |
On 8/23/19 8:40 AM, Anton Leontiev wrote: > From: Anton Leontiev <aleontiev@elvees.com> > > Original commit c61d94d86035 ("pxe: implement fdtdir extlinux.conf tag") > states, that if FDT file cannot be retrieved then FDT packaged in > firmware should be used. It's not meant to say that. I believe the part of the description you're referring to is: if no FDT file was loaded, and $fdtaddr is set: # This indicates an FDT packaged with firmware use the FDT at $fdtaddr That wasn't meant to say anything about "if there was an error loading the FDT file", but rather is meant to mean "if no FDT file was loaded because extlinux.conf contained no fdt or fdtdir statement". Nothing there is intended to refer to errors loading a specified FDT file. > If FDT file cannot be retrieved and it is specified explicitly using > FDT keyword then the label is skipped. If it cannot be found in > FDTDIR then internal FDT is tried first. This makes the fdt/fdtdir keywords work differently. I don't think we want that. What specific problem are you trying to solve? And note that if we do change anything here, we should update the comment at around line 730, which describes the algorithm that's implemented.
2019-08-26 at 18:59, Stephen Warren <swarren@wwwdotorg.org>: > On 8/23/19 8:40 AM, Anton Leontiev wrote: > > From: Anton Leontiev <aleontiev@elvees.com> > > > > Original commit c61d94d86035 ("pxe: implement fdtdir extlinux.conf tag") > > states, that if FDT file cannot be retrieved then FDT packaged in > > firmware should be used. > > It's not meant to say that. I believe the part of the description you're > referring to is: > > if no FDT file was loaded, and $fdtaddr is set: > # This indicates an FDT packaged with firmware > use the FDT at $fdtaddr > > That wasn't meant to say anything about "if there was an error loading > the FDT file", but rather is meant to mean "if no FDT file was loaded > because extlinux.conf contained no fdt or fdtdir statement". Nothing > there is intended to refer to errors loading a specified FDT file. Indeed, I read it strictly as "if no FDT file was loaded" regardless the reason. Thank you for clarification. > > If FDT file cannot be retrieved and it is specified explicitly using > > FDT keyword then the label is skipped. If it cannot be found in > > FDTDIR then internal FDT is tried first. > > This makes the fdt/fdtdir keywords work differently. I don't think we > want that. FDT will work as always. FDTDIR will be less strict. It doesn't specify exact file to be loaded, that's why it should not fail if there is no such file. > What specific problem are you trying to solve? We have a GNU/Linux distribution that use FDTDIR in its extlinux.conf to support several boards. But some boards have FDT embedded in U-Boot and it is't present in FDTDIR. In such configuration U-Boot fails to boot an entry, despite no exact FDT is specified in it. Distribution itself is designed to work on any board. > And note that if we do change anything here, we should update the > comment at around line 730, which describes the algorithm that's > implemented. Thank you, I'll update the comment.
On 8/29/19 5:20 AM, Anton Leontiev wrote: > 2019-08-26 at 18:59, Stephen Warren <swarren@wwwdotorg.org>: >> On 8/23/19 8:40 AM, Anton Leontiev wrote: >>> From: Anton Leontiev <aleontiev@elvees.com> >>> >>> Original commit c61d94d86035 ("pxe: implement fdtdir extlinux.conf tag") >>> states, that if FDT file cannot be retrieved then FDT packaged in >>> firmware should be used. >> >> It's not meant to say that. I believe the part of the description you're >> referring to is: >> >> if no FDT file was loaded, and $fdtaddr is set: >> # This indicates an FDT packaged with firmware >> use the FDT at $fdtaddr >> >> That wasn't meant to say anything about "if there was an error loading >> the FDT file", but rather is meant to mean "if no FDT file was loaded >> because extlinux.conf contained no fdt or fdtdir statement". Nothing >> there is intended to refer to errors loading a specified FDT file. ... >> What specific problem are you trying to solve? > > We have a GNU/Linux distribution that use FDTDIR in its extlinux.conf > to support several boards. But some boards have FDT embedded in U-Boot > and it is't present in FDTDIR. In such configuration U-Boot fails to > boot an entry, despite no exact FDT is specified in it. Distribution > itself is designed to work on any board. I lookead at that referenced commit description in full and the code, and I believe what you want is for U-Boot to set fdt_addr to the location of the in-RAM DT, and leave fdt_addr_r unset. This will indicate to the pxe code that no FDT should be loaded when parsing extlinux.conf, but instead to use fdt_addr directly. Does that work for you, or does it break some other script/use-case on this board?
чт, 29 авг. 2019 г. в 23:35, Stephen Warren <swarren@wwwdotorg.org>: > On 8/29/19 5:20 AM, Anton Leontiev wrote: > > 2019-08-26 at 18:59, Stephen Warren <swarren@wwwdotorg.org>: > > We have a GNU/Linux distribution that use FDTDIR in its extlinux.conf > > to support several boards. But some boards have FDT embedded in U-Boot > > and it is't present in FDTDIR. In such configuration U-Boot fails to > > boot an entry, despite no exact FDT is specified in it. Distribution > > itself is designed to work on any board. > > I lookead at that referenced commit description in full and the code, > and I believe what you want is for U-Boot to set fdt_addr to the > location of the in-RAM DT, and leave fdt_addr_r unset. This will > indicate to the pxe code that no FDT should be loaded when parsing > extlinux.conf, but instead to use fdt_addr directly. > > Does that work for you, or does it break some other script/use-case on > this board? Indeed, it's a possible option. However, if fdt_addr_r is not set, user can't override embedded FDT using extlinux.conf. README.distro says about fdt_addr_r: "This is mandatory even when fdt_addr is provided, since extlinux.conf must always be able to provide a DTB which overrides any copy provided by the HW." So it should be considered as a workaround rather than a solution. Best regards, Anton Leontiev
On 8/31/19 1:52 PM, Anton Leontiev wrote: > чт, 29 авг. 2019 г. в 23:35, Stephen Warren <swarren@wwwdotorg.org>: >> On 8/29/19 5:20 AM, Anton Leontiev wrote: >>> 2019-08-26 at 18:59, Stephen Warren <swarren@wwwdotorg.org>: >>> We have a GNU/Linux distribution that use FDTDIR in its extlinux.conf >>> to support several boards. But some boards have FDT embedded in U-Boot >>> and it is't present in FDTDIR. In such configuration U-Boot fails to >>> boot an entry, despite no exact FDT is specified in it. Distribution >>> itself is designed to work on any board. >> >> I lookead at that referenced commit description in full and the code, >> and I believe what you want is for U-Boot to set fdt_addr to the >> location of the in-RAM DT, and leave fdt_addr_r unset. This will >> indicate to the pxe code that no FDT should be loaded when parsing >> extlinux.conf, but instead to use fdt_addr directly. >> >> Does that work for you, or does it break some other script/use-case on >> this board? > > Indeed, it's a possible option. However, if fdt_addr_r is not set, > user can't override embedded FDT using extlinux.conf. README.distro > says about fdt_addr_r: "This is mandatory even when fdt_addr is > provided, since extlinux.conf must always be able to provide a DTB > which overrides any copy provided by the HW." > > So it should be considered as a workaround rather than a solution. OK, I guess that makes sense. I suppose it's not reasonable to ask that extlinux.conf doesn't contain an FDTDIR statement in the case where the specific board has a built-in DT, since the extlinux.conf content is supposed to be generic, and not tailored to the current HW.
diff --git a/cmd/pxe.c b/cmd/pxe.c index 2059975446..28390c114c 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -795,9 +795,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;