[U-Boot] [PATCH v4 3/4] mtdparts: add new sub-command "spread"

Ben Gardiner
2010-08-27T13:51:50Z

On Thu, Aug 26, 2010 at 5:12 PM, Scott Wood <scottwood@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.