[U-Boot] [PATCH v1 1/1] usb: gadget: fastboot: Add fastboot erase
Dileep Katta
dileep.katta at linaro.org
Mon Feb 16 21:40:33 CET 2015
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
>
>
>
>>>
>>> +
>>>> + 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