[PATCH v2 00/15] drivers: usb: misc: update to use usb_control_msg_{send|recv}()

[PATCH v2 00/15] drivers: usb: misc: update to use usb_control_msg_{send|recv}()

[Anant Thazhemadam]

The new usb_control_msg_{send|recv}() API provides an improved way of 
using usb_control_msg(). Using this, short reads/writes are considered
as errors, data can be used off the stack, and the need for the calling
function to create a raw usb pipe is eliminated.
This patch series aims to update existing instances of usb_control_msg() 
in drivers/usb/misc to usb_control_msg_{send|recv}() appropriately, and
also update the return value checking mechanisms in place (if any), as
necessary so nothing breaks.

I was however unable to update one instance of usb_control_msg() in 
drivers/usb/misc/apple-mfi-fastcharge.c.

The return value checking mechanism present here, is as follows.
	if (retval) {
		dev_dbg(&mfi->udev->dev, "retval = %d\n", retval);
		return retval;
	}

	mfi->charge_type = val->intval;

	return 0;

This implies that mfi->charge_type = val->intval only when number of
bytes transferred = 0, and the return value is directly returned 
otherwise. Since the new API doesn't return the number of bytes 
transferred, I wasn't quite sure how this instance could be updated.
In case this check is logically incorrect, a patch with a fix 
can be sent in as well.

Changes in v2:

  * Buffer variables that were previously dynamically allocated are no
    longer dynamically allocated unless they have a variable length
    (since that threw a warning).
    

Anant Thazhemadam (15):
  usb: misc: appledisplay: update to use the
    usb_control_msg_{send|recv}() API
  usb: misc: cypress_cy7c63: update to use usb_control_msg_recv()
  usb: misc: cytherm: update to use usb_control_msg_recv()
  usb: misc: ehset: update to use the usb_control_msg_{send|recv}() API
  usb: misc: emi26: update to use usb_control_msg_send()
  usb: misc: emi62: update to use usb_control_msg_send()
  usb: misc: ezusb: update to use usb_control_msg_send()
  usb: misc: idmouse: update to use usb_control_msg_send()
  usb: misc: iowarrior: update to use the usb_control_msg_{send|recv}()
    API
  usb: misc: isight_firmware: update to use usb_control_msg_send()
  usb: misc: ldusb: update to use usb_control_msg_send()
  usb: misc: lvstest: update to use the usb_control_msg_{send|recv}()
    API
  usb: misc: trancevibrator: update to use usb_control_msg_send()
  usb: misc: usbsevseg: update to use usb_control_msg_send()
  usb: misc: usbtest: update to use the usb_control_msg_{send|recv}()
    API

 drivers/usb/misc/appledisplay.c    |  46 +++++------
 drivers/usb/misc/cypress_cy7c63.c  |  21 ++---
 drivers/usb/misc/cytherm.c         | 128 ++++++++++-------------------
 drivers/usb/misc/ehset.c           |  76 ++++++++---------
 drivers/usb/misc/emi26.c           |  31 ++-----
 drivers/usb/misc/emi62.c           |  30 ++-----
 drivers/usb/misc/ezusb.c           |  16 +---
 drivers/usb/misc/idmouse.c         |   5 +-
 drivers/usb/misc/iowarrior.c       |  34 ++++----
 drivers/usb/misc/isight_firmware.c |  30 +++----
 drivers/usb/misc/ldusb.c           |   8 +-
 drivers/usb/misc/lvstest.c         |  38 ++++-----
 drivers/usb/misc/trancevibrator.c  |   4 +-
 drivers/usb/misc/usbsevseg.c       |  60 ++++----------
 drivers/usb/misc/usbtest.c         |  69 +++++++---------
 15 files changed, 216 insertions(+), 380 deletions(-)

-- 
2.25.1

[PATCH v2 01/15] usb: misc: appledisplay: update to use the usb_control_msg_{send|recv}() API

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, instances of usb_control_msg() have been replaced with
usb_control_msg_{recv|send}(), and all return value checking
conditions have also been modified appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/appledisplay.c | 46 ++++++++++++++-------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/misc/appledisplay.c b/drivers/usb/misc/appledisplay.c
index c8098e9b432e..117deb2fdc29 100644
--- a/drivers/usb/misc/appledisplay.c
+++ b/drivers/usb/misc/appledisplay.c
@@ -132,21 +132,17 @@ static int appledisplay_bl_update_status(struct backlight_device *bd)
 	pdata->msgdata[0] = 0x10;
 	pdata->msgdata[1] = bd->props.brightness;
 
-	retval = usb_control_msg(
-		pdata->udev,
-		usb_sndctrlpipe(pdata->udev, 0),
-		USB_REQ_SET_REPORT,
-		USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-		ACD_USB_BRIGHTNESS,
-		0,
-		pdata->msgdata, 2,
-		ACD_USB_TIMEOUT);
+	retval = usb_control_msg_send(pdata->udev,
+				      0,
+				      USB_REQ_SET_REPORT,
+				      USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+				      ACD_USB_BRIGHTNESS,
+				      0,
+				      pdata->msgdata, 2,
+				      ACD_USB_TIMEOUT, GFP_KERNEL);
 	mutex_unlock(&pdata->sysfslock);
 
-	if (retval < 0)
-		return retval;
-	else
-		return 0;
+	return retval;
 }
 
 static int appledisplay_bl_get_brightness(struct backlight_device *bd)
@@ -155,21 +151,17 @@ static int appledisplay_bl_get_brightness(struct backlight_device *bd)
 	int retval, brightness;
 
 	mutex_lock(&pdata->sysfslock);
-	retval = usb_control_msg(
-		pdata->udev,
-		usb_rcvctrlpipe(pdata->udev, 0),
-		USB_REQ_GET_REPORT,
-		USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-		ACD_USB_BRIGHTNESS,
-		0,
-		pdata->msgdata, 2,
-		ACD_USB_TIMEOUT);
-	if (retval < 2) {
-		if (retval >= 0)
-			retval = -EMSGSIZE;
-	} else {
+	retval = usb_control_msg_recv(pdata->udev,
+				      0,
+				      USB_REQ_GET_REPORT,
+				      USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+				      ACD_USB_BRIGHTNESS,
+				      0,
+				      pdata->msgdata, 2,
+				      ACD_USB_TIMEOUT, GFP_KERNEL);
+	if (retval == 0)
 		brightness = pdata->msgdata[1];
-	}
+
 	mutex_unlock(&pdata->sysfslock);
 
 	if (retval < 0)
-- 
2.25.1

[PATCH v2 02/15] usb: misc: cypress_cy7c63: update to use usb_control_msg_recv()

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instance of usb_control_msg() has been replaced with
usb_control_msg_recv().

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/cypress_cy7c63.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/misc/cypress_cy7c63.c b/drivers/usb/misc/cypress_cy7c63.c
index 14faec51d7a5..76a320ef17a7 100644
--- a/drivers/usb/misc/cypress_cy7c63.c
+++ b/drivers/usb/misc/cypress_cy7c63.c
@@ -70,24 +70,15 @@ static int vendor_command(struct cypress *dev, unsigned char request,
 			  unsigned char address, unsigned char data)
 {
 	int retval = 0;
-	unsigned int pipe;
-	unsigned char *iobuf;
-
-	/* allocate some memory for the i/o buffer*/
-	iobuf = kzalloc(CYPRESS_MAX_REQSIZE, GFP_KERNEL);
-	if (!iobuf) {
-		retval = -ENOMEM;
-		goto error;
-	}
+	u8 iobuf[CYPRESS_MAX_REQSIZE] = {0};
 
 	dev_dbg(&dev->udev->dev, "Sending usb_control_msg (data: %d)\n", data);
 
 	/* prepare usb control message and send it upstream */
-	pipe = usb_rcvctrlpipe(dev->udev, 0);
-	retval = usb_control_msg(dev->udev, pipe, request,
-				 USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
-				 address, data, iobuf, CYPRESS_MAX_REQSIZE,
-				 USB_CTRL_GET_TIMEOUT);
+	retval = usb_control_msg_recv(dev->udev, 0, request,
+				      USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
+				      address, data, &iobuf, CYPRESS_MAX_REQSIZE,
+				      USB_CTRL_GET_TIMEOUT, GFP_KERNEL);
 
 	/* store returned data (more READs to be added) */
 	switch (request) {
@@ -107,8 +98,6 @@ static int vendor_command(struct cypress *dev, unsigned char request,
 			break;
 	}
 
-	kfree(iobuf);
-error:
 	return retval;
 }
 
-- 
2.25.1

[PATCH v2 03/15] usb: misc: cytherm: update to use usb_control_msg_recv()

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instance of usb_control_msg() has been replaced with
usb_control_msg_recv().

The return value checking enforced by callers of the updated function have
also been appropriately updated.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/cytherm.c | 128 +++++++++++++------------------------
 1 file changed, 43 insertions(+), 85 deletions(-)

diff --git a/drivers/usb/misc/cytherm.c b/drivers/usb/misc/cytherm.c
index 3e3802aaefa3..2ca36ea5b76a 100644
--- a/drivers/usb/misc/cytherm.c
+++ b/drivers/usb/misc/cytherm.c
@@ -51,12 +51,12 @@ static int vendor_command(struct usb_device *dev, unsigned char request,
 			  unsigned char value, unsigned char index,
 			  void *buf, int size)
 {
-	return usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
-			       request, 
-			       USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
-			       value, 
-			       index, buf, size,
-			       USB_CTRL_GET_TIMEOUT);
+	return usb_control_msg_recv(dev, 0,
+				    request,
+				    USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
+				    value,
+				    index, buf, size,
+				    USB_CTRL_GET_TIMEOUT, GFP_KERNEL);
 }
 
 
@@ -78,33 +78,27 @@ static ssize_t brightness_store(struct device *dev, struct device_attribute *att
 	struct usb_interface *intf = to_usb_interface(dev);
 	struct usb_cytherm *cytherm = usb_get_intfdata(intf);
 
-	unsigned char *buffer;
+	unsigned char buffer[8];
 	int retval;
-   
-	buffer = kmalloc(8, GFP_KERNEL);
-	if (!buffer)
-		return 0;
 
 	cytherm->brightness = simple_strtoul(buf, NULL, 10);
-   
+
 	if (cytherm->brightness > 0xFF)
 		cytherm->brightness = 0xFF;
 	else if (cytherm->brightness < 0)
 		cytherm->brightness = 0;
-   
+
 	/* Set brightness */
 	retval = vendor_command(cytherm->udev, WRITE_RAM, BRIGHTNESS, 
-				cytherm->brightness, buffer, 8);
-	if (retval)
-		dev_dbg(&cytherm->udev->dev, "retval = %d\n", retval);
+				cytherm->brightness, &buffer, 8);
+	if (!retval)
+		dev_dbg(&cytherm->udev->dev, "brightness set correctly\n");
 	/* Inform µC that we have changed the brightness setting */
 	retval = vendor_command(cytherm->udev, WRITE_RAM, BRIGHTNESS_SEM,
-				0x01, buffer, 8);
-	if (retval)
-		dev_dbg(&cytherm->udev->dev, "retval = %d\n", retval);
-   
-	kfree(buffer);
-   
+				0x01, &buffer, 8);
+	if (!retval)
+		dev_dbg(&cytherm->udev->dev, "µC informed of change in brightness setting\n");
+
 	return count;
 }
 static DEVICE_ATTR_RW(brightness);
@@ -120,28 +114,22 @@ static ssize_t temp_show(struct device *dev, struct device_attribute *attr, char
 	struct usb_cytherm *cytherm = usb_get_intfdata(intf);
 
 	int retval;
-	unsigned char *buffer;
+	unsigned char buffer[8];
 
 	int temp, sign;
    
-	buffer = kmalloc(8, GFP_KERNEL);
-	if (!buffer)
-		return 0;
-
 	/* read temperature */
-	retval = vendor_command(cytherm->udev, READ_RAM, TEMP, 0, buffer, 8);
-	if (retval)
-		dev_dbg(&cytherm->udev->dev, "retval = %d\n", retval);
+	retval = vendor_command(cytherm->udev, READ_RAM, TEMP, 0, &buffer, 8);
+	if (!retval)
+		dev_dbg(&cytherm->udev->dev, "read temperature successfully\n");
 	temp = buffer[1];
    
 	/* read sign */
-	retval = vendor_command(cytherm->udev, READ_RAM, SIGN, 0, buffer, 8);
-	if (retval)
-		dev_dbg(&cytherm->udev->dev, "retval = %d\n", retval);
+	retval = vendor_command(cytherm->udev, READ_RAM, SIGN, 0, &buffer, 8);
+	if (!retval)
+		dev_dbg(&cytherm->udev->dev, "read sign successfully\n");
 	sign = buffer[1];
 
-	kfree(buffer);
-   
 	return sprintf(buf, "%c%i.%i", sign ? '-' : '+', temp >> 1,
 		       5*(temp - ((temp >> 1) << 1)));
 }
@@ -157,21 +145,15 @@ static ssize_t button_show(struct device *dev, struct device_attribute *attr, ch
 	struct usb_cytherm *cytherm = usb_get_intfdata(intf);
 
 	int retval;
-	unsigned char *buffer;
-
-	buffer = kmalloc(8, GFP_KERNEL);
-	if (!buffer)
-		return 0;
+	unsigned char buffer[8];
 
 	/* check button */
-	retval = vendor_command(cytherm->udev, READ_RAM, BUTTON, 0, buffer, 8);
-	if (retval)
-		dev_dbg(&cytherm->udev->dev, "retval = %d\n", retval);
+	retval = vendor_command(cytherm->udev, READ_RAM, BUTTON, 0, &buffer, 8);
+	if (!retval)
+		dev_dbg(&cytherm->udev->dev, "checked button successfully\n");
    
 	retval = buffer[1];
 
-	kfree(buffer);
-
 	if (retval)
 		return sprintf(buf, "1");
 	else
@@ -186,20 +168,14 @@ static ssize_t port0_show(struct device *dev, struct device_attribute *attr, cha
 	struct usb_cytherm *cytherm = usb_get_intfdata(intf);
 
 	int retval;
-	unsigned char *buffer;
-
-	buffer = kmalloc(8, GFP_KERNEL);
-	if (!buffer)
-		return 0;
+	unsigned char buffer[8];
 
-	retval = vendor_command(cytherm->udev, READ_PORT, 0, 0, buffer, 8);
-	if (retval)
+	retval = vendor_command(cytherm->udev, READ_PORT, 0, 0, &buffer, 8);
+	if (!retval)
 		dev_dbg(&cytherm->udev->dev, "retval = %d\n", retval);
 
 	retval = buffer[1];
 
-	kfree(buffer);
-
 	return sprintf(buf, "%d", retval);
 }
 
@@ -209,28 +185,22 @@ static ssize_t port0_store(struct device *dev, struct device_attribute *attr, co
 	struct usb_interface *intf = to_usb_interface(dev);
 	struct usb_cytherm *cytherm = usb_get_intfdata(intf);
 
-	unsigned char *buffer;
+	unsigned char buffer[8];
 	int retval;
 	int tmp;
-   
-	buffer = kmalloc(8, GFP_KERNEL);
-	if (!buffer)
-		return 0;
 
 	tmp = simple_strtoul(buf, NULL, 10);
-   
+
 	if (tmp > 0xFF)
 		tmp = 0xFF;
 	else if (tmp < 0)
 		tmp = 0;
-   
+
 	retval = vendor_command(cytherm->udev, WRITE_PORT, 0,
-				tmp, buffer, 8);
-	if (retval)
+				tmp, &buffer, 8);
+	if (!retval)
 		dev_dbg(&cytherm->udev->dev, "retval = %d\n", retval);
 
-	kfree(buffer);
-
 	return count;
 }
 static DEVICE_ATTR_RW(port0);
@@ -241,19 +211,13 @@ static ssize_t port1_show(struct device *dev, struct device_attribute *attr, cha
 	struct usb_cytherm *cytherm = usb_get_intfdata(intf);
 
 	int retval;
-	unsigned char *buffer;
+	unsigned char buffer[8];
 
-	buffer = kmalloc(8, GFP_KERNEL);
-	if (!buffer)
-		return 0;
-
-	retval = vendor_command(cytherm->udev, READ_PORT, 1, 0, buffer, 8);
-	if (retval)
+	retval = vendor_command(cytherm->udev, READ_PORT, 1, 0, &buffer, 8);
+	if (!retval)
 		dev_dbg(&cytherm->udev->dev, "retval = %d\n", retval);
-   
-	retval = buffer[1];
 
-	kfree(buffer);
+	retval = buffer[1];
 
 	return sprintf(buf, "%d", retval);
 }
@@ -264,13 +228,9 @@ static ssize_t port1_store(struct device *dev, struct device_attribute *attr, co
 	struct usb_interface *intf = to_usb_interface(dev);
 	struct usb_cytherm *cytherm = usb_get_intfdata(intf);
 
-	unsigned char *buffer;
+	unsigned char buffer[8];
 	int retval;
 	int tmp;
-   
-	buffer = kmalloc(8, GFP_KERNEL);
-	if (!buffer)
-		return 0;
 
 	tmp = simple_strtoul(buf, NULL, 10);
    
@@ -278,14 +238,12 @@ static ssize_t port1_store(struct device *dev, struct device_attribute *attr, co
 		tmp = 0xFF;
 	else if (tmp < 0)
 		tmp = 0;
-   
+
 	retval = vendor_command(cytherm->udev, WRITE_PORT, 1,
-				tmp, buffer, 8);
-	if (retval)
+				tmp, &buffer, 8);
+	if (!retval)
 		dev_dbg(&cytherm->udev->dev, "retval = %d\n", retval);
 
-	kfree(buffer);
-
 	return count;
 }
 static DEVICE_ATTR_RW(port1);
-- 
2.25.1

[PATCH v2 04/15] usb: misc: ehset: update to use the usb_control_msg_{send|recv}() API

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, instances of usb_control_msg() have been replaced with
usb_control_msg_{recv|send}() appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/ehset.c | 76 +++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/misc/ehset.c b/drivers/usb/misc/ehset.c
index 2752e1f4f4d0..f87890f9cd26 100644
--- a/drivers/usb/misc/ehset.c
+++ b/drivers/usb/misc/ehset.c
@@ -24,68 +24,57 @@ static int ehset_probe(struct usb_interface *intf,
 	int ret = -EINVAL;
 	struct usb_device *dev = interface_to_usbdev(intf);
 	struct usb_device *hub_udev = dev->parent;
-	struct usb_device_descriptor *buf;
+	struct usb_device_descriptor buf;
 	u8 portnum = dev->portnum;
 	u16 test_pid = le16_to_cpu(dev->descriptor.idProduct);
 
 	switch (test_pid) {
 	case TEST_SE0_NAK_PID:
-		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
-					USB_REQ_SET_FEATURE, USB_RT_PORT,
-					USB_PORT_FEAT_TEST,
-					(USB_TEST_SE0_NAK << 8) | portnum,
-					NULL, 0, 1000);
+		ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+					   USB_RT_PORT, USB_PORT_FEAT_TEST,
+					   (USB_TEST_SE0_NAK << 8) | portnum,
+					   NULL, 0, 1000, GFP_KERNEL);
 		break;
 	case TEST_J_PID:
-		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
-					USB_REQ_SET_FEATURE, USB_RT_PORT,
-					USB_PORT_FEAT_TEST,
-					(USB_TEST_J << 8) | portnum,
-					NULL, 0, 1000);
+		ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+					   USB_RT_PORT, USB_PORT_FEAT_TEST,
+					   (USB_TEST_J << 8) | portnum, NULL, 0,
+					   1000, GFP_KERNEL);
 		break;
 	case TEST_K_PID:
-		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
-					USB_REQ_SET_FEATURE, USB_RT_PORT,
-					USB_PORT_FEAT_TEST,
-					(USB_TEST_K << 8) | portnum,
-					NULL, 0, 1000);
+		ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+					   USB_RT_PORT, USB_PORT_FEAT_TEST,
+					   (USB_TEST_K << 8) | portnum, NULL, 0,
+					   1000, GFP_KERNEL);
 		break;
 	case TEST_PACKET_PID:
-		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
-					USB_REQ_SET_FEATURE, USB_RT_PORT,
-					USB_PORT_FEAT_TEST,
-					(USB_TEST_PACKET << 8) | portnum,
-					NULL, 0, 1000);
+		ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+					   USB_RT_PORT, USB_PORT_FEAT_TEST,
+					   (USB_TEST_PACKET << 8) | portnum,
+					   NULL, 0, 1000, GFP_KERNEL);
 		break;
 	case TEST_HS_HOST_PORT_SUSPEND_RESUME:
 		/* Test: wait for 15secs -> suspend -> 15secs delay -> resume */
 		msleep(15 * 1000);
-		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
-					USB_REQ_SET_FEATURE, USB_RT_PORT,
-					USB_PORT_FEAT_SUSPEND, portnum,
-					NULL, 0, 1000);
+		ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+					   USB_RT_PORT, USB_PORT_FEAT_SUSPEND,
+					   portnum, NULL, 0, 1000, GFP_KERNEL);
 		if (ret < 0)
 			break;
 
 		msleep(15 * 1000);
-		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
-					USB_REQ_CLEAR_FEATURE, USB_RT_PORT,
-					USB_PORT_FEAT_SUSPEND, portnum,
-					NULL, 0, 1000);
+		ret = usb_control_msg_send(hub_udev, 0, USB_REQ_CLEAR_FEATURE,
+					   USB_RT_PORT, USB_PORT_FEAT_SUSPEND,
+					   portnum, NULL, 0, 1000, GFP_KERNEL);
 		break;
 	case TEST_SINGLE_STEP_GET_DEV_DESC:
 		/* Test: wait for 15secs -> GetDescriptor request */
 		msleep(15 * 1000);
-		buf = kmalloc(USB_DT_DEVICE_SIZE, GFP_KERNEL);
-		if (!buf)
-			return -ENOMEM;
 
-		ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
-					USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
-					USB_DT_DEVICE << 8, 0,
-					buf, USB_DT_DEVICE_SIZE,
-					USB_CTRL_GET_TIMEOUT);
-		kfree(buf);
+		ret = usb_control_msg_recv(dev, 0, USB_REQ_GET_DESCRIPTOR,
+					   USB_DIR_IN, USB_DT_DEVICE << 8, 0,
+					   &buf, USB_DT_DEVICE_SIZE,
+					   USB_CTRL_GET_TIMEOUT, GFP_KERNEL);
 		break;
 	case TEST_SINGLE_STEP_SET_FEATURE:
 		/*
@@ -100,11 +89,10 @@ static int ehset_probe(struct usb_interface *intf,
 			break;
 		}
 
-		ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
-					USB_REQ_SET_FEATURE, USB_RT_PORT,
-					USB_PORT_FEAT_TEST,
-					(6 << 8) | portnum,
-					NULL, 0, 60 * 1000);
+		ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+					   USB_RT_PORT, USB_PORT_FEAT_TEST,
+					   (6 << 8) | portnum, NULL, 0,
+					   60 * 1000, GFP_KERNEL);
 
 		break;
 	default:
@@ -112,7 +100,7 @@ static int ehset_probe(struct usb_interface *intf,
 			__func__, test_pid);
 	}
 
-	return (ret < 0) ? ret : 0;
+	return ret;
 }
 
 static void ehset_disconnect(struct usb_interface *intf)
-- 
2.25.1

[PATCH v2 05/15] usb: misc: emi26: update to use usb_control_msg_send()

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instance of usb_control_msg() has been replaced with
usb_control_msg_send() appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/emi26.c | 31 ++++++++-----------------------
 1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/misc/emi26.c b/drivers/usb/misc/emi26.c
index 24d841850e05..1dd024507f40 100644
--- a/drivers/usb/misc/emi26.c
+++ b/drivers/usb/misc/emi26.c
@@ -27,7 +27,7 @@
 #define INTERNAL_RAM(address)   (address <= MAX_INTERNAL_ADDRESS)
 
 static int emi26_writememory( struct usb_device *dev, int address,
-			      const unsigned char *data, int length,
+			      const void *data, int length,
 			      __u8 bRequest);
 static int emi26_set_reset(struct usb_device *dev, unsigned char reset_bit);
 static int emi26_load_firmware (struct usb_device *dev);
@@ -35,22 +35,12 @@ static int emi26_probe(struct usb_interface *intf, const struct usb_device_id *i
 static void emi26_disconnect(struct usb_interface *intf);
 
 /* thanks to drivers/usb/serial/keyspan_pda.c code */
