[PATCH] OMAP3: Beagle: NAND: Specifying partition offsets directly.

[Felipe Balbi]

Hi again,

On Mon, Jul 25, 2011 at 04:14:06PM +0300, Felipe Balbi wrote:
> On Mon, Jul 25, 2011 at 06:37:23PM +0530, Hrishikesh Bhandiwad wrote:
> > In the nand partition table specifying the offset addresses
> > directly instead of using the macro MTDPART_OFS_APPEND to gain
> > runtime efficiency while nand initialization.
> > MTDPART_OFS_APPEND has the value (-1) ,if assigned to offset,
> > a runtime calculation of actual offset happens each time nand is
> > initialized [ Refer: drivers/mtd/mtdpart.c: allocate_partition()].
> > To avoid this , specify actual offset.
> > 
> > Signed-off-by: Hrishikesh Bhandiwad <hrishikesh.b at ti.com>
> 
> did you measure any significant gains when doing this ? Also, why only
> beagle ? why not all boards already ? Measurment data of how long it
> takes for the runtime calculation of the offset to complete would be
> nice to see.

I was just looking at the code and the offset calculation will always be
done regardless of you using the direct offset or the MTDPART_OFS_APEND.
The only thing you avoid by setting it directly is a branch.

See here:

648 int add_mtd_partitions(struct mtd_info *master,
649                        const struct mtd_partition *parts,
650                        int nbparts)
651 {
652         struct mtd_part *slave;
653         uint64_t cur_offset = 0;
654         int i;
655 
656         printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
657 
658         for (i = 0; i < nbparts; i++) {
659                 slave = allocate_partition(master, parts + i, i, cur_offset);
660                 if (IS_ERR(slave))
661                         return PTR_ERR(slave);
662 
663                 mutex_lock(&mtd_partitions_mutex);
664                 list_add(&slave->list, &mtd_partitions);
665                 mutex_unlock(&mtd_partitions_mutex);
666 
667                 add_mtd_device(&slave->mtd);
668 
669                 cur_offset = slave->offset + slave->mtd.size;
670         }
671 
672         return 0;
673 }

regardless of you passing MTDPART_OFS_APEND or direct slave->offset,
cur_ofsset will be calculated, and that thing looks really cheap to
execute to me. The only thing you're avoid is:

467         slave->offset = part->offset;
468 
469         if (slave->offset == MTDPART_OFS_APPEND)
470                 slave->offset = cur_offset;

the if at line 469. Am I missing something ??

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20110725/c5233151/attachment.sig>