[PATCH 09/15] staging: comedi: usbdux: consolidate the firmware upload

H Hartley Sweeten hsweeten at visionengravers.com
Mon May 20 21:29:39 UTC 2013


Absorb the usbdux_stop(), usbdux_upload(), and usbdux_start()
functions into firmware_upload().

Each of them just do a usb_control_msg() to the device and output
an error message if it fails. A similar message is also output by
firmware_upload() so the extra messages are redundant.

We can also share the malloc'ed local buffer needed for the
usb_control_msg().

Signed-off-by: H Hartley Sweeten <hsweeten at visionengravers.com>
Cc Ian Abbott <abbotti at mev.co.uk>
Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
---
 drivers/staging/comedi/drivers/usbdux.c | 166 +++++++++-----------------------
 1 file changed, 46 insertions(+), 120 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbdux.c b/drivers/staging/comedi/drivers/usbdux.c
index cb63c77..405a345 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -722,154 +722,80 @@ static void usbduxsub_ao_isoc_irq(struct urb *urb)
 	}
 }
 
-static int usbduxsub_start(struct usbduxsub *usbduxsub)
-{
-	int errcode = 0;
-	uint8_t *local_transfer_buffer;
-
-	local_transfer_buffer = kmalloc(1, GFP_KERNEL);
-	if (!local_transfer_buffer)
-		return -ENOMEM;
-
-	/* 7f92 to zero */
-	*local_transfer_buffer = 0;
-	errcode = usb_control_msg(usbduxsub->usbdev,
-				  /* create a pipe for a control transfer */
-				  usb_sndctrlpipe(usbduxsub->usbdev, 0),
-				  /* bRequest, "Firmware" */
-				  USBDUXSUB_FIRMWARE,
-				  /* bmRequestType */
-				  VENDOR_DIR_OUT,
-				  /* Value */
-				  USBDUXSUB_CPUCS,
-				  /* Index */
-				  0x0000,
-				  /* address of the transfer buffer */
-				  local_transfer_buffer,
-				  /* Length */
-				  1,
-				  /* Timeout */
-				  BULK_TIMEOUT);
-	if (errcode < 0)
-		dev_err(&usbduxsub->interface->dev,
-			"comedi_: control msg failed (start)\n");
-
-	kfree(local_transfer_buffer);
-	return errcode;
-}
-
-static int usbduxsub_stop(struct usbduxsub *usbduxsub)
-{
-	int errcode = 0;
-	uint8_t *local_transfer_buffer;
-
-	local_transfer_buffer = kmalloc(1, GFP_KERNEL);
-	if (!local_transfer_buffer)
-		return -ENOMEM;
-
-	/* 7f92 to one */
-	*local_transfer_buffer = 1;
-	errcode = usb_control_msg(usbduxsub->usbdev,
-				  usb_sndctrlpipe(usbduxsub->usbdev, 0),
-				  /* bRequest, "Firmware" */
-				  USBDUXSUB_FIRMWARE,
-				  /* bmRequestType */
-				  VENDOR_DIR_OUT,
-				  /* Value */
-				  USBDUXSUB_CPUCS,
-				  /* Index */
-				  0x0000, local_transfer_buffer,
-				  /* Length */
-				  1,
-				  /* Timeout */
-				  BULK_TIMEOUT);
-	if (errcode < 0)
-		dev_err(&usbduxsub->interface->dev,
-			"comedi_: control msg failed (stop)\n");
-
-	kfree(local_transfer_buffer);
-	return errcode;
-}
-
-static int usbduxsub_upload(struct usbduxsub *usbduxsub,
-			    uint8_t *local_transfer_buffer,
-			    unsigned int start_addr, unsigned int len)
-{
-	int errcode;
-
-	errcode = usb_control_msg(usbduxsub->usbdev,
-				  usb_sndctrlpipe(usbduxsub->usbdev, 0),
-				  /* brequest, firmware */
-				  USBDUXSUB_FIRMWARE,
-				  /* bmRequestType */
-				  VENDOR_DIR_OUT,
-				  /* value */
-				  start_addr,
-				  /* index */
-				  0x0000,
-				  /* our local safe buffer */
-				  local_transfer_buffer,
-				  /* length */
-				  len,
-				  /* timeout */
-				  BULK_TIMEOUT);
-	dev_dbg(&usbduxsub->interface->dev, "comedi_: result=%d\n", errcode);
-	if (errcode < 0) {
-		dev_err(&usbduxsub->interface->dev, "comedi_: upload failed\n");
-		return errcode;
-	}
-	return 0;
-}
-
 #define FIRMWARE_MAX_LEN 0x2000
 
 static int firmware_upload(struct usbduxsub *usbduxsub,
-			  const u8 *firmware_binary, int size_firmware)
+			  const u8 *data, int size)
 {
+	struct usb_device *usb = usbduxsub->usbdev;
+	uint8_t *buf;
+	uint8_t *tmp;
 	int ret;
-	uint8_t *fw_buf;
 
-	if (!firmware_binary)
+	if (!data)
 		return 0;
 
-	if (size_firmware > FIRMWARE_MAX_LEN) {
+	if (size > FIRMWARE_MAX_LEN) {
 		dev_err(&usbduxsub->interface->dev,
 			"usbdux firmware binary it too large for FX2.\n");
 		return -ENOMEM;
 	}
 
 	/* we generate a local buffer for the firmware */
-	fw_buf = kmemdup(firmware_binary, size_firmware, GFP_KERNEL);
-	if (!fw_buf) {
-		dev_err(&usbduxsub->interface->dev,
-			"comedi_: mem alloc for firmware failed\n");
+	buf = kmemdup(data, size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	/* we need a malloc'ed buffer for usb_control_msg() */
+	tmp = kmalloc(1, GFP_KERNEL);
+	if (!tmp) {
+		kfree(buf);
 		return -ENOMEM;
 	}
 
-	ret = usbduxsub_stop(usbduxsub);
+	/* stop the current firmware on the device */
+	*tmp = 1;	/* 7f92 to one */
+	ret = usb_control_msg(usb, usb_sndctrlpipe(usb, 0),
+			      USBDUXSUB_FIRMWARE,
+			      VENDOR_DIR_OUT,
+			      USBDUXSUB_CPUCS, 0x0000,
+			      tmp, 1,
+			      BULK_TIMEOUT);
 	if (ret < 0) {
 		dev_err(&usbduxsub->interface->dev,
 			"comedi_: can not stop firmware\n");
-		kfree(fw_buf);
-		return ret;
+		goto done;
 	}
 
-	ret = usbduxsub_upload(usbduxsub, fw_buf, 0, size_firmware);
+	/* upload the new firmware to the device */
+	ret = usb_control_msg(usb, usb_sndctrlpipe(usb, 0),
+			      USBDUXSUB_FIRMWARE,
+			      VENDOR_DIR_OUT,
+			      0, 0x0000,
+			      buf, size,
+			      BULK_TIMEOUT);
 	if (ret < 0) {
 		dev_err(&usbduxsub->interface->dev,
 			"comedi_: firmware upload failed\n");
-		kfree(fw_buf);
-		return ret;
+		goto done;
 	}
-	ret = usbduxsub_start(usbduxsub);
-	if (ret < 0) {
+
+	/* start the new firmware on the device */
+	*tmp = 0;	/* 7f92 to zero */
+	ret = usb_control_msg(usb, usb_sndctrlpipe(usb, 0),
+			      USBDUXSUB_FIRMWARE,
+			      VENDOR_DIR_OUT,
+			      USBDUXSUB_CPUCS, 0x0000,
+			      tmp, 1,
+			      BULK_TIMEOUT);
+	if (ret < 0)
 		dev_err(&usbduxsub->interface->dev,
 			"comedi_: can not start firmware\n");
-		kfree(fw_buf);
-		return ret;
-	}
-	kfree(fw_buf);
-	return 0;
+
+done:
+	kfree(tmp);
+	kfree(buf);
+	return ret;
 }
 
 static int usbduxsub_submit_inurbs(struct usbduxsub *usbduxsub)
-- 
1.8.1.4




More information about the devel mailing list