[U-Boot] [RFC PATCH 0/3] Implement "fastboot flash" for eMMC
Rob Herring
robh at kernel.org
Wed Jun 25 15:59:57 CEST 2014
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.
Don't you also need the ability to partition a disk with fastboot?
Rob
>
> 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