Implement "fastboot flash" for eMMC

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

This has been tested on ARMv7.
This is a WIP -- and I would appreciate some comments/advice... Thanks, Steve

Steve Rae (3):
usb/gadget: fastboot: add sparse image definitions usb/gadget: fastboot: add eMMC support for flash command usb/gadget: fastboot: add support for flash command

 README                          |  10 +++
 common/Makefile                 |   5 ++
 common/fb_mmc.c                 | 144
++++++++++++++++++++++++++++++++++++++++
doc/README.android-fastboot     |   5 +-
drivers/usb/gadget/f_fastboot.c |  31 +++++++++
include/fb_mmc.h                |   8 +++
include/sparse_format.h         |  58 ++++++++++++++++ 7 files
changed, 259 insertions(+), 2 deletions(-) create mode 100644
common/fb_mmc.c create mode 100644 include/fb_mmc.h
 create mode 100644 include/sparse_format.h

Hi Steve,

Quoted 12 lines

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.

Quoted 18 lines · Lukasz Majewski

Hi Steve,

Quoted 12 lines

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?

Quoted 8 lines

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

Quoted 30 lines · Lukasz Majewski

Quoted 23 lines

Hi Steve,

Quoted 13 lines

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.

Quoted 5 lines

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.

Quoted 9 lines

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:

fb_mmc.c_001 renamed from fb_mmc.c_000 +1 -0

  		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

fb_mmc.c_001 renamed from fb_mmc.c_000 +3 -0

+				buffer =
+				    (void *)c_header +
+

le16_to_cpu(s_header->chunk_hdr_sz); +

fb_mmc.c_001 renamed from fb_mmc.c_000 +1 -0

+				len =

le32_to_cpu(c_header->chunk_sz) * blk_sz;

fb_mmc.c_001 renamed from fb_mmc.c_000 +1 -0

+				ret_dfu = dfu_write_medium_mmc(dfu,

offset,
+
buffer, &len);

fb_mmc.c_001 renamed from fb_mmc.c_000 +2 -0

+				if (ret_dfu) {
+					printf("Write failed %lu\n",

len);

fb_mmc.c_001 renamed from fb_mmc.c_000 +2 -0

+					strcpy(response,
+					       "FAILmmc write

failure");

fb_mmc.c_001 renamed from fb_mmc.c_000 +6 -0

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

fb_mmc.c_001 renamed from fb_mmc.c_000 +2 -0

+#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--;
}

fb_mmc.c_001 renamed from fb_mmc.c_000 +1 -0

  		/* 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);

fb_mmc.c_001 renamed from fb_mmc.c_000 +4 -0

+#else
+		printf("Flashing Raw Image\n");
+
+		ret_dfu = dfu_write_medium_mmc(dfu, offset,

download_buffer, &len);

fb_mmc.c_001 renamed from fb_mmc.c_000 +2 -0

+		if (ret_dfu) {
+			printf("%s: failed writing to mmc device

%d\n",

fb_mmc.c_001 renamed from fb_mmc.c_000 +2 -0

+			       __func__, mmc_dev->dev);
+			strcpy(response, "FAILfailed writing to mmc

device");

fb_mmc.c_001 renamed from fb_mmc.c_000 +4 -0

+			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

Hi Steve,

Quoted 168 lines · Marek Vasut

Quoted 30 lines · Lukasz Majewski

Quoted 23 lines

Hi Steve,

Quoted 13 lines

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.

Quoted 5 lines

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.

Quoted 9 lines

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:

fb_mmc.c_001 renamed from fb_mmc.c_000 +1 -0

   		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

fb_mmc.c_001 renamed from fb_mmc.c_000 +3 -0

+				buffer =
+				    (void *)c_header +
+

le16_to_cpu(s_header->chunk_hdr_sz); +

fb_mmc.c_001 renamed from fb_mmc.c_000 +1 -0

+				len =

le32_to_cpu(c_header->chunk_sz) * blk_sz;

fb_mmc.c_001 renamed from fb_mmc.c_000 +1 -0

+				ret_dfu = dfu_write_medium_mmc(dfu,

offset,
+
buffer, &len);

fb_mmc.c_001 renamed from fb_mmc.c_000 +2 -0

+				if (ret_dfu) {
+					printf("Write failed %lu\n",

len);

fb_mmc.c_001 renamed from fb_mmc.c_000 +2 -0

+					strcpy(response,
+					       "FAILmmc write

failure");

fb_mmc.c_001 renamed from fb_mmc.c_000 +6 -0

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

fb_mmc.c_001 renamed from fb_mmc.c_000 +2 -0

+#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--;
   		}

fb_mmc.c_001 renamed from fb_mmc.c_000 +1 -0

   		/* 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);

fb_mmc.c_001 renamed from fb_mmc.c_000 +4 -0

+#else
+		printf("Flashing Raw Image\n");
+
+		ret_dfu = dfu_write_medium_mmc(dfu, offset,

download_buffer, &len);

fb_mmc.c_001 renamed from fb_mmc.c_000 +2 -0

+		if (ret_dfu) {
+			printf("%s: failed writing to mmc device

%d\n",

fb_mmc.c_001 renamed from fb_mmc.c_000 +2 -0

+			       __func__, mmc_dev->dev);
+			strcpy(response, "FAILfailed writing to mmc

device");

fb_mmc.c_001 renamed from fb_mmc.c_000 +4 -0

+			return;
+		}
+
+		printf("........ wrote %lu bytes to '%s'\n", len,

cmd); +#endif
}

Quoted 3 lines

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 :-).

Quoted 4 lines

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.

Quoted 6 lines

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

Quoted 4 lines

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.

Quoted 2 lines

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

That would be great.

Quoted 3 lines

Please advise,
Thanks, Steve

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

Thanks in advance, Steve

Quoted 223 lines · Lukasz Majewski

Hi Steve,

Quoted 170 lines · Marek Vasut

Quoted 34 lines · Lukasz Majewski

Quoted 25 lines

Hi Steve,

Quoted 13 lines

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.

Quoted 5 lines

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.

Quoted 9 lines

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:

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +1 -0

                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

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +3 -0

+                               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);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                               if (ret_dfu) {
+                                       printf("Write failed %lu\n",

len);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                                       strcpy(response,
+                                              "FAILmmc write

failure");

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +6 -0

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

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+#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--;
                }

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +1 -0

                /* 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);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +4 -0

+#else
+               printf("Flashing Raw Image\n");
+
+               ret_dfu = dfu_write_medium_mmc(dfu, offset,

download_buffer, &len);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+               if (ret_dfu) {
+                       printf("%s: failed writing to mmc device

%d\n",

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                              __func__, mmc_dev->dev);
+                       strcpy(response, "FAILfailed writing to mmc

device");

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +4 -0

+                       return;
+               }
+
+               printf("........ wrote %lu bytes to '%s'\n", len,

cmd); +#endif

Quoted 5 lines

        }

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 :-).

Quoted 4 lines

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.

Quoted 6 lines

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

Quoted 4 lines

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.

Quoted 2 lines

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

That would be great.

Quoted 3 lines

Please advise,
Thanks, Steve

Quoted 5 lines · Steve Rae

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

Quoted 229 lines

Thanks in advance, Steve

Quoted 223 lines · Lukasz Majewski

Hi Steve,

Quoted 170 lines · Marek Vasut

Quoted 34 lines · Lukasz Majewski

Quoted 25 lines

Hi Steve,

Quoted 13 lines

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.

Quoted 5 lines

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.

Quoted 9 lines

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:

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +1 -0

               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

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +3 -0

+                               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);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                               if (ret_dfu) {
+                                       printf("Write failed %lu\n",

len);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                                       strcpy(response,
+                                              "FAILmmc write

failure");

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +6 -0

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

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+#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--;
               }

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +1 -0

               /* 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);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +4 -0

+#else
+               printf("Flashing Raw Image\n");
+
+               ret_dfu = dfu_write_medium_mmc(dfu, offset,

download_buffer, &len);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+               if (ret_dfu) {
+                       printf("%s: failed writing to mmc device

%d\n",

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                              __func__, mmc_dev->dev);
+                       strcpy(response, "FAILfailed writing to mmc

device");

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +4 -0

+                       return;
+               }
+
+               printf("........ wrote %lu bytes to '%s'\n", len,

cmd); +#endif

Quoted 5 lines

       }

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 :-).

Quoted 4 lines

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.

Quoted 6 lines

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

Quoted 4 lines

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.

Quoted 2 lines

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

That would be great.

Quoted 3 lines

Please advise,
Thanks, Steve

Quoted 5 lines · Steve Rae

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

Quoted 229 lines

Thanks in advance, Steve

Quoted 223 lines · Lukasz Majewski

Hi Steve,

Quoted 170 lines · Marek Vasut

Quoted 34 lines · Lukasz Majewski

Quoted 25 lines

Hi Steve,

Quoted 13 lines

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.

Quoted 5 lines

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.

Quoted 9 lines

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:

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +1 -0

                 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

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +3 -0

+                               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);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                               if (ret_dfu) {
+                                       printf("Write failed %lu\n",

len);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                                       strcpy(response,
+                                              "FAILmmc write

failure");

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +6 -0

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

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+#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--;
                 }

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +1 -0

                 /* 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);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +4 -0

+#else
+               printf("Flashing Raw Image\n");
+
+               ret_dfu = dfu_write_medium_mmc(dfu, offset,

download_buffer, &len);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+               if (ret_dfu) {
+                       printf("%s: failed writing to mmc device

%d\n",

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                              __func__, mmc_dev->dev);
+                       strcpy(response, "FAILfailed writing to mmc

device");

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +4 -0

+                       return;
+               }
+
+               printf("........ wrote %lu bytes to '%s'\n", len,

cmd); +#endif

Quoted 5 lines

         }

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 :-).

Quoted 4 lines

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.

Quoted 6 lines

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

Quoted 4 lines

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.

Quoted 2 lines

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

That would be great.

Quoted 3 lines

Please advise,
Thanks, Steve

Rob,

Quoted 11 lines · Rob Herring

Quoted 7 lines · Steve Rae

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

Quoted 6 lines

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

Quoted 2 lines

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

Quoted 23 lines · Steve Rae

Rob,

Quoted 11 lines · Rob Herring

Quoted 7 lines · Steve Rae

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

Quoted 6 lines

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?

Quoted 5 lines

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

Yes, I believe it is dfu_write.

Quoted 7 lines

Quoted 2 lines

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

There are commands for SATA, SCSI (also SATA), eMMC, IDE, etc. They are all pretty much the same set of sub-commands and duplicate the same functionality. Those could all be combined to a single implementation and/or command for block devices. That part is not DFU related, but this problem then proliferates to other areas as it has for DFU. The file drivers/dfu/dfu_mmc.c is mostly generic, but has some eMMC dependencies with find_mmc_device and mmc_switch_part. So read and write are already pretty much generic, but there's still some work to do around device addressing/selection.

Rob

Quoted 1 line

which I already attempted to implement (and abandoned):

Quoted 1 line

for EFI Partitions, this did get accepted:

Quoted 1 line

I have code that works for eMMC device (and potentially for NAMD...)

Quoted 53 lines

Quoted 24 lines · Steve Rae

Rob,

Quoted 12 lines · Rob Herring

Quoted 7 lines · Steve Rae

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

Quoted 6 lines

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?

Quoted 5 lines

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

Yes, I believe it is dfu_write.

Quoted 7 lines

Quoted 2 lines

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

There are commands for SATA, SCSI (also SATA), eMMC, IDE, etc. They are all pretty much the same set of sub-commands and duplicate the same functionality. Those could all be combined to a single implementation and/or command for block devices. That part is not DFU related, but this problem then proliferates to other areas as it has for DFU. The file drivers/dfu/dfu_mmc.c is mostly generic, but has some eMMC dependencies with find_mmc_device and mmc_switch_part. So read and write are already pretty much generic, but there's still some work to do around device addressing/selection.

Quoted 2 lines

Rob

While I agree in general that to make everything generic is ideal, IMO, I don't think that there is a design or a roadmap to get us there yet I would suggest that any generic interface would also need to support:
- handling of multiple HW partitions (0=USER 1-BOOT1 2=BOOT2 etc.)

Quoted 1 line

Quoted 1 line

which I already attempted to implement (and abandoned):

    http://lists.denx.de/pipermail/u-boot/2014-May/180468.html

- handling of partition names

Quoted 1 line

Quoted 1 line

for EFI Partitions, this did get accepted:

    http://lists.denx.de/pipermail/u-boot/2014-May/180292.html

So now I would propose two phases:
(1) short term - get "fastboot flash" working (and "erase", and "oem format", etc.)

Quoted 2 lines

Quoted 2 lines

I have code that works for eMMC device (and potentially for NAMD...)

(2) longer term - define the "generic block device" (probably enhance "block_dev_desc_t" ?!?!?) and move the "short term solution" into this new design.

I will submit a "v2" to see if it will get accepted as part of the "short term solution".

Quoted 24 lines

Rob,

Quoted 18 lines · Rob Herring

Quoted 7 lines · Steve Rae

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?

Quoted 5 lines

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

Yes, I believe it is dfu_write.

Quoted 8 lines

Quoted 3 lines

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

There are commands for SATA, SCSI (also SATA), eMMC, IDE, etc. They are all pretty much the same set of sub-commands and duplicate the same functionality. Those could all be combined to a single implementation and/or command for block devices. That part is not DFU related, but this problem then proliferates to other areas as it has for DFU. The file drivers/dfu/dfu_mmc.c is mostly generic, but has some eMMC dependencies with find_mmc_device and mmc_switch_part.

So
read and write are already pretty much generic, but there's still some work to do around device addressing/selection.

Rob

Quoted 5 lines

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

Quoted 231 lines

Thanks in advance, Steve

Quoted 225 lines · Lukasz Majewski

Hi Steve,

Quoted 170 lines · Marek Vasut

Quoted 34 lines · Lukasz Majewski

Quoted 25 lines

Hi Steve,

Quoted 13 lines

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.

Quoted 5 lines

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.

Quoted 9 lines

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:

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +0 -0

                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

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +3 -0

+                               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);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                               if (ret_dfu) {
+                                       printf("Write failed

%lu\n", len);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                                       strcpy(response,
+                                              "FAILmmc write

failure");

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +6 -0

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

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+#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--;
                }

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +1 -0

                /* 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);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +4 -0

+#else
+               printf("Flashing Raw Image\n");
+
+               ret_dfu = dfu_write_medium_mmc(dfu, offset,

download_buffer, &len);

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+               if (ret_dfu) {
+                       printf("%s: failed writing to mmc device

%d\n",

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +2 -0

+                              __func__, mmc_dev->dev);
+                       strcpy(response, "FAILfailed writing to

mmc device");

fb_mmc.c_001 2014-06-20 14:17:48.688072764 -0700 renamed from fb_mmc.c_000 2014-06-20 14:13:43.271158073 -0700 +4 -0

+                       return;
+               }
+
+               printf("........ wrote %lu bytes to '%s'\n", len,

cmd); +#endif

Quoted 5 lines

        }

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 :-).

Quoted 4 lines

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.

Quoted 7 lines

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

Quoted 4 lines

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.

Quoted 2 lines

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

That would be great.

Quoted 3 lines

Please advise,
Thanks, Steve