[U-Boot] [PATCH v4 3/4] mtdparts: add new sub-command "spread"
Ben Gardiner
bengardiner at nanometrics.ca
Fri Aug 27 15:51:50 CEST 2010
On Thu, Aug 26, 2010 at 5:12 PM, Scott Wood <scottwood at freescale.com> wrote:
> On Mon, Aug 09, 2010 at 04:43:59PM -0400, Ben Gardiner wrote:
>> +static void spread_partition(struct mtd_info *mtd, struct part_info *part,
>> + u32 *next_offset)
>
> As in patch 2, change u32 to uint64_t.
Ok.
>> +{
>> + if (!mtd->block_isbad)
>> + goto out;
>> +
>> + u32 i, bb_delta = 0;
>> +
>> + for (i = part->offset; i - bb_delta < part->offset + part->size;
>> + i += mtd->erasesize) {
>> + if (mtd->block_isbad(mtd, i))
>> + bb_delta += mtd->erasesize;
>> + }
>> +
>> + /*
>> + * Absorb bad blocks immeadiately following this
>> + * partition also into the partition, such that
>> + * the next partition starts with a good block.
>> + */
>> + while (i < mtd->size && mtd->block_isbad(mtd, i)) {
>> + bb_delta += mtd->erasesize;
>> + i += mtd->erasesize;
>> + }
>
> Could this be refactored with get_len_incl_bad()? It should return both the
> updated length and a flag indicating whether it was truncated.
Yes, I think so. Good point.
>> + debug("spread_partitions: device = %s%d, partition %d ="
>> + " (%s) 0x%08x at 0x%08x\n",
>> + MTD_DEV_TYPE(dev->id->type), dev->id->num,
>> + part_num, part->name, part->size,
>> + part->offset);
>
> Why the extra indent on that last line?
>
> IMHO, it's also nicer to line up continuation lines like this:
>
> debug("spread_partitions..."
> " (%s) ..."
> MTD_DEV...
> part_num...
> part->offset);
Right. I think I forgot also about this formatting requirement which
you pointed out in the env.oob review. I'll get it right soon enough.
Best Regards,
Ben Gardiner
---
Nanometrics Inc.
http://www.nanometrics.ca
More information about the U-Boot
mailing list