[U-Boot] [PATCH v1 1/1] usb: gadget: fastboot: Add fastboot erase

[Steve Rae <srae@broadcom.com>]

On 15-02-12 12:29 AM, Dileep Katta wrote:
> Hi Steve,
>
> On 11 February 2015 at 05:25, Steve Rae <srae@broadcom.com> wrote:
>>
>> Hi, Dileep
>>
>>
>> On 15-02-10 12:49 AM, Dileep Katta wrote:
>>>
>>> Adds the fastboot erase functionality, to erase a partition
>>> specified by name. The erase is performed based on erase group size,
>>> to avoid erasing other partitions. The start address and the size
>>> is aligned to the erase group size for this.
>>>
>>> Currently only supports erasing from eMMC.
>>>
>>> Signed-off-by: Dileep Katta <dileep.katta@linaro.org>
>>> ---
>>> Note: The changes are on top of oem command support added by robh at kernel.org
>>>
>>>    common/fb_mmc.c                 | 58 +++++++++++++++++++++++++++++++++++++++++
>>>    drivers/usb/gadget/f_fastboot.c | 23 ++++++++++++++++
>>>    include/fb_mmc.h                |  1 +
>>>    3 files changed, 82 insertions(+)
>>>
>>> diff --git a/common/fb_mmc.c b/common/fb_mmc.c
>>> index 6ea3938..3911989 100644
>>> --- a/common/fb_mmc.c
>>> +++ b/common/fb_mmc.c
>>> @@ -10,6 +10,7 @@
>>>    #include <part.h>
>>>    #include <aboot.h>
>>>    #include <sparse_format.h>
>>> +#include <mmc.h>
>>>
>>>    #ifndef CONFIG_FASTBOOT_GPT_NAME
>>>    #define CONFIG_FASTBOOT_GPT_NAME GPT_ENTRY_NAME
>>> @@ -110,3 +111,60 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
>>>                  write_raw_image(dev_desc, &info, cmd, download_buffer,
>>>                                  download_bytes);
>>>    }
>>> +
>>> +void fb_mmc_erase(const char *cmd, char *response)
>>> +{
>>> +       int ret;
>>> +       block_dev_desc_t *dev_desc;
>>> +       disk_partition_t info;
>>> +       lbaint_t blks, blks_start, blks_size, grp_size;
>>> +       struct mmc *mmc = find_mmc_device(CONFIG_FASTBOOT_FLASH_MMC_DEV);
>>> +
>>> +       if (mmc == NULL) {
>>> +               error("invalid mmc device\n");
>>
>> no newline with error()
> Will remove
>>
>>> +               fastboot_fail("invalid mmc device");
>>> +               return;
>>> +       }
>>> +
>>> +       /* initialize the response buffer */
>>> +       response_str = response;
>>> +
>>> +       dev_desc = get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
>>> +       if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
>>> +               error("invalid mmc device\n");
>>
>> no newline with error()
> Will remove
>>
>>
>>> +               fastboot_fail("invalid mmc device");
>>> +               return;
>>> +       }
>>> +
>>> +       ret = get_partition_info_efi_by_name(dev_desc, cmd, &info);
>>> +       if (ret) {
>>> +               error("cannot find partition: '%s'\n", cmd);
>>
>> no newline with error()
> Will remove
>>
>>> +               fastboot_fail("cannot find partition");
>>> +               return;
>>> +       }
>>> +
>>> +       puts("Erasing partition\n");
>>> +
>>> +       /* Align blocks to erase group size to avoid erasing other partitions */
>>> +       grp_size = mmc->erase_grp_size;
>>> +       blks_start = (info.start + grp_size - 1) & ~(grp_size - 1);
>>> +       if (info.size >= grp_size)
>>> +               blks_size = (info.size - (blks_start - info.start)) &
>>> +                               (~(grp_size - 1));
>>> +       else
>>> +               blks_size = 0;
>>
>>
>> Is this logic correct??? Isn't the "erase_grp_size" in bytes? and the info.start & info.size in LBA's?
> Yes, the math will take care of it. Ref: mmc_berase()

