On Mon, Aug 09, 2010 at 04:43:58PM -0400, Ben Gardiner wrote: > diff --git a/common/cmd_mtdparts.c b/common/cmd_mtdparts.c > index 772ad54..500a38e 100644 > --- a/common/cmd_mtdparts.c > +++ b/common/cmd_mtdparts.c > @@ -1215,18 +1215,65 @@ static int generate_mtdparts_save(char *buf, u32 > buflen) > return ret; > } > > +#if defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES) > /** > - * Format and print out a partition list for each device from global device > - * list. > + * Get the net size (w/o bad blocks) of the given partition. > + * > + * @param mtd the mtd info > + * @param part the partition > + * @return the calculated net size of this partition > */ > -static void list_partitions(void) > +static u32 net_part_size(struct mtd_info *mtd, struct part_info *part)
Don't assume partition size fits in 32 bits. part->size is uint64_t. > +{ > + if (mtd->block_isbad) { > + u32 i, bb_delta = 0; > + > + for (i = 0; i < part->size; i += mtd->erasesize) { > + if (mtd->block_isbad(mtd, part->offset + i)) > + bb_delta += mtd->erasesize; > + } > + > + return part->size - bb_delta; Seems like it'd be slightly simpler to just count up the good blocks, rather than count the bad blocks and subtract. > + } else { > + return part->size; > + } It's usually more readable to do this: if (can't do this) return; do this; than this if (can do this) do this; else don't; When "do this" is more than a line or two, and there's nothing else to be done in the function afterward. > +} > +#endif > + > +static void print_partition_table(void) > { > struct list_head *dentry, *pentry; > struct part_info *part; > struct mtd_device *dev; > int part_num; > > - debug("\n---list_partitions---\n"); > +#if defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES) > + list_for_each(dentry, &devices) { > + struct mtd_info *mtd; > + > + dev = list_entry(dentry, struct mtd_device, link); > + printf("\ndevice %s%d <%s>, # parts = %d\n", > + MTD_DEV_TYPE(dev->id->type), dev->id->num, > + dev->id->mtd_id, dev->num_parts); > + printf(" #: name\t\tsize\t\tnet size\toffset\t\tmask_flags\n"); > + > + if (get_mtd_info(dev->id->type, dev->id->num, &mtd)) > + return; > + > + /* list partitions for given device */ > + part_num = 0; > + list_for_each(pentry, &dev->parts) { > + u32 net_size; > + char *size_note; > + > + part = list_entry(pentry, struct part_info, link); > + net_size = net_part_size(mtd, part); > + size_note = part->size == net_size ? " " : " (!)"; > + printf("%2d: %-20s0x%08x\t0x%08x%s\t0x%08x\t%d\n", > + part_num, part->name, part->size, > + net_size, size_note, part->offset, > + part->mask_flags); > +#else /* !defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES) */ > list_for_each(dentry, &devices) { > dev = list_entry(dentry, struct mtd_device, link); > printf("\ndevice %s%d <%s>, # parts = %d\n", > @@ -1241,12 +1288,25 @@ static void list_partitions(void) > printf("%2d: %-20s0x%08x\t0x%08x\t%d\n", > part_num, part->name, part->size, > part->offset, part->mask_flags); > - > +#endif /* defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES) */ Is there any way you could share more of this between the two branches? -Scott _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot