[RFC] change the new message to provide for memory allocations

[RFC] change the new message to provide for memory allocations

[Oliver Neukum]

Control messages have to work in situtaions where you need to
use GFP_NOIO. Hence wrappers for them that allocate memory must have
an API that allows for that.

Signed-off-by: Oliver Neukum <oneukum@suse.com>

[RFC 01/14] Revert "USB: core: hub.c: use usb_control_msg_send() in a few places"

[Oliver Neukum]

This reverts commit d6a499249543356002a1efbb26254c7272e62f4c.
Control messages are needed in contexts when memory allocations
are restricted, such as handling device resets and runtime PM.

For this reason the control message API internally uses GFP_NOIO.
This is a band aid introduced because when we recognized the issue,
the call chains were highly convoluted. Continuing this trend
is not a good idea.

So I am shooting the whole kennel here.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/core/hub.c | 99 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5742ddeb0455..5b768b80d1ee 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -410,8 +410,8 @@ static int get_hub_descriptor(struct usb_device *hdev,
  */
 static int clear_hub_feature(struct usb_device *hdev, int feature)
 {
-	return usb_control_msg_send(hdev, 0, USB_REQ_CLEAR_FEATURE, USB_RT_HUB,
-				    feature, 0, NULL, 0, 1000);
+	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+		USB_REQ_CLEAR_FEATURE, USB_RT_HUB, feature, 0, NULL, 0, 1000);
 }
 
 /*
@@ -419,8 +419,9 @@ static int clear_hub_feature(struct usb_device *hdev, int feature)
  */
 int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
 {
-	return usb_control_msg_send(hdev, 0, USB_REQ_CLEAR_FEATURE, USB_RT_PORT,
-				    feature, port1, NULL, 0, 1000);
+	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+		USB_REQ_CLEAR_FEATURE, USB_RT_PORT, feature, port1,
+		NULL, 0, 1000);
 }
 
 /*
@@ -428,8 +429,9 @@ int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature)
  */
 static int set_port_feature(struct usb_device *hdev, int port1, int feature)
 {
-	return usb_control_msg_send(hdev, 0, USB_REQ_SET_FEATURE, USB_RT_PORT,
-				    feature, port1, NULL, 0, 1000);
+	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+		USB_REQ_SET_FEATURE, USB_RT_PORT, feature, port1,
+		NULL, 0, 1000);
 }
 
 static char *to_led_name(int selector)
@@ -753,14 +755,15 @@ hub_clear_tt_buffer(struct usb_device *hdev, u16 devinfo, u16 tt)
 	/* Need to clear both directions for control ep */
 	if (((devinfo >> 11) & USB_ENDPOINT_XFERTYPE_MASK) ==
 			USB_ENDPOINT_XFER_CONTROL) {
-		int status = usb_control_msg_send(hdev, 0,
-						  HUB_CLEAR_TT_BUFFER, USB_RT_PORT,
-						  devinfo ^ 0x8000, tt, NULL, 0, 1000);
+		int status = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+				HUB_CLEAR_TT_BUFFER, USB_RT_PORT,
+				devinfo ^ 0x8000, tt, NULL, 0, 1000);
 		if (status)
 			return status;
 	}
-	return usb_control_msg_send(hdev, 0, HUB_CLEAR_TT_BUFFER, USB_RT_PORT,
-				    devinfo, tt, NULL, 0, 1000);
+	return usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+			       HUB_CLEAR_TT_BUFFER, USB_RT_PORT, devinfo,
+			       tt, NULL, 0, 1000);
 }
 
 /*
@@ -1046,10 +1049,11 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 	 */
 	if (type != HUB_RESUME) {
 		if (hdev->parent && hub_is_superspeed(hdev)) {
-			ret = usb_control_msg_send(hdev, 0, HUB_SET_DEPTH, USB_RT_HUB,
-						   hdev->level - 1, 0, NULL, 0,
-						   USB_CTRL_SET_TIMEOUT);
-			if (ret)
+			ret = usb_control_msg(hdev, usb_sndctrlpipe(hdev, 0),
+					HUB_SET_DEPTH, USB_RT_HUB,
+					hdev->level - 1, 0, NULL, 0,
+					USB_CTRL_SET_TIMEOUT);
+			if (ret < 0)
 				dev_err(hub->intfdev,
 						"set hub depth failed\n");
 		}
@@ -2325,10 +2329,13 @@ static int usb_enumerate_device_otg(struct usb_device *udev)
 		/* enable HNP before suspend, it's simpler */
 		if (port1 == bus->otg_port) {
 			bus->b_hnp_enable = 1;
-			err = usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, 0,
-						   USB_DEVICE_B_HNP_ENABLE, 0,
-						   NULL, 0, USB_CTRL_SET_TIMEOUT);
-			if (err) {
+			err = usb_control_msg(udev,
+				usb_sndctrlpipe(udev, 0),
+				USB_REQ_SET_FEATURE, 0,
+				USB_DEVICE_B_HNP_ENABLE,
+				0, NULL, 0,
+				USB_CTRL_SET_TIMEOUT);
+			if (err < 0) {
 				/*
 				 * OTG MESSAGE: report errors here,
 				 * customize to match your product.
@@ -2340,10 +2347,13 @@ static int usb_enumerate_device_otg(struct usb_device *udev)
 		} else if (desc->bLength == sizeof
 				(struct usb_otg_descriptor)) {
 			/* Set a_alt_hnp_support for legacy otg device */
-			err = usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, 0,
-						   USB_DEVICE_A_ALT_HNP_SUPPORT, 0,
-						   NULL, 0, USB_CTRL_SET_TIMEOUT);
-			if (err)
+			err = usb_control_msg(udev,
+				usb_sndctrlpipe(udev, 0),
+				USB_REQ_SET_FEATURE, 0,
+				USB_DEVICE_A_ALT_HNP_SUPPORT,
+				0, NULL, 0,
+				USB_CTRL_SET_TIMEOUT);
+			if (err < 0)
 				dev_err(&udev->dev,
 					"set a_alt_hnp_support failed: %d\n",
 					err);
@@ -3111,8 +3121,10 @@ int usb_disable_ltm(struct usb_device *udev)
 	if (!udev->actconfig)
 		return 0;
 
-	return usb_control_msg_send(udev, 0, USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
-				    USB_DEVICE_LTM_ENABLE, 0, NULL, 0, USB_CTRL_SET_TIMEOUT);
+	return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+			USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
+			USB_DEVICE_LTM_ENABLE, 0, NULL, 0,
+			USB_CTRL_SET_TIMEOUT);
 }
 EXPORT_SYMBOL_GPL(usb_disable_ltm);
 
@@ -3131,8 +3143,10 @@ void usb_enable_ltm(struct usb_device *udev)
 	if (!udev->actconfig)
 		return;
 
-	usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
-			     USB_DEVICE_LTM_ENABLE, 0, NULL, 0, USB_CTRL_SET_TIMEOUT);
+	usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+			USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
+			USB_DEVICE_LTM_ENABLE, 0, NULL, 0,
+			USB_CTRL_SET_TIMEOUT);
 }
 EXPORT_SYMBOL_GPL(usb_enable_ltm);
 
@@ -3149,14 +3163,17 @@ EXPORT_SYMBOL_GPL(usb_enable_ltm);
 static int usb_enable_remote_wakeup(struct usb_device *udev)
 {
 	if (udev->speed < USB_SPEED_SUPER)
-		return usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
-					    USB_DEVICE_REMOTE_WAKEUP, 0,
-					    NULL, 0, USB_CTRL_SET_TIMEOUT);
+		return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				USB_REQ_SET_FEATURE, USB_RECIP_DEVICE,
+				USB_DEVICE_REMOTE_WAKEUP, 0, NULL, 0,
+				USB_CTRL_SET_TIMEOUT);
 	else
-		return usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, USB_RECIP_INTERFACE,
-					    USB_INTRF_FUNC_SUSPEND,
-					    USB_INTRF_FUNC_SUSPEND_RW | USB_INTRF_FUNC_SUSPEND_LP,
-					    NULL, 0, USB_CTRL_SET_TIMEOUT);
+		return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				USB_REQ_SET_FEATURE, USB_RECIP_INTERFACE,
+				USB_INTRF_FUNC_SUSPEND,
+				USB_INTRF_FUNC_SUSPEND_RW |
+					USB_INTRF_FUNC_SUSPEND_LP,
+				NULL, 0, USB_CTRL_SET_TIMEOUT);
 }
 
 /*
@@ -3172,13 +3189,15 @@ static int usb_enable_remote_wakeup(struct usb_device *udev)
 static int usb_disable_remote_wakeup(struct usb_device *udev)
 {
 	if (udev->speed < USB_SPEED_SUPER)
-		return usb_control_msg_send(udev, 0, USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
-					    USB_DEVICE_REMOTE_WAKEUP, 0, NULL, 0,
-					    USB_CTRL_SET_TIMEOUT);
+		return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
+				USB_DEVICE_REMOTE_WAKEUP, 0, NULL, 0,
+				USB_CTRL_SET_TIMEOUT);
 	else
-		return usb_control_msg_send(udev, 0, USB_REQ_SET_FEATURE, USB_RECIP_INTERFACE,
-					    USB_INTRF_FUNC_SUSPEND, 0, NULL, 0,
-					    USB_CTRL_SET_TIMEOUT);
+		return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				USB_REQ_SET_FEATURE, USB_RECIP_INTERFACE,
+				USB_INTRF_FUNC_SUSPEND,	0, NULL, 0,
+				USB_CTRL_SET_TIMEOUT);
 }
 
 /* Count of wakeup-enabled devices at or below udev */
-- 
2.16.4

[RFC 02/14] Revert "Bluetooth: ath3k: use usb_control_msg_send() and usb_control_msg_recv()"

[Oliver Neukum]

This reverts commit e9b20f0fe17ab06c3b55153046209987749daa48.
The API has to be changed

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/bluetooth/ath3k.c | 90 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 64 insertions(+), 26 deletions(-)

diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
index 1472cccfd0b3..4ce270513695 100644
--- a/drivers/bluetooth/ath3k.c
+++ b/drivers/bluetooth/ath3k.c
@@ -212,16 +212,19 @@ static int ath3k_load_firmware(struct usb_device *udev,
 
 	BT_DBG("udev %p", udev);
 
+	pipe = usb_sndctrlpipe(udev, 0);
+
 	send_buf = kmalloc(BULK_SIZE, GFP_KERNEL);
 	if (!send_buf) {
 		BT_ERR("Can't allocate memory chunk for firmware");
 		return -ENOMEM;
 	}
 
-	err = usb_control_msg_send(udev, 0, USB_REQ_DFU_DNLOAD, USB_TYPE_VENDOR,
-				   0, 0, firmware->data, FW_HDR_SIZE,
-				   USB_CTRL_SET_TIMEOUT);
-	if (err) {
+	memcpy(send_buf, firmware->data, FW_HDR_SIZE);
+	err = usb_control_msg(udev, pipe, USB_REQ_DFU_DNLOAD, USB_TYPE_VENDOR,
+			      0, 0, send_buf, FW_HDR_SIZE,
+			      USB_CTRL_SET_TIMEOUT);
+	if (err < 0) {
 		BT_ERR("Can't change to loading configuration err");
 		goto error;
 	}
@@ -256,17 +259,44 @@ static int ath3k_load_firmware(struct usb_device *udev,
 
 static int ath3k_get_state(struct usb_device *udev, unsigned char *state)
 {
-	return usb_control_msg_recv(udev, 0, ATH3K_GETSTATE,
-				    USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
-				    state, 1, USB_CTRL_SET_TIMEOUT);
+	int ret, pipe = 0;
+	char *buf;
+
+	buf = kmalloc(sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	pipe = usb_rcvctrlpipe(udev, 0);
+	ret = usb_control_msg(udev, pipe, ATH3K_GETSTATE,
+			      USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
+			      buf, sizeof(*buf), USB_CTRL_SET_TIMEOUT);
+
+	*state = *buf;
+	kfree(buf);
+
+	return ret;
 }
 
 static int ath3k_get_version(struct usb_device *udev,
 			struct ath3k_version *version)
 {
-	return usb_control_msg_recv(udev, 0, ATH3K_GETVERSION,
-				    USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
-				    version, sizeof(*version), USB_CTRL_SET_TIMEOUT);
+	int ret, pipe = 0;
+	struct ath3k_version *buf;
+	const int size = sizeof(*buf);
+
+	buf = kmalloc(size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	pipe = usb_rcvctrlpipe(udev, 0);
+	ret = usb_control_msg(udev, pipe, ATH3K_GETVERSION,
+			      USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
+			      buf, size, USB_CTRL_SET_TIMEOUT);
+
+	memcpy(version, buf, size);
+	kfree(buf);
+
+	return ret;
 }
 
 static int ath3k_load_fwfile(struct usb_device *udev,
@@ -286,10 +316,13 @@ static int ath3k_load_fwfile(struct usb_device *udev,
 	}
 
 	size = min_t(uint, count, FW_HDR_SIZE);
+	memcpy(send_buf, firmware->data, size);
 
-	ret = usb_control_msg_send(udev, 0, ATH3K_DNLOAD, USB_TYPE_VENDOR, 0, 0,
-				   firmware->data, size, USB_CTRL_SET_TIMEOUT);
-	if (ret) {
+	pipe = usb_sndctrlpipe(udev, 0);
+	ret = usb_control_msg(udev, pipe, ATH3K_DNLOAD,
+			USB_TYPE_VENDOR, 0, 0, send_buf,
+			size, USB_CTRL_SET_TIMEOUT);
+	if (ret < 0) {
 		BT_ERR("Can't change to loading configuration err");
 		kfree(send_buf);
 		return ret;
@@ -322,19 +355,23 @@ static int ath3k_load_fwfile(struct usb_device *udev,
 	return 0;
 }
 
-static void ath3k_switch_pid(struct usb_device *udev)
+static int ath3k_switch_pid(struct usb_device *udev)
 {
-	usb_control_msg_send(udev, 0, USB_REG_SWITCH_VID_PID, USB_TYPE_VENDOR,
-			     0, 0, NULL, 0, USB_CTRL_SET_TIMEOUT);
+	int pipe = 0;
+
+	pipe = usb_sndctrlpipe(udev, 0);
+	return usb_control_msg(udev, pipe, USB_REG_SWITCH_VID_PID,
+			USB_TYPE_VENDOR, 0, 0,
+			NULL, 0, USB_CTRL_SET_TIMEOUT);
 }
 
 static int ath3k_set_normal_mode(struct usb_device *udev)
 {
 	unsigned char fw_state;
-	int ret;
+	int pipe = 0, ret;
 
 	ret = ath3k_get_state(udev, &fw_state);
-	if (ret) {
+	if (ret < 0) {
 		BT_ERR("Can't get state to change to normal mode err");
 		return ret;
 	}
@@ -344,9 +381,10 @@ static int ath3k_set_normal_mode(struct usb_device *udev)
 		return 0;
 	}
 
-	return usb_control_msg_send(udev, 0, ATH3K_SET_NORMAL_MODE,
-				    USB_TYPE_VENDOR, 0, 0, NULL, 0,
-				    USB_CTRL_SET_TIMEOUT);
+	pipe = usb_sndctrlpipe(udev, 0);
+	return usb_control_msg(udev, pipe, ATH3K_SET_NORMAL_MODE,
+			USB_TYPE_VENDOR, 0, 0,
+			NULL, 0, USB_CTRL_SET_TIMEOUT);
 }
 
 static int ath3k_load_patch(struct usb_device *udev)
@@ -359,7 +397,7 @@ static int ath3k_load_patch(struct usb_device *udev)
 	int ret;
 
 	ret = ath3k_get_state(udev, &fw_state);
-	if (ret) {
+	if (ret < 0) {
 		BT_ERR("Can't get state to change to load ram patch err");
 		return ret;
 	}
@@ -370,7 +408,7 @@ static int ath3k_load_patch(struct usb_device *udev)
 	}
 
 	ret = ath3k_get_version(udev, &fw_version);
-	if (ret) {
+	if (ret < 0) {
 		BT_ERR("Can't get version to change to load ram patch err");
 		return ret;
 	}
@@ -411,13 +449,13 @@ static int ath3k_load_syscfg(struct usb_device *udev)
 	int clk_value, ret;
 
 	ret = ath3k_get_state(udev, &fw_state);
-	if (ret) {
+	if (ret < 0) {
 		BT_ERR("Can't get state to change to load configuration err");
 		return -EBUSY;
 	}
 
 	ret = ath3k_get_version(udev, &fw_version);
-	if (ret) {
+	if (ret < 0) {
 		BT_ERR("Can't get version to change to load ram patch err");
 		return ret;
 	}
@@ -491,7 +529,7 @@ static int ath3k_probe(struct usb_interface *intf,
 			return ret;
 		}
 		ret = ath3k_set_normal_mode(udev);
-		if (ret) {
+		if (ret < 0) {
 			BT_ERR("Set normal mode failed");
 			return ret;
 		}
-- 
2.16.4

[RFC 03/14] Revert "sound: hiface: move to use usb_control_msg_send()"

[Oliver Neukum]

This reverts commit 119ae38a5cdfbefdf926b34fbf65cd60dc82c95e.
The API has to be changed.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 sound/usb/hiface/pcm.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c
index f9c924e3964e..a148caa5f48e 100644
--- a/sound/usb/hiface/pcm.c
+++ b/sound/usb/hiface/pcm.c
@@ -156,14 +156,16 @@ static int hiface_pcm_set_rate(struct pcm_runtime *rt, unsigned int rate)
 	 * This control message doesn't have any ack from the
 	 * other side
 	 */
-	ret = usb_control_msg_send(device, 0,
-				   HIFACE_SET_RATE_REQUEST,
-				   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
-				   rate_value, 0, NULL, 0, 100);
-	if (ret)
+	ret = usb_control_msg(device, usb_sndctrlpipe(device, 0),
+			      HIFACE_SET_RATE_REQUEST,
+			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
+			      rate_value, 0, NULL, 0, 100);
+	if (ret < 0) {
 		dev_err(&device->dev, "Error setting samplerate %d.\n", rate);
+		return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 static struct pcm_substream *hiface_pcm_get_substream(struct snd_pcm_substream
-- 
2.16.4

[RFC 04/14] Revert "sound: line6: move to use usb_control_msg_send() and usb_control_msg_recv()"

[Oliver Neukum]

This reverts commit f7ef7614f89e943d7511ee121b0b849f27b60cb2.
The API has to be changed.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 sound/usb/line6/driver.c   | 69 ++++++++++++++++++++++++++++------------------
 sound/usb/line6/podhd.c    | 17 ++++++++----
 sound/usb/line6/toneport.c |  8 +++---
 3 files changed, 57 insertions(+), 37 deletions(-)

diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index 601292c51491..60674ce4879b 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -337,18 +337,23 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
 {
 	struct usb_device *usbdev = line6->usbdev;
 	int ret;
-	u8 len;
+	unsigned char *len;
 	unsigned count;
 
 	if (address > 0xffff || datalen > 0xff)
 		return -EINVAL;
 
+	len = kmalloc(1, GFP_KERNEL);
+	if (!len)
+		return -ENOMEM;
+
 	/* query the serial number: */
-	ret = usb_control_msg_send(usbdev, 0, 0x67,
-				   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
-				   (datalen << 8) | 0x21, address, NULL, 0,
-				   LINE6_TIMEOUT * HZ);
-	if (ret) {
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
+			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+			      (datalen << 8) | 0x21, address,
+			      NULL, 0, LINE6_TIMEOUT * HZ);
+
+	if (ret < 0) {
 		dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
 		goto exit;
 	}
@@ -357,41 +362,45 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
 	for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
 		mdelay(LINE6_READ_WRITE_STATUS_DELAY);
 
-		ret = usb_control_msg_recv(usbdev, 0, 0x67,
-					   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
-					   0x0012, 0x0000, &len, 1,
-					   LINE6_TIMEOUT * HZ);
-		if (ret) {
+		ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
+				      USB_TYPE_VENDOR | USB_RECIP_DEVICE |
+				      USB_DIR_IN,
+				      0x0012, 0x0000, len, 1,
+				      LINE6_TIMEOUT * HZ);
+		if (ret < 0) {
 			dev_err(line6->ifcdev,
 				"receive length failed (error %d)\n", ret);
 			goto exit;
 		}
 
-		if (len != 0xff)
+		if (*len != 0xff)
 			break;
 	}
 
 	ret = -EIO;
-	if (len == 0xff) {
+	if (*len == 0xff) {
 		dev_err(line6->ifcdev, "read failed after %d retries\n",
 			count);
 		goto exit;
-	} else if (len != datalen) {
+	} else if (*len != datalen) {
 		/* should be equal or something went wrong */
 		dev_err(line6->ifcdev,
 			"length mismatch (expected %d, got %d)\n",
-			(int)datalen, len);
+			(int)datalen, (int)*len);
 		goto exit;
 	}
 
 	/* receive the result: */
-	ret = usb_control_msg_recv(usbdev, 0, 0x67,
-				   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
-				   0x0013, 0x0000, data, datalen, LINE6_TIMEOUT * HZ);
-	if (ret)
+	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
+			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+			      0x0013, 0x0000, data, datalen,
+			      LINE6_TIMEOUT * HZ);
+
+	if (ret < 0)
 		dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
 
 exit:
+	kfree(len);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(line6_read_data);
@@ -414,10 +423,12 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
 	if (!status)
 		return -ENOMEM;
 
-	ret = usb_control_msg_send(usbdev, 0, 0x67,
-				   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
-				   0x0022, address, data, datalen, LINE6_TIMEOUT * HZ);
-	if (ret) {
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
+			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+			      0x0022, address, data, datalen,
+			      LINE6_TIMEOUT * HZ);
+
+	if (ret < 0) {
 		dev_err(line6->ifcdev,
 			"write request failed (error %d)\n", ret);
 		goto exit;
@@ -426,10 +437,14 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
 	for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
 		mdelay(LINE6_READ_WRITE_STATUS_DELAY);
 
-		ret = usb_control_msg_recv(usbdev, 0, 0x67,
-					   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
-					   0x0012, 0x0000, status, 1, LINE6_TIMEOUT * HZ);
-		if (ret) {
+		ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
+				      0x67,
+				      USB_TYPE_VENDOR | USB_RECIP_DEVICE |
+				      USB_DIR_IN,
+				      0x0012, 0x0000,
+				      status, 1, LINE6_TIMEOUT * HZ);
+
+		if (ret < 0) {
 			dev_err(line6->ifcdev,
 				"receiving status failed (error %d)\n", ret);
 			goto exit;
diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c
index a1261f55d62b..eef45f7fef0d 100644
--- a/sound/usb/line6/podhd.c
+++ b/sound/usb/line6/podhd.c
@@ -183,25 +183,29 @@ static const struct attribute_group podhd_dev_attr_group = {
 static int podhd_dev_start(struct usb_line6_podhd *pod)
 {
 	int ret;
-	u8 init_bytes[8];
+	u8 *init_bytes;
 	int i;
 	struct usb_device *usbdev = pod->line6.usbdev;
 
-	ret = usb_control_msg_send(usbdev, 0,
+	init_bytes = kmalloc(8, GFP_KERNEL);
+	if (!init_bytes)
+		return -ENOMEM;
+
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
 					0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
 					0x11, 0,
 					NULL, 0, LINE6_TIMEOUT * HZ);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
 		goto exit;
 	}
 
 	/* NOTE: looks like some kind of ping message */
-	ret = usb_control_msg_recv(usbdev, 0, 0x67,
+	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
 					USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
 					0x11, 0x0,
 					init_bytes, 3, LINE6_TIMEOUT * HZ);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(pod->line6.ifcdev,
 			"receive length failed (error %d)\n", ret);
 		goto exit;
@@ -216,12 +220,13 @@ static int podhd_dev_start(struct usb_line6_podhd *pod)
 			goto exit;
 	}
 
-	ret = usb_control_msg_send(usbdev, 0,
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
 					USB_REQ_SET_FEATURE,
 					USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
 					1, 0,
 					NULL, 0, LINE6_TIMEOUT * HZ);
 exit:
+	kfree(init_bytes);
 	return ret;
 }
 
diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c
index a9b56085b76a..94dd5e7ab2e6 100644
--- a/sound/usb/line6/toneport.c
+++ b/sound/usb/line6/toneport.c
@@ -126,11 +126,11 @@ static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2)
 {
 	int ret;
 
-	ret = usb_control_msg_send(usbdev, 0, 0x67,
-				   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
-				   cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
+			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+			      cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
 
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&usbdev->dev, "send failed (error %d)\n", ret);
 		return ret;
 	}
-- 
2.16.4

[RFC 05/14] Revert "sound: 6fire: move to use usb_control_msg_send() and usb_control_msg_recv()"

[Oliver Neukum]

This reverts commit aea67cc1418252d07b9b56688f1b5fa70fcae813.
The API has to be changed.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 sound/usb/6fire/firmware.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/sound/usb/6fire/firmware.c b/sound/usb/6fire/firmware.c
index 5b8994070c96..69137c14d0dc 100644
--- a/sound/usb/6fire/firmware.c
+++ b/sound/usb/6fire/firmware.c
@@ -158,17 +158,29 @@ static int usb6fire_fw_ihex_init(const struct firmware *fw,
 static int usb6fire_fw_ezusb_write(struct usb_device *device,
 		int type, int value, char *data, int len)
 {
-	return usb_control_msg_send(device, 0, type,
-				    USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-				    value, 0, data, len, HZ);
+	int ret;
+
+	ret = usb_control_msg(device, usb_sndctrlpipe(device, 0), type,
+			USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+			value, 0, data, len, HZ);
+	if (ret < 0)
+		return ret;
+	else if (ret != len)
+		return -EIO;
+	return 0;
 }
 
 static int usb6fire_fw_ezusb_read(struct usb_device *device,
 		int type, int value, char *data, int len)
 {
-	return usb_control_msg_recv(device, 0, type,
-				    USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-				    value, 0, data, len, HZ);
+	int ret = usb_control_msg(device, usb_rcvctrlpipe(device, 0), type,
+			USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, value,
+			0, data, len, HZ);
+	if (ret < 0)
+		return ret;
+	else if (ret != len)
+		return -EIO;
+	return 0;
 }
 
 static int usb6fire_fw_fpga_write(struct usb_device *device,
@@ -218,7 +230,7 @@ static int usb6fire_fw_ezusb_upload(
 	/* upload firmware image */
 	data = 0x01; /* stop ezusb cpu */
 	ret = usb6fire_fw_ezusb_write(device, 0xa0, 0xe600, &data, 1);
-	if (ret) {
+	if (ret < 0) {
 		kfree(rec);
 		release_firmware(fw);
 		dev_err(&intf->dev,
@@ -230,7 +242,7 @@ static int usb6fire_fw_ezusb_upload(
 	while (usb6fire_fw_ihex_next_record(rec)) { /* write firmware */
 		ret = usb6fire_fw_ezusb_write(device, 0xa0, rec->address,
 				rec->data, rec->len);
-		if (ret) {
+		if (ret < 0) {
 			kfree(rec);
 			release_firmware(fw);
 			dev_err(&intf->dev,
@@ -245,7 +257,7 @@ static int usb6fire_fw_ezusb_upload(
 	if (postdata) { /* write data after firmware has been uploaded */
 		ret = usb6fire_fw_ezusb_write(device, 0xa0, postaddr,
 				postdata, postlen);
-		if (ret) {
+		if (ret < 0) {
 			dev_err(&intf->dev,
 				"unable to upload ezusb firmware %s: post urb.\n",
 				fwname);
@@ -255,7 +267,7 @@ static int usb6fire_fw_ezusb_upload(
 
 	data = 0x00; /* resume ezusb cpu */
 	ret = usb6fire_fw_ezusb_write(device, 0xa0, 0xe600, &data, 1);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&intf->dev,
 			"unable to upload ezusb firmware %s: end message.\n",
 			fwname);
@@ -290,7 +302,7 @@ static int usb6fire_fw_fpga_upload(
 	end = fw->data + fw->size;
 
 	ret = usb6fire_fw_ezusb_write(device, 8, 0, NULL, 0);
-	if (ret) {
+	if (ret < 0) {
 		kfree(buffer);
 		release_firmware(fw);
 		dev_err(&intf->dev,
@@ -315,7 +327,7 @@ static int usb6fire_fw_fpga_upload(
 	kfree(buffer);
 
 	ret = usb6fire_fw_ezusb_write(device, 9, 0, NULL, 0);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&intf->dev,
 			"unable to upload fpga firmware: end urb.\n");
 		return ret;
@@ -351,7 +363,7 @@ int usb6fire_fw_init(struct usb_interface *intf)
 	u8 buffer[12];
 
 	ret = usb6fire_fw_ezusb_read(device, 1, 0, buffer, 8);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(&intf->dev,
 			"unable to receive device firmware state.\n");
 		return ret;
-- 
2.16.4

[RFC 06/14] Revert "sound: usx2y: move to use usb_control_msg_send()"

[Oliver Neukum]

This reverts commit ec8eeceb06b7a6efb6d924fd2f4ba4ec79ddc7bf.
The API has to be changed.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 sound/usb/usx2y/us122l.c | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/sound/usb/usx2y/us122l.c b/sound/usb/usx2y/us122l.c
index 6d35303b0948..f86f7a61fb36 100644
--- a/sound/usb/usx2y/us122l.c
+++ b/sound/usb/usx2y/us122l.c
@@ -82,13 +82,40 @@ static int us144_create_usbmidi(struct snd_card *card)
 				  &US122L(card)->midi_list, &quirk);
 }
 
+/*
+ * Wrapper for usb_control_msg().
+ * Allocates a temp buffer to prevent dmaing from/to the stack.
+ */
+static int us122l_ctl_msg(struct usb_device *dev, unsigned int pipe,
+			  __u8 request, __u8 requesttype,
+			  __u16 value, __u16 index, void *data,
+			  __u16 size, int timeout)
+{
+	int err;
+	void *buf = NULL;
+
+	if (size > 0) {
+		buf = kmemdup(data, size, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
+	}
+	err = usb_control_msg(dev, pipe, request, requesttype,
+			      value, index, buf, size, timeout);
+	if (size > 0) {
+		memcpy(data, buf, size);
+		kfree(buf);
+	}
+	return err;
+}
+
 static void pt_info_set(struct usb_device *dev, u8 v)
 {
 	int ret;
 
-	ret = usb_control_msg_send(dev, 0, 'I',
-				   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-				   v, 0, NULL, 0, 1000);
+	ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+			      'I',
+			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+			      v, 0, NULL, 0, 1000);
 	snd_printdd(KERN_DEBUG "%i\n", ret);
 }
 
@@ -278,11 +305,10 @@ static int us122l_set_sample_rate(struct usb_device *dev, int rate)
 	data[0] = rate;
 	data[1] = rate >> 8;
 	data[2] = rate >> 16;
-	err = usb_control_msg_send(dev, 0, UAC_SET_CUR,
-				   USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_OUT,
-				   UAC_EP_CS_ATTR_SAMPLE_RATE << 8, ep, data, 3,
-				   1000);
-	if (err)
+	err = us122l_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC_SET_CUR,
+			     USB_TYPE_CLASS|USB_RECIP_ENDPOINT|USB_DIR_OUT,
+			     UAC_EP_CS_ATTR_SAMPLE_RATE << 8, ep, data, 3, 1000);
+	if (err < 0)
 		snd_printk(KERN_ERR "%d: cannot set freq %d to ep 0x%x\n",
 			   dev->devnum, rate, ep);
 	return err;
-- 
2.16.4

[RFC 07/14] Revert "USB: legousbtower: use usb_control_msg_recv()"

[Oliver Neukum]

This reverts commit be40c366416bf6ff74b25fd02e38cb6eaba497d1.
The API has to be changed.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/misc/legousbtower.c | 60 ++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index c3583df4c324..f922544056de 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -308,9 +308,15 @@ static int tower_open(struct inode *inode, struct file *file)
 	int subminor;
 	int retval = 0;
 	struct usb_interface *interface;
-	struct tower_reset_reply reset_reply;
+	struct tower_reset_reply *reset_reply;
 	int result;
 
+	reset_reply = kmalloc(sizeof(*reset_reply), GFP_KERNEL);
+	if (!reset_reply) {
+		retval = -ENOMEM;
+		goto exit;
+	}
+
 	nonseekable_open(inode, file);
 	subminor = iminor(inode);
 
@@ -341,11 +347,15 @@ static int tower_open(struct inode *inode, struct file *file)
 	}
 
 	/* reset the tower */
-	result = usb_control_msg_recv(dev->udev, 0,
-				      LEGO_USB_TOWER_REQUEST_RESET,
-				      USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
-				      0, 0,
-				      &reset_reply, sizeof(reset_reply), 1000);
+	result = usb_control_msg(dev->udev,
+				 usb_rcvctrlpipe(dev->udev, 0),
+				 LEGO_USB_TOWER_REQUEST_RESET,
+				 USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
+				 0,
+				 0,
+				 reset_reply,
+				 sizeof(*reset_reply),
+				 1000);
 	if (result < 0) {
 		dev_err(&dev->udev->dev,
 			"LEGO USB Tower reset control request failed\n");
@@ -384,6 +394,7 @@ static int tower_open(struct inode *inode, struct file *file)
 	mutex_unlock(&dev->lock);
 
 exit:
+	kfree(reset_reply);
 	return retval;
 }
 
@@ -742,7 +753,7 @@ static int tower_probe(struct usb_interface *interface, const struct usb_device_
 	struct device *idev = &interface->dev;
 	struct usb_device *udev = interface_to_usbdev(interface);
 	struct lego_usb_tower *dev;
-	struct tower_get_version_reply get_version_reply;
+	struct tower_get_version_reply *get_version_reply = NULL;
 	int retval = -ENOMEM;
 	int result;
 
@@ -787,25 +798,34 @@ static int tower_probe(struct usb_interface *interface, const struct usb_device_
 	dev->interrupt_in_interval = interrupt_in_interval ? interrupt_in_interval : dev->interrupt_in_endpoint->bInterval;
 	dev->interrupt_out_interval = interrupt_out_interval ? interrupt_out_interval : dev->interrupt_out_endpoint->bInterval;
 
+	get_version_reply = kmalloc(sizeof(*get_version_reply), GFP_KERNEL);
+	if (!get_version_reply) {
+		retval = -ENOMEM;
+		goto error;
+	}
+
 	/* get the firmware version and log it */
-	result = usb_control_msg_recv(udev, 0,
-				      LEGO_USB_TOWER_REQUEST_GET_VERSION,
-				      USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
-				      0,
-				      0,
-				      &get_version_reply,
-				      sizeof(get_version_reply),
-				      1000);
-	if (!result) {
+	result = usb_control_msg(udev,
+				 usb_rcvctrlpipe(udev, 0),
+				 LEGO_USB_TOWER_REQUEST_GET_VERSION,
+				 USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
+				 0,
+				 0,
+				 get_version_reply,
+				 sizeof(*get_version_reply),
+				 1000);
+	if (result != sizeof(*get_version_reply)) {
+		if (result >= 0)
+			result = -EIO;
 		dev_err(idev, "get version request failed: %d\n", result);
 		retval = result;
 		goto error;
 	}
 	dev_info(&interface->dev,
 		 "LEGO USB Tower firmware version is %d.%d build %d\n",
-		 get_version_reply.major,
-		 get_version_reply.minor,
-		 le16_to_cpu(get_version_reply.build_no));
+		 get_version_reply->major,
+		 get_version_reply->minor,
+		 le16_to_cpu(get_version_reply->build_no));
 
 	/* we can register the device now, as it is ready */
 	usb_set_intfdata(interface, dev);
@@ -824,9 +844,11 @@ static int tower_probe(struct usb_interface *interface, const struct usb_device_
 		 USB_MAJOR, dev->minor);
 
 exit:
+	kfree(get_version_reply);
 	return retval;
 
 error:
+	kfree(get_version_reply);
 	tower_delete(dev);
 	return retval;
 }
-- 
2.16.4

[RFC 08/14] USB: correct API of usb_control_msg_send/recv

[Oliver Neukum]

They need to specify how memory is to be allocated,
as control messages need to work in contexts that require GFP_NOIO.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/core/message.c | 25 ++++++++++++++++---------
 include/linux/usb.h        |  6 ++++--
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 1580694e3b95..f4107b9e8c38 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -174,6 +174,7 @@ EXPORT_SYMBOL_GPL(usb_control_msg);
  * @size: length in bytes of the data to send
  * @timeout: time in msecs to wait for the message to complete before timing
  *	out (if 0 the wait is forever)
+ * @memflags: the flags for memory allocation for buffers
  *
  * Context: !in_interrupt ()
  *
@@ -196,7 +197,8 @@ EXPORT_SYMBOL_GPL(usb_control_msg);
  */
 int usb_control_msg_send(struct usb_device *dev, __u8 endpoint, __u8 request,
 			 __u8 requesttype, __u16 value, __u16 index,
-			 const void *driver_data, __u16 size, int timeout)
+			 const void *driver_data, __u16 size, int timeout,
+			 gfp_t memflags)
 {
 	unsigned int pipe = usb_sndctrlpipe(dev, endpoint);
 	int ret;
@@ -206,7 +208,7 @@ int usb_control_msg_send(struct usb_device *dev, __u8 endpoint, __u8 request,
 		return -EINVAL;
 
 	if (size) {
-		data = kmemdup(driver_data, size, GFP_KERNEL);
+		data = kmemdup(driver_data, size, memflags);
 		if (!data)
 			return -ENOMEM;
 	}
@@ -235,6 +237,7 @@ EXPORT_SYMBOL_GPL(usb_control_msg_send);
  * @size: length in bytes of the data to be received
  * @timeout: time in msecs to wait for the message to complete before timing
  *	out (if 0 the wait is forever)
+ * @memflags: the flags for memory allocation for buffers
  *
  * Context: !in_interrupt ()
  *
@@ -263,7 +266,8 @@ EXPORT_SYMBOL_GPL(usb_control_msg_send);
  */
 int usb_control_msg_recv(struct usb_device *dev, __u8 endpoint, __u8 request,
 			 __u8 requesttype, __u16 value, __u16 index,
-			 void *driver_data, __u16 size, int timeout)
+			 void *driver_data, __u16 size, int timeout,
+			 gfp_t memflags)
 {
 	unsigned int pipe = usb_rcvctrlpipe(dev, endpoint);
 	int ret;
@@ -272,7 +276,7 @@ int usb_control_msg_recv(struct usb_device *dev, __u8 endpoint, __u8 request,
 	if (!size || !driver_data || usb_pipe_type_check(dev, pipe))
 		return -EINVAL;
 
-	data = kmalloc(size, GFP_KERNEL);
+	data = kmalloc(size, memflags);
 	if (!data)
 		return -ENOMEM;
 
@@ -1085,7 +1089,8 @@ int usb_set_isoch_delay(struct usb_device *dev)
 			USB_REQ_SET_ISOCH_DELAY,
 			USB_DIR_OUT | USB_TYPE_STANDARD | USB_RECIP_DEVICE,
 			dev->hub_delay, 0, NULL, 0,
-			USB_CTRL_SET_TIMEOUT);
+			USB_CTRL_SET_TIMEOUT,
+			GFP_NOIO);
 }
 
 /**
@@ -1206,7 +1211,7 @@ int usb_clear_halt(struct usb_device *dev, int pipe)
 	result = usb_control_msg_send(dev, 0,
 				      USB_REQ_CLEAR_FEATURE, USB_RECIP_ENDPOINT,
 				      USB_ENDPOINT_HALT, endp, NULL, 0,
-				      USB_CTRL_SET_TIMEOUT);
+				      USB_CTRL_SET_TIMEOUT, GFP_NOIO);
 
 	/* don't un-halt or force to DATA0 except on success */
 	if (result)
@@ -1574,7 +1579,8 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
 		ret = usb_control_msg_send(dev, 0,
 					   USB_REQ_SET_INTERFACE,
 					   USB_RECIP_INTERFACE, alternate,
-					   interface, NULL, 0, 5000);
+					   interface, NULL, 0, 5000,
+					   GFP_NOIO);
 
 	/* 9.4.10 says devices don't need this and are free to STALL the
 	 * request if the interface only has one alternate setting.
@@ -1710,7 +1716,8 @@ int usb_reset_configuration(struct usb_device *dev)
 	}
 	retval = usb_control_msg_send(dev, 0, USB_REQ_SET_CONFIGURATION, 0,
 				      config->desc.bConfigurationValue, 0,
-				      NULL, 0, USB_CTRL_SET_TIMEOUT);
+				      NULL, 0, USB_CTRL_SET_TIMEOUT,
+				      GFP_NOIO);
 	if (retval) {
 		usb_hcd_alloc_bandwidth(dev, NULL, NULL, NULL);
 		usb_enable_lpm(dev);
@@ -2098,7 +2105,7 @@ int usb_set_configuration(struct usb_device *dev, int configuration)
 
 	ret = usb_control_msg_send(dev, 0, USB_REQ_SET_CONFIGURATION, 0,
 				   configuration, 0, NULL, 0,
-				   USB_CTRL_SET_TIMEOUT);
+				   USB_CTRL_SET_TIMEOUT, GFP_NOIO);
 	if (ret && cp) {
 		/*
 		 * All the old state is gone, so what else can we do?
diff --git a/include/linux/usb.h b/include/linux/usb.h
index a5460f08126e..7d72c4e0713c 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1804,10 +1804,12 @@ extern int usb_bulk_msg(struct usb_device *usb_dev, unsigned int pipe,
 /* wrappers around usb_control_msg() for the most common standard requests */
 int usb_control_msg_send(struct usb_device *dev, __u8 endpoint, __u8 request,
 			 __u8 requesttype, __u16 value, __u16 index,
-			 const void *data, __u16 size, int timeout);
+			 const void *data, __u16 size, int timeout,
+			 gfp_t memflags);
 int usb_control_msg_recv(struct usb_device *dev, __u8 endpoint, __u8 request,
 			 __u8 requesttype, __u16 value, __u16 index,
-			 void *data, __u16 size, int timeout);
+			 void *data, __u16 size, int timeout,
+			 gfp_t memflags);
 extern int usb_get_descriptor(struct usb_device *dev, unsigned char desctype,
 	unsigned char descindex, void *buf, int size);
 extern int usb_get_status(struct usb_device *dev,
-- 
2.16.4

[PATCH 0/2] [usb]

[Petko Manolov]

From: Petko Manolov <petko.manolov@konsulko.com>

Avoid open coding usb_control_msg_send/recv()

Petko Manolov (2):
  Convert Pegasus' control messages to the new
    usb_control_msg_send/recv() scheme.
  Convert RTL8150's control messages to the new
    usb_control_msg_send/recv() scheme.

 drivers/net/usb/pegasus.c | 56 +++++++--------------------------------
 drivers/net/usb/rtl8150.c | 32 +++++-----------------
 2 files changed, 15 insertions(+), 73 deletions(-)

-- 
2.28.0

[PATCH 1/2] net: pegasus: convert control messages to the new send/recv scheme.

[Petko Manolov]

From: Petko Manolov <petko.manolov@konsulko.com>

Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
---
 drivers/net/usb/pegasus.c | 56 +++++++--------------------------------
 1 file changed, 9 insertions(+), 47 deletions(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index e92cb51a2c77..0ecc1eb2e71b 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -124,62 +124,24 @@ static void async_ctrl_callback(struct urb *urb)
 
 static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmalloc(size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(pegasus->usb, usb_rcvctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_GET_REGS, PEGASUS_REQT_READ, 0,
-			      indx, buf, size, 1000);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	else if (ret <= size)
-		memcpy(data, buf, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_recv(pegasus->usb, 0, PEGASUS_REQ_GET_REGS,
+				    PEGASUS_REQT_READ, 0, indx, data, size,
+				    1000, GFP_NOIO);
 }
 
 static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
 			 const void *data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmemdup(data, size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_SET_REGS, PEGASUS_REQT_WRITE, 0,
-			      indx, buf, size, 100);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REGS,
+			      PEGASUS_REQT_WRITE, 0, indx, data, size, 1000,
+			      GFP_NOIO);
 }
 
 static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmemdup(&data, 1, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,
-			      indx, buf, 1, 1000);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REG,
+			            PEGASUS_REQT_WRITE, data, indx, &data, 1,
+				    1000, GFP_NOIO);
 }
 
 static int update_eth_regs_async(pegasus_t *pegasus)
-- 
2.28.0

Re: [PATCH 1/2] net: pegasus: convert control messages to the new send/recv scheme.

[Greg KH]

On Fri, Sep 25, 2020 at 12:31:23PM +0300, Petko Manolov wrote:
> From: Petko Manolov <petko.manolov@konsulko.com>
> 
> Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>

I really do not like to take patches without any changelog text at all
:(

Can you fix this up?

> ---
>  drivers/net/usb/pegasus.c | 56 +++++++--------------------------------
>  1 file changed, 9 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index e92cb51a2c77..0ecc1eb2e71b 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -124,62 +124,24 @@ static void async_ctrl_callback(struct urb *urb)
>  
>  static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data)
>  {
> -	u8 *buf;
> -	int ret;
> -
> -	buf = kmalloc(size, GFP_NOIO);
> -	if (!buf)
> -		return -ENOMEM;
> -
> -	ret = usb_control_msg(pegasus->usb, usb_rcvctrlpipe(pegasus->usb, 0),
> -			      PEGASUS_REQ_GET_REGS, PEGASUS_REQT_READ, 0,
> -			      indx, buf, size, 1000);
> -	if (ret < 0)
> -		netif_dbg(pegasus, drv, pegasus->net,
> -			  "%s returned %d\n", __func__, ret);
> -	else if (ret <= size)
> -		memcpy(data, buf, ret);
> -	kfree(buf);
> -	return ret;
> +	return usb_control_msg_recv(pegasus->usb, 0, PEGASUS_REQ_GET_REGS,
> +				    PEGASUS_REQT_READ, 0, indx, data, size,
> +				    1000, GFP_NOIO);
>  }
>  

This change actually fixes a number of problems where the driver never
was checking for "short reads" and so could have had "bad" data used by
the driver.

At the least you should mention that in the changelog :)

>  static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
>  			 const void *data)
>  {
> -	u8 *buf;
> -	int ret;
> -
> -	buf = kmemdup(data, size, GFP_NOIO);
> -	if (!buf)
> -		return -ENOMEM;
> -
> -	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
> -			      PEGASUS_REQ_SET_REGS, PEGASUS_REQT_WRITE, 0,
> -			      indx, buf, size, 100);
> -	if (ret < 0)
> -		netif_dbg(pegasus, drv, pegasus->net,
> -			  "%s returned %d\n", __func__, ret);
> -	kfree(buf);
> -	return ret;
> +	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REGS,
> +			      PEGASUS_REQT_WRITE, 0, indx, data, size, 1000,
> +			      GFP_NOIO);
>  }
>  
>  static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
>  {
> -	u8 *buf;
> -	int ret;
> -
> -	buf = kmemdup(&data, 1, GFP_NOIO);
> -	if (!buf)
> -		return -ENOMEM;
> -
> -	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
> -			      PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,
> -			      indx, buf, 1, 1000);
> -	if (ret < 0)
> -		netif_dbg(pegasus, drv, pegasus->net,
> -			  "%s returned %d\n", __func__, ret);
> -	kfree(buf);
> -	return ret;
> +	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REG,
> +			            PEGASUS_REQT_WRITE, data, indx, &data, 1,
> +				    1000, GFP_NOIO);

Just call set_registers() above instead of "open coding" this?

thanks,

greg k-h

[PATCH 0/2] [patch v2] utilize the new control message send and receive API

[Petko Manolov]

From: Petko Manolov <petko.manolov@konsulko.com>

As discussed at the linux-usb this patch series is moving from old style
usb_control_msg() to the new API.  For obvious reasons we don't want to open
code usb_control_msg_send() and usb_control_msg_recv() functions.

Petko Manolov (2):
  Convert Pegasus' control messages to the new
    usb_control_msg_send/recv() scheme.
  Convert RTL8150's control messages to the new
    usb_control_msg_send/recv() scheme.

 drivers/net/usb/pegasus.c | 56 +++++++--------------------------------
 drivers/net/usb/rtl8150.c | 32 +++++-----------------
 2 files changed, 15 insertions(+), 73 deletions(-)

-- 
2.28.0

[PATCH 1/2] net: pegasus: convert control messages to the new send/recv scheme.

[Petko Manolov]

From: Petko Manolov <petko.manolov@konsulko.com>

Move all control transfers to safer usb_control_msg_send/recv() API.

Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
---
 drivers/net/usb/pegasus.c | 56 +++++++--------------------------------
 1 file changed, 9 insertions(+), 47 deletions(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index e92cb51a2c77..0ecc1eb2e71b 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -124,62 +124,24 @@ static void async_ctrl_callback(struct urb *urb)
 
 static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmalloc(size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(pegasus->usb, usb_rcvctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_GET_REGS, PEGASUS_REQT_READ, 0,
-			      indx, buf, size, 1000);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	else if (ret <= size)
-		memcpy(data, buf, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_recv(pegasus->usb, 0, PEGASUS_REQ_GET_REGS,
+				    PEGASUS_REQT_READ, 0, indx, data, size,
+				    1000, GFP_NOIO);
 }
 
 static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
 			 const void *data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmemdup(data, size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_SET_REGS, PEGASUS_REQT_WRITE, 0,
-			      indx, buf, size, 100);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REGS,
+			      PEGASUS_REQT_WRITE, 0, indx, data, size, 1000,
+			      GFP_NOIO);
 }
 
 static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmemdup(&data, 1, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,
-			      indx, buf, 1, 1000);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REG,
+			            PEGASUS_REQT_WRITE, data, indx, &data, 1,
+				    1000, GFP_NOIO);
 }
 
 static int update_eth_regs_async(pegasus_t *pegasus)
-- 
2.28.0

Re: [PATCH 1/2] net: pegasus: convert control messages to the new send/recv scheme.

[Greg KH]

On Fri, Sep 25, 2020 at 07:01:59PM +0300, Petko Manolov wrote:
> From: Petko Manolov <petko.manolov@konsulko.com>
> 
> Move all control transfers to safer usb_control_msg_send/recv() API.

This says _what_ the patch does, but we can see that from the diff.  You
should describe _why_ we are doing what we are doing, so that everyone
understands the need for the change.

Also, can you add something like:
	This fixes a number of issues where short reads were not
	properly handled by the driver.

Take a look at "The canonical patch format" in the kernel file,
Documentation/SubmittingPatches for a description of how to write good
changelogs that we can understand 5 years in the future when we have to
look back at the files.

> Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
> ---
>  drivers/net/usb/pegasus.c | 56 +++++++--------------------------------
>  1 file changed, 9 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index e92cb51a2c77..0ecc1eb2e71b 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -124,62 +124,24 @@ static void async_ctrl_callback(struct urb *urb)
>  
>  static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data)
>  {
> -	u8 *buf;
> -	int ret;
> -
> -	buf = kmalloc(size, GFP_NOIO);
> -	if (!buf)
> -		return -ENOMEM;
> -
> -	ret = usb_control_msg(pegasus->usb, usb_rcvctrlpipe(pegasus->usb, 0),
> -			      PEGASUS_REQ_GET_REGS, PEGASUS_REQT_READ, 0,
> -			      indx, buf, size, 1000);
> -	if (ret < 0)
> -		netif_dbg(pegasus, drv, pegasus->net,
> -			  "%s returned %d\n", __func__, ret);
> -	else if (ret <= size)
> -		memcpy(data, buf, ret);
> -	kfree(buf);
> -	return ret;
> +	return usb_control_msg_recv(pegasus->usb, 0, PEGASUS_REQ_GET_REGS,
> +				    PEGASUS_REQT_READ, 0, indx, data, size,
> +				    1000, GFP_NOIO);
>  }
>  
>  static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
>  			 const void *data)
>  {
> -	u8 *buf;
> -	int ret;
> -
> -	buf = kmemdup(data, size, GFP_NOIO);
> -	if (!buf)
> -		return -ENOMEM;
> -
> -	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
> -			      PEGASUS_REQ_SET_REGS, PEGASUS_REQT_WRITE, 0,
> -			      indx, buf, size, 100);
> -	if (ret < 0)
> -		netif_dbg(pegasus, drv, pegasus->net,
> -			  "%s returned %d\n", __func__, ret);
> -	kfree(buf);
> -	return ret;
> +	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REGS,
> +			      PEGASUS_REQT_WRITE, 0, indx, data, size, 1000,
> +			      GFP_NOIO);
>  }
>  
>  static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
>  {
> -	u8 *buf;
> -	int ret;
> -
> -	buf = kmemdup(&data, 1, GFP_NOIO);
> -	if (!buf)
> -		return -ENOMEM;
> -
> -	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
> -			      PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,
> -			      indx, buf, 1, 1000);
> -	if (ret < 0)
> -		netif_dbg(pegasus, drv, pegasus->net,
> -			  "%s returned %d\n", __func__, ret);
> -	kfree(buf);
> -	return ret;
> +	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REG,
> +			            PEGASUS_REQT_WRITE, data, indx, &data, 1,
> +				    1000, GFP_NOIO);
>  }

Again, why isn't set_register() now rewritten to just be:

static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
{
	return set_registers(pegasus, indx, 1, data);
}

Much easier to show that it's all converted properly :)

thanks,

greg k-h

Re: [PATCH 1/2] net: pegasus: convert control messages to the new send/recv scheme.

[Petko Manolov]

On 20-09-26 07:44:57, Greg KH wrote:
> On Fri, Sep 25, 2020 at 07:01:59PM +0300, Petko Manolov wrote:
> > From: Petko Manolov <petko.manolov@konsulko.com>
> > 
> > Move all control transfers to safer usb_control_msg_send/recv() API.
> 
> This says _what_ the patch does, but we can see that from the diff.  You
> should describe _why_ we are doing what we are doing, so that everyone
> understands the need for the change.

Didn't want to parrot the reason for usb_control_msg_send/recv() existence, but 
i guess you're right.  5 years from now i would not remember anything. :)

> >  static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
> >  {
> > -	u8 *buf;
> > -	int ret;
> > -
> > -	buf = kmemdup(&data, 1, GFP_NOIO);
> > -	if (!buf)
> > -		return -ENOMEM;
> > -
> > -	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
> > -			      PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,
> > -			      indx, buf, 1, 1000);
> > -	if (ret < 0)
> > -		netif_dbg(pegasus, drv, pegasus->net,
> > -			  "%s returned %d\n", __func__, ret);
> > -	kfree(buf);
> > -	return ret;
> > +	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REG,
> > +			            PEGASUS_REQT_WRITE, data, indx, &data, 1,
> > +				    1000, GFP_NOIO);
> >  }
> 
> Again, why isn't set_register() now rewritten to just be:
> 
> static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
> {
> 	return set_registers(pegasus, indx, 1, data);
> }
> 
> Much easier to show that it's all converted properly :)

There's a catch - adm8511-based devices can only write to a single register via 
specific control request.  This request expects the register number in wIndex 
and the value in wValue.  A bit of a brain fart which is fixed in adm8515.

Admittedly, I could open code set_register() and avoid a kmemdup() call since in 
this case 'data' is going to be NULL.  However, set_register() isn't heavily 
used, except for the setup phase, so i went for the prettier/simpler approach.  
Which reminds me that i should put a comment explaining why is that.


cheers,
Petko

Re: [PATCH 1/2] net: pegasus: convert control messages to the new send/recv scheme.

[Greg KH]

On Sat, Sep 26, 2020 at 11:21:08AM +0300, Petko Manolov wrote:
> On 20-09-26 07:44:57, Greg KH wrote:
> > On Fri, Sep 25, 2020 at 07:01:59PM +0300, Petko Manolov wrote:
> > > From: Petko Manolov <petko.manolov@konsulko.com>
> > > 
> > > Move all control transfers to safer usb_control_msg_send/recv() API.
> > 
> > This says _what_ the patch does, but we can see that from the diff.  You
> > should describe _why_ we are doing what we are doing, so that everyone
> > understands the need for the change.
> 
> Didn't want to parrot the reason for usb_control_msg_send/recv() existence, but 
> i guess you're right.  5 years from now i would not remember anything. :)
> 
> > >  static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
> > >  {
> > > -	u8 *buf;
> > > -	int ret;
> > > -
> > > -	buf = kmemdup(&data, 1, GFP_NOIO);
> > > -	if (!buf)
> > > -		return -ENOMEM;
> > > -
> > > -	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
> > > -			      PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,
> > > -			      indx, buf, 1, 1000);
> > > -	if (ret < 0)
> > > -		netif_dbg(pegasus, drv, pegasus->net,
> > > -			  "%s returned %d\n", __func__, ret);
> > > -	kfree(buf);
> > > -	return ret;
> > > +	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REG,
> > > +			            PEGASUS_REQT_WRITE, data, indx, &data, 1,
> > > +				    1000, GFP_NOIO);
> > >  }
> > 
> > Again, why isn't set_register() now rewritten to just be:
> > 
> > static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
> > {
> > 	return set_registers(pegasus, indx, 1, data);
> > }
> > 
> > Much easier to show that it's all converted properly :)
> 
> There's a catch - adm8511-based devices can only write to a single register via 
> specific control request.  This request expects the register number in wIndex 
> and the value in wValue.  A bit of a brain fart which is fixed in adm8515.

Ah, I missed that, good catch, sorry about that.

greg k-h

[PATCH 2/2] net: rtl8150: convert control messages to the new send/recv scheme.

[Petko Manolov]

From: Petko Manolov <petko.manolov@konsulko.com>

Move all control transfers to safer usb_control_msg_send/recv() API.

Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
---
 drivers/net/usb/rtl8150.c | 32 ++++++--------------------------
 1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 733f120c852b..f76d20d290d9 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -152,36 +152,16 @@ static const char driver_name [] = "rtl8150";
 */
 static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
 {
-	void *buf;
-	int ret;
-
-	buf = kmalloc(size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-			      RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
-			      indx, 0, buf, size, 500);
-	if (ret > 0 && ret <= size)
-		memcpy(data, buf, ret);
-	kfree(buf);
-	return ret;
+	ret = usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
+				   RTL8150_REQT_READ, indx, 0, data, size,
+				   1000, GFP_NOIO);
 }
 
 static int set_registers(rtl8150_t * dev, u16 indx, u16 size, const void *data)
 {
-	void *buf;
-	int ret;
-
-	buf = kmemdup(data, size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-			      RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
-			      indx, 0, buf, size, 500);
-	kfree(buf);
-	return ret;
+	ret = usb_control_msg_send(dev->udev, 0, RTL8150_REQ_SET_REGS,
+				   RTL8150_REQT_WRITE, indx, 0, data, size,
+				   1000, GFP_NOIO);
 }
 
 static void async_set_reg_cb(struct urb *urb)
-- 
2.28.0

Re: [PATCH 2/2] net: rtl8150: convert control messages to the new send/recv scheme.

[kernel test robot]

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

Hi Petko,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.9-rc6 next-20200925]
[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/Petko-Manolov/utilize-the-new-control-message-send-and-receive-API/20200926-000418
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 171d4ff79f965c1f164705ef0aaea102a6ad238b
config: sh-allmodconfig (attached as .config)
compiler: sh4-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/dc81887ca37bd0f3530fae6a7ab5979b30e0b21e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Petko-Manolov/utilize-the-new-control-message-send-and-receive-API/20200926-000418
        git checkout dc81887ca37bd0f3530fae6a7ab5979b30e0b21e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh 

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

All error/warnings (new ones prefixed by >>):

   drivers/net/usb/rtl8150.c: In function 'get_registers':
>> drivers/net/usb/rtl8150.c:155:2: error: 'ret' undeclared (first use in this function); did you mean 'net'?
     155 |  ret = usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
         |  ^~~
         |  net
   drivers/net/usb/rtl8150.c:155:2: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/usb/rtl8150.c:155:8: error: implicit declaration of function 'usb_control_msg_recv'; did you mean 'usb_control_msg'? [-Werror=implicit-function-declaration]
     155 |  ret = usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
         |        ^~~~~~~~~~~~~~~~~~~~
         |        usb_control_msg
>> drivers/net/usb/rtl8150.c:158:1: warning: no return statement in function returning non-void [-Wreturn-type]
     158 | }
         | ^
   drivers/net/usb/rtl8150.c: In function 'set_registers':
   drivers/net/usb/rtl8150.c:162:2: error: 'ret' undeclared (first use in this function); did you mean 'net'?
     162 |  ret = usb_control_msg_send(dev->udev, 0, RTL8150_REQ_SET_REGS,
         |  ^~~
         |  net
   drivers/net/usb/rtl8150.c:162:8: error: implicit declaration of function 'usb_control_msg_send'; did you mean 'usb_control_msg'? [-Werror=implicit-function-declaration]
     162 |  ret = usb_control_msg_send(dev->udev, 0, RTL8150_REQ_SET_REGS,
         |        ^~~~~~~~~~~~~~~~~~~~
         |        usb_control_msg
   drivers/net/usb/rtl8150.c:165:1: warning: no return statement in function returning non-void [-Wreturn-type]
     165 | }
         | ^
   cc1: some warnings being treated as errors

vim +155 drivers/net/usb/rtl8150.c

   147	
   148	/*
   149	**
   150	**	device related part of the code
   151	**
   152	*/
   153	static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
   154	{
 > 155		ret = usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
   156					   RTL8150_REQT_READ, indx, 0, data, size,
   157					   1000, GFP_NOIO);
 > 158	}
   159	

---
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: 52717 bytes --]

Re: [PATCH 2/2] net: rtl8150: convert control messages to the new send/recv scheme.

[kernel test robot]

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

Hi Petko,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.9-rc6 next-20200925]
[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/Petko-Manolov/utilize-the-new-control-message-send-and-receive-API/20200926-000418
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 171d4ff79f965c1f164705ef0aaea102a6ad238b
config: sh-allmodconfig (attached as .config)
compiler: sh4-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/dc81887ca37bd0f3530fae6a7ab5979b30e0b21e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Petko-Manolov/utilize-the-new-control-message-send-and-receive-API/20200926-000418
        git checkout dc81887ca37bd0f3530fae6a7ab5979b30e0b21e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh 

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

All error/warnings (new ones prefixed by >>):

   drivers/net/usb/rtl8150.c: In function 'get_registers':
>> drivers/net/usb/rtl8150.c:155:2: error: 'ret' undeclared (first use in this function); did you mean 'net'?
     155 |  ret = usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
         |  ^~~
         |  net
   drivers/net/usb/rtl8150.c:155:2: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/usb/rtl8150.c:155:8: error: implicit declaration of function 'usb_control_msg_recv'; did you mean 'usb_control_msg'? [-Werror=implicit-function-declaration]
     155 |  ret = usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
         |        ^~~~~~~~~~~~~~~~~~~~
         |        usb_control_msg
>> drivers/net/usb/rtl8150.c:158:1: warning: no return statement in function returning non-void [-Wreturn-type]
     158 | }
         | ^
   drivers/net/usb/rtl8150.c: In function 'set_registers':
   drivers/net/usb/rtl8150.c:162:2: error: 'ret' undeclared (first use in this function); did you mean 'net'?
     162 |  ret = usb_control_msg_send(dev->udev, 0, RTL8150_REQ_SET_REGS,
         |  ^~~
         |  net
   drivers/net/usb/rtl8150.c:162:8: error: implicit declaration of function 'usb_control_msg_send'; did you mean 'usb_control_msg'? [-Werror=implicit-function-declaration]
     162 |  ret = usb_control_msg_send(dev->udev, 0, RTL8150_REQ_SET_REGS,
         |        ^~~~~~~~~~~~~~~~~~~~
         |        usb_control_msg
   drivers/net/usb/rtl8150.c:165:1: warning: no return statement in function returning non-void [-Wreturn-type]
     165 | }
         | ^
   cc1: some warnings being treated as errors

vim +155 drivers/net/usb/rtl8150.c

   147	
   148	/*
   149	**
   150	**	device related part of the code
   151	**
   152	*/
   153	static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
   154	{
 > 155		ret = usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
   156					   RTL8150_REQT_READ, indx, 0, data, size,
   157					   1000, GFP_NOIO);
 > 158	}
   159	

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

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

Re: [PATCH 1/2] net: pegasus: convert control messages to the new send/recv scheme.

[Petko Manolov]

On 20-09-25 16:37:30, Greg KH wrote:
> On Fri, Sep 25, 2020 at 12:31:23PM +0300, Petko Manolov wrote:
> > From: Petko Manolov <petko.manolov@konsulko.com>
> > 
> > Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
> 
> I really do not like to take patches without any changelog text at all
> :(
> 
> Can you fix this up?

I am sorry about this.  Hope v2 is better.


cheers,
Petko

Re: [PATCH 1/2] net: pegasus: convert control messages to the new send/recv scheme.

[Greg KH]

On Fri, Sep 25, 2020 at 07:05:37PM +0300, Petko Manolov wrote:
> On 20-09-25 16:37:30, Greg KH wrote:
> > On Fri, Sep 25, 2020 at 12:31:23PM +0300, Petko Manolov wrote:
> > > From: Petko Manolov <petko.manolov@konsulko.com>
> > > 
> > > Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
> > 
> > I really do not like to take patches without any changelog text at all
> > :(
> > 
> > Can you fix this up?
> 
> I am sorry about this.  Hope v2 is better.

Better, but still a bit more work is needed :)

thanks,

greg k-h

[PATCH 2/2] net: rtl8150: convert control messages to the new send/recv scheme.

[Petko Manolov]

From: Petko Manolov <petko.manolov@konsulko.com>

Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
---
 drivers/net/usb/rtl8150.c | 32 ++++++--------------------------
 1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 733f120c852b..f76d20d290d9 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -152,36 +152,16 @@ static const char driver_name [] = "rtl8150";
 */
 static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
 {
-	void *buf;
-	int ret;
-
-	buf = kmalloc(size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-			      RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
-			      indx, 0, buf, size, 500);
-	if (ret > 0 && ret <= size)
-		memcpy(data, buf, ret);
-	kfree(buf);
-	return ret;
+	ret = usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
+				   RTL8150_REQT_READ, indx, 0, data, size,
+				   1000, GFP_NOIO);
 }
 
 static int set_registers(rtl8150_t * dev, u16 indx, u16 size, const void *data)
 {
-	void *buf;
-	int ret;
-
-	buf = kmemdup(data, size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-			      RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
-			      indx, 0, buf, size, 500);
-	kfree(buf);
-	return ret;
+	ret = usb_control_msg_send(dev->udev, 0, RTL8150_REQ_SET_REGS,
+				   RTL8150_REQT_WRITE, indx, 0, data, size,
+				   1000, GFP_NOIO);
 }
 
 static void async_set_reg_cb(struct urb *urb)
-- 
2.28.0

Re: [PATCH 2/2] net: rtl8150: convert control messages to the new send/recv scheme.

[Greg KH]

On Fri, Sep 25, 2020 at 12:31:24PM +0300, Petko Manolov wrote:
> From: Petko Manolov <petko.manolov@konsulko.com>
> 
> Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>

Again, a changelog is good.

thanks,

greg k-h

[PATCH v3 0/2] Use the new usb control message API.

[Petko Manolov]

From: Petko Manolov <petko.manolov@konsulko.com>

Open coding, ccasional improper error handling by the caller of
usb_control_msg() and not flagging partial read as an error requires a new API
that takes care of these issues.  It took the form of
usb_control_msg_send/recv() and this patch series is converting Pegasus and
RTL8150 drivers to using the proper calls.

Petko Manolov (2):
  net: pegasus: Use the new usb control message API.
  net: rtl8150: Use the new usb control message API.

 drivers/net/usb/pegasus.c | 61 ++++++++++-----------------------------
 drivers/net/usb/rtl8150.c | 32 ++++----------------
 2 files changed, 21 insertions(+), 72 deletions(-)

-- 
2.28.0

[PATCH v3 1/2] net: pegasus: Use the new usb control message API.

[Petko Manolov]

From: Petko Manolov <petko.manolov@konsulko.com>

The old usb_control_msg() let the caller handle the error and also did not
account for partial reads.  Since these are now considered harmful, move the
driver over to usb_control_msg_recv/send() calls.

Added small note about why set_registers() can't be used to substitute
set_register().

Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
---
 drivers/net/usb/pegasus.c | 61 ++++++++++-----------------------------
 1 file changed, 15 insertions(+), 46 deletions(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index e92cb51a2c77..3d1bcb320ca1 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -124,62 +124,31 @@ static void async_ctrl_callback(struct urb *urb)
 
 static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmalloc(size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(pegasus->usb, usb_rcvctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_GET_REGS, PEGASUS_REQT_READ, 0,
-			      indx, buf, size, 1000);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	else if (ret <= size)
-		memcpy(data, buf, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_recv(pegasus->usb, 0, PEGASUS_REQ_GET_REGS,
+				   PEGASUS_REQT_READ, 0, indx, data, size,
+				   1000, GFP_NOIO);
 }
 
 static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
 			 const void *data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmemdup(data, size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_SET_REGS, PEGASUS_REQT_WRITE, 0,
-			      indx, buf, size, 100);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REGS,
+				    PEGASUS_REQT_WRITE, 0, indx, data, size,
+				    1000, GFP_NOIO);
 }
 
+/*
+ * There is only one way to write to a single ADM8511 register and this is via
+ * specific control request.  'data' is ignored by the device, but it is here to
+ * not break the API.
+ */
 static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmemdup(&data, 1, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
+	void *buf = &data;
 
-	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,
-			      indx, buf, 1, 1000);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REG,
+				    PEGASUS_REQT_WRITE, data, indx, buf, 1,
+				    1000, GFP_NOIO);
 }
 
 static int update_eth_regs_async(pegasus_t *pegasus)
-- 
2.28.0

[PATCH v3 2/2] net: rtl8150: Use the new usb control message API.

[Petko Manolov]

From: Petko Manolov <petko.manolov@konsulko.com>

The old usb_control_msg() let the caller handle the error and also did not
account for partial reads.  Since these are now considered harmful, move the
driver over to usb_control_msg_recv/send() calls.

Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
---
 drivers/net/usb/rtl8150.c | 32 ++++++--------------------------
 1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 733f120c852b..b3a0b188b1a1 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -152,36 +152,16 @@ static const char driver_name [] = "rtl8150";
 */
 static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
 {
-	void *buf;
-	int ret;
-
-	buf = kmalloc(size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-			      RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
-			      indx, 0, buf, size, 500);
-	if (ret > 0 && ret <= size)
-		memcpy(data, buf, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
+				    RTL8150_REQT_READ, indx, 0, data, size,
+				    1000, GFP_NOIO);
 }
 
 static int set_registers(rtl8150_t * dev, u16 indx, u16 size, const void *data)
 {
-	void *buf;
-	int ret;
-
-	buf = kmemdup(data, size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-			      RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
-			      indx, 0, buf, size, 500);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_send(dev->udev, 0, RTL8150_REQ_SET_REGS,
+				    RTL8150_REQT_WRITE, indx, 0, data, size,
+				    1000, GFP_NOIO);
 }
 
 static void async_set_reg_cb(struct urb *urb)
-- 
2.28.0

Re: [PATCH v3 0/2] Use the new usb control message API.

[Greg KH]

On Sat, Sep 26, 2020 at 12:13:25PM +0300, Petko Manolov wrote:
> From: Petko Manolov <petko.manolov@konsulko.com>
> 
> Open coding, ccasional improper error handling by the caller of
> usb_control_msg() and not flagging partial read as an error requires a new API
> that takes care of these issues.  It took the form of
> usb_control_msg_send/recv() and this patch series is converting Pegasus and
> RTL8150 drivers to using the proper calls.
> 
> Petko Manolov (2):
>   net: pegasus: Use the new usb control message API.
>   net: rtl8150: Use the new usb control message API.
> 
>  drivers/net/usb/pegasus.c | 61 ++++++++++-----------------------------
>  drivers/net/usb/rtl8150.c | 32 ++++----------------
>  2 files changed, 21 insertions(+), 72 deletions(-)

Normally drivers/net/ stuff gets sent to the netdev mailing list.

I don't want to take patches that those maintainers/developers have not
seen and acked yet, so can you resend this series and also cc: them?

thanks,

greg k-h

Re: [PATCH v3 0/2] Use the new usb control message API.

[Petko Manolov]

On 20-09-27 12:16:31, Greg KH wrote:
> On Sat, Sep 26, 2020 at 12:13:25PM +0300, Petko Manolov wrote:
> > From: Petko Manolov <petko.manolov@konsulko.com>
> > 
> > Open coding, ccasional improper error handling by the caller of 
> > usb_control_msg() and not flagging partial read as an error requires a new 
> > API that takes care of these issues.  It took the form of 
> > usb_control_msg_send/recv() and this patch series is converting Pegasus and 
> > RTL8150 drivers to using the proper calls.
> > 
> > Petko Manolov (2): net: pegasus: Use the new usb control message API. net: 
> >   rtl8150: Use the new usb control message API.
> > 
> >  drivers/net/usb/pegasus.c | 61 ++++++++++----------------------------- 
> >  drivers/net/usb/rtl8150.c | 32 ++++---------------- 2 files changed, 21 
> >  insertions(+), 72 deletions(-)
> 
> Normally drivers/net/ stuff gets sent to the netdev mailing list.
> 
> I don't want to take patches that those maintainers/developers have not seen 
> and acked yet, so can you resend this series and also cc: them?

The changes are entirely in the USB land and thus unrelated to the networking 
part of the driver.  However, getting their ack is perhaps the right thing to do 
so i just followed your advice. :)


