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