[7/7] mtd: nand: Add a common spl layer for nand subsystem

2268 diff mbox series

Avoid code duplication accross drivers but make them use
the same implementation. Create nand_common_spl to implement
the part that does not depend on nand chipset. Apply to

- mxs nand spl driver
- mt7621 spl driver

The mt7621 now as side effect implement nand_spl_adjust_offset, that
implements bad block handle for complex image like fitImage

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
 drivers/mtd/nand/raw/Makefile          |   4 +-
 drivers/mtd/nand/raw/mt7621_nand_spl.c | 188 +------------------
 drivers/mtd/nand/raw/mxs_nand_spl.c    | 176 +-----------------
 drivers/mtd/nand/raw/nand_common_spl.c | 245 +++++++++++++++++++++++++
 drivers/mtd/nand/raw/nand_common_spl.h |  15 ++
 5 files changed, 269 insertions(+), 359 deletions(-)
 create mode 100644 drivers/mtd/nand/raw/nand_common_spl.c
 create mode 100644 drivers/mtd/nand/raw/nand_common_spl.h

Hi Michael,

On Wed, Jul 27, 2022 at 11:38 AM Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Avoid code duplication accross drivers but make them use
> the same implementation. Create nand_common_spl to implement
> the part that does not depend on nand chipset. Apply to
>
> - mxs nand spl driver
> - mt7621 spl driver
>
> The mt7621 now as side effect implement nand_spl_adjust_offset, that
> implements bad block handle for complex image like fitImage
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/mtd/nand/raw/Makefile          |   4 +-
>  drivers/mtd/nand/raw/mt7621_nand_spl.c | 188 +------------------
>  drivers/mtd/nand/raw/mxs_nand_spl.c    | 176 +-----------------
>  drivers/mtd/nand/raw/nand_common_spl.c | 245 +++++++++++++++++++++++++
>  drivers/mtd/nand/raw/nand_common_spl.h |  15 ++
>  5 files changed, 269 insertions(+), 359 deletions(-)
>  create mode 100644 drivers/mtd/nand/raw/nand_common_spl.c
>  create mode 100644 drivers/mtd/nand/raw/nand_common_spl.h
>
> diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
> index a398aa9d88..82ddb2b5d8 100644
> --- a/drivers/mtd/nand/raw/Makefile
> +++ b/drivers/mtd/nand/raw/Makefile
> @@ -87,8 +87,8 @@ else  # minimal SPL drivers
>  obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o
>  obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_spl.o
>  obj-$(CONFIG_NAND_MXC) += mxc_nand_spl.o
> -obj-$(CONFIG_NAND_MXS) += mxs_nand_spl.o mxs_nand.o
> +obj-$(CONFIG_NAND_MXS) += mxs_nand_spl.o mxs_nand.o nand_common_spl.o
>  obj-$(CONFIG_NAND_SUNXI) += sunxi_nand_spl.o
> -obj-$(CONFIG_NAND_MT7621) += mt7621_nand_spl.o mt7621_nand.o
> +obj-$(CONFIG_NAND_MT7621) += mt7621_nand_spl.o mt7621_nand.o nand_common_spl.o
>
>  endif # drivers
> diff --git a/drivers/mtd/nand/raw/mt7621_nand_spl.c b/drivers/mtd/nand/raw/mt7621_nand_spl.c
> index 114fc8b7ce..254e14a553 100644
> --- a/drivers/mtd/nand/raw/mt7621_nand_spl.c
> +++ b/drivers/mtd/nand/raw/mt7621_nand_spl.c
> @@ -10,187 +10,13 @@
>  #include <linux/sizes.h>
>  #include <linux/delay.h>
>  #include <linux/mtd/rawnand.h>
> +#include "nand_common_spl.h"
>  #include "mt7621_nand.h"
> +#include "nand_common_spl.h"

You already included "nand_common_spl.h"

>
>  static struct mt7621_nfc nfc_dev;
> -static u8 *buffer;
>  static int nand_valid;
>
> -static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> -                           int column, int page_addr)
> -{
> -       register struct nand_chip *chip = mtd_to_nand(mtd);
> -
> -       /* Command latch cycle */
> -       chip->cmd_ctrl(mtd, command, NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> -
> -       if (column != -1 || page_addr != -1) {
> -               int ctrl = NAND_CTRL_CHANGE | NAND_NCE | NAND_ALE;
> -
> -               /* Serially input address */
> -               if (column != -1) {
> -                       chip->cmd_ctrl(mtd, column, ctrl);
> -                       ctrl &= ~NAND_CTRL_CHANGE;
> -                       if (command != NAND_CMD_READID)
> -                               chip->cmd_ctrl(mtd, column >> 8, ctrl);
> -               }
> -               if (page_addr != -1) {
> -                       chip->cmd_ctrl(mtd, page_addr, ctrl);
> -                       chip->cmd_ctrl(mtd, page_addr >> 8,
> -                                      NAND_NCE | NAND_ALE);
> -                       if (chip->options & NAND_ROW_ADDR_3)
> -                               chip->cmd_ctrl(mtd, page_addr >> 16,
> -                                              NAND_NCE | NAND_ALE);
> -               }
> -       }
> -       chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
> -
> -       /*
> -        * Program and erase have their own busy handlers status, sequential
> -        * in and status need no delay.
> -        */
> -       switch (command) {
> -       case NAND_CMD_STATUS:
> -       case NAND_CMD_READID:
> -       case NAND_CMD_SET_FEATURES:
> -               return;
> -
> -       case NAND_CMD_READ0:
> -               chip->cmd_ctrl(mtd, NAND_CMD_READSTART,
> -                              NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> -               chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> -                              NAND_NCE | NAND_CTRL_CHANGE);
> -       }
> -
> -       /*
> -        * Apply this short delay always to ensure that we do wait tWB in
> -        * any case on any machine.
> -        */
> -       ndelay(100);
> -
> -       nand_wait_ready(mtd);
> -}
> -
> -static int nfc_read_page_hwecc(struct mtd_info *mtd, void *buf,
> -                              unsigned int page)
> -{
> -       struct nand_chip *chip = mtd_to_nand(mtd);
> -       int ret;
> -
> -       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> -
> -       ret = chip->ecc.read_page(mtd, chip, buf, 1, page);
> -       if (ret < 0 || ret > chip->ecc.strength)
> -               return -1;
> -
> -       return 0;
> -}
> -
> -static int nfc_read_oob_hwecc(struct mtd_info *mtd, void *buf, u32 len,
> -                             unsigned int page)
> -{
> -       struct nand_chip *chip = mtd_to_nand(mtd);
> -       int ret;
> -
> -       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> -
> -       ret = chip->ecc.read_page(mtd, chip, NULL, 1, page);
> -       if (ret < 0)
> -               return -1;
> -
> -       if (len > mtd->oobsize)
> -               len = mtd->oobsize;
> -
> -       memcpy(buf, chip->oob_poi, len);
> -
> -       return 0;
> -}
> -
> -static int nfc_check_bad_block(struct mtd_info *mtd, unsigned int page)
> -{
> -       struct nand_chip *chip = mtd_to_nand(mtd);
> -       u32 pages_per_block, i = 0;
> -       int ret;
> -       u8 bad;
> -
> -       pages_per_block = 1 << (mtd->erasesize_shift - mtd->writesize_shift);
> -
> -       /* Read from first/last page(s) if necessary */
> -       if (chip->bbt_options & NAND_BBT_SCANLASTPAGE) {
> -               page += pages_per_block - 1;
> -               if (chip->bbt_options & NAND_BBT_SCAN2NDPAGE)
> -                       page--;
> -       }
> -
> -       do {
> -               ret = nfc_read_oob_hwecc(mtd, &bad, 1, page);
> -               if (ret)
> -                       return ret;
> -
> -               ret = bad != 0xFF;
> -
> -               i++;
> -               page++;
> -       } while (!ret && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
> -
> -       return ret;
> -}
> -
> -int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
> -{
> -       struct mt7621_nfc *nfc = &nfc_dev;
> -       struct nand_chip *chip = &nfc->nand;
> -       struct mtd_info *mtd = &chip->mtd;
> -       u32 addr, col, page, chksz;
> -       bool check_bad = true;
> -
> -       if (!nand_valid)
> -               return -ENODEV;
> -
> -       while (size) {
> -               if (check_bad || !(offs & mtd->erasesize_mask)) {
> -                       addr = offs & (~mtd->erasesize_mask);
> -                       page = addr >> mtd->writesize_shift;
> -                       if (nfc_check_bad_block(mtd, page)) {
> -                               /* Skip bad block */
> -                               if (addr >= mtd->size - mtd->erasesize)
> -                                       return -1;
> -
> -                               offs += mtd->erasesize;
> -                               continue;
> -                       }
> -
> -                       check_bad = false;
> -               }
> -
> -               col = offs & mtd->writesize_mask;
> -               page = offs >> mtd->writesize_shift;
> -               chksz = min(mtd->writesize - col, (uint32_t)size);
> -
> -               if (unlikely(chksz < mtd->writesize)) {
> -                       /* Not reading a full page */
> -                       if (nfc_read_page_hwecc(mtd, buffer, page))
> -                               return -1;
> -
> -                       memcpy(dest, buffer + col, chksz);
> -               } else {
> -                       if (nfc_read_page_hwecc(mtd, dest, page))
> -                               return -1;
> -               }
> -
> -               dest += chksz;
> -               offs += chksz;
> -               size -= chksz;
> -       }
> -
> -       return 0;
> -}
> -
> -int nand_default_bbt(struct mtd_info *mtd)
> -{
> -       return 0;
> -}
> -
>  unsigned long nand_size(void)
>  {
>         if (!nand_valid)
> @@ -203,10 +29,6 @@ unsigned long nand_size(void)
>         return SZ_2G;
>  }
>
> -void nand_deselect(void)
> -{
> -}
> -
>  void nand_init(void)
>  {
>         struct mtd_info *mtd;
> @@ -219,7 +41,7 @@ void nand_init(void)
>
>         chip = &nfc_dev.nand;
>         mtd = &chip->mtd;
> -       chip->cmdfunc = nand_command_lp;
> +       chip->cmdfunc = nand_spl_command_lp;
>
>         if (mt7621_nfc_spl_post_init(&nfc_dev))
>                 return;
> @@ -229,9 +51,7 @@ void nand_init(void)
>         mtd->erasesize_mask = (1 << mtd->erasesize_shift) - 1;
>         mtd->writesize_mask = (1 << mtd->writesize_shift) - 1;
>
> -       buffer = malloc(mtd->writesize);
> -       if (!buffer)
> -               return;
> +       nand_spl_init(chip);
>
>         nand_valid = 1;
>  }
> diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
> index c7ea09e2f9..5cff6020c4 100644
> --- a/drivers/mtd/nand/raw/mxs_nand_spl.c
> +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
> @@ -14,66 +14,11 @@
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/mtd/rawnand.h>
> +#include "nand_common_spl.h"
>
>  static struct mtd_info *mtd;
>  static struct nand_chip nand_chip;
>
> -static void mxs_nand_command(struct mtd_info *mtd, unsigned int command,
> -                            int column, int page_addr)
> -{
> -       register struct nand_chip *chip = mtd_to_nand(mtd);
> -       u32 timeo, time_start;
> -
> -       /* write out the command to the device */
> -       chip->cmd_ctrl(mtd, command, NAND_CLE);
> -
> -       /* Serially input address */
> -       if (column != -1) {
> -               /* Adjust columns for 16 bit buswidth */
> -               if (chip->options & NAND_BUSWIDTH_16 &&
> -                               !nand_opcode_8bits(command))
> -                       column >>= 1;
> -               chip->cmd_ctrl(mtd, column, NAND_ALE);
> -
> -               /*
> -                * Assume LP NAND here, so use two bytes column address
> -                * but not for CMD_READID and CMD_PARAM, which require
> -                * only one byte column address
> -                */
> -               if (command != NAND_CMD_READID &&
> -                       command != NAND_CMD_PARAM)
> -                       chip->cmd_ctrl(mtd, column >> 8, NAND_ALE);
> -       }
> -       if (page_addr != -1) {
> -               chip->cmd_ctrl(mtd, page_addr, NAND_ALE);
> -               chip->cmd_ctrl(mtd, page_addr >> 8, NAND_ALE);
> -               /* One more address cycle for devices > 128MiB */
> -               if (chip->chipsize > (128 << 20))
> -                       chip->cmd_ctrl(mtd, page_addr >> 16, NAND_ALE);
> -       }
> -       chip->cmd_ctrl(mtd, NAND_CMD_NONE, 0);
> -
> -       if (command == NAND_CMD_READ0) {
> -               chip->cmd_ctrl(mtd, NAND_CMD_READSTART, NAND_CLE);
> -               chip->cmd_ctrl(mtd, NAND_CMD_NONE, 0);
> -       } else if (command == NAND_CMD_RNDOUT) {
> -               /* No ready / busy check necessary */
> -               chip->cmd_ctrl(mtd, NAND_CMD_RNDOUTSTART,
> -                              NAND_NCE | NAND_CLE);
> -               chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> -                              NAND_NCE);
> -       }
> -
> -       /* wait for nand ready */
> -       ndelay(100);
> -       timeo = (CONFIG_SYS_HZ * 20) / 1000;
> -       time_start = get_timer(0);
> -       while (get_timer(time_start) < timeo) {
> -               if (chip->dev_ready(mtd))
> -                       break;
> -       }
> -}
> -
>  #if defined (CONFIG_SPL_NAND_IDENT)
>
>  /* Trying to detect the NAND flash using ONFi, JEDEC, and (extended) IDs */
> @@ -175,35 +120,6 @@ static int mxs_flash_ident(struct mtd_info *mtd)
>         return ret;
>  }
>
> -static int mxs_read_page_ecc(struct mtd_info *mtd, void *buf, unsigned int page)
> -{
> -       register struct nand_chip *chip = mtd_to_nand(mtd);
> -       int ret;
> -
> -       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> -       ret = nand_chip.ecc.read_page(mtd, chip, buf, 1, page);
> -       if (ret < 0) {
> -               printf("read_page failed %d\n", ret);
> -               return -1;
> -       }
> -       return 0;
> -}
> -
> -static int is_badblock(struct mtd_info *mtd, loff_t offs, int allowbbt)
> -{
> -       register struct nand_chip *chip = mtd_to_nand(mtd);
> -       unsigned int block = offs >> chip->phys_erase_shift;
> -       unsigned int page = offs >> chip->page_shift;
> -
> -       debug("%s offs=0x%08x block:%d page:%d\n", __func__, (int)offs, block,
> -             page);
> -       chip->cmdfunc(mtd, NAND_CMD_READ0, mtd->writesize, page);
> -       memset(chip->oob_poi, 0, mtd->oobsize);
> -       chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
> -
> -       return chip->oob_poi[0] != 0xff;
> -}
> -
>  /* setup mtd and nand structs and init mxs_nand driver */
>  void nand_init(void)
>  {
> @@ -215,7 +131,7 @@ void nand_init(void)
>         mxs_nand_init_spl(&nand_chip);
>         mtd = nand_to_mtd(&nand_chip);
>         /* set mtd functions */
> -       nand_chip.cmdfunc = mxs_nand_command;
> +       nand_chip.cmdfunc = nand_spl_command_lp;
>         nand_chip.scan_bbt = nand_default_bbt;
>         nand_chip.numchips = 1;
>
> @@ -234,92 +150,6 @@ void nand_init(void)
>         mtd->size = nand_chip.chipsize;
>         nand_chip.scan_bbt(mtd);
>         mxs_nand_setup_ecc(mtd);
> -}
> -
> -int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> -{
> -       unsigned int sz;
> -       unsigned int block, lastblock;
> -       unsigned int page, page_offset;
> -       unsigned int nand_page_per_block;
> -       struct nand_chip *chip;
> -       u8 *page_buf = NULL;
> -
> -       chip = mtd_to_nand(mtd);
> -       if (!chip->numchips)
> -               return -ENODEV;
> -
> -       page_buf = malloc(mtd->writesize);
> -       if (!page_buf)
> -               return -ENOMEM;
> -
> -       /* offs has to be aligned to a page address! */
> -       block = offs / mtd->erasesize;
> -       lastblock = (offs + size - 1) / mtd->erasesize;
> -       page = (offs % mtd->erasesize) / mtd->writesize;
> -       page_offset = offs % mtd->writesize;
> -       nand_page_per_block = mtd->erasesize / mtd->writesize;
> -
> -       while (block <= lastblock) {
> -               if (!is_badblock(mtd, mtd->erasesize * block, 1)) {
> -                       /* Skip bad blocks */
> -                       while (page < nand_page_per_block && size > 0) {
> -                               int curr_page = nand_page_per_block * block + page;
> -
> -                               if (mxs_read_page_ecc(mtd, page_buf, curr_page) < 0) {
> -                                       free(page_buf);
> -                                       return -EIO;
> -                               }
> -
> -                               if (size > (mtd->writesize - page_offset))
> -                                       sz = (mtd->writesize - page_offset);
> -                               else
> -                                       sz = size;
> -
> -                               memcpy(dst, page_buf + page_offset, sz);
> -                               dst += sz;
> -                               size -= sz;
> -                               page_offset = 0;
> -                               page++;
> -                       }
> -
> -                       page = 0;
> -               } else {
> -                       lastblock++;
> -               }
> -
> -               block++;
> -       }
> -
> -       free(page_buf);
> -
> -       return 0;
> -}
> -
> -int nand_default_bbt(struct mtd_info *mtd)
> -{
> -       return 0;
> -}
> -
> -void nand_deselect(void)
> -{
> -}
> -
> -u32 nand_spl_adjust_offset(u32 sector, u32 offs)
> -{
> -       unsigned int block, lastblock;
> -
> -       block = sector / mtd->erasesize;
> -       lastblock = (sector + offs) / mtd->erasesize;
> -
> -       while (block <= lastblock) {
> -               if (is_badblock(mtd, block * mtd->erasesize, 1)) {
> -                       offs += mtd->erasesize;
> -                       lastblock++;
> -               }
> -
> -               block++;
> -       }
>
> -       return offs;
> +       nand_spl_init(&nand_chip);
>  }
> diff --git a/drivers/mtd/nand/raw/nand_common_spl.c b/drivers/mtd/nand/raw/nand_common_spl.c
> new file mode 100644
> index 0000000000..0595fcbc26
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/nand_common_spl.c
> @@ -0,0 +1,245 @@
> +// SPDX-License-Identifier: GPL-2.0
I> +/*
> + * Copyright (C) 2022 Amarula Solutions B.V. All rights reserved.
> + *
> + * Author: Michael Trimarchi <michael@amarulasolutions.com>
> + * Author: Weijie Gao <weijie.gao@mediatek.com>
> + */
> +
> +#include <image.h>
> +#include <malloc.h>
> +#include <time.h>
> +#include <linux/sizes.h>
> +#include <linux/delay.h>
> +#include <linux/mtd/rawnand.h>
> +
> +static struct nand_chip *nand;
> +static u8 *buffer;
> +static int nand_valid;
> +

I don't like these static variables. I've looked around the code and I
think we could think about making
some of the improvements that Sean was thinking about. For example
nand_spl_adjust_offset ()
contained in nand_spl_loaders.c and nand_common_spl.c are almost the
same. In this case,
Sitara would also be useful for refactoring and testing it.

