[u-boot][PATCH] spl: spl_legacy: Fix NAND boot on OMAP3 BeagleBoard

Michael Nazzareno Trimarchi michael at amarulasolutions.com
Wed Oct 26 08:30:40 CEST 2022


Hi Roger

On Mon, Oct 10, 2022 at 12:38 PM Roger Quadros <rogerq at kernel.org> wrote:
>
> +Dario
>
> On 29/09/2022 13:11, Roger Quadros wrote:
> > OMAP3 BeagleBoard NAND boot hangs when spl_load_legacy_img() tries
> > to read the header into 'struct hdr' which is allocated on the
> > stack.
> >
> > As the header has already been read once before by spl_nand.c,
> > we can avoid the extra header read here by simply passing around
> > the pointer to the header.
> >
> > This fixes NAND boot on OMAP3 BeagleBoard.
> >

I can see that fix is only a collateral effect of reduce the
allocation on this scenario.

I think that is better to mention as it. So the real change is to
avoid to allocate two times
mostly

Reviewed-By: Michael Trimarchi <michael at amarulasolutions.com>

> > Signed-off-by: Roger Quadros <rogerq at kernel.org>
> > ---
> >  common/spl/spl_legacy.c | 19 ++++++++-----------
> >  common/spl/spl_nand.c   |  2 +-
> >  common/spl/spl_nor.c    |  6 +++++-
> >  include/spl.h           |  7 +++++--
> >  4 files changed, 19 insertions(+), 15 deletions(-)
> >
> > diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
> > index ae8731c782..2e9226c990 100644
> > --- a/common/spl/spl_legacy.c
> > +++ b/common/spl/spl_legacy.c
> > @@ -77,32 +77,29 @@ static inline int spl_image_get_comp(const struct image_header *hdr)
> >
> >  int spl_load_legacy_img(struct spl_image_info *spl_image,
> >                       struct spl_boot_device *bootdev,
> > -                     struct spl_load_info *load, ulong header)
> > +                     struct spl_load_info *load, ulong offset,
> > +                     struct image_header *hdr)
> >  {
> >       __maybe_unused SizeT lzma_len;
> >       __maybe_unused void *src;
> > -     struct image_header hdr;
> >       ulong dataptr;
> >       int ret;
> >
> > -     /* Read header into local struct */
> > -     load->read(load, header, sizeof(hdr), &hdr);
> > -
> >       /*
> >        * If the payload is compressed, the decompressed data should be
> >        * directly write to its load address.
> >        */
> > -     if (spl_image_get_comp(&hdr) != IH_COMP_NONE)
> > +     if (spl_image_get_comp(hdr) != IH_COMP_NONE)
> >               spl_image->flags |= SPL_COPY_PAYLOAD_ONLY;
> >
> > -     ret = spl_parse_image_header(spl_image, bootdev, &hdr);
> > +     ret = spl_parse_image_header(spl_image, bootdev, hdr);
> >       if (ret)
> >               return ret;
> >
> >       /* Read image */
> > -     switch (spl_image_get_comp(&hdr)) {
> > +     switch (spl_image_get_comp(hdr)) {
> >       case IH_COMP_NONE:
> > -             dataptr = header;
> > +             dataptr = offset;
> >
> >               /*
> >                * Image header will be skipped only if SPL_COPY_PAYLOAD_ONLY
> > @@ -119,7 +116,7 @@ int spl_load_legacy_img(struct spl_image_info *spl_image,
> >               lzma_len = LZMA_LEN;
> >
> >               /* dataptr points to compressed payload  */
> > -             dataptr = header + sizeof(hdr);
> > +             dataptr = offset + sizeof(hdr);
> >
> >               debug("LZMA: Decompressing %08lx to %08lx\n",
> >                     dataptr, spl_image->load_addr);
> > @@ -143,7 +140,7 @@ int spl_load_legacy_img(struct spl_image_info *spl_image,
> >
> >       default:
> >               debug("Compression method %s is not supported\n",
> > -                   genimg_get_comp_short_name(image_get_comp(&hdr)));
> > +                   genimg_get_comp_short_name(image_get_comp(hdr)));
> >               return -EINVAL;
> >       }
> >
> > diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c
> > index 7b7579a2df..5eb67b5468 100644
> > --- a/common/spl/spl_nand.c
> > +++ b/common/spl/spl_nand.c
> > @@ -119,7 +119,7 @@ static int spl_nand_load_element(struct spl_image_info *spl_image,
> >               load.bl_len = 1;
> >               load.read = spl_nand_legacy_read;
> >
> > -             return spl_load_legacy_img(spl_image, bootdev, &load, offset);
> > +             return spl_load_legacy_img(spl_image, bootdev, &load, offset, header);
> >       } else {
> >               err = spl_parse_image_header(spl_image, bootdev, header);
> >               if (err)
> > diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c
> > index 7986e930d2..f00a5c395b 100644
> > --- a/common/spl/spl_nor.c
> > +++ b/common/spl/spl_nor.c
> > @@ -111,10 +111,14 @@ static int spl_nor_load_image(struct spl_image_info *spl_image,
> >
> >       /* Legacy image handling */
> >       if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_FORMAT)) {
> > +             struct image_header hdr;
> > +
> >               load.bl_len = 1;
> >               load.read = spl_nor_load_read;
> > +             spl_nor_load_read(&load, spl_nor_get_uboot_base(), sizeof(hdr), &hdr);
> >               return spl_load_legacy_img(spl_image, bootdev, &load,
> > -                                        spl_nor_get_uboot_base());
> > +                                        spl_nor_get_uboot_base(),
> > +                                        &hdr);
> >       }
> >
> >       return 0;
> > diff --git a/include/spl.h b/include/spl.h
> > index aac6648f94..7fa5e51c39 100644
> > --- a/include/spl.h
> > +++ b/include/spl.h
> > @@ -353,7 +353,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
> >   * spl_load_legacy_img() - Loads a legacy image from a device.
> >   * @spl_image:       Image description to set up
> >   * @load:    Structure containing the information required to load data.
> > - * @header:  Pointer to image header (including appended image)
> > + * @offset:  Pointer to image
> > + * @hdr:     Pointer to image header
> >   *
> >   * Reads an legacy image from the device. Loads u-boot image to
> >   * specified load address.
> > @@ -361,7 +362,9 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
> >   */
> >  int spl_load_legacy_img(struct spl_image_info *spl_image,
> >                       struct spl_boot_device *bootdev,
> > -                     struct spl_load_info *load, ulong header);
> > +                     struct spl_load_info *load, ulong offset,
> > +                     struct image_header *hdr);
> > +
> >
> >  /**
> >   * spl_load_imx_container() - Loads a imx container image from a device.



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael at amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info at amarulasolutions.com
www.amarulasolutions.com


More information about the U-Boot mailing list