cheers,
Petko

[PATCH RESEND v3 0/2] Use the new usb control message API.

[Petko Manolov]

Re-sending these, now CC-ing the folks at linux-netdev.

Open coding, occasional improper error handling by the caller of
usb_control_msg() and not flagging partial read as an error requires a new API
that takes care of these issues.  It took the form of
usb_control_msg_send/recv() and this patch series is converting Pegasus and
RTL8150 drivers to using the proper calls.

Petko Manolov (2):
  net: pegasus: Use the new usb control message API.
  net: rtl8150: Use the new usb control message API.

 drivers/net/usb/pegasus.c | 61 ++++++++++-----------------------------
 drivers/net/usb/rtl8150.c | 32 ++++----------------
 2 files changed, 21 insertions(+), 72 deletions(-)

-- 
2.28.0

[PATCH RESEND v3 1/2] net: pegasus: Use the new usb control message API.

[Petko Manolov]

The old usb_control_msg() let the caller handle the error and also did not
account for partial reads.  Since these are now considered harmful, move the
driver over to usb_control_msg_recv/send() calls.

Added small note about why set_registers() can't be used to substitute
set_register().

Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
---
 drivers/net/usb/pegasus.c | 61 ++++++++++-----------------------------
 1 file changed, 15 insertions(+), 46 deletions(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index e92cb51a2c77..26b4e48bf91f 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -124,62 +124,31 @@ static void async_ctrl_callback(struct urb *urb)
 
 static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmalloc(size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(pegasus->usb, usb_rcvctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_GET_REGS, PEGASUS_REQT_READ, 0,
-			      indx, buf, size, 1000);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	else if (ret <= size)
-		memcpy(data, buf, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_recv(pegasus->usb, 0, PEGASUS_REQ_GET_REGS,
+				   PEGASUS_REQT_READ, 0, indx, data, size,
+				   1000, GFP_NOIO);
 }
 
 static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
 			 const void *data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmemdup(data, size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_SET_REGS, PEGASUS_REQT_WRITE, 0,
-			      indx, buf, size, 100);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REGS,
+				    PEGASUS_REQT_WRITE, 0, indx, data, size,
+				    1000, GFP_NOIO);
 }
 
