[U-Boot] Regression after "distro: not taint environment variables if possible"
Hi,
I found out that my Beaglebone didn't boot after:
https://github.com/u-boot/u-boot/commit/13dd6665ed18f72380ca596931d609bc108d...
I digged out the reason that this patch makes devnum a local variable, and it ends up shadowed by other code that sets devnum as a env variable.
For example to boot on the beaglebone I had to remove the setenv as in the patch below.
This only fixes for my board. Many other will likely have regressions.
Fixing it for all boards in a reliable way I think is very complex, unless the strategy is to wait for board maintainers to fix it as they need it, but I wonder how many latent bugs this will create for corner boot cases.
Maybe this change is not worth it?
Thanks, Nuno
diff --git a/include/environment/ti/mmc.h b/include/environment/ti/mmc.h index 785fc15345..bb4af0a3d5 100644 --- a/include/environment/ti/mmc.h +++ b/include/environment/ti/mmc.h @@ -56,7 +56,7 @@ "bootz; " \ "fi;\0" \ "mmcboot=mmc dev ${mmcdev}; " \ - "setenv devnum ${mmcdev}; " \ + "devnum=${mmcdev}; " \ "setenv devtype mmc; " \ "if mmc rescan; then " \ "echo SD/MMC found on device ${mmcdev};" \
On Wed, Jul 10, 2019 at 01:46:57PM +0200, Nuno Gonçalves wrote:
Hi,
I found out that my Beaglebone didn't boot after:
https://github.com/u-boot/u-boot/commit/13dd6665ed18f72380ca596931d609bc108d...
I digged out the reason that this patch makes devnum a local variable, and it ends up shadowed by other code that sets devnum as a env variable.
For example to boot on the beaglebone I had to remove the setenv as in the patch below.
This only fixes for my board. Many other will likely have regressions.
Fixing it for all boards in a reliable way I think is very complex, unless the strategy is to wait for board maintainers to fix it as they need it, but I wonder how many latent bugs this will create for corner boot cases.
Maybe this change is not worth it?
Thanks, Nuno
I've checked other usages here and only the TI case looks to have been a problem. I reworded the commit message as well.
Applied to u-boot/master, thanks!
Hi,
wrt my other mail "TI mmc env bug?", Nuno sent a proper patch, yet the commit has the syntax error (missing "="):
https://gitlab.denx.de/u-boot/u-boot/commit/27e0f3bcf07530a9cd272953797efda5...
Regards, Andre
On 27/08/2019 02:17, Tom Rini wrote:
On Wed, Jul 10, 2019 at 01:46:57PM +0200, Nuno Gonçalves wrote:
Hi,
I found out that my Beaglebone didn't boot after:
https://github.com/u-boot/u-boot/commit/13dd6665ed18f72380ca596931d609bc108d...
I digged out the reason that this patch makes devnum a local variable, and it ends up shadowed by other code that sets devnum as a env variable.
For example to boot on the beaglebone I had to remove the setenv as in the patch below.
This only fixes for my board. Many other will likely have regressions.
Fixing it for all boards in a reliable way I think is very complex, unless the strategy is to wait for board maintainers to fix it as they need it, but I wonder how many latent bugs this will create for corner boot cases.
Maybe this change is not worth it?
Thanks, Nuno
I've checked other usages here and only the TI case looks to have been a problem. I reworded the commit message as well.
Applied to u-boot/master, thanks!
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On Mon, Sep 09, 2019 at 12:17:47PM +0200, Andre Heider wrote:
Hi,
wrt my other mail "TI mmc env bug?", Nuno sent a proper patch, yet the commit has the syntax error (missing "="):
https://gitlab.denx.de/u-boot/u-boot/commit/27e0f3bcf07530a9cd272953797efda5...
Ugh, I don't know how that happened. Thanks for pointing it out, I'll fix it ASAP.
Regards, Andre
On 27/08/2019 02:17, Tom Rini wrote:
On Wed, Jul 10, 2019 at 01:46:57PM +0200, Nuno Gonçalves wrote:
Hi,
I found out that my Beaglebone didn't boot after:
https://github.com/u-boot/u-boot/commit/13dd6665ed18f72380ca596931d609bc108d...
I digged out the reason that this patch makes devnum a local variable, and it ends up shadowed by other code that sets devnum as a env variable.
For example to boot on the beaglebone I had to remove the setenv as in the patch below.
This only fixes for my board. Many other will likely have regressions.
Fixing it for all boards in a reliable way I think is very complex, unless the strategy is to wait for board maintainers to fix it as they need it, but I wonder how many latent bugs this will create for corner boot cases.
Maybe this change is not worth it?
Thanks, Nuno
I've checked other usages here and only the TI case looks to have been a problem. I reworded the commit message as well.
Applied to u-boot/master, thanks!
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
participants (3)
-
Andre Heider -
Nuno Gonçalves -
Tom Rini