Dear Simon Schwarz,
Am 28.06.2011 16:14, schrieb simonschwarz...@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 <schw...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
