From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Rae Date: Thu, 12 Feb 2015 09:21:08 -0800 Subject: [U-Boot] [PATCH v1 1/1] usb: gadget: fastboot: Add fastboot erase In-Reply-To: References: <1423558178-23105-1-git-send-email-dileep.katta@linaro.org> Message-ID: <54DCE104.8010009@broadcom.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 15-02-12 01:21 AM, Dileep Katta wrote: > Hi Rob, > > On 12 February 2015 at 14:35, Rob Herring wrote: >> On Tue, Feb 10, 2015 at 2: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 >>> --- >>> 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 >>> #include >>> #include >>> +#include >>> >>> #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 >