Hi On Fri, Jul 20, 2018 at 10:11 PM, Tom Rini <tr...@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 <tr...@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 <mich...@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 -- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com | _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot