sahara: Drop "single image" concept

The Sahara implementation was written without understanding of the
special meaning of image id #13. As the implementation grew support for
multiple images the special casing of "single image" was introduced, and
this spread to the calling bodies.

Prior to the introduction of multi-programmer platforms this didn't
matter, the logic was fairly simple and usage was straight forward.

But when a single programmer image is provided on a multi-programmer
target "single_image" is true and hence the image id is ignored on the
first read, the one provided file is loaded. The typical outcome is that
the following SAHARA_END_OF_IMAGE_CMD fails with a message stating
"non-successful end-of-image result".

Few users draws the conclusion that this is because they didn't provide
the appropriate programmers.

But 13 is the image id for the programmer, so it should be fine to drop
the special logic. This results in a (somewhat) more helpful error
message telling the user that an invalid image is being requested.

Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>

Author: quic-bjorande

ks.c

@@ -134,7 +134,7 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
-	ret = sahara_run(&qdl, mappings, false, NULL, NULL);
+	ret = sahara_run(&qdl, mappings, NULL, NULL);
 	if (ret < 0)
 		return 1;
 

qdl.c

@@ -149,6 +149,7 @@ static uint32_t parse_ascii_hex32(const char *s)
 
 /**
  * decode_programmer_archive() - Attempt to decode a programmer CPIO archive
+ * @blob: Loaded image to be decoded as archive
  * @images: List of Sahara images, with @images[0] populated
  *
  * The single blob provided in @images[0] might be a CPIO archive containing
@@ -159,9 +160,8 @@ static uint32_t parse_ascii_hex32(const char *s)
  *
  * Returns: 0 if no archive was found, 1 if archive was decoded, -1 on error
  */
