[U-Boot] [PATCH 02/11] SPL: NAND: add support for mxs nand

[Stefano Babic]

Hi Tim,

sorry for late review.

On 03/04/2014 08:01, Tim Harvey wrote:
> This utilizes existing nand support, including the mtd layer to provide a
> a method to load an image off nand for SPL. It is somewhat bulky but avoids
> duplicating code.
> 
> This will need to be split out and re-based on top of Tom's recent patch to
> add MTD NAND support which is needed here.
> 
> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> ---
>  drivers/mtd/Makefile            | 11 ++++++++++
>  drivers/mtd/nand/Makefile       |  9 ++++++++
>  drivers/mtd/nand/mxs_nand_spl.c | 46 +++++++++++++++++++++++++++++++++++++++++
>  spl/Makefile                    |  1 +
>  4 files changed, 67 insertions(+)
>  create mode 100644 drivers/mtd/nand/mxs_nand_spl.c
> 
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index 5467a95..10ec470 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -5,6 +5,15 @@
>  # SPDX-License-Identifier:	GPL-2.0+
>  #
>  
> +ifdef CONFIG_SPL_BUILD
> +
> +ifdef CONFIG_SPL_NAND_MXS
> +obj-y += mtdcore.o
> +obj-y += mtdpart.o
> +endif

This is usually:

obj-$(CONFIG_SPL_NAND_MXS) += mtdcore.o mtdpart.o

> +
> +else # not spl
> +
>  ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)))
>  obj-y += mtdcore.o
>  endif
> @@ -18,3 +27,5 @@ obj-$(CONFIG_FTSMC020) += ftsmc020.o
>  obj-$(CONFIG_FLASH_CFI_LEGACY) += jedec_flash.o
>  obj-$(CONFIG_MW_EEPROM) += mw_eeprom.o
>  obj-$(CONFIG_ST_SMI) += st_smi.o
> +
> +endif
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 02b149c..8b1a37a 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -19,6 +19,14 @@ obj-$(CONFIG_SPL_NAND_ECC) += nand_ecc.o
>  obj-$(CONFIG_SPL_NAND_BASE) += nand_base.o
>  obj-$(CONFIG_SPL_NAND_INIT) += nand.o
>  
> +ifdef CONFIG_SPL_NAND_MXS
> +obj-y += mxs_nand_spl.o
> +obj-y += nand_bbt.o
> +obj-y += nand_ids.o
> +obj-y += nand_util.o
> +obj-y += nand_ecc.o
> +endif
> +

Ditto.

>  else # not spl
>  
>  NORMAL_DRIVERS=y
> @@ -65,5 +73,6 @@ 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
>  
>  endif # drivers
> diff --git a/drivers/mtd/nand/mxs_nand_spl.c b/drivers/mtd/nand/mxs_nand_spl.c
> new file mode 100644
> index 0000000..ec07eb1
> --- /dev/null
> +++ b/drivers/mtd/nand/mxs_nand_spl.c
> @@ -0,0 +1,46 @@
> +/*
> + * (C) Copyright 2014
> + * Tim Harvey, Gateworks Corporation, tharvey at gateworks.com
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <nand.h>
> +
> +static nand_info_t mtd;
> +static struct nand_chip nand_chip;
> +
> +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> +{
> +	size_t sz, actual;
> +
> +	debug("%s offset:0x%08x len:%d\n", __func__, offs, size);
> +	sz = size;
> +	nand_read_skip_bad(&mtd, (loff_t) offs, &sz, &actual, size, dst);
> +	debug("read %d/%d bytes\n", sz, actual);
> +
> +	return 0;
> +}
> +
> +void nand_init(void)
> +{
> +	int ret;
> +
> +	board_nand_init(&nand_chip);
> +	mtd.priv = &nand_chip;
> +	if (nand_chip.select_chip)
> +		nand_chip.select_chip(&mtd, 0);
> +
> +	ret = nand_scan(&mtd, 1);
> +	if (!ret)
> +		ret = nand_scan_tail(&mtd);
> +	debug("oobsize=%d pagesize=%d\n", mtd.oobsize, mtd.writesize);
> +	printf("NAND: %llu MiB\n", (mtd.size / (1024 * 1024)));
> +}
> +
> +void nand_deselect(void)
> +{
> +	if (nand_chip.select_chip)
> +		nand_chip.select_chip(&mtd, -1);
> +}
> diff --git a/spl/Makefile b/spl/Makefile
> index 346d0aa..6a2c80f 100644
> --- a/spl/Makefile
> +++ b/spl/Makefile
> @@ -103,6 +103,7 @@ libs-y += fs/
>  libs-$(CONFIG_SPL_LIBGENERIC_SUPPORT) += lib/
>  libs-$(CONFIG_SPL_POWER_SUPPORT) += drivers/power/ drivers/power/pmic/
>  libs-$(if $(CONFIG_CMD_NAND),$(CONFIG_SPL_NAND_SUPPORT)) += drivers/mtd/nand/
> +libs-$(CONFIG_SPL_NAND_MXS) += drivers/mtd/
>  libs-$(CONFIG_SPL_DRIVERS_MISC_SUPPORT) += drivers/misc/
>  libs-$(CONFIG_SPL_ONENAND_SUPPORT) += drivers/mtd/onenand/
>  libs-$(CONFIG_SPL_DMA_SUPPORT) += drivers/dma/
> 

The main issue (I read also Scott's answer for Patch 4/11) is due to the
fact that you link a lot of MTD code, increasing the footprint. This
becomes an exception, because only a restricted number of function were
introduces for other SOCs (see mxc_nand_spl.c or code in omap_gpmc.c).
Sure that we cannot get rid of it ?

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================