Add fastboot UDP support

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 <[email protected]>

---

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

common/fb_mmc.c +4 -4

index cf5b77c..02864aa 100644
@@ -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

common/fb_mmc.c +2 -1

@@ -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");

common/fb_mmc.c +1 -1

-		fastboot_fail("invalid number of boot sectors: 0");
+		fastboot_fail("invalid number of boot sectors: 0",

response); return 0;

common/fb_mmc.c +2 -1

 	}

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

common/fb_mmc.c +1 -1

 	}

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

common/fb_mmc.c +2 -1

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

common/fb_mmc.c +1 -1

-		fastboot_fail("cannot find boot partition");
+		fastboot_fail("cannot find boot partition",

response); return -1;

common/fb_mmc.c +4 -3

 	}

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

common/fb_mmc.c +2 -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;

common/fb_mmc.c +5 -5

 	}

@@ -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");

common/fb_mmc.c +5 -4

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

common/fb_mmc.c +7 -5

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

common/fb_mmc.c +3 -3

 		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;

common/fb_mmc.c +5 -5

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

}

common/fb_nand.c +16 -15

index aa28046..3a0b101 100644
@@ -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);
 }

common/image-sparse.c +4 -4

index ddf5772..616c2bd 100644
@@ -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);

common/image-sparse.c +2 -2

 				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;

common/image-sparse.c +3 -4

 			}

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

common/image-sparse.c +2 -2

 				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;

common/image-sparse.c +2 -2

 			}

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

common/image-sparse.c +2 -2

 			}

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

common/image-sparse.c +2 -2

 					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;

common/image-sparse.c +2 -2

 			}
 			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

common/image-sparse.c +1 -1

-		fastboot_okay("");
+		fastboot_okay("", response);
 
 	return;
 }

(quoted diff) +0 -0

b/drivers/usb/gadget/f_fastboot.c index 7acffb6..6ae1d97 100644

drivers/usb/gadget/f_fastboot.c +6 -8

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

drivers/usb/gadget/f_fastboot.c +7 -14

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

include/fastboot.h +2 -2

index 616631e..f22080a 100644
@@ -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_ */

include/fb_mmc.h +2 -2

index 12b99cb..402ba9b 100644
@@ -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);

include/fb_nand.h +1 -2

index aaf7cf7..88bdf36 100644
@@ -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);

include/image-sparse.h +0 -0

index b0cc500..02453c8 100644

Hi Alex,

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

Maybe fastboot would be more appropriate?

Quoted 3 lines

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?

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 <[email protected]>

---

 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

common/Makefile +3 -0

index 7011dad..8177341 100644
@@ -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

common/fb_common.c new file +9 -0

new file mode 100644
index 0000000..53cffe3
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2008 - 2009
+ * Windriver, <www.windriver.com>
+ * Tom Rix <[email protected]>
+ *
+ * Copyright 2011 Sebastian Andrzej Siewior <[email protected]>
+ *
+ * Copyright 2014 Linaro, Ltd.

common/fb_common.c new file +7 -0

+}
+
+void fastboot_okay(const char *reason, char *response)
+{
+	strncpy(response, "OKAY\0", 5);
+	strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
+}

(quoted diff) +0 -0

b/drivers/usb/gadget/f_fastboot.c index 6ae1d97..fda4505 100644

drivers/usb/gadget/f_fastboot.c +0 -6

@@ -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);
-}

Hi Alex,

Quoted 48 lines

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 <[email protected]>

---

 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

common/Makefile +3 -0

index 7011dad..8177341 100644
@@ -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

common/fb_common.c new file +15 -0

new file mode 100644
index 0000000..53cffe3
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2008 - 2009
+ * Windriver, <www.windriver.com>
+ * Tom Rix <[email protected]>
+ *
+ * Copyright 2011 Sebastian Andrzej Siewior <[email protected]>
+ *
+ * Copyright 2014 Linaro, Ltd.
+ * Rob Herring <[email protected]>
+ */
+
+#include <common.h>
+#include <fastboot.h>
+

common/fb_common.c line 19

Quoted 4 lines

common/fb_common.c new file +4 -0

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

Fix recursive dependencies in Kconfig introduced by fastboot UDP

Signed-off-by Alex Kiernan <[email protected]>

