Re: [PATCH-v5 2/4] OMAP3: Add support for NAND on ZOOM2/LDP boards

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Vimal Singh <vimal.newwork@xxxxxxxxx> [091129 22:11]:
> On Mon, Nov 23, 2009 at 11:15 PM, Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> > * Vimal Singh <vimal.newwork@xxxxxxxxx> [091119 00:52]:
> >> On Wed, Nov 18, 2009 at 10:37 PM, Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> >> > * Vimal Singh <vimal.newwork@xxxxxxxxx> [091118 06:38]:
> >> >> Tony,
> >> >>
> >> >> On Fri, Nov 13, 2009 at 2:14 AM, Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> >> >> > * Vimal Singh <vimal.newwork@xxxxxxxxx> [091110 02:08]:
> >> >> >> From 6f535d7128ca392458dd0cb31d138cda84747c06 Mon Sep 17 00:00:00 2001
> >> >> >> From: Vimal Singh <vimalsingh@xxxxxx>
> >> >> >> Date: Tue, 10 Nov 2009 11:42:45 +0530
> >> >> >> Subject: [PATCH] OMAP3: Add support for NAND on ZOOM2/LDP boards
> >> >>
> >> >> [...]
> >> >>
> >> >> >> +static int omap_ldp_nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> >> >> >> +{
> >> >> >> +     int ret = 0;
> >> >> >> +     int chipnr;
> >> >> >> +     int status;
> >> >> >> +     unsigned long page;
> >> >> >> +     struct nand_chip *this = mtd->priv;
> >> >> >> +     printk(KERN_INFO "nand_unlock: start: %08x, length: %d!\n",
> >> >> >> +                     (int)ofs, (int)len);
> >> >> >> +
> >> >> >> +     /* select the NAND device */
> >> >> >> +     chipnr = (int)(ofs >> this->chip_shift);
> >> >> >> +     this->select_chip(mtd, chipnr);
> >> >> >> +     /* check the WP bit */
> >> >> >> +     this->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
> >> >> >> +     if ((this->read_byte(mtd) & 0x80) == 0) {
> >> >> >> +             printk(KERN_ERR "nand_unlock: Device is write protected!\n");
> >> >> >> +             ret = -EINVAL;
> >> >> >> +             goto out;
> >> >> >> +     }
> >> >> >> +
> >> >> >> +     if ((ofs & (mtd->writesize - 1)) != 0) {
> >> >> >> +             printk(KERN_ERR "nand_unlock: Start address must be"
> >> >> >> +                             "beginning of nand page!\n");
> >> >> >> +             ret = -EINVAL;
> >> >> >> +             goto out;
> >> >> >> +     }
> >> >> >> +
> >> >> >> +     if (len == 0 || (len & (mtd->writesize - 1)) != 0) {
> >> >> >> +             printk(KERN_ERR "nand_unlock: Length must be a multiple of "
> >> >> >> +                             "nand page size!\n");
> >> >> >> +             ret = -EINVAL;
> >> >> >> +             goto out;
> >> >> >> +     }
> >> >> >> +
> >> >> >> +     /* submit address of first page to unlock */
> >> >> >> +     page = (unsigned long)(ofs >> this->page_shift);
> >> >> >> +     this->cmdfunc(mtd, NAND_CMD_UNLOCK1, -1, page & this->pagemask);
> >> >> >> +
> >> >> >> +     /* submit ADDRESS of LAST page to unlock */
> >> >> >> +     page += (unsigned long)((ofs + len) >> this->page_shift) ;
> >> >> >> +     this->cmdfunc(mtd, NAND_CMD_UNLOCK2, -1, page & this->pagemask);
> >> >> >> +
> >> >> >> +     /* call wait ready function */
> >> >> >> +     status = this->waitfunc(mtd, this);
> >> >> >> +     udelay(1000);
> >> >> >> +     /* see if device thinks it succeeded */
> >> >> >> +     if (status & 0x01) {
> >> >> >> +             /* there was an error */
> >> >> >> +             printk(KERN_ERR "nand_unlock: error status =0x%08x ", status);
> >> >> >> +             ret = -EIO;
> >> >> >> +             goto out;
> >> >> >> +     }
> >> >> >> +
> >> >> >> + out:
> >> >> >> +     /* de-select the NAND device */
> >> >> >> +     this->select_chip(mtd, -1);
> >> >> >> +     return ret;
> >> >> >> +}
> >> >> >
> >> >> > Isn't the unlocking generic to the NAND device driver? Or is it somehow board
> >> >> > specific?
> >> >>
> >> >> Yes, zoom2 boards come up with whole NAND locked, whereas it is not
> >> >> case for SDP boards.
> >> >
> >> > But the procedure should be done under drivers/mtd I believe using some
> >> > standard tools.
> >>
> >> OK, I'll take this discussion to mtd mailing list. For now I'll remove
> >> this from here.
> >
> > OK, I'd assume there's some standard way to handle this for all NOR
> > drivers.
> >
> >> >
> >> >> >
> >> >> >
> >> >> >> +static struct mtd_partition ldp_nand_partitions[] = {
> >> >> >> +     /* All the partition sizes are listed in terms of NAND block size */
> >> >> >> +     {
> >> >> >> +             .name           = "X-Loader-NAND",
> >> >> >> +             .offset         = 0,
> >> >> >> +             .size           = 4 * (64 * 2048),      /* 512KB, 0x80000 */
> >> >> >> +             .mask_flags     = MTD_WRITEABLE,        /* force read-only */
> >> >> >> +     },
> >> >> >> +     {
> >> >> >> +             .name           = "U-Boot-NAND",
> >> >> >> +             .offset         = MTDPART_OFS_APPEND,   /* Offset = 0x80000 */
> >> >> >> +             .size           = 10 * (64 * 2048),     /* 1.25MB, 0x140000 */
> >> >> >> +             .mask_flags     = MTD_WRITEABLE,        /* force read-only */
> >> >> >> +     },
> >> >> >> +     {
> >> >> >> +             .name           = "Boot Env-NAND",
> >> >> >> +             .offset         = MTDPART_OFS_APPEND,   /* Offset = 0x1c0000 */
> >> >> >> +             .size           = 2 * (64 * 2048),      /* 256KB, 0x40000 */
> >> >> >> +     },
> >> >> >> +     {
> >> >> >> +             .name           = "Kernel-NAND",
> >> >> >> +             .offset         = MTDPART_OFS_APPEND,   /* Offset = 0x0200000*/
> >> >> >> +             .size           = 240 * (64 * 2048),    /* 30M, 0x1E00000 */
> >> >> >> +     },
> >> >> >> +#ifdef CONFIG_MACH_OMAP_ZOOM2
> >> >> >> +     {
> >> >> >> +             .name           = "system",
> >> >> >> +             .offset         = MTDPART_OFS_APPEND,   /* Offset = 0x2000000 */
> >> >> >> +             .size           = 1280 * (64 * 2048),   /* 160M, 0xA000000 */
> >> >> >> +     },
> >> >> >> +     {
> >> >> >> +             .name           = "userdata",
> >> >> >> +             .offset         = MTDPART_OFS_APPEND,   /* Offset = 0xC000000 */
> >> >> >> +             .size           = 256 * (64 * 2048),    /* 32M, 0x2000000 */
> >> >> >> +     },
> >> >> >> +     {
> >> >> >> +             .name           = "cache",
> >> >> >> +             .offset         = MTDPART_OFS_APPEND,   /* Offset = 0xE000000 */
> >> >> >> +             .size           = 256 * (64 * 2048),    /* 32M, 0x2000000 */
> >> >> >> +     },
> >> >> >> +#else
> >> >> >> +     {
> >> >> >> +             .name           = "File System - NAND",
> >> >> >> +             .offset         = MTDPART_OFS_APPEND,   /* Offset = 0x2000000 */
> >> >> >> +             .size           = MTDPART_SIZ_FULL,     /* 96MB, 0x6000000 */
> >> >> >> +     },
> >> >> >> +#endif
> >> >> >
> >> >> > Please remove the ifdefs, you should be able to compile in all mach-omap2
> >> >> > boards into the same kernel binary. You should set the size dynamically as
> >> >> > needed.
> >> >>
> >> >> We can always provide partitioning information from cmdline
> >> >> (mtdparts:). Which will anyway have higher precedence than this. Here
> >> >> are trying provide default static partitions. Which IMHO is fine.
> >> >>
> >> >> see this too:
> >> >> http://marc.info/?l=linux-omap&m=125178914327011&w=2
> >> >
> >> > No way we're adding any more of these ifdef else hacks.
> >> >
> >> > The whole idea of the platform_data is to pass the board specific
> >> > configuration to the driver. The driver should be generic.
> >> >
> >>
> >> Hmmm... In that case I'll remove extra partition informations (for
> >> zoom2 and sdp) and will have very only few common partitions here.
> >> Something like this:
> >>
> >> +static struct mtd_partition ldp_nand_partitions[] = {
> >> +       /* All the partition sizes are listed in terms of NAND block size */
> >> +       {
> >> +               .name           = "X-Loader-NAND",
> >> +               .offset         = 0,
> >> +               .size           = 4 * (64 * 2048),      /* 512KB, 0x80000 */
> >> +               .mask_flags     = MTD_WRITEABLE,        /* force read-only */
> >> +       },
> >> +       {
> >> +               .name           = "U-Boot-NAND",
> >> +               .offset         = MTDPART_OFS_APPEND,   /* Offset = 0x80000 */
> >> +               .size           = 10 * (64 * 2048),     /* 1.25MB, 0x140000 */
> >> +               .mask_flags     = MTD_WRITEABLE,        /* force read-only */
> >> +       },
> >> +       {
> >> +               .name           = "Boot Env-NAND",
> >> +               .offset         = MTDPART_OFS_APPEND,   /* Offset = 0x1c0000 */
> >> +               .size           = 2 * (64 * 2048),      /* 256KB, 0x40000 */
> >> +       },
> >> +       {
> >> +               .name           = "Kernel-NAND",
> >> +               .offset         = MTDPART_OFS_APPEND,   /* Offset = 0x0200000*/
> >> +               .size           = 240 * (64 * 2048),    /* 30M, 0x1E00000 */
> >> +       },
> >> +       {
> >> +               .name           = "File System - NAND",
> >> +               .offset         = MTDPART_OFS_APPEND,   /* Offset = 0x2000000 */
> >> +               .size           = MTDPART_SIZ_FULL,
> >> +       },
> >> +};
> >>
> >>
> >> And then, as I said earlier, if someone want different partitions, he
> >> can always pass his partition information from cmdline (bootargs).
> >
> > Why don't you just set ldp_nand_paritions[X].size = something during init
> > based on the machine_is_XXX()?
> 
> Rather I am thinking of moving partition definitions to core board
> file, and pass a pointer to 'board-*-flash.c'. How about this?
> This way any board can have its own partition information specified.

Yeah that would be the most flexible way of doing it. You should also
pass the CS number and size.

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux