Hi
On Fri, Jul 20, 2018 at 10:11 PM, Tom Rini trini@konsulko.com wrote:
On Fri, Jul 20, 2018 at 10:09:00PM +0200, Michael Nazzareno Trimarchi wrote:
Hi Tom
On Fri, Jul 20, 2018 at 9:54 PM, Tom Rini trini@konsulko.com wrote:
On Fri, Jul 06, 2018 at 05:09:22PM +0200, Michael Trimarchi wrote:
We need to address the redundat image case and undestand if the image is corrupted or not. In error case we need to try the fallback copy. The function used before was always return 0 without any evaluation of the error. We try to make it work properly
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com
Changes V2->V3: Fix patch mistake due the a wrong edit of it Changes V1->V2: Address the comments on using the err variable
common/spl/spl_nand.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)
I see two problems here. First, this is a generic issue (any legacy-style U-Boot image that we load should be verified). Second, we need to make this behavior configurable as as-is this overflows one board (omapl138_lcdk) and I expect would be problematic for many more boards when we make it done more commonly.
This patch fix a no-working uboot feature and this was the address problem on the specific case. We can call ->verify image every ->load, anyway can you explain better why you need a configurable behavior?
It fixes the legitimate case of having read in bad data and the controller didn't return up an error to us, and it wasn't in the header, yes. And it needs to be configurable as adding in these checks increases the binary size and various targets will fail to build, such as omapl138_lcdk does with your patch as-is. Thanks!
+#if defined(CONFIG_VERIFY_IMAGE) +int verify_image(spl_image) +{ +... +} +#else +int verify_image(spl_image) { return 0; } +#endif + static int spl_load_image(struct spl_image_info *spl_image, struct spl_image_loader *loader) { struct spl_boot_device bootdev; + int ret;
bootdev.boot_device = loader->boot_device; bootdev.boot_device_name = NULL;
- return loader->load_image(spl_image, &bootdev); + ret = loader->load_image(spl_image, &bootdev); + if (ret) + return -EINVAL; + + return verify_image(spl_image); }
Somenthing like this?
or return loader->verify_image()
Michael
-- Tom