-static int emi26_writememory (struct usb_device *dev, int address,
-			      const unsigned char *data, int length,
+static int emi26_writememory(struct usb_device *dev, int address,
+			      const void *data, int length,
 			      __u8 request)
 {
-	int result;
-	unsigned char *buffer =  kmemdup(data, length, GFP_KERNEL);
-
-	if (!buffer) {
-		dev_err(&dev->dev, "kmalloc(%d) failed.\n", length);
-		return -ENOMEM;
-	}
-	/* Note: usb_control_msg returns negative value on error or length of the
-	 * 		 data that was written! */
-	result = usb_control_msg (dev, usb_sndctrlpipe(dev, 0), request, 0x40, address, 0, buffer, length, 300);
-	kfree (buffer);
-	return result;
+	return usb_control_msg_send(dev, 0, request, 0x40, address, 0,
+				    data, length, 300, GFP_KERNEL);
 }
 
 /* thanks to drivers/usb/serial/keyspan_pda.c code */
@@ -77,11 +67,7 @@ static int emi26_load_firmware (struct usb_device *dev)
 	int err = -ENOMEM;
 	int i;
 	__u32 addr;	/* Address to write */
-	__u8 *buf;
-
-	buf = kmalloc(FW_LOAD_SIZE, GFP_KERNEL);
-	if (!buf)
-		goto wraperr;
+	__u8 buf[FW_LOAD_SIZE];
 
 	err = request_ihex_firmware(&loader_fw, "emi26/loader.fw", &dev->dev);
 	if (err)
@@ -133,11 +119,11 @@ static int emi26_load_firmware (struct usb_device *dev)
 
 		/* intel hex records are terminated with type 0 element */
 		while (rec && (i + be16_to_cpu(rec->len) < FW_LOAD_SIZE)) {
-			memcpy(buf + i, rec->data, be16_to_cpu(rec->len));
+			memcpy(&buf[i], rec->data, be16_to_cpu(rec->len));
 			i += be16_to_cpu(rec->len);
 			rec = ihex_next_binrec(rec);
 		}
-		err = emi26_writememory(dev, addr, buf, i, ANCHOR_LOAD_FPGA);
+		err = emi26_writememory(dev, addr, &buf, i, ANCHOR_LOAD_FPGA);
 		if (err < 0)
 			goto wraperr;
 	} while (rec);
@@ -211,7 +197,6 @@ static int emi26_load_firmware (struct usb_device *dev)
 	release_firmware(bitstream_fw);
 	release_firmware(firmware_fw);
 
-	kfree(buf);
 	return err;
 }
 
-- 
2.25.1

[PATCH v2 06/15] usb: misc: emi62: update to use usb_control_msg_send()

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instance of usb_control_msg() has been replaced with
usb_control_msg_send() appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/emi62.c | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/misc/emi62.c b/drivers/usb/misc/emi62.c
index 3eea60437f56..6ac4e72d53e0 100644
--- a/drivers/usb/misc/emi62.c
+++ b/drivers/usb/misc/emi62.c
@@ -36,7 +36,7 @@
 #define INTERNAL_RAM(address)   (address <= MAX_INTERNAL_ADDRESS)
 
 static int emi62_writememory(struct usb_device *dev, int address,
-			     const unsigned char *data, int length,
+			     const void *data, int length,
 			     __u8 bRequest);
 static int emi62_set_reset(struct usb_device *dev, unsigned char reset_bit);
 static int emi62_load_firmware (struct usb_device *dev);
@@ -45,21 +45,11 @@ static void emi62_disconnect(struct usb_interface *intf);
 
 /* thanks to drivers/usb/serial/keyspan_pda.c code */
 static int emi62_writememory(struct usb_device *dev, int address,
-			     const unsigned char *data, int length,
+			     const void *data, int length,
 			     __u8 request)
 {
-	int result;
-	unsigned char *buffer =  kmemdup(data, length, GFP_KERNEL);
-
-	if (!buffer) {
-		dev_err(&dev->dev, "kmalloc(%d) failed.\n", length);
-		return -ENOMEM;
-	}
-	/* Note: usb_control_msg returns negative value on error or length of the
-	 * 		 data that was written! */
-	result = usb_control_msg (dev, usb_sndctrlpipe(dev, 0), request, 0x40, address, 0, buffer, length, 300);
-	kfree (buffer);
-	return result;
+	return usb_control_msg_send(dev, 0, request, 0x40, address,
+				    0, data, length, 300, GFP_KERNEL);
 }
 
 /* thanks to drivers/usb/serial/keyspan_pda.c code */
@@ -85,12 +75,9 @@ static int emi62_load_firmware (struct usb_device *dev)
 	int err = -ENOMEM;
 	int i;
 	__u32 addr;	/* Address to write */
-	__u8 *buf;
+	__u8 buf[FW_LOAD_SIZE];
 
 	dev_dbg(&dev->dev, "load_firmware\n");
-	buf = kmalloc(FW_LOAD_SIZE, GFP_KERNEL);
-	if (!buf)
-		goto wraperr;
 
 	err = request_ihex_firmware(&loader_fw, "emi62/loader.fw", &dev->dev);
 	if (err)
@@ -140,11 +127,11 @@ static int emi62_load_firmware (struct usb_device *dev)
 
 		/* intel hex records are terminated with type 0 element */
 		while (rec && (i + be16_to_cpu(rec->len) < FW_LOAD_SIZE)) {
-			memcpy(buf + i, rec->data, be16_to_cpu(rec->len));
+			memcpy(&buf[i], rec->data, be16_to_cpu(rec->len));
 			i += be16_to_cpu(rec->len);
 			rec = ihex_next_binrec(rec);
 		}
-		err = emi62_writememory(dev, addr, buf, i, ANCHOR_LOAD_FPGA);
+		err = emi62_writememory(dev, addr, &buf, i, ANCHOR_LOAD_FPGA);
 		if (err < 0)
 			goto wraperr;
 	} while (rec);
@@ -209,8 +196,6 @@ static int emi62_load_firmware (struct usb_device *dev)
 	release_firmware(bitstream_fw);
 	release_firmware(firmware_fw);
 
-	kfree(buf);
-
 	/* return 1 to fail the driver inialization
 	 * and give real driver change to load */
 	return 1;
@@ -223,7 +208,6 @@ static int emi62_load_firmware (struct usb_device *dev)
 	release_firmware(bitstream_fw);
 	release_firmware(firmware_fw);
 
-	kfree(buf);
 	dev_err(&dev->dev, "Error\n");
 	return err;
 }
-- 
2.25.1

[PATCH v2 07/15] usb: misc: ezusb: update to use usb_control_msg_send()

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instance of usb_control_msg() has been replaced with
usb_control_msg_send() appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/ezusb.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/misc/ezusb.c b/drivers/usb/misc/ezusb.c
index f058d8029761..78aaee56c2b7 100644
--- a/drivers/usb/misc/ezusb.c
+++ b/drivers/usb/misc/ezusb.c
@@ -31,24 +31,12 @@ static const struct ezusb_fx_type ezusb_fx1 = {
 static int ezusb_writememory(struct usb_device *dev, int address,
 				unsigned char *data, int length, __u8 request)
 {
-	int result;
-	unsigned char *transfer_buffer;
-
 	if (!dev)
 		return -ENODEV;
 
-	transfer_buffer = kmemdup(data, length, GFP_KERNEL);
-	if (!transfer_buffer) {
-		dev_err(&dev->dev, "%s - kmalloc(%d) failed.\n",
-							__func__, length);
-		return -ENOMEM;
-	}
-	result = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request,
+	return usb_control_msg_send(dev, 0, request,
 				 USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-				 address, 0, transfer_buffer, length, 3000);
-
-	kfree(transfer_buffer);
-	return result;
+				 address, 0, data, length, 3000, GFP_KERNEL);
 }
 
 static int ezusb_set_reset(struct usb_device *dev, unsigned short cpucs_reg,
-- 
2.25.1

[PATCH v2 08/15] usb: misc: idmouse: update to use usb_control_msg_send()

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instance of usb_control_msg() has been replaced with
usb_control_msg_send() appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/idmouse.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/misc/idmouse.c b/drivers/usb/misc/idmouse.c
index e9437a176518..52126441a633 100644
--- a/drivers/usb/misc/idmouse.c
+++ b/drivers/usb/misc/idmouse.c
@@ -56,8 +56,9 @@ static const struct usb_device_id idmouse_table[] = {
 #define FTIP_SCROLL  0x24
 
 #define ftip_command(dev, command, value, index) \
-	usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0), command, \
-	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, value, index, NULL, 0, 1000)
+	usb_control_msg_send(dev->udev, 0, command, \
+	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, \
+	value, index, NULL, 0, 1000, GFP_KERNEL)
 
 MODULE_DEVICE_TABLE(usb, idmouse_table);
 
-- 
2.25.1

[PATCH v2 09/15] usb: misc: iowarrior: update to use the usb_control_msg_{send|recv}() API

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, instances of usb_control_msg() have been replaced with
usb_control_msg_{recv|send}() appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/iowarrior.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c
index 70ec29681526..53726fffe5df 100644
--- a/drivers/usb/misc/iowarrior.c
+++ b/drivers/usb/misc/iowarrior.c
@@ -109,12 +109,12 @@ static int usb_get_report(struct usb_device *dev,
 			  struct usb_host_interface *inter, unsigned char type,
 			  unsigned char id, void *buf, int size)
 {
-	return usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
-			       USB_REQ_GET_REPORT,
-			       USB_DIR_IN | USB_TYPE_CLASS |
-			       USB_RECIP_INTERFACE, (type << 8) + id,
-			       inter->desc.bInterfaceNumber, buf, size,
-			       GET_TIMEOUT*HZ);
+	return usb_control_msg_recv(dev, 0,
+				    USB_REQ_GET_REPORT,
+				    USB_DIR_IN | USB_TYPE_CLASS |
+				    USB_RECIP_INTERFACE, (type << 8) + id,
+				    inter->desc.bInterfaceNumber, buf, size,
+				    GET_TIMEOUT*HZ, GFP_KERNEL);
 }
 //#endif
 
