From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heiko Schocher <hs@denx.de>
Date: Wed, 24 Jul 2013 06:30:55 +0200
Subject: [U-Boot] [PATCH v4] dfu, nand,
 ubi: add partubi alt settings for updating ubi partition
In-Reply-To: <1374621757.15592.43@snotra>
References: <1374621757.15592.43@snotra>
Message-ID: <51EF587F.30206@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hello Scott,

Am 24.07.2013 01:22, schrieb Scott Wood:
> On 07/18/2013 11:32:14 PM, Heiko Schocher wrote:
>> +static int dfu_flush_medium_nand(struct dfu_entity *dfu)
>> +{
>> + int ret = 0;
>> +
>> + /* in case of ubi partition, erase rest of the partition */
>> + if (dfu->data.nand.ubi) {
>> + nand_info_t *nand;
>> + nand_erase_options_t opts;
>> +
>> + if (nand_curr_device < 0 ||
>> + nand_curr_device >= CONFIG_SYS_MAX_NAND_DEVICE ||
>> + !nand_info[nand_curr_device].name) {
>> + printf("%s: invalid nand device\n", __func__);
>> + return -1;
>> + }
>> +
>> + nand = &nand_info[nand_curr_device];
>> +
>> + memset(&opts, 0, sizeof(opts));
>> + opts.offset = dfu->data.nand.start + dfu->offset +
>> + dfu->bad_skip;
>> + opts.length = dfu->data.nand.start +
>> + dfu->data.nand.size - opts.offset;
>
> opts.length is equivalent to dfu->data.nand.size - dfu->offset - dfu->bad_skip. Is this correct? dfu->data.nand.size includes dfu->offset, but dfu->data.nand.start doesn't?

Yes, it is correct!

I could not parse "dfu->data.nand.size includes dfu->offset, but
dfu->data.nand.start doesn't" ... What do you mean?

I need to calculate "opts-offset" and "opts.length" for nand_erase_opts():
opts.offset contains the starting sector where the erase should begin
      dfu->data.nand.start is equal to the starting sector of the
      partition, so this is needed here + dfu->offset + dfu->bad_skip
      and we are at our starting point.

opts.length contains the count of sectors to erase.
      I just did here end_sector - start_sector with:
      end_sector = dfu->data.nand.start + dfu->data.nand.size
      start_sector = opts.offset

Maybe my calculation of opts.length is confusing, and I should
use "dfu->data.nand.size - dfu->offset - dfu->bad_skip" ?

>> @@ -185,7 +217,32 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
>>
>> dfu->data.nand.start = pi->offset;
>> + } else if (!strcmp(st, "partubi")) {
>> dfu->data.nand.size = pi->size;
>> + char mtd_id[32];
>> + struct mtd_device *mtd_dev;
>> + u8 part_num;
>> + struct part_info *pi;
>>
>> + dfu->layout = DFU_RAW_ADDR;
>> +
>> + dev = simple_strtoul(s, &s, 10);
>> + s++;
>> + part = simple_strtoul(s, &s, 10);
>> +
>> + sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
>> + printf("using id '%s'\n", mtd_id);
>> +
>> + mtdparts_init();
>> +
>> + ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num, &pi);
>> + if (ret != 0) {
>> + printf("Could not locate '%s'\n", mtd_id);
>> + return -1;
>> + }
>> +
>> + dfu->data.nand.start = pi->offset;
>> + dfu->data.nand.size = pi->size;
>> + dfu->data.nand.ubi = 1;
>
> Please don't duplicate all of this just to set one flag at the end.

You are right! Fixed!

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany