All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
@ 2018-04-24  9:37 Alex Kiernan
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 1/5] dfu: Refactor fastboot_okay/fail to take response Alex Kiernan
                   ` (7 more replies)
  0 siblings, 8 replies; 31+ messages in thread
From: Alex Kiernan @ 2018-04-24  9:37 UTC (permalink / raw)
  To: u-boot


This series merges the fastboot UDP support from AOSP into mainline
U-Boot.

Open questions:

- what's the correct way of attributing the original authors? I've added
  Co-authored-by, is that right? checkpatch doesn't seem to know any
  of the co- tags
- currently there's no NAND support and I've no way of testing that, so
  my inclination is towards leaving it like that until someone with that
  particular itch to scratch can look at it
- you can select both USB and UDP fastboot, but the comments in the
  AOSP code suggest that needs fixing - again, I've no board I can test
  USB fastboot on
- the USB and UDP code want consolidating, with this series there would
  then be two separate implementations of the same protocol
- the syntax for the USB fastboot command changes from `fastboot <controller>`
  to `fastboot usb <controller>`, that feels unacceptable and we probably
  want something like `fastboot <controller>` or `fastboot udp`?
- unrelated to this series, but a show-stopper for me, there's no FIT image
  support, but that doesn't feel unsolveable - something like add an option
  to pass in the load address and/or use loadaddr, then something else to
  let you override the bootm invocation on the server side

I've not tested all the code paths yet, but the obvious stuff works
(basic interaction, download, boot) - every interaction elicits a
`WARNING: unknown variable: slot-count` on the console; I'm guessing that
my local end is sending a getvar for that, but I've not investigated.

Without any way of testing any of the USB/NAND code I'm nervous of wading
into that kind of refactoring, would that be a pre-requisite for merging?


Alex Kiernan (5):
  dfu: Refactor fastboot_okay/fail to take response
  dfu: Extract fastboot_okay/fail to fb_common.c
  net: dfu: Merge AOSP UDP fastboot
  dfu: Resolve Kconfig dependency loops
  net: dfu: Support building without MMC

 cmd/fastboot.c                  |  32 ++-
 cmd/fastboot/Kconfig            |  21 +-
 cmd/net.c                       |   6 +
 common/Makefile                 |   4 +
 common/fb_common.c              |  44 ++++
 common/fb_mmc.c                 | 114 ++++++---
 common/fb_nand.c                |  31 +--
 common/image-sparse.c           |  41 ++-
 drivers/usb/gadget/f_fastboot.c |  36 +--
 include/fastboot.h              |  17 +-
 include/fb_mmc.h                |   4 +-
 include/fb_nand.h               |   4 +-
 include/image-sparse.h          |   2 +-
 include/net.h                   |   6 +-
 include/net/fastboot.h          |  27 ++
 net/Makefile                    |   1 +
 net/fastboot.c                  | 548 ++++++++++++++++++++++++++++++++++++++++
 net/net.c                       |   9 +
 18 files changed, 824 insertions(+), 123 deletions(-)
 create mode 100644 common/fb_common.c
 create mode 100644 include/net/fastboot.h
 create mode 100644 net/fastboot.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 1/5] dfu: Refactor fastboot_okay/fail to take response
  2018-04-24  9:37 [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Kiernan
@ 2018-04-24  9:37 ` Alex Kiernan
  2018-04-25  7:27   ` Lukasz Majewski
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 2/5] dfu: Extract fastboot_okay/fail to fb_common.c Alex Kiernan
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: Alex Kiernan @ 2018-04-24  9:37 UTC (permalink / raw)
  To: u-boot

Add the response string as a parameter to fastboot_okay/fail, instead
of modifying a global, to match the contract expected by the AOSP
U-Boot code.

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

 common/fb_mmc.c                 | 80 ++++++++++++++++++++++-------------------
 common/fb_nand.c                | 31 ++++++++--------
 common/image-sparse.c           | 41 +++++++++++----------
 drivers/usb/gadget/f_fastboot.c | 35 +++++++-----------
 include/fastboot.h              |  4 +--
 include/fb_mmc.h                |  4 +--
 include/fb_nand.h               |  4 +--
 include/image-sparse.h          |  2 +-
 8 files changed, 100 insertions(+), 101 deletions(-)

diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index cf5b77c..02864aa 100644
--- a/common/fb_mmc.c
+++ b/common/fb_mmc.c
@@ -74,7 +74,7 @@ static lbaint_t fb_mmc_sparse_reserve(struct sparse_storage *info,
 
 static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info,
 		const char *part_name, void *buffer,
-		unsigned int download_bytes)
+		unsigned int download_bytes, char *response)
 {
 	lbaint_t blkcnt;
 	lbaint_t blks;
@@ -85,7 +85,7 @@ static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info,
 
 	if (blkcnt > info->size) {
 		pr_err("too large for partition: '%s'\n", part_name);
-		fastboot_fail("too large for partition");
+		fastboot_fail("too large for partition", response);
 		return;
 	}
 
@@ -94,13 +94,13 @@ static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info,
 	blks = blk_dwrite(dev_desc, info->start, blkcnt, buffer);
 	if (blks != blkcnt) {
 		pr_err("failed writing to device %d\n", dev_desc->devnum);
-		fastboot_fail("failed writing to device");
+		fastboot_fail("failed writing to device", response);
 		return;
 	}
 
 	printf("........ wrote " LBAFU " bytes to '%s'\n", blkcnt * info->blksz,
 	       part_name);
-	fastboot_okay("");
+	fastboot_okay("", response);
 }
 
 #ifdef CONFIG_ANDROID_BOOT_IMAGE
@@ -115,7 +115,8 @@ static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info,
  */
 static lbaint_t fb_mmc_get_boot_header(struct blk_desc *dev_desc,
 				       disk_partition_t *info,
-				       struct andr_img_hdr *hdr)
+				       struct andr_img_hdr *hdr,
+				       char *response)
 {
 	ulong sector_size;		/* boot partition sector size */
 	lbaint_t hdr_sectors;		/* boot image header sectors count */
@@ -126,7 +127,7 @@ static lbaint_t fb_mmc_get_boot_header(struct blk_desc *dev_desc,
 	hdr_sectors = DIV_ROUND_UP(sizeof(struct andr_img_hdr), sector_size);
 	if (hdr_sectors == 0) {
 		pr_err("invalid number of boot sectors: 0");
-		fastboot_fail("invalid number of boot sectors: 0");
+		fastboot_fail("invalid number of boot sectors: 0", response);
 		return 0;
 	}
 
@@ -134,7 +135,8 @@ static lbaint_t fb_mmc_get_boot_header(struct blk_desc *dev_desc,
 	res = blk_dread(dev_desc, info->start, hdr_sectors, (void *)hdr);
 	if (res != hdr_sectors) {
 		pr_err("cannot read header from boot partition");
-		fastboot_fail("cannot read header from boot partition");
+		fastboot_fail("cannot read header from boot partition",
+			      response);
 		return 0;
 	}
 
@@ -142,7 +144,7 @@ static lbaint_t fb_mmc_get_boot_header(struct blk_desc *dev_desc,
 	res = android_image_check_header(hdr);
 	if (res != 0) {
 		pr_err("bad boot image magic");
-		fastboot_fail("boot partition not initialized");
+		fastboot_fail("boot partition not initialized", response);
 		return 0;
 	}
 
@@ -160,7 +162,8 @@ static lbaint_t fb_mmc_get_boot_header(struct blk_desc *dev_desc,
  */
 static int fb_mmc_update_zimage(struct blk_desc *dev_desc,
 				void *download_buffer,
-				unsigned int download_bytes)
+				unsigned int download_bytes,
+				char *response)
 {
 	uintptr_t hdr_addr;			/* boot image header address */
 	struct andr_img_hdr *hdr;		/* boot image header */
@@ -180,7 +183,7 @@ static int fb_mmc_update_zimage(struct blk_desc *dev_desc,
 	res = part_get_info_by_name(dev_desc, BOOT_PARTITION_NAME, &info);
 	if (res < 0) {
 		pr_err("cannot find boot partition");
-		fastboot_fail("cannot find boot partition");
+		fastboot_fail("cannot find boot partition", response);
 		return -1;
 	}
 
@@ -189,17 +192,18 @@ static int fb_mmc_update_zimage(struct blk_desc *dev_desc,
 	hdr = (struct andr_img_hdr *)hdr_addr;
 
 	/* Read boot image header */
-	hdr_sectors = fb_mmc_get_boot_header(dev_desc, &info, hdr);
+	hdr_sectors = fb_mmc_get_boot_header(dev_desc, &info, hdr, response);
 	if (hdr_sectors == 0) {
 		pr_err("unable to read boot image header");
-		fastboot_fail("unable to read boot image header");
+		fastboot_fail("unable to read boot image header", response);
 		return -1;
 	}
 
 	/* Check if boot image has second stage in it (we don't support it) */
 	if (hdr->second_size > 0) {
 		pr_err("moving second stage is not supported yet");
-		fastboot_fail("moving second stage is not supported yet");
+		fastboot_fail("moving second stage is not supported yet",
+			      response);
 		return -1;
 	}
 
@@ -217,7 +221,8 @@ static int fb_mmc_update_zimage(struct blk_desc *dev_desc,
 			ramdisk_buffer);
 	if (res != ramdisk_sectors) {
 		pr_err("cannot read ramdisk from boot partition");
-		fastboot_fail("cannot read ramdisk from boot partition");
+		fastboot_fail("cannot read ramdisk from boot partition",
+			      response);
 		return -1;
 	}
 
@@ -226,7 +231,7 @@ static int fb_mmc_update_zimage(struct blk_desc *dev_desc,
 	res = blk_dwrite(dev_desc, info.start, hdr_sectors, (void *)hdr);
 	if (res == 0) {
 		pr_err("cannot writeback boot image header");
-		fastboot_fail("cannot write back boot image header");
+		fastboot_fail("cannot write back boot image header", response);
 		return -1;
 	}
 
@@ -238,7 +243,7 @@ static int fb_mmc_update_zimage(struct blk_desc *dev_desc,
 			 download_buffer);
 	if (res == 0) {
 		pr_err("cannot write new kernel");
-		fastboot_fail("cannot write new kernel");
+		fastboot_fail("cannot write new kernel", response);
 		return -1;
 	}
 
@@ -250,18 +255,18 @@ static int fb_mmc_update_zimage(struct blk_desc *dev_desc,
 			 ramdisk_buffer);
 	if (res == 0) {
 		pr_err("cannot write back original ramdisk");
-		fastboot_fail("cannot write back original ramdisk");
+		fastboot_fail("cannot write back original ramdisk", response);
 		return -1;
 	}
 
 	puts("........ zImage was updated in boot partition\n");
-	fastboot_okay("");
+	fastboot_okay("", response);
 	return 0;
 }
 #endif
 
 void fb_mmc_flash_write(const char *cmd, void *download_buffer,
-			unsigned int download_bytes)
+			unsigned int download_bytes, char *response)
 {
 	struct blk_desc *dev_desc;
 	disk_partition_t info;
@@ -269,7 +274,7 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
 	dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
 	if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
 		pr_err("invalid mmc device\n");
-		fastboot_fail("invalid mmc device");
+		fastboot_fail("invalid mmc device", response);
 		return;
 	}
 
@@ -280,16 +285,17 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
 		if (is_valid_gpt_buf(dev_desc, download_buffer)) {
 			printf("%s: invalid GPT - refusing to write to flash\n",
 			       __func__);
-			fastboot_fail("invalid GPT partition");
+			fastboot_fail("invalid GPT partition", response);
 			return;
 		}
 		if (write_mbr_and_gpt_partitions(dev_desc, download_buffer)) {
 			printf("%s: writing GPT partitions failed\n", __func__);
-			fastboot_fail("writing GPT partitions failed");
+			fastboot_fail("writing GPT partitions failed",
+				      response);
 			return;
 		}
 		printf("........ success\n");
-		fastboot_okay("");
+		fastboot_okay("", response);
 		return;
 	}
 #endif
@@ -300,30 +306,32 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
 		if (is_valid_dos_buf(download_buffer)) {
 			printf("%s: invalid MBR - refusing to write to flash\n",
 			       __func__);
-			fastboot_fail("invalid MBR partition");
+			fastboot_fail("invalid MBR partition", response);
 			return;
 		}
 		if (write_mbr_partition(dev_desc, download_buffer)) {
 			printf("%s: writing MBR partition failed\n", __func__);
-			fastboot_fail("writing MBR partition failed");
+			fastboot_fail("writing MBR partition failed",
+				      response);
 			return;
 		}
 		printf("........ success\n");
-		fastboot_okay("");
+		fastboot_okay("", response);
 		return;
 	}
 #endif
 
 #ifdef CONFIG_ANDROID_BOOT_IMAGE
 	if (strncasecmp(cmd, "zimage", 6) == 0) {
-		fb_mmc_update_zimage(dev_desc, download_buffer, download_bytes);
+		fb_mmc_update_zimage(dev_desc, download_buffer,
+				     download_bytes, response);
 		return;
 	}
 #endif
 
 	if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) {
 		pr_err("cannot find partition: '%s'\n", cmd);
-		fastboot_fail("cannot find partition");
+		fastboot_fail("cannot find partition", response);
 		return;
 	}
 
@@ -344,14 +352,14 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
 
 		sparse.priv = &sparse_priv;
 		write_sparse_image(&sparse, cmd, download_buffer,
-				   download_bytes);
+				   download_bytes, response);
 	} else {
 		write_raw_image(dev_desc, &info, cmd, download_buffer,
-				download_bytes);
+				download_bytes, response);
 	}
 }
 
-void fb_mmc_erase(const char *cmd)
+void fb_mmc_erase(const char *cmd, char *response)
 {
 	int ret;
 	struct blk_desc *dev_desc;
@@ -361,21 +369,21 @@ void fb_mmc_erase(const char *cmd)
 
 	if (mmc == NULL) {
 		pr_err("invalid mmc device");
-		fastboot_fail("invalid mmc device");
+		fastboot_fail("invalid mmc device", response);
 		return;
 	}
 
 	dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
 	if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
 		pr_err("invalid mmc device");
-		fastboot_fail("invalid mmc device");
+		fastboot_fail("invalid mmc device", response);
 		return;
 	}
 
 	ret = part_get_info_by_name_or_alias(dev_desc, cmd, &info);
 	if (ret < 0) {
 		pr_err("cannot find partition: '%s'", cmd);
-		fastboot_fail("cannot find partition");
+		fastboot_fail("cannot find partition", response);
 		return;
 	}
 
@@ -394,11 +402,11 @@ void fb_mmc_erase(const char *cmd)
 	blks = blk_derase(dev_desc, blks_start, blks_size);
 	if (blks != blks_size) {
 		pr_err("failed erasing from device %d", dev_desc->devnum);
-		fastboot_fail("failed erasing from device");
+		fastboot_fail("failed erasing from device", response);
 		return;
 	}
 
 	printf("........ erased " LBAFU " bytes from '%s'\n",
 	       blks_size * info.blksz, cmd);
-	fastboot_okay("");
+	fastboot_okay("", response);
 }
diff --git a/common/fb_nand.c b/common/fb_nand.c
index aa28046..3a0b101 100644
--- a/common/fb_nand.c
+++ b/common/fb_nand.c
@@ -32,7 +32,8 @@ __weak int board_fastboot_write_partition_setup(char *name)
 
 static int fb_nand_lookup(const char *partname,
 			  struct mtd_info **mtd,
-			  struct part_info **part)
+			  struct part_info **part,
+			  char *response)
 {
 	struct mtd_device *dev;
 	int ret;
@@ -41,21 +42,21 @@ static int fb_nand_lookup(const char *partname,
 	ret = mtdparts_init();
 	if (ret) {
 		pr_err("Cannot initialize MTD partitions\n");
-		fastboot_fail("cannot init mtdparts");
+		fastboot_fail("cannot init mtdparts", response);
 		return ret;
 	}
 
 	ret = find_dev_and_part(partname, &dev, &pnum, part);
 	if (ret) {
 		pr_err("cannot find partition: '%s'", partname);
-		fastboot_fail("cannot find partition");
+		fastboot_fail("cannot find partition", response);
 		return ret;
 	}
 
 	if (dev->id->type != MTD_DEV_TYPE_NAND) {
 		pr_err("partition '%s' is not stored on a NAND device",
 		      partname);
-		fastboot_fail("not a NAND device");
+		fastboot_fail("not a NAND device", response);
 		return -EINVAL;
 	}
 
@@ -146,16 +147,16 @@ static lbaint_t fb_nand_sparse_reserve(struct sparse_storage *info,
 }
 
 void fb_nand_flash_write(const char *cmd, void *download_buffer,
-			 unsigned int download_bytes)
+			 unsigned int download_bytes, char *response)
 {
 	struct part_info *part;
 	struct mtd_info *mtd = NULL;
 	int ret;
 
-	ret = fb_nand_lookup(cmd, &mtd, &part);
+	ret = fb_nand_lookup(cmd, &mtd, &part, response);
 	if (ret) {
 		pr_err("invalid NAND device");
-		fastboot_fail("invalid NAND device");
+		fastboot_fail("invalid NAND device", response);
 		return;
 	}
 
@@ -181,7 +182,7 @@ void fb_nand_flash_write(const char *cmd, void *download_buffer,
 
 		sparse.priv = &sparse_priv;
 		write_sparse_image(&sparse, cmd, download_buffer,
-				   download_bytes);
+				   download_bytes, response);
 	} else {
 		printf("Flashing raw image at offset 0x%llx\n",
 		       part->offset);
@@ -194,23 +195,23 @@ void fb_nand_flash_write(const char *cmd, void *download_buffer,
 	}
 
 	if (ret) {
-		fastboot_fail("error writing the image");
+		fastboot_fail("error writing the image", response);
 		return;
 	}
 
-	fastboot_okay("");
+	fastboot_okay("", response);
 }
 
-void fb_nand_erase(const char *cmd)
+void fb_nand_erase(const char *cmd, char *response)
 {
 	struct part_info *part;
 	struct mtd_info *mtd = NULL;
 	int ret;
 
-	ret = fb_nand_lookup(cmd, &mtd, &part);
+	ret = fb_nand_lookup(cmd, &mtd, &part, response);
 	if (ret) {
 		pr_err("invalid NAND device");
-		fastboot_fail("invalid NAND device");
+		fastboot_fail("invalid NAND device", response);
 		return;
 	}
 
@@ -221,9 +222,9 @@ void fb_nand_erase(const char *cmd)
 	ret = _fb_nand_erase(mtd, part);
 	if (ret) {
 		pr_err("failed erasing from device %s", mtd->name);
-		fastboot_fail("failed erasing from device");
+		fastboot_fail("failed erasing from device", response);
 		return;
 	}
 
-	fastboot_okay("");
+	fastboot_okay("", response);
 }
diff --git a/common/image-sparse.c b/common/image-sparse.c
index ddf5772..616c2bd 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, unsigned int sz, char *response)
 {
 	lbaint_t blk;
 	lbaint_t blkcnt;
@@ -101,7 +101,7 @@ 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");
+		fastboot_fail("sparse image block size issue", response);
 		return;
 	}
 
@@ -136,8 +136,8 @@ 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");
+				fastboot_fail("Bogus chunk size for chunk type Raw",
+					      response);
 				return;
 			}
 
@@ -145,8 +145,8 @@ void write_sparse_image(
 				printf(
 				    "%s: Request would exceed partition size!\n",
 				    __func__);
-				fastboot_fail(
-				    "Request would exceed partition size!");
+				fastboot_fail("Request would exceed partition size!",
+					      response);
 				return;
 			}
 
@@ -156,8 +156,7 @@ void write_sparse_image(
 				printf("%s: %s" LBAFU " [" LBAFU "]\n",
 				       __func__, "Write failed, block #",
 				       blk, blks);
-				fastboot_fail(
-					      "flash write failure");
+				fastboot_fail("flash write failure", response);
 				return;
 			}
 			blk += blks;
@@ -169,8 +168,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");
+				fastboot_fail("Bogus chunk size for chunk type FILL",
+					      response);
 				return;
 			}
 
@@ -180,8 +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");
+				fastboot_fail("Malloc failed for: CHUNK_TYPE_FILL",
+					      response);
 				return;
 			}
 
@@ -198,8 +197,8 @@ void write_sparse_image(
 				printf(
 				    "%s: Request would exceed partition size!\n",
 				    __func__);
-				fastboot_fail(
-				    "Request would exceed partition size!");
+				fastboot_fail("Request would exceed partition size!",
+					      response);
 				return;
 			}
 
@@ -214,8 +213,8 @@ void write_sparse_image(
 					       __func__,
 					       "Write failed, block #",
 					       blk, j);
-					fastboot_fail(
-						      "flash write failure");
+					fastboot_fail("flash write failure",
+						      response);
 					free(fill_buf);
 					return;
 				}
@@ -235,8 +234,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");
+				fastboot_fail("Bogus chunk size for chunk type Dont Care",
+					      response);
 				return;
 			}
 			total_blocks += chunk_header->chunk_sz;
@@ -246,7 +245,7 @@ void write_sparse_image(
 		default:
 			printf("%s: Unknown chunk type: %x\n", __func__,
 			       chunk_header->chunk_type);
-			fastboot_fail("Unknown chunk type");
+			fastboot_fail("Unknown chunk type", response);
 			return;
 		}
 	}
@@ -256,9 +255,9 @@ void write_sparse_image(
 	printf("........ wrote %u bytes to '%s'\n", bytes_written, part_name);
 
 	if (total_blocks != sparse_header->total_blks)
-		fastboot_fail("sparse image write failure");
+		fastboot_fail("sparse image write failure", response);
 	else
-		fastboot_okay("");
+		fastboot_okay("", response);
 
 	return;
 }
diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index 7acffb6..6ae1d97 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -151,18 +151,16 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req);
 static int strcmp_l1(const char *s1, const char *s2);
 
 
-static char *fb_response_str;
-
-void fastboot_fail(const char *reason)
+void fastboot_fail(const char *reason, char *response)
 {
-	strncpy(fb_response_str, "FAIL\0", 5);
-	strncat(fb_response_str, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
+	strncpy(response, "FAIL\0", 5);
+	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
 }
 
-void fastboot_okay(const char *reason)
+void fastboot_okay(const char *reason, char *response)
 {
-	strncpy(fb_response_str, "OKAY\0", 5);
-	strncat(fb_response_str, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
+	strncpy(response, "OKAY\0", 5);
+	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
 }
 
 static void fastboot_complete(struct usb_ep *ep, struct usb_request *req)
@@ -598,18 +596,14 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req)
 		return;
 	}
 
-	/* initialize the response buffer */
-	fb_response_str = response;
-
-	fastboot_fail("no flash device defined");
+	fastboot_fail("no flash device defined", response);
 #ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
 	fb_mmc_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR,
-			   download_bytes);
+			   download_bytes, response);
 #endif
 #ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
-	fb_nand_flash_write(cmd,
-			    (void *)CONFIG_FASTBOOT_BUF_ADDR,
-			    download_bytes);
+	fb_nand_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR,
+			    download_bytes, response);
 #endif
 	fastboot_tx_write_str(response);
 }
@@ -650,15 +644,12 @@ static void cb_erase(struct usb_ep *ep, struct usb_request *req)
 		return;
 	}
 
-	/* initialize the response buffer */
-	fb_response_str = response;
-
-	fastboot_fail("no flash device defined");
+	fastboot_fail("no flash device defined", response);
 #ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
-	fb_mmc_erase(cmd);
+	fb_mmc_erase(cmd, response);
 #endif
 #ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
-	fb_nand_erase(cmd);
+	fb_nand_erase(cmd, response);
 #endif
 	fastboot_tx_write_str(response);
 }
diff --git a/include/fastboot.h b/include/fastboot.h
index 616631e..f22080a 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -16,7 +16,7 @@
 /* The 64 defined bytes plus \0 */
 #define FASTBOOT_RESPONSE_LEN	(64 + 1)
 
-void fastboot_fail(const char *reason);
-void fastboot_okay(const char *reason);
+void fastboot_fail(const char *reason, char *response);
+void fastboot_okay(const char *reason, char *response);
 
 #endif /* _FASTBOOT_H_ */
diff --git a/include/fb_mmc.h b/include/fb_mmc.h
index 12b99cb..402ba9b 100644
--- a/include/fb_mmc.h
+++ b/include/fb_mmc.h
@@ -5,5 +5,5 @@
  */
 
 void fb_mmc_flash_write(const char *cmd, void *download_buffer,
-			unsigned int download_bytes);
-void fb_mmc_erase(const char *cmd);
+			unsigned int download_bytes, char *response);
+void fb_mmc_erase(const char *cmd, char *response);
diff --git a/include/fb_nand.h b/include/fb_nand.h
index aaf7cf7..88bdf36 100644
--- a/include/fb_nand.h
+++ b/include/fb_nand.h
@@ -6,5 +6,5 @@
  */
 
 void fb_nand_flash_write(const char *cmd, void *download_buffer,
-			 unsigned int download_bytes);
-void fb_nand_erase(const char *cmd);
+			 unsigned int download_bytes, char *response);
+void fb_nand_erase(const char *cmd, char *response);
diff --git a/include/image-sparse.h b/include/image-sparse.h
index b0cc500..02453c8 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, unsigned int sz, char *response);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 2/5] dfu: Extract fastboot_okay/fail to fb_common.c
  2018-04-24  9:37 [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Kiernan
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 1/5] dfu: Refactor fastboot_okay/fail to take response Alex Kiernan
@ 2018-04-24  9:37 ` Alex Kiernan
  2018-04-25  7:32   ` Lukasz Majewski
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 3/5] net: dfu: Merge AOSP UDP fastboot Alex Kiernan
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: Alex Kiernan @ 2018-04-24  9:37 UTC (permalink / raw)
  To: u-boot

Added common/fb_common.c, where fastboot_okay/fail are implemented
so we can call them from a non-USB implementation.

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

 common/Makefile                 |  4 ++++
 common/fb_common.c              | 26 ++++++++++++++++++++++++++
 drivers/usb/gadget/f_fastboot.c | 13 -------------
 3 files changed, 30 insertions(+), 13 deletions(-)
 create mode 100644 common/fb_common.c

diff --git a/common/Makefile b/common/Makefile
index 7011dad..8177341 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -124,6 +124,10 @@ endif
 endif
 endif
 
+ifneq ($(or $(CONFIG_USB_FUNCTION_FASTBOOT),$(CONFIG_UDP_FUNCTION_FASTBOOT)),)
+obj-y += fb_common.o
+endif
+
 ifdef CONFIG_CMD_EEPROM_LAYOUT
 obj-y += eeprom/eeprom_field.o eeprom/eeprom_layout.o
 endif
diff --git a/common/fb_common.c b/common/fb_common.c
new file mode 100644
index 0000000..53cffe3
--- /dev/null
+++ b/common/fb_common.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2008 - 2009
+ * Windriver, <www.windriver.com>
+ * Tom Rix <Tom.Rix@windriver.com>
+ *
+ * Copyright 2011 Sebastian Andrzej Siewior <bigeasy@linutronix.de>
+ *
+ * Copyright 2014 Linaro, Ltd.
+ * Rob Herring <robh@kernel.org>
+ */
+
+#include <common.h>
+#include <fastboot.h>
+
+void fastboot_fail(const char *reason, char *response)
+{
+	strncpy(response, "FAIL\0", 5);
+	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
+}
+
+void fastboot_okay(const char *reason, char *response)
+{
+	strncpy(response, "OKAY\0", 5);
+	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
+}
diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index 6ae1d97..fda4505 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -150,19 +150,6 @@ static struct usb_gadget_strings *fastboot_strings[] = {
 static void rx_handler_command(struct usb_ep *ep, struct usb_request *req);
 static int strcmp_l1(const char *s1, const char *s2);
 
-
-void fastboot_fail(const char *reason, char *response)
-{
-	strncpy(response, "FAIL\0", 5);
-	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
-}
-
-void fastboot_okay(const char *reason, char *response)
-{
-	strncpy(response, "OKAY\0", 5);
-	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
-}
-
 static void fastboot_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	int status = req->status;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 3/5] net: dfu: Merge AOSP UDP fastboot
  2018-04-24  9:37 [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Kiernan
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 1/5] dfu: Refactor fastboot_okay/fail to take response Alex Kiernan
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 2/5] dfu: Extract fastboot_okay/fail to fb_common.c Alex Kiernan
@ 2018-04-24  9:37 ` Alex Kiernan
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 4/5] dfu: Resolve Kconfig dependency loops Alex Kiernan
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 31+ messages in thread
From: Alex Kiernan @ 2018-04-24  9:37 UTC (permalink / raw)
  To: u-boot

Merge UDP fastboot support from AOSP:

https://android.googlesource.com/platform/external/u-boot/+/android-o-mr1-iot-preview-8

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
Co-authored-by: Alex Deymo <deymo@google.com>
Co-authored-by: Jocelyn Bohr <bohr@google.com>
Co-authored-by: Neal Ostrem <nealo@google.com>
---

 cmd/fastboot.c         |  32 ++-
 cmd/fastboot/Kconfig   |  18 +-
 cmd/net.c              |   6 +
 common/fb_common.c     |  18 ++
 common/fb_mmc.c        |  34 +++-
 include/fastboot.h     |  13 ++
 include/net.h          |   6 +-
 include/net/fastboot.h |  27 +++
 net/Makefile           |   1 +
 net/fastboot.c         | 542 +++++++++++++++++++++++++++++++++++++++++++++++++
 net/net.c              |   9 +
 11 files changed, 692 insertions(+), 14 deletions(-)
 create mode 100644 include/net/fastboot.h
 create mode 100644 net/fastboot.c

diff --git a/cmd/fastboot.c b/cmd/fastboot.c
index 8adcca5..ce5ac1e 100644
--- a/cmd/fastboot.c
+++ b/cmd/fastboot.c
@@ -11,18 +11,37 @@
 #include <command.h>
 #include <console.h>
 #include <g_dnl.h>
+#include <net.h>
 #include <usb.h>
 
 static int do_fastboot(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 {
+#ifdef CONFIG_USB_FUNCTION_FASTBOOT
 	int controller_index;
 	char *usb_controller;
 	int ret;
+#endif
 
 	if (argc < 2)
 		return CMD_RET_USAGE;
 
-	usb_controller = argv[1];
+	if (!strcmp(argv[1], "udp")) {
+#ifndef CONFIG_UDP_FUNCTION_FASTBOOT
+		pr_err("Fastboot UDP not enabled\n");
+		return -1;
+#else
+		return do_fastboot_udp(cmdtp, flag, argc, argv);
+#endif
+	}
+
+	if (strcmp(argv[1], "usb") || argc < 3)
+		return CMD_RET_USAGE;
+
+#ifndef CONFIG_USB_FUNCTION_FASTBOOT
+	pr_err("Fastboot USB not enabled\n");
+	return -1;
+#else
+	usb_controller = argv[2];
 	controller_index = simple_strtoul(usb_controller, NULL, 0);
 
 	ret = board_usb_init(controller_index, USB_INIT_DEVICE);
@@ -59,11 +78,14 @@ exit:
 	board_usb_cleanup(controller_index, USB_INIT_DEVICE);
 
 	return ret;
+#endif
 }
 
 U_BOOT_CMD(
-	fastboot, 2, 1, do_fastboot,
-	"use USB Fastboot protocol",
-	"<USB_controller>\n"
-	"    - run as a fastboot usb device"
+	fastboot, 3, 1, do_fastboot,
+	"use USB or UDP Fastboot protocol",
+	"[usb,udp] <USB_controller>\n"
+	" - run as a fastboot usb or udp device\n"
+	"   usb: specify <USB_controller>\n"
+	"   udp: requires ip_addr set and ethernet initialized\n"
 );
diff --git a/cmd/fastboot/Kconfig b/cmd/fastboot/Kconfig
index 0d2c2f1..0c57616 100644
--- a/cmd/fastboot/Kconfig
+++ b/cmd/fastboot/Kconfig
@@ -16,17 +16,25 @@ config USB_FUNCTION_FASTBOOT
 	help
 	  This enables the USB part of the fastboot gadget.
 
+config UDP_FUNCTION_FASTBOOT
+	select NET
+	bool "Enable fastboot protocol over UDP"
+	help
+	  This enables the fastboot protocol over UDP.
+
 config CMD_FASTBOOT
 	bool "Enable FASTBOOT command"
+	depends on USB_FUNCTION_FASTBOOT || UDP_FUNCTION_FASTBOOT
 	help
 	  This enables the command "fastboot" which enables the Android
-	  fastboot mode for the platform's USB device. Fastboot is a USB
-	  protocol for downloading images, flashing and device control
-	  used on Android devices.
+	  fastboot mode for the platform. Fastboot is a protocol for
+	  downloading images, flashing and device control used on
+	  Android devices. Fastboot requires either network stack
+	  enabled or support for acting as a USB device.
 
 	  See doc/README.android-fastboot for more information.
 
-if USB_FUNCTION_FASTBOOT
+if USB_FUNCTION_FASTBOOT || UDP_FUNCTION_FASTBOOT
 
 config FASTBOOT_BUF_ADDR
 	hex "Define FASTBOOT buffer address"
@@ -129,6 +137,6 @@ config FASTBOOT_MBR_NAME
 	  specified on the "fastboot flash" command line matches the value
 	  defined here. The default target name for updating MBR is "mbr".
 
-endif # USB_FUNCTION_FASTBOOT
+endif # USB_FUNCTION_FASTBOOT || UDP_FUNCTION_FASTBOOT
 
 endif # FASTBOOT
diff --git a/cmd/net.c b/cmd/net.c
index 67888d4..668f344 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -74,6 +74,12 @@ U_BOOT_CMD(
 );
 #endif
 
+#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
+int do_fastboot_udp(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	return netboot_common(FASTBOOT, cmdtp, argc, argv);
+}
+#endif
 
 #ifdef CONFIG_CMD_RARP
 int do_rarpb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
diff --git a/common/fb_common.c b/common/fb_common.c
index 53cffe3..cd79a82 100644
--- a/common/fb_common.c
+++ b/common/fb_common.c
@@ -12,6 +12,9 @@
 
 #include <common.h>
 #include <fastboot.h>
+#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
+#include <net/fastboot.h>
+#endif
 
 void fastboot_fail(const char *reason, char *response)
 {
@@ -24,3 +27,18 @@ void fastboot_okay(const char *reason, char *response)
 	strncpy(response, "OKAY\0", 5);
 	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
 }
+
+void timed_send_info(ulong *start, const char *msg)
+{
+#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
+	/* Initialize timer */
+	if (*start == 0)
+		*start = get_timer(0);
+	ulong time = get_timer(*start);
+	/* Send INFO packet to host every 30 seconds */
+	if (time >= 30000) {
+		*start = get_timer(0);
+		fastboot_send_info(msg);
+	}
+#endif
+}
diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index 02864aa..304bda1 100644
--- a/common/fb_mmc.c
+++ b/common/fb_mmc.c
@@ -29,6 +29,9 @@
 #define CONFIG_FASTBOOT_MBR_NAME "mbr"
 #endif
 
+#define FASTBOOT_MAX_BLK_WRITE 16384
+static ulong timer;
+
 #define BOOT_PARTITION_NAME "boot"
 
 struct fb_mmc_sparse {
@@ -57,13 +60,38 @@ static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
 	return ret;
 }
 
+static lbaint_t fb_mmc_blk_write(struct blk_desc *block_dev, lbaint_t start,
+				 lbaint_t blkcnt, const void *buffer)
+{
+	lbaint_t blk = start;
+	lbaint_t blks_written;
+	lbaint_t cur_blkcnt;
+	lbaint_t blks = 0;
+	int i;
+
+	for (i = 0; i < blkcnt; i += FASTBOOT_MAX_BLK_WRITE) {
+		cur_blkcnt = min((int)blkcnt - i, FASTBOOT_MAX_BLK_WRITE);
+		if (!buffer) {
+			timed_send_info(&timer, "writing");
+			blks_written = blk_dwrite(block_dev, blk, cur_blkcnt,
+						  buffer + (i * block_dev->blksz));
+		} else {
+			timed_send_info(&timer, "erasing");
+			blks_written = blk_derase(block_dev, blk, cur_blkcnt);
+		}
+		blk += blks_written;
+		blks += blks_written;
+	}
+	return blks;
+}
+
 static lbaint_t fb_mmc_sparse_write(struct sparse_storage *info,
 		lbaint_t blk, lbaint_t blkcnt, const void *buffer)
 {
 	struct fb_mmc_sparse *sparse = info->priv;
 	struct blk_desc *dev_desc = sparse->dev_desc;
 
-	return blk_dwrite(dev_desc, blk, blkcnt, buffer);
+	return fb_mmc_blk_write(dev_desc, blk, blkcnt, buffer);
 }
 
 static lbaint_t fb_mmc_sparse_reserve(struct sparse_storage *info,
@@ -91,7 +119,7 @@ static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info,
 
 	puts("Flashing Raw Image\n");
 
-	blks = blk_dwrite(dev_desc, info->start, blkcnt, buffer);
+	blks = fb_mmc_blk_write(dev_desc, info->start, blkcnt, buffer);
 	if (blks != blkcnt) {
 		pr_err("failed writing to device %d\n", dev_desc->devnum);
 		fastboot_fail("failed writing to device", response);
@@ -399,7 +427,7 @@ void fb_mmc_erase(const char *cmd, char *response)
 	printf("Erasing blocks " LBAFU " to " LBAFU " due to alignment\n",
 	       blks_start, blks_start + blks_size);
 
-	blks = blk_derase(dev_desc, blks_start, blks_size);
+	blks = fb_mmc_blk_write(dev_desc, blks_start, blks_size, NULL);
 	if (blks != blks_size) {
 		pr_err("failed erasing from device %d", dev_desc->devnum);
 		fastboot_fail("failed erasing from device", response);
diff --git a/include/fastboot.h b/include/fastboot.h
index f22080a..f93a03b 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -19,4 +19,17 @@
 void fastboot_fail(const char *reason, char *response);
 void fastboot_okay(const char *reason, char *response);
 
+/**
+ * Send an INFO packet during long commands based on timer. If
+ * CONFIG_UDP_FUNCTION_FASTBOOT is defined, an INFO packet is sent
+ * if the time is 30 seconds after start. Else, noop.
+ *
+ * TODO: Handle the situation where both UDP and USB fastboot are
+ *       enabled.
+ *
+ * @param start:  Time since last INFO packet was sent.
+ * @param msg:    String describing the reason for waiting
+ */
+void timed_send_info(ulong *start, const char *msg);
+
 #endif /* _FASTBOOT_H_ */
diff --git a/include/net.h b/include/net.h
index 3469811..890ae27 100644
--- a/include/net.h
+++ b/include/net.h
@@ -535,7 +535,7 @@ extern int		net_restart_wrap;	/* Tried all network devices */
 
 enum proto_t {
 	BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
-	TFTPSRV, TFTPPUT, LINKLOCAL
+	TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT
 };
 
 extern char	net_boot_file_name[1024];/* Boot File name */
@@ -549,6 +549,10 @@ extern char *net_dns_resolve;		/* The host to resolve  */
 extern char *net_dns_env_var;		/* the env var to put the ip into */
 #endif
 
+#if defined(CONFIG_UDP_FUNCTION_FASTBOOT)
+int do_fastboot_udp(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
+#endif
+
 #if defined(CONFIG_CMD_PING)
 extern struct in_addr net_ping_ip;	/* the ip address to ping */
 #endif
diff --git a/include/net/fastboot.h b/include/net/fastboot.h
new file mode 100644
index 0000000..c0dd033
--- /dev/null
+++ b/include/net/fastboot.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (C) 2016 The Android Open Source Project
+ */
+
+#ifndef __NET_FASTBOOT_H__
+#define __NET_FASTBOOT_H__
+
+/**********************************************************************/
+/*
+ *	Global functions and variables.
+ */
+
+/**
+ * Wait for incoming fastboot comands.
+ */
+void fastboot_start_server(void);
+/**
+ * Send an INFO packet during long commands
+ *
+ * @param msg: String describing the reason for waiting
+ */
+void fastboot_send_info(const char *msg);
+
+/**********************************************************************/
+
+#endif /* __NET_FASTBOOT_H__ */
diff --git a/net/Makefile b/net/Makefile
index ce6e5ad..3489ce5 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_CMD_PING) += ping.o
 obj-$(CONFIG_CMD_RARP) += rarp.o
 obj-$(CONFIG_CMD_SNTP) += sntp.o
 obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
+obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
 
 # Disable this warning as it is triggered by:
 # sprintf(buf, index ? "foo%d" : "foo", index)
diff --git a/net/fastboot.c b/net/fastboot.c
new file mode 100644
index 0000000..32cb581
--- /dev/null
+++ b/net/fastboot.c
@@ -0,0 +1,542 @@
+// SPDX-License-Identifier: BSD-2-Clause
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ */
+
+#include <common.h>
+#include <fastboot.h>
+#include <fb_mmc.h>
+#include <net.h>
+#include <net/fastboot.h>
+#include <part.h>
+#include <stdlib.h>
+#include <version.h>
+
+/* Fastboot port # defined in spec */
+#define WELL_KNOWN_PORT 5554
+
+enum {
+	FASTBOOT_ERROR = 0,
+	FASTBOOT_QUERY = 1,
+	FASTBOOT_INIT = 2,
+	FASTBOOT_FASTBOOT = 3,
+};
+
+struct __packed fastboot_header {
+	uchar id;
+	uchar flags;
+	unsigned short seq;
+};
+
+#define PACKET_SIZE 1024
+#define FASTBOOT_HEADER_SIZE sizeof(struct fastboot_header)
+#define DATA_SIZE (PACKET_SIZE - FASTBOOT_HEADER_SIZE)
+#define FASTBOOT_VERSION "0.4"
+
+/* Sequence number sent for every packet */
+static unsigned short fb_sequence_number = 1;
+static const unsigned short fb_packet_size = PACKET_SIZE;
+static const unsigned short fb_udp_version = 1;
+
+/* Keep track of last packet for resubmission */
+static uchar last_packet[PACKET_SIZE];
+static unsigned int last_packet_len;
+
+/* Parsed from first fastboot command packet */
+static char *cmd_string;
+static char *cmd_parameter;
+
+/* Fastboot download parameters */
+static unsigned int bytes_received;
+static unsigned int bytes_expected;
+static unsigned int image_size;
+
+static struct in_addr fastboot_remote_ip;
+/* The UDP port at their end */
+static int fastboot_remote_port;
+/* The UDP port at our end */
+static int fastboot_our_port;
+
+static void fb_getvar(char *);
+static void fb_download(char *, unsigned int, char *);
+static void fb_flash(char *);
+static void fb_erase(char *);
+static void fb_continue(char *);
+static void fb_reboot(char *);
+static void boot_downloaded_image(void);
+static void cleanup_command_data(void);
+static void write_fb_response(const char *, const char *, char *);
+
+void fastboot_send_info(const char *msg)
+{
+	uchar *packet;
+	uchar *packet_base;
+	int len = 0;
+	char response[FASTBOOT_RESPONSE_LEN] = {0};
+
+	struct fastboot_header fb_response_header = {
+		.id = FASTBOOT_FASTBOOT,
+		.flags = 0,
+		.seq = htons(fb_sequence_number)
+	};
+	++fb_sequence_number;
+	packet = net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
+	packet_base = packet;
+
+	/* Write headers */
+	memcpy(packet, &fb_response_header, sizeof(fb_response_header));
+	packet += sizeof(fb_response_header);
+	/* Write response */
+	write_fb_response("INFO", msg, response);
+	memcpy(packet, response, strlen(response));
+	packet += strlen(response);
+
+	len = packet - packet_base;
+
+	/* Save packet for retransmitting */
+	last_packet_len = len;
+	memcpy(last_packet, packet_base, last_packet_len);
+
+	net_send_udp_packet(net_server_ethaddr, fastboot_remote_ip,
+			    fastboot_remote_port, fastboot_our_port, len);
+}
+
+/**
+ * Constructs and sends a packet in response to received fastboot packet
+ *
+ * @param fb_header            Header for response packet
+ * @param fastboot_data        Pointer to received fastboot data
+ * @param fastboot_data_len    Length of received fastboot data
+ * @param retransmit           Nonzero if sending last sent packet
+ */
+static void fastboot_send(struct fastboot_header fb_header, char *fastboot_data,
+			  unsigned int fastboot_data_len, uchar retransmit)
+{
+	uchar *packet;
+	uchar *packet_base;
+	int len = 0;
+	const char *error_msg = "An error occurred.";
+	short tmp;
+	struct fastboot_header fb_response_header = fb_header;
+	char response[FASTBOOT_RESPONSE_LEN] = {0};
+	/*
+	 *	We will always be sending some sort of packet, so
+	 *	cobble together the packet headers now.
+	 */
+	packet = net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
+	packet_base = packet;
+
+	/* Resend last packet */
+	if (retransmit) {
+		memcpy(packet, last_packet, last_packet_len);
+		net_send_udp_packet(net_server_ethaddr, fastboot_remote_ip,
+				    fastboot_remote_port, fastboot_our_port,
+				    last_packet_len);
+		return;
+	}
+
+	fb_response_header.seq = htons(fb_response_header.seq);
+	memcpy(packet, &fb_response_header, sizeof(fb_response_header));
+	packet += sizeof(fb_response_header);
+
+	switch (fb_header.id) {
+	case FASTBOOT_QUERY:
+		tmp = htons(fb_sequence_number);
+		memcpy(packet, &tmp, sizeof(tmp));
+		packet += sizeof(tmp);
+		break;
+	case FASTBOOT_INIT:
+		tmp = htons(fb_udp_version);
+		memcpy(packet, &tmp, sizeof(tmp));
+		packet += sizeof(tmp);
+		tmp = htons(fb_packet_size);
+		memcpy(packet, &tmp, sizeof(tmp));
+		packet += sizeof(tmp);
+		break;
+	case FASTBOOT_ERROR:
+		memcpy(packet, error_msg, strlen(error_msg));
+		packet += strlen(error_msg);
+		break;
+	case FASTBOOT_FASTBOOT:
+		if (!cmd_string) {
+			/* Parse command and send ack */
+			cmd_parameter = fastboot_data;
+			cmd_string = strsep(&cmd_parameter, ":");
+			cmd_string = strdup(cmd_string);
+			if (cmd_parameter)
+				cmd_parameter = strdup(cmd_parameter);
+		} else if (!strcmp("getvar", cmd_string)) {
+			fb_getvar(response);
+		} else if (!strcmp("download", cmd_string)) {
+			fb_download(fastboot_data, fastboot_data_len, response);
+		} else if (!strcmp("flash", cmd_string)) {
+			fb_flash(response);
+		} else if (!strcmp("erase", cmd_string)) {
+			fb_erase(response);
+		} else if (!strcmp("boot", cmd_string)) {
+			write_fb_response("OKAY", "", response);
+		} else if (!strcmp("continue", cmd_string)) {
+			fb_continue(response);
+		} else if (!strncmp("reboot", cmd_string, 6)) {
+			fb_reboot(response);
+		} else if (!strcmp("set_active", cmd_string)) {
+			/* A/B not implemented, for now do nothing */
+			write_fb_response("OKAY", "", response);
+		} else {
+			pr_err("command %s not implemented.\n", cmd_string);
+			write_fb_response("FAIL", "unrecognized command",
+					  response);
+		}
+		/* Sent some INFO packets, need to update sequence number in
+		 * header
+		 */
+		if (fb_header.seq != fb_sequence_number) {
+			fb_response_header.seq = htons(fb_sequence_number);
+			memcpy(packet_base, &fb_response_header,
+			       sizeof(fb_response_header));
+		}
+		/* Write response to packet */
+		memcpy(packet, response, strlen(response));
+		packet += strlen(response);
+		break;
+	default:
+		pr_err("ID %d not implemented.\n", fb_header.id);
+		return;
+	}
+
+	len = packet - packet_base;
+
+	/* Save packet for retransmitting */
+	last_packet_len = len;
+	memcpy(last_packet, packet_base, last_packet_len);
+
+	net_send_udp_packet(net_server_ethaddr, fastboot_remote_ip,
+			    fastboot_remote_port, fastboot_our_port, len);
+
+	/* Continue boot process after sending response */
+	if (!strncmp("OKAY", response, 4)) {
+		if (!strcmp("boot", cmd_string)) {
+			boot_downloaded_image();
+		} else if (!strcmp("continue", cmd_string)) {
+			run_command(env_get("bootcmd"), CMD_FLAG_ENV);
+		} else if (!strncmp("reboot", cmd_string, 6)) {
+			/* Matches reboot or reboot-bootloader */
+			do_reset(NULL, 0, 0, NULL);
+		}
+	}
+
+	/* OKAY and FAIL indicate command is complete */
+	if (!strncmp("OKAY", response, 4) || !strncmp("FAIL", response, 4))
+		cleanup_command_data();
+}
+
+/**
+ * Writes ascii string specified by cmd_parameter to response.
+ *
+ * @param repsonse    Pointer to fastboot response buffer
+ */
+static void fb_getvar(char *response)
+{
+	if (!cmd_parameter) {
+		write_fb_response("FAIL", "missing var", response);
+	} else if (!strcmp("version", cmd_parameter)) {
+		write_fb_response("OKAY", FASTBOOT_VERSION, response);
+	} else if (!strcmp("bootloader-version", cmd_parameter) ||
+		   !strcmp("version-bootloader", cmd_parameter)) {
+		write_fb_response("OKAY", U_BOOT_VERSION, response);
+	} else if (!strcmp("downloadsize", cmd_parameter) ||
+		   !strcmp("max-download-size", cmd_parameter)) {
+		char buf_size_str[12];
+
+		sprintf(buf_size_str, "0x%08x", CONFIG_FASTBOOT_BUF_SIZE);
+		write_fb_response("OKAY", buf_size_str, response);
+	} else if (!strcmp("serialno", cmd_parameter)) {
+		const char *tmp = env_get("serial#");
+
+		if (tmp)
+			write_fb_response("OKAY", tmp, response);
+		else
+			write_fb_response("FAIL", "Value not set", response);
+	} else if (!strcmp("version-baseband", cmd_parameter)) {
+		write_fb_response("OKAY", "N/A", response);
+	} else if (!strcmp("product", cmd_parameter)) {
+		const char *board = env_get("board");
+
+		if (board)
+			write_fb_response("OKAY", board, response);
+		else
+			write_fb_response("FAIL", "Board not set", response);
+	} else if (!strcmp("current-slot", cmd_parameter)) {
+		/* A/B not implemented, for now always return _a */
+		write_fb_response("OKAY", "_a", response);
+	} else if (!strcmp("slot-suffixes", cmd_parameter)) {
+		write_fb_response("OKAY", "_a,_b", response);
+	} else if (!strncmp("has-slot", cmd_parameter, 8)) {
+		char *part_name = cmd_parameter;
+
+		cmd_parameter = strsep(&part_name, ":");
+		if (!strcmp(part_name, "boot") || !strcmp(part_name, "system"))
+			write_fb_response("OKAY", "yes", response);
+		else
+			write_fb_response("OKAY", "no", response);
+	} else if (!strncmp("partition-type", cmd_parameter, 14) ||
+		   !strncmp("partition-size", cmd_parameter, 14)) {
+		disk_partition_t part_info;
+		struct blk_desc *dev_desc;
+		char *part_name = cmd_parameter;
+		char part_size_str[20];
+
+		cmd_parameter = strsep(&part_name, ":");
+		dev_desc = blk_get_dev("mmc", 0);
+		if (!dev_desc) {
+			write_fb_response("FAIL", "block device not found",
+					  response);
+		} else if (part_get_info_by_name(dev_desc, part_name,
+						 &part_info) < 0) {
+			write_fb_response("FAIL", "partition not found",
+					  response);
+		} else if (!strncmp("partition-type", cmd_parameter, 14)) {
+			write_fb_response("OKAY", (char *)part_info.type,
+					  response);
+		} else if (!strncmp("partition-size", cmd_parameter, 14)) {
+			sprintf(part_size_str, "0x%016x", (int)part_info.size);
+			write_fb_response("OKAY", part_size_str, response);
+		}
+	} else {
+		printf("WARNING: unknown variable: %s\n", cmd_parameter);
+		write_fb_response("FAIL", "Variable not implemented",
+				  response);
+	}
+}
+
+/**
+ * Copies image data from fastboot_data to CONFIG_FASTBOOT_BUF_ADDR.
+ * Writes to response.
+ *
+ * @param fastboot_data        Pointer to received fastboot data
+ * @param fastboot_data_len    Length of received fastboot data
+ * @param repsonse             Pointer to fastboot response buffer
+ */
+static void fb_download(char *fastboot_data, unsigned int fastboot_data_len,
+			char *response)
+{
+	char *tmp;
+
+	if (bytes_expected == 0) {
+		if (!cmd_parameter) {
+			write_fb_response("FAIL", "Expected command parameter",
+					  response);
+			return;
+		}
+		bytes_expected = simple_strtoul(cmd_parameter, &tmp, 16);
+		if (bytes_expected == 0) {
+			write_fb_response("FAIL", "Expected nonzero image size",
+					  response);
+			return;
+		}
+	}
+	if (fastboot_data_len == 0 && bytes_received == 0) {
+		/* Nothing to download yet. Response is of the form:
+		 * [DATA|FAIL]$cmd_parameter
+		 *
+		 * where cmd_parameter is an 8 digit hexadecimal number
+		 */
+		if (bytes_expected > CONFIG_FASTBOOT_BUF_SIZE)
+			write_fb_response("FAIL", cmd_parameter, response);
+		else
+			write_fb_response("DATA", cmd_parameter, response);
+	} else if (fastboot_data_len == 0 &&
+		   (bytes_received >= bytes_expected)) {
+		/* Download complete. Respond with "OKAY" */
+		write_fb_response("OKAY", "", response);
+		image_size = bytes_received;
+		bytes_expected = 0;
+		bytes_received = 0;
+	} else {
+		if (fastboot_data_len == 0 ||
+		    (bytes_received + fastboot_data_len) > bytes_expected) {
+			write_fb_response("FAIL",
+					  "Received invalid data length",
+					  response);
+			return;
+		}
+		/* Download data to CONFIG_FASTBOOT_BUF_ADDR */
+		memcpy((void *)CONFIG_FASTBOOT_BUF_ADDR + bytes_received,
+		       fastboot_data, fastboot_data_len);
+		bytes_received += fastboot_data_len;
+	}
+}
+
+/**
+ * Writes the previously downloaded image to the partition indicated by
+ * cmd_parameter. Writes to response.
+ *
+ * @param repsonse    Pointer to fastboot response buffer
+ */
+static void fb_flash(char *response)
+{
+	fb_mmc_flash_write(cmd_parameter, (void *)CONFIG_FASTBOOT_BUF_ADDR,
+			   image_size, response);
+}
+
+/**
+ * Erases the partition indicated by cmd_parameter (clear to 0x00s). Writes
+ * to response.
+ *
+ * @param repsonse    Pointer to fastboot response buffer
+ */
+static void fb_erase(char *response)
+{
+	fb_mmc_erase(cmd_parameter, response);
+}
+
+/**
+ * Continues normal boot process by running "bootcmd". Writes
+ * to response.
+ *
+ * @param repsonse    Pointer to fastboot response buffer
+ */
+static void fb_continue(char *response)
+{
+	char *bootcmd;
+
+	bootcmd = env_get("bootcmd");
+	if (bootcmd)
+		write_fb_response("OKAY", "", response);
+	else
+		write_fb_response("FAIL", "bootcmd not set", response);
+}
+
+/**
+ * Sets reboot bootloader flag if requested. Writes to response.
+ *
+ * @param repsonse    Pointer to fastboot response buffer
+ */
+static void fb_reboot(char *response)
+{
+	write_fb_response("OKAY", "", response);
+	if (!strcmp("reboot-bootloader", cmd_string))
+		strcpy((char *)CONFIG_FASTBOOT_BUF_ADDR, "reboot-bootloader");
+}
+
+/**
+ * Boots into downloaded image.
+ */
+static void boot_downloaded_image(void)
+{
+	char kernel_addr[12];
+	char *fdt_addr = env_get("fdt_addr_r");
+	char *const bootm_args[] = {
+		"bootm", kernel_addr, "-", fdt_addr, NULL
+	};
+
+	sprintf(kernel_addr, "0x%lx", (long)CONFIG_FASTBOOT_BUF_ADDR);
+
+	printf("\nBooting kernel at %s with fdt at %s...\n\n\n",
+	       kernel_addr, fdt_addr);
+	do_bootm(NULL, 0, 4, bootm_args);
+
+	/* This only happens if image is faulty so we start over. */
+	do_reset(NULL, 0, 0, NULL);
+}
+
+/**
+ * Writes a response to response buffer of the form "$tag$reason".
+ *
+ * @param tag         The first part of the response
+ * @param reason      The second part of the response
+ * @param repsonse    Pointer to fastboot response buffer
+ */
+static void write_fb_response(const char *tag, const char *reason,
+			      char *response)
+{
+	strncpy(response, tag, strlen(tag));
+	strncat(response, reason, FASTBOOT_RESPONSE_LEN - strlen(tag) - 1);
+}
+
+/**
+ * Frees any resources allocated during current fastboot command.
+ */
+static void cleanup_command_data(void)
+{
+	/* cmd_parameter and cmd_string potentially point to memory allocated by
+	 * strdup
+	 */
+	if (cmd_parameter)
+		free(cmd_parameter);
+	if (cmd_string)
+		free(cmd_string);
+	cmd_parameter = NULL;
+	cmd_string = NULL;
+}
+
+/**
+ * Incoming UDP packet handler.
+ *
+ * @param packet  Pointer to incoming UDP packet
+ * @param dport   Destination UDP port
+ * @param sip     Source IP address
+ * @param sport   Source UDP port
+ * @param len     Packet length
+ */
+static void fastboot_handler(uchar *packet, unsigned int dport,
+			     struct in_addr sip, unsigned int sport,
+			     unsigned int len)
+{
+	struct fastboot_header fb_header;
+	char fastboot_data[DATA_SIZE] = {0};
+	unsigned int fastboot_data_len = 0;
+
+	if (dport != fastboot_our_port)
+		return;
+
+	fastboot_remote_ip = sip;
+	fastboot_remote_port = sport;
+
+	if (len < FASTBOOT_HEADER_SIZE || len > PACKET_SIZE)
+		return;
+	memcpy(&fb_header, packet, sizeof(fb_header));
+	fb_header.flags = 0;
+	fb_header.seq = ntohs(fb_header.seq);
+	packet += sizeof(fb_header);
+	len -= sizeof(fb_header);
+
+	switch (fb_header.id) {
+	case FASTBOOT_QUERY:
+		fastboot_send(fb_header, fastboot_data, 0, 0);
+		break;
+	case FASTBOOT_INIT:
+	case FASTBOOT_FASTBOOT:
+		fastboot_data_len = len;
+		if (len > 0)
+			memcpy(fastboot_data, packet, len);
+		if (fb_header.seq == fb_sequence_number) {
+			fastboot_send(fb_header, fastboot_data,
+				      fastboot_data_len, 0);
+			fb_sequence_number++;
+		} else if (fb_header.seq == fb_sequence_number - 1) {
+			/* Retransmit last sent packet */
+			fastboot_send(fb_header, fastboot_data,
+				      fastboot_data_len, 1);
+		}
+		break;
+	default:
+		pr_err("ID %d not implemented.\n", fb_header.id);
+		fb_header.id = FASTBOOT_ERROR;
+		fastboot_send(fb_header, fastboot_data, 0, 0);
+		break;
+	}
+}
+
+void fastboot_start_server(void)
+{
+	printf("Using %s device\n", eth_get_name());
+	printf("Listening for fastboot command on %pI4\n", &net_ip);
+
+	fastboot_our_port = WELL_KNOWN_PORT;
+
+	net_set_udp_handler(fastboot_handler);
+
+	/* zero out server ether in case the server ip has changed */
+	memset(net_server_ethaddr, 0, 6);
+}
diff --git a/net/net.c b/net/net.c
index 8a9b69c..42ecb1a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -87,6 +87,9 @@
 #include <environment.h>
 #include <errno.h>
 #include <net.h>
+#if defined(CONFIG_UDP_FUNCTION_FASTBOOT)
+#include <net/fastboot.h>
+#endif
 #include <net/tftp.h>
 #if defined(CONFIG_LED_STATUS)
 #include <miiphy.h>
@@ -453,6 +456,11 @@ restart:
 			tftp_start_server();
 			break;
 #endif
+#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
+		case FASTBOOT:
+			fastboot_start_server();
+			break;
+#endif
 #if defined(CONFIG_CMD_DHCP)
 		case DHCP:
 			bootp_reset();
@@ -1324,6 +1332,7 @@ common:
 		/* Fall through */
 
 	case NETCONS:
+	case FASTBOOT:
 	case TFTPSRV:
 		if (net_ip.s_addr == 0) {
 			puts("*** ERROR: `ipaddr' not set\n");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 4/5] dfu: Resolve Kconfig dependency loops
  2018-04-24  9:37 [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Kiernan
                   ` (2 preceding siblings ...)
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 3/5] net: dfu: Merge AOSP UDP fastboot Alex Kiernan
@ 2018-04-24  9:37 ` Alex Kiernan
  2018-04-25  7:53   ` Lukasz Majewski
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 5/5] net: dfu: Support building without MMC Alex Kiernan
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: Alex Kiernan @ 2018-04-24  9:37 UTC (permalink / raw)
  To: u-boot

Fix recursive dependencies in Kconfig introduced by fastboot UDP

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

 cmd/fastboot/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cmd/fastboot/Kconfig b/cmd/fastboot/Kconfig
index 0c57616..0f804ea 100644
--- a/cmd/fastboot/Kconfig
+++ b/cmd/fastboot/Kconfig
@@ -2,13 +2,13 @@ comment "FASTBOOT"
 
 menuconfig FASTBOOT
 	bool "Fastboot support"
-	depends on USB_GADGET
 	default y if ARCH_SUNXI && USB_MUSB_GADGET
 
 if FASTBOOT
 
 config USB_FUNCTION_FASTBOOT
 	bool "Enable USB fastboot gadget"
+	depends on USB_GADGET
 	default y
 	select USB_GADGET_DOWNLOAD
 	imply ANDROID_BOOT_IMAGE
@@ -17,7 +17,7 @@ config USB_FUNCTION_FASTBOOT
 	  This enables the USB part of the fastboot gadget.
 
 config UDP_FUNCTION_FASTBOOT
-	select NET
+	depends on NET
 	bool "Enable fastboot protocol over UDP"
 	help
 	  This enables the fastboot protocol over UDP.
@@ -66,6 +66,7 @@ config FASTBOOT_BUF_SIZE
 
 config FASTBOOT_USB_DEV
 	int "USB controller number"
+	depends on USB_FUNCTION_FASTBOOT
 	default 0
 	help
 	  Some boards have USB OTG controller other than 0. Define this
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 5/5] net: dfu: Support building without MMC
  2018-04-24  9:37 [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Kiernan
                   ` (3 preceding siblings ...)
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 4/5] dfu: Resolve Kconfig dependency loops Alex Kiernan
@ 2018-04-24  9:37 ` Alex Kiernan
  2018-04-24 10:24 ` [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Deymo
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 31+ messages in thread
From: Alex Kiernan @ 2018-04-24  9:37 UTC (permalink / raw)
  To: u-boot

If the fastboot flash/erase commands are disabled, remove that support
so we still build correctly.

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

 net/fastboot.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/fastboot.c b/net/fastboot.c
index 32cb581..87c12e5 100644
--- a/net/fastboot.c
+++ b/net/fastboot.c
@@ -59,8 +59,10 @@ static int fastboot_our_port;
 
 static void fb_getvar(char *);
 static void fb_download(char *, unsigned int, char *);
+#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
 static void fb_flash(char *);
 static void fb_erase(char *);
+#endif
 static void fb_continue(char *);
 static void fb_reboot(char *);
 static void boot_downloaded_image(void);
@@ -169,10 +171,12 @@ static void fastboot_send(struct fastboot_header fb_header, char *fastboot_data,
 			fb_getvar(response);
 		} else if (!strcmp("download", cmd_string)) {
 			fb_download(fastboot_data, fastboot_data_len, response);
+#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
 		} else if (!strcmp("flash", cmd_string)) {
 			fb_flash(response);
 		} else if (!strcmp("erase", cmd_string)) {
 			fb_erase(response);
+#endif
 		} else if (!strcmp("boot", cmd_string)) {
 			write_fb_response("OKAY", "", response);
 		} else if (!strcmp("continue", cmd_string)) {
@@ -367,6 +371,7 @@ static void fb_download(char *fastboot_data, unsigned int fastboot_data_len,
 	}
 }
 
+#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
 /**
  * Writes the previously downloaded image to the partition indicated by
  * cmd_parameter. Writes to response.
@@ -389,6 +394,7 @@ static void fb_erase(char *response)
 {
 	fb_mmc_erase(cmd_parameter, response);
 }
+#endif
 
 /**
  * Continues normal boot process by running "bootcmd". Writes
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-24  9:37 [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Kiernan
                   ` (4 preceding siblings ...)
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 5/5] net: dfu: Support building without MMC Alex Kiernan
@ 2018-04-24 10:24 ` Alex Deymo
  2018-04-24 11:58   ` Alex Kiernan
  2018-04-25  7:52 ` Lukasz Majewski
  2018-04-27 19:10 ` Sam Protsenko
  7 siblings, 1 reply; 31+ messages in thread
From: Alex Deymo @ 2018-04-24 10:24 UTC (permalink / raw)
  To: u-boot

+Jocelyn.

Thanks Alex for taking the time to port this!! Some comments inline here
about your questions.

2018-04-24 11:37 GMT+02:00 Alex Kiernan <alex.kiernan@gmail.com>:

>
> This series merges the fastboot UDP support from AOSP into mainline
> U-Boot.
>
> Open questions:
>
> - what's the correct way of attributing the original authors? I've added
>   Co-authored-by, is that right? checkpatch doesn't seem to know any
>   of the co- tags
> - currently there's no NAND support and I've no way of testing that, so
>   my inclination is towards leaving it like that until someone with that
>   particular itch to scratch can look at it
>

Fastboot uses partition names, like "system" and "boot" which have a
meaning in the Android partition scheme. For GPT, we just use the partition
names as the android names; but if you are booting from some other storage
like NAND where you don't have that then you need some mapping glue
("system" --> device and partition number). I've seen U-Boot modifications
for several devices where they just stick a table hard-coded in the U-Boot
code; which works great for that device but it isn't really a generic
approach. Other than handling the names issue, I don't see any problem with
supporting NAND here, but a generic way to set global names / alias would
be needed for this.



> - you can select both USB and UDP fastboot, but the comments in the
>   AOSP code suggest that needs fixing - again, I've no board I can test
>   USB fastboot on
>

I thought we checked in the Kconfig that you couldn't enable both. I don't
remember the details now but yeah you can't wait for network or USB traffic
on the current code.

- the USB and UDP code want consolidating, with this series there would
>   then be two separate implementations of the same protocol
> - the syntax for the USB fastboot command changes from `fastboot
> <controller>`
>   to `fastboot usb <controller>`, that feels unacceptable and we probably
>   want something like `fastboot <controller>` or `fastboot udp`?
> - unrelated to this series, but a show-stopper for me, there's no FIT image
>   support, but that doesn't feel unsolveable - something like add an option
>   to pass in the load address and/or use loadaddr, then something else to
>   let you override the bootm invocation on the server side
>
> I've not tested all the code paths yet, but the obvious stuff works
> (basic interaction, download, boot) - every interaction elicits a
> `WARNING: unknown variable: slot-count` on the console; I'm guessing that
> my local end is sending a getvar for that, but I've not investigated.
>

Yes, there's a bit of different handling of partitions for A/B android
devices. Instead of "system" you have two partitions: "system_a" and
"system_b" (potentially more although 2 is the most common case) so to know
whether you have an old device or a newer device the fastboot client checks
the "slot-count" variable. Undefined means of course that you have an
old-style device, but if you return something like "2" you would be able to
flash "system" on the "slot A" which is translated (again in the fastboot
client) to flash "system_a" partition. This is useful when using the higher
level operations like flashall/update and you want to specify to flash only
"slot A", "slot B" or even both of them. There are other fastboot variables
that require some plumbing, such as "has-slot:<partition>" to tell whether
"system" is a partition that indeed has the two versions _a and _b.
Typically some partitions are twice (like "system" and "boot") and some
other are not (like "misc" or "userdata"). Anyway, this as is should work
for either old-style partition schemes or by force flashing "system_a" with
the system.img.



> Without any way of testing any of the USB/NAND code I'm nervous of wading
> into that kind of refactoring, would that be a pre-requisite for merging?
>
>
> Alex Kiernan (5):
>   dfu: Refactor fastboot_okay/fail to take response
>   dfu: Extract fastboot_okay/fail to fb_common.c
>   net: dfu: Merge AOSP UDP fastboot
>   dfu: Resolve Kconfig dependency loops
>   net: dfu: Support building without MMC
>
>  cmd/fastboot.c                  |  32 ++-
>  cmd/fastboot/Kconfig            |  21 +-
>  cmd/net.c                       |   6 +
>  common/Makefile                 |   4 +
>  common/fb_common.c              |  44 ++++
>  common/fb_mmc.c                 | 114 ++++++---
>  common/fb_nand.c                |  31 +--
>  common/image-sparse.c           |  41 ++-
>  drivers/usb/gadget/f_fastboot.c |  36 +--
>  include/fastboot.h              |  17 +-
>  include/fb_mmc.h                |   4 +-
>  include/fb_nand.h               |   4 +-
>  include/image-sparse.h          |   2 +-
>  include/net.h                   |   6 +-
>  include/net/fastboot.h          |  27 ++
>  net/Makefile                    |   1 +
>  net/fastboot.c                  | 548 ++++++++++++++++++++++++++++++
> ++++++++++
>  net/net.c                       |   9 +
>  18 files changed, 824 insertions(+), 123 deletions(-)
>  create mode 100644 common/fb_common.c
>  create mode 100644 include/net/fastboot.h
>  create mode 100644 net/fastboot.c
>
> --
> 2.7.4
>
>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-24 10:24 ` [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Deymo
@ 2018-04-24 11:58   ` Alex Kiernan
  2018-04-24 17:10     ` Jocelyn Bohr
  2018-04-25  7:40     ` Lukasz Majewski
  0 siblings, 2 replies; 31+ messages in thread
From: Alex Kiernan @ 2018-04-24 11:58 UTC (permalink / raw)
  To: u-boot

On Tue, Apr 24, 2018 at 11:24 AM, Alex Deymo <deymo+@google.com> wrote:
> +Jocelyn.
>
> Thanks Alex for taking the time to port this!!

It turned out to be a great opportunity to play with coccinelle on
something trivial, which I'd been meaning to do for ages... the
refactor to add response into the fastboot_okay/fail call chain was a
breeze with it.

> 2018-04-24 11:37 GMT+02:00 Alex Kiernan <alex.kiernan@gmail.com>:
>>
>>
>> This series merges the fastboot UDP support from AOSP into mainline
>> U-Boot.
>>
>> Open questions:
>>
>> - what's the correct way of attributing the original authors? I've added
>>   Co-authored-by, is that right? checkpatch doesn't seem to know any
>>   of the co- tags
>> - currently there's no NAND support and I've no way of testing that, so
>>   my inclination is towards leaving it like that until someone with that
>>   particular itch to scratch can look at it
>
>
> Fastboot uses partition names, like "system" and "boot" which have a meaning
> in the Android partition scheme. For GPT, we just use the partition names as
> the android names; but if you are booting from some other storage like NAND
> where you don't have that then you need some mapping glue ("system" -->
> device and partition number). I've seen U-Boot modifications for several
> devices where they just stick a table hard-coded in the U-Boot code; which
> works great for that device but it isn't really a generic approach. Other
> than handling the names issue, I don't see any problem with supporting NAND
> here, but a generic way to set global names / alias would be needed for
> this.
>

With the fastboot NAND support in mainline it looks like we end up in
mtdparts to deliver that mapping through environment variables. No
actual idea what that looks like when you're driving it.

>>
>> - you can select both USB and UDP fastboot, but the comments in the
>>   AOSP code suggest that needs fixing - again, I've no board I can test
>>   USB fastboot on
>
>
> I thought we checked in the Kconfig that you couldn't enable both. I don't
> remember the details now but yeah you can't wait for network or USB traffic
> on the current code.
>

The version I picked from (o-mr1-iot-preview-8) has them as
independent symbols, but making them a choice would resolve the issue
for now.

>> I've not tested all the code paths yet, but the obvious stuff works
>> (basic interaction, download, boot) - every interaction elicits a
>> `WARNING: unknown variable: slot-count` on the console; I'm guessing that
>> my local end is sending a getvar for that, but I've not investigated.
>
>
> Yes, there's a bit of different handling of partitions for A/B android
> devices. Instead of "system" you have two partitions: "system_a" and
> "system_b" (potentially more although 2 is the most common case) so to know
> whether you have an old device or a newer device the fastboot client checks
> the "slot-count" variable. Undefined means of course that you have an
> old-style device, but if you return something like "2" you would be able to
> flash "system" on the "slot A" which is translated (again in the fastboot
> client) to flash "system_a" partition. This is useful when using the higher
> level operations like flashall/update and you want to specify to flash only
> "slot A", "slot B" or even both of them. There are other fastboot variables
> that require some plumbing, such as "has-slot:<partition>" to tell whether
> "system" is a partition that indeed has the two versions _a and _b.
> Typically some partitions are twice (like "system" and "boot") and some
> other are not (like "misc" or "userdata"). Anyway, this as is should work
> for either old-style partition schemes or by force flashing "system_a" with
> the system.img.
>

Cool, thanks... that saves me a bunch of investigation!

-- 
Alex Kiernan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-24 11:58   ` Alex Kiernan
@ 2018-04-24 17:10     ` Jocelyn Bohr
  2018-04-25  7:53       ` Alex Kiernan
  2018-04-26  3:27       ` Kever Yang
  2018-04-25  7:40     ` Lukasz Majewski
  1 sibling, 2 replies; 31+ messages in thread
From: Jocelyn Bohr @ 2018-04-24 17:10 UTC (permalink / raw)
  To: u-boot

Thanks so much for porting this, Alex!

On Tue, Apr 24, 2018 at 4:58 AM Alex Kiernan <alex.kiernan@gmail.com> wrote:

> On Tue, Apr 24, 2018 at 11:24 AM, Alex Deymo <deymo+@google.com> wrote:
> > +Jocelyn.
> >
> > Thanks Alex for taking the time to port this!!
>
> It turned out to be a great opportunity to play with coccinelle on
> something trivial, which I'd been meaning to do for ages... the
> refactor to add response into the fastboot_okay/fail call chain was a
> breeze with it.
>
> > 2018-04-24 11:37 GMT+02:00 Alex Kiernan <alex.kiernan@gmail.com>:
> >>
> >>
> >> This series merges the fastboot UDP support from AOSP into mainline
> >> U-Boot.
> >>
> >> Open questions:
> >>
> >> - what's the correct way of attributing the original authors? I've added
> >>   Co-authored-by, is that right? checkpatch doesn't seem to know any
> >>   of the co- tags
> >> - currently there's no NAND support and I've no way of testing that, so
> >>   my inclination is towards leaving it like that until someone with that
> >>   particular itch to scratch can look at it
> >
> >
> > Fastboot uses partition names, like "system" and "boot" which have a
> meaning
> > in the Android partition scheme. For GPT, we just use the partition
> names as
> > the android names; but if you are booting from some other storage like
> NAND
> > where you don't have that then you need some mapping glue ("system" -->
> > device and partition number). I've seen U-Boot modifications for several
> > devices where they just stick a table hard-coded in the U-Boot code;
> which
> > works great for that device but it isn't really a generic approach. Other
> > than handling the names issue, I don't see any problem with supporting
> NAND
> > here, but a generic way to set global names / alias would be needed for
> > this.
> >
>
> With the fastboot NAND support in mainline it looks like we end up in
> mtdparts to deliver that mapping through environment variables. No
> actual idea what that looks like when you're driving it.
>
> >>
> >> - you can select both USB and UDP fastboot, but the comments in the
> >>   AOSP code suggest that needs fixing - again, I've no board I can test
> >>   USB fastboot on
> >
> >
> > I thought we checked in the Kconfig that you couldn't enable both. I
> don't
> > remember the details now but yeah you can't wait for network or USB
> traffic
> > on the current code.
> >
>
> The version I picked from (o-mr1-iot-preview-8) has them as
> independent symbols, but making them a choice would resolve the issue
> for now.
>

You can select both network and USB fastboot to be enabled with the Kconfig,
but at runtime you have to select to wait on either USB or network by
running
"fastboot udp" or "fastboot usb <USB_controller>". When the Android
bootloader
gets the command to reboot back to fastboot, it will read the "fastbootcmd"
env
variable and run that as a command (common/android_bootloader.c:151).


>
> >> I've not tested all the code paths yet, but the obvious stuff works
> >> (basic interaction, download, boot) - every interaction elicits a
> >> `WARNING: unknown variable: slot-count` on the console; I'm guessing
> that
> >> my local end is sending a getvar for that, but I've not investigated.
> >
> >
> > Yes, there's a bit of different handling of partitions for A/B android
> > devices. Instead of "system" you have two partitions: "system_a" and
> > "system_b" (potentially more although 2 is the most common case) so to
> know
> > whether you have an old device or a newer device the fastboot client
> checks
> > the "slot-count" variable. Undefined means of course that you have an
> > old-style device, but if you return something like "2" you would be able
> to
> > flash "system" on the "slot A" which is translated (again in the fastboot
> > client) to flash "system_a" partition. This is useful when using the
> higher
> > level operations like flashall/update and you want to specify to flash
> only
> > "slot A", "slot B" or even both of them. There are other fastboot
> variables
> > that require some plumbing, such as "has-slot:<partition>" to tell
> whether
> > "system" is a partition that indeed has the two versions _a and _b.
> > Typically some partitions are twice (like "system" and "boot") and some
> > other are not (like "misc" or "userdata"). Anyway, this as is should work
> > for either old-style partition schemes or by force flashing "system_a"
> with
> > the system.img.
> >
>
> Cool, thanks... that saves me a bunch of investigation!
>
> --
> Alex Kiernan
>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 1/5] dfu: Refactor fastboot_okay/fail to take response
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 1/5] dfu: Refactor fastboot_okay/fail to take response Alex Kiernan
@ 2018-04-25  7:27   ` Lukasz Majewski
  2018-04-25  8:10     ` Alex Kiernan
  0 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2018-04-25  7:27 UTC (permalink / raw)
  To: u-boot

Hi Alex,

Please change the subject prefix - i.e. dfu: is a totally different
code.

Maybe fastboot would be more appropriate?

> Add the response string as a parameter to fastboot_okay/fail, instead
> of modifying a global, to match the contract expected by the AOSP
> U-Boot code.

I suppose that this change is to make the u-boot mainline code matching
the current Android?

> 
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> ---
> 
>  common/fb_mmc.c                 | 80
> ++++++++++++++++++++++-------------------
> common/fb_nand.c                | 31 ++++++++--------
> common/image-sparse.c           | 41 +++++++++++----------
> drivers/usb/gadget/f_fastboot.c | 35 +++++++-----------
> include/fastboot.h              |  4 +--
> include/fb_mmc.h                |  4 +--
> include/fb_nand.h               |  4 +--
> include/image-sparse.h          |  2 +- 8 files changed, 100
> insertions(+), 101 deletions(-)
> 
> diff --git a/common/fb_mmc.c b/common/fb_mmc.c
> index cf5b77c..02864aa 100644
> --- a/common/fb_mmc.c
> +++ b/common/fb_mmc.c
> @@ -74,7 +74,7 @@ static lbaint_t fb_mmc_sparse_reserve(struct
> sparse_storage *info, 
>  static void write_raw_image(struct blk_desc *dev_desc,
> disk_partition_t *info, const char *part_name, void *buffer,
> -		unsigned int download_bytes)
> +		unsigned int download_bytes, char *response)
>  {
>  	lbaint_t blkcnt;
>  	lbaint_t blks;
> @@ -85,7 +85,7 @@ static void write_raw_image(struct blk_desc
> *dev_desc, disk_partition_t *info, 
>  	if (blkcnt > info->size) {
>  		pr_err("too large for partition: '%s'\n", part_name);
> -		fastboot_fail("too large for partition");
> +		fastboot_fail("too large for partition", response);
>  		return;
>  	}
>  
> @@ -94,13 +94,13 @@ static void write_raw_image(struct blk_desc
> *dev_desc, disk_partition_t *info, blks = blk_dwrite(dev_desc,
> info->start, blkcnt, buffer); if (blks != blkcnt) {
>  		pr_err("failed writing to device %d\n",
> dev_desc->devnum);
> -		fastboot_fail("failed writing to device");
> +		fastboot_fail("failed writing to device", response);
>  		return;
>  	}
>  
>  	printf("........ wrote " LBAFU " bytes to '%s'\n", blkcnt *
> info->blksz, part_name);
> -	fastboot_okay("");
> +	fastboot_okay("", response);
>  }
>  
>  #ifdef CONFIG_ANDROID_BOOT_IMAGE
> @@ -115,7 +115,8 @@ static void write_raw_image(struct blk_desc
> *dev_desc, disk_partition_t *info, */
>  static lbaint_t fb_mmc_get_boot_header(struct blk_desc *dev_desc,
>  				       disk_partition_t *info,
> -				       struct andr_img_hdr *hdr)
> +				       struct andr_img_hdr *hdr,
> +				       char *response)
>  {
>  	ulong sector_size;		/* boot partition sector
> size */ lbaint_t hdr_sectors;		/* boot image header
> sectors count */ @@ -126,7 +127,7 @@ static lbaint_t
> fb_mmc_get_boot_header(struct blk_desc *dev_desc, hdr_sectors =
> DIV_ROUND_UP(sizeof(struct andr_img_hdr), sector_size); if
> (hdr_sectors == 0) { pr_err("invalid number of boot sectors: 0");
> -		fastboot_fail("invalid number of boot sectors: 0");
> +		fastboot_fail("invalid number of boot sectors: 0",
> response); return 0;
>  	}
>  
> @@ -134,7 +135,8 @@ static lbaint_t fb_mmc_get_boot_header(struct
> blk_desc *dev_desc, res = blk_dread(dev_desc, info->start,
> hdr_sectors, (void *)hdr); if (res != hdr_sectors) {
>  		pr_err("cannot read header from boot partition");
> -		fastboot_fail("cannot read header from boot
> partition");
> +		fastboot_fail("cannot read header from boot
> partition",
> +			      response);
>  		return 0;
>  	}
>  
> @@ -142,7 +144,7 @@ static lbaint_t fb_mmc_get_boot_header(struct
> blk_desc *dev_desc, res = android_image_check_header(hdr);
>  	if (res != 0) {
>  		pr_err("bad boot image magic");
> -		fastboot_fail("boot partition not initialized");
> +		fastboot_fail("boot partition not initialized",
> response); return 0;
>  	}
>  
> @@ -160,7 +162,8 @@ static lbaint_t fb_mmc_get_boot_header(struct
> blk_desc *dev_desc, */
>  static int fb_mmc_update_zimage(struct blk_desc *dev_desc,
>  				void *download_buffer,
> -				unsigned int download_bytes)
> +				unsigned int download_bytes,
> +				char *response)
>  {
>  	uintptr_t hdr_addr;			/* boot image
> header address */ struct andr_img_hdr *hdr;		/* boot
> image header */ @@ -180,7 +183,7 @@ static int
> fb_mmc_update_zimage(struct blk_desc *dev_desc, res =
> part_get_info_by_name(dev_desc, BOOT_PARTITION_NAME, &info); if (res
> < 0) { pr_err("cannot find boot partition");
> -		fastboot_fail("cannot find boot partition");
> +		fastboot_fail("cannot find boot partition",
> response); return -1;
>  	}
>  
> @@ -189,17 +192,18 @@ static int fb_mmc_update_zimage(struct blk_desc
> *dev_desc, hdr = (struct andr_img_hdr *)hdr_addr;
>  
>  	/* Read boot image header */
> -	hdr_sectors = fb_mmc_get_boot_header(dev_desc, &info, hdr);
> +	hdr_sectors = fb_mmc_get_boot_header(dev_desc, &info, hdr,
> response); if (hdr_sectors == 0) {
>  		pr_err("unable to read boot image header");
> -		fastboot_fail("unable to read boot image header");
> +		fastboot_fail("unable to read boot image header",
> response); return -1;
>  	}
>  
>  	/* Check if boot image has second stage in it (we don't
> support it) */ if (hdr->second_size > 0) {
>  		pr_err("moving second stage is not supported yet");
> -		fastboot_fail("moving second stage is not supported
> yet");
> +		fastboot_fail("moving second stage is not supported
> yet",
> +			      response);
>  		return -1;
>  	}
>  
> @@ -217,7 +221,8 @@ static int fb_mmc_update_zimage(struct blk_desc
> *dev_desc, ramdisk_buffer);
>  	if (res != ramdisk_sectors) {
>  		pr_err("cannot read ramdisk from boot partition");
> -		fastboot_fail("cannot read ramdisk from boot
> partition");
> +		fastboot_fail("cannot read ramdisk from boot
> partition",
> +			      response);
>  		return -1;
>  	}
>  
> @@ -226,7 +231,7 @@ static int fb_mmc_update_zimage(struct blk_desc
> *dev_desc, res = blk_dwrite(dev_desc, info.start, hdr_sectors, (void
> *)hdr); if (res == 0) {
>  		pr_err("cannot writeback boot image header");
> -		fastboot_fail("cannot write back boot image header");
> +		fastboot_fail("cannot write back boot image header",
> response); return -1;
>  	}
>  
> @@ -238,7 +243,7 @@ static int fb_mmc_update_zimage(struct blk_desc
> *dev_desc, download_buffer);
>  	if (res == 0) {
>  		pr_err("cannot write new kernel");
> -		fastboot_fail("cannot write new kernel");
> +		fastboot_fail("cannot write new kernel", response);
>  		return -1;
>  	}
>  
> @@ -250,18 +255,18 @@ static int fb_mmc_update_zimage(struct blk_desc
> *dev_desc, ramdisk_buffer);
>  	if (res == 0) {
>  		pr_err("cannot write back original ramdisk");
> -		fastboot_fail("cannot write back original ramdisk");
> +		fastboot_fail("cannot write back original ramdisk",
> response); return -1;
>  	}
>  
>  	puts("........ zImage was updated in boot partition\n");
> -	fastboot_okay("");
> +	fastboot_okay("", response);
>  	return 0;
>  }
>  #endif
>  
>  void fb_mmc_flash_write(const char *cmd, void *download_buffer,
> -			unsigned int download_bytes)
> +			unsigned int download_bytes, char *response)
>  {
>  	struct blk_desc *dev_desc;
>  	disk_partition_t info;
> @@ -269,7 +274,7 @@ void fb_mmc_flash_write(const char *cmd, void
> *download_buffer, dev_desc = blk_get_dev("mmc",
> CONFIG_FASTBOOT_FLASH_MMC_DEV); if (!dev_desc || dev_desc->type ==
> DEV_TYPE_UNKNOWN) { pr_err("invalid mmc device\n");
> -		fastboot_fail("invalid mmc device");
> +		fastboot_fail("invalid mmc device", response);
>  		return;
>  	}
>  
> @@ -280,16 +285,17 @@ void fb_mmc_flash_write(const char *cmd, void
> *download_buffer, if (is_valid_gpt_buf(dev_desc, download_buffer)) {
>  			printf("%s: invalid GPT - refusing to write
> to flash\n", __func__);
> -			fastboot_fail("invalid GPT partition");
> +			fastboot_fail("invalid GPT partition",
> response); return;
>  		}
>  		if (write_mbr_and_gpt_partitions(dev_desc,
> download_buffer)) { printf("%s: writing GPT partitions failed\n",
> __func__);
> -			fastboot_fail("writing GPT partitions
> failed");
> +			fastboot_fail("writing GPT partitions
> failed",
> +				      response);
>  			return;
>  		}
>  		printf("........ success\n");
> -		fastboot_okay("");
> +		fastboot_okay("", response);
>  		return;
>  	}
>  #endif
> @@ -300,30 +306,32 @@ void fb_mmc_flash_write(const char *cmd, void
> *download_buffer, if (is_valid_dos_buf(download_buffer)) {
>  			printf("%s: invalid MBR - refusing to write
> to flash\n", __func__);
> -			fastboot_fail("invalid MBR partition");
> +			fastboot_fail("invalid MBR partition",
> response); return;
>  		}
>  		if (write_mbr_partition(dev_desc, download_buffer)) {
>  			printf("%s: writing MBR partition failed\n",
> __func__);
> -			fastboot_fail("writing MBR partition
> failed");
> +			fastboot_fail("writing MBR partition failed",
> +				      response);
>  			return;
>  		}
>  		printf("........ success\n");
> -		fastboot_okay("");
> +		fastboot_okay("", response);
>  		return;
>  	}
>  #endif
>  
>  #ifdef CONFIG_ANDROID_BOOT_IMAGE
>  	if (strncasecmp(cmd, "zimage", 6) == 0) {
> -		fb_mmc_update_zimage(dev_desc, download_buffer,
> download_bytes);
> +		fb_mmc_update_zimage(dev_desc, download_buffer,
> +				     download_bytes, response);
>  		return;
>  	}
>  #endif
>  
>  	if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) <
> 0) { pr_err("cannot find partition: '%s'\n", cmd);
> -		fastboot_fail("cannot find partition");
> +		fastboot_fail("cannot find partition", response);
>  		return;
>  	}
>  
> @@ -344,14 +352,14 @@ void fb_mmc_flash_write(const char *cmd, void
> *download_buffer, 
>  		sparse.priv = &sparse_priv;
>  		write_sparse_image(&sparse, cmd, download_buffer,
> -				   download_bytes);
> +				   download_bytes, response);
>  	} else {
>  		write_raw_image(dev_desc, &info, cmd,
> download_buffer,
> -				download_bytes);
> +				download_bytes, response);
>  	}
>  }
>  
> -void fb_mmc_erase(const char *cmd)
> +void fb_mmc_erase(const char *cmd, char *response)
>  {
>  	int ret;
>  	struct blk_desc *dev_desc;
> @@ -361,21 +369,21 @@ void fb_mmc_erase(const char *cmd)
>  
>  	if (mmc == NULL) {
>  		pr_err("invalid mmc device");
> -		fastboot_fail("invalid mmc device");
> +		fastboot_fail("invalid mmc device", response);
>  		return;
>  	}
>  
>  	dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
>  	if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
>  		pr_err("invalid mmc device");
> -		fastboot_fail("invalid mmc device");
> +		fastboot_fail("invalid mmc device", response);
>  		return;
>  	}
>  
>  	ret = part_get_info_by_name_or_alias(dev_desc, cmd, &info);
>  	if (ret < 0) {
>  		pr_err("cannot find partition: '%s'", cmd);
> -		fastboot_fail("cannot find partition");
> +		fastboot_fail("cannot find partition", response);
>  		return;
>  	}
>  
> @@ -394,11 +402,11 @@ void fb_mmc_erase(const char *cmd)
>  	blks = blk_derase(dev_desc, blks_start, blks_size);
>  	if (blks != blks_size) {
>  		pr_err("failed erasing from device %d",
> dev_desc->devnum);
> -		fastboot_fail("failed erasing from device");
> +		fastboot_fail("failed erasing from device",
> response); return;
>  	}
>  
>  	printf("........ erased " LBAFU " bytes from '%s'\n",
>  	       blks_size * info.blksz, cmd);
> -	fastboot_okay("");
> +	fastboot_okay("", response);
>  }
> diff --git a/common/fb_nand.c b/common/fb_nand.c
> index aa28046..3a0b101 100644
> --- a/common/fb_nand.c
> +++ b/common/fb_nand.c
> @@ -32,7 +32,8 @@ __weak int
> board_fastboot_write_partition_setup(char *name) 
>  static int fb_nand_lookup(const char *partname,
>  			  struct mtd_info **mtd,
> -			  struct part_info **part)
> +			  struct part_info **part,
> +			  char *response)
>  {
>  	struct mtd_device *dev;
>  	int ret;
> @@ -41,21 +42,21 @@ static int fb_nand_lookup(const char *partname,
>  	ret = mtdparts_init();
>  	if (ret) {
>  		pr_err("Cannot initialize MTD partitions\n");
> -		fastboot_fail("cannot init mtdparts");
> +		fastboot_fail("cannot init mtdparts", response);
>  		return ret;
>  	}
>  
>  	ret = find_dev_and_part(partname, &dev, &pnum, part);
>  	if (ret) {
>  		pr_err("cannot find partition: '%s'", partname);
> -		fastboot_fail("cannot find partition");
> +		fastboot_fail("cannot find partition", response);
>  		return ret;
>  	}
>  
>  	if (dev->id->type != MTD_DEV_TYPE_NAND) {
>  		pr_err("partition '%s' is not stored on a NAND
> device", partname);
> -		fastboot_fail("not a NAND device");
> +		fastboot_fail("not a NAND device", response);
>  		return -EINVAL;
>  	}
>  
> @@ -146,16 +147,16 @@ static lbaint_t fb_nand_sparse_reserve(struct
> sparse_storage *info, }
>  
>  void fb_nand_flash_write(const char *cmd, void *download_buffer,
> -			 unsigned int download_bytes)
> +			 unsigned int download_bytes, char *response)
>  {
>  	struct part_info *part;
>  	struct mtd_info *mtd = NULL;
>  	int ret;
>  
> -	ret = fb_nand_lookup(cmd, &mtd, &part);
> +	ret = fb_nand_lookup(cmd, &mtd, &part, response);
>  	if (ret) {
>  		pr_err("invalid NAND device");
> -		fastboot_fail("invalid NAND device");
> +		fastboot_fail("invalid NAND device", response);
>  		return;
>  	}
>  
> @@ -181,7 +182,7 @@ void fb_nand_flash_write(const char *cmd, void
> *download_buffer, 
>  		sparse.priv = &sparse_priv;
>  		write_sparse_image(&sparse, cmd, download_buffer,
> -				   download_bytes);
> +				   download_bytes, response);
>  	} else {
>  		printf("Flashing raw image at offset 0x%llx\n",
>  		       part->offset);
> @@ -194,23 +195,23 @@ void fb_nand_flash_write(const char *cmd, void
> *download_buffer, }
>  
>  	if (ret) {
> -		fastboot_fail("error writing the image");
> +		fastboot_fail("error writing the image", response);
>  		return;
>  	}
>  
> -	fastboot_okay("");
> +	fastboot_okay("", response);
>  }
>  
> -void fb_nand_erase(const char *cmd)
> +void fb_nand_erase(const char *cmd, char *response)
>  {
>  	struct part_info *part;
>  	struct mtd_info *mtd = NULL;
>  	int ret;
>  
> -	ret = fb_nand_lookup(cmd, &mtd, &part);
> +	ret = fb_nand_lookup(cmd, &mtd, &part, response);
>  	if (ret) {
>  		pr_err("invalid NAND device");
> -		fastboot_fail("invalid NAND device");
> +		fastboot_fail("invalid NAND device", response);
>  		return;
>  	}
>  
> @@ -221,9 +222,9 @@ void fb_nand_erase(const char *cmd)
>  	ret = _fb_nand_erase(mtd, part);
>  	if (ret) {
>  		pr_err("failed erasing from device %s", mtd->name);
> -		fastboot_fail("failed erasing from device");
> +		fastboot_fail("failed erasing from device",
> response); return;
>  	}
>  
> -	fastboot_okay("");
> +	fastboot_okay("", response);
>  }
> diff --git a/common/image-sparse.c b/common/image-sparse.c
> index ddf5772..616c2bd 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, unsigned int sz, char *response)
>  {
>  	lbaint_t blk;
>  	lbaint_t blkcnt;
> @@ -101,7 +101,7 @@ 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");
> +		fastboot_fail("sparse image block size issue",
> response); return;
>  	}
>  
> @@ -136,8 +136,8 @@ 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");
> +				fastboot_fail("Bogus chunk size for
> chunk type Raw",
> +					      response);
>  				return;
>  			}
>  
> @@ -145,8 +145,8 @@ void write_sparse_image(
>  				printf(
>  				    "%s: Request would exceed
> partition size!\n", __func__);
> -				fastboot_fail(
> -				    "Request would exceed partition
> size!");
> +				fastboot_fail("Request would exceed
> partition size!",
> +					      response);
>  				return;
>  			}
>  
> @@ -156,8 +156,7 @@ void write_sparse_image(
>  				printf("%s: %s" LBAFU " [" LBAFU
> "]\n", __func__, "Write failed, block #",
>  				       blk, blks);
> -				fastboot_fail(
> -					      "flash write failure");
> +				fastboot_fail("flash write failure",
> response); return;
>  			}
>  			blk += blks;
> @@ -169,8 +168,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");
> +				fastboot_fail("Bogus chunk size for
> chunk type FILL",
> +					      response);
>  				return;
>  			}
>  
> @@ -180,8 +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");
> +				fastboot_fail("Malloc failed for:
> CHUNK_TYPE_FILL",
> +					      response);
>  				return;
>  			}
>  
> @@ -198,8 +197,8 @@ void write_sparse_image(
>  				printf(
>  				    "%s: Request would exceed
> partition size!\n", __func__);
> -				fastboot_fail(
> -				    "Request would exceed partition
> size!");
> +				fastboot_fail("Request would exceed
> partition size!",
> +					      response);
>  				return;
>  			}
>  
> @@ -214,8 +213,8 @@ void write_sparse_image(
>  					       __func__,
>  					       "Write failed, block
> #", blk, j);
> -					fastboot_fail(
> -						      "flash write
> failure");
> +					fastboot_fail("flash write
> failure",
> +						      response);
>  					free(fill_buf);
>  					return;
>  				}
> @@ -235,8 +234,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");
> +				fastboot_fail("Bogus chunk size for
> chunk type Dont Care",
> +					      response);
>  				return;
>  			}
>  			total_blocks += chunk_header->chunk_sz;
> @@ -246,7 +245,7 @@ void write_sparse_image(
>  		default:
>  			printf("%s: Unknown chunk type: %x\n",
> __func__, chunk_header->chunk_type);
> -			fastboot_fail("Unknown chunk type");
> +			fastboot_fail("Unknown chunk type",
> response); return;
>  		}
>  	}
> @@ -256,9 +255,9 @@ void write_sparse_image(
>  	printf("........ wrote %u bytes to '%s'\n", bytes_written,
> part_name); 
>  	if (total_blocks != sparse_header->total_blks)
> -		fastboot_fail("sparse image write failure");
> +		fastboot_fail("sparse image write failure",
> response); else
> -		fastboot_okay("");
> +		fastboot_okay("", response);
>  
>  	return;
>  }
> diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c index 7acffb6..6ae1d97 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -151,18 +151,16 @@ static void rx_handler_command(struct usb_ep
> *ep, struct usb_request *req); static int strcmp_l1(const char *s1,
> const char *s2); 
>  
> -static char *fb_response_str;
> -
> -void fastboot_fail(const char *reason)
> +void fastboot_fail(const char *reason, char *response)
>  {
> -	strncpy(fb_response_str, "FAIL\0", 5);
> -	strncat(fb_response_str, reason, FASTBOOT_RESPONSE_LEN - 4 -
> 1);
> +	strncpy(response, "FAIL\0", 5);
> +	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
>  }
>  
> -void fastboot_okay(const char *reason)
> +void fastboot_okay(const char *reason, char *response)
>  {
> -	strncpy(fb_response_str, "OKAY\0", 5);
> -	strncat(fb_response_str, reason, FASTBOOT_RESPONSE_LEN - 4 -
> 1);
> +	strncpy(response, "OKAY\0", 5);
> +	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
>  }
>  
>  static void fastboot_complete(struct usb_ep *ep, struct usb_request
> *req) @@ -598,18 +596,14 @@ static void cb_flash(struct usb_ep *ep,
> struct usb_request *req) return;
>  	}
>  
> -	/* initialize the response buffer */
> -	fb_response_str = response;
> -
> -	fastboot_fail("no flash device defined");
> +	fastboot_fail("no flash device defined", response);
>  #ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
>  	fb_mmc_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR,
> -			   download_bytes);
> +			   download_bytes, response);
>  #endif
>  #ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
> -	fb_nand_flash_write(cmd,
> -			    (void *)CONFIG_FASTBOOT_BUF_ADDR,
> -			    download_bytes);
> +	fb_nand_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR,
> +			    download_bytes, response);
>  #endif
>  	fastboot_tx_write_str(response);
>  }
> @@ -650,15 +644,12 @@ static void cb_erase(struct usb_ep *ep, struct
> usb_request *req) return;
>  	}
>  
> -	/* initialize the response buffer */
> -	fb_response_str = response;
> -
> -	fastboot_fail("no flash device defined");
> +	fastboot_fail("no flash device defined", response);
>  #ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
> -	fb_mmc_erase(cmd);
> +	fb_mmc_erase(cmd, response);
>  #endif
>  #ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
> -	fb_nand_erase(cmd);
> +	fb_nand_erase(cmd, response);
>  #endif
>  	fastboot_tx_write_str(response);
>  }
> diff --git a/include/fastboot.h b/include/fastboot.h
> index 616631e..f22080a 100644
> --- a/include/fastboot.h
> +++ b/include/fastboot.h
> @@ -16,7 +16,7 @@
>  /* The 64 defined bytes plus \0 */
>  #define FASTBOOT_RESPONSE_LEN	(64 + 1)
>  
> -void fastboot_fail(const char *reason);
> -void fastboot_okay(const char *reason);
> +void fastboot_fail(const char *reason, char *response);
> +void fastboot_okay(const char *reason, char *response);
>  
>  #endif /* _FASTBOOT_H_ */
> diff --git a/include/fb_mmc.h b/include/fb_mmc.h
> index 12b99cb..402ba9b 100644
> --- a/include/fb_mmc.h
> +++ b/include/fb_mmc.h
> @@ -5,5 +5,5 @@
>   */
>  
>  void fb_mmc_flash_write(const char *cmd, void *download_buffer,
> -			unsigned int download_bytes);
> -void fb_mmc_erase(const char *cmd);
> +			unsigned int download_bytes, char *response);
> +void fb_mmc_erase(const char *cmd, char *response);
> diff --git a/include/fb_nand.h b/include/fb_nand.h
> index aaf7cf7..88bdf36 100644
> --- a/include/fb_nand.h
> +++ b/include/fb_nand.h
> @@ -6,5 +6,5 @@
>   */
>  
>  void fb_nand_flash_write(const char *cmd, void *download_buffer,
> -			 unsigned int download_bytes);
> -void fb_nand_erase(const char *cmd);
> +			 unsigned int download_bytes, char
> *response); +void fb_nand_erase(const char *cmd, char *response);
> diff --git a/include/image-sparse.h b/include/image-sparse.h
> index b0cc500..02453c8 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, unsigned int sz, char *response);




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180425/6e3c2fa3/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 2/5] dfu: Extract fastboot_okay/fail to fb_common.c
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 2/5] dfu: Extract fastboot_okay/fail to fb_common.c Alex Kiernan
@ 2018-04-25  7:32   ` Lukasz Majewski
  2018-04-25  8:15     ` Alex Kiernan
  0 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2018-04-25  7:32 UTC (permalink / raw)
  To: u-boot

Hi Alex,

> Added common/fb_common.c, where fastboot_okay/fail are implemented
> so we can call them from a non-USB implementation.
> 
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> ---
> 
>  common/Makefile                 |  4 ++++
>  common/fb_common.c              | 26 ++++++++++++++++++++++++++
>  drivers/usb/gadget/f_fastboot.c | 13 -------------
>  3 files changed, 30 insertions(+), 13 deletions(-)
>  create mode 100644 common/fb_common.c
> 
> diff --git a/common/Makefile b/common/Makefile
> index 7011dad..8177341 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -124,6 +124,10 @@ endif
>  endif
>  endif
>  
> +ifneq ($(or
> $(CONFIG_USB_FUNCTION_FASTBOOT),$(CONFIG_UDP_FUNCTION_FASTBOOT)),)
> +obj-y += fb_common.o +endif
> +
>  ifdef CONFIG_CMD_EEPROM_LAYOUT
>  obj-y += eeprom/eeprom_field.o eeprom/eeprom_layout.o
>  endif
> diff --git a/common/fb_common.c b/common/fb_common.c
> new file mode 100644
> index 0000000..53cffe3
> --- /dev/null
> +++ b/common/fb_common.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2008 - 2009
> + * Windriver, <www.windriver.com>
> + * Tom Rix <Tom.Rix@windriver.com>
> + *
> + * Copyright 2011 Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> + *
> + * Copyright 2014 Linaro, Ltd.
> + * Rob Herring <robh@kernel.org>
> + */
> +
> +#include <common.h>
> +#include <fastboot.h>
> +
> +void fastboot_fail(const char *reason, char *response)
> +{
> +	strncpy(response, "FAIL\0", 5);
> +	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
				  ^^^^^^^ This looks a bit
				  "hand-crafted". I suppose that it is
				  to prevent not adding "reason" when
				  response is large ?
> +}
> +
> +void fastboot_okay(const char *reason, char *response)
> +{
> +	strncpy(response, "OKAY\0", 5);
> +	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
> +}
> diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c index 6ae1d97..fda4505 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -150,19 +150,6 @@ static struct usb_gadget_strings
> *fastboot_strings[] = { static void rx_handler_command(struct usb_ep
> *ep, struct usb_request *req); static int strcmp_l1(const char *s1,
> const char *s2); 
> -
> -void fastboot_fail(const char *reason, char *response)
> -{
> -	strncpy(response, "FAIL\0", 5);
> -	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
> -}
> -
> -void fastboot_okay(const char *reason, char *response)
> -{
> -	strncpy(response, "OKAY\0", 5);
> -	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
> -}
> -
>  static void fastboot_complete(struct usb_ep *ep, struct usb_request
> *req) {
>  	int status = req->status;




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180425/5a77ee0b/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-24 11:58   ` Alex Kiernan
  2018-04-24 17:10     ` Jocelyn Bohr
@ 2018-04-25  7:40     ` Lukasz Majewski
  1 sibling, 0 replies; 31+ messages in thread
From: Lukasz Majewski @ 2018-04-25  7:40 UTC (permalink / raw)
  To: u-boot

Hi Alex,

> On Tue, Apr 24, 2018 at 11:24 AM, Alex Deymo <deymo+@google.com>
> wrote:
> > +Jocelyn.
> >
> > Thanks Alex for taking the time to port this!!  
> 
> It turned out to be a great opportunity to play with coccinelle on
> something trivial, which I'd been meaning to do for ages... the
> refactor to add response into the fastboot_okay/fail call chain was a
> breeze with it.
> 
> > 2018-04-24 11:37 GMT+02:00 Alex Kiernan <alex.kiernan@gmail.com>:  
> >>
> >>
> >> This series merges the fastboot UDP support from AOSP into mainline
> >> U-Boot.
> >>
> >> Open questions:
> >>
> >> - what's the correct way of attributing the original authors? I've
> >> added Co-authored-by, is that right? checkpatch doesn't seem to
> >> know any of the co- tags
> >> - currently there's no NAND support and I've no way of testing
> >> that, so my inclination is towards leaving it like that until
> >> someone with that particular itch to scratch can look at it  
> >
> >
> > Fastboot uses partition names, like "system" and "boot" which have
> > a meaning in the Android partition scheme. For GPT, we just use the
> > partition names as the android names; but if you are booting from
> > some other storage like NAND where you don't have that then you
> > need some mapping glue ("system" --> device and partition number).
> > I've seen U-Boot modifications for several devices where they just
> > stick a table hard-coded in the U-Boot code; which works great for
> > that device but it isn't really a generic approach. Other than
> > handling the names issue, I don't see any problem with supporting
> > NAND here, but a generic way to set global names / alias would be
> > needed for this. 
> 
> With the fastboot NAND support in mainline it looks like we end up in
> mtdparts to deliver that mapping through environment variables. 

When you mentioned about NAND partitions - I also have thought about
mtdparts. They are well supported in mainline.

> No
> actual idea what that looks like when you're driving it.

Many boards use them - so there should not be any issue with finding
examples.

> 
> >>
> >> - you can select both USB and UDP fastboot, but the comments in the
> >>   AOSP code suggest that needs fixing - again, I've no board I can
> >> test USB fastboot on  
> >
> >
> > I thought we checked in the Kconfig that you couldn't enable both.
> > I don't remember the details now but yeah you can't wait for
> > network or USB traffic on the current code.
> >  
> 
> The version I picked from (o-mr1-iot-preview-8) has them as
> independent symbols, but making them a choice would resolve the issue
> for now.
> 
> >> I've not tested all the code paths yet, but the obvious stuff works
> >> (basic interaction, download, boot) - every interaction elicits a
> >> `WARNING: unknown variable: slot-count` on the console; I'm
> >> guessing that my local end is sending a getvar for that, but I've
> >> not investigated.  
> >
> >
> > Yes, there's a bit of different handling of partitions for A/B
> > android devices. Instead of "system" you have two partitions:
> > "system_a" and "system_b" (potentially more although 2 is the most
> > common case) so to know whether you have an old device or a newer
> > device the fastboot client checks the "slot-count" variable.
> > Undefined means of course that you have an old-style device, but if
> > you return something like "2" you would be able to flash "system"
> > on the "slot A" which is translated (again in the fastboot client)
> > to flash "system_a" partition. This is useful when using the higher
> > level operations like flashall/update and you want to specify to
> > flash only "slot A", "slot B" or even both of them. There are other
> > fastboot variables that require some plumbing, such as
> > "has-slot:<partition>" to tell whether "system" is a partition that
> > indeed has the two versions _a and _b. Typically some partitions
> > are twice (like "system" and "boot") and some other are not (like
> > "misc" or "userdata"). Anyway, this as is should work for either
> > old-style partition schemes or by force flashing "system_a" with
> > the system.img. 
> 
> Cool, thanks... that saves me a bunch of investigation!
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180425/f5ded8ea/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-24  9:37 [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Kiernan
                   ` (5 preceding siblings ...)
  2018-04-24 10:24 ` [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Deymo
@ 2018-04-25  7:52 ` Lukasz Majewski
  2018-04-25  7:57   ` Lukasz Majewski
  2018-04-25  8:43   ` Alex Kiernan
  2018-04-27 19:10 ` Sam Protsenko
  7 siblings, 2 replies; 31+ messages in thread
From: Lukasz Majewski @ 2018-04-25  7:52 UTC (permalink / raw)
  To: u-boot

Hi Alex,

> Open questions:
> 
> - what's the correct way of attributing the original authors? I've
> added Co-authored-by, is that right? checkpatch doesn't seem to know
> any of the co- tags

I think that two authors (Alex and Jocelyn) have replied to your e-mail.

Maybe it would be doable to have S-o-B or Acked-by from them?

> - currently there's no NAND support and I've no way of testing that,
> so my inclination is towards leaving it like that until someone with
> that particular itch to scratch can look at it

As I've written in the other mail - please look into "mtdparts".

> - you can select both USB and UDP fastboot, but the comments in the
>   AOSP code suggest that needs fixing - again, I've no board I can
> test USB fastboot on

I need to check if I do posses a bort with fastboot support (as I
mostly test DFU).

> - the USB and UDP code want consolidating, with this series there
> would then be two separate implementations of the same protocol

Yes - this is an issue. 

For USB the fastboot protocol (at least some its logic) is implemented
in drivers/usb/gadget/f_fastboot.c

I think that some medium agnostic code (like protocol itself) can be
unified.

I also think that it would be a good idea to have ./drivers/fastboot
directory introduced to move common code there - and leave only USB
related code in f_fastboot.c

Please as a reference, look on how ./drivers/dfu is organised.

> - the syntax for the USB fastboot command changes from `fastboot
> <controller>` to `fastboot usb <controller>`, that feels unacceptable
> and we probably want something like `fastboot <controller>` or
> `fastboot udp`?

Similar issue was with "thor"/"ums"/"dfu" command previously.

For backward compatibility we can make an alias:

fastboot <controller> -> fastboot usb <controller>

IMHO, it would be great to have

fastboot usb <controller>
fastboot udp ip.... (etc)

(It would be great to have the same syntax as it is in Android).

> - unrelated to this series, but a show-stopper for me, there's no FIT
> image support, but that doesn't feel unsolveable - something like add
> an option to pass in the load address and/or use loadaddr, then
> something else to let you override the bootm invocation on the server
> side




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180425/1e77583c/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 4/5] dfu: Resolve Kconfig dependency loops
  2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 4/5] dfu: Resolve Kconfig dependency loops Alex Kiernan
@ 2018-04-25  7:53   ` Lukasz Majewski
  2018-04-25  8:55     ` Alex Deymo
  0 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2018-04-25  7:53 UTC (permalink / raw)
  To: u-boot

Hi Alex,

> Fix recursive dependencies in Kconfig introduced by fastboot UDP
> 
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> ---
> 
>  cmd/fastboot/Kconfig | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/cmd/fastboot/Kconfig b/cmd/fastboot/Kconfig
> index 0c57616..0f804ea 100644
> --- a/cmd/fastboot/Kconfig
> +++ b/cmd/fastboot/Kconfig
> @@ -2,13 +2,13 @@ comment "FASTBOOT"
>  
>  menuconfig FASTBOOT
>  	bool "Fastboot support"
> -	depends on USB_GADGET
>  	default y if ARCH_SUNXI && USB_MUSB_GADGET
>  
>  if FASTBOOT
>  
>  config USB_FUNCTION_FASTBOOT
>  	bool "Enable USB fastboot gadget"
> +	depends on USB_GADGET
>  	default y
>  	select USB_GADGET_DOWNLOAD
>  	imply ANDROID_BOOT_IMAGE
> @@ -17,7 +17,7 @@ config USB_FUNCTION_FASTBOOT
>  	  This enables the USB part of the fastboot gadget.
>  
>  config UDP_FUNCTION_FASTBOOT
> -	select NET
> +	depends on NET
>  	bool "Enable fastboot protocol over UDP"
>  	help
>  	  This enables the fastboot protocol over UDP.
> @@ -66,6 +66,7 @@ config FASTBOOT_BUF_SIZE
>  
>  config FASTBOOT_USB_DEV
>  	int "USB controller number"
> +	depends on USB_FUNCTION_FASTBOOT
>  	default 0
>  	help
>  	  Some boards have USB OTG controller other than 0. Define
> this

I think that it should be possible to have fastboot support enabled for
both USB and ETH if a board has those interfaces present.

Then by using proper commands:

fastboot usb or fastboot udp we can decide which medium would be used.

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180425/edf14849/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-24 17:10     ` Jocelyn Bohr
@ 2018-04-25  7:53       ` Alex Kiernan
  2018-04-25 12:50         ` Alex Kiernan
  2018-04-26  3:27       ` Kever Yang
  1 sibling, 1 reply; 31+ messages in thread
From: Alex Kiernan @ 2018-04-25  7:53 UTC (permalink / raw)
  To: u-boot

On Tue, Apr 24, 2018 at 6:10 PM, Jocelyn Bohr <bohr@verily.com> wrote:
> Thanks so much for porting this, Alex!
>
> On Tue, Apr 24, 2018 at 4:58 AM Alex Kiernan <alex.kiernan@gmail.com> wrote:
>>
>> On Tue, Apr 24, 2018 at 11:24 AM, Alex Deymo <deymo+@google.com> wrote:
>> > +Jocelyn.
>> >
>> > Thanks Alex for taking the time to port this!!
>>
>> It turned out to be a great opportunity to play with coccinelle on
>> something trivial, which I'd been meaning to do for ages... the
>> refactor to add response into the fastboot_okay/fail call chain was a
>> breeze with it.
>>
>> > 2018-04-24 11:37 GMT+02:00 Alex Kiernan <alex.kiernan@gmail.com>:
>> >>
>> >>
>> >> This series merges the fastboot UDP support from AOSP into mainline
>> >> U-Boot.
>> >>
>> >> Open questions:
>> >>
>> >> - what's the correct way of attributing the original authors? I've
>> >> added
>> >>   Co-authored-by, is that right? checkpatch doesn't seem to know any
>> >>   of the co- tags
>> >> - currently there's no NAND support and I've no way of testing that, so
>> >>   my inclination is towards leaving it like that until someone with
>> >> that
>> >>   particular itch to scratch can look at it
>> >
>> >
>> > Fastboot uses partition names, like "system" and "boot" which have a
>> > meaning
>> > in the Android partition scheme. For GPT, we just use the partition
>> > names as
>> > the android names; but if you are booting from some other storage like
>> > NAND
>> > where you don't have that then you need some mapping glue ("system" -->
>> > device and partition number). I've seen U-Boot modifications for several
>> > devices where they just stick a table hard-coded in the U-Boot code;
>> > which
>> > works great for that device but it isn't really a generic approach.
>> > Other
>> > than handling the names issue, I don't see any problem with supporting
>> > NAND
>> > here, but a generic way to set global names / alias would be needed for
>> > this.
>> >
>>
>> With the fastboot NAND support in mainline it looks like we end up in
>> mtdparts to deliver that mapping through environment variables. No
>> actual idea what that looks like when you're driving it.
>>
>> >>
>> >> - you can select both USB and UDP fastboot, but the comments in the
>> >>   AOSP code suggest that needs fixing - again, I've no board I can test
>> >>   USB fastboot on
>> >
>> >
>> > I thought we checked in the Kconfig that you couldn't enable both. I
>> > don't
>> > remember the details now but yeah you can't wait for network or USB
>> > traffic
>> > on the current code.
>> >
>>
>> The version I picked from (o-mr1-iot-preview-8) has them as
>> independent symbols, but making them a choice would resolve the issue
>> for now.
>
>
> You can select both network and USB fastboot to be enabled with the Kconfig,
> but at runtime you have to select to wait on either USB or network by
> running
> "fastboot udp" or "fastboot usb <USB_controller>". When the Android
> bootloader
> gets the command to reboot back to fastboot, it will read the "fastbootcmd"
> env
> variable and run that as a command (common/android_bootloader.c:151).
>

Thanks for that (especially the detail on android_bootloader which I'd
not read through). The bit that concerns me is in timed_send_info:

  +/**
  + * Send an INFO packet during long commands based on timer. If
  + * CONFIG_UDP_FUNCTION_FASTBOOT is defined, an INFO packet is sent
  + * if the time is 30 seconds after start. Else, noop.
  + *
  + * TODO: Handle the situation where both UDP and USB fastboot are
  + *       enabled.
  + *
  + * @param start:  Time since last INFO packet was sent.
  + * @param msg:    String describing the reason for waiting
  + */
  +void timed_send_info(ulong *start, const char *msg);

The code in timed_send_info is guarded with an ifdef, rather than
based on the transport that's been selected at runtime. Do we need to
make timed_send_info work based on the runtime state, rather than the
compile time, or can we drop the ifdef guard and remove the TODO? I
guess I'm assuming the former, but with no way to test USB I don't
want head off down the wrong road!

-- 
Alex Kiernan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-25  7:52 ` Lukasz Majewski
@ 2018-04-25  7:57   ` Lukasz Majewski
  2018-04-25  8:43   ` Alex Kiernan
  1 sibling, 0 replies; 31+ messages in thread
From: Lukasz Majewski @ 2018-04-25  7:57 UTC (permalink / raw)
  To: u-boot

Hi Alex,

> > - the USB and UDP code want consolidating, with this series there
> > would then be two separate implementations of the same protocol  
> 
> Yes - this is an issue. 
> 
> For USB the fastboot protocol (at least some its logic) is implemented
> in drivers/usb/gadget/f_fastboot.c
> 
> I think that some medium agnostic code (like protocol itself) can be
> unified.
> 
> I also think that it would be a good idea to have ./drivers/fastboot
> directory introduced to move common code there - and leave only USB
> related code in f_fastboot.c

Especially that we do have fb_nand.c and fb_mmc.c in ./common.

In that way we could move them to ./drivers/fastboot.

> 
> Please as a reference, look on how ./drivers/dfu is organised.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180425/83fe83f8/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 1/5] dfu: Refactor fastboot_okay/fail to take response
  2018-04-25  7:27   ` Lukasz Majewski
@ 2018-04-25  8:10     ` Alex Kiernan
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Kiernan @ 2018-04-25  8:10 UTC (permalink / raw)
  To: u-boot

On Wed, Apr 25, 2018 at 8:27 AM, Lukasz Majewski <lukma@denx.de> wrote:
> Hi Alex,
>
> Please change the subject prefix - i.e. dfu: is a totally different
> code.
>
> Maybe fastboot would be more appropriate?
>

Sorry, was trying to pick something in the right kind of space/known
to git-mailrc, but I'll change that.

>> Add the response string as a parameter to fastboot_okay/fail, instead
>> of modifying a global, to match the contract expected by the AOSP
>> U-Boot code.
>
> I suppose that this change is to make the u-boot mainline code matching
> the current Android?
>

Yes. When you get to the guts of it, the interface that was there, was
a bit odd - callers had to initialise the global fb_response_str
before calling into the rest of the fastboot code.

-- 
Alex Kiernan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 2/5] dfu: Extract fastboot_okay/fail to fb_common.c
  2018-04-25  7:32   ` Lukasz Majewski
@ 2018-04-25  8:15     ` Alex Kiernan
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Kiernan @ 2018-04-25  8:15 UTC (permalink / raw)
  To: u-boot

On Wed, Apr 25, 2018 at 8:32 AM, Lukasz Majewski <lukma@denx.de> wrote:
> Hi Alex,
>
>> Added common/fb_common.c, where fastboot_okay/fail are implemented
>> so we can call them from a non-USB implementation.
>>
>> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
>> ---
>>
>>  common/Makefile                 |  4 ++++
>>  common/fb_common.c              | 26 ++++++++++++++++++++++++++
>>  drivers/usb/gadget/f_fastboot.c | 13 -------------
>>  3 files changed, 30 insertions(+), 13 deletions(-)
>>  create mode 100644 common/fb_common.c
>>
>> diff --git a/common/Makefile b/common/Makefile
>> index 7011dad..8177341 100644
>> --- a/common/Makefile
>> +++ b/common/Makefile
>> @@ -124,6 +124,10 @@ endif
>>  endif
>>  endif
>>
>> +ifneq ($(or
>> $(CONFIG_USB_FUNCTION_FASTBOOT),$(CONFIG_UDP_FUNCTION_FASTBOOT)),)
>> +obj-y += fb_common.o +endif
>> +
>>  ifdef CONFIG_CMD_EEPROM_LAYOUT
>>  obj-y += eeprom/eeprom_field.o eeprom/eeprom_layout.o
>>  endif
>> diff --git a/common/fb_common.c b/common/fb_common.c
>> new file mode 100644
>> index 0000000..53cffe3
>> --- /dev/null
>> +++ b/common/fb_common.c
>> @@ -0,0 +1,26 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2008 - 2009
>> + * Windriver, <www.windriver.com>
>> + * Tom Rix <Tom.Rix@windriver.com>
>> + *
>> + * Copyright 2011 Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> + *
>> + * Copyright 2014 Linaro, Ltd.
>> + * Rob Herring <robh@kernel.org>
>> + */
>> +
>> +#include <common.h>
>> +#include <fastboot.h>
>> +
>> +void fastboot_fail(const char *reason, char *response)
>> +{
>> +     strncpy(response, "FAIL\0", 5);
>> +     strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
>                                   ^^^^^^^ This looks a bit
>                                   "hand-crafted". I suppose that it is
>                                   to prevent not adding "reason" when
>                                   response is large ?

It is a bit, isn't it. Obviously it was a simple cut & paste refactor,
but I can't think of any reason not to turn it into an snprintf.

-- 
Alex Kiernan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-25  7:52 ` Lukasz Majewski
  2018-04-25  7:57   ` Lukasz Majewski
@ 2018-04-25  8:43   ` Alex Kiernan
  2018-04-25  8:46     ` Alex Deymo
  1 sibling, 1 reply; 31+ messages in thread
From: Alex Kiernan @ 2018-04-25  8:43 UTC (permalink / raw)
  To: u-boot

On Wed, Apr 25, 2018 at 8:52 AM, Lukasz Majewski <lukma@denx.de> wrote:
> Hi Alex,
>
>> Open questions:
>>
>> - what's the correct way of attributing the original authors? I've
>> added Co-authored-by, is that right? checkpatch doesn't seem to know
>> any of the co- tags
>
> I think that two authors (Alex and Jocelyn) have replied to your e-mail.
>
> Maybe it would be doable to have S-o-B or Acked-by from them?
>

Alex, Jocelyn, would you be happy for me to add a Signed-off-by from you?

-- 
Alex Kiernan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-25  8:43   ` Alex Kiernan
@ 2018-04-25  8:46     ` Alex Deymo
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Deymo @ 2018-04-25  8:46 UTC (permalink / raw)
  To: u-boot

2018-04-25 10:43 GMT+02:00 Alex Kiernan <alex.kiernan@gmail.com>:

> >> - what's the correct way of attributing the original authors? I've
> >> added Co-authored-by, is that right? checkpatch doesn't seem to know
> >> any of the co- tags
> >
> > I think that two authors (Alex and Jocelyn) have replied to your e-mail.
> >
> > Maybe it would be doable to have S-o-B or Acked-by from them?
> >
>
> Alex, Jocelyn, would you be happy for me to add a Signed-off-by from you?
>

Sure!

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 4/5] dfu: Resolve Kconfig dependency loops
  2018-04-25  7:53   ` Lukasz Majewski
@ 2018-04-25  8:55     ` Alex Deymo
  0 siblings, 0 replies; 31+ messages in thread
From: Alex Deymo @ 2018-04-25  8:55 UTC (permalink / raw)
  To: u-boot

2018-04-25 9:53 GMT+02:00 Lukasz Majewski <lukma@denx.de>:

> Hi Alex,
>
> > Fix recursive dependencies in Kconfig introduced by fastboot UDP
> >
> > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> > ---
> >
> >  cmd/fastboot/Kconfig | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/cmd/fastboot/Kconfig b/cmd/fastboot/Kconfig
> > index 0c57616..0f804ea 100644
> > --- a/cmd/fastboot/Kconfig
> > +++ b/cmd/fastboot/Kconfig
> > @@ -2,13 +2,13 @@ comment "FASTBOOT"
> >
> >  menuconfig FASTBOOT
> >       bool "Fastboot support"
> > -     depends on USB_GADGET
> >       default y if ARCH_SUNXI && USB_MUSB_GADGET
> >
> >  if FASTBOOT
> >
> >  config USB_FUNCTION_FASTBOOT
> >       bool "Enable USB fastboot gadget"
> > +     depends on USB_GADGET
> >       default y
> >       select USB_GADGET_DOWNLOAD
> >       imply ANDROID_BOOT_IMAGE
> > @@ -17,7 +17,7 @@ config USB_FUNCTION_FASTBOOT
> >         This enables the USB part of the fastboot gadget.
> >
> >  config UDP_FUNCTION_FASTBOOT
> > -     select NET
> > +     depends on NET
> >       bool "Enable fastboot protocol over UDP"
> >       help
> >         This enables the fastboot protocol over UDP.
> > @@ -66,6 +66,7 @@ config FASTBOOT_BUF_SIZE
> >
> >  config FASTBOOT_USB_DEV
> >       int "USB controller number"
> > +     depends on USB_FUNCTION_FASTBOOT
> >       default 0
> >       help
> >         Some boards have USB OTG controller other than 0. Define
> > this
>
> I think that it should be possible to have fastboot support enabled for
> both USB and ETH if a board has those interfaces present.
>
> Then by using proper commands:
>
> fastboot usb or fastboot udp we can decide which medium would be used.
>
> I agree that compiling support for both should be possible (I'm not sure
if it is really that useful). I was referring to that supporting fastboot
in both interfaces at the same time would be very complicated; but
selecting at runtime whether you want "fastboot udp" or "fastboot usb" is
easy.

deymo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-25  7:53       ` Alex Kiernan
@ 2018-04-25 12:50         ` Alex Kiernan
  2018-04-25 15:19           ` Lukasz Majewski
  0 siblings, 1 reply; 31+ messages in thread
From: Alex Kiernan @ 2018-04-25 12:50 UTC (permalink / raw)
  To: u-boot

On Wed, Apr 25, 2018 at 8:53 AM, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> On Tue, Apr 24, 2018 at 6:10 PM, Jocelyn Bohr <bohr@verily.com> wrote:
>> Thanks so much for porting this, Alex!
>>

...

>>
>> You can select both network and USB fastboot to be enabled with the Kconfig,
>> but at runtime you have to select to wait on either USB or network by
>> running
>> "fastboot udp" or "fastboot usb <USB_controller>". When the Android
>> bootloader
>> gets the command to reboot back to fastboot, it will read the "fastbootcmd"
>> env
>> variable and run that as a command (common/android_bootloader.c:151).
>>
>
> Thanks for that (especially the detail on android_bootloader which I'd
> not read through). The bit that concerns me is in timed_send_info:
>
>   +/**
>   + * Send an INFO packet during long commands based on timer. If
>   + * CONFIG_UDP_FUNCTION_FASTBOOT is defined, an INFO packet is sent
>   + * if the time is 30 seconds after start. Else, noop.
>   + *
>   + * TODO: Handle the situation where both UDP and USB fastboot are
>   + *       enabled.
>   + *
>   + * @param start:  Time since last INFO packet was sent.
>   + * @param msg:    String describing the reason for waiting
>   + */
>   +void timed_send_info(ulong *start, const char *msg);
>
> The code in timed_send_info is guarded with an ifdef, rather than
> based on the transport that's been selected at runtime. Do we need to
> make timed_send_info work based on the runtime state, rather than the
> compile time, or can we drop the ifdef guard and remove the TODO? I
> guess I'm assuming the former, but with no way to test USB I don't
> want head off down the wrong road!
>

Having started digging through how we'd merge the two code paths, it's
clear if you had UDP and USB both enabled at compile time and then try
and run the USB path it'll try and do UDP things in timed_send_info,
which can't be good!

-- 
Alex Kiernan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-25 12:50         ` Alex Kiernan
@ 2018-04-25 15:19           ` Lukasz Majewski
  0 siblings, 0 replies; 31+ messages in thread
From: Lukasz Majewski @ 2018-04-25 15:19 UTC (permalink / raw)
  To: u-boot

On Wed, 25 Apr 2018 13:50:50 +0100
Alex Kiernan <alex.kiernan@gmail.com> wrote:

> On Wed, Apr 25, 2018 at 8:53 AM, Alex Kiernan
> <alex.kiernan@gmail.com> wrote:
> > On Tue, Apr 24, 2018 at 6:10 PM, Jocelyn Bohr <bohr@verily.com>
> > wrote:  
> >> Thanks so much for porting this, Alex!
> >>  
> 
> ...
> 
> >>
> >> You can select both network and USB fastboot to be enabled with
> >> the Kconfig, but at runtime you have to select to wait on either
> >> USB or network by running
> >> "fastboot udp" or "fastboot usb <USB_controller>". When the Android
> >> bootloader
> >> gets the command to reboot back to fastboot, it will read the
> >> "fastbootcmd" env
> >> variable and run that as a command
> >> (common/android_bootloader.c:151). 
> >
> > Thanks for that (especially the detail on android_bootloader which
> > I'd not read through). The bit that concerns me is in
> > timed_send_info:
> >
> >   +/**
> >   + * Send an INFO packet during long commands based on timer. If
> >   + * CONFIG_UDP_FUNCTION_FASTBOOT is defined, an INFO packet is
> > sent
> >   + * if the time is 30 seconds after start. Else, noop.
> >   + *
> >   + * TODO: Handle the situation where both UDP and USB fastboot are
> >   + *       enabled.
> >   + *
> >   + * @param start:  Time since last INFO packet was sent.
> >   + * @param msg:    String describing the reason for waiting
> >   + */
> >   +void timed_send_info(ulong *start, const char *msg);
> >
> > The code in timed_send_info is guarded with an ifdef, rather than
> > based on the transport that's been selected at runtime. Do we need
> > to make timed_send_info work based on the runtime state, rather
> > than the compile time, or can we drop the ifdef guard and remove
> > the TODO? I guess I'm assuming the former, but with no way to test
> > USB I don't want head off down the wrong road!
> >  
> 
> Having started digging through how we'd merge the two code paths, it's
> clear if you had UDP and USB both enabled at compile time and then try
> and run the USB path it'll try and do UDP things in timed_send_info,
> which can't be good!
> 

I think that we can assume operation of only one medium in a time - i.e.

when you issue fastboot usb then no UDP support (and opposite).


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180425/d9c5972a/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-24 17:10     ` Jocelyn Bohr
  2018-04-25  7:53       ` Alex Kiernan
@ 2018-04-26  3:27       ` Kever Yang
  2018-04-27  8:04         ` Lukasz Majewski
  1 sibling, 1 reply; 31+ messages in thread
From: Kever Yang @ 2018-04-26  3:27 UTC (permalink / raw)
  To: u-boot

Hi Jocelyn and Alex,


    It's great to see you are getting this feature from google code to be
mainline code.

    My opinion is that we should have the same fastboot cmd handling
in both UDP and USB in a common file, but not have separate code in
net/fastboot.c and drivers/usb/gadget/f_fastboot.c, could you try to merge
these two in one?

Thanks,
- Kever

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-26  3:27       ` Kever Yang
@ 2018-04-27  8:04         ` Lukasz Majewski
  2018-04-27 12:20           ` Alex Kiernan
  0 siblings, 1 reply; 31+ messages in thread
From: Lukasz Majewski @ 2018-04-27  8:04 UTC (permalink / raw)
  To: u-boot

Hi Kever,

> Hi Jocelyn and Alex,
> 
> 
>     It's great to see you are getting this feature from google code
> to be mainline code.
> 
>     My opinion is that we should have the same fastboot cmd handling
> in both UDP and USB in a common file, but not have separate code in
> net/fastboot.c and drivers/usb/gadget/f_fastboot.c, could you try to
> merge these two in one?

I've already proposed to put common code (the protocol handling)
into /drivers/fastboot and have only small glue code for USB and NET.

> 
> Thanks,
> - Kever
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180427/e7970950/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-27  8:04         ` Lukasz Majewski
@ 2018-04-27 12:20           ` Alex Kiernan
  2018-04-30  8:37             ` Alex Kiernan
  0 siblings, 1 reply; 31+ messages in thread
From: Alex Kiernan @ 2018-04-27 12:20 UTC (permalink / raw)
  To: u-boot

On Fri, Apr 27, 2018 at 9:04 AM, Lukasz Majewski <lukma@denx.de> wrote:
> Hi Kever,
>
>> Hi Jocelyn and Alex,
>>
>>
>>     It's great to see you are getting this feature from google code
>> to be mainline code.
>>
>>     My opinion is that we should have the same fastboot cmd handling
>> in both UDP and USB in a common file, but not have separate code in
>> net/fastboot.c and drivers/usb/gadget/f_fastboot.c, could you try to
>> merge these two in one?
>
> I've already proposed to put common code (the protocol handling)
> into /drivers/fastboot and have only small glue code for USB and NET.
>

I'm working towards that. I'm starting with the command
implementations, then I'll look at the protocol.

Hopefully I'll get an RFCv2 out sometime over the weekend with the
first bits done.

-- 
Alex Kiernan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-24  9:37 [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Kiernan
                   ` (6 preceding siblings ...)
  2018-04-25  7:52 ` Lukasz Majewski
@ 2018-04-27 19:10 ` Sam Protsenko
  2018-04-27 19:23   ` Alex Kiernan
  2018-04-30  8:56   ` Alex Deymo
  7 siblings, 2 replies; 31+ messages in thread
From: Sam Protsenko @ 2018-04-27 19:10 UTC (permalink / raw)
  To: u-boot

On 24 April 2018 at 12:37, Alex Kiernan <alex.kiernan@gmail.com> wrote:
>
> This series merges the fastboot UDP support from AOSP into mainline
> U-Boot.
>

Can you please point me out from which AOSP code exactly this code was
ported? Links to code, maybe some docs. Thanks.

> Open questions:
>
> - what's the correct way of attributing the original authors? I've added
>   Co-authored-by, is that right? checkpatch doesn't seem to know any
>   of the co- tags
> - currently there's no NAND support and I've no way of testing that, so
>   my inclination is towards leaving it like that until someone with that
>   particular itch to scratch can look at it
> - you can select both USB and UDP fastboot, but the comments in the
>   AOSP code suggest that needs fixing - again, I've no board I can test
>   USB fastboot on
> - the USB and UDP code want consolidating, with this series there would
>   then be two separate implementations of the same protocol
> - the syntax for the USB fastboot command changes from `fastboot <controller>`
>   to `fastboot usb <controller>`, that feels unacceptable and we probably
>   want something like `fastboot <controller>` or `fastboot udp`?
> - unrelated to this series, but a show-stopper for me, there's no FIT image
>   support, but that doesn't feel unsolveable - something like add an option
>   to pass in the load address and/or use loadaddr, then something else to
>   let you override the bootm invocation on the server side
>
> I've not tested all the code paths yet, but the obvious stuff works
> (basic interaction, download, boot) - every interaction elicits a
> `WARNING: unknown variable: slot-count` on the console; I'm guessing that
> my local end is sending a getvar for that, but I've not investigated.
>
> Without any way of testing any of the USB/NAND code I'm nervous of wading
> into that kind of refactoring, would that be a pre-requisite for merging?
>
>
> Alex Kiernan (5):
>   dfu: Refactor fastboot_okay/fail to take response
>   dfu: Extract fastboot_okay/fail to fb_common.c
>   net: dfu: Merge AOSP UDP fastboot
>   dfu: Resolve Kconfig dependency loops
>   net: dfu: Support building without MMC
>
>  cmd/fastboot.c                  |  32 ++-
>  cmd/fastboot/Kconfig            |  21 +-
>  cmd/net.c                       |   6 +
>  common/Makefile                 |   4 +
>  common/fb_common.c              |  44 ++++
>  common/fb_mmc.c                 | 114 ++++++---
>  common/fb_nand.c                |  31 +--
>  common/image-sparse.c           |  41 ++-
>  drivers/usb/gadget/f_fastboot.c |  36 +--
>  include/fastboot.h              |  17 +-
>  include/fb_mmc.h                |   4 +-
>  include/fb_nand.h               |   4 +-
>  include/image-sparse.h          |   2 +-
>  include/net.h                   |   6 +-
>  include/net/fastboot.h          |  27 ++
>  net/Makefile                    |   1 +
>  net/fastboot.c                  | 548 ++++++++++++++++++++++++++++++++++++++++
>  net/net.c                       |   9 +
>  18 files changed, 824 insertions(+), 123 deletions(-)
>  create mode 100644 common/fb_common.c
>  create mode 100644 include/net/fastboot.h
>  create mode 100644 net/fastboot.c
>
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-27 19:10 ` Sam Protsenko
@ 2018-04-27 19:23   ` Alex Kiernan
  2018-04-30  8:56   ` Alex Deymo
  1 sibling, 0 replies; 31+ messages in thread
From: Alex Kiernan @ 2018-04-27 19:23 UTC (permalink / raw)
  To: u-boot

Hi Sam,

On Fri, Apr 27, 2018 at 8:10 PM, Sam Protsenko
<semen.protsenko@linaro.org> wrote:
> On 24 April 2018 at 12:37, Alex Kiernan <alex.kiernan@gmail.com> wrote:
>>
>> This series merges the fastboot UDP support from AOSP into mainline
>> U-Boot.
>>
>
> Can you please point me out from which AOSP code exactly this code was
> ported? Links to code, maybe some docs. Thanks.
>

Sure:

https://android.googlesource.com/platform/external/u-boot/+/android-o-mr1-iot-preview-8

-- 
Alex Kiernan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-27 12:20           ` Alex Kiernan
@ 2018-04-30  8:37             ` Alex Kiernan
  2018-05-01  6:16               ` Jocelyn Bohr
  0 siblings, 1 reply; 31+ messages in thread
From: Alex Kiernan @ 2018-04-30  8:37 UTC (permalink / raw)
  To: u-boot

On Fri, Apr 27, 2018 at 1:20 PM, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> On Fri, Apr 27, 2018 at 9:04 AM, Lukasz Majewski <lukma@denx.de> wrote:
>> Hi Kever,
>>
>>> Hi Jocelyn and Alex,
>>>
>>>
>>>     It's great to see you are getting this feature from google code
>>> to be mainline code.
>>>
>>>     My opinion is that we should have the same fastboot cmd handling
>>> in both UDP and USB in a common file, but not have separate code in
>>> net/fastboot.c and drivers/usb/gadget/f_fastboot.c, could you try to
>>> merge these two in one?
>>
>> I've already proposed to put common code (the protocol handling)
>> into /drivers/fastboot and have only small glue code for USB and NET.
>>
>
> I'm working towards that. I'm starting with the command
> implementations, then I'll look at the protocol.
>
> Hopefully I'll get an RFCv2 out sometime over the weekend with the
> first bits done.
>

Just posted a v2, still needs more work, but getting feedback on the
direction would be useful.

-- 
Alex Kiernan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-27 19:10 ` Sam Protsenko
  2018-04-27 19:23   ` Alex Kiernan
@ 2018-04-30  8:56   ` Alex Deymo
  1 sibling, 0 replies; 31+ messages in thread
From: Alex Deymo @ 2018-04-30  8:56 UTC (permalink / raw)
  To: u-boot

>
>
> On 24 April 2018 at 12:37, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> >
> > This series merges the fastboot UDP support from AOSP into mainline
> > U-Boot.
> >
>
> Can you please point me out from which AOSP code exactly this code was
> ported? Links to code, maybe some docs. Thanks.
>

The documentation for fastboot over UDP (as well as the rest of the
transport layers) is in the userspace fastboot tool, you can read the
README.md in AOSP:
https://android.googlesource.com/platform/system/core/+/master/fastboot

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support
  2018-04-30  8:37             ` Alex Kiernan
@ 2018-05-01  6:16               ` Jocelyn Bohr
  0 siblings, 0 replies; 31+ messages in thread
From: Jocelyn Bohr @ 2018-05-01  6:16 UTC (permalink / raw)
  To: u-boot

Great, I will check out v2! And yes free to add a "Signed-off-by" for me on
any of the original patchsets.

On Mon, Apr 30, 2018 at 1:37 AM Alex Kiernan <alex.kiernan@gmail.com> wrote:

> On Fri, Apr 27, 2018 at 1:20 PM, Alex Kiernan <alex.kiernan@gmail.com>
> wrote:
> > On Fri, Apr 27, 2018 at 9:04 AM, Lukasz Majewski <lukma@denx.de> wrote:
> >> Hi Kever,
> >>
> >>> Hi Jocelyn and Alex,
> >>>
> >>>
> >>>     It's great to see you are getting this feature from google code
> >>> to be mainline code.
> >>>
> >>>     My opinion is that we should have the same fastboot cmd handling
> >>> in both UDP and USB in a common file, but not have separate code in
> >>> net/fastboot.c and drivers/usb/gadget/f_fastboot.c, could you try to
> >>> merge these two in one?
> >>
> >> I've already proposed to put common code (the protocol handling)
> >> into /drivers/fastboot and have only small glue code for USB and NET.
> >>
> >
> > I'm working towards that. I'm starting with the command
> > implementations, then I'll look at the protocol.
> >
> > Hopefully I'll get an RFCv2 out sometime over the weekend with the
> > first bits done.
> >
>
> Just posted a v2, still needs more work, but getting feedback on the
> direction would be useful.
>
> --
> Alex Kiernan
>

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2018-05-01  6:16 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-24  9:37 [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Kiernan
2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 1/5] dfu: Refactor fastboot_okay/fail to take response Alex Kiernan
2018-04-25  7:27   ` Lukasz Majewski
2018-04-25  8:10     ` Alex Kiernan
2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 2/5] dfu: Extract fastboot_okay/fail to fb_common.c Alex Kiernan
2018-04-25  7:32   ` Lukasz Majewski
2018-04-25  8:15     ` Alex Kiernan
2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 3/5] net: dfu: Merge AOSP UDP fastboot Alex Kiernan
2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 4/5] dfu: Resolve Kconfig dependency loops Alex Kiernan
2018-04-25  7:53   ` Lukasz Majewski
2018-04-25  8:55     ` Alex Deymo
2018-04-24  9:37 ` [U-Boot] [RFC PATCH v1 5/5] net: dfu: Support building without MMC Alex Kiernan
2018-04-24 10:24 ` [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support Alex Deymo
2018-04-24 11:58   ` Alex Kiernan
2018-04-24 17:10     ` Jocelyn Bohr
2018-04-25  7:53       ` Alex Kiernan
2018-04-25 12:50         ` Alex Kiernan
2018-04-25 15:19           ` Lukasz Majewski
2018-04-26  3:27       ` Kever Yang
2018-04-27  8:04         ` Lukasz Majewski
2018-04-27 12:20           ` Alex Kiernan
2018-04-30  8:37             ` Alex Kiernan
2018-05-01  6:16               ` Jocelyn Bohr
2018-04-25  7:40     ` Lukasz Majewski
2018-04-25  7:52 ` Lukasz Majewski
2018-04-25  7:57   ` Lukasz Majewski
2018-04-25  8:43   ` Alex Kiernan
2018-04-25  8:46     ` Alex Deymo
2018-04-27 19:10 ` Sam Protsenko
2018-04-27 19:23   ` Alex Kiernan
2018-04-30  8:56   ` Alex Deymo

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.