[U-Boot] [4/5]devkit8000 nand_spl: Add SPL NAND support to omap_gpmc driver

[Andreas Bießmann]

Dear Simon Schwarz,

Am 28.06.2011 16:14, schrieb simonschwarzcor at googlemail.com:
> Add support for NAND_SPL to omap gpmc driver. This means adding nand_read_buf16 to read from GPMC 32bit buffer (16 here means 16bit bus!) and adding omap_dev_ready as indicator if the GPMC is ready.  

You also set some special ECC handling in this patch, please honour this
in your commit message!

> 
> Signed-off-by: Simon Schwarz <schwarz at corscience.de>
> --
> 
> diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
> index 99b9cef..7dfb05d 100644
> --- a/drivers/mtd/nand/omap_gpmc.c
> +++ b/drivers/mtd/nand/omap_gpmc.c
> @@ -29,6 +29,9 @@
>  #include <linux/mtd/nand_ecc.h>
>  #include <nand.h>
>  
> +

no additional empty line

> +#define GPMC_WAIT0_PIN_ACTIVE (1 << 8)

this is only used once, why don't you use just (1<<8) where needed? Is
there some special meaning with 8 bit shift, can you use a register name
instead? Should it be configurable in any way for other omap3 devices?

> +
>  static uint8_t cs;
>  static struct nand_ecclayout hw_nand_oob = GPMC_NAND_HW_ECC_LAYOUT;
>  
> @@ -61,6 +64,37 @@ static void omap_nand_hwcontrol(struct mtd_info *mtd, int32_t cmd,
>  		writeb(cmd, this->IO_ADDR_W);
>  }
>  
> +/* Check wait pin as dev ready indicator */
> +int omap_dev_ready(struct mtd_info *mtd)
> +{
> +	return gpmc_cfg->status & GPMC_WAIT0_PIN_ACTIVE;
> +}
> +
> +#ifdef CONFIG_PRELOADER

Isn't that SPL related? Why is it required for SPL and not for standard
U-Boot?

> +
> +/**
> + * nand_read_buf16 - [DEFAULT] read chip data into buffer
> + * @mtd:    MTD device structure
> + * @buf:    buffer to store date
> + * @len:    number of bytes to read
> + *
> + * Default read function for 16bit buswith
> + *
> + * This function is based on nand_read_buf16 from nand_base.c. This version
> + * reads 32bit not 16bit although the bus only has 16bit.
> + */
> +static void gpmc_read_buf16(struct mtd_info *mtd, uint8_t *buf, int len)
> +{
> +	int i;
> +	struct nand_chip *chip = mtd->priv;
> +	u32 *p = (u32 *) buf;
> +	len >>= 2;
> +
> +	for (i = 0; i < len; i++)
> +		p[i] = readl(chip->IO_ADDR_R);
> +}
> +#endif
> +
>  /*
>   * omap_hwecc_init - Initialize the Hardware ECC for NAND flash in
>   *                   GPMC controller
> @@ -278,7 +312,9 @@ void omap_nand_switch_ecc(int32_t hardware)
>  	/* Update NAND handling after ECC mode switch */
>  	nand_scan_tail(mtd);
>  
> +	#ifndef CONFIG_SPL
>  	nand->options &= ~NAND_OWN_BUFFERS;
> +	#endif
>  }
>  
>  /*
> @@ -337,8 +373,23 @@ int board_nand_init(struct nand_chip *nand)
>  		nand->options |= NAND_BUSWIDTH_16;
>  
>  	nand->chip_delay = 100;
> +	nand->dev_ready = omap_dev_ready;
>  	/* Default ECC mode */
> +#ifndef CONFIG_PRELOADER

should't this some CONFIG_USE_SOFT_ECC (or whatever config variable
define that behaviour)?

>  	nand->ecc.mode = NAND_ECC_SOFT;
> +#else
> +	nand->ecc.mode = NAND_ECC_HW;
> +	nand->ecc.layout = &hw_nand_oob;
> +	nand->ecc.size = 512;
> +	nand->ecc.bytes = 24;

Ouch, these two values are extremely HW spwcific and need to be
configurable then.

> +	nand->ecc.hwctl = omap_enable_hwecc;
> +	nand->ecc.correct = omap_correct_data;
> +	nand->ecc.calculate = omap_calculate_ecc;

Isn't that some kind of CONFIG_NAND_BUSWDITH (or whatever config
variable define that behaviour) related setting?

> +	nand->read_buf = gpmc_read_buf16;
> +	omap_hwecc_init(nand);
> +#endif
>  
>  	return 0;
>  }
> +
> +

please remove these two additional empty lines

regards

Andreas Bießmann