All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steve Rae <srae@broadcom.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC PATCH 0/3] Implement "fastboot flash" for eMMC
Date: Fri, 20 Jun 2014 14:55:51 -0700	[thread overview]
Message-ID: <53A4ADE7.2080306@broadcom.com> (raw)
In-Reply-To: <201406200832.19841.marex@denx.de>



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

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; 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 case of 
the "mmc_block_op()" be acceptable?
- a method which uses "get_partition_info_efi_by_name()"
	-- no ideas yet...

If the consensus is to use this DFU backend, then I will continue is 
this direction.

Please advise,
Thanks, Steve

  reply	other threads:[~2014-06-20 21:55 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-19 20:52 [U-Boot] [RFC PATCH 0/3] Implement "fastboot flash" for eMMC Steve Rae
2014-06-19 20:52 ` [U-Boot] [RFC PATCH 1/3] usb/gadget: fastboot: add sparse image definitions Steve Rae
2014-06-19 20:52 ` [U-Boot] [RFC PATCH 2/3] usb/gadget: fastboot: add eMMC support for flash command Steve Rae
2014-06-19 20:52 ` [U-Boot] [RFC PATCH 3/3] usb/gadget: fastboot: add " Steve Rae
2014-06-20  6:18 ` [U-Boot] [RFC PATCH 0/3] Implement "fastboot flash" for eMMC Lukasz Majewski
2014-06-20  6:32   ` Marek Vasut
2014-06-20 21:55     ` Steve Rae [this message]
2014-06-23 12:58       ` Lukasz Majewski
2014-06-23 18:37         ` Steve Rae
2014-06-25 13:59           ` Rob Herring
2014-06-25 14:03             ` Pantelis Antoniou
2014-06-26  0:16             ` Steve Rae
2014-06-26 13:20               ` Rob Herring
2014-06-26 17:18                 ` Steve Rae
2014-06-27  8:50                   ` Lukasz Majewski
2014-06-27  8:39                 ` Lukasz Majewski
2014-06-27  8:34             ` Lukasz Majewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53A4ADE7.2080306@broadcom.com \
    --to=srae@broadcom.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.