Hi

On Wed, Jul 27, 2016 at 2:57 PM, Julian Scheel julian@jusst.de wrote:

Hi Michael,

On 08.06.2016 10:18, Michael Trimarchi wrote:

...

This patch try to parse name=userdata,size=-,uuid=${uuid_gpt_userdata};

gpt mmc write 0 $partitions gpt mmc verify 0 $partitions

Signed-off-by: Michael Trimarchi michael@amarulasolutions.com

cmd/gpt.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/cmd/gpt.c b/cmd/gpt.c index 8ffaef3..3d9706b 100644 --- a/cmd/gpt.c +++ b/cmd/gpt.c @@ -181,6 +181,7 @@ static int set_gpt_info(struct blk_desc *dev_desc, disk_partition_t *parts; int errno = 0; uint64_t size_ll, start_ll;

•   lbaint_t offset = 0;
  
    debug("%s:  lba num: 0x%x %d\n", __func__,
          (unsigned int)dev_desc->lba, (unsigned int)dev_desc->lba);
  

@@ -296,8 +297,14 @@ static int set_gpt_info(struct blk_desc *dev_desc, } if (extract_env(val, &p)) p = val;

•           size_ll = ustrtoull(p, &p, 0);
  

•           parts[i].size = lldiv(size_ll, dev_desc->blksz);
  

•           if ((strcmp(p, "-") == 0)) {
  

•                   /* remove first usable lba and last block */
  

•                   parts[i].size = dev_desc->lba - 34  - 1 - offset;
  

Looking into part_efi.c the last_usable block is dev_desc->lba - 34 and the first usable block is 34. So 34 needs to be substracted twice, otherwise you hit the "Partitions layout exceds disk size" error case in part_efi.c

...

•           } else {
  

•                   size_ll = ustrtoull(p, &p, 0);
  

•                   parts[i].size = lldiv(size_ll, dev_desc->blksz);
  

•           }
  

•           free(val);
  
            /* start address */
  

@@ -310,6 +317,8 @@ static int set_gpt_info(struct blk_desc *dev_desc, free(val); }

•           offset += parts[i].size + parts[i].start;
  

This appears to be wrong. We have two cases here:

a) start is not specified for parition i In this case the code works as parts[i].start is 0 and size accumulates to the proper offsets

b) start is specified for a partition i, example table:

part[0].size = 10

part[1].size = 10 part[1].start = 10

part[2].size = 10

This table should end up in the same table as if part[1].start = 10 would have been omited. In fact it will lead to: part[0] = 0-10 part[1] = 10-20 part[2] = 30-40

So it introduced a gap, because start is assumed to be relative to previous offset, which is not the case.

I think the proper handling would be: if (parts[i].start) offset = parts[i].size + parts[i].start; else offset += parts[i].size;

...

•           /* bootable */
            if (found_key(tok, "bootable"))
                    parts[i].bootable = 1;
  
  

While preparing a patch to fix the previously mentioned issues, so that gpt writing becomes usable again, I started to wonder why you added this patch at all. In fact the usecase you describe in the commit message did work perfectly without this patch, because part_efi.c automatically scales a partition without given size to the available space, in case it is the last partition. (See: http://git.denx.de/?p=u-boot.git;a=blob;f=disk/part_efi.c;h=01f71bee79e2656a...) So imho this patch should simply be reverted or did I miss something important?

The idea was to properly handle the verification part in an easy way, to make command consistent. Your fix look correct. I have some try on my side and I miss this point. I'm working on imx7d platform and and fastboot on latest uboot but I was busy more on other part.

Michael

Regards, Julian