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@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 -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