---

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

cmd/fastboot/Kconfig +3 -2

index 0c57616..0f804ea 100644
@@ -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

Hi Alex,

Quoted 44 lines

Fix recursive dependencies in Kconfig introduced by fastboot UDP

Signed-off-by Alex Kiernan <[email protected]>

---

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

cmd/fastboot/Kconfig +3 -2

index 0c57616..0f804ea 100644
@@ -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

Quoted 1 line

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

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

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

+Jocelyn.

Thanks Alex for taking the time to port this!!

2018-04-24 11:37 GMT+02:00 Alex Kiernan <[email protected]>:

Quoted 5 lines

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

Quoted 8 lines

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.

Quoted 4 lines

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

Quoted 4 lines

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.

Quoted 3 lines · Alex Deymo

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

Quoted 16 lines

2018-04-24 11:37 GMT+02:00 Alex Kiernan <[email protected]>:

Quoted 5 lines

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

Quoted 8 lines

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

Quoted 1 line

Fastboot uses partition names, like "system" and "boot" which have a

meaning

Quoted 1 line

in the Android partition scheme. For GPT, we just use the partition

names as

Quoted 1 line

the android names; but if you are booting from some other storage like

NAND

Quoted 3 lines

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

Quoted 2 lines

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

Quoted 3 lines

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.

Quoted 7 lines

Quoted 4 lines

- 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

Quoted 1 line

remember the details now but yeah you can't wait for network or USB

traffic

Quoted 2 lines

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.

Quoted 4 lines

Quoted 4 lines

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

Quoted 6 lines

Quoted 1 line

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

Quoted 1 line

whether you have an old device or a newer device the fastboot client

checks

Quoted 2 lines

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

Quoted 2 lines

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

Quoted 1 line

level operations like flashall/update and you want to specify to flash

only

Quoted 1 line

"slot A", "slot B" or even both of them. There are other fastboot

variables

Quoted 1 line

that require some plumbing, such as "has-slot:<partition>" to tell

whether

Quoted 4 lines

"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

Quoted 2 lines

the system.img.

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

--
Alex Kiernan

Thanks so much for porting this, Alex!

Quoted 64 lines · Alex Kiernan

Quoted 3 lines · Alex Deymo

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

Quoted 18 lines

2018-04-24 11:37 GMT+02:00 Alex Kiernan <[email protected]>:

Quoted 7 lines

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

Open questions:

Quoted 8 lines

- 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

Quoted 17 lines

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.

Quoted 6 lines

Quoted 4 lines

- 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

Quoted 6 lines

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.

Quoted 3 lines

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

Quoted 2 lines · Jocelyn Bohr

Thanks so much for porting this, Alex!

Quoted 10 lines

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!

Quoted 4 lines · Alex Kiernan

Quoted 2 lines · Jocelyn Bohr

Thanks so much for porting this, Alex!

...

Quoted 29 lines

Quoted 10 lines

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

Quoted 7 lines

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!

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

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

Hi Kever,

Quoted 6 lines

Hi Jocelyn and Alex,

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

Quoted 4 lines

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.

Quoted 8 lines · Lukasz Majewski

Hi Kever,

Quoted 6 lines

Hi Jocelyn and Alex,

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

Quoted 8 lines

Quoted 4 lines

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.

Quoted 18 lines · Alex Kiernan

Quoted 8 lines · Lukasz Majewski

Hi Kever,

Quoted 6 lines

Hi Jocelyn and Alex,

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

Quoted 8 lines

Quoted 4 lines

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.

Quoted 6 lines

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

Quoted 3 lines · Alex Deymo

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

Quoted 16 lines

2018-04-24 11:37 GMT+02:00 Alex Kiernan <[email protected]>:

Quoted 5 lines

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

Quoted 8 lines

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

Quoted 11 lines

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.

Quoted 11 lines

Quoted 5 lines

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

Quoted 17 lines

Quoted 5 lines

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

Quoted 8 lines

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!

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

Quoted 2 lines

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

Hi Alex,

Quoted 2 lines

Open questions:

Quoted 3 lines

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

Quoted 8 lines

Quoted 3 lines

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

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

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

Quoted 4 lines · Alex Kiernan

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.

Quoted 4 lines

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.