+/*
+ * There is only one way to write to a single ADM8511 register and this is via
+ * specific control request.  'data' is ignored by the device, but it is here to
+ * not break the API.
+ */
 static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
 {
-	u8 *buf;
-	int ret;
-
-	buf = kmemdup(&data, 1, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
+	void *buf = &data;
 
-	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
-			      PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,
-			      indx, buf, 1, 1000);
-	if (ret < 0)
-		netif_dbg(pegasus, drv, pegasus->net,
-			  "%s returned %d\n", __func__, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REG,
+				    PEGASUS_REQT_WRITE, data, indx, buf, 1,
+				    1000, GFP_NOIO);
 }
 
 static int update_eth_regs_async(pegasus_t *pegasus)
-- 
2.28.0

[PATCH RESEND v3 2/2] net: rtl8150: Use the new usb control message API.

[Petko Manolov]

The old usb_control_msg() let the caller handle the error and also did not
account for partial reads.  Since these are now considered harmful, move the
driver over to usb_control_msg_recv/send() calls.

Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
---
 drivers/net/usb/rtl8150.c | 32 ++++++--------------------------
 1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 733f120c852b..b3a0b188b1a1 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -152,36 +152,16 @@ static const char driver_name [] = "rtl8150";
 */
 static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
 {
-	void *buf;
-	int ret;
-
-	buf = kmalloc(size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-			      RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
-			      indx, 0, buf, size, 500);
-	if (ret > 0 && ret <= size)
-		memcpy(data, buf, ret);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
+				    RTL8150_REQT_READ, indx, 0, data, size,
+				    1000, GFP_NOIO);
 }
 
 static int set_registers(rtl8150_t * dev, u16 indx, u16 size, const void *data)
 {
-	void *buf;
-	int ret;
-
-	buf = kmemdup(data, size, GFP_NOIO);
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-			      RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
-			      indx, 0, buf, size, 500);
-	kfree(buf);
-	return ret;
+	return usb_control_msg_send(dev->udev, 0, RTL8150_REQ_SET_REGS,
+				    RTL8150_REQT_WRITE, indx, 0, data, size,
+				    1000, GFP_NOIO);
 }
 
 static void async_set_reg_cb(struct urb *urb)
