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 <dario...@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 <mich...@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dario...@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 <mich...@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 |