@@ -123,13 +123,13 @@ static int usb_get_report(struct usb_device *dev,
 static int usb_set_report(struct usb_interface *intf, unsigned char type,
 			  unsigned char id, void *buf, int size)
 {
-	return usb_control_msg(interface_to_usbdev(intf),
-			       usb_sndctrlpipe(interface_to_usbdev(intf), 0),
-			       USB_REQ_SET_REPORT,
-			       USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-			       (type << 8) + id,
-			       intf->cur_altsetting->desc.bInterfaceNumber, buf,
-			       size, HZ);
+	return usb_control_msg_send(interface_to_usbdev(intf),
+				    0,
+				    USB_REQ_SET_REPORT,
+				    USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+				    (type << 8) + id,
+				    intf->cur_altsetting->desc.bInterfaceNumber, buf,
+				    size, HZ, GFP_KERNEL);
 }
 
 /*---------------------*/
@@ -854,10 +854,10 @@ static int iowarrior_probe(struct usb_interface *interface,
 
 	/* Set the idle timeout to 0, if this is interface 0 */
 	if (dev->interface->cur_altsetting->desc.bInterfaceNumber == 0) {
-	    usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-			    0x0A,
-			    USB_TYPE_CLASS | USB_RECIP_INTERFACE, 0,
-			    0, NULL, 0, USB_CTRL_SET_TIMEOUT);
+		usb_control_msg_send(udev, 0,
+				     0x0A,
+				     USB_TYPE_CLASS | USB_RECIP_INTERFACE, 0,
+				     0, NULL, 0, USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
 	}
 	/* allow device read and ioctl */
 	dev->present = 1;
-- 
2.25.1

[PATCH v2 10/15] usb: misc: isight_firmware: update to use usb_control_msg_send()

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instances of usb_control_msg() have been replaced with
usb_control_msg_send(), and return value checking has also been
appropriately enforced.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/isight_firmware.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/misc/isight_firmware.c b/drivers/usb/misc/isight_firmware.c
index 4d30095d6ad2..1bd14a431f6c 100644
--- a/drivers/usb/misc/isight_firmware.c
+++ b/drivers/usb/misc/isight_firmware.c
@@ -37,13 +37,10 @@ static int isight_firmware_load(struct usb_interface *intf,
 	struct usb_device *dev = interface_to_usbdev(intf);
 	int llen, len, req, ret = 0;
 	const struct firmware *firmware;
-	unsigned char *buf = kmalloc(50, GFP_KERNEL);
+	unsigned char buf[50];
 	unsigned char data[4];
 	const u8 *ptr;
 
-	if (!buf)
-		return -ENOMEM;
-
 	if (request_firmware(&firmware, "isight.fw", &dev->dev) != 0) {
 		printk(KERN_ERR "Unable to load isight firmware\n");
 		ret = -ENODEV;
@@ -53,11 +50,11 @@ static int isight_firmware_load(struct usb_interface *intf,
 	ptr = firmware->data;
 
 	buf[0] = 0x01;
-	if (usb_control_msg
-	    (dev, usb_sndctrlpipe(dev, 0), 0xa0, 0x40, 0xe600, 0, buf, 1,
-	     300) != 1) {
+	ret = usb_control_msg_send(dev, 0, 0xa0, 0x40, 0xe600,
+				   0, &buf, 1, 300, GFP_KERNEL);
+	if (ret != 0) {
 		printk(KERN_ERR
-		       "Failed to initialise isight firmware loader\n");
+			"Failed to initialise isight firmware loader\n");
 		ret = -ENODEV;
 		goto out;
 	}
@@ -82,15 +79,15 @@ static int isight_firmware_load(struct usb_interface *intf,
 				ret = -ENODEV;
 				goto out;
 			}
-			memcpy(buf, ptr, llen);
+			memcpy(&buf, ptr, llen);
 
 			ptr += llen;
 
-			if (usb_control_msg
-			    (dev, usb_sndctrlpipe(dev, 0), 0xa0, 0x40, req, 0,
-			     buf, llen, 300) != llen) {
+			ret = usb_control_msg_send(dev, 0, 0xa0, 0x40, req, 0,
+						   &buf, llen, 300, GFP_KERNEL);
+			if (ret != 0) {
 				printk(KERN_ERR
-				       "Failed to load isight firmware\n");
+					"Failed to load isight firmware\n");
 				ret = -ENODEV;
 				goto out;
 			}
@@ -99,15 +96,14 @@ static int isight_firmware_load(struct usb_interface *intf,
 	}
 
 	buf[0] = 0x00;
-	if (usb_control_msg
-	    (dev, usb_sndctrlpipe(dev, 0), 0xa0, 0x40, 0xe600, 0, buf, 1,
-	     300) != 1) {
+	ret = usb_control_msg_send(dev, 0, 0xa0, 0x40, 0xe600,
+				   0, &buf, 1, 300, GFP_KERNEL);
+	if (ret != 0) {
 		printk(KERN_ERR "isight firmware loading completion failed\n");
 		ret = -ENODEV;
 	}
 
 out:
-	kfree(buf);
 	release_firmware(firmware);
 	return ret;
 }
-- 
2.25.1

[PATCH v2 11/15] usb: misc: ldusb: update to use usb_control_msg_send()

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instance of usb_control_msg_send() has been replaced
with usb_control_msg_send() appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/ldusb.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/misc/ldusb.c b/drivers/usb/misc/ldusb.c
index 670e4d91e9ca..259ead4edecb 100644
--- a/drivers/usb/misc/ldusb.c
+++ b/drivers/usb/misc/ldusb.c
@@ -573,15 +573,13 @@ static ssize_t ld_usb_write(struct file *file, const char __user *buffer,
 	}
 
 	if (dev->interrupt_out_endpoint == NULL) {
-		/* try HID_REQ_SET_REPORT=9 on control_endpoint instead of interrupt_out_endpoint */
-		retval = usb_control_msg(interface_to_usbdev(dev->intf),
-					 usb_sndctrlpipe(interface_to_usbdev(dev->intf), 0),
-					 9,
+		retval = usb_control_msg_send(interface_to_usbdev(dev->intf),
+					 0, 9,
 					 USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT,
 					 1 << 8, 0,
 					 dev->interrupt_out_buffer,
 					 bytes_to_write,
-					 USB_CTRL_SET_TIMEOUT);
+					 USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
 		if (retval < 0)
 			dev_err(&dev->intf->dev,
 				"Couldn't submit HID_REQ_SET_REPORT %d\n",
-- 
2.25.1

[PATCH v2 12/15] usb: misc: lvstest: update to use the usb_control_msg_{send|recv}() API

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, instances of usb_control_msg() have been replaced with
usb_control_msg_{recv|send}() and the return value checking conditions have
also been modified appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/lvstest.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/misc/lvstest.c b/drivers/usb/misc/lvstest.c
index f8686139d6f3..daa1b2c9139f 100644
--- a/drivers/usb/misc/lvstest.c
+++ b/drivers/usb/misc/lvstest.c
@@ -85,17 +85,17 @@ static void destroy_lvs_device(struct usb_device *udev)
 static int lvs_rh_clear_port_feature(struct usb_device *hdev,
 		int port1, int feature)
 {
-	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+	return usb_control_msg_send(hdev, 0,
 		USB_REQ_CLEAR_FEATURE, USB_RT_PORT, feature, port1,
-		NULL, 0, 1000);
+		NULL, 0, 1000, GFP_KERNEL);
 }
 
 static int lvs_rh_set_port_feature(struct usb_device *hdev,
 		int port1, int feature)
 {
-	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+	return usb_control_msg_send(hdev, 0,
 		USB_REQ_SET_FEATURE, USB_RT_PORT, feature, port1,
-		NULL, 0, 1000);
+		NULL, 0, 1000, GFP_KERNEL);
 }
 
 static ssize_t u3_entry_store(struct device *dev,
@@ -257,13 +257,9 @@ static ssize_t get_dev_desc_store(struct device *dev,
 {
 	struct usb_interface *intf = to_usb_interface(dev);
 	struct usb_device *udev;
-	struct usb_device_descriptor *descriptor;
+	struct usb_device_descriptor descriptor;
 	int ret;
 
-	descriptor = kmalloc(sizeof(*descriptor), GFP_KERNEL);
-	if (!descriptor)
-		return -ENOMEM;
-
 	udev = create_lvs_device(intf);
 	if (!udev) {
 		dev_err(dev, "failed to create lvs device\n");
@@ -271,18 +267,16 @@ static ssize_t get_dev_desc_store(struct device *dev,
 		goto free_desc;
 	}
 
-	ret = usb_control_msg(udev, (PIPE_CONTROL << 30) | USB_DIR_IN,
-			USB_REQ_GET_DESCRIPTOR, USB_DIR_IN, USB_DT_DEVICE << 8,
-			0, descriptor, sizeof(*descriptor),
-			USB_CTRL_GET_TIMEOUT);
+	ret = usb_control_msg_recv(udev, 0, USB_REQ_GET_DESCRIPTOR,
+				   USB_DIR_IN, USB_DT_DEVICE << 8,
+				   0, &descriptor, sizeof(descriptor),
+				   USB_CTRL_GET_TIMEOUT, GFP_KERNEL);
 	if (ret < 0)
 		dev_err(dev, "can't read device descriptor %d\n", ret);
 
 	destroy_lvs_device(udev);
 
 free_desc:
-	kfree(descriptor);
-
 	if (ret < 0)
 		return ret;
 
@@ -336,10 +330,10 @@ static void lvs_rh_work(struct work_struct *work)
 
 	/* Examine each root port */
 	for (i = 1; i <= descriptor->bNbrPorts; i++) {
-		ret = usb_control_msg(hdev, usb_rcvctrlpipe(hdev, 0),
+		ret = usb_control_msg_recv(hdev, 0,
 			USB_REQ_GET_STATUS, USB_DIR_IN | USB_RT_PORT, 0, i,
-			port_status, sizeof(*port_status), 1000);
-		if (ret < 4)
+			port_status, sizeof(*port_status), 1000, GFP_KERNEL);
+		if (ret < 0)
 			continue;
 
 		portchange = le16_to_cpu(port_status->wPortChange);
@@ -420,13 +414,13 @@ static int lvs_rh_probe(struct usb_interface *intf,
 	usb_set_intfdata(intf, lvs);
 
 	/* how many number of ports this root hub has */
-	ret = usb_control_msg(hdev, usb_rcvctrlpipe(hdev, 0),
+	ret = usb_control_msg_recv(hdev, 0,
 			USB_REQ_GET_DESCRIPTOR, USB_DIR_IN | USB_RT_HUB,
 			USB_DT_SS_HUB << 8, 0, &lvs->descriptor,
-			USB_DT_SS_HUB_SIZE, USB_CTRL_GET_TIMEOUT);
-	if (ret < (USB_DT_HUB_NONVAR_SIZE + 2)) {
+			USB_DT_SS_HUB_SIZE, USB_CTRL_GET_TIMEOUT, GFP_KERNEL);
+	if (ret < 0) {
 		dev_err(&hdev->dev, "wrong root hub descriptor read %d\n", ret);
-		return ret < 0 ? ret : -EINVAL;
+		return ret;
 	}
 
 	/* submit urb to poll interrupt endpoint */
-- 
2.25.1

[PATCH v2 13/15] usb: misc: trancevibrator: update to use usb_control_msg_send()

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, the instance of usb_control_msg() has been replaced with
usb_control_msg_send() and the return value checking condition has also
been modified appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/trancevibrator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/misc/trancevibrator.c b/drivers/usb/misc/trancevibrator.c
index a3dfc77578ea..c50807b4f4ef 100644
--- a/drivers/usb/misc/trancevibrator.c
+++ b/drivers/usb/misc/trancevibrator.c
@@ -59,11 +59,11 @@ static ssize_t speed_store(struct device *dev, struct device_attribute *attr,
 	dev_dbg(&tv->udev->dev, "speed = %d\n", tv->speed);
 
 	/* Set speed */
-	retval = usb_control_msg(tv->udev, usb_sndctrlpipe(tv->udev, 0),
+	retval = usb_control_msg_send(tv->udev, 0,
 				 0x01, /* vendor request: set speed */
 				 USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
 				 tv->speed, /* speed value */
-				 0, NULL, 0, USB_CTRL_GET_TIMEOUT);
+				 0, NULL, 0, USB_CTRL_GET_TIMEOUT, GFP_KERNEL);
 	if (retval) {
 		tv->speed = old;
 		dev_dbg(&tv->udev->dev, "retval = %d\n", retval);
-- 
2.25.1

[PATCH v2 14/15] usb: misc: usbsevseg: update to use usb_control_msg_send()

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, instances of usb_control_msg() have been replaced with
usb_control_msg_send() appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/usbsevseg.c | 60 ++++++++++--------------------------
 1 file changed, 17 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/misc/usbsevseg.c b/drivers/usb/misc/usbsevseg.c
index 551074f5b7ad..ade4bc58d5f2 100644
--- a/drivers/usb/misc/usbsevseg.c
+++ b/drivers/usb/misc/usbsevseg.c
@@ -74,15 +74,10 @@ static void update_display_powered(struct usb_sevsegdev *mydev)
 	if (mydev->shadow_power != 1)
 		return;
 
-	rc = usb_control_msg(mydev->udev,
-			usb_sndctrlpipe(mydev->udev, 0),
-			0x12,
-			0x48,
-			(80 * 0x100) + 10, /*  (power mode) */
-			(0x00 * 0x100) + (mydev->powered ? 1 : 0),
-			NULL,
-			0,
-			2000);
+	rc = usb_control_msg_send(mydev->udev, 0, 0x12, 0x48,
+				  (80 * 0x100) + 10, /*  (power mode) */
+				  (0x00 * 0x100) + (mydev->powered ? 1 : 0),
+				  NULL, 0, 2000, GFP_KERNEL);
 	if (rc < 0)
 		dev_dbg(&mydev->udev->dev, "power retval = %d\n", rc);
 
@@ -99,15 +94,10 @@ static void update_display_mode(struct usb_sevsegdev *mydev)
 	if(mydev->shadow_power != 1)
 		return;
 
-	rc = usb_control_msg(mydev->udev,
-			usb_sndctrlpipe(mydev->udev, 0),
-			0x12,
-			0x48,
-			(82 * 0x100) + 10, /* (set mode) */
-			(mydev->mode_msb * 0x100) + mydev->mode_lsb,
-			NULL,
-			0,
-			2000);
+	rc = usb_control_msg_send(mydev->udev, 0, 0x12, 0x48,
+				  (82 * 0x100) + 10, /* (set mode) */
+				  (mydev->mode_msb * 0x100) + mydev->mode_lsb,
+				  NULL, 0, 2000, GFP_KERNEL);
 
 	if (rc < 0)
 		dev_dbg(&mydev->udev->dev, "mode retval = %d\n", rc);
@@ -117,48 +107,32 @@ static void update_display_visual(struct usb_sevsegdev *mydev, gfp_t mf)
 {
 	int rc;
 	int i;
-	unsigned char *buffer;
+	unsigned char buffer[MAXLEN] = {0};
 	u8 decimals = 0;
 
 	if(mydev->shadow_power != 1)
 		return;
 
-	buffer = kzalloc(MAXLEN, mf);
-	if (!buffer)
-		return;
-
 	/* The device is right to left, where as you write left to right */
 	for (i = 0; i < mydev->textlength; i++)
 		buffer[i] = mydev->text[mydev->textlength-1-i];
 
-	rc = usb_control_msg(mydev->udev,
-			usb_sndctrlpipe(mydev->udev, 0),
-			0x12,
-			0x48,
-			(85 * 0x100) + 10, /* (write text) */
-			(0 * 0x100) + mydev->textmode, /* mode  */
-			buffer,
-			mydev->textlength,
-			2000);
+	rc = usb_control_msg_send(mydev->udev, 0, 0x12, 0x48,
+				  (85 * 0x100) + 10, /* (write text) */
+				  (0 * 0x100) + mydev->textmode, /* mode  */
+				  &buffer, mydev->textlength, 2000, mf);
 
 	if (rc < 0)
 		dev_dbg(&mydev->udev->dev, "write retval = %d\n", rc);
 
-	kfree(buffer);
-
 	/* The device is right to left, where as you write left to right */
 	for (i = 0; i < sizeof(mydev->decimals); i++)
 		decimals |= mydev->decimals[i] << i;
 
-	rc = usb_control_msg(mydev->udev,
-			usb_sndctrlpipe(mydev->udev, 0),
-			0x12,
-			0x48,
-			(86 * 0x100) + 10, /* (set decimal) */
-			(0 * 0x100) + decimals, /* decimals */
-			NULL,
-			0,
-			2000);
+	rc = usb_control_msg_send(mydev->udev, 0, 0x12, 0x48,
+				  (86 * 0x100) + 10, /* (set decimal) */
+				  (0 * 0x100) + decimals, /* decimals */
+				  NULL, 0, 2000, mf);
 
 	if (rc < 0)
 		dev_dbg(&mydev->udev->dev, "decimal retval = %d\n", rc);
-- 
2.25.1

[PATCH v2 15/15] usb: misc: usbtest: update to use the usb_control_msg_{send|recv}() API

[Anant Thazhemadam]

The newer usb_control_msg_{send|recv}() API are an improvement on the
existing usb_control_msg() as it ensures that a short read/write is treated
as an error, data can be used off the stack, and raw usb pipes need not be
created in the calling functions.
For this reason, instances of usb_control_msg() have been replaced with
usb_control_msg_{recv|send}() and the return value checking conditions have
also been modified appropriately.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
 drivers/usb/misc/usbtest.c | 69 ++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 150090ee4ec1..4337eff2a749 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -672,19 +672,15 @@ static int get_altsetting(struct usbtest_dev *dev)
 	struct usb_device	*udev = interface_to_usbdev(iface);
 	int			retval;
 
-	retval = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
-			USB_REQ_GET_INTERFACE, USB_DIR_IN|USB_RECIP_INTERFACE,
-			0, iface->altsetting[0].desc.bInterfaceNumber,
-			dev->buf, 1, USB_CTRL_GET_TIMEOUT);
-	switch (retval) {
-	case 1:
-		return dev->buf[0];
-	case 0:
-		retval = -ERANGE;
-		fallthrough;
-	default:
+	retval = usb_control_msg_recv(udev, 0, USB_REQ_GET_INTERFACE,
+				      USB_DIR_IN|USB_RECIP_INTERFACE,
+				      0, iface->altsetting[0].desc.bInterfaceNumber,
+				      dev->buf, 1, USB_CTRL_GET_TIMEOUT, GFP_KERNEL);
+
+	if (retval < 0)
 		return retval;
-	}
+
+	return dev->buf[0];
 }
 
 static int set_altsetting(struct usbtest_dev *dev, int alternate)
@@ -872,14 +868,15 @@ static int ch9_postconfig(struct usbtest_dev *dev)
 		 * ... although some cheap devices (like one TI Hub I've got)
 		 * won't return config descriptors except before set_config.
 		 */
-		retval = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
-				USB_REQ_GET_CONFIGURATION,
-				USB_DIR_IN | USB_RECIP_DEVICE,
-				0, 0, dev->buf, 1, USB_CTRL_GET_TIMEOUT);
-		if (retval != 1 || dev->buf[0] != expected) {
+		retval = usb_control_msg_recv(udev, 0, USB_REQ_GET_CONFIGURATION,
+					      USB_DIR_IN | USB_RECIP_DEVICE,  0,
+					      0, dev->buf, 1, USB_CTRL_GET_TIMEOUT,
+					      GFP_KERNEL);
+
+		if (retval != 0 || dev->buf[0] != expected) {
 			dev_err(&iface->dev, "get config --> %d %d (1 %d)\n",
 				retval, dev->buf[0], expected);
-			return (retval < 0) ? retval : -EDOM;
+			return retval;
 		}
 	}
 
@@ -1683,10 +1680,10 @@ static int test_halt(struct usbtest_dev *tdev, int ep, struct urb *urb)
 		return retval;
 
 	/* set halt (protocol test only), verify it worked */
-	retval = usb_control_msg(urb->dev, usb_sndctrlpipe(urb->dev, 0),
-			USB_REQ_SET_FEATURE, USB_RECIP_ENDPOINT,
-			USB_ENDPOINT_HALT, ep,
-			NULL, 0, USB_CTRL_SET_TIMEOUT);
+	retval = usb_control_msg_send(urb->dev, 0, USB_REQ_SET_FEATURE,
+				      USB_RECIP_ENDPOINT, USB_ENDPOINT_HALT,
+				      ep, NULL, 0, USB_CTRL_SET_TIMEOUT,
+				      GFP_KERNEL);
 	if (retval < 0) {
 		ERROR(tdev, "ep %02x couldn't set halt, %d\n", ep, retval);
 		return retval;
@@ -1845,30 +1842,22 @@ static int ctrl_out(struct usbtest_dev *dev,
 		/* write patterned data */
 		for (j = 0; j < len; j++)
 			buf[j] = (u8)(i + j);
-		retval = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-				0x5b, USB_DIR_OUT|USB_TYPE_VENDOR,
-				0, 0, buf, len, USB_CTRL_SET_TIMEOUT);
-		if (retval != len) {
+		retval = usb_control_msg_send(udev, 0, 0x5b,
+					      USB_DIR_OUT | USB_TYPE_VENDOR, 0,
+					      0, buf, len, USB_CTRL_SET_TIMEOUT,
+					      GFP_KERNEL);
+		if (retval < 0) {
 			what = "write";
-			if (retval >= 0) {
-				ERROR(dev, "ctrl_out, wlen %d (expected %d)\n",
-						retval, len);
-				retval = -EBADMSG;
-			}
 			break;
 		}
 
 		/* read it back -- assuming nothing intervened!!  */
-		retval = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
-				0x5c, USB_DIR_IN|USB_TYPE_VENDOR,
-				0, 0, buf, len, USB_CTRL_GET_TIMEOUT);
-		if (retval != len) {
+		retval = usb_control_msg_recv(udev, 0,
+					      0x5c, USB_DIR_IN|USB_TYPE_VENDOR,
+					      0, 0, buf, len, USB_CTRL_GET_TIMEOUT,
+					      GFP_KERNEL);
+		if (retval < 0) {
 			what = "read";
-			if (retval >= 0) {
-				ERROR(dev, "ctrl_out, rlen %d (expected %d)\n",
-						retval, len);
-				retval = -EBADMSG;
-			}
 			break;
 		}
 
-- 
2.25.1

Re: [PATCH v2 06/15] usb: misc: emi62: update to use usb_control_msg_send()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 13522 bytes --]

Hi Anant,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on balbi-usb/testing/next peter.chen-usb/ci-for-usb-next v5.10-rc6 next-20201127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anant-Thazhemadam/drivers-usb-misc-update-to-use-usb_control_msg_-send-recv/20201130-093816
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: h8300-randconfig-s032-20201130 (attached as .config)
compiler: h8300-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-170-g3bc348f6-dirty
        # https://github.com/0day-ci/linux/commit/a9e2333efa48de6856185ec35c82b659ff1c1215
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Anant-Thazhemadam/drivers-usb-misc-update-to-use-usb_control_msg_-send-recv/20201130-093816
        git checkout a9e2333efa48de6856185ec35c82b659ff1c1215
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=h8300 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/usb/misc/emi62.c: In function 'emi62_load_firmware':
>> drivers/usb/misc/emi62.c:213:1: warning: the frame size of 1048 bytes is larger than 1024 bytes [-Wframe-larger-than=]
     213 | }
         | ^

vim +213 drivers/usb/misc/emi62.c

^1da177e4c3f415 Linus Torvalds     2005-04-16   68  
^1da177e4c3f415 Linus Torvalds     2005-04-16   69  static int emi62_load_firmware (struct usb_device *dev)
^1da177e4c3f415 Linus Torvalds     2005-04-16   70  {
b8e24bfabb03527 David Woodhouse    2008-05-30   71  	const struct firmware *loader_fw = NULL;
b8e24bfabb03527 David Woodhouse    2008-05-30   72  	const struct firmware *bitstream_fw = NULL;
b8e24bfabb03527 David Woodhouse    2008-05-30   73  	const struct firmware *firmware_fw = NULL;
b8e24bfabb03527 David Woodhouse    2008-05-30   74  	const struct ihex_binrec *rec;
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20   75  	int err = -ENOMEM;
^1da177e4c3f415 Linus Torvalds     2005-04-16   76  	int i;
^1da177e4c3f415 Linus Torvalds     2005-04-16   77  	__u32 addr;	/* Address to write */
a9e2333efa48de6 Anant Thazhemadam  2020-11-30   78  	__u8 buf[FW_LOAD_SIZE];
^1da177e4c3f415 Linus Torvalds     2005-04-16   79  
^1da177e4c3f415 Linus Torvalds     2005-04-16   80  	dev_dbg(&dev->dev, "load_firmware\n");
^1da177e4c3f415 Linus Torvalds     2005-04-16   81  
b8e24bfabb03527 David Woodhouse    2008-05-30   82  	err = request_ihex_firmware(&loader_fw, "emi62/loader.fw", &dev->dev);
b8e24bfabb03527 David Woodhouse    2008-05-30   83  	if (err)
b8e24bfabb03527 David Woodhouse    2008-05-30   84  		goto nofw;
b8e24bfabb03527 David Woodhouse    2008-05-30   85  
b8e24bfabb03527 David Woodhouse    2008-05-30   86  	err = request_ihex_firmware(&bitstream_fw, "emi62/bitstream.fw",
b8e24bfabb03527 David Woodhouse    2008-05-30   87  				    &dev->dev);
b8e24bfabb03527 David Woodhouse    2008-05-30   88  	if (err)
b8e24bfabb03527 David Woodhouse    2008-05-30   89  		goto nofw;
b8e24bfabb03527 David Woodhouse    2008-05-30   90  
b8e24bfabb03527 David Woodhouse    2008-05-30   91  	err = request_ihex_firmware(&firmware_fw, FIRMWARE_FW, &dev->dev);
b8e24bfabb03527 David Woodhouse    2008-05-30   92  	if (err) {
b8e24bfabb03527 David Woodhouse    2008-05-30   93  	nofw:
b8e24bfabb03527 David Woodhouse    2008-05-30   94  		goto wraperr;
b8e24bfabb03527 David Woodhouse    2008-05-30   95  	}
b8e24bfabb03527 David Woodhouse    2008-05-30   96  
^1da177e4c3f415 Linus Torvalds     2005-04-16   97  	/* Assert reset (stop the CPU in the EMI) */
^1da177e4c3f415 Linus Torvalds     2005-04-16   98  	err = emi62_set_reset(dev,1);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20   99  	if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  100  		goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  101  
b8e24bfabb03527 David Woodhouse    2008-05-30  102  	rec = (const struct ihex_binrec *)loader_fw->data;
b8e24bfabb03527 David Woodhouse    2008-05-30  103  
^1da177e4c3f415 Linus Torvalds     2005-04-16  104  	/* 1. We need to put the loader for the FPGA into the EZ-USB */
b8e24bfabb03527 David Woodhouse    2008-05-30  105  	while (rec) {
b8e24bfabb03527 David Woodhouse    2008-05-30  106  		err = emi62_writememory(dev, be32_to_cpu(rec->addr),
b8e24bfabb03527 David Woodhouse    2008-05-30  107  					rec->data, be16_to_cpu(rec->len),
b8e24bfabb03527 David Woodhouse    2008-05-30  108  					ANCHOR_LOAD_INTERNAL);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  109  		if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  110  			goto wraperr;
b8e24bfabb03527 David Woodhouse    2008-05-30  111  		rec = ihex_next_binrec(rec);
^1da177e4c3f415 Linus Torvalds     2005-04-16  112  	}
^1da177e4c3f415 Linus Torvalds     2005-04-16  113  
^1da177e4c3f415 Linus Torvalds     2005-04-16  114  	/* De-assert reset (let the CPU run) */
^1da177e4c3f415 Linus Torvalds     2005-04-16  115  	err = emi62_set_reset(dev,0);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  116  	if (err < 0)
5919a43bbc649f4 Oliver Neukum      2007-10-25  117  		goto wraperr;
16c23f7d88cbcce Monty              2006-05-09  118  	msleep(250);	/* let device settle */
^1da177e4c3f415 Linus Torvalds     2005-04-16  119  
^1da177e4c3f415 Linus Torvalds     2005-04-16  120  	/* 2. We upload the FPGA firmware into the EMI
^1da177e4c3f415 Linus Torvalds     2005-04-16  121  	 * Note: collect up to 1023 (yes!) bytes and send them with
^1da177e4c3f415 Linus Torvalds     2005-04-16  122  	 * a single request. This is _much_ faster! */
b8e24bfabb03527 David Woodhouse    2008-05-30  123  	rec = (const struct ihex_binrec *)bitstream_fw->data;
^1da177e4c3f415 Linus Torvalds     2005-04-16  124  	do {
^1da177e4c3f415 Linus Torvalds     2005-04-16  125  		i = 0;
b8e24bfabb03527 David Woodhouse    2008-05-30  126  		addr = be32_to_cpu(rec->addr);
^1da177e4c3f415 Linus Torvalds     2005-04-16  127  
^1da177e4c3f415 Linus Torvalds     2005-04-16  128  		/* intel hex records are terminated with type 0 element */
b8e24bfabb03527 David Woodhouse    2008-05-30  129  		while (rec && (i + be16_to_cpu(rec->len) < FW_LOAD_SIZE)) {
a9e2333efa48de6 Anant Thazhemadam  2020-11-30  130  			memcpy(&buf[i], rec->data, be16_to_cpu(rec->len));
b8e24bfabb03527 David Woodhouse    2008-05-30  131  			i += be16_to_cpu(rec->len);
b8e24bfabb03527 David Woodhouse    2008-05-30  132  			rec = ihex_next_binrec(rec);
^1da177e4c3f415 Linus Torvalds     2005-04-16  133  		}
a9e2333efa48de6 Anant Thazhemadam  2020-11-30  134  		err = emi62_writememory(dev, addr, &buf, i, ANCHOR_LOAD_FPGA);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  135  		if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  136  			goto wraperr;
ac06c06770bb876 Clemens Ladisch    2009-12-21  137  	} while (rec);
^1da177e4c3f415 Linus Torvalds     2005-04-16  138  
^1da177e4c3f415 Linus Torvalds     2005-04-16  139  	/* Assert reset (stop the CPU in the EMI) */
^1da177e4c3f415 Linus Torvalds     2005-04-16  140  	err = emi62_set_reset(dev,1);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  141  	if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  142  		goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  143  
^1da177e4c3f415 Linus Torvalds     2005-04-16  144  	/* 3. We need to put the loader for the firmware into the EZ-USB (again...) */
b8e24bfabb03527 David Woodhouse    2008-05-30  145  	for (rec = (const struct ihex_binrec *)loader_fw->data;
b8e24bfabb03527 David Woodhouse    2008-05-30  146  	     rec; rec = ihex_next_binrec(rec)) {
b8e24bfabb03527 David Woodhouse    2008-05-30  147  		err = emi62_writememory(dev, be32_to_cpu(rec->addr),
b8e24bfabb03527 David Woodhouse    2008-05-30  148  					rec->data, be16_to_cpu(rec->len),
b8e24bfabb03527 David Woodhouse    2008-05-30  149  					ANCHOR_LOAD_INTERNAL);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  150  		if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  151  			goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  152  	}
^1da177e4c3f415 Linus Torvalds     2005-04-16  153  
^1da177e4c3f415 Linus Torvalds     2005-04-16  154  	/* De-assert reset (let the CPU run) */
^1da177e4c3f415 Linus Torvalds     2005-04-16  155  	err = emi62_set_reset(dev,0);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  156  	if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  157  		goto wraperr;
16c23f7d88cbcce Monty              2006-05-09  158  	msleep(250);	/* let device settle */
^1da177e4c3f415 Linus Torvalds     2005-04-16  159  
^1da177e4c3f415 Linus Torvalds     2005-04-16  160  	/* 4. We put the part of the firmware that lies in the external RAM into the EZ-USB */
^1da177e4c3f415 Linus Torvalds     2005-04-16  161  
b8e24bfabb03527 David Woodhouse    2008-05-30  162  	for (rec = (const struct ihex_binrec *)firmware_fw->data;
b8e24bfabb03527 David Woodhouse    2008-05-30  163  	     rec; rec = ihex_next_binrec(rec)) {
b8e24bfabb03527 David Woodhouse    2008-05-30  164  		if (!INTERNAL_RAM(be32_to_cpu(rec->addr))) {
b8e24bfabb03527 David Woodhouse    2008-05-30  165  			err = emi62_writememory(dev, be32_to_cpu(rec->addr),
b8e24bfabb03527 David Woodhouse    2008-05-30  166  						rec->data, be16_to_cpu(rec->len),
b8e24bfabb03527 David Woodhouse    2008-05-30  167  						ANCHOR_LOAD_EXTERNAL);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  168  			if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  169  				goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  170  		}
^1da177e4c3f415 Linus Torvalds     2005-04-16  171  	}
b8e24bfabb03527 David Woodhouse    2008-05-30  172  
^1da177e4c3f415 Linus Torvalds     2005-04-16  173  	/* Assert reset (stop the CPU in the EMI) */
^1da177e4c3f415 Linus Torvalds     2005-04-16  174  	err = emi62_set_reset(dev,1);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  175  	if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  176  		goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  177  
b8e24bfabb03527 David Woodhouse    2008-05-30  178  	for (rec = (const struct ihex_binrec *)firmware_fw->data;
b8e24bfabb03527 David Woodhouse    2008-05-30  179  	     rec; rec = ihex_next_binrec(rec)) {
b8e24bfabb03527 David Woodhouse    2008-05-30  180  		if (INTERNAL_RAM(be32_to_cpu(rec->addr))) {
b8e24bfabb03527 David Woodhouse    2008-05-30  181  			err = emi62_writememory(dev, be32_to_cpu(rec->addr),
b8e24bfabb03527 David Woodhouse    2008-05-30  182  						rec->data, be16_to_cpu(rec->len),
b8e24bfabb03527 David Woodhouse    2008-05-30  183  						ANCHOR_LOAD_EXTERNAL);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  184  			if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  185  				goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  186  		}
^1da177e4c3f415 Linus Torvalds     2005-04-16  187  	}
^1da177e4c3f415 Linus Torvalds     2005-04-16  188  
^1da177e4c3f415 Linus Torvalds     2005-04-16  189  	/* De-assert reset (let the CPU run) */
^1da177e4c3f415 Linus Torvalds     2005-04-16  190  	err = emi62_set_reset(dev,0);
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  191  	if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  192  		goto wraperr;
16c23f7d88cbcce Monty              2006-05-09  193  	msleep(250);	/* let device settle */
^1da177e4c3f415 Linus Torvalds     2005-04-16  194  
b8e24bfabb03527 David Woodhouse    2008-05-30  195  	release_firmware(loader_fw);
b8e24bfabb03527 David Woodhouse    2008-05-30  196  	release_firmware(bitstream_fw);
b8e24bfabb03527 David Woodhouse    2008-05-30  197  	release_firmware(firmware_fw);
b8e24bfabb03527 David Woodhouse    2008-05-30  198  
^1da177e4c3f415 Linus Torvalds     2005-04-16  199  	/* return 1 to fail the driver inialization
^1da177e4c3f415 Linus Torvalds     2005-04-16  200  	 * and give real driver change to load */
^1da177e4c3f415 Linus Torvalds     2005-04-16  201  	return 1;
^1da177e4c3f415 Linus Torvalds     2005-04-16  202  
^1da177e4c3f415 Linus Torvalds     2005-04-16  203  wraperr:
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  204  	if (err < 0)
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  205  		dev_err(&dev->dev,"%s - error loading firmware: error = %d\n",
e9a527dae346c0a Greg Kroah-Hartman 2012-04-20  206  			__func__, err);
b8e24bfabb03527 David Woodhouse    2008-05-30  207  	release_firmware(loader_fw);
b8e24bfabb03527 David Woodhouse    2008-05-30  208  	release_firmware(bitstream_fw);
b8e24bfabb03527 David Woodhouse    2008-05-30  209  	release_firmware(firmware_fw);
b8e24bfabb03527 David Woodhouse    2008-05-30  210  
^1da177e4c3f415 Linus Torvalds     2005-04-16  211  	dev_err(&dev->dev, "Error\n");
^1da177e4c3f415 Linus Torvalds     2005-04-16  212  	return err;
^1da177e4c3f415 Linus Torvalds     2005-04-16 @213  }
^1da177e4c3f415 Linus Torvalds     2005-04-16  214  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30287 bytes --]

Re: [PATCH v2 05/15] usb: misc: emi26: update to use usb_control_msg_send()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 13104 bytes --]

Hi Anant,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on balbi-usb/testing/next peter.chen-usb/ci-for-usb-next v5.10-rc6 next-20201127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anant-Thazhemadam/drivers-usb-misc-update-to-use-usb_control_msg_-send-recv/20201130-093816
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/bd85eb79b555200026380c4f93e83c4a667564e5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Anant-Thazhemadam/drivers-usb-misc-update-to-use-usb_control_msg_-send-recv/20201130-093816
        git checkout bd85eb79b555200026380c4f93e83c4a667564e5
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/usb/misc/emi26.c: In function 'emi26_load_firmware':
>> drivers/usb/misc/emi26.c:201:1: warning: the frame size of 1040 bytes is larger than 1024 bytes [-Wframe-larger-than=]
     201 | }
         | ^

vim +201 drivers/usb/misc/emi26.c

^1da177e4c3f415 Linus Torvalds     2005-04-16   60  
^1da177e4c3f415 Linus Torvalds     2005-04-16   61  static int emi26_load_firmware (struct usb_device *dev)
^1da177e4c3f415 Linus Torvalds     2005-04-16   62  {
ae93a55bf948753 David Woodhouse    2008-05-30   63  	const struct firmware *loader_fw = NULL;
ae93a55bf948753 David Woodhouse    2008-05-30   64  	const struct firmware *bitstream_fw = NULL;
ae93a55bf948753 David Woodhouse    2008-05-30   65  	const struct firmware *firmware_fw = NULL;
ae93a55bf948753 David Woodhouse    2008-05-30   66  	const struct ihex_binrec *rec;
b412284b9698456 Greg Kroah-Hartman 2012-04-20   67  	int err = -ENOMEM;
^1da177e4c3f415 Linus Torvalds     2005-04-16   68  	int i;
^1da177e4c3f415 Linus Torvalds     2005-04-16   69  	__u32 addr;	/* Address to write */
bd85eb79b555200 Anant Thazhemadam  2020-11-30   70  	__u8 buf[FW_LOAD_SIZE];
^1da177e4c3f415 Linus Torvalds     2005-04-16   71  
ae93a55bf948753 David Woodhouse    2008-05-30   72  	err = request_ihex_firmware(&loader_fw, "emi26/loader.fw", &dev->dev);
ae93a55bf948753 David Woodhouse    2008-05-30   73  	if (err)
ae93a55bf948753 David Woodhouse    2008-05-30   74  		goto nofw;
ae93a55bf948753 David Woodhouse    2008-05-30   75  
ae93a55bf948753 David Woodhouse    2008-05-30   76  	err = request_ihex_firmware(&bitstream_fw, "emi26/bitstream.fw",
ae93a55bf948753 David Woodhouse    2008-05-30   77  				    &dev->dev);
ae93a55bf948753 David Woodhouse    2008-05-30   78  	if (err)
ae93a55bf948753 David Woodhouse    2008-05-30   79  		goto nofw;
ae93a55bf948753 David Woodhouse    2008-05-30   80  
ae93a55bf948753 David Woodhouse    2008-05-30   81  	err = request_ihex_firmware(&firmware_fw, "emi26/firmware.fw",
ae93a55bf948753 David Woodhouse    2008-05-30   82  				    &dev->dev);
ae93a55bf948753 David Woodhouse    2008-05-30   83  	if (err) {
ae93a55bf948753 David Woodhouse    2008-05-30   84  	nofw:
fd3f1917e345d85 Greg Kroah-Hartman 2008-08-14   85  		dev_err(&dev->dev, "%s - request_firmware() failed\n",
fd3f1917e345d85 Greg Kroah-Hartman 2008-08-14   86  			__func__);
ae93a55bf948753 David Woodhouse    2008-05-30   87  		goto wraperr;
ae93a55bf948753 David Woodhouse    2008-05-30   88  	}
ae93a55bf948753 David Woodhouse    2008-05-30   89  
^1da177e4c3f415 Linus Torvalds     2005-04-16   90  	/* Assert reset (stop the CPU in the EMI) */
^1da177e4c3f415 Linus Torvalds     2005-04-16   91  	err = emi26_set_reset(dev,1);
b412284b9698456 Greg Kroah-Hartman 2012-04-20   92  	if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16   93  		goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16   94  
ae93a55bf948753 David Woodhouse    2008-05-30   95  	rec = (const struct ihex_binrec *)loader_fw->data;
^1da177e4c3f415 Linus Torvalds     2005-04-16   96  	/* 1. We need to put the loader for the FPGA into the EZ-USB */
ae93a55bf948753 David Woodhouse    2008-05-30   97  	while (rec) {
ae93a55bf948753 David Woodhouse    2008-05-30   98  		err = emi26_writememory(dev, be32_to_cpu(rec->addr),
ae93a55bf948753 David Woodhouse    2008-05-30   99  					rec->data, be16_to_cpu(rec->len),
ae93a55bf948753 David Woodhouse    2008-05-30  100  					ANCHOR_LOAD_INTERNAL);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  101  		if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  102  			goto wraperr;
ae93a55bf948753 David Woodhouse    2008-05-30  103  		rec = ihex_next_binrec(rec);
^1da177e4c3f415 Linus Torvalds     2005-04-16  104  	}
^1da177e4c3f415 Linus Torvalds     2005-04-16  105  
^1da177e4c3f415 Linus Torvalds     2005-04-16  106  	/* De-assert reset (let the CPU run) */
^1da177e4c3f415 Linus Torvalds     2005-04-16  107  	err = emi26_set_reset(dev,0);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  108  	if (err < 0)
cf4cf0bb89cbff9 Oliver Neukum      2007-10-25  109  		goto wraperr;
16c23f7d88cbcce Monty              2006-05-09  110  	msleep(250);	/* let device settle */
^1da177e4c3f415 Linus Torvalds     2005-04-16  111  
^1da177e4c3f415 Linus Torvalds     2005-04-16  112  	/* 2. We upload the FPGA firmware into the EMI
^1da177e4c3f415 Linus Torvalds     2005-04-16  113  	 * Note: collect up to 1023 (yes!) bytes and send them with
^1da177e4c3f415 Linus Torvalds     2005-04-16  114  	 * a single request. This is _much_ faster! */
ae93a55bf948753 David Woodhouse    2008-05-30  115  	rec = (const struct ihex_binrec *)bitstream_fw->data;
^1da177e4c3f415 Linus Torvalds     2005-04-16  116  	do {
^1da177e4c3f415 Linus Torvalds     2005-04-16  117  		i = 0;
ae93a55bf948753 David Woodhouse    2008-05-30  118  		addr = be32_to_cpu(rec->addr);
^1da177e4c3f415 Linus Torvalds     2005-04-16  119  
^1da177e4c3f415 Linus Torvalds     2005-04-16  120  		/* intel hex records are terminated with type 0 element */
ae93a55bf948753 David Woodhouse    2008-05-30  121  		while (rec && (i + be16_to_cpu(rec->len) < FW_LOAD_SIZE)) {
bd85eb79b555200 Anant Thazhemadam  2020-11-30  122  			memcpy(&buf[i], rec->data, be16_to_cpu(rec->len));
ae93a55bf948753 David Woodhouse    2008-05-30  123  			i += be16_to_cpu(rec->len);
ae93a55bf948753 David Woodhouse    2008-05-30  124  			rec = ihex_next_binrec(rec);
^1da177e4c3f415 Linus Torvalds     2005-04-16  125  		}
bd85eb79b555200 Anant Thazhemadam  2020-11-30  126  		err = emi26_writememory(dev, addr, &buf, i, ANCHOR_LOAD_FPGA);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  127  		if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  128  			goto wraperr;
327d74f6b65ddc8 Marcin Slusarz     2009-01-04  129  	} while (rec);
^1da177e4c3f415 Linus Torvalds     2005-04-16  130  
^1da177e4c3f415 Linus Torvalds     2005-04-16  131  	/* Assert reset (stop the CPU in the EMI) */
^1da177e4c3f415 Linus Torvalds     2005-04-16  132  	err = emi26_set_reset(dev,1);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  133  	if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  134  		goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  135  
^1da177e4c3f415 Linus Torvalds     2005-04-16  136  	/* 3. We need to put the loader for the firmware into the EZ-USB (again...) */
ae93a55bf948753 David Woodhouse    2008-05-30  137  	for (rec = (const struct ihex_binrec *)loader_fw->data;
ae93a55bf948753 David Woodhouse    2008-05-30  138  	     rec; rec = ihex_next_binrec(rec)) {
ae93a55bf948753 David Woodhouse    2008-05-30  139  		err = emi26_writememory(dev, be32_to_cpu(rec->addr),
ae93a55bf948753 David Woodhouse    2008-05-30  140  					rec->data, be16_to_cpu(rec->len),
ae93a55bf948753 David Woodhouse    2008-05-30  141  					ANCHOR_LOAD_INTERNAL);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  142  		if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  143  			goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  144  	}
16c23f7d88cbcce Monty              2006-05-09  145  	msleep(250);	/* let device settle */
^1da177e4c3f415 Linus Torvalds     2005-04-16  146  
^1da177e4c3f415 Linus Torvalds     2005-04-16  147  	/* De-assert reset (let the CPU run) */
^1da177e4c3f415 Linus Torvalds     2005-04-16  148  	err = emi26_set_reset(dev,0);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  149  	if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  150  		goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  151  
^1da177e4c3f415 Linus Torvalds     2005-04-16  152  	/* 4. We put the part of the firmware that lies in the external RAM into the EZ-USB */
ae93a55bf948753 David Woodhouse    2008-05-30  153  
ae93a55bf948753 David Woodhouse    2008-05-30  154  	for (rec = (const struct ihex_binrec *)firmware_fw->data;
ae93a55bf948753 David Woodhouse    2008-05-30  155  	     rec; rec = ihex_next_binrec(rec)) {
ae93a55bf948753 David Woodhouse    2008-05-30  156  		if (!INTERNAL_RAM(be32_to_cpu(rec->addr))) {
ae93a55bf948753 David Woodhouse    2008-05-30  157  			err = emi26_writememory(dev, be32_to_cpu(rec->addr),
ae93a55bf948753 David Woodhouse    2008-05-30  158  						rec->data, be16_to_cpu(rec->len),
ae93a55bf948753 David Woodhouse    2008-05-30  159  						ANCHOR_LOAD_EXTERNAL);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  160  			if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  161  				goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  162  		}
^1da177e4c3f415 Linus Torvalds     2005-04-16  163  	}
^1da177e4c3f415 Linus Torvalds     2005-04-16  164  
^1da177e4c3f415 Linus Torvalds     2005-04-16  165  	/* Assert reset (stop the CPU in the EMI) */
^1da177e4c3f415 Linus Torvalds     2005-04-16  166  	err = emi26_set_reset(dev,1);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  167  	if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  168  		goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  169  
ae93a55bf948753 David Woodhouse    2008-05-30  170  	for (rec = (const struct ihex_binrec *)firmware_fw->data;
ae93a55bf948753 David Woodhouse    2008-05-30  171  	     rec; rec = ihex_next_binrec(rec)) {
ae93a55bf948753 David Woodhouse    2008-05-30  172  		if (INTERNAL_RAM(be32_to_cpu(rec->addr))) {
ae93a55bf948753 David Woodhouse    2008-05-30  173  			err = emi26_writememory(dev, be32_to_cpu(rec->addr),
ae93a55bf948753 David Woodhouse    2008-05-30  174  						rec->data, be16_to_cpu(rec->len),
ae93a55bf948753 David Woodhouse    2008-05-30  175  						ANCHOR_LOAD_INTERNAL);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  176  			if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  177  				goto wraperr;
^1da177e4c3f415 Linus Torvalds     2005-04-16  178  		}
^1da177e4c3f415 Linus Torvalds     2005-04-16  179  	}
^1da177e4c3f415 Linus Torvalds     2005-04-16  180  
^1da177e4c3f415 Linus Torvalds     2005-04-16  181  	/* De-assert reset (let the CPU run) */
^1da177e4c3f415 Linus Torvalds     2005-04-16  182  	err = emi26_set_reset(dev,0);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  183  	if (err < 0)
^1da177e4c3f415 Linus Torvalds     2005-04-16  184  		goto wraperr;
16c23f7d88cbcce Monty              2006-05-09  185  	msleep(250);	/* let device settle */
^1da177e4c3f415 Linus Torvalds     2005-04-16  186  
^1da177e4c3f415 Linus Torvalds     2005-04-16  187  	/* return 1 to fail the driver inialization
^1da177e4c3f415 Linus Torvalds     2005-04-16  188  	 * and give real driver change to load */
^1da177e4c3f415 Linus Torvalds     2005-04-16  189  	err = 1;
^1da177e4c3f415 Linus Torvalds     2005-04-16  190  
^1da177e4c3f415 Linus Torvalds     2005-04-16  191  wraperr:
b412284b9698456 Greg Kroah-Hartman 2012-04-20  192  	if (err < 0)
b412284b9698456 Greg Kroah-Hartman 2012-04-20  193  		dev_err(&dev->dev,"%s - error loading firmware: error = %d\n",
b412284b9698456 Greg Kroah-Hartman 2012-04-20  194  			__func__, err);
b412284b9698456 Greg Kroah-Hartman 2012-04-20  195  
ae93a55bf948753 David Woodhouse    2008-05-30  196  	release_firmware(loader_fw);
ae93a55bf948753 David Woodhouse    2008-05-30  197  	release_firmware(bitstream_fw);
ae93a55bf948753 David Woodhouse    2008-05-30  198  	release_firmware(firmware_fw);
ae93a55bf948753 David Woodhouse    2008-05-30  199  
^1da177e4c3f415 Linus Torvalds     2005-04-16  200  	return err;
^1da177e4c3f415 Linus Torvalds     2005-04-16 @201  }
^1da177e4c3f415 Linus Torvalds     2005-04-16  202  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58894 bytes --]

Re: [PATCH v2 05/15] usb: misc: emi26: update to use usb_control_msg_send()

[Johan Hovold]

On Mon, Nov 30, 2020 at 06:58:47AM +0530, Anant Thazhemadam wrote:
> The newer usb_control_msg_{send|recv}() API are an improvement on the
> existing usb_control_msg() as it ensures that a short read/write is treated
> as an error,

Short writes have always been treated as an error. The new send helper
only changes the return value from the transfer size to 0.

And this driver never reads.

Try to describe the motivation for changing this driver which is to
avoid the explicit kmemdup().

> data can be used off the stack, and raw usb pipes need not be
> created in the calling functions.
> For this reason, the instance of usb_control_msg() has been replaced with
> usb_control_msg_send() appropriately.
> 
> Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
> ---
>  drivers/usb/misc/emi26.c | 31 ++++++++-----------------------
>  1 file changed, 8 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/usb/misc/emi26.c b/drivers/usb/misc/emi26.c
> index 24d841850e05..1dd024507f40 100644
> --- a/drivers/usb/misc/emi26.c
> +++ b/drivers/usb/misc/emi26.c
> @@ -27,7 +27,7 @@
>  #define INTERNAL_RAM(address)   (address <= MAX_INTERNAL_ADDRESS)
>  
>  static int emi26_writememory( struct usb_device *dev, int address,
> -			      const unsigned char *data, int length,
> +			      const void *data, int length,

Why is this needed?

>  			      __u8 bRequest);
>  static int emi26_set_reset(struct usb_device *dev, unsigned char reset_bit);
>  static int emi26_load_firmware (struct usb_device *dev);
> @@ -35,22 +35,12 @@ static int emi26_probe(struct usb_interface *intf, const struct usb_device_id *i
>  static void emi26_disconnect(struct usb_interface *intf);
>  
>  /* thanks to drivers/usb/serial/keyspan_pda.c code */
> -static int emi26_writememory (struct usb_device *dev, int address,
> -			      const unsigned char *data, int length,
> +static int emi26_writememory(struct usb_device *dev, int address,
> +			      const void *data, int length,
>  			      __u8 request)
>  {
> -	int result;
> -	unsigned char *buffer =  kmemdup(data, length, GFP_KERNEL);
> -
> -	if (!buffer) {
> -		dev_err(&dev->dev, "kmalloc(%d) failed.\n", length);
> -		return -ENOMEM;
> -	}
> -	/* Note: usb_control_msg returns negative value on error or length of the
> -	 * 		 data that was written! */
> -	result = usb_control_msg (dev, usb_sndctrlpipe(dev, 0), request, 0x40, address, 0, buffer, length, 300);
> -	kfree (buffer);
> -	return result;
> +	return usb_control_msg_send(dev, 0, request, 0x40, address, 0,
> +				    data, length, 300, GFP_KERNEL);

So you're changing the return value on success from length to 0 here.
Did you make sure that all callers can handle that?

>  }
>  
>  /* thanks to drivers/usb/serial/keyspan_pda.c code */
> @@ -77,11 +67,7 @@ static int emi26_load_firmware (struct usb_device *dev)
>  	int err = -ENOMEM;
>  	int i;
>  	__u32 addr;	/* Address to write */
> -	__u8 *buf;
> -
> -	buf = kmalloc(FW_LOAD_SIZE, GFP_KERNEL);
> -	if (!buf)
> -		goto wraperr;
> +	__u8 buf[FW_LOAD_SIZE];

As the build bots reported, you must not put large structures like this
on the stack.

>  
>  	err = request_ihex_firmware(&loader_fw, "emi26/loader.fw", &dev->dev);
>  	if (err)
> @@ -133,11 +119,11 @@ static int emi26_load_firmware (struct usb_device *dev)
>  
>  		/* intel hex records are terminated with type 0 element */
>  		while (rec && (i + be16_to_cpu(rec->len) < FW_LOAD_SIZE)) {
> -			memcpy(buf + i, rec->data, be16_to_cpu(rec->len));
> +			memcpy(&buf[i], rec->data, be16_to_cpu(rec->len));
>  			i += be16_to_cpu(rec->len);
>  			rec = ihex_next_binrec(rec);
>  		}
> -		err = emi26_writememory(dev, addr, buf, i, ANCHOR_LOAD_FPGA);
> +		err = emi26_writememory(dev, addr, &buf, i, ANCHOR_LOAD_FPGA);
>  		if (err < 0)
>  			goto wraperr;
>  	} while (rec);
> @@ -211,7 +197,6 @@ static int emi26_load_firmware (struct usb_device *dev)
>  	release_firmware(bitstream_fw);
>  	release_firmware(firmware_fw);
>  
> -	kfree(buf);
>  	return err;
>  }

Looks good otherwise.

Johan

Re: [PATCH v2 06/15] usb: misc: emi62: update to use usb_control_msg_send()

[Johan Hovold]

On Mon, Nov 30, 2020 at 06:59:25AM +0530, Anant Thazhemadam wrote:
> The newer usb_control_msg_{send|recv}() API are an improvement on the
> existing usb_control_msg() as it ensures that a short read/write is treated
> as an error, data can be used off the stack, and raw usb pipes need not be
> created in the calling functions.
> For this reason, the instance of usb_control_msg() has been replaced with
> usb_control_msg_send() appropriately.
> 
> Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
> ---

This driver looks like it shares origins with the emi26 one so the same
comments apply (and especially not put large structures on the stack).

Johan

Re: [PATCH v2 08/15] usb: misc: idmouse: update to use usb_control_msg_send()

[Johan Hovold]

On Mon, Nov 30, 2020 at 07:00:31AM +0530, Anant Thazhemadam wrote:
> The newer usb_control_msg_{send|recv}() API are an improvement on the
> existing usb_control_msg() as it ensures that a short read/write is treated
> as an error, data can be used off the stack, and raw usb pipes need not be
> created in the calling functions.
> For this reason, the instance of usb_control_msg() has been replaced with
> usb_control_msg_send() appropriately.
> 
> Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
> ---
>  drivers/usb/misc/idmouse.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Especially for control transfers without a data stage there isn't
really any benefit of the new helper.

I'd just leave this one unchanged.

> diff --git a/drivers/usb/misc/idmouse.c b/drivers/usb/misc/idmouse.c
> index e9437a176518..52126441a633 100644
> --- a/drivers/usb/misc/idmouse.c
> +++ b/drivers/usb/misc/idmouse.c
> @@ -56,8 +56,9 @@ static const struct usb_device_id idmouse_table[] = {
>  #define FTIP_SCROLL  0x24
>  
>  #define ftip_command(dev, command, value, index) \
> -	usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0), command, \
> -	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, value, index, NULL, 0, 1000)
> +	usb_control_msg_send(dev->udev, 0, command, \
> +	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, \
> +	value, index, NULL, 0, 1000, GFP_KERNEL)
>  
>  MODULE_DEVICE_TABLE(usb, idmouse_table);

Johan

Re: [PATCH v2 08/15] usb: misc: idmouse: update to use usb_control_msg_send()

[Anant Thazhemadam]

On 04/12/20 8:16 pm, Johan Hovold wrote:
> On Mon, Nov 30, 2020 at 07:00:31AM +0530, Anant Thazhemadam wrote:
>> The newer usb_control_msg_{send|recv}() API are an improvement on the
>> existing usb_control_msg() as it ensures that a short read/write is treated
>> as an error, data can be used off the stack, and raw usb pipes need not be
>> created in the calling functions.
>> For this reason, the instance of usb_control_msg() has been replaced with
>> usb_control_msg_send() appropriately.
>>
>> Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
>> ---
>>  drivers/usb/misc/idmouse.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
> Especially for control transfers without a data stage there isn't
> really any benefit of the new helper.
>
> I'd just leave this one unchanged.
>
>> diff --git a/drivers/usb/misc/idmouse.c b/drivers/usb/misc/idmouse.c
>> index e9437a176518..52126441a633 100644
>> --- a/drivers/usb/misc/idmouse.c
>> +++ b/drivers/usb/misc/idmouse.c
>> @@ -56,8 +56,9 @@ static const struct usb_device_id idmouse_table[] = {
>>  #define FTIP_SCROLL  0x24
>>  
>>  #define ftip_command(dev, command, value, index) \
>> -	usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0), command, \
>> -	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, value, index, NULL, 0, 1000)
>> +	usb_control_msg_send(dev->udev, 0, command, \
>> +	USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, \
>> +	value, index, NULL, 0, 1000, GFP_KERNEL)
>>  
>>  MODULE_DEVICE_TABLE(usb, idmouse_table);
> Johan

Understood. I'll make sure this is left out in the v3.

Thanks,
Anant

Re: [PATCH v2 05/15] usb: misc: emi26: update to use usb_control_msg_send()

[Anant Thazhemadam]

On 04/12/20 8:11 pm, Johan Hovold wrote:
> On Mon, Nov 30, 2020 at 06:58:47AM +0530, Anant Thazhemadam wrote:
>> The newer usb_control_msg_{send|recv}() API are an improvement on the
>> existing usb_control_msg() as it ensures that a short read/write is treated
>> as an error,
> Short writes have always been treated as an error. The new send helper
> only changes the return value from the transfer size to 0.
>
> And this driver never reads.
>
> Try to describe the motivation for changing this driver which is to
> avoid the explicit kmemdup().
>
Thank you. I will try and put a more appropriate commit message.
>> data can be used off the stack, and raw usb pipes need not be
>> created in the calling functions.
>> For this reason, the instance of usb_control_msg() has been replaced with
>> usb_control_msg_send() appropriately.
>>
>> Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
>> ---
>>  drivers/usb/misc/emi26.c | 31 ++++++++-----------------------
>>  1 file changed, 8 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/usb/misc/emi26.c b/drivers/usb/misc/emi26.c
>> index 24d841850e05..1dd024507f40 100644
>> --- a/drivers/usb/misc/emi26.c
>> +++ b/drivers/usb/misc/emi26.c
>> @@ -27,7 +27,7 @@
>>  #define INTERNAL_RAM(address)   (address <= MAX_INTERNAL_ADDRESS)
>>  
>>  static int emi26_writememory( struct usb_device *dev, int address,
>> -			      const unsigned char *data, int length,
>> +			      const void *data, int length,
> Why is this needed?
>
>>  			      __u8 bRequest);
>>  static int emi26_set_reset(struct usb_device *dev, unsigned char reset_bit);
>>  static int emi26_load_firmware (struct usb_device *dev);
>> @@ -35,22 +35,12 @@ static int emi26_probe(struct usb_interface *intf, const struct usb_device_id *i
>>  static void emi26_disconnect(struct usb_interface *intf);
>>  
>>  /* thanks to drivers/usb/serial/keyspan_pda.c code */
>> -static int emi26_writememory (struct usb_device *dev, int address,
>> -			      const unsigned char *data, int length,
>> +static int emi26_writememory(struct usb_device *dev, int address,
>> +			      const void *data, int length,
>>  			      __u8 request)
>>  {
>> -	int result;
>> -	unsigned char *buffer =  kmemdup(data, length, GFP_KERNEL);
>> -
>> -	if (!buffer) {
>> -		dev_err(&dev->dev, "kmalloc(%d) failed.\n", length);
>> -		return -ENOMEM;
>> -	}
>> -	/* Note: usb_control_msg returns negative value on error or length of the
>> -	 * 		 data that was written! */
>> -	result = usb_control_msg (dev, usb_sndctrlpipe(dev, 0), request, 0x40, address, 0, buffer, length, 300);
>> -	kfree (buffer);
>> -	return result;
>> +	return usb_control_msg_send(dev, 0, request, 0x40, address, 0,
>> +				    data, length, 300, GFP_KERNEL);
> So you're changing the return value on success from length to 0 here.
> Did you make sure that all callers can handle that?

All the callers presently contain only an error checking condition for a return value < 0,
which still applies even with this change. So this wouldn't raise any issues.

>>  }
>>  
>>  /* thanks to drivers/usb/serial/keyspan_pda.c code */
>> @@ -77,11 +67,7 @@ static int emi26_load_firmware (struct usb_device *dev)
>>  	int err = -ENOMEM;
>>  	int i;
>>  	__u32 addr;	/* Address to write */
>> -	__u8 *buf;
>> -
>> -	buf = kmalloc(FW_LOAD_SIZE, GFP_KERNEL);
>> -	if (!buf)
>> -		goto wraperr;
>> +	__u8 buf[FW_LOAD_SIZE];
> As the build bots reported, you must not put large structures like this
> on the stack.

Understood. 
But I'm considering dropping this change (and the one proposed for emi62)
altogether in v3 - since these would end up requiring memory to dynamically allocated
twice for the same purpose.
However, if you still think the pros of updating this (and emi62) outweigh the cons,
please let me know, and I'll make sure to send in another version fixing it.


>>  
>>  	err = request_ihex_firmware(&loader_fw, "emi26/loader.fw", &dev->dev);
>>  	if (err)
>> @@ -133,11 +119,11 @@ static int emi26_load_firmware (struct usb_device *dev)
>>  
>>  		/* intel hex records are terminated with type 0 element */
>>  		while (rec && (i + be16_to_cpu(rec->len) < FW_LOAD_SIZE)) {
>> -			memcpy(buf + i, rec->data, be16_to_cpu(rec->len));
>> +			memcpy(&buf[i], rec->data, be16_to_cpu(rec->len));
>>  			i += be16_to_cpu(rec->len);
>>  			rec = ihex_next_binrec(rec);
>>  		}
>> -		err = emi26_writememory(dev, addr, buf, i, ANCHOR_LOAD_FPGA);
>> +		err = emi26_writememory(dev, addr, &buf, i, ANCHOR_LOAD_FPGA);
>>  		if (err < 0)
>>  			goto wraperr;
>>  	} while (rec);
>> @@ -211,7 +197,6 @@ static int emi26_load_firmware (struct usb_device *dev)
>>  	release_firmware(bitstream_fw);
>>  	release_firmware(firmware_fw);
>>  
>> -	kfree(buf);
>>  	return err;
>>  }
> Looks good otherwise.
>
> Johan

Thanks,
Anant

Re: [PATCH v2 05/15] usb: misc: emi26: update to use usb_control_msg_send()

[Johan Hovold]

On Thu, Jan 07, 2021 at 07:43:54PM +0530, Anant Thazhemadam wrote:
> On 04/12/20 8:11 pm, Johan Hovold wrote:
> > On Mon, Nov 30, 2020 at 06:58:47AM +0530, Anant Thazhemadam wrote:
> >> The newer usb_control_msg_{send|recv}() API are an improvement on the
> >> existing usb_control_msg() as it ensures that a short read/write is treated
> >> as an error,
> > Short writes have always been treated as an error. The new send helper
> > only changes the return value from the transfer size to 0.
> >
> > And this driver never reads.
> >
> > Try to describe the motivation for changing this driver which is to
> > avoid the explicit kmemdup().

> >>  /* thanks to drivers/usb/serial/keyspan_pda.c code */
> >> @@ -77,11 +67,7 @@ static int emi26_load_firmware (struct usb_device *dev)
> >>  	int err = -ENOMEM;
> >>  	int i;
> >>  	__u32 addr;	/* Address to write */
> >> -	__u8 *buf;
> >> -
> >> -	buf = kmalloc(FW_LOAD_SIZE, GFP_KERNEL);
> >> -	if (!buf)
> >> -		goto wraperr;
> >> +	__u8 buf[FW_LOAD_SIZE];
> > As the build bots reported, you must not put large structures like this
> > on the stack.
> 
> Understood. 
> But I'm considering dropping this change (and the one proposed for
> emi62) altogether in v3 - since these would end up requiring memory to
> dynamically allocated twice for the same purpose.  However, if you
> still think the pros of updating this (and emi62) outweigh the cons,
> please let me know, and I'll make sure to send in another version
> fixing it.

The redundant memdup() is already there for the firmware buffer and
changing to usb_control_msg_send() will only make it slightly harder to
get rid of that, if anyone would bother.

But yeah, it's probably not worth switching usb_control_msg_send() for
these drivers.

Johan