-- 
2.28.0

Re: [PATCH RESEND v3 0/2] Use the new usb control message API.

[David Miller]

From: Petko Manolov <petko.manolov@konsulko.com>
Date: Sun, 27 Sep 2020 15:49:07 +0300

> Re-sending these, now CC-ing the folks at linux-netdev.

I can't apply these since the helpers do not exist in the
networking tree.

Re: [PATCH RESEND v3 0/2] Use the new usb control message API.

[Petko Manolov]

On 20-09-28 16:00:58, David Miller wrote:
> From: Petko Manolov <petko.manolov@konsulko.com> Date: Sun, 27 Sep 2020 
> 15:49:07 +0300
> 
> > Re-sending these, now CC-ing the folks at linux-netdev.
> 
> I can't apply these since the helpers do not exist in the networking tree.

Right, Greg was only asking for ack (or nack) from your side.


cheers,
Petko

Re: [PATCH RESEND v3 0/2] Use the new usb control message API.

[David Miller]

From: Petko Manolov <petko.manolov@konsulko.com>
Date: Tue, 29 Sep 2020 07:59:11 +0300

> On 20-09-28 16:00:58, David Miller wrote:
>> From: Petko Manolov <petko.manolov@konsulko.com> Date: Sun, 27 Sep 2020 
>> 15:49:07 +0300
>> 
>> > Re-sending these, now CC-ing the folks at linux-netdev.
>> 
>> I can't apply these since the helpers do not exist in the networking tree.
> 
> Right, Greg was only asking for ack (or nack) from your side.

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH RESEND v3 0/2] Use the new usb control message API.

