USB: io_ti: Fix firmware version handling

The io_ti driver fails to download firmware to Edgeport
devices such as the EP/416, even when the on-disk firmware image
(/lib/firmware/edgeport/down3.bin) is more current than the version
on the EP/416.  The current download code is broken in a few ways.
Notably it mis-uses global variables OperationalMajorVersion and
OperationalMinorVersion (reading their values before they've been
properly initialized and subsequently initializing them multiple times
without synchronization).  This patch drops the global variables and
replaces the redundant calls to request_firmware()/release_firmware()
in download_fw() with a single call pair in edge_startup(); the firmware
image pointer is then passed to download_fw() and build_i2c_fw_hdr().

Signed-off-by: Peter E. Berger <pberger@brimson.com>
Signed-off-by: Johan Hovold <johan@kernel.org>

1 files changed, 65 insertions, 49 deletions

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 69378a7..b562665 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -71,6 +71,25 @@ struct product_info {
 	__u8	hardware_type;		/* Type of hardware */
 } __attribute__((packed));
 
+/*
+ * Edgeport firmware header
+ *
+ * "build_number" has been set to 0 in all three of the images I have
+ * seen, and Digi Tech Support suggests that it is safe to ignore it.
+ *
+ * "length" is the number of bytes of actual data following the header.
+ *
+ * "checksum" is the low order byte resulting from adding the values of
+ * all the data bytes.
+ */
+struct edgeport_fw_hdr {
+	u8 major_version;
+	u8 minor_version;
+	__le16 build_number;
+	__le16 length;
+	u8 checksum;
+} __packed;
+
 struct edgeport_port {
 	__u16 uart_base;
 	__u16 dma_address;
@@ -101,6 +120,7 @@ struct edgeport_serial {
 	struct mutex es_lock;
 	int num_ports_open;
 	struct usb_serial *serial;
+	int fw_version;
 };
 
 
@@ -187,10 +207,6 @@ static const struct usb_device_id id_table_combined[] = {
 
 MODULE_DEVICE_TABLE(usb, id_table_combined);
 
-static unsigned char OperationalMajorVersion;
-static unsigned char OperationalMinorVersion;
-static unsigned short OperationalBuildNumber;
-
 static int closing_wait = EDGE_CLOSING_WAIT;
 static bool ignore_cpu_rev;
 static int default_uart_mode;		/* RS232 */
@@ -751,18 +767,17 @@ exit:
 }
 
 /* Build firmware header used for firmware update */
-static int build_i2c_fw_hdr(__u8 *header, struct device *dev)
+static int build_i2c_fw_hdr(u8 *header, struct device *dev,
+		const struct firmware *fw)
 {
 	__u8 *buffer;
 	int buffer_size;
 	int i;
-	int err;
 	__u8 cs = 0;
 	struct ti_i2c_desc *i2c_header;
 	struct ti_i2c_image_header *img_header;
 	struct ti_i2c_firmware_rec *firmware_rec;
-	const struct firmware *fw;
-	const char *fw_name = "edgeport/down3.bin";
+	struct edgeport_fw_hdr *fw_hdr = (struct edgeport_fw_hdr *)fw->data;
 
 	/* In order to update the I2C firmware we must change the type 2 record
 	 * to type 0xF2.  This will force the UMP to come up in Boot Mode.
@@ -785,24 +800,11 @@ static int build_i2c_fw_hdr(__u8 *header, struct device *dev)
 	// Set entire image of 0xffs
 	memset(buffer, 0xff, buffer_size);
 
-	err = request_firmware(&fw, fw_name, dev);
-	if (err) {
-		dev_err(dev, "Failed to load image \"%s\" err %d\n",
-			fw_name, err);
-		kfree(buffer);
-		return err;
-	}
-
-	/* Save Download Version Number */
-	OperationalMajorVersion = fw->data[0];
-	OperationalMinorVersion = fw->data[1];
-	OperationalBuildNumber = fw->data[2] | (fw->data[3] << 8);
-
 	/* Copy version number into firmware record */
 	firmware_rec = (struct ti_i2c_firmware_rec *)buffer;
 
-	firmware_rec->Ver_Major	= OperationalMajorVersion;
-	firmware_rec->Ver_Minor	= OperationalMinorVersion;
+	firmware_rec->Ver_Major	= fw_hdr->major_version;
+	firmware_rec->Ver_Minor	= fw_hdr->minor_version;
 
 	/* Pointer to fw_down memory image */
 	img_header = (struct ti_i2c_image_header *)&fw->data[4];
@@ -811,8 +813,6 @@ static int build_i2c_fw_hdr(__u8 *header, struct device *dev)
 		&fw->data[4 + sizeof(struct ti_i2c_image_header)],
 		le16_to_cpu(img_header->Length));
 
-	release_firmware(fw);
-
 	for (i=0; i < buffer_size; i++) {
 		cs = (__u8)(cs + buffer[i]);
 	}
@@ -826,8 +826,8 @@ static int build_i2c_fw_hdr(__u8 *header, struct device *dev)
 	i2c_header->Type	= I2C_DESC_TYPE_FIRMWARE_BLANK;
 	i2c_header->Size	= cpu_to_le16(buffer_size);
 	i2c_header->CheckSum	= cs;
-	firmware_rec->Ver_Major	= OperationalMajorVersion;
-	firmware_rec->Ver_Minor	= OperationalMinorVersion;
+	firmware_rec->Ver_Major	= fw_hdr->major_version;
+	firmware_rec->Ver_Minor	= fw_hdr->minor_version;
 
 	return 0;
 }
@@ -934,7 +934,8 @@ static int ti_cpu_rev(struct edge_ti_manuf_descriptor *desc)
  * This routine downloads the main operating code into the TI5052, using the
  * boot code already burned into E2PROM or ROM.
  */
-static int download_fw(struct edgeport_serial *serial)
+static int download_fw(struct edgeport_serial *serial,
+		const struct firmware *fw)
 {
 	struct device *dev = &serial->serial->dev->dev;
 	int status = 0;
@@ -943,6 +944,17 @@ static int download_fw(struct edgeport_serial *serial)
 	struct usb_interface_descriptor *interface;
 	int download_cur_ver;
 	int download_new_ver;
+	struct edgeport_fw_hdr *fw_hdr = (struct edgeport_fw_hdr *)fw->data;
+
+	if (fw->size < sizeof(struct edgeport_fw_hdr)) {
+		dev_err(&serial->serial->interface->dev,
+				"Incomplete firmware header.\n");
+		return -EINVAL;
+	}
+
+	/* If on-board version is newer, "fw_version" will be updated below. */
+	serial->fw_version = (fw_hdr->major_version << 8) +
+			fw_hdr->minor_version;
 
 	/* This routine is entered by both the BOOT mode and the Download mode
 	 * We can determine which code is running by the reading the config
@@ -1050,14 +1062,13 @@ static int download_fw(struct edgeport_serial *serial)
 			   version in I2c */
 			download_cur_ver = (firmware_version->Ver_Major << 8) +
 					   (firmware_version->Ver_Minor);
-			download_new_ver = (OperationalMajorVersion << 8) +
-					   (OperationalMinorVersion);
+			download_new_ver = (fw_hdr->major_version << 8) +
+					   (fw_hdr->minor_version);
 
 			dev_dbg(dev, "%s - >> FW Versions Device %d.%d  Driver %d.%d\n",
 				__func__, firmware_version->Ver_Major,
 				firmware_version->Ver_Minor,
-				OperationalMajorVersion,
-				OperationalMinorVersion);
+				fw_hdr->major_version, fw_hdr->minor_version);
 
 			/* Check if we have an old version in the I2C and
 			   update if necessary */
@@ -1066,8 +1077,8 @@ static int download_fw(struct edgeport_serial *serial)
 					__func__,
 					firmware_version->Ver_Major,
 					firmware_version->Ver_Minor,
-					OperationalMajorVersion,
-					OperationalMinorVersion);
+					fw_hdr->major_version,
+					fw_hdr->minor_version);
 
 				record = kmalloc(1, GFP_KERNEL);
 				if (!record) {
@@ -1143,6 +1154,9 @@ static int download_fw(struct edgeport_serial *serial)
 				kfree(rom_desc);
 				kfree(ti_manuf_desc);
 				return -ENODEV;
+			} else {
+				/* Same or newer fw version is already loaded */
+				serial->fw_version = download_cur_ver;
 			}
 			kfree(firmware_version);
 		}
@@ -1181,7 +1195,7 @@ static int download_fw(struct edgeport_serial *serial)
 			 * UMP Ram to I2C and the firmware will update the
 			 * record type from 0xf2 to 0x02.
 			 */
-			status = build_i2c_fw_hdr(header, dev);
+			status = build_i2c_fw_hdr(header, dev, fw);
 			if (status) {
 				kfree(vheader);
 				kfree(header);
@@ -1284,9 +1298,6 @@ static int download_fw(struct edgeport_serial *serial)
 		__u8 cs = 0;
 		__u8 *buffer;
 		int buffer_size;
-		int err;
-		const struct firmware *fw;
-		const char *fw_name = "edgeport/down3.bin";
 
 		/* Validate Hardware version number
 		 * Read Manufacturing Descriptor from TI Based Edgeport
@@ -1334,16 +1345,7 @@ static int download_fw(struct edgeport_serial *serial)
 
 		/* Initialize the buffer to 0xff (pad the buffer) */
 		memset(buffer, 0xff, buffer_size);
-
-		err = request_firmware(&fw, fw_name, dev);
-		if (err) {
-			dev_err(dev, "Failed to load image \"%s\" err %d\n",
-				fw_name, err);
-			kfree(buffer);
-			return err;
-		}
 		memcpy(buffer, &fw->data[4], fw->size - 4);
-		release_firmware(fw);
 
 		for (i = sizeof(struct ti_i2c_image_header);
 				i < buffer_size; i++) {
@@ -1358,7 +1360,9 @@ static int download_fw(struct edgeport_serial *serial)
 		header->CheckSum = cs;
 
 		/* Download the operational code  */
-		dev_dbg(dev, "%s - Downloading operational code image (TI UMP)\n", __func__);
+		dev_dbg(dev, "%s - Downloading operational code image version %d.%d (TI UMP)\n",
+				__func__,
+				fw_hdr->major_version, fw_hdr->minor_version);
 		status = download_code(serial, buffer, buffer_size);
 
 		kfree(buffer);
@@ -2383,6 +2387,9 @@ static int edge_startup(struct usb_serial *serial)
 {
 	struct edgeport_serial *edge_serial;
 	int status;
+	const struct firmware *fw;
+	const char *fw_name = "edgeport/down3.bin";
+	struct device *dev = &serial->interface->dev;
 
 	/* create our private serial structure */
 	edge_serial = kzalloc(sizeof(struct edgeport_serial), GFP_KERNEL);
@@ -2393,7 +2400,16 @@ static int edge_startup(struct usb_serial *serial)
 	edge_serial->serial = serial;
 	usb_set_serial_data(serial, edge_serial);
 
-	status = download_fw(edge_serial);
+	status = request_firmware(&fw, fw_name, dev);
+	if (status) {
+		dev_err(dev, "Failed to load image \"%s\" err %d\n",
+				fw_name, status);
+		kfree(edge_serial);
+		return status;
+	}
+
+	status = download_fw(edge_serial, fw);
+	release_firmware(fw);
 	if (status) {
 		kfree(edge_serial);
 		return status;