[U-Boot] [PATCH 3/3] i.MX: nand: add nandbcb update command
Fabio Estevam
festevam at gmail.com
Tue Jan 9 22:07:02 UTC 2018
Hi Jagan,
Thanks for working on upstreaming this feature. It is a very useful one!
Last time Scott Wood made several comments about this patch.
Please Cc him next time and make sure to address all of his previous feedback.
Also, you could submit this patch independently of the other two previous one.
Make sure to make ./scripts/checkpatch.pl happy in your next version.
On Tue, Jan 9, 2018 at 4:49 PM, Jagan Teki <jagan at amarulasolutions.com> wrote:
> writing/updating boot image in nand device is not
Writing
> straight forward in i.MX6 platform and it require
requires
> boot control block(BCB) configure.
to be configured
>
> It become difficult to use uboot 'nand' command to
becomes
> write BCB since it require platform specific attributes
requires
> need to taken care.
need to be take care of.
>
> It even difficult to use existing msx-nand.c driver by
It is even
> incorporating BCB attributes like mxs_dma_desc does
> because it require change in mtd and nand command.
requires
>
> So, cmd_nandbcb implemented in arch/arm/mach-imx
>
> BCB contains two data structures, Firmware Configuration Block(FCB)
> and Discovered Bad Block Table(DBBT). FCB has nand timings,
> DBBT search area, page address of primary and secondary firmware.
>
> for nand boot, up on reset bootrom look for FCB structure in
> first block's if FCB found the nand timings are loaded for
> further reads. once FCB read done, DTTB will load and
> finally primary or secondary firmware will load which is boot image.
>
> cmd_nandbcb will create FCB these structures
> by taking mtd partition as an example.
> - initial code will erase entire partition
> - followed by FCB setup, like first 4 block for FCB/DBBT write,
> rest is split into two for primary and secondary firmware
> - write firmware at primary and secondary blocks(two times same image)
> - finally write fcb/dttb in first 4 block.
>
> sample test:
> ============
> icorem6qdl> fatload mmc 0:1 ${loadaddr} SPL
> reading SPL
> 31744 bytes read in 15 ms (2 MiB/s)
>
> icorem6qdl> nandbcb update $loadaddr spl $filesize
> device 0 offset 0x0, size 0x7c00
> Erasing at 0x1c0000 -- 100% complete.
> NAND fw write: 0x100000 offset, 0x9000 bytes written: OK
> NAND fw write: 0x180000 offset, 0x9000 bytes written: OK
It would be nice to put a more complete example into a README file.
Showing an example of flashing u-boot.img into NAND would be nice.
> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
> Signed-off-by: Sergey Kubushyn <ksi at koi8.net>
> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
You added two Signed-off-by line from you. One is enough.
> +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 nandtimingstate;
> + u8 rea;
> + u8 rloh;
> + u8 rhoh;
> + u32 pagedatasize; /* 2048 for 2K pages, 4096 for 4K pages */
> + u32 totalpagesize; /* 2112 for 2K pages, 4314 for 4K pages */
> + u32 sectorsperblock; /* Number of 2K sections per block */
> + u32 numberofnands; /* Total Number of NANDs - not used by ROM */
> + u32 totalinternaldie; /* Number of separate chips in this NAND */
> + u32 celltype; /* MLC or SLC */
> + u32 eccblocknecctype; /* Type of ECC, can be one of BCH-0-20 */
> + u32 eccblock0size; /* Number of bytes for Block0 - BCH */
> + u32 eccblocknsize; /* Block size in bytes for all blocks other than Block0 - BCH */
> + u32 eccblock0ecctype; /* Ecc level for Block 0 - BCH */
> + u32 metadatabytes; /* Metadata size - BCH */
> + u32 numeccblocksperpage; /* Number of blocks per page for ROM use - BCH */
> + u32 eccblocknecclevelsdk; /* Type of ECC, can be one of BCH-0-20 */
> + u32 eccblock0sizesdk; /* Number of bytes for Block0 - BCH */
> + u32 eccblocknsizesdk; /* Block size in bytes for all blocks other than Block0 - BCH */
> + u32 eccblock0ecclevelsdk; /* Ecc level for Block 0 - BCH */
> + u32 numeccblocksperpagesdk; /* Number of blocks per page for SDK use - BCH */
> + u32 metadatabytessdk; /* Metadata size - BCH */
> + u32 erasethreshold; /* To set into BCH_MODE register */
> + u32 bootpatch; /* 0 for normal boot and 1 to load patch starting next to FCB */
> + u32 patchsectors; /* Size of patch in sectors */
> + u32 firmware1_startingpage; /* Firmware image starts on this sector */
> + u32 firmware2_startingpage; /* Secondary FW Image starting Sector */
> + u32 pagesinfirmware1; /* Number of sectors in firmware image */
> + u32 pagesinfirmware2; /* Number of sector in secondary FW image */
> + u32 dbbtsearchareastartaddress; /* Page address where dbbt search area begins */
Some variable names are too large.
> +
> + /* Byte in page data that have manufacturer marked bad block marker,
> + * this will be swapped with metadata[0] to complete page data.
> + */
> + u32 badblockmarkerbyte; /* Byte in page data that have manufacturer marked bad block marker, */
> +
> + /* For BCH ECC sizes other than 8 and 16 the bad block marker does not
> + * start at 0th bit of badblockmarkerbyte. This field is used to get to
> + * the start bit of bad block marker byte with in badblockmarkerbyte
> + */
> + u32 badblockmarkerstartbit;
> +
> + /* FCB value that gives byte offset for
> + * bad block marker on physical NAND page
> + */
> + u32 bbmarkerphysicaloffset;
> + u32 bchtype;
> +
> + u32 tmtiming2_readlatency;
> + u32 tmtiming2_preambledelay;
> + u32 tmtiming2_cedelay;
> + u32 tmtiming2_postambledelay;
> + u32 tmtiming2_cmdaddpause;
> + u32 tmtiming2_datapause;
Keep the variable names shorter, please. Get rid of the tmtiming2_ prefix.
> +
> + /* the flag to enable (1)/disable(0) bi swap */
> + u32 disbbm;
> +
> + /* The swap position of main area in spare area */
> + u32 bbmarkerphysicaloffsetinsparedata;
Gigantic variable name :-)
> +#endif /* _MXS_NAND_ */
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index e687048..dd7d4e9 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -65,6 +65,17 @@ config CMD_HDMIDETECT
> This enables the 'hdmidet' command which detects if an HDMI monitor
> is connected.
>
> +config CMD_NANDBCB
Please add a user for this symbol in a separate patch, so that we can
have a real user of this feature.
> + /*
> + * 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);
kzalloc may fail. You should do a NULL check and return an error code
on failure.
> + 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);
Ditto.
More information about the U-Boot
mailing list