[Greg KH]

On Tue, Sep 29, 2020 at 12:58:49PM -0700, David Miller wrote:
> From: Petko Manolov <petko.manolov@konsulko.com>
> Date: Tue, 29 Sep 2020 07:59:11 +0300
> 
> > On 20-09-28 16:00:58, David Miller wrote:
> >> From: Petko Manolov <petko.manolov@konsulko.com> Date: Sun, 27 Sep 2020 
> >> 15:49:07 +0300
> >> 
> >> > Re-sending these, now CC-ing the folks at linux-netdev.
> >> 
> >> I can't apply these since the helpers do not exist in the networking tree.
> > 
> > Right, Greg was only asking for ack (or nack) from your side.
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Thanks!

[RFC 09/14] sound: usx2y: move to use usb_control_msg_send()

[Oliver Neukum]

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

The usb_control_msg_send() call can handle data on the stack, as well as
returning an error if a "short" write happens, so move the driver over
to using that call instead.  This ends up removing a helper function
that is no longer needed.

v2: API change in usb_control_msg_send()

Cc: Jaroslav Kysela <perex@perex.cz>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20200914153756.3412156-7-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 sound/usb/usx2y/us122l.c | 44 +++++++++-----------------------------------
 1 file changed, 9 insertions(+), 35 deletions(-)

diff --git a/sound/usb/usx2y/us122l.c b/sound/usb/usx2y/us122l.c
index f86f7a61fb36..3614e6802dc1 100644
--- a/sound/usb/usx2y/us122l.c
+++ b/sound/usb/usx2y/us122l.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
+// SPDX-License-Identifier: GPL-2.0-or-late
 /*
  * Copyright (C) 2007, 2008 Karsten Wiese <fzu@wemgehoertderstaat.de>
  */
@@ -82,40 +82,13 @@ static int us144_create_usbmidi(struct snd_card *card)
 				  &US122L(card)->midi_list, &quirk);
 }
 
-/*
- * Wrapper for usb_control_msg().
- * Allocates a temp buffer to prevent dmaing from/to the stack.
- */
-static int us122l_ctl_msg(struct usb_device *dev, unsigned int pipe,
-			  __u8 request, __u8 requesttype,
-			  __u16 value, __u16 index, void *data,
-			  __u16 size, int timeout)
-{
-	int err;
-	void *buf = NULL;
-
-	if (size > 0) {
-		buf = kmemdup(data, size, GFP_KERNEL);
-		if (!buf)
-			return -ENOMEM;
-	}
-	err = usb_control_msg(dev, pipe, request, requesttype,
-			      value, index, buf, size, timeout);
-	if (size > 0) {
-		memcpy(data, buf, size);
-		kfree(buf);
-	}
-	return err;
-}
-
 static void pt_info_set(struct usb_device *dev, u8 v)
 {
 	int ret;
 
-	ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
-			      'I',
-			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-			      v, 0, NULL, 0, 1000);
+	ret = usb_control_msg_send(dev, 0, 'I',
+				   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+				   v, 0, NULL, 0, 1000, GFP_NOIO);
 	snd_printdd(KERN_DEBUG "%i\n", ret);
 }
 
@@ -305,10 +278,11 @@ static int us122l_set_sample_rate(struct usb_device *dev, int rate)
 	data[0] = rate;
 	data[1] = rate >> 8;
 	data[2] = rate >> 16;
-	err = us122l_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC_SET_CUR,
-			     USB_TYPE_CLASS|USB_RECIP_ENDPOINT|USB_DIR_OUT,
-			     UAC_EP_CS_ATTR_SAMPLE_RATE << 8, ep, data, 3, 1000);
-	if (err < 0)
+	err = usb_control_msg_send(dev, 0, UAC_SET_CUR,
+				   USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_OUT,
+				   UAC_EP_CS_ATTR_SAMPLE_RATE << 8, ep, data, 3,
+				   1000, GFP_NOIO);
+	if (err)
 		snd_printk(KERN_ERR "%d: cannot set freq %d to ep 0x%x\n",
 			   dev->devnum, rate, ep);
 	return err;
-- 
2.16.4

Re: [RFC 09/14] sound: usx2y: move to use usb_control_msg_send()

[Greg KH]

On Wed, Sep 23, 2020 at 03:43:43PM +0200, Oliver Neukum wrote:
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> The usb_control_msg_send() call can handle data on the stack, as well as
> returning an error if a "short" write happens, so move the driver over
> to using that call instead.  This ends up removing a helper function
> that is no longer needed.
> 
> v2: API change in usb_control_msg_send()
> 
> Cc: Jaroslav Kysela <perex@perex.cz>
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
> Link: https://lore.kernel.org/r/20200914153756.3412156-7-gregkh@linuxfoundation.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  sound/usb/usx2y/us122l.c | 44 +++++++++-----------------------------------
>  1 file changed, 9 insertions(+), 35 deletions(-)
> 
> diff --git a/sound/usb/usx2y/us122l.c b/sound/usb/usx2y/us122l.c
> index f86f7a61fb36..3614e6802dc1 100644
> --- a/sound/usb/usx2y/us122l.c
> +++ b/sound/usb/usx2y/us122l.c
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> +// SPDX-License-Identifier: GPL-2.0-or-late

Odd, my original patch did not change this line :)

I'll go drop this from the patch...

thanks,

greg k-h

[RFC 10/14] sound: 6fire: move to use usb_control_msg_send() and usb_control_msg_recv()

[Oliver Neukum]

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

The usb_control_msg_send() and usb_control_msg_recv() calls can return
an error if a "short" write/read happens, so move the driver over to
using those calls instead, saving some logic in the wrapper functions
that were being used in this driver.

This also resolves a long-staging bug where data on the stack was being
sent in a USB control message, which was not allowed.

