[U-Boot] [PATCH v2 1/4] Flex-OneNAND driver
Scott Wood
scottwood at freescale.com
Mon Mar 9 22:31:50 CET 2009
On Sun, Mar 08, 2009 at 11:45:37AM +0530, Rohit Hagargundgi wrote:
> Sorries for the delay
>
> On Tuesday 16 December 2008 03:14:22 Scott Wood wrote:
> > On Thu, Dec 11, 2008 at 07:57:18PM +0530, Rohit Hagargundgi wrote:
> > > This patch adds support for MLC OneNAND and Flex-OneNAND devices.
> >
> > Patch does not apply to u-boot-nand-flash/next (the master branch is for
> > 2008.12 bugfixes only).
>
> rebased to next branch.
Please send patches with a ready-for-git commit message.
>
> Thanks,
> Rohit
>
> Signed-off-by: Rohit Hagargundgi <h.rohit at samsung.com>
> ---
Comments that don't go into git, such as changes from a previous version,
go below the above --- line.
> drivers/mtd/onenand/onenand_base.c | 718 +++++++++++++++++++++++++++++++-----
> drivers/mtd/onenand/onenand_bbt.c | 14
> drivers/mtd/onenand/onenand_uboot.c | 4
> include/linux/mtd/onenand.h | 16
> include/linux/mtd/onenand_regs.h | 18
> include/onenand_uboot.h | 10
> 6 files changed, 680 insertions(+), 100 deletions(-)
Do you see this going into Linux soon? Unless there's some particular
obstacle to that (other than the code's readiness) I think I'd rather
wait for it to be accepted there, with (hopefully) more eyes looking over
it, and then bring the result into u-boot.
> + boundary = this->boundary[die];
> + ofs += block << (this->erase_shift - 1);
> + if (block > (boundary + 1))
> + ofs += (block - boundary - 1) << (this->erase_shift - 1);
> + return ofs;
> +}
In the Linux patch here:
http://groups.google.com/group/fa.linux.kernel/msg/8edda44803373f33?hl=en&&q=flex+onenand
you deal with 64-bit offsets, but that's missing above.
> +inline loff_t onenand_addr(struct onenand_chip *this, int block)
> +{
> + if (!FLEXONENAND(this))
> + return block << this->erase_shift;
> + return flexonenand_addr(this, block);
> +}
Let the compiler figure out what is inline. Can this function be static?
> + if (FLEXONENAND(this)) {
> + /* Find the eraseregion of this address */
> + i = flexonenand_region(mtd, addr);
> + region = &mtd->eraseregions[i];
> +
> + block_size = region->erasesize;
> + region_end = region->offset + region->erasesize * region->numblocks;
> +
> + /* Start address within region must align on block boundary.
> + * Erase region's start offset is always block start address.
> + */
> + if (unlikely((addr - region->offset) & (block_size - 1))) {
> + MTDDEBUG (MTD_DEBUG_LEVEL0, "onenand_erase: Unaligned address\n");
> + return -EINVAL;
> + }
> + } else {
> + block_size = 1 << this->erase_shift;
> +
> + /* Start address must align on block boundary */
> + if (unlikely(addr & (block_size - 1))) {
> + MTDDEBUG (MTD_DEBUG_LEVEL0, "onenand_erase: Unaligned address\n");
> + return -EINVAL;
> + }
> }
It might be worthwhile to factor out the various if/else bits into some
sort of "ops" struct, so that the differences in chip type are
concentrated in one spot (and it's easier to add a third type if it comes
along).
> @@ -1512,9 +1755,6 @@ int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
> onenand_get_device(mtd, FL_ERASING);
>
> /* Loop throught the pages */
> - len = instr->len;
> - addr = instr->addr;
> -
> instr->state = MTD_ERASING;
>
> while (len) {
> @@ -1541,14 +1781,7 @@ int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
> else
> MTDDEBUG (MTD_DEBUG_LEVEL0, "onenand_erase: "
> "Failed erase, block %d\n",
> - (unsigned)(addr >> this->erase_shift));
> - if (ret == -EPERM)
> - printk("onenand_erase: "
> - "Device is write protected!!!\n");
> - else
> - printk("onenand_erase: "
> - "Failed erase, block %d\n",
> - (unsigned)(addr >> this->erase_shift));
> + onenand_block(this, addr));
> instr->state = MTD_ERASE_FAILED;
> instr->fail_addr = addr;
>
> @@ -1557,6 +1790,21 @@ int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
>
> len -= block_size;
> addr += block_size;
> +
> + if (addr == region_end) {
> + if (!len)
> + break;
> + region++;
> +
> + block_size = region->erasesize;
> + region_end = region->offset + region->erasesize * region->numblocks;
> +
> + if (len & (block_size - 1)) {
> + /* This has been checked at MTD partitioning level. */
> + printk(KERN_ERR "onenand_erase: Unaligned address\n");
> + goto erase_exit;
> + }
> + }
> }
>
> instr->state = MTD_ERASE_DONE;
> @@ -1634,7 +1882,7 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
> int block;
>
> /* Get block number */
> - block = ((int) ofs) >> bbm->bbt_erase_shift;
> + block = onenand_block(this, ofs);
> if (bbm->bbt)
> bbm->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1);
>
> @@ -1682,8 +1930,8 @@ static int onenand_do_lock_cmd(struct mtd_info *mtd, loff_t ofs, size_t len, int
> int start, end, block, value, status;
> int wp_status_mask;
>
> - start = ofs >> this->erase_shift;
> - end = len >> this->erase_shift;
> + start = onenand_block(this, ofs);
> + end = onenand_block(this, ofs + len);
>
> if (cmd == ONENAND_CMD_LOCK)
> wp_status_mask = ONENAND_WP_LS;
> @@ -1718,7 +1966,7 @@ static int onenand_do_lock_cmd(struct mtd_info *mtd, loff_t ofs, size_t len, int
> }
>
> /* Block lock scheme */
> - for (block = start; block < start + end; block++) {
> + for (block = start; block < end; block++) {
> /* Set block address */
> value = onenand_block_address(this, block);
> this->write_word(value, this->base + ONENAND_REG_START_ADDRESS1);
> @@ -1831,7 +2079,7 @@ static void onenand_unlock_all(struct mtd_info *mtd)
> {
> struct onenand_chip *this = mtd->priv;
> loff_t ofs = 0;
> - size_t len = this->chipsize;
> + size_t len = mtd->size;
>
> if (this->options & ONENAND_HAS_UNLOCK_ALL) {
> /* Set start block address */
> @@ -1847,14 +2095,12 @@ static void onenand_unlock_all(struct mtd_info *mtd)
> & ONENAND_CTRL_ONGO)
> continue;
>
> - return;
> -
> /* Check lock status */
> if (onenand_check_lock_status(this))
> return;
>
> /* Workaround for all block unlock in DDP */
> - if (ONENAND_IS_DDP(this)) {
> + if (ONENAND_IS_DDP(this) && !FLEXONENAND(this)) {
> /* All blocks on another chip */
> ofs = this->chipsize >> 1;
> len = this->chipsize >> 1;
> @@ -1906,6 +2152,14 @@ static void onenand_check_features(struct mtd_info *mtd)
> break;
> }
>
> + if (ONENAND_IS_MLC(this))
> + this->options &= ~ONENAND_HAS_2PLANE;
> +
> + if (FLEXONENAND(this)) {
> + this->options &= ~ONENAND_HAS_CONT_LOCK;
> + this->options |= ONENAND_HAS_UNLOCK_ALL;
> + }
> +
> if (this->options & ONENAND_HAS_CONT_LOCK)
> printk(KERN_DEBUG "Lock scheme is Continuous Lock\n");
> if (this->options & ONENAND_HAS_UNLOCK_ALL)
> @@ -1922,16 +2176,18 @@ static void onenand_check_features(struct mtd_info *mtd)
> */
> char *onenand_print_device_info(int device, int version)
> {
> - int vcc, demuxed, ddp, density;
> + int vcc, demuxed, ddp, density, flexonenand;
> char *dev_info = malloc(80);
> char *p = dev_info;
>
> vcc = device & ONENAND_DEVICE_VCC_MASK;
> demuxed = device & ONENAND_DEVICE_IS_DEMUX;
> ddp = device & ONENAND_DEVICE_IS_DDP;
> - density = device >> ONENAND_DEVICE_DENSITY_SHIFT;
> - p += sprintf(dev_info, "%sOneNAND%s %dMB %sV 16-bit (0x%02x)",
> + density = onenand_get_density(device);
> + flexonenand = device & DEVICE_IS_FLEXONENAND;
> + p += sprintf(dev_info, "%s%sOneNAND%s %dMB %sV 16-bit (0x%02x)",
> demuxed ? "" : "Muxed ",
> + flexonenand ? "Flex-" : "",
> ddp ? "(DDP)" : "",
> (16 << density), vcc ? "2.65/3.3" : "1.8", device);
>
> @@ -1957,7 +2213,7 @@ static int onenand_check_maf(int manuf)
> char *name;
> int i;
>
> - for (i = 0; size; i++)
> + for (i = 0; i < size; i++)
> if (manuf == onenand_manuf_ids[i].id)
> break;
>
> @@ -1974,6 +2230,258 @@ static int onenand_check_maf(int manuf)
> }
>
> /**
> +* flexonenand_get_boundary - Reads the SLC boundary
> +* @param onenand_info - onenand info structure
> +*
> +* Fill up boundary[] field in onenand_chip
> +**/
> +static int flexonenand_get_boundary(struct mtd_info *mtd)
> +{
> + struct onenand_chip *this = mtd->priv;
> + unsigned int die, bdry;
> + int ret, syscfg, locked;
> +
> + /* Disable ECC */
> + syscfg = this->read_word(this->base + ONENAND_REG_SYS_CFG1);
> + this->write_word((syscfg | 0x0100), this->base + ONENAND_REG_SYS_CFG1);
> +
> + for (die = 0; die < this->dies; die++) {
> + this->command(mtd, FLEXONENAND_CMD_PI_ACCESS, die, 0);
> + this->wait(mtd, FL_SYNCING);
> +
> + this->command(mtd, FLEXONENAND_CMD_READ_PI, die, 0);
> + ret = this->wait(mtd, FL_READING);
> +
> + bdry = this->read_word(this->base + ONENAND_DATARAM);
> + if ((bdry >> FLEXONENAND_PI_UNLOCK_SHIFT) == 3)
> + locked = 0;
> + else
> + locked = 1;
> + this->boundary[die] = bdry & FLEXONENAND_PI_MASK;
> +
> + this->command(mtd, ONENAND_CMD_RESET, 0, 0);
> + ret = this->wait(mtd, FL_RESETING);
> +
> + printk(KERN_INFO "Die %d boundary: %d%s\n", die,
> + this->boundary[die], locked ? "(Locked)" : "(Unlocked)");
> + }
> +
> + /* Enable ECC */
> + this->write_word(syscfg, this->base + ONENAND_REG_SYS_CFG1);
> + return 0;
> +}
> +
> +/**
> + * flexonenand_get_size - Fill up fields in onenand_chip and mtd_info
> + * boundary[], diesize[], mtd->size, mtd->erasesize,
> + * mtd->eraseregions
> + * @param mtd - MTD device structure
> + */
> +static void flexonenand_get_size(struct mtd_info *mtd)
> +{
> + struct onenand_chip *this = mtd->priv;
> + int die, ofs, i, eraseshift, density;
> + int blksperdie, maxbdry;
> +
> + density = onenand_get_density(this->device_id);
> + blksperdie = ((16 << density) << 20) >> (this->erase_shift);
Can this overflow? Might be better as:
blksperdie = 16 << (density + 20 - this->erase_shift);
> + die = ofs = 0;
> + i = -1;
> + for (; die < this->dies; die++) {
> + if (!die || this->boundary[die-1] != maxbdry) {
> + i++;
> + mtd->eraseregions[i].offset = ofs;
> + mtd->eraseregions[i].erasesize = 1 << eraseshift;
> + mtd->eraseregions[i].numblocks =
> + this->boundary[die] + 1;
> + ofs += mtd->eraseregions[i].numblocks << eraseshift;
> + eraseshift++;
> + } else {
> + mtd->numeraseregions -= 1;
> + mtd->eraseregions[i].numblocks +=
> + this->boundary[die] + 1;
> + ofs += (this->boundary[die] + 1) << (eraseshift - 1);
> + }
> + if (this->boundary[die] != maxbdry) {
> + i++;
> + mtd->eraseregions[i].offset = ofs;
> + mtd->eraseregions[i].erasesize = 1 << eraseshift;
> + mtd->eraseregions[i].numblocks = maxbdry ^
> + this->boundary[die];
> + ofs += mtd->eraseregions[i].numblocks << eraseshift;
> + eraseshift--;
> + } else
> + mtd->numeraseregions -= 1;
> + }
Can the above be simplified, or at least documented? I'm having a hard
time following it.
-Scott
More information about the U-Boot
mailing list