Patchwork OMAP3: Beagle: NAND: Specifying partition offsets directly.

login
register
mail settings
Submitter Hrishikesh Bhandiwad
Date July 25, 2011, 1:07 p.m.
Message ID <1311599243-875-1-git-send-email-hrishikesh.b@ti.com>
Download mbox | patch
Permalink /patch/106660/
State New
Headers show

Comments

Hrishikesh Bhandiwad - July 25, 2011, 1:07 p.m.
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@ti.com>
---
 arch/arm/mach-omap2/board-omap3beagle.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Felipe Balbi - July 25, 2011, 1:14 p.m.
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@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.
Felipe Balbi - July 25, 2011, 1:19 p.m.
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@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 ??

Patch

diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index 7f21d24..09f7571 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -140,23 +140,23 @@  static struct mtd_partition omap3beagle_nand_partitions[] = {
 	},
 	{
 		.name		= "U-Boot",
-		.offset		= MTDPART_OFS_APPEND,	/* Offset = 0x80000 */
+		.offset		= 0x80000
 		.size		= 15 * NAND_BLOCK_SIZE,
 		.mask_flags	= MTD_WRITEABLE,	/* force read-only */
 	},
 	{
 		.name		= "U-Boot Env",
-		.offset		= MTDPART_OFS_APPEND,	/* Offset = 0x260000 */
+		.offset		= 0x260000
 		.size		= 1 * NAND_BLOCK_SIZE,
 	},
 	{
 		.name		= "Kernel",
-		.offset		= MTDPART_OFS_APPEND,	/* Offset = 0x280000 */
+		.offset		= 0x280000
 		.size		= 32 * NAND_BLOCK_SIZE,
 	},
 	{
 		.name		= "File System",
-		.offset		= MTDPART_OFS_APPEND,	/* Offset = 0x680000 */
+		.offset		= 0x680000
 		.size		= MTDPART_SIZ_FULL,
 	},
 };