[U-Boot] [PATCH 0/3] Enable mmc to write sparse images

[U-Boot] [PATCH 0/3] Enable mmc to write sparse images

[jassisinghbrar at gmail.com]

From: Jassi Brar <jaswinder.singh@linaro.org>

Hi,

 Having capability to flash sparse images from mmc subsystem could be useful.
For example, non-android/fastboot platforms could leverage the concept. Or
some platforms that need to 'pull' updates (get images over tftp and flash
using mmc). Or when we want to flash image at some non-zero offset from the
start of a partition. Or simply when USB-Device has issues for fastboot
to work (my itch).

 This patchset first cleans and makes write_sparse_image agnostic of
fastboot and then adds a new 'swrite' command to 'mmc'. 

Jassi Brar (3):
  fastboot: sparse: remove redundant argument to write_sparse_image
  fastboot: sparse: make write_sparse_image useable for non-fastboot
  mmc: support writing sparse images

 cmd/mmc.c              | 67 ++++++++++++++++++++++++++++++++++++++++++++++++
 common/fb_mmc.c        |  7 +++--
 common/fb_nand.c       |  6 +++--
 common/image-sparse.c  | 69 +++++++++++++++++++++++---------------------------
 include/image-sparse.h |  6 +++--
 5 files changed, 112 insertions(+), 43 deletions(-)

-- 
2.7.4

[U-Boot] [PATCH 1/3] fastboot: sparse: remove redundant argument to write_sparse_image

[jassisinghbrar at gmail.com]

From: Jassi Brar <jaswinder.singh@linaro.org>

'sz' has no use for write_sparse_image, remove it simplifying the api.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 common/fb_mmc.c        | 3 +--
 common/fb_nand.c       | 3 +--
 common/image-sparse.c  | 2 +-
 include/image-sparse.h | 2 +-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index cf5b77c..6993309 100644
--- a/common/fb_mmc.c
+++ b/common/fb_mmc.c
@@ -343,8 +343,7 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
 		       sparse.start);
 
 		sparse.priv = &sparse_priv;
-		write_sparse_image(&sparse, cmd, download_buffer,
-				   download_bytes);
+		write_sparse_image(&sparse, cmd, download_buffer);
 	} else {
 		write_raw_image(dev_desc, &info, cmd, download_buffer,
 				download_bytes);
diff --git a/common/fb_nand.c b/common/fb_nand.c
index aa28046..6130612 100644
--- a/common/fb_nand.c
+++ b/common/fb_nand.c
@@ -180,8 +180,7 @@ void fb_nand_flash_write(const char *cmd, void *download_buffer,
 		       sparse.start);
 
 		sparse.priv = &sparse_priv;
-		write_sparse_image(&sparse, cmd, download_buffer,
-				   download_bytes);
+		write_sparse_image(&sparse, cmd, download_buffer);
 	} else {
 		printf("Flashing raw image at offset 0x%llx\n",
 		       part->offset);
diff --git a/common/image-sparse.c b/common/image-sparse.c
index ddf5772..8ebd647 100644
--- a/common/image-sparse.c
+++ b/common/image-sparse.c
@@ -51,7 +51,7 @@
 
 void write_sparse_image(
 		struct sparse_storage *info, const char *part_name,
-		void *data, unsigned sz)
+		void *data)
 {
 	lbaint_t blk;
 	lbaint_t blkcnt;
diff --git a/include/image-sparse.h b/include/image-sparse.h
index b0cc500..d92d0f3 100644
--- a/include/image-sparse.h
+++ b/include/image-sparse.h
@@ -37,4 +37,4 @@ static inline int is_sparse_image(void *buf)
 }
 
 void write_sparse_image(struct sparse_storage *info, const char *part_name,
-			void *data, unsigned sz);
+			void *data);
-- 
2.7.4

[U-Boot] [U-Boot, 1/3] fastboot: sparse: remove redundant argument to write_sparse_image

[Tom Rini]

On Fri, Apr 06, 2018 at 12:04:52PM +0530, jassisinghbrar at gmail.com wrote:

> From: Jassi Brar <jaswinder.singh@linaro.org>
> 
> 'sz' has no use for write_sparse_image, remove it simplifying the api.
> 
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180508/b74e644e/attachment.sig>

[U-Boot] [PATCH 2/3] fastboot: sparse: make write_sparse_image useable for non-fastboot

[jassisinghbrar at gmail.com]

From: Jassi Brar <jaswinder.singh@linaro.org>

write_sparse_image could be useful for non-fastboot users.
For ex a platform, without usb-device/fastboot support, could
get sparse images over tftp and write using the mmc command.
Or non-android systems could also leverage the sparse format.

Towards that, this patch removes anything fastboot specific from
the write_sparse_image implementation. Which includes making the
function return integer as error code and calls for fastboot logging
via an optional callback function 'mssg'.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 common/fb_mmc.c        |  6 ++++-
 common/fb_nand.c       |  5 +++-
 common/image-sparse.c  | 69 +++++++++++++++++++++++---------------------------
 include/image-sparse.h |  6 +++--
 4 files changed, 45 insertions(+), 41 deletions(-)

diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index 6993309..d14b22f 100644
--- a/common/fb_mmc.c
+++ b/common/fb_mmc.c
@@ -330,6 +330,7 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
 	if (is_sparse_image(download_buffer)) {
 		struct fb_mmc_sparse sparse_priv;
 		struct sparse_storage sparse;
+		int err;
 
 		sparse_priv.dev_desc = dev_desc;
 
@@ -338,12 +339,15 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
 		sparse.size = info.size;
 		sparse.write = fb_mmc_sparse_write;
 		sparse.reserve = fb_mmc_sparse_reserve;
+		sparse.mssg = fastboot_fail;
 
 		printf("Flashing sparse image at offset " LBAFU "\n",
 		       sparse.start);
 
 		sparse.priv = &sparse_priv;
-		write_sparse_image(&sparse, cmd, download_buffer);
+		err = write_sparse_image(&sparse, cmd, download_buffer);
+		if (!err)
+			fastboot_okay("");
 	} else {
 		write_raw_image(dev_desc, &info, cmd, download_buffer,
 				download_bytes);
diff --git a/common/fb_nand.c b/common/fb_nand.c
index 6130612..ad2b112 100644
--- a/common/fb_nand.c
+++ b/common/fb_nand.c
@@ -175,12 +175,15 @@ void fb_nand_flash_write(const char *cmd, void *download_buffer,
 		sparse.size = part->size / sparse.blksz;
 		sparse.write = fb_nand_sparse_write;
 		sparse.reserve = fb_nand_sparse_reserve;
+		sparse.mssg = fastboot_fail;
 
 		printf("Flashing sparse image at offset " LBAFU "\n",
 		       sparse.start);
 
 		sparse.priv = &sparse_priv;
-		write_sparse_image(&sparse, cmd, download_buffer);
+		ret = write_sparse_image(&sparse, cmd, download_buffer);
+		if (!ret)
+			fastboot_okay("");
 	} else {
 		printf("Flashing raw image at offset 0x%llx\n",
 		       part->offset);
diff --git a/common/image-sparse.c b/common/image-sparse.c
index 8ebd647..9223b9a 100644
--- a/common/image-sparse.c
+++ b/common/image-sparse.c
@@ -41,7 +41,6 @@
 #include <malloc.h>
 #include <part.h>
 #include <sparse_format.h>
-#include <fastboot.h>
 
 #include <linux/math64.h>
 
@@ -49,9 +48,10 @@
 #define CONFIG_FASTBOOT_FLASH_FILLBUF_SIZE (1024 * 512)
 #endif
 
-void write_sparse_image(
-		struct sparse_storage *info, const char *part_name,
-		void *data)
+static void default_log(const char *ignored) {}
+
+int write_sparse_image(struct sparse_storage *info,
+		       const char *part_name, void *data)
 {
 	lbaint_t blk;
 	lbaint_t blkcnt;
@@ -83,6 +83,9 @@ void write_sparse_image(
 		data += (sparse_header->file_hdr_sz - sizeof(sparse_header_t));
 	}
 
+	if (!info->mssg)
+		info->mssg = default_log;
+
 	debug("=== Sparse Image Header ===\n");
 	debug("magic: 0x%x\n", sparse_header->magic);
 	debug("major_version: 0x%x\n", sparse_header->major_version);
@@ -101,8 +104,8 @@ void write_sparse_image(
 	if (offset) {
 		printf("%s: Sparse image block size issue [%u]\n",
 		       __func__, sparse_header->blk_sz);
-		fastboot_fail("sparse image block size issue");
-		return;
+		info->mssg("sparse image block size issue");
+		return -1;
 	}
 
 	puts("Flashing Sparse Image\n");
@@ -136,18 +139,16 @@ void write_sparse_image(
 		case CHUNK_TYPE_RAW:
 			if (chunk_header->total_sz !=
 			    (sparse_header->chunk_hdr_sz + chunk_data_sz)) {
-				fastboot_fail(
-					"Bogus chunk size for chunk type Raw");
-				return;
+				info->mssg("Bogus chunk size for chunk type Raw");
+				return -1;
 			}
 
 			if (blk + blkcnt > info->start + info->size) {
 				printf(
 				    "%s: Request would exceed partition size!\n",
 				    __func__);
-				fastboot_fail(
-				    "Request would exceed partition size!");
-				return;
+				info->mssg("Request would exceed partition size!");
+				return -1;
 			}
 
 			blks = info->write(info, blk, blkcnt, data);
@@ -156,9 +157,8 @@ void write_sparse_image(
 				printf("%s: %s" LBAFU " [" LBAFU "]\n",
 				       __func__, "Write failed, block #",
 				       blk, blks);
-				fastboot_fail(
-					      "flash write failure");
-				return;
+				info->mssg("flash write failure");
+				return -1;
 			}
 			blk += blks;
 			bytes_written += blkcnt * info->blksz;
@@ -169,9 +169,8 @@ void write_sparse_image(
 		case CHUNK_TYPE_FILL:
 			if (chunk_header->total_sz !=
 			    (sparse_header->chunk_hdr_sz + sizeof(uint32_t))) {
-				fastboot_fail(
-					"Bogus chunk size for chunk type FILL");
-				return;
+				info->mssg("Bogus chunk size for chunk type FILL");
+				return -1;
 			}
 
 			fill_buf = (uint32_t *)
@@ -180,9 +179,8 @@ void write_sparse_image(
 						info->blksz * fill_buf_num_blks,
 						ARCH_DMA_MINALIGN));
 			if (!fill_buf) {
-				fastboot_fail(
-					"Malloc failed for: CHUNK_TYPE_FILL");
-				return;
+				info->mssg("Malloc failed for: CHUNK_TYPE_FILL");
+				return -1;
 			}
 
 			fill_val = *(uint32_t *)data;
@@ -198,9 +196,8 @@ void write_sparse_image(
 				printf(
 				    "%s: Request would exceed partition size!\n",
 				    __func__);
-				fastboot_fail(
-				    "Request would exceed partition size!");
-				return;
+				info->mssg("Request would exceed partition size!");
+				return -1;
 			}
 
 			for (i = 0; i < blkcnt;) {
@@ -214,10 +211,9 @@ void write_sparse_image(
 					       __func__,
 					       "Write failed, block #",
 					       blk, j);
-					fastboot_fail(
-						      "flash write failure");
+					info->mssg("flash write failure");
 					free(fill_buf);
-					return;
+					return -1;
 				}
 				blk += blks;
 				i += j;
@@ -235,9 +231,8 @@ void write_sparse_image(
 		case CHUNK_TYPE_CRC32:
 			if (chunk_header->total_sz !=
 			    sparse_header->chunk_hdr_sz) {
-				fastboot_fail(
-					"Bogus chunk size for chunk type Dont Care");
-				return;
+				info->mssg("Bogus chunk size for chunk type Dont Care");
+				return -1;
 			}
 			total_blocks += chunk_header->chunk_sz;
 			data += chunk_data_sz;
@@ -246,8 +241,8 @@ void write_sparse_image(
 		default:
 			printf("%s: Unknown chunk type: %x\n", __func__,
 			       chunk_header->chunk_type);
-			fastboot_fail("Unknown chunk type");
-			return;
+			info->mssg("Unknown chunk type");
+			return -1;
 		}
 	}
 
@@ -255,10 +250,10 @@ void write_sparse_image(
 	      total_blocks, sparse_header->total_blks);
 	printf("........ wrote %u bytes to '%s'\n", bytes_written, part_name);
 
-	if (total_blocks != sparse_header->total_blks)
-		fastboot_fail("sparse image write failure");
-	else
-		fastboot_okay("");
+	if (total_blocks != sparse_header->total_blks) {
+		info->mssg("sparse image write failure");
+		return -1;
+	}
 
-	return;
+	return 0;
 }
diff --git a/include/image-sparse.h b/include/image-sparse.h
index d92d0f3..d2b1c2e 100644
--- a/include/image-sparse.h
+++ b/include/image-sparse.h
@@ -23,6 +23,8 @@ struct sparse_storage {
 	lbaint_t	(*reserve)(struct sparse_storage *info,
 				 lbaint_t blk,
 				 lbaint_t blkcnt);
+
+	void		(*mssg)(const char *str);
 };
 
 static inline int is_sparse_image(void *buf)
@@ -36,5 +38,5 @@ static inline int is_sparse_image(void *buf)
 	return 0;
 }
 
-void write_sparse_image(struct sparse_storage *info, const char *part_name,
-			void *data);
+int write_sparse_image(struct sparse_storage *info, const char *part_name,
+		       void *data);
-- 
2.7.4

[U-Boot] [U-Boot, 2/3] fastboot: sparse: make write_sparse_image useable for non-fastboot

[Tom Rini]

On Fri, Apr 06, 2018 at 12:05:09PM +0530, jassisinghbrar at gmail.com wrote:

> From: Jassi Brar <jaswinder.singh@linaro.org>
> 
> write_sparse_image could be useful for non-fastboot users.
> For ex a platform, without usb-device/fastboot support, could
> get sparse images over tftp and write using the mmc command.
> Or non-android systems could also leverage the sparse format.
> 
> Towards that, this patch removes anything fastboot specific from
> the write_sparse_image implementation. Which includes making the
> function return integer as error code and calls for fastboot logging
> via an optional callback function 'mssg'.
> 
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180508/d575f86c/attachment.sig>

[U-Boot] [PATCH 3/3] mmc: support writing sparse images

[jassisinghbrar at gmail.com]

From: Jassi Brar <jaswinder.singh@linaro.org>

Provide an alternate path for sparse-images to be
written to MMC. For example, via tftp on platforms
that don't support fastboot protocol. Or when an
image is to written at some offset, rather than the
start of a partition.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 cmd/mmc.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/cmd/mmc.c b/cmd/mmc.c
index 58fdc36..5ced6e7 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -9,6 +9,8 @@
 #include <command.h>
 #include <console.h>
 #include <mmc.h>
+#include <sparse_format.h>
+#include <image-sparse.h>
 
 static int curr_device = -1;
 
@@ -308,6 +310,69 @@ static int do_mmc_read(cmd_tbl_t *cmdtp, int flag,
 }
 
 #if CONFIG_IS_ENABLED(MMC_WRITE)
+static lbaint_t mmc_sparse_write(struct sparse_storage *info, lbaint_t blk,
+				 lbaint_t blkcnt, const void *buffer)
+{
+	struct blk_desc *dev_desc = info->priv;
+
+	return blk_dwrite(dev_desc, blk, blkcnt, buffer);
+}
+
+static lbaint_t mmc_sparse_reserve(struct sparse_storage *info,
+				   lbaint_t blk, lbaint_t blkcnt)
+{
+	return blkcnt;
+}
+
+static int do_mmc_sparse_write(cmd_tbl_t *cmdtp, int flag,
+			       int argc, char * const argv[])
+{
+	struct sparse_storage sparse;
+	struct blk_desc *dev_desc;
+	struct mmc *mmc;
+	char dest[11];
+	void *addr;
+	u32 blk;
+
+	if (argc != 3)
+		return CMD_RET_USAGE;
+
+	addr = (void *)simple_strtoul(argv[1], NULL, 16);
+	blk = simple_strtoul(argv[2], NULL, 16);
+
+	if (!is_sparse_image(addr)) {
+		printf("Not a sparse image\n");
+		return CMD_RET_FAILURE;
+	}
+
+	mmc = init_mmc_device(curr_device, false);
+	if (!mmc)
+		return CMD_RET_FAILURE;
+
+	printf("\nMMC Sparse write: dev # %d, block # %d ... ",
+	       curr_device, blk);
+
+	if (mmc_getwp(mmc) == 1) {
+		printf("Error: card is write protected!\n");
+		return CMD_RET_FAILURE;
+	}
+
+	dev_desc = mmc_get_blk_desc(mmc);
+	sparse.priv = dev_desc;
+	sparse.blksz = 512;
+	sparse.start = blk;
+	sparse.size = dev_desc->lba - blk;
+	sparse.write = mmc_sparse_write;
+	sparse.reserve = mmc_sparse_reserve;
+	sparse.mssg = NULL;
+	sprintf(dest, "0x%08lX", sparse.start * sparse.blksz);
+
+	if (write_sparse_image(&sparse, dest, addr))
+		return CMD_RET_FAILURE;
+	else
+		return CMD_RET_SUCCESS;
+}
+
 static int do_mmc_write(cmd_tbl_t *cmdtp, int flag,
 			int argc, char * const argv[])
 {
@@ -802,6 +867,7 @@ static cmd_tbl_t cmd_mmc[] = {
 	U_BOOT_CMD_MKENT(read, 4, 1, do_mmc_read, "", ""),
 #if CONFIG_IS_ENABLED(MMC_WRITE)
 	U_BOOT_CMD_MKENT(write, 4, 0, do_mmc_write, "", ""),
+	U_BOOT_CMD_MKENT(swrite, 3, 0, do_mmc_sparse_write, "", ""),
 	U_BOOT_CMD_MKENT(erase, 3, 0, do_mmc_erase, "", ""),
 #endif
 	U_BOOT_CMD_MKENT(rescan, 1, 1, do_mmc_rescan, "", ""),
@@ -858,6 +924,7 @@ U_BOOT_CMD(
 	"info - display info of the current MMC device\n"
 	"mmc read addr blk# cnt\n"
 	"mmc write addr blk# cnt\n"
+	"mmc swrite addr blk#\n"
 	"mmc erase blk# cnt\n"
 	"mmc rescan\n"
 	"mmc part - lists available partition on current mmc device\n"
-- 
2.7.4

[U-Boot] [U-Boot,3/3] mmc: support writing sparse images

[Tom Rini]

On Fri, Apr 06, 2018 at 12:05:24PM +0530, jassisinghbrar at gmail.com wrote:

> From: Jassi Brar <jaswinder.singh@linaro.org>
> 
> Provide an alternate path for sparse-images to be
> written to MMC. For example, via tftp on platforms
> that don't support fastboot protocol. Or when an
> image is to written at some offset, rather than the
> start of a partition.
> 
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180508/beda500c/attachment.sig>

[U-Boot] [U-Boot,3/3] mmc: support writing sparse images

[Jassi Brar]

Hi Tom,

On Tue, May 8, 2018 at 10:45 PM, Tom Rini <trini@konsulko.com> wrote:
> On Fri, Apr 06, 2018 at 12:05:24PM +0530, jassisinghbrar at gmail.com wrote:
>
>> From: Jassi Brar <jaswinder.singh@linaro.org>
>>
>> Provide an alternate path for sparse-images to be
>> written to MMC. For example, via tftp on platforms
>> that don't support fastboot protocol. Or when an
>> image is to written at some offset, rather than the
>> start of a partition.
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>
> Applied to u-boot/master, thanks!
>
I see you modified the patch to protect the feature with
CONFIG_FASTBOOT_FLASH, which kills the purpose -- this feature is for
platforms that don't support fastboot.

Do you want me to send the patch to revert the protection?

Thanks.

[U-Boot] [U-Boot,3/3] mmc: support writing sparse images

[Tom Rini]

On Mon, May 14, 2018 at 06:42:41PM +0530, Jassi Brar wrote:
> Hi Tom,
> 
> On Tue, May 8, 2018 at 10:45 PM, Tom Rini <trini@konsulko.com> wrote:
> > On Fri, Apr 06, 2018 at 12:05:24PM +0530, jassisinghbrar at gmail.com wrote:
> >
> >> From: Jassi Brar <jaswinder.singh@linaro.org>
> >>
> >> Provide an alternate path for sparse-images to be
> >> written to MMC. For example, via tftp on platforms
> >> that don't support fastboot protocol. Or when an
> >> image is to written at some offset, rather than the
> >> start of a partition.
> >>
> >> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> >
> > Applied to u-boot/master, thanks!
>
> I see you modified the patch to protect the feature with
> CONFIG_FASTBOOT_FLASH, which kills the purpose -- this feature is for
> platforms that don't support fastboot.
> 
> Do you want me to send the patch to revert the protection?

Sorry, I guess maybe things weren't clear enough all around, and we
should (functionally) revert patches 2 and 3 and try something
different.  It is OK to say "lets make writing sparse images more widely
available".  It's not OK to make every platform with MMC write grow a
decent bit in binary size.  Making a quick pass at re-enabling things on
a platform without fastboot support already grew things by nearly 2KiB.
The other part which is I believe got me down this path was that without
a change to common/Makefile to always (outside of SPL) include
common/image-sparse.o things don't link.

In sum, a new patch to add an option to allow people to opt-in for
swrite would be good.  And please make sure to do something like:
diff --git a/common/Makefile b/common/Makefile
index d0681c7dd96a..92b2aa1ca8f0 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -120,6 +120,7 @@ ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
 obj-y += fb_nand.o
 endif
 endif
+obj-$(CONFIG_MMC_WRITE) += image-sparse.o
 endif
 
 ifdef CONFIG_CMD_EEPROM_LAYOUT

too so it links everywhere.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180514/d7364bbe/attachment.sig>

[U-Boot] [U-Boot,3/3] mmc: support writing sparse images

[Alex Kiernan]

On Mon, May 14, 2018 at 3:47 PM Tom Rini <trini@konsulko.com> wrote:

> On Mon, May 14, 2018 at 06:42:41PM +0530, Jassi Brar wrote:
> > Hi Tom,
> >
> > On Tue, May 8, 2018 at 10:45 PM, Tom Rini <trini@konsulko.com> wrote:
> > > On Fri, Apr 06, 2018 at 12:05:24PM +0530, jassisinghbrar at gmail.com
wrote:
> > >
> > >> From: Jassi Brar <jaswinder.singh@linaro.org>
> > >>
> > >> Provide an alternate path for sparse-images to be
> > >> written to MMC. For example, via tftp on platforms
> > >> that don't support fastboot protocol. Or when an
> > >> image is to written at some offset, rather than the
> > >> start of a partition.
> > >>
> > >> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> > >
> > > Applied to u-boot/master, thanks!
> >
> > I see you modified the patch to protect the feature with
> > CONFIG_FASTBOOT_FLASH, which kills the purpose -- this feature is for
> > platforms that don't support fastboot.
> >
> > Do you want me to send the patch to revert the protection?

> Sorry, I guess maybe things weren't clear enough all around, and we
> should (functionally) revert patches 2 and 3 and try something
> different.  It is OK to say "lets make writing sparse images more widely
> available".  It's not OK to make every platform with MMC write grow a
> decent bit in binary size.  Making a quick pass at re-enabling things on
> a platform without fastboot support already grew things by nearly 2KiB.
> The other part which is I believe got me down this path was that without
> a change to common/Makefile to always (outside of SPL) include
> common/image-sparse.o things don't link.

> In sum, a new patch to add an option to allow people to opt-in for
> swrite would be good.  And please make sure to do something like:
> diff --git a/common/Makefile b/common/Makefile
> index d0681c7dd96a..92b2aa1ca8f0 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -120,6 +120,7 @@ ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
>       obj-y += fb_nand.o
>       endif
>       endif
> +obj-$(CONFIG_MMC_WRITE) += image-sparse.o
>       endif

Isn't this just move image-sparse to lib and add a separate guard for it
(LIB_IMAGE_SPARSE?) which can be selected by both FASTBOOT and a new
command symbol (CMD_MMC_SWRITE)?

This is all overlapping with the UDP fastboot code I've been posting, so
I'd kinda like it to fit nicely with that, rather than have to refactor it
to fit something different!

--
Alex Kiernan

[U-Boot] [U-Boot,3/3] mmc: support writing sparse images

[Tom Rini]

On Mon, May 14, 2018 at 05:14:42PM +0100, Alex Kiernan wrote:
> On Mon, May 14, 2018 at 3:47 PM Tom Rini <trini@konsulko.com> wrote:
> 
> > On Mon, May 14, 2018 at 06:42:41PM +0530, Jassi Brar wrote:
> > > Hi Tom,
> > >
> > > On Tue, May 8, 2018 at 10:45 PM, Tom Rini <trini@konsulko.com> wrote:
> > > > On Fri, Apr 06, 2018 at 12:05:24PM +0530, jassisinghbrar at gmail.com
> wrote:
> > > >
> > > >> From: Jassi Brar <jaswinder.singh@linaro.org>
> > > >>
> > > >> Provide an alternate path for sparse-images to be
> > > >> written to MMC. For example, via tftp on platforms
> > > >> that don't support fastboot protocol. Or when an
> > > >> image is to written at some offset, rather than the
> > > >> start of a partition.
> > > >>
> > > >> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> > > >
> > > > Applied to u-boot/master, thanks!
> > >
> > > I see you modified the patch to protect the feature with
> > > CONFIG_FASTBOOT_FLASH, which kills the purpose -- this feature is for
> > > platforms that don't support fastboot.
> > >
> > > Do you want me to send the patch to revert the protection?
> 
> > Sorry, I guess maybe things weren't clear enough all around, and we
> > should (functionally) revert patches 2 and 3 and try something
> > different.  It is OK to say "lets make writing sparse images more widely
> > available".  It's not OK to make every platform with MMC write grow a
> > decent bit in binary size.  Making a quick pass at re-enabling things on
> > a platform without fastboot support already grew things by nearly 2KiB.
> > The other part which is I believe got me down this path was that without
> > a change to common/Makefile to always (outside of SPL) include
> > common/image-sparse.o things don't link.
> 
> > In sum, a new patch to add an option to allow people to opt-in for
> > swrite would be good.  And please make sure to do something like:
> > diff --git a/common/Makefile b/common/Makefile
> > index d0681c7dd96a..92b2aa1ca8f0 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -120,6 +120,7 @@ ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
> >       obj-y += fb_nand.o
> >       endif
> >       endif
> > +obj-$(CONFIG_MMC_WRITE) += image-sparse.o
> >       endif
> 
> Isn't this just move image-sparse to lib and add a separate guard for it
> (LIB_IMAGE_SPARSE?) which can be selected by both FASTBOOT and a new
> command symbol (CMD_MMC_SWRITE)?
> 
> This is all overlapping with the UDP fastboot code I've been posting, so
> I'd kinda like it to fit nicely with that, rather than have to refactor it
> to fit something different!

That sounds like a good idea to me, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180514/44e1f35e/attachment.sig>

[U-Boot] [U-Boot,3/3] mmc: support writing sparse images

[Alex Kiernan]

On Mon, May 14, 2018 at 9:50 PM Tom Rini <trini@konsulko.com> wrote:

> On Mon, May 14, 2018 at 05:14:42PM +0100, Alex Kiernan wrote:
> > On Mon, May 14, 2018 at 3:47 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > > On Mon, May 14, 2018 at 06:42:41PM +0530, Jassi Brar wrote:
> > > > Hi Tom,
> > > >
> > > > On Tue, May 8, 2018 at 10:45 PM, Tom Rini <trini@konsulko.com>
wrote:
> > > > > On Fri, Apr 06, 2018 at 12:05:24PM +0530, jassisinghbrar at gmail.com
> > wrote:
> > > > >
> > > > >> From: Jassi Brar <jaswinder.singh@linaro.org>
> > > > >>
> > > > >> Provide an alternate path for sparse-images to be
> > > > >> written to MMC. For example, via tftp on platforms
> > > > >> that don't support fastboot protocol. Or when an
> > > > >> image is to written at some offset, rather than the
> > > > >> start of a partition.
> > > > >>
> > > > >> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> > > > >
> > > > > Applied to u-boot/master, thanks!
> > > >
> > > > I see you modified the patch to protect the feature with
> > > > CONFIG_FASTBOOT_FLASH, which kills the purpose -- this feature is
for
> > > > platforms that don't support fastboot.
> > > >
> > > > Do you want me to send the patch to revert the protection?
> >
> > > Sorry, I guess maybe things weren't clear enough all around, and we
> > > should (functionally) revert patches 2 and 3 and try something
> > > different.  It is OK to say "lets make writing sparse images more
widely
> > > available".  It's not OK to make every platform with MMC write grow a
> > > decent bit in binary size.  Making a quick pass at re-enabling things
on
> > > a platform without fastboot support already grew things by nearly
2KiB.
> > > The other part which is I believe got me down this path was that
without
> > > a change to common/Makefile to always (outside of SPL) include
> > > common/image-sparse.o things don't link.
> >
> > > In sum, a new patch to add an option to allow people to opt-in for
> > > swrite would be good.  And please make sure to do something like:
> > > diff --git a/common/Makefile b/common/Makefile
> > > index d0681c7dd96a..92b2aa1ca8f0 100644
> > > --- a/common/Makefile
> > > +++ b/common/Makefile
> > > @@ -120,6 +120,7 @@ ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
> > >       obj-y += fb_nand.o
> > >       endif
> > >       endif
> > > +obj-$(CONFIG_MMC_WRITE) += image-sparse.o
> > >       endif
> >
> > Isn't this just move image-sparse to lib and add a separate guard for it
> > (LIB_IMAGE_SPARSE?) which can be selected by both FASTBOOT and a new
> > command symbol (CMD_MMC_SWRITE)?
> >
> > This is all overlapping with the UDP fastboot code I've been posting, so
> > I'd kinda like it to fit nicely with that, rather than have to refactor
it
> > to fit something different!

> That sounds like a good idea to me, thanks!


I'll work it into that patch series... having just had a quick look at it,
it's going to be easiest part way through it after I've done most of the
Kconfig reworking.

-- 
Alex Kiernan

[U-Boot] [PATCH 0/3] Enable mmc to write sparse images

[Jassi Brar]

On Fri, Apr 6, 2018 at 12:04 PM,  <jassisinghbrar@gmail.com> wrote:
> From: Jassi Brar <jaswinder.singh@linaro.org>
>
> Hi,
>
>  Having capability to flash sparse images from mmc subsystem could be useful.
> For example, non-android/fastboot platforms could leverage the concept. Or
> some platforms that need to 'pull' updates (get images over tftp and flash
> using mmc). Or when we want to flash image at some non-zero offset from the
> start of a partition. Or simply when USB-Device has issues for fastboot
> to work (my itch).
>
>  This patchset first cleans and makes write_sparse_image agnostic of
> fastboot and then adds a new 'swrite' command to 'mmc'.
>
> Jassi Brar (3):
>   fastboot: sparse: remove redundant argument to write_sparse_image
>   fastboot: sparse: make write_sparse_image useable for non-fastboot
>   mmc: support writing sparse images
>
>  cmd/mmc.c              | 67 ++++++++++++++++++++++++++++++++++++++++++++++++
>  common/fb_mmc.c        |  7 +++--
>  common/fb_nand.c       |  6 +++--
>  common/image-sparse.c  | 69 +++++++++++++++++++++++---------------------------
>  include/image-sparse.h |  6 +++--
>  5 files changed, 112 insertions(+), 43 deletions(-)
>
A polite reminder to look into the patchset please.

Thank