[PATCH 2/3] ARM: i.MX: implement GPMI NAND xload

[Sascha Hauer]

Hi Andrej,

Some comments inline.

On Wed, Jan 20, 2021 at 01:51:06PM +0100, Andrej Picej wrote:
> From: Sascha Hauer <s.hauer at pengutronix.de>
> 
> Commit is based on initial Sascha Hauer's work. It implements PBL xload
> mechanism to load image from GPMI NAND flash.
> 
> Additional work was done, so that the NAND's size, page size and OOB's
> size are autodetected and not hardcoded. Detection method follows the
> same methods as used in NAND driver, meaning NAND ONFI support is probed
> and if NAND supports ONFI, NAND memory organization is read from ONFI
> parameter page otherwise "READ ID" is used.
> 
> Currently only implemented for i.MX6 familly of SoCs.
> 
> Signed-off-by: Sascha Hauer <s.hauer at pengutronix.de>
> Signed-off-by: Primoz Fiser <primoz.fiser at norik.com>
> Signed-off-by: Andrej Picej <andrej.picej at norik.com>
> ---
>  arch/arm/mach-imx/Makefile             |    2 +-
>  arch/arm/mach-imx/include/mach/xload.h |    1 +
>  arch/arm/mach-imx/xload-gpmi-nand.c    | 1163 ++++++++++++++++++++++++
>  3 files changed, 1165 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c
> 
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index e45f758e9..d94c846a1 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> +static int mxs_nand_get_info(struct mxs_nand_info *info, void *databuf)
> +{
> +	int ret, i;
> +
> +	ret = mxs_nand_check_onfi(info, databuf);
> +	if (ret) {
> +		if (ret != 1)
> +			return ret;
> +		pr_warn("ONFI not supported, try \"READ ID\"...\n");

You already printed a "ONFI not supported\n" message. Printing it once
is enough. Also this message appears with every non-ONFI nand, right? In
that case it should rather be pr_info()

> +	/*
> +	 * If ONFI is not supported or if it fails try to get NAND's info from
> +	 * "READ ID" command.
> +	 */
> +	pr_debug("Trying \"READ ID\" command...\n");
> +	ret = mxs_nand_get_readid(info, databuf);
> +	if (ret) {
> +		if (ret != -EOVERFLOW)
> +			return ret;
> +
> +		/*
> +		 * If NAND's "READ ID" returns bad values, try to set them to
> +		 * default (most common NAND memory org.) and continue.
> +		 */
> +		pr_warn("NANDs READ ID command returned bad values" \
> +			" set them to default and try to continue!\n");
> +		info->organization.pagesize = 2048;
> +		info->organization.oobsize = 64;
> +		info->nand_size = SZ_1G;

Is this worth it? READ ID is the most basic command, when this doesn't
work I don't think there's a point in continuing.

> +	}
> +succ:
> +	pr_debug("NAND page_size: %d\n", info->organization.pagesize);
> +	pr_debug("NAND block_size: %d\n",
> +		info->organization.pages_per_eraseblock
> +		* info->organization.pagesize);
> +	pr_debug("NAND oob_size: %d\n", info->organization.oobsize);
> +	pr_debug("NAND nand_size: %lu\n", info->nand_size);
> +	pr_debug("NAND bits_per_cell: %d\n", info->organization.bits_per_cell);
> +	pr_debug("NAND planes_per_lun: %d\n",
> +		info->organization.planes_per_lun);
> +	pr_debug("NAND luns_per_target: %d\n",
> +		info->organization.luns_per_target);
> +	pr_debug("NAND eraseblocks_per_lun: %d\n",
> +		info->organization.eraseblocks_per_lun);
> +	pr_debug("NAND ntargets: %d\n", info->organization.ntargets);
> +
> +
> +	return 0;
> +}
> +
> +/* ---------------------------- BCB handling part -------------------------- */
> +
> +static uint32_t calc_chksum(void *buf, size_t size)
> +{
> +	u32 chksum = 0;
> +	u8 *bp = buf;
> +	size_t i;
> +
> +	for (i = 0; i < size; i++)
> +		chksum += bp[i];
> +
> +	return ~chksum;
> +}
> +
> +static int get_fcb(struct mxs_nand_info *info, void *databuf)
> +{
> +	int i, pagenum, ret;
> +	uint32_t checksum;
> +	struct fcb_block *fcb = &info->fcb;
> +
> +	/* First page read fails, this shouldn't be necessary */
> +	mxs_nand_read_page(info, info->organization.pagesize,
> +		info->organization.oobsize, 0, databuf, 1);
> +
> +	for (i = 0; i < 4; i++) {
> +		pagenum = 64 * i;

I haven't looked at the documentation, but the '64' should probably be pages
per block, so better info->organization.pages_per_eraseblock?

> +
> +		ret = mxs_nand_read_page(info, info->organization.pagesize,
> +			info->organization.oobsize, pagenum, databuf, 1);
> +		if (ret)
> +			continue;
> +
> +		memcpy(fcb, databuf + mxs_nand_aux_status_offset(),
> +			sizeof(*fcb));
> +
> +		if (fcb->FingerPrint != FCB_FINGERPRINT) {
> +			pr_err("No FCB found on page %d\n", pagenum);
> +			continue;
> +		}
> +
> +		checksum = calc_chksum((void *)fcb +
> +			sizeof(uint32_t), sizeof(*fcb) - sizeof(uint32_t));
> +
> +		if (checksum != fcb->Checksum) {
> +			pr_err("FCB on page %d has invalid checksum. " \
> +				"Expected: 0x%08x, calculated: 0x%08x",
> +				pagenum, fcb->Checksum, checksum);
> +			continue;
> +		}
> +
> +		pr_debug("Found FCB:\n");
> +		pr_debug("PageDataSize:     0x%08x\n", fcb->PageDataSize);
> +		pr_debug("TotalPageSize:    0x%08x\n", fcb->TotalPageSize);
> +		pr_debug("SectorsPerBlock:  0x%08x\n", fcb->SectorsPerBlock);
> +		pr_debug("FW1_startingPage: 0x%08x\n",
> +			fcb->Firmware1_startingPage);
> +		pr_debug("PagesInFW1:       0x%08x\n", fcb->PagesInFirmware1);
> +		pr_debug("FW2_startingPage: 0x%08x\n",
> +			fcb->Firmware2_startingPage);
> +		pr_debug("PagesInFW2:       0x%08x\n", fcb->PagesInFirmware2);
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int get_dbbt(struct mxs_nand_info *info, void *databuf)
> +{
> +	int i, ret;
> +	int page;
> +	int startpage = info->fcb.DBBTSearchAreaStartAddress;
> +	struct dbbt_block dbbt;
> +
> +	for (i = 0; i < 4; i++) {
> +		page = startpage + i * 64;

Same here.

Regards,
 Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |