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

[Lukasz Majewski <l.majewski@samsung.com>]

Hi Rob,

> On Mon, Jun 23, 2014 at 1:37 PM, Steve Rae <srae@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.

USB mass storage is from its very beginning tied with eMMC/SD card.

DFU/thor are a different in a way, that they allow storing data to
other media like raw memory, eMMC, NAND, etc. The dfu backend tries to
handle writing to many media and also file systems.

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



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group