Hi Art, On Tue, 12 Oct 2021 at 21:45, Art Nikpal <email2t...@gmail.com> wrote: > > Yes changes inside include/config_distro_bootcmd.h not the best solution for > this issue. > I think it is better to change sysboot cmd and i have prepared another > solution already! > https://patchwork.ozlabs.org/project/uboot/patch/20211013033912.3297227-1-...@khadas.com/ > how about this ?
Seem reasonable to me, but could you check out my syslinux refactor series? It could use some review. http://patchwork.ozlabs.org/project/uboot/list/?series=264265 - Simon > > On Wed, Oct 13, 2021 at 5:07 AM Tom Rini <tr...@konsulko.com> wrote: >> >> On Tue, Oct 12, 2021 at 02:31:18PM -0600, Simon Glass wrote: >> > Hi Tom, >> > >> > On Tue, 12 Oct 2021 at 13:44, Tom Rini <tr...@konsulko.com> wrote: >> > > >> > > On Tue, Oct 12, 2021 at 04:55:44PM +0800, Artem Lapkin wrote: >> > > >> > > > Problem >> > > > >> > > > PXE cannot boot normally after Sysboot changed the bootfile env (called >> > > > from boot_extlinux) from the default "boot.scr.uimg" to >> > > > "/boot/extlinux/extlinux.conf". >> > > > >> > > > In addition, an unbootable extlinux configuration will also make the >> > > > PXE >> > > > boot unbootable, because it will use the incorrect path >> > > > "/boot/extlinux/" >> > > > from the bootfile env. >> > > > >> > > > Solution >> > > > >> > > > Save and restore default bootfile env value when boot_extlinux is used. >> > > > >> > > > Example >> > > > ================================================================ >> > > > Hit SPACE in 2 seconds to stop autoboot >> > > > ... is now current device >> > > > Found /boot/extlinux/extlinux.conf >> > > > Retrieving file: /boot/extlinux/extlinux.conf >> > > > 413 bytes read in 2 ms (201.2 KiB/s) >> > > > Skipping Krescue for failure retrieving kernel >> > > > SCRIPT FAILED: continuing... >> > > > ... >> > > > Speed: 1000, full duplex >> > > > BOOTP broadcast 1 >> > > > DHCP client bound to address 192.168.11.151 (8 ms) >> > > > Using ethernet@ff3f0000 device >> > > > TFTP from server 192.168.11.1; our IP address is 192.168.11.151 >> > > > Filename '/boot/extlinux/pxelinux.cfg/default'. >> > > > Not retrying... >> > > > ================================================================ >> > > > >> > > > Signed-off-by: Artem Lapkin <a...@khadas.com> >> > > > --- >> > > > include/config_distro_bootcmd.h | 2 ++ >> > > > 1 file changed, 2 insertions(+) >> > > > >> > > > diff --git a/include/config_distro_bootcmd.h >> > > > b/include/config_distro_bootcmd.h >> > > > index 3f724aa10f..db3d1b2362 100644 >> > > > --- a/include/config_distro_bootcmd.h >> > > > +++ b/include/config_distro_bootcmd.h >> > > > @@ -445,7 +445,9 @@ >> > > > "${devnum}:${distro_bootpart} " >> > > > \ >> > > > "${prefix}${boot_syslinux_conf}; then " >> > > > \ >> > > > "echo Found ${prefix}${boot_syslinux_conf}; " >> > > > \ >> > > > + "bootfile_bak=${bootfile}; " >> > > > \ >> > > > "run boot_extlinux; " >> > > > \ >> > > > + "setenv bootfile ${bootfile_bak}; " >> > > > \ >> > > > "echo SCRIPT FAILED: continuing...; " >> > > > \ >> > > > "fi\0" >> > > > \ >> > > > \ >> > > >> > > We've had this kind of problem before, and the answer is that variables >> > > should be local, not global in scope. In this case, I see that the way >> > > the pxe/sysboot code works is that we have to env_set("..") in one place >> > > to env_get("..") in another, so I don't see a way around this. >> > > >> > > Reviewed-by: Tom Rini <tr...@konsulko.com> >> > >> > IMO a better approach will be the bootflow implementation. I hope to >> > get v2 out early next week. >> >> I'm not sure if the bootflow way of going here would or would not have >> the same problem, or perhaps a slightly different problem. At heart >> here, "sysboot" calls env_set(...) and then calls in to the pxe code >> which does env_get(...). So now I wonder how this would be fixed in >> bootflow, since we aren't dealing with the environment directly. >> >> -- >> Tom