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@xxxxxx>
>
> 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
Attachment:
signature.asc
Description: Digital signature
