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

[Michael Nazzareno Trimarchi]

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 at 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 at amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dariobin at 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 at 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
>


-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |