[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