Message ID | 1309270480-31918-5-git-send-email-schwarz@corscience.de |
---|---|
State | Superseded |
Headers | show |
Dear Simon Schwarz, Am 28.06.2011 16:14, schrieb simonschwarzcor@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@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
Dear Andreas, > You also set some special ECC handling in this patch, please honour this > in your commit message! will do. >> + >> +/** >> + * 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)? Its the default ECC setting this is software for standard U-Boot. Just for the Preloader it is HW. So i think using CONFIG_PRELOADER is ok here. >> + nand->ecc.size = 512; >> + nand->ecc.bytes = 24; > > Ouch, these two values are extremely HW spwcific and need to be > configurable then. will change. > >> + 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? Sorry don't understand that. These three functions are not related to the bus width IMHO. Thank you for the feedback! Simon
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> + +#define GPMC_WAIT0_PIN_ACTIVE (1 << 8) + 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 + +/** + * 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 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; + nand->ecc.hwctl = omap_enable_hwecc; + nand->ecc.correct = omap_correct_data; + nand->ecc.calculate = omap_calculate_ecc; + nand->read_buf = gpmc_read_buf16; + omap_hwecc_init(nand); +#endif return 0; } + +
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. Signed-off-by: Simon Schwarz <schwarz@corscience.de> --