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

Steve Rae
2015-02-12T17:21:08Z

On 15-02-12 01:21 AM, Dileep Katta wrote:
> Hi Rob,
>
> On 12 February 2015 at 14:35, Rob Herring <robherring2@gmail.com> wrote:
>> On Tue, Feb 10, 2015 at 2:49 AM, Dileep Katta <dileep.katta@linaro.org> 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");
>>> + fastboot_fail("invalid mmc device");
>>
>> Perhaps fastboot_fail should also call error().
> There is error() before every fastboot_fail(). You mean to move
> error() inside fastboot_fail()? If yes, I will do it.

I disagree - most of the error() report more information about the
issue; and the fastboot_fail() just informs the host that it was
unsuccessful -- I don't think that we should force these to be the same....

Thanks, Steve
>>
>>> + 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");
>>> + 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);
>>> + fastboot_fail("cannot find partition");
>>> + return;
>>> + }
>>> +
>>> + puts("Erasing partition\n");
>>
>> This can probably be dropped since you have a print below.
> Will remove.
>>> +
>>> + /* Align blocks to erase group size to avoid erasing other partitions */
>>
>> The partitioning could should probably enforce optimal alignment, but
>> I guess that's a separate patch.
> Fine. Will do it as a separate patch.
>>
>>> + 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;
>>> +
>>> + 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);
>>> + 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)
>>
>> This should be conditional on CONFIG_FASTBOOT_FLASH
> Will make it conditional as in cb_oem().
>>
>>> +{
>>> + char *cmd = req->buf;
>>> + char response[RESPONSE_LEN];
>>> +
>>> + strsep(&cmd, ":");
>>> + if (!cmd) {
>>> + error("missing partition name\n");
>>> + 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);
>>> --
>>> 1.8.3.2
>>>
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>
> Thanks for the comments. Will send v2 once I get your input on fastboot_fail().
>
> Regards,
> Dileep
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>