[U-Boot] [RFC PATCH 0/3] Implement "fastboot flash" for eMMC

Pantelis Antoniou panto at antoniou-consulting.com
Wed Jun 25 16:03:02 CEST 2014


Hi Rob,

On Jun 25, 2014, at 4:59 PM, Rob Herring wrote:

> On Mon, Jun 23, 2014 at 1:37 PM, Steve Rae <srae at broadcom.com> wrote:
>> Rob & Sebastian
>> 
>> I would appreciate your comments on this issue; I suspect that you had some
>> ideas regarding the implementation of the fastboot "flash" and "erase"
>> commands....
> 
> I agree with Lukasz's and Marek's comments unless there are good
> reasons not to use it which can't be fixed. Curiously, USB mass
> storage does not use the DFU backend, but I don't know why. Perhaps
> there are incompatibilities or converting it is on the todo list. Are
> your performance concerns measurable or it's just the fact you are
> adding another layer?
> 
> I'd really like to see the eMMC backend be a generic block device
> backend. There's no good reason for it to be eMMC/SD specific.
> 

I completely agree. Started looking into this but there's lots of inertia :(
We have device specific backends where a generic one should suffice...

> Don't you also need the ability to partition a disk with fastboot?
> 
> Rob
> 

Regards

-- Pantelis

