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

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

[Dileep Katta]

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");
+		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");
+
+	/* 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;
+
+	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)
+{
+	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] [PATCH v1 1/1] usb: gadget: fastboot: Add fastboot erase

[Steve Rae]

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()

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

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

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


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

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

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

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

[Dileep Katta]

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

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

[Steve Rae]

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
>

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

[Dileep Katta]

Hi Steve,

On 14 February 2015 at 02:15, Steve Rae <srae@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@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"....
>

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

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

[Steve Rae]

On 15-02-16 12:40 PM, Dileep Katta wrote:
> Hi Steve,
>
> On 14 February 2015 at 02:15, Steve Rae <srae@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@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"....
>>
>
> 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!
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

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

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

[Dileep Katta]

On 17 February 2015 at 02:51, Steve Rae <srae@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@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@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"....
>>>
>>>
>> 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
>>>>
>>>>
>>>>
>>

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

[Dileep Katta]

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>
---
Changes in v2:
	- Removed unnecessary newline for error() messages
	- Made the command conditional on CONFIG_FASTBOOT_FLASH
	- Updated doc/README.android-fastboot to reflect the support
 common/fb_mmc.c                 | 56 +++++++++++++++++++++++++++++++++++++++++
 doc/README.android-fastboot     |  5 ++--
 drivers/usb/gadget/f_fastboot.c | 25 ++++++++++++++++++
 include/fb_mmc.h                |  1 +
 4 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index 6ea3938..513b7ab 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,58 @@ 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");
+		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");
+		fastboot_fail("invalid mmc device");
+		return;
+	}
+
+	ret = get_partition_info_efi_by_name(dev_desc, cmd, &info);
+	if (ret) {
+		error("cannot find partition: '%s'", cmd);
+		fastboot_fail("cannot find partition");
+		return;
+	}
+
+	/* 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;
+
+	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", 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/doc/README.android-fastboot b/doc/README.android-fastboot
index 1677609..5526a43 100644
--- a/doc/README.android-fastboot
+++ b/doc/README.android-fastboot
@@ -6,9 +6,8 @@ Overview
 The protocol that is used over USB is described in
 README.android-fastboot-protocol in same directory.
 
-The current implementation does not yet support the erase command or the
-"oem format" command, and there is minimal support for the flash command;
-it only supports eMMC devices.
+The current implementation is a minimal support of the erase command,the
+"oem format" command and flash command;it only supports eMMC devices.
 
 Client installation
 ===================
diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index f7d84bf..6138906 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -535,6 +535,28 @@ static void cb_oem(struct usb_ep *ep, struct usb_request *req)
 	}
 }
 
+#ifdef CONFIG_FASTBOOT_FLASH
+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");
+		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);
+}
+#endif
+
 struct cmd_dispatch_info {
 	char *cmd;
 	void (*cb)(struct usb_ep *ep, struct usb_request *req);
@@ -561,6 +583,9 @@ static const struct cmd_dispatch_info cmd_dispatch_info[] = {
 	{
 		.cmd = "flash",
 		.cb = cb_flash,
+	}, {
+		.cmd = "erase",
+		.cb = cb_erase,
 	},
 #endif
 	{
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] [PATCH v2 1/1] usb: gadget: fastboot: Add fastboot erase

[Lukasz Majewski]

Hi Dileep,

> 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>
> ---
> Changes in v2:
> 	- Removed unnecessary newline for error() messages
> 	- Made the command conditional on CONFIG_FASTBOOT_FLASH
> 	- Updated doc/README.android-fastboot to reflect the support
>  common/fb_mmc.c                 | 56
> +++++++++++++++++++++++++++++++++++++++++
> doc/README.android-fastboot     |  5 ++--
> drivers/usb/gadget/f_fastboot.c | 25 ++++++++++++++++++
> include/fb_mmc.h                |  1 + 4 files changed, 84
> insertions(+), 3 deletions(-)
> 
> diff --git a/common/fb_mmc.c b/common/fb_mmc.c
> index 6ea3938..513b7ab 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,58 @@ 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");
> +		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");
> +		fastboot_fail("invalid mmc device");
> +		return;
> +	}
> +
> +	ret = get_partition_info_efi_by_name(dev_desc, cmd, &info);
> +	if (ret) {
> +		error("cannot find partition: '%s'", cmd);
> +		fastboot_fail("cannot find partition");
> +		return;
> +	}
> +
> +	/* 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;
> +
> +	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",
> 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/doc/README.android-fastboot b/doc/README.android-fastboot
> index 1677609..5526a43 100644
> --- a/doc/README.android-fastboot
> +++ b/doc/README.android-fastboot
> @@ -6,9 +6,8 @@ Overview
>  The protocol that is used over USB is described in
>  README.android-fastboot-protocol in same directory.
>  
> -The current implementation does not yet support the erase command or
> the -"oem format" command, and there is minimal support for the flash
> command; -it only supports eMMC devices.
> +The current implementation is a minimal support of the erase
> command,the +"oem format" command and flash command;it only supports
> eMMC devices. 
>  Client installation
>  ===================
> diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c index f7d84bf..6138906 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -535,6 +535,28 @@ static void cb_oem(struct usb_ep *ep, struct
> usb_request *req) }
>  }
>  
> +#ifdef CONFIG_FASTBOOT_FLASH
> +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");
> +		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);
> +}
> +#endif
> +
>  struct cmd_dispatch_info {
>  	char *cmd;
>  	void (*cb)(struct usb_ep *ep, struct usb_request *req);
> @@ -561,6 +583,9 @@ static const struct cmd_dispatch_info
> cmd_dispatch_info[] = { {
>  		.cmd = "flash",
>  		.cb = cb_flash,
> +	}, {
> +		.cmd = "erase",
> +		.cb = cb_erase,
>  	},
>  #endif
>  	{
> 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);

Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>

I'm going to apply this patch to -dfu tree.

Thanks Dileep!

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

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

[Dileep Katta]

Thanks Lukasz.

Regards,
Dileep

On 24 February 2015 at 14:44, Lukasz Majewski <l.majewski@samsung.com>
wrote:

> Hi Dileep,
>
> > 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>
> > ---
> > Changes in v2:
> >       - Removed unnecessary newline for error() messages
> >       - Made the command conditional on CONFIG_FASTBOOT_FLASH
> >       - Updated doc/README.android-fastboot to reflect the support
> >  common/fb_mmc.c                 | 56
> > +++++++++++++++++++++++++++++++++++++++++
> > doc/README.android-fastboot     |  5 ++--
> > drivers/usb/gadget/f_fastboot.c | 25 ++++++++++++++++++
> > include/fb_mmc.h                |  1 + 4 files changed, 84
> > insertions(+), 3 deletions(-)
> >
> > diff --git a/common/fb_mmc.c b/common/fb_mmc.c
> > index 6ea3938..513b7ab 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,58 @@ 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");
> > +             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");
> > +             fastboot_fail("invalid mmc device");
> > +             return;
> > +     }
> > +
> > +     ret = get_partition_info_efi_by_name(dev_desc, cmd, &info);
> > +     if (ret) {
> > +             error("cannot find partition: '%s'", cmd);
> > +             fastboot_fail("cannot find partition");
> > +             return;
> > +     }
> > +
> > +     /* 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;
> > +
> > +     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",
> > 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/doc/README.android-fastboot b/doc/README.android-fastboot
> > index 1677609..5526a43 100644
> > --- a/doc/README.android-fastboot
> > +++ b/doc/README.android-fastboot
> > @@ -6,9 +6,8 @@ Overview
> >  The protocol that is used over USB is described in
> >  README.android-fastboot-protocol in same directory.
> >
> > -The current implementation does not yet support the erase command or
> > the -"oem format" command, and there is minimal support for the flash
> > command; -it only supports eMMC devices.
> > +The current implementation is a minimal support of the erase
> > command,the +"oem format" command and flash command;it only supports
> > eMMC devices.
> >  Client installation
> >  ===================
> > diff --git a/drivers/usb/gadget/f_fastboot.c
> > b/drivers/usb/gadget/f_fastboot.c index f7d84bf..6138906 100644
> > --- a/drivers/usb/gadget/f_fastboot.c
> > +++ b/drivers/usb/gadget/f_fastboot.c
> > @@ -535,6 +535,28 @@ static void cb_oem(struct usb_ep *ep, struct
> > usb_request *req) }
> >  }
> >
> > +#ifdef CONFIG_FASTBOOT_FLASH
> > +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");
> > +             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);
> > +}
> > +#endif
> > +
> >  struct cmd_dispatch_info {
> >       char *cmd;
> >       void (*cb)(struct usb_ep *ep, struct usb_request *req);
> > @@ -561,6 +583,9 @@ static const struct cmd_dispatch_info
> > cmd_dispatch_info[] = { {
> >               .cmd = "flash",
> >               .cb = cb_flash,
> > +     }, {
> > +             .cmd = "erase",
> > +             .cb = cb_erase,
> >       },
> >  #endif
> >       {
> > 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);
>
> Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
>
> I'm going to apply this patch to -dfu tree.
>
> Thanks Dileep!
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
>

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

[Matt Reimer]

On Tue, Feb 17, 2015 at 5:57 AM, Dileep Katta <dileep.katta@linaro.org>
wrote:

> On 17 February 2015 at 02:51, Steve Rae <srae@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@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@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"....
> >>>
> >>>
> >> 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.
>

Actually according to this comment from fastboot
(system/core/fastboot/fastboot.c), sometimes it is necessary to erase
before flashing:

/* Until we get lazy inode table init working in make_ext4fs, we need to
 * erase partitions of type ext4 before flashing a filesystem so no stale
 * inodes are left lying around.  Otherwise, e2fsck gets very upset.
 */

In order for fastboot to know that it needs to erase the partition first,
we'll also need to implement a fastboot var "partition-type" that will
return "ext4" for ext4 partitions. See
system/core/fastboot/engine.c:fb_format_supported().

Matt

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

[Rob Herring]

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().

> +               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.
> +
> +       /* 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.

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

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

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

[Dileep Katta]

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.
>
>> +               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] [PATCH v1 1/1] usb: gadget: fastboot: Add fastboot erase

[Steve Rae]

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
>