-static int decode_programmer_archive(struct sahara_image *images)
+static int decode_programmer_archive(struct sahara_image *blob, struct sahara_image *images)
 {
-	struct sahara_image *blob = &images[0];
 	struct cpio_newc_header *hdr;
 	size_t filesize;
 	size_t namesize;
@@ -235,6 +235,7 @@ static int decode_programmer_archive(struct sahara_image *images)
 
 /**
  * decode_sahara_config() - Attempt to decode a Sahara config XML document
+ * @blob: Loaded image to be decoded as Sahara config
  * @images: List of Sahara images, with @images[0] populated
  *
  * The single blob provided in @images[0] might be a XML blob containing
@@ -246,9 +247,8 @@ static int decode_programmer_archive(struct sahara_image *images)
  *
  * Returns: 0 if no archive was found, 1 if archive was decoded, -1 on error
  */
-static int decode_sahara_config(struct sahara_image *images)
+static int decode_sahara_config(struct sahara_image *blob, struct sahara_image *images)
 {
-	struct sahara_image *blob = &images[0];
 	char image_path_full[PATH_MAX];
 	const char *image_path;
 	unsigned int image_id;
@@ -344,24 +344,25 @@ static int decode_sahara_config(struct sahara_image *images)
  * decode_programmer() - decodes the programmer specifier
  * @s: programmer specifier, from the user
  * @images: array of images to populate
- * @single: legacy single image specifier, for which image id should be ignored
  *
  * This parses the progammer specifier @s, which can either be a single
  * filename, or a comma-separated series of <id>:<filename> entries.
  *
- * In the first case @images[0] is assigned the provided filename and @single is
- * set to true. In the second case, each comma-separated entry will be split on
- * ':' and the given <filename> will be assigned to the @image entry indicated
- * by the given <id>.
+ * In the first case an attempt will be made to decode the Sahara archive and
+ * each programmer part will be loaded into their requestd @images entry. If
+ * the file isn't an archive @images[SAHARA_ID_EHOSTDL_IMG] is assigned. In the
+ * second case, each comma-separated entry will be split on ':' and the given
+ * <filename> will be assigned to the @image entry indicated by the given <id>.
  *
  * Memory is not allocated for the various strings, instead @s will be modified
  * by the tokenizer and pointers to the individual parts will be stored in the
  * @images array.
  *
  * Returns: 0 on success, -1 otherwise.
  */
-static int decode_programmer(char *s, struct sahara_image *images, bool *single)
+static int decode_programmer(char *s, struct sahara_image *images)
 {
+	struct sahara_image archive;
 	char *filename;
 	char *save1;
 	char *pair;
@@ -387,25 +388,21 @@ static int decode_programmer(char *s, struct sahara_image *images, bool *single)
 			ret = load_sahara_image(filename, &images[id]);
 			if (ret < 0)
 				return -1;
-
-			*single = false;
 		}
 	} else {
-		ret = load_sahara_image(s, &images[0]);
+		ret = load_sahara_image(s, &archive);
 		if (ret < 0)
 			return -1;
 
-		ret = decode_programmer_archive(images);
-		if (ret < 0)
-			return -1;
+		ret = decode_programmer_archive(&archive, images);
+		if (ret < 0 || ret == 1)
+			return ret;
 
-		if (!ret) {
-			ret = decode_sahara_config(images);
-			if (ret < 0)
-				return -1;
-		}
+		ret = decode_sahara_config(&archive, images);
+		if (ret < 0 || ret == 1)
+			return -1;
 
-		*single = (ret == 0);
+		images[SAHARA_ID_EHOSTDL_IMG] = archive;
 	}
 
 	return 0;
@@ -468,7 +465,6 @@ int main(int argc, char **argv)
 {
 	enum qdl_storage_type storage_type = QDL_STORAGE_UFS;
 	struct sahara_image sahara_images[MAPPING_SZ] = {};
-	bool single_image = true;
 	char *incdir = NULL;
 	char *serial = NULL;
 	const char *vip_generate_dir = NULL;
@@ -593,7 +589,7 @@ int main(int argc, char **argv)
 	if (qdl_debug)
 		print_version();
 
-	ret = decode_programmer(argv[optind++], sahara_images, &single_image);
+	ret = decode_programmer(argv[optind++], sahara_images);
 	if (ret < 0)
 		exit(1);
 
@@ -658,7 +654,7 @@ int main(int argc, char **argv)
 
 	qdl->storage_type = storage_type;
 
-	ret = sahara_run(qdl, sahara_images, single_image, NULL, NULL);
+	ret = sahara_run(qdl, sahara_images, NULL, NULL);
 	if (ret < 0)
 		goto out_cleanup;
 

qdl.h

@@ -36,6 +36,8 @@
 
 #define MAPPING_SZ 64
 
+#define SAHARA_ID_EHOSTDL_IMG	13
+
 enum QDL_DEVICE_TYPE {
 	QDL_DEVICE_USB,
 	QDL_DEVICE_SIM,
@@ -101,7 +103,7 @@ int firehose_run(struct qdl_device *qdl);
 int firehose_provision(struct qdl_device *qdl);
 int firehose_read_buf(struct qdl_device *qdl, struct read_op *read_op, void *out_buf, size_t out_size);
 int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
-	       bool single_image, const char *ramdump_path,
+	       const char *ramdump_path,
 	       const char *ramdump_filter);
 int load_sahara_image(const char *filename, struct sahara_image *image);
 void print_hex_dump(const char *prefix, const void *buf, size_t len);

ramdump.c

@@ -85,7 +85,7 @@ int main(int argc, char **argv)
 		goto out_cleanup;
 	}
 
-	ret = sahara_run(qdl, NULL, true, ramdump_path, filter);
+	ret = sahara_run(qdl, NULL, ramdump_path, filter);
 	if (ret < 0) {
 		ret = 1;
 		goto out_cleanup;

sahara.c

@@ -145,8 +145,7 @@ static void sahara_hello(struct qdl_device *qdl, struct sahara_pkt *pkt)
 }
 
 static void sahara_read(struct qdl_device *qdl, struct sahara_pkt *pkt,
-			const struct sahara_image *images,
-			bool single_image)
+			const struct sahara_image *images)
 {
 	const struct sahara_image *image;
 	unsigned int image_idx;
@@ -159,11 +158,7 @@ static void sahara_read(struct qdl_device *qdl, struct sahara_pkt *pkt,
 	ux_debug("READ image: %d offset: 0x%x length: 0x%x\n",
 		 pkt->read_req.image, pkt->read_req.offset, pkt->read_req.length);
 
-	if (single_image)
-		image_idx = 0;
-	else
-		image_idx = pkt->read_req.image;
-
+	image_idx = pkt->read_req.image;
 	if (image_idx >= MAPPING_SZ || !images[image_idx].ptr) {
 		ux_err("device requested invalid image: %u\n", image_idx);
 		sahara_send_reset(qdl);
@@ -185,8 +180,7 @@ static void sahara_read(struct qdl_device *qdl, struct sahara_pkt *pkt,
 }
 
 static void sahara_read64(struct qdl_device *qdl, struct sahara_pkt *pkt,
-			  const struct sahara_image *images,
-			  bool single_image)
+			  const struct sahara_image *images)
 {
 	const struct sahara_image *image;
 	unsigned int image_idx;
@@ -199,11 +193,7 @@ static void sahara_read64(struct qdl_device *qdl, struct sahara_pkt *pkt,
 	ux_debug("READ64 image: %" PRId64 " offset: 0x%" PRIx64 " length: 0x%" PRIx64 "\n",
 		 pkt->read64_req.image, pkt->read64_req.offset, pkt->read64_req.length);
 
-	if (single_image)
-		image_idx = 0;
-	else
-		image_idx = pkt->read64_req.image;
-
+	image_idx = pkt->read64_req.image;
 	if (image_idx >= MAPPING_SZ || !images[image_idx].ptr) {
 		ux_err("device requested invalid image: %u\n", image_idx);
 		sahara_send_reset(qdl);
@@ -414,12 +404,26 @@ static void sahara_debug64(struct qdl_device *qdl, struct sahara_pkt *pkt,
 	sahara_send_reset(qdl);
 }
 
-static void sahara_debug_list_images(const struct sahara_image *images, bool single_image)
+static bool sahara_has_done_pending_quirk(const struct sahara_image *images)
 {
+	unsigned int count = 0;
 	int i;
 
-	if (single_image)
-		ux_debug("Sahara image id in read requests will be ignored\n");
+	/*
+	 * E.g MSM8916 EDL reports done = pending, allow this when one a single
+	 * image is provided, and it's used as SAHARA_ID_EHOSTDL_IMG.
+	 */
+	for (i = 0; i < MAPPING_SZ; i++) {
+		if (images[i].ptr)
+			count++;
+	}
+
+	return count == 1 && images[SAHARA_ID_EHOSTDL_IMG].ptr;
+}
+
+static void sahara_debug_list_images(const struct sahara_image *images)
+{
+	int i;
 
 	ux_debug("Sahara images:\n");
 	for (i = 0; i < MAPPING_SZ; i++) {
@@ -429,7 +433,7 @@ static void sahara_debug_list_images(const struct sahara_image *images, bool sin
 }
 
 int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
-	       bool single_image, const char *ramdump_path,
+	       const char *ramdump_path,
 	       const char *ramdump_filter)
 {
 	struct sahara_pkt *pkt;
@@ -439,7 +443,7 @@ int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
 	int n;
 
 	if (images)
-		sahara_debug_list_images(images, single_image);
+		sahara_debug_list_images(images);
 
 	/*
 	 * Don't need to do anything in simulation mode with Sahara,
@@ -466,7 +470,7 @@ int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
 			sahara_hello(qdl, pkt);
 			break;
 		case SAHARA_READ_DATA_CMD:
-			sahara_read(qdl, pkt, images, single_image);
+			sahara_read(qdl, pkt, images);
 			break;
 		case SAHARA_END_OF_IMAGE_CMD:
 			sahara_eoi(qdl, pkt);
@@ -475,14 +479,14 @@ int sahara_run(struct qdl_device *qdl, const struct sahara_image *images,
 			done = sahara_done(qdl, pkt);
 
 			/* E.g MSM8916 EDL reports done = 0 here */
-			if (single_image)
+			if (sahara_has_done_pending_quirk(images))
 				done = true;
 			break;
 		case SAHARA_MEM_DEBUG64_CMD:
 			sahara_debug64(qdl, pkt, ramdump_path, ramdump_filter);
 			break;
 		case SAHARA_READ_DATA64_CMD:
-			sahara_read64(qdl, pkt, images, single_image);
+			sahara_read64(qdl, pkt, images);
 			break;
 		case SAHARA_RESET_RESP_CMD:
 			assert(pkt->length == SAHARA_RESET_LENGTH);