On Fri, Jul 20, 2018 at 10:27:26PM +0200, Michael Nazzareno Trimarchi wrote:
> 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)
CONFIG_SPL_VERIFY_IMAGE, for namespace.
> +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()
Yes, something like that. And please pass it through travis-ci for the
world build and seeing if anything fails to fit into size constraints,
thanks!
--
Tom
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
