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]

 



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.

-- 
Regards,
Vimal Singh
--
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