On 10/04/2013 03:35 PM, Wolfgang Denk wrote:
Dear Tom Rini,
In message 1380901758-30360-1-git-send-email-trini@ti.com you wrote:
setenv_hex is only called with hex values, but does not prefix the strings with '0x', as in general U-Boot assumes hex values not decimal values, and this saves space in the environment. However, some functions such as 'load' take some values that are most easily described in hex (load address) and decimal (size, offset within a file).
This can lead to the situation where, for example, spl export is run, which leads to a call of setenv_hex of the fdtaddr, which will be re-set in the environment. Then 'saveenv' may be run (after updating other parts of the environment for falcon mode), causing an invalid for 'load' fdtaddr to be saved to the environment and leading to future boots to fail if using 'load' to read the fdt file.
I think this is not a correct way to fix the issues at hand.
All U-Boot (with the single unfortunate exception of the "sleep" command) defaults to hex input - that's what's documented, and what people have always been using for many, many years. Applying your patch means that we modify just a single use case, while setting the same environment variables from the command line (without the leading 0x) would still trigger a problem.
I understand that the actual cause of these issues is commit 3f83c87 "fs: fix number base behaviour change in fatload/ext*load". Reviewing this patch I think that it should have never been applied. The fact that "load" and "fatload" / "ext*load" behave differently are reason enough to reject this patch. "load" should just behave like every other command, and default to hex input.
I think we should NAK your patch, and suggest to fix the problem by reverting commit 3f83c87 and making "load" default to hex input mode.
Reverting 3f83c87 would do the opposite of what you want; it'd make extload/fatload require 0x prefixes instead of assuming hex. Perhaps what you want is a tweak to that patch so that the generic load/ls commands always expect a hex value, rather than requiring the 0x prefix?