[v3,3/4] spl: fit: nand: fix fit loading in case of bad blocks

The offset at which the image to be loaded from NAND is located is
retrieved from the itb header. The presence of bad blocks in the area
of the NAND where the itb image is located could invalidate the offset
which must therefore be adjusted taking into account the state of the
sectors concerned.

cc: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dariobin@libero.it>

---

Changes in v3:
The previous versions were buggy for devices others than NAND. This
because the 'adjust_offset' helper was properly set only for the NAND
case but called even for devices like MMC, RAM, and so on, crashing the
boot up by that devices. Continuing to use the adjust_offset helper
would mean checking the helper before calling it and patching too many
files to set it properly before calling the spl_load_simple_fit routine.
For this reason, the adjust_offset helper has been removed from the
spl_image_info structure and the offset fixed inside the read helper for
the NAND device only. This solution fixes the problem for the NAND device
without side effects for other types of devices.

Changes in v2: None

 common/spl/spl_nand.c                   |  5 ++++-
 drivers/mtd/nand/raw/nand_spl_loaders.c | 28 +++++++++++++++++++++++++
 include/nand.h                          |  1 +
 3 files changed, 33 insertions(+), 1 deletion(-)

Hi Dario

I know that is an important bug to be addressed and I would like to
add even Tom on this

On Wed, May 27, 2020 at 1:56 PM Dario Binacchi <dariobin@libero.it> wrote:
>
> The offset at which the image to be loaded from NAND is located is
> retrieved from the itb header. The presence of bad blocks in the area
> of the NAND where the itb image is located could invalidate the offset
> which must therefore be adjusted taking into account the state of the
> sectors concerned.
>
> cc: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dariobin@libero.it>
>
> ---
>
> Changes in v3:
> The previous versions were buggy for devices others than NAND. This
> because the 'adjust_offset' helper was properly set only for the NAND
> case but called even for devices like MMC, RAM, and so on, crashing the
> boot up by that devices. Continuing to use the adjust_offset helper
> would mean checking the helper before calling it and patching too many
> files to set it properly before calling the spl_load_simple_fit routine.
> For this reason, the adjust_offset helper has been removed from the
> spl_image_info structure and the offset fixed inside the read helper for
> the NAND device only. This solution fixes the problem for the NAND device
> without side effects for other types of devices.
>
> Changes in v2: None
>
>  common/spl/spl_nand.c                   |  5 ++++-
>  drivers/mtd/nand/raw/nand_spl_loaders.c | 28 +++++++++++++++++++++++++
>  include/nand.h                          |  1 +
>  3 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c
> index 48c97549eb..1e6f2dce56 100644
> --- a/common/spl/spl_nand.c
> +++ b/common/spl/spl_nand.c
> @@ -42,8 +42,11 @@ static int spl_nand_load_image(struct spl_image_info *spl_image,
>  static ulong spl_nand_fit_read(struct spl_load_info *load, ulong offs,
>                                ulong size, void *dst)
>  {
> +       ulong sector;
>         int ret;
>
> +       sector = *(int *)load->priv;
> +       offs = sector + nand_spl_adjust_offset(sector, offs - sector);

Check if is needed to call adjust offset in case offs and sector are
the same. Maybe a comment is needed
on top of the function to describe a bit more why is called

>         ret = nand_spl_load_image(offs, size, dst);
>         if (!ret)
>                 return size;
> @@ -66,7 +69,7 @@ static int spl_nand_load_element(struct spl_image_info *spl_image,
>
>                 debug("Found FIT\n");
>                 load.dev = NULL;
> -               load.priv = NULL;
> +               load.priv = &offset;

It's a bit an abuse here but I don't have a better plan on my side

Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com>

Michael

>                 load.filename = NULL;
>                 load.bl_len = 1;
>                 load.read = spl_nand_fit_read;
> diff --git a/drivers/mtd/nand/raw/nand_spl_loaders.c b/drivers/mtd/nand/raw/nand_spl_loaders.c
> index 177c12b581..4befc75c04 100644
> --- a/drivers/mtd/nand/raw/nand_spl_loaders.c
> +++ b/drivers/mtd/nand/raw/nand_spl_loaders.c
> @@ -41,6 +41,34 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
>         return 0;
>  }
>
> +/**
> + * nand_spl_adjust_offset - Adjust offset from a starting sector
> + * @sector:    Address of the sector
> + * @offs:      Offset starting from @sector
> + *
> + * If one or more bad blocks are in the address space between @sector
> + * and @sector + @offs, @offs is increased by the NAND block size for
> + * each bad block found.
> + */
> +u32 nand_spl_adjust_offset(u32 sector, u32 offs)
> +{
> +       unsigned int block, lastblock;
> +
> +       block = sector / CONFIG_SYS_NAND_BLOCK_SIZE;
> +       lastblock = (sector + offs) / CONFIG_SYS_NAND_BLOCK_SIZE;
> +
> +       while (block <= lastblock) {
> +               if (nand_is_bad_block(block)) {
> +                       offs += CONFIG_SYS_NAND_BLOCK_SIZE;
> +                       lastblock++;
> +               }
> +
> +               block++;
> +       }
> +
> +       return offs;
> +}
> +
>  #ifdef CONFIG_SPL_UBI
>  /*
>   * Temporary storage for non NAND page aligned and non NAND page sized
> diff --git a/include/nand.h b/include/nand.h
> index 93cbe1e25d..80dd6469bc 100644
> --- a/include/nand.h
> +++ b/include/nand.h
> @@ -120,6 +120,7 @@ int nand_unlock(struct mtd_info *mtd, loff_t start, size_t length,
>                 int allexcept);
>  int nand_get_lock_status(struct mtd_info *mtd, loff_t offset);
>
> +u32 nand_spl_adjust_offset(u32 sector, u32 offs);
>  int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst);
>  int nand_spl_read_block(int block, int offset, int len, void *dst);
>  void nand_deselect(void);
> --
> 2.17.1
>

On Wed, May 27, 2020 at 01:56:20PM +0200, Dario Binacchi wrote:

> The offset at which the image to be loaded from NAND is located is
> retrieved from the itb header. The presence of bad blocks in the area
> of the NAND where the itb image is located could invalidate the offset
> which must therefore be adjusted taking into account the state of the
> sectors concerned.
> 
> cc: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dariobin@libero.it>
> Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com>

Applied to u-boot/master, thanks!