Re: [U-Boot] [PATCH v2 1/2] i.MX6: nand: add nandbcb update command

Hi Jagan,

On Tue, Feb 6, 2018 at 5:41 PM, Jagan Teki jagannadh.teki@gmail.com wrote:

---

Changes for v2:

• Fixed commit message notes
• Updated proper commit message
• Update doc/README.imx6 with NAND boot details
• Fixed long length variable names.
• Fixed Gigantic variable name.
• NULL checks for kzalloc
• Move Kconfig option in separate patch
• Fixed checkpatch warninigs

Thanks for reworking this patch. It looks much better!

arch/arm/include/asm/mach-imx/imx-nandbcb.h | 107 ++++++++ arch/arm/include/asm/mach-imx/mxs-nand.h | 15 ++ arch/arm/mach-imx/Makefile | 1 + arch/arm/mach-imx/cmd_nandbcb.c | 371 ++++++++++++++++++++++++++++ doc/README.imx6 | 77 ++++++ drivers/mtd/nand/mxs_nand.c | 8 +- 6 files changed, 575 insertions(+), 4 deletions(-) create mode 100644 arch/arm/include/asm/mach-imx/imx-nandbcb.h create mode 100644 arch/arm/include/asm/mach-imx/mxs-nand.h create mode 100644 arch/arm/mach-imx/cmd_nandbcb.c

diff --git a/arch/arm/include/asm/mach-imx/imx-nandbcb.h b/arch/arm/include/asm/mach-imx/imx-nandbcb.h new file mode 100644 index 0000000..807b16b --- /dev/null +++ b/arch/arm/include/asm/mach-imx/imx-nandbcb.h @@ -0,0 +1,107 @@ +/*

• • Copyright (C) 2017 Jagan Teki jagan@amarulasolutions.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#ifndef _IMX_NAND_BCB_H_ +#define _IMX_NAND_BCB_H_

+#define FCB_FINGERPRINT 0x20424346 /* 'FCB' */ +#define FCB_VERSION_1 0x01000000

+#define DBBT_FINGERPRINT2 0x54424244 /* 'DBBT' */ +#define DBBT_VERSION_1 0x01000000

+struct dbbt_block {

•   u32 checksum;   /* reserved on i.MX6 */
  

•   u32 fingerprint;
  

•   u32 version;
  

•   u32 numberbb;   /* reserved on i.MX6 */
  

•   u32 dbbtnumofpages;
  

+};

+struct fcb_block {

•   u32 checksum;           /* First fingerprint in first byte */
  

•   u32 fingerprint;        /* 2nd fingerprint at byte 4 */
  

•   u32 version;            /* 3rd fingerprint at byte 8 */
  

•   u8 datasetup;
  

•   u8 datahold;
  

•   u8 addresssetup;
  

•   u8 dsample_time;
  

•   /* These are for application use only and not for ROM. */
  

•   u8 nandtiming;
  

•   u8 rea;
  

•   u8 rloh;
  

•   u8 rhoh;
  

•   u32 pagesize;           /* 2048 for 2K pages, 4096 for 4K pages */
  

•   u32 oob_pagesize;       /* 2112 for 2K pages, 4314 for 4K pages */
  

•   u32 sectors;            /* Number of 2K sections per block */
  

•   u32 nr_nand;            /* Total Number of NANDs - not used by ROM */
  

•   u32 nr_die;             /* Number of separate chips in this NAND */
  

•   u32 celltype;           /* MLC or SLC */
  

•   u32 ecc_type;           /* Type of ECC, can be one of BCH-0-20 */
  

•   u32 ecc_nr;             /* Number of bytes for Block0 - BCH */
  

•   /* Block size in bytes for all blocks other than Block0 - BCH */
  

•   u32 ecc_size;
  

•   u32 ecc_level;          /* Ecc level for Block 0 - BCH */
  

•   u32 meta_size;          /* Metadata size - BCH */
  

•   /* Number of blocks per page for ROM use - BCH */
  

•   u32 nr_blocks;
  

•   u32 ecc_type_sdk;       /* Type of ECC, can be one of BCH-0-20 */
  

•   u32 ecc_nr_sdk;         /* Number of bytes for Block0 - BCH */
  

•   /* Block size in bytes for all blocks other than Block0 - BCH */
  

•   u32 ecc_size_sdk;
  

•   u32 ecc_level_sdk;      /* Ecc level for Block 0 - BCH */
  

•   /* Number of blocks per page for SDK use - BCH */
  

•   u32 nr_blocks_sdk;
  

•   u32 meta_size_sdk;      /* Metadata size - BCH */
  

•   u32 erase_th;           /* To set into BCH_MODE register */
  

•   /* 0: normal boot
  

•    * 1: to load patch starting next to FCB
  

•    */
  

Multi-line style is wrong here.

+static int nandbcb_update(struct mtd_info *mtd, loff_t off, size_t size,

•                     size_t maxsize, const u_char *buf)
  

+{

•   nand_erase_options_t opts;
  

•   struct fcb_block *fcb;
  

•   struct dbbt_block *dbbt;
  

•   loff_t fw1_off, fw2_off;
  

•   void *fwbuf, *fcb_raw_page, *dbbt_page, *dbbt_data_page;
  

•   int nr_blks, nr_blks_fcb, nr_blks_fw, fw1_blk, fw2_blk;
  

•   size_t fwsize, dummy;
  

•   int i, ret;
  

•   /* erase */
  

•   memset(&opts, 0, sizeof(opts));
  

•   opts.offset = off;
  

•   opts.length = maxsize - 1;
  

•   ret = nand_erase_opts(mtd, &opts);
  

•   if (ret) {
  

•           printf("%s: erase failed\n", __func__);
  

You could also show the 'ret' value for helping debugging.

•           return ret;
  

•   }
  

•   /*
  

•    * Reference documentation from i.MX6DQRM section 8.5.2.2
  

•    *
  

•    * Nand Boot Control Block(BCB) contains two data structures,
  

•    * - Firmware Configuration Block(FCB)
  

•    * - Discovered Bad Block Table(DBBT)
  

•    *
  

•    * FCB contains,
  

•    * - nand timings
  

•    * - DBBT search page address,
  

•    * - start page address of primary firmware
  

•    * - start page address of secondary firmware
  

•    *
  

•    * setup fcb:
  

•    * - number of blocks = mtd partition size / mtd erasesize
  

•    * - two firmware blocks, primary and secondary
  

•    * - first 4 block for FCB/DBBT
  

•    * - rest split in half for primary and secondary firmware
  

•    * - same firmware will write two times
  

•    */
  

•   nr_blks_fcb = 4;
  

•   nr_blks = maxsize / mtd->erasesize;
  

•   nr_blks_fw = (nr_blks - nr_blks_fcb) / 2;
  

•   fw1_blk = nr_blks_fcb;
  

•   fw2_blk = fw1_blk + nr_blks_fw;
  

•   /* write fw */
  

•   fwsize = ALIGN(size + FLASH_OFFSET_STANDARD + mtd->writesize,
  

•                  mtd->writesize);
  

•   fwbuf = kzalloc(fwsize, GFP_KERNEL);
  

•   if (!fwbuf) {
  

•           debug("failed to allocate fwbuf\n");
  

•           return -EFAULT;
  

We usually return -ENOMEM when kzalloc() fails.

•   }
  

•   memcpy(fwbuf + FLASH_OFFSET_STANDARD, buf, size);
  

•   fw1_off = fw1_blk * mtd->erasesize;
  

•   ret = nand_write_skip_bad(mtd, fw1_off, &fwsize, NULL, maxsize,
  

•                             (u_char *)fwbuf, WITH_WR_VERIFY);
  

•   printf("NAND fw write: 0x%llx offset, 0x%x bytes written: %s\n",
  

•          fw1_off, fwsize, ret ? "ERROR" : "OK");
  

•   if (ret)
  

•           goto fw_err;
  

•   /* write fw, 2nd time */
  

•   fw2_off = fw2_blk * mtd->erasesize;
  

•   ret = nand_write_skip_bad(mtd, fw2_off, &fwsize, NULL, maxsize,
  

•                             (u_char *)fwbuf, WITH_WR_VERIFY);
  

•   printf("NAND fw write: 0x%llx offset, 0x%x bytes written: %s\n",
  

•          fw2_off, fwsize, ret ? "ERROR" : "OK");
  

•   if (ret)
  

•           goto fw_err;
  

•   /* fill fcb */
  

•   fcb = kzalloc(sizeof(*fcb), GFP_KERNEL);
  

•   if (!fcb) {
  

•           debug("failed to allocate fcb\n");
  

•           return -EFAULT;
  

You should better do:

ret = -ENOMEM; goto XXXX

so that you can free the previously allocated kzalloc()

•   }
  

•   fcb->fw1_start = (fw1_blk * mtd->erasesize) / mtd->writesize;
  

•   fcb->fw2_start = (fw2_blk * mtd->erasesize) / mtd->writesize;
  

•   fcb->fw1_pages = size / mtd->writesize + 1;
  

•   fcb->fw2_pages = fcb->fw1_pages;
  

•   fill_fcb(fcb, mtd);
  

•   /* fill dbbt */
  

•   dbbt_page = kzalloc(mtd->writesize, GFP_KERNEL);
  

•   if (!dbbt_page) {
  

•           debug("failed to allocate dbbt_page\n");
  

•           return -EFAULT;
  

Ditto.

•   }
  

•   dbbt_data_page = kzalloc(mtd->writesize, GFP_KERNEL);
  

•   if (!dbbt_data_page) {
  

•           debug("failed to allocate dbbt_data_page\n");
  

•           return -EFAULT;
  

Ditto.

•   }
  

•   dbbt = dbbt_page;
  

•   dbbt->checksum = 0;
  

•   dbbt->fingerprint = DBBT_FINGERPRINT2;
  

•   dbbt->version = DBBT_VERSION_1;
  

•   ret = dbbt_fill_data(mtd, dbbt_data_page, nr_blks);
  

•   if (ret < 0)
  

•           goto dbbt_err;
  

•   else if (ret > 0)
  

•           dbbt->dbbtnumofpages = 1;
  

•   /* write fcb/dbbt */
  

•   fcb_raw_page = kzalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
  

•   if (!fcb_raw_page) {
  

•           debug("failed to allocate fcb_raw_page\n");
  

•           return -EFAULT;
  

Ditto.