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

Stefano Babic
2014-04-14T11:38:17Z

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@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
=====================================================================