[PATCH] Rename at76_get_fw_info() to at76_parse_fw(), fully rewrite

Use a structure to describe the firmware header.  Don't pass any
arguments fpr output, just pass the pointer to the private data.  Move
some debugging output there to avoid code duplication.

Parse firmware even if it doesn't need to be loaded.  Keep firmware
version information in the private data.  It's still re-read from the
card, but now we have a valid version earlier.

Signed-off-by: Pavel Roskin <proski@gnu.org>

2 files changed, 55 insertions, 107 deletions

diff --git a/at76_usb.c b/at76_usb.c
index 649b1ce..59d9129 100644
--- a/at76_usb.c
+++ b/at76_usb.c
@@ -6028,58 +6028,43 @@ static int at76_init_new_device(struct at76_priv *dev)
 }
 
 
-/**
- * at76_get_fw_info - disassembles the firmware image
- *
- * get version, str, internal and external fw part.
- * returns 0 on success, < 0 on error
- */
-static int at76_get_fw_info(u8 *fw_data, int fw_size,
-				u32 *board, u32 *version, char **str,
-				u8 **int_fw, int *int_fw_size,
-				u8 **ext_fw, int *ext_fw_size)
-{
-/* fw structure (all numbers are little_endian)
-   offset  length  description
-   0       4       crc 32 (seed ~0, no post, all gaps are zeros, header included)
-   4       4       board type (see at76_usb_ids.h)
-   8       4       version (major<<24|middle<<16|minor<<8|build)
-   c       4       offset of printable string (id) area from begin of image
-                   (must be \0 terminated !)
-  10       4       offset of internal fw part area
-  14       4       length of internal fw part
-  18       4       offset of external fw part area (may be first byte _behind_
-                   image in case we have no external part)
-  1c       4       length of external fw part
-*/
-
-	__le32 val;
+/* Parse the firmware image */
+static int at76_parse_fw(struct at76_priv *dev, u8 *fw_data, int fw_size,
+			 int board_type)
+{
+	char *str;
+	struct at76_fw_header *fw = (struct at76_fw_header *)fw_data;
 
-	if (fw_size < 0x21) {
-		err("fw too short (x%x)", fw_size);
+	if (fw_size <= sizeof(*fw)) {
+		err("firmware is too short (0x%x)", fw_size);
 		return -EFAULT;
 	}
 
-	/* crc currently not checked */
-
-	memcpy(&val, fw_data + 4, 4);
-	*board = le32_to_cpu(val);
-
-	memcpy(&val, fw_data + 8, 4);
-	*version = le32_to_cpu(val);
-
-	memcpy(&val, fw_data + 0xc, 4);
-	*str = fw_data + le32_to_cpu(val);
-
-	memcpy(&val, fw_data + 0x10, 4);
-	*int_fw = fw_data + le32_to_cpu(val);
-	memcpy(&val, fw_data + 0x14, 4);
-	*int_fw_size = le32_to_cpu(val);
+	/* CRC currently not checked */
+	dev->board_type = le32_to_cpu(fw->board_type);
+	dev->fw_version.major = fw->major;
+	dev->fw_version.minor = fw->minor;
+	dev->fw_version.patch = fw->patch;
+	dev->fw_version.build = fw->build;
+	str = (char *)fw_data + le32_to_cpu(fw->str_offset);
+	dev->intfw = (u8 *)fw + le32_to_cpu(fw->int_fw_offset);
+	dev->intfw_size = le32_to_cpu(fw->int_fw_len);
+	dev->extfw = (u8 *)fw + le32_to_cpu(fw->ext_fw_offset);
+	dev->extfw_size = le32_to_cpu(fw->ext_fw_len);
+
+	at76_dbg(DBG_DEVSTART, "firmware board %u version %u.%u.%u#%u "
+		 "(int %x:%tx, ext %x:%tx)", dev->board_type,
+		 dev->fw_version.major, dev->fw_version.minor,
+		 dev->fw_version.patch, dev->fw_version.build,
+		 dev->intfw_size, dev->intfw - fw_data,
+		 dev->extfw_size, dev->extfw - fw_data);
+	at76_dbg(DBG_DEVSTART, "firmware id %s", str);
 
-	memcpy(&val, fw_data + 0x18, 4);
-	*ext_fw = fw_data + le32_to_cpu(val);
-	memcpy(&val, fw_data + 0x1c, 4);
-	*ext_fw_size = le32_to_cpu(val);
+	if (dev->board_type != board_type) {
+		err("inconsistent board types %u != %u", board_type,
+		    dev->board_type);
+		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -6094,8 +6079,6 @@ static int at76_probe(struct usb_interface *interface,
 	const struct firmware *fw = firmwares[board_type].fw;
 	struct usb_device *udev = interface_to_usbdev(interface);
 	int op_mode;
-	char *id_str;
-	u32 version;
 
 	if (fw == NULL) {
 		at76_dbg(DBG_FW, "downloading firmware %s", fw_name);
@@ -6115,7 +6098,7 @@ static int at76_probe(struct usb_interface *interface,
 
 	if ((dev = at76_alloc_new_device(udev, (u8) board_type)) == NULL) {
 		ret = -ENOMEM;
-		goto error;
+		goto error_alloc;
 	}
 
 	op_mode = at76_get_op_mode(udev);
@@ -6135,41 +6118,17 @@ static int at76_probe(struct usb_interface *interface,
 		goto error;
 	}
 
+	/* parse the firmware */
+	ret = at76_parse_fw(dev, fw->data, fw->size, board_type);
+	if (ret)
+		goto error;
+
 	if (op_mode != OPMODE_NORMAL_NIC_WITH_FLASH
 	    && op_mode != OPMODE_NORMAL_NIC_WITHOUT_FLASH) {
-
-		at76_dbg(DBG_DEVSTART, "need to download firmware");
-
-		/* parse the firmware */
-		ret = at76_get_fw_info(fw->data, fw->size,
-				       &dev->board_type, &version, &id_str,
-				       &dev->intfw, &dev->intfw_size,
-				       &dev->extfw, &dev->extfw_size);
-		if (ret)
-			goto error;
-
-		at76_dbg(DBG_DEVSTART,
-			 "firmware board %u version %u.%u.%u#%u "
-			 "(int %x:%tx, ext %x:%tx)", dev->board_type,
-			 version >> 24, (version >> 16) & 0xff,
-			 (version >> 8) & 0xff, version & 0xff, dev->intfw_size,
-			 dev->intfw - fw->data, dev->extfw_size,
-			 dev->extfw - fw->data);
-		if (*id_str)
-			at76_dbg(DBG_DEVSTART, "firmware id %s", id_str);
-
-		if (dev->board_type != board_type) {
-			err("inconsistent board types %u != %u", board_type,
-			    dev->board_type);
-			at76_delete_device(dev);
-			goto error;
-		}
-
 		/* download internal firmware part */
 		at76_dbg(DBG_DEVSTART, "downloading internal firmware");
 		dev->istate = INTFW_DOWNLOAD;
 		schedule_work(&dev->work_internal_fw);
-
 	} else {
 		/* Internal firmware already inside the device.  Get firmware
 		 * version to test if external firmware is loaded.
@@ -6178,17 +6137,9 @@ static int at76_probe(struct usb_interface *interface,
 		 * at76_get_op_mode() fail too :-( */
 		int force_fw_dwl = 0;
 
-		/* parse the firmware */
-		ret = at76_get_fw_info(fw->data, fw->size,
-				       &dev->board_type, &version, &id_str,
-				       &dev->intfw, &dev->intfw_size,
-				       &dev->extfw, &dev->extfw_size);
-		if (ret)
-			goto error;
-
 		/* if version >= 0.100.x.y or device with built-in flash we can
 		 * query the device for the fw version */
-		if (version >= ((0 << 24) | (100 << 16))
+		if ((dev->fw_version.major > 0 || dev->fw_version.minor >= 100)
 		    || (op_mode == OPMODE_NORMAL_NIC_WITH_FLASH)) {
 			ret = at76_get_mib(udev, MIB_FW_VERSION,
 					   &dev->fw_version,
@@ -6210,36 +6161,20 @@ static int at76_probe(struct usb_interface *interface,
 				at76_dbg(DBG_DEVSTART,
 					 "cannot get firmware (ret %d) or all zeros "
 					 "- download external firmware", ret);
-			at76_dbg(DBG_DEVSTART,
-				 "firmware board %u version %u.%u.%u#%u "
-				 "(int %x:%tx, ext %x:%tx)", dev->board_type,
-				 version >> 24, (version >> 16) & 0xff,
-				 (version >> 8) & 0xff, version & 0xff,
-				 dev->intfw_size, dev->intfw - fw->data,
-				 dev->extfw_size, dev->extfw - fw->data);
-			if (*id_str)
-				at76_dbg(DBG_DEVSTART, "firmware id %s",
-					 id_str);
-
-			if (dev->board_type != board_type) {
-				err("inconsistent board types %u != %u",
-				    board_type, dev->board_type);
-				at76_delete_device(dev);
-				goto error;
-			}
 
 			dev->istate = EXTFW_DOWNLOAD;
 			schedule_work(&dev->work_external_fw);
 		} else {
 			dev->istate = INIT;
-			if ((ret = at76_init_new_device(dev)) < 0) {
+			if ((ret = at76_init_new_device(dev)) < 0)
 				goto error;
-			}
 		}
 	}
 	return 0;
 
       error:
+	at76_delete_device(dev);
+      error_alloc:
 	usb_put_dev(udev);
 	return ret;
 }
diff --git a/at76_usb.h b/at76_usb.h
index a74b3c5..97f46d6 100644
--- a/at76_usb.h
+++ b/at76_usb.h
@@ -364,6 +364,19 @@ struct mib_mdomain {
 	u8 channel_list[14];	/* 0 for invalid channels */
 } __attribute__ ((packed));
 
+struct at76_fw_header {
+	__le32 crc;			/* CRC32 of the whole image */
+	__le32 board_type;		/* firmware compatibility code */
+	u8 build;			/* firmware build number */
+	u8 patch;			/* firmware patch level */
+	u8 minor;			/* firmware minor version */
+	u8 major;			/* firmware major version */
+	__le32 str_offset;		/* offset of the copyright string */
+	__le32 int_fw_offset;		/* internal firmware image offset */
+	__le32 int_fw_len;		/* internal firmware image length */
+	__le32 ext_fw_offset;		/* external firmware image offset */
+	__le32 ext_fw_len;		/* external firmware image length */
+} __attribute__ ((packed));
 
 /* states in infrastructure mode */
 enum infra_state {