[U-Boot] [PATCH] mtd: nand: omap: add CONFIG_SPL_NAND_DEVICE_WIDTH to determine NAND device bus-width

[Scott Wood]

On Thu, 2013-12-05 at 18:09 +0530, Pekon Gupta wrote:
> This patch adds CONFIG_SPL_NAND_DEVICE_WIDTH to specify bus-width of NAND device
>   CONFIG_SPL_NAND_DEVICE_WIDTH == 16: NAND device with x16 bus-width
>   CONFIG_SPL_NAND_DEVICE_WIDTH == 8:  NAND device with x8 bus-width
> 
> Need for a separate CONFIG_xx arise from following situations.
> (1) SPL NAND drivers does not have framework to parse ONFI parameter page.

Yes, at least for smaller SPLs.

> (2) if !defined(CONFIG_SYS_NAND_SELF_INIT)
>          |- board_nand_init()
>          |- nand_scan()
>                |- nand_scan_ident()
>                |- nand_scan_tail()
>    This means board_nand_init() is called before nand_scan_ident(). So NAND
>    controller is initialized before the actual probing of NAND device.
>    However some controller (like GPMC) need to be specifically configured for
>    bus-width of NAND device.
>    In such cases, bus-width of the NAND device should be known in advance
>    of actual device probing. Hence, CONFIG_SPL_NAND_DEVICE_WIDTH is useful.

See below.

> (3) Non-ONFI compliant devices need some mechanism to specify device bus-width
>    to driver.

Does an x8 "READ ID" work with non-ONFI devices?  If not, you need
something, and I suppose you need to hardcode whether an ONFI device is
present -- but for non-SPL it's better to do it in a way that is
per-device, such as having board code initialize certain registers.  If
you really want to do it this way, though, there's already a variable
for this: CONFIG_SYS_NAND_BUSWIDTH_16BIT

Just be sure to update the documentation to add to the list of drivers
that care about that symbol.

> Signed-off-by: Pekon Gupta <pekon at ti.com>
> ---
>  doc/README.nand                        |  9 +++++++++
>  drivers/mtd/nand/omap_gpmc.c           | 14 ++++++++++----
>  include/configs/am335x_evm.h           |  1 +
>  include/configs/am335x_igep0033.h      |  1 +
>  include/configs/am3517_crane.h         |  1 +
>  include/configs/am3517_evm.h           |  1 +
>  include/configs/cm_t35.h               |  1 +
>  include/configs/devkit8000.h           |  1 +
>  include/configs/dig297.h               |  1 +
>  include/configs/mcx.h                  |  1 +
>  include/configs/omap3_beagle.h         |  1 +
>   include/configs/omap3_evm_common.h     |  2 +-
>  include/configs/omap3_igep00x0.h       |  1 +
>  include/configs/omap3_logic.h          |  1 +
>  include/configs/omap3_overo.h          |  1 +
>  include/configs/omap3_pandora.h        |  2 +-
>  include/configs/omap3_zoom1.h          |  1 +
>  include/configs/omap3_zoom2.h          |  1 +
>  include/configs/siemens-am33x-common.h |  1 +
>  include/configs/tam3517-common.h       |  1 +
>  include/configs/tricorder.h            |  1 +
>  21 files changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/README.nand b/doc/README.nand
> index b91f198..a07863a 100644
> --- a/doc/README.nand
> +++ b/doc/README.nand
> @@ -190,6 +190,15 @@ Configuration Options:
>  	This is used by SoC platforms which do not have built-in ELM
>  	hardware engine required for BCH ECC correction.
>  
> +   CONFIG_SPL_NAND_DEVICE_WIDTH
> +	Specifies bus-width of the default NAND device connected to SoC.
> +	This config is useful for driver which cannot self initialize or
> +	parse ONFI parameter (like SPL drivers), or for supporting non-ONFI
> +	compliant devices.
> +	This config can take following values:
> +	- 8: x8 NAND devices is connected
> +	- 16: x16 NAND device is connected

I don't understand "cannot self initialize".  I understand "doesn't
currently self initialize", but if that's causing a problem then why not
convert the driver to use self-init?

SPL is a different situation as it typically doesn't have dynamic ID
code at all (as opposed to the question of whether driver code can be
inserted between nand_scan_ident and nand_scan_tail).

>  Platform specific options
>  =========================
> diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
> index fae00be..1870152 100644
> --- a/drivers/mtd/nand/omap_gpmc.c
> +++ b/drivers/mtd/nand/omap_gpmc.c
> @@ -861,13 +861,19 @@ int board_nand_init(struct nand_chip *nand)
>  	nand->priv	= &bch_priv;
>  	nand->cmd_ctrl	= omap_nand_hwcontrol;
>  	nand->options	|= NAND_NO_PADDING | NAND_CACHEPRG;
> -	/* If we are 16 bit dev, our gpmc config tells us that */
> -	if ((readl(&gpmc_cfg->cs[cs].config1) & 0x3000) == 0x1000)
> -		nand->options |= NAND_BUSWIDTH_16;
> -
>  	nand->chip_delay = 100;
>  	nand->ecc.layout = &omap_ecclayout;
>  
> +	/* configure driver and controller based on NAND device bus-width */
> +	gpmc_config = readl(&gpmc_cfg->cs[cs].config1);
> +	if (CONFIG_SPL_NAND_DEVICE_WIDTH == 16) {
> +		nand->options |= NAND_BUSWIDTH_16;
> +		writel(gpmc_config | (0x1 << 12), &gpmc_cfg->cs[cs].config1);
> +	} else {
> +		nand->options &= ~NAND_BUSWIDTH_16;
> +		writel(gpmc_config & ~(0x1 << 12), &gpmc_cfg->cs[cs].config1);
> +	}

This doesn't appear to be limited to SPL, or even to (SPL || non-ONFI).

-Scott