So, I have a partition:

   2     0x00000300      0x000003ff      "test"
         attrs:  0x0000000000000000
         type:   9e312af1-18fe-fa41-45f3-37b3bb1d1061
         guid:   18a5d0db-d23a-aac1-0d4c-692c7ba9ab1c

and 'fastboot erase test' produces:
   Erasing blocks 1024 to 1024 due to alignment
   ........ erased 0 bytes from 'test'
which is not correct!

Furthermore, doesn't the mmc_berase() already handle the 
"erase_grp_size"....
If I remove your alignment logic, then it produces:
   Erasing blocks 768 to 1024 due to alignment

   Caution! Your devices Erase group is 0x400
   The erase range would be change to 0x0~0x7ff

   ........ erased 131072 bytes from 'test'
which looks correct (except for the "Caution" message)....

Thanks, Steve

>>
>>
>>> +
>>> +       printf("Erasing blocks " LBAFU " to " LBAFU " due to alignment\n",
>>> +              blks_start, blks_start + blks_size);
>>> +
>>> +       blks = dev_desc->block_erase(dev_desc->dev, blks_start, blks_size);
>>> +       if (blks != blks_size) {
>>> +               error("failed erasing from device %d\n", dev_desc->dev);
>>
>> no newline with error()
> Will remove
>>
>>> +               fastboot_fail("failed erasing from device");
>>> +               return;
>>> +       }
>>> +
>>> +       printf("........ erased " LBAFU " bytes from '%s'\n",
>>> +              blks_size * info.blksz, cmd);
>>> +       fastboot_okay("");
>>> +}
>>> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
>>> index f7d84bf..a8d8205 100644
>>> --- a/drivers/usb/gadget/f_fastboot.c
>>> +++ b/drivers/usb/gadget/f_fastboot.c
>>> @@ -535,6 +535,26 @@ static void cb_oem(struct usb_ep *ep, struct usb_request *req)
>>>          }
>>>    }
>>>
>>> +static void cb_erase(struct usb_ep *ep, struct usb_request *req)
>>> +{
>>> +       char *cmd = req->buf;
>>> +       char response[RESPONSE_LEN];
>>> +
>>> +       strsep(&cmd, ":");
>>> +       if (!cmd) {
>>> +               error("missing partition name\n");
>>
>> no newline with error()
> Will remove
>>
>>> +               fastboot_tx_write_str("FAILmissing partition name");
>>> +               return;
>>> +       }
>>> +
>>> +       strcpy(response, "FAILno flash device defined");
>>> +
>>> +#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
>>> +       fb_mmc_erase(cmd, response);
>>> +#endif
>>> +       fastboot_tx_write_str(response);
>>> +}
>>> +
>>>    struct cmd_dispatch_info {
>>>          char *cmd;
>>>          void (*cb)(struct usb_ep *ep, struct usb_request *req);
>>> @@ -566,6 +586,9 @@ static const struct cmd_dispatch_info cmd_dispatch_info[] = {
>>>          {
>>>                  .cmd = "oem",
>>>                  .cb = cb_oem,
>>> +       }, {
>>> +               .cmd = "erase",
>>> +               .cb = cb_erase,
>>>          },
>>>    };
>>>
>>> diff --git a/include/fb_mmc.h b/include/fb_mmc.h
>>> index 1ad1d13..402ba9b 100644
>>> --- a/include/fb_mmc.h
>>> +++ b/include/fb_mmc.h
>>> @@ -6,3 +6,4 @@
>>>
>>>    void fb_mmc_flash_write(const char *cmd, void *download_buffer,
>>>                          unsigned int download_bytes, char *response);
>>> +void fb_mmc_erase(const char *cmd, char *response);
>>>
>
> Thanks for the review. Will send v2 with changes.
>
> Regards,
> Dileep
>