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

Dileep Katta dileep.katta at linaro.org
Tue Feb 17 13:57:55 CET 2015


On 17 February 2015 at 02:51, Steve Rae <srae at broadcom.com> wrote:

>
>
> On 15-02-16 12:40 PM, Dileep Katta wrote:
>
>> Hi Steve,
>>
>> On 14 February 2015 at 02:15, Steve Rae <srae at broadcom.com> wrote:
>>
>>
>>>
>>> On 15-02-12 12:29 AM, Dileep Katta wrote:
>>>
>>>  Hi Steve,
>>>>
>>>> On 11 February 2015 at 05:25, Steve Rae <srae at 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 at 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"....
>>>
>>>
>> It does handle it, in a way, but the way it handles it is to erase more
>> blocks than requested if the request isn't aligned. In your example, you
>> requested the "test" partition to be erased (0x300 to 0x3ff), but what was
>> actually erased (as printed in the "Caution" message from mmc_berase) was
>> 0x0 to 0x7ff.
>>
>> 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
>>>
>>
>>
>> Except that this one has now erased 0x0 to 0x300 and 0x400 to 0x7ff also,
>> which you did not want to erase, right?
>> Aligning the start address is meant to protect this data from being erased
>> unintentionally. The trade-off is that some small partitions won't get
>> erased at all, if they are too small and not aligned.
>>
>> Regards, Dileep
>>
>>  I'm sorry, but I am really confused....
> --  131072 bytes agrees with 0x300 to 0x3ff...
> --  I'm suspicious that the Caution message and the actual erased bytes do
> not match....
> --  I'm not certain that the existing mmc_berase() code is correct...
> --  If this "erase_grp_size" is a method to erase large sections
> "quickly", then it would seem that it needs to be combined with erasing (by
> writing "0" or "1" appropriately) to the smaller sections to erase the
> "exact specified size". And this should be handled in the mmc_berase() code!
>

Thanks for your suggestions Steve. I will try to send a separate patch for
mmc_berase() if required, after validating all the scenarios.


> Additionally, because it is not necessary to "erase before writing", I
> personally do not have any need for this "fastboot erase" command at all.
> So I think that I am going to withdraw from this conversation....
> Thanks, Steve


We wish fastboot to be complete with all the commands of the
protocol supported in u-boot.

Regards,
Dileep

>
>
>
>>>
>>>
>>>
>>>>>   +
>>>>>
>>>>>> +       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
>>>>
>>>>
>>>>
>>


More information about the U-Boot mailing list