> +int nand_spl_init(struct nand_chip *chip)
> +{
> +       struct mtd_info *mtd = nand_to_mtd(chip);
> +
> +       if (!mtd)
> +               return -EINVAL;
> +
> +       buffer = malloc(mtd->writesize);
> +       if (!buffer)
> +               return -ENOMEM;
> +
> +       nand = chip;
> +       nand_valid = 1;
> +
> +       return 0;
> +}
> +
> +static struct nand_chip *nand_spl_get_chip(void)
> +{
> +       return nand;
> +}
> +
> +void nand_spl_command_lp(struct mtd_info *mtd, unsigned int command,
> +                        int column, int page_addr)
> +{
> +       register struct nand_chip *chip = mtd_to_nand(mtd);
> +       u32 timeo, time_start;
> +
> +       /* Command latch cycle */
> +       chip->cmd_ctrl(mtd, command, NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> +
> +       if (column != -1 || page_addr != -1) {
> +               int ctrl = NAND_CTRL_CHANGE | NAND_NCE | NAND_ALE;
> +
> +               /* Serially input address */
> +               if (column != -1) {
> +                       chip->cmd_ctrl(mtd, column, ctrl);
> +                       ctrl &= ~NAND_CTRL_CHANGE;
> +                       if (command != NAND_CMD_READID)
> +                               chip->cmd_ctrl(mtd, column >> 8, ctrl);
> +               }
> +               if (page_addr != -1) {
> +                       chip->cmd_ctrl(mtd, page_addr, ctrl);
> +                       chip->cmd_ctrl(mtd, page_addr >> 8,
> +                                      NAND_NCE | NAND_ALE);
> +                       if (chip->options & NAND_ROW_ADDR_3)
> +                               chip->cmd_ctrl(mtd, page_addr >> 16,
> +                                              NAND_NCE | NAND_ALE);
> +               }
> +       }
> +       chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
> +
> +       /*
> +        * Program and erase have their own busy handlers status, sequential
> +        * in and status need no delay.
> +        */
> +       switch (command) {
> +       case NAND_CMD_STATUS:
> +       case NAND_CMD_READID:
> +       case NAND_CMD_SET_FEATURES:
> +               return;
> +
> +       case NAND_CMD_READ0:
> +               chip->cmd_ctrl(mtd, NAND_CMD_READSTART,
> +                              NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> +               chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> +                              NAND_NCE | NAND_CTRL_CHANGE);
> +       }
> +
> +       /*
> +        * Apply this short delay always to ensure that we do wait tWB in
> +        * any case on any machine.
> +        */
> +       ndelay(100);
> +
> +       timeo = (CONFIG_SYS_HZ * 20) / 1000;
> +       time_start = get_timer(0);
> +       while (get_timer(time_start) < timeo) {
> +               if (chip->dev_ready(mtd))
> +                       break;
> +       }
> +}
> +
> +static int nand_spl_read_page_hwecc(struct mtd_info *mtd, void *buf,
> +                              unsigned int page)
> +{
> +       struct nand_chip *chip = mtd_to_nand(mtd);
> +       int ret;
> +
> +       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> +
> +       ret = chip->ecc.read_page(mtd, chip, buf, 1, page);
> +       if (ret < 0 || ret > chip->ecc.strength)
> +               return -1;
> +
> +       return 0;
> +}
> +
> +static int nand_spl_read_oob_hwecc(struct mtd_info *mtd, void *buf, u32 len,
> +                             unsigned int page)
> +{
> +       struct nand_chip *chip = mtd_to_nand(mtd);
> +       int ret;
> +
> +       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> +
> +       ret = chip->ecc.read_page(mtd, chip, NULL, 1, page);
> +       if (ret < 0)
> +               return -1;
> +
> +       if (len > mtd->oobsize)
> +               len = mtd->oobsize;
> +
> +       memcpy(buf, chip->oob_poi, len);
> +
> +       return 0;
> +}
> +
> +static int nand_spl_check_bad_block(struct mtd_info *mtd, unsigned int page)
> +{
> +       struct nand_chip *chip = mtd_to_nand(mtd);
> +       u32 pages_per_block, i = 0;
> +       int ret;
> +       u8 bad;
> +
> +       pages_per_block = 1 << (mtd->erasesize_shift - mtd->writesize_shift);
> +
> +       /* Read from first/last page(s) if necessary */
> +       if (chip->bbt_options & NAND_BBT_SCANLASTPAGE) {
> +               page += pages_per_block - 1;
> +               if (chip->bbt_options & NAND_BBT_SCAN2NDPAGE)
> +                       page--;
> +       }
> +
> +       do {
> +               ret = nand_spl_read_oob_hwecc(mtd, &bad, 1, page);
> +               if (ret)
> +                       return ret;
> +
> +               ret = bad != 0xFF;
> +
> +               i++;
> +               page++;
> +       } while (!ret && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
> +
> +       return ret;
> +}
> +
> +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
> +{
> +       struct nand_chip *chip = nand_spl_get_chip();
> +       struct mtd_info *mtd = &chip->mtd;
> +       u32 addr, col, page, chksz;
> +       bool check_bad = true;
> +
> +       if (!nand_valid)
> +               return -ENODEV;
> +
> +       while (size) {
> +               if (check_bad || !(offs & mtd->erasesize_mask)) {
> +                       addr = offs & (~mtd->erasesize_mask);
> +                       page = addr >> mtd->writesize_shift;
> +                       if (nand_spl_check_bad_block(mtd, page)) {
> +                               /* Skip bad block */
> +                               if (addr >= mtd->size - mtd->erasesize)
> +                                       return -ENXIO;
> +
> +                               offs += mtd->erasesize;
> +                               continue;
> +                       }
> +
> +                       check_bad = false;
> +               }
> +
> +               col = offs & mtd->writesize_mask;
> +               page = offs >> mtd->writesize_shift;
> +               chksz = min(mtd->writesize - col, (uint32_t)size);
> +
> +               if (unlikely(chksz < mtd->writesize)) {
> +                       /* Not reading a full page */
> +                       if (nand_spl_read_page_hwecc(mtd, buffer, page))
> +                               return -EIO;
> +
> +                       memcpy(dest, buffer + col, chksz);
> +               } else {
> +                       if (nand_spl_read_page_hwecc(mtd, dest, page))
> +                               return -EIO;
> +               }
> +
> +               dest += chksz;
> +               offs += chksz;
> +               size -= chksz;
> +       }
> +
> +       return 0;
> +}
> +
> +int nand_default_bbt(struct mtd_info *mtd)
> +{
> +       return 0;
> +}
> +
> +void nand_deselect(void)
> +{
> +}
> +
> +u32 nand_spl_adjust_offset(u32 sector, u32 offs)
> +{
> +       struct nand_chip *chip = nand_spl_get_chip();
> +       struct mtd_info *mtd = nand_to_mtd(chip);
> +       unsigned int block, lastblock;
> +
> +       block = sector / mtd->erasesize;
> +       lastblock = (sector + offs) / mtd->erasesize;
> +
> +       while (block <= lastblock) {
> +               if (nand_spl_check_bad_block(mtd, block * mtd->erasesize)) {
> +                       offs += mtd->erasesize;
> +                       lastblock++;
> +               }
> +
> +               block++;
> +       }
> +
> +       return offs;
> +}
> diff --git a/drivers/mtd/nand/raw/nand_common_spl.h b/drivers/mtd/nand/raw/nand_common_spl.h
> new file mode 100644
> index 0000000000..36bf63b230
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/nand_common_spl.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 Amarula Solution B.V. All rights reserved.
> + *
> + * Author: Michael Trimarchi <michael@amarulasolutions.com>
> + */
> +
> +#ifndef _NAND_COMMON_SPL_H
> +#define _NAND_COMMON_SPL_H
> +
> +void nand_spl_command_lp(struct mtd_info *mtd, unsigned int command,
> +                        int column, int page_addr);
> +int nand_spl_init(struct nand_chip *chip);
> +
> +#endif /* _NAND_COMMON_SPL_H_ */
> --
> 2.34.1
>

It is difficult to do a safe review. Better than test it on bsh.
Dario

Hi Dario

On Wed, Jul 27, 2022 at 3:50 PM Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> Hi Michael,
>
> On Wed, Jul 27, 2022 at 11:38 AM Michael Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > Avoid code duplication accross drivers but make them use
> > the same implementation. Create nand_common_spl to implement
> > the part that does not depend on nand chipset. Apply to
> >
> > - mxs nand spl driver
> > - mt7621 spl driver
> >
> > The mt7621 now as side effect implement nand_spl_adjust_offset, that
> > implements bad block handle for complex image like fitImage
> >
> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > ---
> >  drivers/mtd/nand/raw/Makefile          |   4 +-
> >  drivers/mtd/nand/raw/mt7621_nand_spl.c | 188 +------------------
> >  drivers/mtd/nand/raw/mxs_nand_spl.c    | 176 +-----------------
> >  drivers/mtd/nand/raw/nand_common_spl.c | 245 +++++++++++++++++++++++++
> >  drivers/mtd/nand/raw/nand_common_spl.h |  15 ++
> >  5 files changed, 269 insertions(+), 359 deletions(-)
> >  create mode 100644 drivers/mtd/nand/raw/nand_common_spl.c
> >  create mode 100644 drivers/mtd/nand/raw/nand_common_spl.h
> >
> > diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
> > index a398aa9d88..82ddb2b5d8 100644
> > --- a/drivers/mtd/nand/raw/Makefile
> > +++ b/drivers/mtd/nand/raw/Makefile
> > @@ -87,8 +87,8 @@ else  # minimal SPL drivers
> >  obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o
> >  obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_spl.o
> >  obj-$(CONFIG_NAND_MXC) += mxc_nand_spl.o
> > -obj-$(CONFIG_NAND_MXS) += mxs_nand_spl.o mxs_nand.o
> > +obj-$(CONFIG_NAND_MXS) += mxs_nand_spl.o mxs_nand.o nand_common_spl.o
> >  obj-$(CONFIG_NAND_SUNXI) += sunxi_nand_spl.o
> > -obj-$(CONFIG_NAND_MT7621) += mt7621_nand_spl.o mt7621_nand.o
> > +obj-$(CONFIG_NAND_MT7621) += mt7621_nand_spl.o mt7621_nand.o nand_common_spl.o
> >
> >  endif # drivers
> > diff --git a/drivers/mtd/nand/raw/mt7621_nand_spl.c b/drivers/mtd/nand/raw/mt7621_nand_spl.c
> > index 114fc8b7ce..254e14a553 100644
> > --- a/drivers/mtd/nand/raw/mt7621_nand_spl.c
> > +++ b/drivers/mtd/nand/raw/mt7621_nand_spl.c
> > @@ -10,187 +10,13 @@
> >  #include <linux/sizes.h>
> >  #include <linux/delay.h>
> >  #include <linux/mtd/rawnand.h>
> > +#include "nand_common_spl.h"
> >  #include "mt7621_nand.h"
> > +#include "nand_common_spl.h"
>
> You already included "nand_common_spl.h"
>

Yes sorry

> >
> >  static struct mt7621_nfc nfc_dev;
> > -static u8 *buffer;
> >  static int nand_valid;
> >
> > -static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> > -                           int column, int page_addr)
> > -{
> > -       register struct nand_chip *chip = mtd_to_nand(mtd);
> > -
> > -       /* Command latch cycle */
> > -       chip->cmd_ctrl(mtd, command, NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> > -
> > -       if (column != -1 || page_addr != -1) {
> > -               int ctrl = NAND_CTRL_CHANGE | NAND_NCE | NAND_ALE;
> > -
> > -               /* Serially input address */
> > -               if (column != -1) {
> > -                       chip->cmd_ctrl(mtd, column, ctrl);
> > -                       ctrl &= ~NAND_CTRL_CHANGE;
> > -                       if (command != NAND_CMD_READID)
> > -                               chip->cmd_ctrl(mtd, column >> 8, ctrl);
> > -               }
> > -               if (page_addr != -1) {
> > -                       chip->cmd_ctrl(mtd, page_addr, ctrl);
> > -                       chip->cmd_ctrl(mtd, page_addr >> 8,
> > -                                      NAND_NCE | NAND_ALE);
> > -                       if (chip->options & NAND_ROW_ADDR_3)
> > -                               chip->cmd_ctrl(mtd, page_addr >> 16,
> > -                                              NAND_NCE | NAND_ALE);
> > -               }
> > -       }
> > -       chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
> > -
> > -       /*
> > -        * Program and erase have their own busy handlers status, sequential
> > -        * in and status need no delay.
> > -        */
> > -       switch (command) {
> > -       case NAND_CMD_STATUS:
> > -       case NAND_CMD_READID:
> > -       case NAND_CMD_SET_FEATURES:
> > -               return;
> > -
> > -       case NAND_CMD_READ0:
> > -               chip->cmd_ctrl(mtd, NAND_CMD_READSTART,
> > -                              NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> > -               chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> > -                              NAND_NCE | NAND_CTRL_CHANGE);
> > -       }
> > -
> > -       /*
> > -        * Apply this short delay always to ensure that we do wait tWB in
> > -        * any case on any machine.
> > -        */
> > -       ndelay(100);
> > -
> > -       nand_wait_ready(mtd);
> > -}
> > -
> > -static int nfc_read_page_hwecc(struct mtd_info *mtd, void *buf,
> > -                              unsigned int page)
> > -{
> > -       struct nand_chip *chip = mtd_to_nand(mtd);
> > -       int ret;
> > -
> > -       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> > -
> > -       ret = chip->ecc.read_page(mtd, chip, buf, 1, page);
> > -       if (ret < 0 || ret > chip->ecc.strength)
> > -               return -1;
> > -
> > -       return 0;
> > -}
> > -
> > -static int nfc_read_oob_hwecc(struct mtd_info *mtd, void *buf, u32 len,
> > -                             unsigned int page)
> > -{
> > -       struct nand_chip *chip = mtd_to_nand(mtd);
> > -       int ret;
> > -
> > -       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> > -
> > -       ret = chip->ecc.read_page(mtd, chip, NULL, 1, page);
> > -       if (ret < 0)
> > -               return -1;
> > -
> > -       if (len > mtd->oobsize)
> > -               len = mtd->oobsize;
> > -
> > -       memcpy(buf, chip->oob_poi, len);
> > -
> > -       return 0;
> > -}
> > -
> > -static int nfc_check_bad_block(struct mtd_info *mtd, unsigned int page)
> > -{
> > -       struct nand_chip *chip = mtd_to_nand(mtd);
> > -       u32 pages_per_block, i = 0;
> > -       int ret;
> > -       u8 bad;
> > -
> > -       pages_per_block = 1 << (mtd->erasesize_shift - mtd->writesize_shift);
> > -
> > -       /* Read from first/last page(s) if necessary */
> > -       if (chip->bbt_options & NAND_BBT_SCANLASTPAGE) {
> > -               page += pages_per_block - 1;
> > -               if (chip->bbt_options & NAND_BBT_SCAN2NDPAGE)
> > -                       page--;
> > -       }
> > -
> > -       do {
> > -               ret = nfc_read_oob_hwecc(mtd, &bad, 1, page);
> > -               if (ret)
> > -                       return ret;
> > -
> > -               ret = bad != 0xFF;
> > -
> > -               i++;
> > -               page++;
> > -       } while (!ret && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
> > -
> > -       return ret;
> > -}
> > -
> > -int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
> > -{
> > -       struct mt7621_nfc *nfc = &nfc_dev;
> > -       struct nand_chip *chip = &nfc->nand;
> > -       struct mtd_info *mtd = &chip->mtd;
> > -       u32 addr, col, page, chksz;
> > -       bool check_bad = true;
> > -
> > -       if (!nand_valid)
> > -               return -ENODEV;
> > -
> > -       while (size) {
> > -               if (check_bad || !(offs & mtd->erasesize_mask)) {
> > -                       addr = offs & (~mtd->erasesize_mask);
> > -                       page = addr >> mtd->writesize_shift;
> > -                       if (nfc_check_bad_block(mtd, page)) {
> > -                               /* Skip bad block */
> > -                               if (addr >= mtd->size - mtd->erasesize)
> > -                                       return -1;
> > -
> > -                               offs += mtd->erasesize;
> > -                               continue;
> > -                       }
> > -
> > -                       check_bad = false;
> > -               }
> > -
> > -               col = offs & mtd->writesize_mask;
> > -               page = offs >> mtd->writesize_shift;
> > -               chksz = min(mtd->writesize - col, (uint32_t)size);
> > -
> > -               if (unlikely(chksz < mtd->writesize)) {
> > -                       /* Not reading a full page */
> > -                       if (nfc_read_page_hwecc(mtd, buffer, page))
> > -                               return -1;
> > -
> > -                       memcpy(dest, buffer + col, chksz);
> > -               } else {
> > -                       if (nfc_read_page_hwecc(mtd, dest, page))
> > -                               return -1;
> > -               }
> > -
> > -               dest += chksz;
> > -               offs += chksz;
> > -               size -= chksz;
> > -       }
> > -
> > -       return 0;
> > -}
> > -
> > -int nand_default_bbt(struct mtd_info *mtd)
> > -{
> > -       return 0;
> > -}
> > -
> >  unsigned long nand_size(void)
> >  {
> >         if (!nand_valid)
> > @@ -203,10 +29,6 @@ unsigned long nand_size(void)
> >         return SZ_2G;
> >  }
> >
> > -void nand_deselect(void)
> > -{
> > -}
> > -
> >  void nand_init(void)
> >  {
> >         struct mtd_info *mtd;
> > @@ -219,7 +41,7 @@ void nand_init(void)
> >
> >         chip = &nfc_dev.nand;
> >         mtd = &chip->mtd;
> > -       chip->cmdfunc = nand_command_lp;
> > +       chip->cmdfunc = nand_spl_command_lp;
> >
> >         if (mt7621_nfc_spl_post_init(&nfc_dev))
> >                 return;
> > @@ -229,9 +51,7 @@ void nand_init(void)
> >         mtd->erasesize_mask = (1 << mtd->erasesize_shift) - 1;
> >         mtd->writesize_mask = (1 << mtd->writesize_shift) - 1;
> >
> > -       buffer = malloc(mtd->writesize);
> > -       if (!buffer)
> > -               return;
> > +       nand_spl_init(chip);
> >
> >         nand_valid = 1;
> >  }
> > diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
> > index c7ea09e2f9..5cff6020c4 100644
> > --- a/drivers/mtd/nand/raw/mxs_nand_spl.c
> > +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
> > @@ -14,66 +14,11 @@
> >  #include <linux/delay.h>
> >  #include <linux/err.h>
> >  #include <linux/mtd/rawnand.h>
> > +#include "nand_common_spl.h"
> >
> >  static struct mtd_info *mtd;
> >  static struct nand_chip nand_chip;
> >
> > -static void mxs_nand_command(struct mtd_info *mtd, unsigned int command,
> > -                            int column, int page_addr)
> > -{
> > -       register struct nand_chip *chip = mtd_to_nand(mtd);
> > -       u32 timeo, time_start;
> > -
> > -       /* write out the command to the device */
> > -       chip->cmd_ctrl(mtd, command, NAND_CLE);
> > -
> > -       /* Serially input address */
> > -       if (column != -1) {
> > -               /* Adjust columns for 16 bit buswidth */
> > -               if (chip->options & NAND_BUSWIDTH_16 &&
> > -                               !nand_opcode_8bits(command))
> > -                       column >>= 1;
> > -               chip->cmd_ctrl(mtd, column, NAND_ALE);
> > -
> > -               /*
> > -                * Assume LP NAND here, so use two bytes column address
> > -                * but not for CMD_READID and CMD_PARAM, which require
> > -                * only one byte column address
> > -                */
> > -               if (command != NAND_CMD_READID &&
> > -                       command != NAND_CMD_PARAM)
> > -                       chip->cmd_ctrl(mtd, column >> 8, NAND_ALE);
> > -       }
> > -       if (page_addr != -1) {
> > -               chip->cmd_ctrl(mtd, page_addr, NAND_ALE);
> > -               chip->cmd_ctrl(mtd, page_addr >> 8, NAND_ALE);
> > -               /* One more address cycle for devices > 128MiB */
> > -               if (chip->chipsize > (128 << 20))
> > -                       chip->cmd_ctrl(mtd, page_addr >> 16, NAND_ALE);
> > -       }
> > -       chip->cmd_ctrl(mtd, NAND_CMD_NONE, 0);
> > -
> > -       if (command == NAND_CMD_READ0) {
> > -               chip->cmd_ctrl(mtd, NAND_CMD_READSTART, NAND_CLE);
> > -               chip->cmd_ctrl(mtd, NAND_CMD_NONE, 0);
> > -       } else if (command == NAND_CMD_RNDOUT) {
> > -               /* No ready / busy check necessary */
> > -               chip->cmd_ctrl(mtd, NAND_CMD_RNDOUTSTART,
> > -                              NAND_NCE | NAND_CLE);
> > -               chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> > -                              NAND_NCE);
> > -       }
> > -
> > -       /* wait for nand ready */
> > -       ndelay(100);
> > -       timeo = (CONFIG_SYS_HZ * 20) / 1000;
> > -       time_start = get_timer(0);
> > -       while (get_timer(time_start) < timeo) {
> > -               if (chip->dev_ready(mtd))
> > -                       break;
> > -       }
> > -}
> > -
> >  #if defined (CONFIG_SPL_NAND_IDENT)
> >
> >  /* Trying to detect the NAND flash using ONFi, JEDEC, and (extended) IDs */
> > @@ -175,35 +120,6 @@ static int mxs_flash_ident(struct mtd_info *mtd)
> >         return ret;
> >  }
> >
> > -static int mxs_read_page_ecc(struct mtd_info *mtd, void *buf, unsigned int page)
> > -{
> > -       register struct nand_chip *chip = mtd_to_nand(mtd);
> > -       int ret;
> > -
> > -       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> > -       ret = nand_chip.ecc.read_page(mtd, chip, buf, 1, page);
> > -       if (ret < 0) {
> > -               printf("read_page failed %d\n", ret);
> > -               return -1;
> > -       }
> > -       return 0;
> > -}
> > -
> > -static int is_badblock(struct mtd_info *mtd, loff_t offs, int allowbbt)
> > -{
> > -       register struct nand_chip *chip = mtd_to_nand(mtd);
> > -       unsigned int block = offs >> chip->phys_erase_shift;
> > -       unsigned int page = offs >> chip->page_shift;
> > -
> > -       debug("%s offs=0x%08x block:%d page:%d\n", __func__, (int)offs, block,
> > -             page);
> > -       chip->cmdfunc(mtd, NAND_CMD_READ0, mtd->writesize, page);
> > -       memset(chip->oob_poi, 0, mtd->oobsize);
> > -       chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
> > -
> > -       return chip->oob_poi[0] != 0xff;
> > -}
> > -
> >  /* setup mtd and nand structs and init mxs_nand driver */
> >  void nand_init(void)
> >  {
> > @@ -215,7 +131,7 @@ void nand_init(void)
> >         mxs_nand_init_spl(&nand_chip);
> >         mtd = nand_to_mtd(&nand_chip);
> >         /* set mtd functions */
> > -       nand_chip.cmdfunc = mxs_nand_command;
> > +       nand_chip.cmdfunc = nand_spl_command_lp;
> >         nand_chip.scan_bbt = nand_default_bbt;
> >         nand_chip.numchips = 1;
> >
> > @@ -234,92 +150,6 @@ void nand_init(void)
> >         mtd->size = nand_chip.chipsize;
> >         nand_chip.scan_bbt(mtd);
> >         mxs_nand_setup_ecc(mtd);
> > -}
> > -
> > -int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> > -{
> > -       unsigned int sz;
> > -       unsigned int block, lastblock;
> > -       unsigned int page, page_offset;
> > -       unsigned int nand_page_per_block;
> > -       struct nand_chip *chip;
> > -       u8 *page_buf = NULL;
> > -
> > -       chip = mtd_to_nand(mtd);
> > -       if (!chip->numchips)
> > -               return -ENODEV;
> > -
> > -       page_buf = malloc(mtd->writesize);
> > -       if (!page_buf)
> > -               return -ENOMEM;
> > -
> > -       /* offs has to be aligned to a page address! */
> > -       block = offs / mtd->erasesize;
> > -       lastblock = (offs + size - 1) / mtd->erasesize;
> > -       page = (offs % mtd->erasesize) / mtd->writesize;
> > -       page_offset = offs % mtd->writesize;
> > -       nand_page_per_block = mtd->erasesize / mtd->writesize;
> > -
> > -       while (block <= lastblock) {
> > -               if (!is_badblock(mtd, mtd->erasesize * block, 1)) {
> > -                       /* Skip bad blocks */
> > -                       while (page < nand_page_per_block && size > 0) {
> > -                               int curr_page = nand_page_per_block * block + page;
> > -
> > -                               if (mxs_read_page_ecc(mtd, page_buf, curr_page) < 0) {
> > -                                       free(page_buf);
> > -                                       return -EIO;
> > -                               }
> > -
> > -                               if (size > (mtd->writesize - page_offset))
> > -                                       sz = (mtd->writesize - page_offset);
> > -                               else
> > -                                       sz = size;
> > -
> > -                               memcpy(dst, page_buf + page_offset, sz);
> > -                               dst += sz;
> > -                               size -= sz;
> > -                               page_offset = 0;
> > -                               page++;
> > -                       }
> > -
> > -                       page = 0;
> > -               } else {
> > -                       lastblock++;
> > -               }
> > -
> > -               block++;
> > -       }
> > -
> > -       free(page_buf);
> > -
> > -       return 0;
> > -}
> > -
> > -int nand_default_bbt(struct mtd_info *mtd)
> > -{
> > -       return 0;
> > -}
> > -
> > -void nand_deselect(void)
> > -{
> > -}
> > -
> > -u32 nand_spl_adjust_offset(u32 sector, u32 offs)
> > -{
> > -       unsigned int block, lastblock;
> > -
> > -       block = sector / mtd->erasesize;
> > -       lastblock = (sector + offs) / mtd->erasesize;
> > -
> > -       while (block <= lastblock) {
> > -               if (is_badblock(mtd, block * mtd->erasesize, 1)) {
> > -                       offs += mtd->erasesize;
> > -                       lastblock++;
> > -               }
> > -
> > -               block++;
> > -       }
> >
> > -       return offs;
> > +       nand_spl_init(&nand_chip);
> >  }
> > diff --git a/drivers/mtd/nand/raw/nand_common_spl.c b/drivers/mtd/nand/raw/nand_common_spl.c
> > new file mode 100644
> > index 0000000000..0595fcbc26
> > --- /dev/null
> > +++ b/drivers/mtd/nand/raw/nand_common_spl.c
> > @@ -0,0 +1,245 @@
> > +// SPDX-License-Identifier: GPL-2.0
> I> +/*
> > + * Copyright (C) 2022 Amarula Solutions B.V. All rights reserved.
> > + *
> > + * Author: Michael Trimarchi <michael@amarulasolutions.com>
> > + * Author: Weijie Gao <weijie.gao@mediatek.com>
> > + */
> > +
> > +#include <image.h>
> > +#include <malloc.h>
> > +#include <time.h>
> > +#include <linux/sizes.h>
> > +#include <linux/delay.h>
> > +#include <linux/mtd/rawnand.h>
> > +
> > +static struct nand_chip *nand;
> > +static u8 *buffer;
> > +static int nand_valid;
> > +
>
> I don't like these static variables. I've looked around the code and I
> think we could think about making

The reason of the variable is to reuse as much the code of the other
architecture
that was already tested.

> some of the improvements that Sean was thinking about. For example
> nand_spl_adjust_offset ()
> contained in nand_spl_loaders.c and nand_common_spl.c are almost the
> same. In this case,
> Sitara would also be useful for refactoring and testing it.

I would like to keep things in step and able to be tested. If you want
to merge them, I think
this is can be done in separated patch. You are welcome to merge them
all. I can not do proper testing
but I can prepare a new patch to merge them all

Michael

>
> > +int nand_spl_init(struct nand_chip *chip)
> > +{
> > +       struct mtd_info *mtd = nand_to_mtd(chip);
> > +
> > +       if (!mtd)
> > +               return -EINVAL;
> > +
> > +       buffer = malloc(mtd->writesize);
> > +       if (!buffer)
> > +               return -ENOMEM;
> > +
> > +       nand = chip;
> > +       nand_valid = 1;
> > +
> > +       return 0;
> > +}
> > +
> > +static struct nand_chip *nand_spl_get_chip(void)
> > +{
> > +       return nand;
> > +}
> > +
> > +void nand_spl_command_lp(struct mtd_info *mtd, unsigned int command,
> > +                        int column, int page_addr)
> > +{
> > +       register struct nand_chip *chip = mtd_to_nand(mtd);
> > +       u32 timeo, time_start;
> > +
> > +       /* Command latch cycle */
> > +       chip->cmd_ctrl(mtd, command, NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> > +
> > +       if (column != -1 || page_addr != -1) {
> > +               int ctrl = NAND_CTRL_CHANGE | NAND_NCE | NAND_ALE;
> > +
> > +               /* Serially input address */
> > +               if (column != -1) {
> > +                       chip->cmd_ctrl(mtd, column, ctrl);
> > +                       ctrl &= ~NAND_CTRL_CHANGE;
> > +                       if (command != NAND_CMD_READID)
> > +                               chip->cmd_ctrl(mtd, column >> 8, ctrl);
> > +               }
> > +               if (page_addr != -1) {
> > +                       chip->cmd_ctrl(mtd, page_addr, ctrl);
> > +                       chip->cmd_ctrl(mtd, page_addr >> 8,
> > +                                      NAND_NCE | NAND_ALE);
> > +                       if (chip->options & NAND_ROW_ADDR_3)
> > +                               chip->cmd_ctrl(mtd, page_addr >> 16,
> > +                                              NAND_NCE | NAND_ALE);
> > +               }
> > +       }
> > +       chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
> > +
> > +       /*
> > +        * Program and erase have their own busy handlers status, sequential
> > +        * in and status need no delay.
> > +        */
> > +       switch (command) {
> > +       case NAND_CMD_STATUS:
> > +       case NAND_CMD_READID:
> > +       case NAND_CMD_SET_FEATURES:
> > +               return;
> > +
> > +       case NAND_CMD_READ0:
> > +               chip->cmd_ctrl(mtd, NAND_CMD_READSTART,
> > +                              NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> > +               chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> > +                              NAND_NCE | NAND_CTRL_CHANGE);
> > +       }
> > +
> > +       /*
> > +        * Apply this short delay always to ensure that we do wait tWB in
> > +        * any case on any machine.
> > +        */
> > +       ndelay(100);
> > +
> > +       timeo = (CONFIG_SYS_HZ * 20) / 1000;
> > +       time_start = get_timer(0);
> > +       while (get_timer(time_start) < timeo) {
> > +               if (chip->dev_ready(mtd))
> > +                       break;
> > +       }
> > +}
> > +
> > +static int nand_spl_read_page_hwecc(struct mtd_info *mtd, void *buf,
> > +                              unsigned int page)
> > +{
> > +       struct nand_chip *chip = mtd_to_nand(mtd);
> > +       int ret;
> > +
> > +       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> > +
> > +       ret = chip->ecc.read_page(mtd, chip, buf, 1, page);
> > +       if (ret < 0 || ret > chip->ecc.strength)
> > +               return -1;
> > +
> > +       return 0;
> > +}
> > +
> > +static int nand_spl_read_oob_hwecc(struct mtd_info *mtd, void *buf, u32 len,
> > +                             unsigned int page)
> > +{
> > +       struct nand_chip *chip = mtd_to_nand(mtd);
> > +       int ret;
> > +
> > +       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> > +
> > +       ret = chip->ecc.read_page(mtd, chip, NULL, 1, page);
> > +       if (ret < 0)
> > +               return -1;
> > +
> > +       if (len > mtd->oobsize)
> > +               len = mtd->oobsize;
> > +
> > +       memcpy(buf, chip->oob_poi, len);
> > +
> > +       return 0;
> > +}
> > +
> > +static int nand_spl_check_bad_block(struct mtd_info *mtd, unsigned int page)
> > +{
> > +       struct nand_chip *chip = mtd_to_nand(mtd);
> > +       u32 pages_per_block, i = 0;
> > +       int ret;
> > +       u8 bad;
> > +
> > +       pages_per_block = 1 << (mtd->erasesize_shift - mtd->writesize_shift);
> > +
> > +       /* Read from first/last page(s) if necessary */
> > +       if (chip->bbt_options & NAND_BBT_SCANLASTPAGE) {
> > +               page += pages_per_block - 1;
> > +               if (chip->bbt_options & NAND_BBT_SCAN2NDPAGE)
> > +                       page--;
> > +       }
> > +
> > +       do {
> > +               ret = nand_spl_read_oob_hwecc(mtd, &bad, 1, page);
> > +               if (ret)
> > +                       return ret;
> > +
> > +               ret = bad != 0xFF;
> > +
> > +               i++;
> > +               page++;
> > +       } while (!ret && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
> > +
> > +       return ret;
> > +}
> > +
> > +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
> > +{
> > +       struct nand_chip *chip = nand_spl_get_chip();
> > +       struct mtd_info *mtd = &chip->mtd;
> > +       u32 addr, col, page, chksz;
> > +       bool check_bad = true;
> > +
> > +       if (!nand_valid)
> > +               return -ENODEV;
> > +
> > +       while (size) {
> > +               if (check_bad || !(offs & mtd->erasesize_mask)) {
> > +                       addr = offs & (~mtd->erasesize_mask);
> > +                       page = addr >> mtd->writesize_shift;
> > +                       if (nand_spl_check_bad_block(mtd, page)) {
> > +                               /* Skip bad block */
> > +                               if (addr >= mtd->size - mtd->erasesize)
> > +                                       return -ENXIO;
> > +
> > +                               offs += mtd->erasesize;
> > +                               continue;
> > +                       }
> > +
> > +                       check_bad = false;
> > +               }
> > +
> > +               col = offs & mtd->writesize_mask;
> > +               page = offs >> mtd->writesize_shift;
> > +               chksz = min(mtd->writesize - col, (uint32_t)size);
> > +
> > +               if (unlikely(chksz < mtd->writesize)) {
> > +                       /* Not reading a full page */
> > +                       if (nand_spl_read_page_hwecc(mtd, buffer, page))
> > +                               return -EIO;
> > +
> > +                       memcpy(dest, buffer + col, chksz);
> > +               } else {
> > +                       if (nand_spl_read_page_hwecc(mtd, dest, page))
> > +                               return -EIO;
> > +               }
> > +
> > +               dest += chksz;
> > +               offs += chksz;
> > +               size -= chksz;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +int nand_default_bbt(struct mtd_info *mtd)
> > +{
> > +       return 0;
> > +}
> > +
> > +void nand_deselect(void)
> > +{
> > +}
> > +
> > +u32 nand_spl_adjust_offset(u32 sector, u32 offs)
> > +{
> > +       struct nand_chip *chip = nand_spl_get_chip();
> > +       struct mtd_info *mtd = nand_to_mtd(chip);
> > +       unsigned int block, lastblock;
> > +
> > +       block = sector / mtd->erasesize;
> > +       lastblock = (sector + offs) / mtd->erasesize;
> > +
> > +       while (block <= lastblock) {
> > +               if (nand_spl_check_bad_block(mtd, block * mtd->erasesize)) {
> > +                       offs += mtd->erasesize;
> > +                       lastblock++;
> > +               }
> > +
> > +               block++;
> > +       }
> > +
> > +       return offs;
> > +}
> > diff --git a/drivers/mtd/nand/raw/nand_common_spl.h b/drivers/mtd/nand/raw/nand_common_spl.h
> > new file mode 100644
> > index 0000000000..36bf63b230
> > --- /dev/null
> > +++ b/drivers/mtd/nand/raw/nand_common_spl.h
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2022 Amarula Solution B.V. All rights reserved.
> > + *
> > + * Author: Michael Trimarchi <michael@amarulasolutions.com>
> > + */
> > +
> > +#ifndef _NAND_COMMON_SPL_H
> > +#define _NAND_COMMON_SPL_H
> > +
> > +void nand_spl_command_lp(struct mtd_info *mtd, unsigned int command,
> > +                        int column, int page_addr);
> > +int nand_spl_init(struct nand_chip *chip);
> > +
> > +#endif /* _NAND_COMMON_SPL_H_ */
> > --
> > 2.34.1
> >
>
> It is difficult to do a safe review. Better than test it on bsh.

Ok

Michael

> Dario
>
> --
>
> Dario Binacchi
>
> Embedded Linux Developer
>
> dario.binacchi@amarulasolutions.com
>
> __________________________________
>
>
> Amarula Solutions SRL
>
> Via Le Canevare 30, 31100 Treviso, Veneto, IT
>
> T. +39 042 243 5310
> info@amarulasolutions.com
>
> www.amarulasolutions.com

Hi Michael,

On Wed, Jul 27, 2022 at 11:38 AM Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Avoid code duplication accross drivers but make them use
> the same implementation. Create nand_common_spl to implement
> the part that does not depend on nand chipset. Apply to
>
> - mxs nand spl driver
> - mt7621 spl driver
>
> The mt7621 now as side effect implement nand_spl_adjust_offset, that
> implements bad block handle for complex image like fitImage
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/mtd/nand/raw/Makefile          |   4 +-
>  drivers/mtd/nand/raw/mt7621_nand_spl.c | 188 +------------------
>  drivers/mtd/nand/raw/mxs_nand_spl.c    | 176 +-----------------
>  drivers/mtd/nand/raw/nand_common_spl.c | 245 +++++++++++++++++++++++++
>  drivers/mtd/nand/raw/nand_common_spl.h |  15 ++
>  5 files changed, 269 insertions(+), 359 deletions(-)
>  create mode 100644 drivers/mtd/nand/raw/nand_common_spl.c
>  create mode 100644 drivers/mtd/nand/raw/nand_common_spl.h
>
> diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
> index a398aa9d88..82ddb2b5d8 100644
> --- a/drivers/mtd/nand/raw/Makefile
> +++ b/drivers/mtd/nand/raw/Makefile
> @@ -87,8 +87,8 @@ else  # minimal SPL drivers
>  obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o
>  obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_spl.o
>  obj-$(CONFIG_NAND_MXC) += mxc_nand_spl.o
> -obj-$(CONFIG_NAND_MXS) += mxs_nand_spl.o mxs_nand.o
> +obj-$(CONFIG_NAND_MXS) += mxs_nand_spl.o mxs_nand.o nand_common_spl.o
>  obj-$(CONFIG_NAND_SUNXI) += sunxi_nand_spl.o
> -obj-$(CONFIG_NAND_MT7621) += mt7621_nand_spl.o mt7621_nand.o
> +obj-$(CONFIG_NAND_MT7621) += mt7621_nand_spl.o mt7621_nand.o nand_common_spl.o
>
>  endif # drivers
> diff --git a/drivers/mtd/nand/raw/mt7621_nand_spl.c b/drivers/mtd/nand/raw/mt7621_nand_spl.c
> index 114fc8b7ce..254e14a553 100644
> --- a/drivers/mtd/nand/raw/mt7621_nand_spl.c
> +++ b/drivers/mtd/nand/raw/mt7621_nand_spl.c
> @@ -10,187 +10,13 @@
>  #include <linux/sizes.h>
>  #include <linux/delay.h>
>  #include <linux/mtd/rawnand.h>
> +#include "nand_common_spl.h"
>  #include "mt7621_nand.h"
> +#include "nand_common_spl.h"
>
>  static struct mt7621_nfc nfc_dev;
> -static u8 *buffer;
>  static int nand_valid;
>
> -static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> -                           int column, int page_addr)
> -{
> -       register struct nand_chip *chip = mtd_to_nand(mtd);
> -
> -       /* Command latch cycle */
> -       chip->cmd_ctrl(mtd, command, NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> -
> -       if (column != -1 || page_addr != -1) {
> -               int ctrl = NAND_CTRL_CHANGE | NAND_NCE | NAND_ALE;
> -
> -               /* Serially input address */
> -               if (column != -1) {
> -                       chip->cmd_ctrl(mtd, column, ctrl);
> -                       ctrl &= ~NAND_CTRL_CHANGE;
> -                       if (command != NAND_CMD_READID)
> -                               chip->cmd_ctrl(mtd, column >> 8, ctrl);
> -               }
> -               if (page_addr != -1) {
> -                       chip->cmd_ctrl(mtd, page_addr, ctrl);
> -                       chip->cmd_ctrl(mtd, page_addr >> 8,
> -                                      NAND_NCE | NAND_ALE);
> -                       if (chip->options & NAND_ROW_ADDR_3)
> -                               chip->cmd_ctrl(mtd, page_addr >> 16,
> -                                              NAND_NCE | NAND_ALE);
> -               }
> -       }
> -       chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
> -
> -       /*
> -        * Program and erase have their own busy handlers status, sequential
> -        * in and status need no delay.
> -        */
> -       switch (command) {
> -       case NAND_CMD_STATUS:
> -       case NAND_CMD_READID:
> -       case NAND_CMD_SET_FEATURES:
> -               return;
> -
> -       case NAND_CMD_READ0:
> -               chip->cmd_ctrl(mtd, NAND_CMD_READSTART,
> -                              NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> -               chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> -                              NAND_NCE | NAND_CTRL_CHANGE);
> -       }
> -
> -       /*
> -        * Apply this short delay always to ensure that we do wait tWB in
> -        * any case on any machine.
> -        */
> -       ndelay(100);
> -
> -       nand_wait_ready(mtd);
> -}
> -
> -static int nfc_read_page_hwecc(struct mtd_info *mtd, void *buf,
> -                              unsigned int page)

Alignment should match open parenthesis

> -{
> -       struct nand_chip *chip = mtd_to_nand(mtd);
> -       int ret;
> -
> -       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> -
> -       ret = chip->ecc.read_page(mtd, chip, buf, 1, page);
> -       if (ret < 0 || ret > chip->ecc.strength)
> -               return -1;
> -
> -       return 0;
> -}
> -
> -static int nfc_read_oob_hwecc(struct mtd_info *mtd, void *buf, u32 len,
> -                             unsigned int page)

Alignment should match open parenthesis

> -{
> -       struct nand_chip *chip = mtd_to_nand(mtd);
> -       int ret;
> -
> -       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> -
> -       ret = chip->ecc.read_page(mtd, chip, NULL, 1, page);
> -       if (ret < 0)
> -               return -1;
> -
> -       if (len > mtd->oobsize)
> -               len = mtd->oobsize;
> -
> -       memcpy(buf, chip->oob_poi, len);
> -
> -       return 0;
> -}
> -
> -static int nfc_check_bad_block(struct mtd_info *mtd, unsigned int page)
> -{
> -       struct nand_chip *chip = mtd_to_nand(mtd);
> -       u32 pages_per_block, i = 0;
> -       int ret;
> -       u8 bad;
> -
> -       pages_per_block = 1 << (mtd->erasesize_shift - mtd->writesize_shift);
> -
> -       /* Read from first/last page(s) if necessary */
> -       if (chip->bbt_options & NAND_BBT_SCANLASTPAGE) {
> -               page += pages_per_block - 1;
> -               if (chip->bbt_options & NAND_BBT_SCAN2NDPAGE)
> -                       page--;
> -       }
> -
> -       do {
> -               ret = nfc_read_oob_hwecc(mtd, &bad, 1, page);
> -               if (ret)
> -                       return ret;
> -
> -               ret = bad != 0xFF;
> -
> -               i++;
> -               page++;
> -       } while (!ret && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
> -
> -       return ret;
> -}
> -
> -int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)

Prefer kernel type 'u32' over 'uint32_t'
These was the warnings raised by patman

Dario

> -{
> -       struct mt7621_nfc *nfc = &nfc_dev;
> -       struct nand_chip *chip = &nfc->nand;
> -       struct mtd_info *mtd = &chip->mtd;
> -       u32 addr, col, page, chksz;
> -       bool check_bad = true;
> -
> -       if (!nand_valid)
> -               return -ENODEV;
> -
> -       while (size) {
> -               if (check_bad || !(offs & mtd->erasesize_mask)) {
> -                       addr = offs & (~mtd->erasesize_mask);
> -                       page = addr >> mtd->writesize_shift;
> -                       if (nfc_check_bad_block(mtd, page)) {
> -                               /* Skip bad block */
> -                               if (addr >= mtd->size - mtd->erasesize)
> -                                       return -1;
> -
> -                               offs += mtd->erasesize;
> -                               continue;
> -                       }
> -
> -                       check_bad = false;
> -               }
> -
> -               col = offs & mtd->writesize_mask;
> -               page = offs >> mtd->writesize_shift;
> -               chksz = min(mtd->writesize - col, (uint32_t)size);
> -
> -               if (unlikely(chksz < mtd->writesize)) {
> -                       /* Not reading a full page */
> -                       if (nfc_read_page_hwecc(mtd, buffer, page))
> -                               return -1;
> -
> -                       memcpy(dest, buffer + col, chksz);
> -               } else {
> -                       if (nfc_read_page_hwecc(mtd, dest, page))
> -                               return -1;
> -               }
> -
> -               dest += chksz;
> -               offs += chksz;
> -               size -= chksz;
> -       }
> -
> -       return 0;
> -}
> -
> -int nand_default_bbt(struct mtd_info *mtd)
> -{
> -       return 0;
> -}
> -
>  unsigned long nand_size(void)
>  {
>         if (!nand_valid)
> @@ -203,10 +29,6 @@ unsigned long nand_size(void)
>         return SZ_2G;
>  }
>
> -void nand_deselect(void)
> -{
> -}
> -
>  void nand_init(void)
>  {
>         struct mtd_info *mtd;
> @@ -219,7 +41,7 @@ void nand_init(void)
>
>         chip = &nfc_dev.nand;
>         mtd = &chip->mtd;
> -       chip->cmdfunc = nand_command_lp;
> +       chip->cmdfunc = nand_spl_command_lp;
>
>         if (mt7621_nfc_spl_post_init(&nfc_dev))
>                 return;
> @@ -229,9 +51,7 @@ void nand_init(void)
>         mtd->erasesize_mask = (1 << mtd->erasesize_shift) - 1;
>         mtd->writesize_mask = (1 << mtd->writesize_shift) - 1;
>
> -       buffer = malloc(mtd->writesize);
> -       if (!buffer)
> -               return;
> +       nand_spl_init(chip);
>
>         nand_valid = 1;
>  }
> diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
> index c7ea09e2f9..5cff6020c4 100644
> --- a/drivers/mtd/nand/raw/mxs_nand_spl.c
> +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
> @@ -14,66 +14,11 @@
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/mtd/rawnand.h>
> +#include "nand_common_spl.h"
>
>  static struct mtd_info *mtd;
>  static struct nand_chip nand_chip;
>
> -static void mxs_nand_command(struct mtd_info *mtd, unsigned int command,
> -                            int column, int page_addr)
> -{
> -       register struct nand_chip *chip = mtd_to_nand(mtd);
> -       u32 timeo, time_start;
> -
> -       /* write out the command to the device */
> -       chip->cmd_ctrl(mtd, command, NAND_CLE);
> -
> -       /* Serially input address */
> -       if (column != -1) {
> -               /* Adjust columns for 16 bit buswidth */
> -               if (chip->options & NAND_BUSWIDTH_16 &&
> -                               !nand_opcode_8bits(command))
> -                       column >>= 1;
> -               chip->cmd_ctrl(mtd, column, NAND_ALE);
> -
> -               /*
> -                * Assume LP NAND here, so use two bytes column address
> -                * but not for CMD_READID and CMD_PARAM, which require
> -                * only one byte column address
> -                */
> -               if (command != NAND_CMD_READID &&
> -                       command != NAND_CMD_PARAM)
> -                       chip->cmd_ctrl(mtd, column >> 8, NAND_ALE);
> -       }
> -       if (page_addr != -1) {
> -               chip->cmd_ctrl(mtd, page_addr, NAND_ALE);
> -               chip->cmd_ctrl(mtd, page_addr >> 8, NAND_ALE);
> -               /* One more address cycle for devices > 128MiB */
> -               if (chip->chipsize > (128 << 20))
> -                       chip->cmd_ctrl(mtd, page_addr >> 16, NAND_ALE);
> -       }
> -       chip->cmd_ctrl(mtd, NAND_CMD_NONE, 0);
> -
> -       if (command == NAND_CMD_READ0) {
> -               chip->cmd_ctrl(mtd, NAND_CMD_READSTART, NAND_CLE);
> -               chip->cmd_ctrl(mtd, NAND_CMD_NONE, 0);
> -       } else if (command == NAND_CMD_RNDOUT) {
> -               /* No ready / busy check necessary */
> -               chip->cmd_ctrl(mtd, NAND_CMD_RNDOUTSTART,
> -                              NAND_NCE | NAND_CLE);
> -               chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> -                              NAND_NCE);
> -       }
> -
> -       /* wait for nand ready */
> -       ndelay(100);
> -       timeo = (CONFIG_SYS_HZ * 20) / 1000;
> -       time_start = get_timer(0);
> -       while (get_timer(time_start) < timeo) {
> -               if (chip->dev_ready(mtd))
> -                       break;
> -       }
> -}
> -
>  #if defined (CONFIG_SPL_NAND_IDENT)
>
>  /* Trying to detect the NAND flash using ONFi, JEDEC, and (extended) IDs */
> @@ -175,35 +120,6 @@ static int mxs_flash_ident(struct mtd_info *mtd)
>         return ret;
>  }
>
> -static int mxs_read_page_ecc(struct mtd_info *mtd, void *buf, unsigned int page)
> -{
> -       register struct nand_chip *chip = mtd_to_nand(mtd);
> -       int ret;
> -
> -       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> -       ret = nand_chip.ecc.read_page(mtd, chip, buf, 1, page);
> -       if (ret < 0) {
> -               printf("read_page failed %d\n", ret);
> -               return -1;
> -       }
> -       return 0;
> -}
> -
> -static int is_badblock(struct mtd_info *mtd, loff_t offs, int allowbbt)
> -{
> -       register struct nand_chip *chip = mtd_to_nand(mtd);
> -       unsigned int block = offs >> chip->phys_erase_shift;
> -       unsigned int page = offs >> chip->page_shift;
> -
> -       debug("%s offs=0x%08x block:%d page:%d\n", __func__, (int)offs, block,
> -             page);
> -       chip->cmdfunc(mtd, NAND_CMD_READ0, mtd->writesize, page);
> -       memset(chip->oob_poi, 0, mtd->oobsize);
> -       chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
> -
> -       return chip->oob_poi[0] != 0xff;
> -}
> -
>  /* setup mtd and nand structs and init mxs_nand driver */
>  void nand_init(void)
>  {
> @@ -215,7 +131,7 @@ void nand_init(void)
>         mxs_nand_init_spl(&nand_chip);
>         mtd = nand_to_mtd(&nand_chip);
>         /* set mtd functions */
> -       nand_chip.cmdfunc = mxs_nand_command;
> +       nand_chip.cmdfunc = nand_spl_command_lp;
>         nand_chip.scan_bbt = nand_default_bbt;
>         nand_chip.numchips = 1;
>
> @@ -234,92 +150,6 @@ void nand_init(void)
>         mtd->size = nand_chip.chipsize;
>         nand_chip.scan_bbt(mtd);
>         mxs_nand_setup_ecc(mtd);
> -}
> -
> -int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> -{
> -       unsigned int sz;
> -       unsigned int block, lastblock;
> -       unsigned int page, page_offset;
> -       unsigned int nand_page_per_block;
> -       struct nand_chip *chip;
> -       u8 *page_buf = NULL;
> -
> -       chip = mtd_to_nand(mtd);
> -       if (!chip->numchips)
> -               return -ENODEV;
> -
> -       page_buf = malloc(mtd->writesize);
> -       if (!page_buf)
> -               return -ENOMEM;
> -
> -       /* offs has to be aligned to a page address! */
> -       block = offs / mtd->erasesize;
> -       lastblock = (offs + size - 1) / mtd->erasesize;
> -       page = (offs % mtd->erasesize) / mtd->writesize;
> -       page_offset = offs % mtd->writesize;
> -       nand_page_per_block = mtd->erasesize / mtd->writesize;
> -
> -       while (block <= lastblock) {
> -               if (!is_badblock(mtd, mtd->erasesize * block, 1)) {
> -                       /* Skip bad blocks */
> -                       while (page < nand_page_per_block && size > 0) {
> -                               int curr_page = nand_page_per_block * block + page;
> -
> -                               if (mxs_read_page_ecc(mtd, page_buf, curr_page) < 0) {
> -                                       free(page_buf);
> -                                       return -EIO;
> -                               }
> -
> -                               if (size > (mtd->writesize - page_offset))
> -                                       sz = (mtd->writesize - page_offset);
> -                               else
> -                                       sz = size;
> -
> -                               memcpy(dst, page_buf + page_offset, sz);
> -                               dst += sz;
> -                               size -= sz;
> -                               page_offset = 0;
> -                               page++;
> -                       }
> -
> -                       page = 0;
> -               } else {
> -                       lastblock++;
> -               }
> -
> -               block++;
> -       }
> -
> -       free(page_buf);
> -
> -       return 0;
> -}
> -
> -int nand_default_bbt(struct mtd_info *mtd)
> -{
> -       return 0;
> -}
> -
> -void nand_deselect(void)
> -{
> -}
> -
> -u32 nand_spl_adjust_offset(u32 sector, u32 offs)
> -{
> -       unsigned int block, lastblock;
> -
> -       block = sector / mtd->erasesize;
> -       lastblock = (sector + offs) / mtd->erasesize;
> -
> -       while (block <= lastblock) {
> -               if (is_badblock(mtd, block * mtd->erasesize, 1)) {
> -                       offs += mtd->erasesize;
> -                       lastblock++;
> -               }
> -
> -               block++;
> -       }
>
> -       return offs;
> +       nand_spl_init(&nand_chip);
>  }
> diff --git a/drivers/mtd/nand/raw/nand_common_spl.c b/drivers/mtd/nand/raw/nand_common_spl.c
> new file mode 100644
> index 0000000000..0595fcbc26
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/nand_common_spl.c
> @@ -0,0 +1,245 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Amarula Solutions B.V. All rights reserved.
> + *
> + * Author: Michael Trimarchi <michael@amarulasolutions.com>
> + * Author: Weijie Gao <weijie.gao@mediatek.com>
> + */
> +
> +#include <image.h>
> +#include <malloc.h>
> +#include <time.h>
> +#include <linux/sizes.h>
> +#include <linux/delay.h>
> +#include <linux/mtd/rawnand.h>
> +
> +static struct nand_chip *nand;
> +static u8 *buffer;
> +static int nand_valid;
> +
> +int nand_spl_init(struct nand_chip *chip)
> +{
> +       struct mtd_info *mtd = nand_to_mtd(chip);
> +
> +       if (!mtd)
> +               return -EINVAL;
> +
> +       buffer = malloc(mtd->writesize);
> +       if (!buffer)
> +               return -ENOMEM;
> +
> +       nand = chip;
> +       nand_valid = 1;
> +
> +       return 0;
> +}
> +
> +static struct nand_chip *nand_spl_get_chip(void)
> +{
> +       return nand;
> +}
> +
> +void nand_spl_command_lp(struct mtd_info *mtd, unsigned int command,
> +                        int column, int page_addr)
> +{
> +       register struct nand_chip *chip = mtd_to_nand(mtd);
> +       u32 timeo, time_start;
> +
> +       /* Command latch cycle */
> +       chip->cmd_ctrl(mtd, command, NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> +
> +       if (column != -1 || page_addr != -1) {
> +               int ctrl = NAND_CTRL_CHANGE | NAND_NCE | NAND_ALE;
> +
> +               /* Serially input address */
> +               if (column != -1) {
> +                       chip->cmd_ctrl(mtd, column, ctrl);
> +                       ctrl &= ~NAND_CTRL_CHANGE;
> +                       if (command != NAND_CMD_READID)
> +                               chip->cmd_ctrl(mtd, column >> 8, ctrl);
> +               }
> +               if (page_addr != -1) {
> +                       chip->cmd_ctrl(mtd, page_addr, ctrl);
> +                       chip->cmd_ctrl(mtd, page_addr >> 8,
> +                                      NAND_NCE | NAND_ALE);
> +                       if (chip->options & NAND_ROW_ADDR_3)
> +                               chip->cmd_ctrl(mtd, page_addr >> 16,
> +                                              NAND_NCE | NAND_ALE);
> +               }
> +       }
> +       chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
> +
> +       /*
> +        * Program and erase have their own busy handlers status, sequential
> +        * in and status need no delay.
> +        */
> +       switch (command) {
> +       case NAND_CMD_STATUS:
> +       case NAND_CMD_READID:
> +       case NAND_CMD_SET_FEATURES:
> +               return;
> +
> +       case NAND_CMD_READ0:
> +               chip->cmd_ctrl(mtd, NAND_CMD_READSTART,
> +                              NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
> +               chip->cmd_ctrl(mtd, NAND_CMD_NONE,
> +                              NAND_NCE | NAND_CTRL_CHANGE);
> +       }
> +
> +       /*
> +        * Apply this short delay always to ensure that we do wait tWB in
> +        * any case on any machine.
> +        */
> +       ndelay(100);
> +
> +       timeo = (CONFIG_SYS_HZ * 20) / 1000;
> +       time_start = get_timer(0);
> +       while (get_timer(time_start) < timeo) {
> +               if (chip->dev_ready(mtd))
> +                       break;
> +       }
> +}
> +
> +static int nand_spl_read_page_hwecc(struct mtd_info *mtd, void *buf,
> +                              unsigned int page)
> +{
> +       struct nand_chip *chip = mtd_to_nand(mtd);
> +       int ret;
> +
> +       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> +
> +       ret = chip->ecc.read_page(mtd, chip, buf, 1, page);
> +       if (ret < 0 || ret > chip->ecc.strength)
> +               return -1;
> +
> +       return 0;
> +}
> +
> +static int nand_spl_read_oob_hwecc(struct mtd_info *mtd, void *buf, u32 len,
> +                             unsigned int page)
> +{
> +       struct nand_chip *chip = mtd_to_nand(mtd);
> +       int ret;
> +
> +       chip->cmdfunc(mtd, NAND_CMD_READ0, 0x0, page);
> +
> +       ret = chip->ecc.read_page(mtd, chip, NULL, 1, page);
> +       if (ret < 0)
> +               return -1;
> +
> +       if (len > mtd->oobsize)
> +               len = mtd->oobsize;
> +
> +       memcpy(buf, chip->oob_poi, len);
> +
> +       return 0;
> +}
> +
> +static int nand_spl_check_bad_block(struct mtd_info *mtd, unsigned int page)
> +{
> +       struct nand_chip *chip = mtd_to_nand(mtd);
> +       u32 pages_per_block, i = 0;
> +       int ret;
> +       u8 bad;
> +
> +       pages_per_block = 1 << (mtd->erasesize_shift - mtd->writesize_shift);
> +
> +       /* Read from first/last page(s) if necessary */
> +       if (chip->bbt_options & NAND_BBT_SCANLASTPAGE) {
> +               page += pages_per_block - 1;
> +               if (chip->bbt_options & NAND_BBT_SCAN2NDPAGE)
> +                       page--;
> +       }
> +
> +       do {
> +               ret = nand_spl_read_oob_hwecc(mtd, &bad, 1, page);
> +               if (ret)
> +                       return ret;
> +
> +               ret = bad != 0xFF;
> +
> +               i++;
> +               page++;
> +       } while (!ret && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2);
> +
> +       return ret;
> +}
> +
> +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
> +{
> +       struct nand_chip *chip = nand_spl_get_chip();
> +       struct mtd_info *mtd = &chip->mtd;
> +       u32 addr, col, page, chksz;
> +       bool check_bad = true;
> +
> +       if (!nand_valid)
> +               return -ENODEV;
> +
> +       while (size) {
> +               if (check_bad || !(offs & mtd->erasesize_mask)) {
> +                       addr = offs & (~mtd->erasesize_mask);
> +                       page = addr >> mtd->writesize_shift;
> +                       if (nand_spl_check_bad_block(mtd, page)) {
> +                               /* Skip bad block */
> +                               if (addr >= mtd->size - mtd->erasesize)
> +                                       return -ENXIO;
> +
> +                               offs += mtd->erasesize;
> +                               continue;
> +                       }
> +
> +                       check_bad = false;
> +               }
> +
> +               col = offs & mtd->writesize_mask;
> +               page = offs >> mtd->writesize_shift;
> +               chksz = min(mtd->writesize - col, (uint32_t)size);
> +
> +               if (unlikely(chksz < mtd->writesize)) {
> +                       /* Not reading a full page */
> +                       if (nand_spl_read_page_hwecc(mtd, buffer, page))
> +                               return -EIO;
> +
> +                       memcpy(dest, buffer + col, chksz);
> +               } else {
> +                       if (nand_spl_read_page_hwecc(mtd, dest, page))
> +                               return -EIO;
> +               }
> +
> +               dest += chksz;
> +               offs += chksz;
> +               size -= chksz;
> +       }
> +
> +       return 0;
> +}
> +
> +int nand_default_bbt(struct mtd_info *mtd)
> +{
> +       return 0;
> +}
> +
> +void nand_deselect(void)
> +{
> +}
> +
> +u32 nand_spl_adjust_offset(u32 sector, u32 offs)
> +{
> +       struct nand_chip *chip = nand_spl_get_chip();
> +       struct mtd_info *mtd = nand_to_mtd(chip);
> +       unsigned int block, lastblock;
> +
> +       block = sector / mtd->erasesize;
> +       lastblock = (sector + offs) / mtd->erasesize;
> +
> +       while (block <= lastblock) {
> +               if (nand_spl_check_bad_block(mtd, block * mtd->erasesize)) {
> +                       offs += mtd->erasesize;
> +                       lastblock++;
> +               }
> +
> +               block++;
> +       }
> +
> +       return offs;
> +}
> diff --git a/drivers/mtd/nand/raw/nand_common_spl.h b/drivers/mtd/nand/raw/nand_common_spl.h
> new file mode 100644
> index 0000000000..36bf63b230
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/nand_common_spl.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 Amarula Solution B.V. All rights reserved.
> + *
> + * Author: Michael Trimarchi <michael@amarulasolutions.com>
> + */
> +
> +#ifndef _NAND_COMMON_SPL_H
> +#define _NAND_COMMON_SPL_H
> +
> +void nand_spl_command_lp(struct mtd_info *mtd, unsigned int command,
> +                        int column, int page_addr);
> +int nand_spl_init(struct nand_chip *chip);
> +
> +#endif /* _NAND_COMMON_SPL_H_ */
> --
> 2.34.1
>