[U-Boot] [RFC PATCH 0/3] Implement "fastboot flash" for eMMC
Steve Rae
srae at broadcom.com
Thu Jun 26 02:16:43 CEST 2014
Rob,
On 14-06-25 06:59 AM, 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?
The concern is not performance related -- just the amount of (overhead)
code required to implement the "DFU backend" versus calling
mmc_dev->block_write()
(maybe someone can tell me where to interface into DFU: is it at
"dfu_write() or ????)
>
> 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.
As I understand it, the "block_write" callback function is in the
"block_dev_desc_t". Isn't this the part of the "generic block device"
interface? Please explain...
>
> Don't you also need the ability to partition a disk with fastboot?
yes: though "fastboot oem format" is outside of this RFC -- because I
wanted to minimize this request to ensure that licensing wasn't going to
kill it.
>
> 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