v2: API change of usb_control_msg_send()

Cc: Jaroslav Kysela <perex@perex.cz>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20200914153756.3412156-8-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 sound/usb/6fire/firmware.c | 38 +++++++++++++-------------------------
 1 file changed, 13 insertions(+), 25 deletions(-)

diff --git a/sound/usb/6fire/firmware.c b/sound/usb/6fire/firmware.c
index 69137c14d0dc..8981e61f2da4 100644
--- a/sound/usb/6fire/firmware.c
+++ b/sound/usb/6fire/firmware.c
@@ -158,29 +158,17 @@ static int usb6fire_fw_ihex_init(const struct firmware *fw,
 static int usb6fire_fw_ezusb_write(struct usb_device *device,
 		int type, int value, char *data, int len)
 {
-	int ret;
-
-	ret = usb_control_msg(device, usb_sndctrlpipe(device, 0), type,
-			USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-			value, 0, data, len, HZ);
-	if (ret < 0)
-		return ret;
-	else if (ret != len)
-		return -EIO;
-	return 0;
+	return usb_control_msg_send(device, 0, type,
+				    USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+				    value, 0, data, len, HZ, GFP_KERNEL);
 }
 
 static int usb6fire_fw_ezusb_read(struct usb_device *device,
 		int type, int value, char *data, int len)
 {
-	int ret = usb_control_msg(device, usb_rcvctrlpipe(device, 0), type,
-			USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, value,
-			0, data, len, HZ);
-	if (ret < 0)
-		return ret;
-	else if (ret != len)
-		return -EIO;
-	return 0;
+	return usb_control_msg_recv(device, 0, type,
+				    USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+				    value, 0, data, len, HZ, GFP_KERNEL);
 }
 
 static int usb6fire_fw_fpga_write(struct usb_device *device,
@@ -230,7 +218,7 @@ static int usb6fire_fw_ezusb_upload(
 	/* upload firmware image */
 	data = 0x01; /* stop ezusb cpu */
 	ret = usb6fire_fw_ezusb_write(device, 0xa0, 0xe600, &data, 1);
-	if (ret < 0) {
+	if (ret) {
 		kfree(rec);
 		release_firmware(fw);
 		dev_err(&intf->dev,
@@ -242,7 +230,7 @@ static int usb6fire_fw_ezusb_upload(
 	while (usb6fire_fw_ihex_next_record(rec)) { /* write firmware */
 		ret = usb6fire_fw_ezusb_write(device, 0xa0, rec->address,
 				rec->data, rec->len);
-		if (ret < 0) {
+		if (ret) {
 			kfree(rec);
 			release_firmware(fw);
 			dev_err(&intf->dev,
@@ -257,7 +245,7 @@ static int usb6fire_fw_ezusb_upload(
 	if (postdata) { /* write data after firmware has been uploaded */
 		ret = usb6fire_fw_ezusb_write(device, 0xa0, postaddr,
 				postdata, postlen);
-		if (ret < 0) {
+		if (ret) {
 			dev_err(&intf->dev,
 				"unable to upload ezusb firmware %s: post urb.\n",
 				fwname);
@@ -267,7 +255,7 @@ static int usb6fire_fw_ezusb_upload(
 
 	data = 0x00; /* resume ezusb cpu */
 	ret = usb6fire_fw_ezusb_write(device, 0xa0, 0xe600, &data, 1);
-	if (ret < 0) {
+	if (ret) {
 		dev_err(&intf->dev,
 			"unable to upload ezusb firmware %s: end message.\n",
 			fwname);
@@ -302,7 +290,7 @@ static int usb6fire_fw_fpga_upload(
 	end = fw->data + fw->size;
 
 	ret = usb6fire_fw_ezusb_write(device, 8, 0, NULL, 0);
-	if (ret < 0) {
+	if (ret) {
 		kfree(buffer);
 		release_firmware(fw);
 		dev_err(&intf->dev,
@@ -327,7 +315,7 @@ static int usb6fire_fw_fpga_upload(
 	kfree(buffer);
 
 	ret = usb6fire_fw_ezusb_write(device, 9, 0, NULL, 0);
-	if (ret < 0) {
+	if (ret) {
 		dev_err(&intf->dev,
 			"unable to upload fpga firmware: end urb.\n");
 		return ret;
@@ -363,7 +351,7 @@ int usb6fire_fw_init(struct usb_interface *intf)
 	u8 buffer[12];
 
 	ret = usb6fire_fw_ezusb_read(device, 1, 0, buffer, 8);
-	if (ret < 0) {
+	if (ret) {
 		dev_err(&intf->dev,
 			"unable to receive device firmware state.\n");
 		return ret;
-- 
2.16.4

[RFC 11/14] USB: legousbtower: use usb_control_msg_recv()

[Oliver Neukum]

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

The usb_control_msg_recv() function can handle data on the stack, as
well as properly detecting short reads, so move to use that function
instead of the older usb_control_msg() call.  This ends up removing a
lot of extra lines in the driver.

v2: change API of usb_control_msg_send()

Cc: Juergen Stuber <starblue@users.sourceforge.net>
Link: https://lore.kernel.org/r/20200914153756.3412156-6-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/misc/legousbtower.c | 61 ++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 41 deletions(-)

diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index f922544056de..ba655b4af4fc 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -308,15 +308,9 @@ static int tower_open(struct inode *inode, struct file *file)
 	int subminor;
 	int retval = 0;
 	struct usb_interface *interface;
-	struct tower_reset_reply *reset_reply;
+	struct tower_reset_reply reset_reply;
 	int result;
 
-	reset_reply = kmalloc(sizeof(*reset_reply), GFP_KERNEL);
-	if (!reset_reply) {
-		retval = -ENOMEM;
-		goto exit;
-	}
-
 	nonseekable_open(inode, file);
 	subminor = iminor(inode);
 
@@ -347,15 +341,12 @@ static int tower_open(struct inode *inode, struct file *file)
 	}
 
 	/* reset the tower */
-	result = usb_control_msg(dev->udev,
-				 usb_rcvctrlpipe(dev->udev, 0),
-				 LEGO_USB_TOWER_REQUEST_RESET,
-				 USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
-				 0,
-				 0,
-				 reset_reply,
-				 sizeof(*reset_reply),
-				 1000);
+	result = usb_control_msg_recv(dev->udev, 0,
+				      LEGO_USB_TOWER_REQUEST_RESET,
+				      USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
+				      0, 0,
+				      &reset_reply, sizeof(reset_reply), 1000,
+				      GFP_KERNEL);
 	if (result < 0) {
 		dev_err(&dev->udev->dev,
 			"LEGO USB Tower reset control request failed\n");
@@ -394,7 +385,6 @@ static int tower_open(struct inode *inode, struct file *file)
 	mutex_unlock(&dev->lock);
 
 exit:
-	kfree(reset_reply);
 	return retval;
 }
 
@@ -753,7 +743,7 @@ static int tower_probe(struct usb_interface *interface, const struct usb_device_
 	struct device *idev = &interface->dev;
 	struct usb_device *udev = interface_to_usbdev(interface);
 	struct lego_usb_tower *dev;
-	struct tower_get_version_reply *get_version_reply = NULL;
+	struct tower_get_version_reply get_version_reply;
 	int retval = -ENOMEM;
 	int result;
 
@@ -798,34 +788,25 @@ static int tower_probe(struct usb_interface *interface, const struct usb_device_
 	dev->interrupt_in_interval = interrupt_in_interval ? interrupt_in_interval : dev->interrupt_in_endpoint->bInterval;
 	dev->interrupt_out_interval = interrupt_out_interval ? interrupt_out_interval : dev->interrupt_out_endpoint->bInterval;
 
-	get_version_reply = kmalloc(sizeof(*get_version_reply), GFP_KERNEL);
-	if (!get_version_reply) {
-		retval = -ENOMEM;
-		goto error;
-	}
-
 	/* get the firmware version and log it */
-	result = usb_control_msg(udev,
-				 usb_rcvctrlpipe(udev, 0),
-				 LEGO_USB_TOWER_REQUEST_GET_VERSION,
-				 USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
-				 0,
-				 0,
-				 get_version_reply,
-				 sizeof(*get_version_reply),
-				 1000);
-	if (result != sizeof(*get_version_reply)) {
-		if (result >= 0)
-			result = -EIO;
+	result = usb_control_msg_recv(udev, 0,
+				      LEGO_USB_TOWER_REQUEST_GET_VERSION,
+				      USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
+				      0,
+				      0,
+				      &get_version_reply,
+				      sizeof(get_version_reply),
+				      1000, GFP_KERNEL);
+	if (!result) {
 		dev_err(idev, "get version request failed: %d\n", result);
 		retval = result;
 		goto error;
 	}
 	dev_info(&interface->dev,
 		 "LEGO USB Tower firmware version is %d.%d build %d\n",
-		 get_version_reply->major,
-		 get_version_reply->minor,
-		 le16_to_cpu(get_version_reply->build_no));
+		 get_version_reply.major,
+		 get_version_reply.minor,
+		 le16_to_cpu(get_version_reply.build_no));
 
 	/* we can register the device now, as it is ready */
 	usb_set_intfdata(interface, dev);
@@ -844,11 +825,9 @@ static int tower_probe(struct usb_interface *interface, const struct usb_device_
 		 USB_MAJOR, dev->minor);
 
 exit:
-	kfree(get_version_reply);
 	return retval;
 
 error:
-	kfree(get_version_reply);
 	tower_delete(dev);
 	return retval;
 }
-- 
2.16.4

[RFC 12/14] sound: line6: move to use usb_control_msg_send() and usb_control_msg_recv()

[Oliver Neukum]

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

The usb_control_msg_send() and usb_control_msg_recv() calls can return
an error if a "short" write/read happens, and they can handle data off
of the stack, so move the driver over to using those calls instead,
saving some logic when dynamically allocating memory.

v2: API change of use usb_control_msg_send() and usb_control_msg_recv()

Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Vasily Khoruzhick <anarsoul@gmail.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20200914153756.3412156-9-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 sound/usb/line6/driver.c   | 72 +++++++++++++++++++---------------------------
 sound/usb/line6/podhd.c    | 23 ++++++---------
 sound/usb/line6/toneport.c |  9 +++---
 3 files changed, 44 insertions(+), 60 deletions(-)

diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index 60674ce4879b..a030dd65eb28 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -337,23 +337,18 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
 {
 	struct usb_device *usbdev = line6->usbdev;
 	int ret;
-	unsigned char *len;
+	u8 len;
 	unsigned count;
 
 	if (address > 0xffff || datalen > 0xff)
 		return -EINVAL;
 
-	len = kmalloc(1, GFP_KERNEL);
-	if (!len)
-		return -ENOMEM;
-
 	/* query the serial number: */
-	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
-			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
-			      (datalen << 8) | 0x21, address,
-			      NULL, 0, LINE6_TIMEOUT * HZ);
-
-	if (ret < 0) {
+	ret = usb_control_msg_send(usbdev, 0, 0x67,
+				   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+				   (datalen << 8) | 0x21, address, NULL, 0,
+				   LINE6_TIMEOUT * HZ, GFP_KERNEL);
+	if (ret) {
 		dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
 		goto exit;
 	}
@@ -362,45 +357,42 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data,
 	for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
 		mdelay(LINE6_READ_WRITE_STATUS_DELAY);
 
-		ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
-				      USB_TYPE_VENDOR | USB_RECIP_DEVICE |
-				      USB_DIR_IN,
-				      0x0012, 0x0000, len, 1,
-				      LINE6_TIMEOUT * HZ);
-		if (ret < 0) {
+		ret = usb_control_msg_recv(usbdev, 0, 0x67,
+					   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+					   0x0012, 0x0000, &len, 1,
+					   LINE6_TIMEOUT * HZ, GFP_KERNEL);
+		if (ret) {
 			dev_err(line6->ifcdev,
 				"receive length failed (error %d)\n", ret);
 			goto exit;
 		}
 
-		if (*len != 0xff)
+		if (len != 0xff)
 			break;
 	}
 
 	ret = -EIO;
-	if (*len == 0xff) {
+	if (len == 0xff) {
 		dev_err(line6->ifcdev, "read failed after %d retries\n",
 			count);
 		goto exit;
-	} else if (*len != datalen) {
+	} else if (len != datalen) {
 		/* should be equal or something went wrong */
 		dev_err(line6->ifcdev,
 			"length mismatch (expected %d, got %d)\n",
-			(int)datalen, (int)*len);
+			(int)datalen, len);
 		goto exit;
 	}
 
 	/* receive the result: */
-	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
-			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
-			      0x0013, 0x0000, data, datalen,
-			      LINE6_TIMEOUT * HZ);
-
-	if (ret < 0)
+	ret = usb_control_msg_recv(usbdev, 0, 0x67,
+				   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+				   0x0013, 0x0000, data, datalen, LINE6_TIMEOUT * HZ,
+				   GFP_KERNEL);
+	if (ret)
 		dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
 
 exit:
-	kfree(len);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(line6_read_data);
@@ -423,12 +415,11 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
 	if (!status)
 		return -ENOMEM;
 
-	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
-			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
-			      0x0022, address, data, datalen,
-			      LINE6_TIMEOUT * HZ);
-
-	if (ret < 0) {
+	ret = usb_control_msg_send(usbdev, 0, 0x67,
+				   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+				   0x0022, address, data, datalen, LINE6_TIMEOUT * HZ,
+				   GFP_KERNEL);
+	if (ret) {
 		dev_err(line6->ifcdev,
 			"write request failed (error %d)\n", ret);
 		goto exit;
@@ -437,14 +428,11 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data,
 	for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
 		mdelay(LINE6_READ_WRITE_STATUS_DELAY);
 
-		ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
-				      0x67,
-				      USB_TYPE_VENDOR | USB_RECIP_DEVICE |
-				      USB_DIR_IN,
-				      0x0012, 0x0000,
-				      status, 1, LINE6_TIMEOUT * HZ);
-
-		if (ret < 0) {
+		ret = usb_control_msg_recv(usbdev, 0, 0x67,
+					   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+					   0x0012, 0x0000, status, 1, LINE6_TIMEOUT * HZ,
+					   GFP_KERNEL);
+		if (ret) {
 			dev_err(line6->ifcdev,
 				"receiving status failed (error %d)\n", ret);
 			goto exit;
diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c
index eef45f7fef0d..28794a35949d 100644
--- a/sound/usb/line6/podhd.c
+++ b/sound/usb/line6/podhd.c
@@ -183,29 +183,25 @@ static const struct attribute_group podhd_dev_attr_group = {
 static int podhd_dev_start(struct usb_line6_podhd *pod)
 {
 	int ret;
-	u8 *init_bytes;
+	u8 init_bytes[8];
 	int i;
 	struct usb_device *usbdev = pod->line6.usbdev;
 
-	init_bytes = kmalloc(8, GFP_KERNEL);
-	if (!init_bytes)
-		return -ENOMEM;
-
-	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+	ret = usb_control_msg_send(usbdev, 0,
 					0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
 					0x11, 0,
-					NULL, 0, LINE6_TIMEOUT * HZ);
-	if (ret < 0) {
+					NULL, 0, LINE6_TIMEOUT * HZ, GFP_KERNEL);
+	if (ret) {
 		dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
 		goto exit;
 	}
 
 	/* NOTE: looks like some kind of ping message */
-	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
+	ret = usb_control_msg_recv(usbdev, 0, 0x67,
 					USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
 					0x11, 0x0,
-					init_bytes, 3, LINE6_TIMEOUT * HZ);
-	if (ret < 0) {
+					init_bytes, 3, LINE6_TIMEOUT * HZ, GFP_KERNEL);
+	if (ret) {
 		dev_err(pod->line6.ifcdev,
 			"receive length failed (error %d)\n", ret);
 		goto exit;
@@ -220,13 +216,12 @@ static int podhd_dev_start(struct usb_line6_podhd *pod)
 			goto exit;
 	}
 
-	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+	ret = usb_control_msg_send(usbdev, 0,
 					USB_REQ_SET_FEATURE,
 					USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
 					1, 0,
-					NULL, 0, LINE6_TIMEOUT * HZ);
+					NULL, 0, LINE6_TIMEOUT * HZ, GFP_KERNEL);
 exit:
-	kfree(init_bytes);
 	return ret;
 }
 
diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c
index 94dd5e7ab2e6..4e5693c97aa4 100644
--- a/sound/usb/line6/toneport.c
+++ b/sound/usb/line6/toneport.c
@@ -126,11 +126,12 @@ static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2)
 {
 	int ret;
 
-	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
-			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
-			      cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
+	ret = usb_control_msg_send(usbdev, 0, 0x67,
+				   USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+				   cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ,
+				   GFP_KERNEL);
 
-	if (ret < 0) {
+	if (ret) {
 		dev_err(&usbdev->dev, "send failed (error %d)\n", ret);
 		return ret;
 	}
-- 
2.16.4

[RFC 13/14] sound: hiface: move to use usb_control_msg_send()

[Oliver Neukum]

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

The usb_control_msg_send() call can return an error if a "short" write
happens, so move the driver over to using that call instead.

v2: API change of use usb_control_msg_send()

Cc: Jaroslav Kysela <perex@perex.cz>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20200914153756.3412156-10-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 sound/usb/hiface/pcm.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c
index a148caa5f48e..d942179ca095 100644
--- a/sound/usb/hiface/pcm.c
+++ b/sound/usb/hiface/pcm.c
@@ -156,16 +156,14 @@ static int hiface_pcm_set_rate(struct pcm_runtime *rt, unsigned int rate)
 	 * This control message doesn't have any ack from the
 	 * other side
 	 */
-	ret = usb_control_msg(device, usb_sndctrlpipe(device, 0),
-			      HIFACE_SET_RATE_REQUEST,
-			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
-			      rate_value, 0, NULL, 0, 100);
-	if (ret < 0) {
+	ret = usb_control_msg_send(device, 0,
+				   HIFACE_SET_RATE_REQUEST,
+				   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
+				   rate_value, 0, NULL, 0, 100, GFP_KERNEL);
+	if (ret)
 		dev_err(&device->dev, "Error setting samplerate %d.\n", rate);
-		return ret;
-	}
 
-	return 0;
+	return ret;
 }
 
 static struct pcm_substream *hiface_pcm_get_substream(struct snd_pcm_substream
-- 
2.16.4

[RFC 14/14] Bluetooth: ath3k: use usb_control_msg_send() and usb_control_msg_recv()

[Oliver Neukum]

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

The usb_control_msg_send() and usb_control_msg_recv() calls can return
an error if a "short" write/read happens, and they can handle data off
of the stack, so move the driver over to using those calls instead,
saving some logic when dynamically allocating memory.

v2: changed API of use usb_control_msg_send() and usb_control_msg_recv()

Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Link: https://lore.kernel.org/r/20200914153756.3412156-11-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/bluetooth/ath3k.c | 93 +++++++++++++++--------------------------------
 1 file changed, 29 insertions(+), 64 deletions(-)

diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
index 4ce270513695..759d7828931d 100644
--- a/drivers/bluetooth/ath3k.c
+++ b/drivers/bluetooth/ath3k.c
@@ -212,19 +212,16 @@ static int ath3k_load_firmware(struct usb_device *udev,
 
 	BT_DBG("udev %p", udev);
 
-	pipe = usb_sndctrlpipe(udev, 0);
-
 	send_buf = kmalloc(BULK_SIZE, GFP_KERNEL);
 	if (!send_buf) {
 		BT_ERR("Can't allocate memory chunk for firmware");
 		return -ENOMEM;
 	}
 
-	memcpy(send_buf, firmware->data, FW_HDR_SIZE);
-	err = usb_control_msg(udev, pipe, USB_REQ_DFU_DNLOAD, USB_TYPE_VENDOR,
-			      0, 0, send_buf, FW_HDR_SIZE,
-			      USB_CTRL_SET_TIMEOUT);
-	if (err < 0) {
+	err = usb_control_msg_send(udev, 0, USB_REQ_DFU_DNLOAD, USB_TYPE_VENDOR,
+				   0, 0, firmware->data, FW_HDR_SIZE,
+				   USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
+	if (err) {
 		BT_ERR("Can't change to loading configuration err");
 		goto error;
 	}
@@ -259,44 +256,19 @@ static int ath3k_load_firmware(struct usb_device *udev,
 
 static int ath3k_get_state(struct usb_device *udev, unsigned char *state)
 {
-	int ret, pipe = 0;
-	char *buf;
-
-	buf = kmalloc(sizeof(*buf), GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	pipe = usb_rcvctrlpipe(udev, 0);
-	ret = usb_control_msg(udev, pipe, ATH3K_GETSTATE,
-			      USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
-			      buf, sizeof(*buf), USB_CTRL_SET_TIMEOUT);
-
-	*state = *buf;
-	kfree(buf);
-
-	return ret;
+	return usb_control_msg_recv(udev, 0, ATH3K_GETSTATE,
+				    USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
+				    state, 1, USB_CTRL_SET_TIMEOUT,
+				    GFP_KERNEL);
 }
 
 static int ath3k_get_version(struct usb_device *udev,
 			struct ath3k_version *version)
 {
-	int ret, pipe = 0;
-	struct ath3k_version *buf;
-	const int size = sizeof(*buf);
-
-	buf = kmalloc(size, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	pipe = usb_rcvctrlpipe(udev, 0);
-	ret = usb_control_msg(udev, pipe, ATH3K_GETVERSION,
-			      USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
-			      buf, size, USB_CTRL_SET_TIMEOUT);
-
-	memcpy(version, buf, size);
-	kfree(buf);
-
-	return ret;
+	return usb_control_msg_recv(udev, 0, ATH3K_GETVERSION,
+				    USB_TYPE_VENDOR | USB_DIR_IN, 0, 0,
+				    version, sizeof(*version), USB_CTRL_SET_TIMEOUT,
+				    GFP_KERNEL);
 }
 
 static int ath3k_load_fwfile(struct usb_device *udev,
@@ -316,13 +288,11 @@ static int ath3k_load_fwfile(struct usb_device *udev,
 	}
 
 	size = min_t(uint, count, FW_HDR_SIZE);
-	memcpy(send_buf, firmware->data, size);
 
-	pipe = usb_sndctrlpipe(udev, 0);
-	ret = usb_control_msg(udev, pipe, ATH3K_DNLOAD,
-			USB_TYPE_VENDOR, 0, 0, send_buf,
-			size, USB_CTRL_SET_TIMEOUT);
-	if (ret < 0) {
+	ret = usb_control_msg_send(udev, 0, ATH3K_DNLOAD, USB_TYPE_VENDOR, 0, 0,
+				   firmware->data, size, USB_CTRL_SET_TIMEOUT,
+				   GFP_KERNEL);
+	if (ret) {
 		BT_ERR("Can't change to loading configuration err");
 		kfree(send_buf);
 		return ret;
@@ -355,23 +325,19 @@ static int ath3k_load_fwfile(struct usb_device *udev,
 	return 0;
 }
 
-static int ath3k_switch_pid(struct usb_device *udev)
+static void ath3k_switch_pid(struct usb_device *udev)
 {
-	int pipe = 0;
-
-	pipe = usb_sndctrlpipe(udev, 0);
-	return usb_control_msg(udev, pipe, USB_REG_SWITCH_VID_PID,
-			USB_TYPE_VENDOR, 0, 0,
-			NULL, 0, USB_CTRL_SET_TIMEOUT);
+	usb_control_msg_send(udev, 0, USB_REG_SWITCH_VID_PID, USB_TYPE_VENDOR,
+			     0, 0, NULL, 0, USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
 }
 
 static int ath3k_set_normal_mode(struct usb_device *udev)
 {
 	unsigned char fw_state;
-	int pipe = 0, ret;
+	int ret;
 
 	ret = ath3k_get_state(udev, &fw_state);
-	if (ret < 0) {
+	if (ret) {
 		BT_ERR("Can't get state to change to normal mode err");
 		return ret;
 	}
@@ -381,10 +347,9 @@ static int ath3k_set_normal_mode(struct usb_device *udev)
 		return 0;
 	}
 
-	pipe = usb_sndctrlpipe(udev, 0);
-	return usb_control_msg(udev, pipe, ATH3K_SET_NORMAL_MODE,
-			USB_TYPE_VENDOR, 0, 0,
-			NULL, 0, USB_CTRL_SET_TIMEOUT);
+	return usb_control_msg_send(udev, 0, ATH3K_SET_NORMAL_MODE,
+				    USB_TYPE_VENDOR, 0, 0, NULL, 0,
+				    USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
 }
 
 static int ath3k_load_patch(struct usb_device *udev)
@@ -397,7 +362,7 @@ static int ath3k_load_patch(struct usb_device *udev)
 	int ret;
 
 	ret = ath3k_get_state(udev, &fw_state);
-	if (ret < 0) {
+	if (ret) {
 		BT_ERR("Can't get state to change to load ram patch err");
 		return ret;
 	}
@@ -408,7 +373,7 @@ static int ath3k_load_patch(struct usb_device *udev)
 	}
 
 	ret = ath3k_get_version(udev, &fw_version);
-	if (ret < 0) {
+	if (ret) {
 		BT_ERR("Can't get version to change to load ram patch err");
 		return ret;
 	}
@@ -449,13 +414,13 @@ static int ath3k_load_syscfg(struct usb_device *udev)
 	int clk_value, ret;
 
 	ret = ath3k_get_state(udev, &fw_state);
-	if (ret < 0) {
+	if (ret) {
 		BT_ERR("Can't get state to change to load configuration err");
 		return -EBUSY;
 	}
 
 	ret = ath3k_get_version(udev, &fw_version);
-	if (ret < 0) {
+	if (ret) {
 		BT_ERR("Can't get version to change to load ram patch err");
 		return ret;
 	}
@@ -529,7 +494,7 @@ static int ath3k_probe(struct usb_interface *intf,
 			return ret;
 		}
 		ret = ath3k_set_normal_mode(udev);
-		if (ret < 0) {
+		if (ret) {
 			BT_ERR("Set normal mode failed");
 			return ret;
 		}
-- 
2.16.4

Re: [RFC] change the new message to provide for memory allocations

[Greg KH]

On Wed, Sep 23, 2020 at 03:43:34PM +0200, Oliver Neukum wrote:
> Control messages have to work in situtaions where you need to
> use GFP_NOIO. Hence wrappers for them that allocate memory must have
> an API that allows for that.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> 

So you unwind my mess and then fix it back up, nice, no objections from
me, thanks for doing this.

You sent this as "RFC", but it seems sane, want me to take this as-is or
do you want to wait to send a "real" set of patches?

thanks,

greg k-h

Re: [RFC] change the new message to provide for memory allocations

[Oliver Neukum]

Am Mittwoch, den 23.09.2020, 19:21 +0200 schrieb Greg KH:
> On Wed, Sep 23, 2020 at 03:43:34PM +0200, Oliver Neukum wrote:
> > Control messages have to work in situtaions where you need to
> > use GFP_NOIO. Hence wrappers for them that allocate memory must have
> > an API that allows for that.
> > 
> > Signed-off-by: Oliver Neukum <oneukum@suse.com>
> > 
> 
> So you unwind my mess and then fix it back up, nice, no objections from
> me, thanks for doing this.
> 
> You sent this as "RFC", but it seems sane, want me to take this as-is or
> do you want to wait to send a "real" set of patches?

Hi,

this is a fairly fundamental API. If we overlooked something the first
time I would hate to see it happen twice. Are you in a hurry?

	Regards
		Oliver

Re: [RFC] change the new message to provide for memory allocations

[Greg KH]

On Wed, Sep 23, 2020 at 07:32:11PM +0200, Oliver Neukum wrote:
> Am Mittwoch, den 23.09.2020, 19:21 +0200 schrieb Greg KH:
> > On Wed, Sep 23, 2020 at 03:43:34PM +0200, Oliver Neukum wrote:
> > > Control messages have to work in situtaions where you need to
> > > use GFP_NOIO. Hence wrappers for them that allocate memory must have
> > > an API that allows for that.
> > > 
> > > Signed-off-by: Oliver Neukum <oneukum@suse.com>
> > > 
> > 
> > So you unwind my mess and then fix it back up, nice, no objections from
> > me, thanks for doing this.
> > 
> > You sent this as "RFC", but it seems sane, want me to take this as-is or
> > do you want to wait to send a "real" set of patches?
> 
> Hi,
> 
> this is a fairly fundamental API. If we overlooked something the first
> time I would hate to see it happen twice. Are you in a hurry?

No, I just don't want to see more people build on this as-is :)