[U-Boot] [PATCH v1 1/1] usb: gadget: fastboot: Add fastboot erase
Matt Reimer
mreimer at sdgsystems.com
Tue Feb 17 16:55:40 CET 2015
On Tue, Feb 17, 2015 at 5:57 AM, Dileep Katta <dileep.katta at linaro.org>
wrote:
> On 17 February 2015 at 02:51, Steve Rae <srae at broadcom.com> wrote:
>
> >
> >
> > On 15-02-16 12:40 PM, Dileep Katta wrote:
> >
> >> Hi Steve,
> >>
> >> On 14 February 2015 at 02:15, Steve Rae <srae at broadcom.com> wrote:
> >>
> >>
> >>>
> >>> On 15-02-12 12:29 AM, Dileep Katta wrote:
> >>>
> >>> Hi Steve,
> >>>>
> >>>> On 11 February 2015 at 05:25, Steve Rae <srae at broadcom.com> wrote:
> >>>>
> >>>>
> >>>>> Hi, Dileep
> >>>>>
> >>>>>
> >>>>> On 15-02-10 12:49 AM, Dileep Katta wrote:
> >>>>>
> >>>>>
> >>>>>> Adds the fastboot erase functionality, to erase a partition
> >>>>>> specified by name. The erase is performed based on erase group size,
> >>>>>> to avoid erasing other partitions. The start address and the size
> >>>>>> is aligned to the erase group size for this.
> >>>>>>
> >>>>>> Currently only supports erasing from eMMC.
> >>>>>>
> >>>>>> Signed-off-by: Dileep Katta <dileep.katta at linaro.org>
> >>>>>> ---
> >>>>>> Note: The changes are on top of oem command support added by
> >>>>>> robh at kernel.org
> >>>>>>
> >>>>>> common/fb_mmc.c | 58
> >>>>>> ++++++++++++++++++++++++++++++
> >>>>>> +++++++++++
> >>>>>> drivers/usb/gadget/f_fastboot.c | 23 ++++++++++++++++
> >>>>>> include/fb_mmc.h | 1 +
> >>>>>> 3 files changed, 82 insertions(+)
> >>>>>>
> >>>>>> diff --git a/common/fb_mmc.c b/common/fb_mmc.c
> >>>>>> index 6ea3938..3911989 100644
> >>>>>> --- a/common/fb_mmc.c
> >>>>>> +++ b/common/fb_mmc.c
> >>>>>> @@ -10,6 +10,7 @@
> >>>>>> #include <part.h>
> >>>>>> #include <aboot.h>
> >>>>>> #include <sparse_format.h>
> >>>>>> +#include <mmc.h>
> >>>>>>
> >>>>>> #ifndef CONFIG_FASTBOOT_GPT_NAME
> >>>>>> #define CONFIG_FASTBOOT_GPT_NAME GPT_ENTRY_NAME
> >>>>>> @@ -110,3 +111,60 @@ void fb_mmc_flash_write(const char *cmd, void
> >>>>>> *download_buffer,
> >>>>>> write_raw_image(dev_desc, &info, cmd,
> >>>>>> download_buffer,
> >>>>>> download_bytes);
> >>>>>> }
> >>>>>> +
> >>>>>> +void fb_mmc_erase(const char *cmd, char *response)
> >>>>>> +{
> >>>>>> + int ret;
> >>>>>> + block_dev_desc_t *dev_desc;
> >>>>>> + disk_partition_t info;
> >>>>>> + lbaint_t blks, blks_start, blks_size, grp_size;
> >>>>>> + struct mmc *mmc = find_mmc_device(CONFIG_
> >>>>>> FASTBOOT_FLASH_MMC_DEV);
> >>>>>> +
> >>>>>> + if (mmc == NULL) {
> >>>>>> + error("invalid mmc device\n");
> >>>>>>
> >>>>>>
> >>>>> no newline with error()
> >>>>>
> >>>>> Will remove
> >>>>
> >>>>
> >>>>> + fastboot_fail("invalid mmc device");
> >>>>>
> >>>>>> + return;
> >>>>>> + }
> >>>>>> +
> >>>>>> + /* initialize the response buffer */
> >>>>>> + response_str = response;
> >>>>>> +
> >>>>>> + dev_desc = get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
> >>>>>> + if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
> >>>>>> + error("invalid mmc device\n");
> >>>>>>
> >>>>>>
> >>>>> no newline with error()
> >>>>>
> >>>>> Will remove
> >>>>
> >>>>
> >>>>>
> >>>>> + fastboot_fail("invalid mmc device");
> >>>>>
> >>>>>> + return;
> >>>>>> + }
> >>>>>> +
> >>>>>> + ret = get_partition_info_efi_by_name(dev_desc, cmd, &info);
> >>>>>> + if (ret) {
> >>>>>> + error("cannot find partition: '%s'\n", cmd);
> >>>>>>
> >>>>>>
> >>>>> no newline with error()
> >>>>>
> >>>>> Will remove
> >>>>
> >>>>
> >>>>> + fastboot_fail("cannot find partition");
> >>>>>
> >>>>>> + return;
> >>>>>> + }
> >>>>>> +
> >>>>>> + puts("Erasing partition\n");
> >>>>>> +
> >>>>>> + /* Align blocks to erase group size to avoid erasing other
> >>>>>> partitions */
> >>>>>> + grp_size = mmc->erase_grp_size;
> >>>>>> + blks_start = (info.start + grp_size - 1) & ~(grp_size - 1);
> >>>>>> + if (info.size >= grp_size)
> >>>>>> + blks_size = (info.size - (blks_start - info.start))
> &
> >>>>>> + (~(grp_size - 1));
> >>>>>> + else
> >>>>>> + blks_size = 0;
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> Is this logic correct??? Isn't the "erase_grp_size" in bytes? and the
> >>>>> info.start & info.size in LBA's?
> >>>>>
> >>>>> Yes, the math will take care of it. Ref: mmc_berase()
> >>>>
> >>>>
> >>> So, I have a partition:
> >>>
> >>> 2 0x00000300 0x000003ff "test"
> >>> attrs: 0x0000000000000000
> >>> type: 9e312af1-18fe-fa41-45f3-37b3bb1d1061
> >>> guid: 18a5d0db-d23a-aac1-0d4c-692c7ba9ab1c
> >>>
> >>> and 'fastboot erase test' produces:
> >>> Erasing blocks 1024 to 1024 due to alignment
> >>> ........ erased 0 bytes from 'test'
> >>> which is not correct!
> >>>
> >>> Furthermore, doesn't the mmc_berase() already handle the
> >>> "erase_grp_size"....
> >>>
> >>>
> >> It does handle it, in a way, but the way it handles it is to erase more
> >> blocks than requested if the request isn't aligned. In your example, you
> >> requested the "test" partition to be erased (0x300 to 0x3ff), but what
> was
> >> actually erased (as printed in the "Caution" message from mmc_berase)
> was
> >> 0x0 to 0x7ff.
> >>
> >> If I remove your alignment logic, then it produces:
> >>
> >>> Erasing blocks 768 to 1024 due to alignment
> >>>
> >>> Caution! Your devices Erase group is 0x400
> >>> The erase range would be change to 0x0~0x7ff
> >>>
> >>> ........ erased 131072 bytes from 'test'
> >>> which looks correct (except for the "Caution" message)....
> >>>
> >>> Thanks, Steve
> >>>
> >>
> >>
> >> Except that this one has now erased 0x0 to 0x300 and 0x400 to 0x7ff
> also,
> >> which you did not want to erase, right?
> >> Aligning the start address is meant to protect this data from being
> erased
> >> unintentionally. The trade-off is that some small partitions won't get
> >> erased at all, if they are too small and not aligned.
> >>
> >> Regards, Dileep
> >>
> >> I'm sorry, but I am really confused....
> > -- 131072 bytes agrees with 0x300 to 0x3ff...
> > -- I'm suspicious that the Caution message and the actual erased bytes
> do
> > not match....
> > -- I'm not certain that the existing mmc_berase() code is correct...
> > -- If this "erase_grp_size" is a method to erase large sections
> > "quickly", then it would seem that it needs to be combined with erasing
> (by
> > writing "0" or "1" appropriately) to the smaller sections to erase the
> > "exact specified size". And this should be handled in the mmc_berase()
> code!
> >
>
> Thanks for your suggestions Steve. I will try to send a separate patch for
> mmc_berase() if required, after validating all the scenarios.
>
>
> > Additionally, because it is not necessary to "erase before writing", I
> > personally do not have any need for this "fastboot erase" command at all.
> > So I think that I am going to withdraw from this conversation....
> > Thanks, Steve
>
>
> We wish fastboot to be complete with all the commands of the
> protocol supported in u-boot.
>
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
More information about the U-Boot
mailing list