>> 
>> Thanks in advance, Steve
>> 
>> 
>> On 14-06-23 05:58 AM, Lukasz Majewski wrote:
>>> 
>>> Hi Steve,
>>> 
>>>> 
>>>> 
>>>> On 14-06-19 11:32 PM, Marek Vasut wrote:
>>>>> 
>>>>> On Friday, June 20, 2014 at 08:18:42 AM, Lukasz Majewski wrote:
>>>>>> 
>>>>>> Hi Steve,
>>>>>> 
>>>>>>> This series implements the "fastboot flash" command for eMMC
>>>>>>> devices. It supports both raw and sparse images.
>>>>>>> 
>>>>>>> NOTES:
>>>>>>> - the support for the "fastboot flash" command is enabled with
>>>>>>> CONFIG_FASTBOOT_FLASH
>>>>>>> - the support for eMMC is enabled with
>>>>>>> CONFIG_FASTBOOT_FLASH_MMC_DEV
>>>>>>> - (future) the support for NAND would be enabled with
>>>>>>> CONFIG_FASTBOOT_FLASH_NAND(???)
>>>>>>> - thus the proposal is to place the code in common/fb_mmc.c and
>>>>>>> (future) common/fb_nand.c(???), however, this may not be the
>>>>>>> appropriate location....
>>>>>> 
>>>>>> 
>>>>>> Would you consider another approach for providing flashing backend
>>>>>> for fastboot?
>>>>>> 
>>>>>> I'd like to propose reusing of the dfu flashing code for this
>>>>>> purpose. Such approach has been used successfully with USB "thor"
>>>>>> downloading function.
>>>>>> 
>>>>>> Since the "fastboot" is using gadget infrastructure (thanks to the
>>>>>> effort of Rob Herring) I think that it would be feasible to reuse
>>>>>> the same approach as "thor" does. In this way the low level code
>>>>>> would be kept in one place and we could refine and test it more
>>>>>> thoroughly.
>>>>> 
>>>>> 
>>>>> I'm all for this approach as well if possible.
>>>>> 
>>>>> Best regards,
>>>>> Marek Vasut
>>>>> _______________________________________________
>>>>> U-Boot mailing list
>>>>> U-Boot at lists.denx.de
>>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>> 
>>>> 
>>>> I have briefly investigated this suggestion....
>>>> And have 'hacked' some code as follows:
>>>> 
>>>> --- common/fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700
>>>> +++ common/fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700
>>>>                while (remaining_chunks) {
>>>>                        switch (le16_to_cpu(c_header->chunk_type)) {
>>>>                        case CHUNK_TYPE_RAW:
>>>> +#if 0
>>>>                                blkcnt =
>>>>                                    (le32_to_cpu(c_header->chunk_sz)
>>>> * blk_sz) / info.blksz;
>>>>                                buffer =
>>>>                                    (void *)c_header +
>>>>                                    le16_to_cpu(s_header->chunk_hdr_sz);
>>>> 
>>>>                                blks =
>>>> mmc_dev->block_write(mmc_dev->dev, blk, blkcnt, buffer);
>>>>                                if (blks != blkcnt) {
>>>>                                        printf("Write failed
>>>> %lu\n", blks); strcpy(response,
>>>>                                               "FAILmmc write
>>>> failure"); return;
>>>>                                }
>>>> 
>>>>                                bytes_written += blkcnt *
>>>> info.blksz; +#else
>>>> +                               buffer =
>>>> +                                   (void *)c_header +
>>>> +
>>>> le16_to_cpu(s_header->chunk_hdr_sz); +
>>>> +                               len =
>>>> le32_to_cpu(c_header->chunk_sz) * blk_sz;
>>>> +                               ret_dfu = dfu_write_medium_mmc(dfu,
>>>> offset,
>>>> +
>>>> buffer, &len);
>>>> +                               if (ret_dfu) {
>>>> +                                       printf("Write failed %lu\n",
>>>> len);
>>>> +                                       strcpy(response,
>>>> +                                              "FAILmmc write
>>>> failure");
>>>> +                                       return;
>>>> +                               }
>>>> +
>>>> +
>>>> +                               bytes_written += len;
>>>> +#endif
>>>>                                break;
>>>> 
>>>>                        case CHUNK_TYPE_FILL:
>>>>                        case CHUNK_TYPE_DONT_CARE:
>>>>                        case CHUNK_TYPE_CRC32:
>>>>                                /* do nothing */
>>>>                                break;
>>>> 
>>>>                        default:
>>>>                                /* error */
>>>>                                printf("Unknown chunk type\n");
>>>>                                strcpy(response,
>>>>                                       "FAILunknown chunk type in
>>>> sparse image"); return;
>>>>                        }
>>>> 
>>>> +#if 0
>>>>                        blk += (le32_to_cpu(c_header->chunk_sz) *
>>>> blk_sz) / info.blksz;
>>>> +#else
>>>> +                       offset += le32_to_cpu(c_header->chunk_sz) *
>>>> blk_sz; +#endif
>>>>                        c_header = (chunk_header_t *)((void
>>>> *)c_header + le32_to_cpu(c_header->total_sz));
>>>>                        remaining_chunks--;
>>>>                }
>>>> 
>>>> 
>>>> --- common/fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700
>>>> +++ common/fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700
>>>>                /* raw image */
>>>> 
>>>> +#if 0
>>>>                /* determine number of blocks to write */
>>>>                blkcnt =
>>>>                    ((download_bytes + (info.blksz - 1)) &
>>>> ~(info.blksz - 1)); blkcnt = blkcnt / info.blksz;
>>>> 
>>>>                if (blkcnt > info.size) {
>>>>                        printf("%s: too large for partition:
>>>> '%s'\n", __func__, cmd);
>>>>                        strcpy(response, "FAILtoo large for
>>>> partition"); return;
>>>>                }
>>>> 
>>>>                printf("Flashing Raw Image\n");
>>>> 
>>>>                blks = mmc_dev->block_write(mmc_dev->dev,
>>>> info.start, blkcnt, download_buffer);
>>>>                if (blks != blkcnt) {
>>>>                        printf("%s: failed writing to mmc device
>>>> %d\n", __func__, mmc_dev->dev);
>>>>                        strcpy(response, "FAILfailed writing to mmc
>>>> device"); return;
>>>>                }
>>>> 
>>>>                printf("........ wrote %lu bytes to '%s'\n",
>>>>                       blkcnt * info.blksz, cmd);
>>>> +#else
>>>> +               printf("Flashing Raw Image\n");
>>>> +
>>>> +               ret_dfu = dfu_write_medium_mmc(dfu, offset,
>>>> download_buffer, &len);
>>>> +               if (ret_dfu) {
>>>> +                       printf("%s: failed writing to mmc device
>>>> %d\n",
>>>> +                              __func__, mmc_dev->dev);
>>>> +                       strcpy(response, "FAILfailed writing to mmc
>>>> device");
>>>> +                       return;
>>>> +               }
>>>> +
>>>> +               printf("........ wrote %lu bytes to '%s'\n", len,
>>>> cmd); +#endif
>>>>        }
>>>> 
>>>> NOTE:
>>>> - I know that I cannot call "dfu_write_medium_mmc()" directly -- but
>>>> I just wanted to test this functionality
>>> 
>>> 
>>> Indeed, it looks like an early proof-of-concept code :-).
>>> 
>>>> 
>>>> My initial reaction is that using the DFU backend to effectively call
>>>> the mmc block_write() function seems to cause an unnecessary amount
>>>> of overhead;
>>> 
>>> 
>>> It also allows to access/write data to other media - like NAND memory.
>>> 
>>>> and the only thing that it really provides is a proven
>>>> method of calculating the "number of blocks to write"...
>>>> 
>>>> I would be more interested in this backend if it would provide:
>>>> - handling of the "sparse image format"
>>>>        -- would a CONFIG option to include this in the DFU_OP_WRITE
>>> 
>>> 
>>> You are welcome to prepare patch which adds such functionality.
>>> Moreover, in the u-boot-dfu repository (master branch) you can find
>>> initial version of the regression tests for DFU.
>>> Extending the current one, or adding your own would be awesome :-)
>>> 
>>> 
>>>> case of the "mmc_block_op()" be acceptable?
>>>> - a method which uses "get_partition_info_efi_by_name()"
>>>>        -- no ideas yet...
>>>> 
>>> 
>>> I'm looking forward for RFC.
>>> 
>>>> If the consensus is to use this DFU backend, then I will continue is
>>>> this direction.
>>> 
>>> 
>>> That would be great.
>>> 
>>>> 
>>>> Please advise,
>>>> Thanks, Steve
>>> 
>>> 
>>> 
>> 



More information about the U-Boot mailing list