[PATCH 16/19] drivers: mtd: nand: spl: Add support for nand SPL load image

[Maniyam, Dinesh]

> -----Original Message-----
> From: Michael Nazzareno Trimarchi <michael at amarulasolutions.com>
> Sent: Saturday, 5 October 2024 11:23 pm
> To: Maniyam, Dinesh <dinesh.maniyam at intel.com>
> Cc: u-boot at lists.denx.de; Marek <marex at denx.de>; Simon
> <simon.k.r.goldschmidt at gmail.com>; Tom Rini <trini at konsulko.com>; Dario
> Binacchi <dario.binacchi at amarulasolutions.com>; Johan Jonker
> <jbx6244 at gmail.com>; Michal Simek <michal.simek at amd.com>; Arseniy
> Krasnov <avkrasnov at salutedevices.com>; Alexander Dahl <ada at thorsis.com>;
> William Zhang <william.zhang at broadcom.com>; Igor Prusov
> <ivprusov at salutedevices.com>; Chee, Tien Fong <tien.fong.chee at intel.com>;
> Hea, Kok Kiang <kok.kiang.hea at intel.com>; Ng, Boon Khai
> <boon.khai.ng at intel.com>; Yuslaimi, Alif Zakuan
> <alif.zakuan.yuslaimi at intel.com>; Teik Heng <teik.heng.chong at intel.com>;
> Zamri, Muhammad Hazim Izzat <muhammad.hazim.izzat.zamri at intel.com>;
> Meng, Tingting <tingting.meng at intel.com>; Lim, Jit Loon
> <jit.loon.lim at intel.com>; Tang, Sieu Mun <sieu.mun.tang at intel.com>
> Subject: Re: [PATCH 16/19] drivers: mtd: nand: spl: Add support for nand SPL load
> image
> 
> Hi Dinesh
> 
> On Thu, Sep 19, 2024 at 5:56 AM <dinesh.maniyam at intel.com> wrote:
> >
> > From: Dinesh Maniyam <dinesh.maniyam at intel.com>
> >
> > This patch is add support for spl nand to load binary image from NAND
> > to RAM.
> >
> > Signed-off-by: Dinesh Maniyam <dinesh.maniyam at intel.com>
> > ---
> >  drivers/mtd/nand/raw/cadence_spl.c | 96
> > ++++++++++++++++++++++++++++++
> >  1 file changed, 96 insertions(+)
> >  create mode 100644 drivers/mtd/nand/raw/cadence_spl.c
> >
> > diff --git a/drivers/mtd/nand/raw/cadence_spl.c
> > b/drivers/mtd/nand/raw/cadence_spl.c
> > new file mode 100644
> > index 0000000000..841801420d
> > --- /dev/null
> > +++ b/drivers/mtd/nand/raw/cadence_spl.c
> > @@ -0,0 +1,96 @@
> > +// SPDX-License-Identifier:     GPL-2.0
> > +/*
> > + * Copyright (C) 2024 Intel Corporation <www.intel.com>  */
> > +
> > +#include <cadence-nand.h>
> > +#include <dm.h>
> > +#include <hang.h>
> > +#include <malloc.h>
> > +#include <memalign.h>
> > +#include <nand.h>
> > +#include <linux/bitfield.h>
> > +
> > +int nand_spl_load_image(u32 offset, u32 len, void *dst) {
> > +       size_t count = len, actual = 0, page_align_overhead = 0;
> > +       u32 page_align_offset = 0;
> > +       u8 *page_buffer;
> > +       int err = 0;
> > +       struct mtd_info *mtd;
> > +
> > +       if (!len || !dst)
> > +               return -EINVAL;
> > +
> > +       mtd = get_nand_dev_by_index(nand_curr_device);
> > +       if (!mtd)
> > +               hang();
> > +
> > +       if ((offset & (mtd->writesize - 1)) != 0) {
> > +               page_buffer = malloc_cache_aligned(mtd->writesize);
> > +               if (!page_buffer) {
> > +                       debug("Error: allocating buffer\n");
> > +                       return -ENOMEM;
> > +               }
> > +
> > +               page_align_overhead = offset % mtd->writesize;
> > +               page_align_offset = (offset / mtd->writesize) * mtd->writesize;
> > +               count = mtd->writesize;
> > +
> > +               err = nand_read_skip_bad(mtd, page_align_offset, &count,
> > +                                        &actual, mtd->size,
> > + page_buffer);
> > +
> > +               if (err) {
> > +                       free(page_buffer);
> > +                       return err;
> > +               }
> > +
> > +               count -= page_align_overhead;
> > +               count = min((size_t)len, count);
> > +               memcpy(dst, page_buffer + page_align_overhead, count);
> > +               free(page_buffer);
> > +
> > +               len -= count;
> > +               if (!len)
> > +                       return err;
> > +
> > +               offset += count;
> > +               dst += count;
> > +               count = len;
> > +       }
> > +
> > +       return nand_read_skip_bad(mtd, offset, &count, &actual,
> > +mtd->size, dst); }
> > +
> 
> We should rethink that way we have nand_spl_load_image and create a way to
> unify more SoC
> 
> > +/*
> > + * 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
> > + */
> > +u32 nand_spl_adjust_offset(u32 sector, u32 offs) {
> > +       u32 sector_align_offset, sector_align_end_offset;
> > +       struct mtd_info *mtd;
> > +
> > +       mtd = get_nand_dev_by_index(nand_curr_device);
> > +       if (!mtd)
> > +               hang();
> > +
> > +       sector_align_offset = sector & (~(mtd->erasesize - 1));
> > +
> > +       sector_align_end_offset = (sector + offs) & (~(mtd->erasesize
> > + - 1));
> > +
> > +       while (sector_align_offset <= sector_align_end_offset) {
> > +               if (nand_block_isbad(mtd, sector_align_offset)) {
> > +                       offs += mtd->erasesize;
> > +                       sector_align_end_offset += mtd->erasesize;
> > +               }
> > +               sector_align_offset += mtd->erasesize;
> > +       }
> > +
> > +       return offs;
> > +}
> > +
> 
> Here the same
> 
> Michael
> > +void nand_deselect(void) {}
> > --
> > 2.26.2
> >
> 
> 
> --
> 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

I agree with your point.
I will leverage the existing nand_spl_load_image to corporate with this new Cadence Nand driver.

Thanks
Dinesh