* [PATCH v4 0/3] drivers: usb: misc: update to use usb_control_msg_{send|recv}() API
@ 2021-03-26 22:32 Anant Thazhemadam
2021-03-26 22:32 ` [PATCH v4 1/3] usb: misc: ehset: update to use the " Anant Thazhemadam
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Anant Thazhemadam @ 2021-03-26 22:32 UTC (permalink / raw)
To: Greg Kroah-Hartman, Anant Thazhemadam, Johan Hovold,
Minas Harutyunyan, Peter Chen
Cc: linux-usb, linux-kernel
The new usb_control_msg_{send|recv}() API provides a more convenient way
of using usb_control_msg() in some usecases. Using this, short reads are
considered as errors, data can be used off the stack, and the need for
the calling function to create a raw usb pipe is eliminated.
This patch series aims to update existing instances of usb_control_msg()
in drivers/usb/misc/* to usb_control_msg_{send|recv}() appropriately
wherever it would be a good fit, and also update the return value
checking mechanisms in place (if any), as necessary so nothing breaks.
Changes in v4:
* Drop all proposed changes to drivers (from v3) where the new API
doesn't fit appropriately.
* Update commit messages.
* Link to v3:
https://lore.kernel.org/linux-usb/YBF9exziI12OCSuA@hovoldconsulting.com/T/#m269ab33b52331c134bbbc77d13cb65c2194a6093
Changes in v3:
* idmouse, emi26 and emi62 are left unchanged, and are not updated.
-> since control transfers in idmouse are without a data stage, there's no
real advantage in using the new helper here.
-> in emi26, and emi62, FW_LOAD_SIZE = 1048 (> 1024). Thus, if we try to use the
new helpers, it will result in either build warnings, or memory being allocated.
* Link to v2:
https://lore.kernel.org/linux-usb/20201130013103.2580467-1-anant.thazhemadam@gmail.com/T/
Changes in v2:
* Buffer variables that were previously dynamically allocated are no
longer dynamically allocated unless they have a variable length
(since that threw a warning).
* Link to v1:
https://lore.kernel.org/linux-usb/20201129160612.1908074-1-anant.thazhemadam@gmail.com/
Anant Thazhemadam (3):
usb: misc: ehset: update to use the usb_control_msg_{send|recv}() API
usb: misc: ezusb: update to use usb_control_msg_send()
usb: misc: usbsevseg: update to use usb_control_msg_send()
drivers/usb/misc/ehset.c | 76 +++++++++++++++---------------------
drivers/usb/misc/ezusb.c | 16 +-------
drivers/usb/misc/usbsevseg.c | 60 ++++++++--------------------
3 files changed, 51 insertions(+), 101 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v4 1/3] usb: misc: ehset: update to use the usb_control_msg_{send|recv}() API
2021-03-26 22:32 [PATCH v4 0/3] drivers: usb: misc: update to use usb_control_msg_{send|recv}() API Anant Thazhemadam
@ 2021-03-26 22:32 ` Anant Thazhemadam
2021-03-26 22:32 ` [PATCH v4 2/3] usb: misc: ezusb: update to use usb_control_msg_send() Anant Thazhemadam
2021-03-26 22:32 ` [PATCH v4 3/3] usb: misc: usbsevseg: " Anant Thazhemadam
2 siblings, 0 replies; 4+ messages in thread
From: Anant Thazhemadam @ 2021-03-26 22:32 UTC (permalink / raw)
To: Greg Kroah-Hartman, Peter Chen, Minas Harutyunyan, Chunfeng Yun,
Anant Thazhemadam
Cc: linux-usb, linux-kernel, Johan Hovold
The newer usb_control_msg_{send|recv}() API ensures that a short read
is treated as an error, data can be used off the stack, and raw usb
pipes need not be created in the calling functions.
For this reason, instances of usb_control_msg() have been replaced with
usb_control_msg_{recv|send}() appropriately.
Now, we also test for a short device descriptor (which USB core
should already have fetched if you get to probe this driver), but which
wasn't verified again here before.
Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
Reviewed-by: Peter Chen <peter.chen@nxp.com>
Reviewed-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/misc/ehset.c | 76 +++++++++++++++++-----------------------
1 file changed, 32 insertions(+), 44 deletions(-)
diff --git a/drivers/usb/misc/ehset.c b/drivers/usb/misc/ehset.c
index 2752e1f4f4d0..f87890f9cd26 100644
--- a/drivers/usb/misc/ehset.c
+++ b/drivers/usb/misc/ehset.c
@@ -24,68 +24,57 @@ static int ehset_probe(struct usb_interface *intf,
int ret = -EINVAL;
struct usb_device *dev = interface_to_usbdev(intf);
struct usb_device *hub_udev = dev->parent;
- struct usb_device_descriptor *buf;
+ struct usb_device_descriptor buf;
u8 portnum = dev->portnum;
u16 test_pid = le16_to_cpu(dev->descriptor.idProduct);
switch (test_pid) {
case TEST_SE0_NAK_PID:
- ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
- USB_REQ_SET_FEATURE, USB_RT_PORT,
- USB_PORT_FEAT_TEST,
- (USB_TEST_SE0_NAK << 8) | portnum,
- NULL, 0, 1000);
+ ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+ USB_RT_PORT, USB_PORT_FEAT_TEST,
+ (USB_TEST_SE0_NAK << 8) | portnum,
+ NULL, 0, 1000, GFP_KERNEL);
break;
case TEST_J_PID:
- ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
- USB_REQ_SET_FEATURE, USB_RT_PORT,
- USB_PORT_FEAT_TEST,
- (USB_TEST_J << 8) | portnum,
- NULL, 0, 1000);
+ ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+ USB_RT_PORT, USB_PORT_FEAT_TEST,
+ (USB_TEST_J << 8) | portnum, NULL, 0,
+ 1000, GFP_KERNEL);
break;
case TEST_K_PID:
- ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
- USB_REQ_SET_FEATURE, USB_RT_PORT,
- USB_PORT_FEAT_TEST,
- (USB_TEST_K << 8) | portnum,
- NULL, 0, 1000);
+ ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+ USB_RT_PORT, USB_PORT_FEAT_TEST,
+ (USB_TEST_K << 8) | portnum, NULL, 0,
+ 1000, GFP_KERNEL);
break;
case TEST_PACKET_PID:
- ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
- USB_REQ_SET_FEATURE, USB_RT_PORT,
- USB_PORT_FEAT_TEST,
- (USB_TEST_PACKET << 8) | portnum,
- NULL, 0, 1000);
+ ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+ USB_RT_PORT, USB_PORT_FEAT_TEST,
+ (USB_TEST_PACKET << 8) | portnum,
+ NULL, 0, 1000, GFP_KERNEL);
break;
case TEST_HS_HOST_PORT_SUSPEND_RESUME:
/* Test: wait for 15secs -> suspend -> 15secs delay -> resume */
msleep(15 * 1000);
- ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
- USB_REQ_SET_FEATURE, USB_RT_PORT,
- USB_PORT_FEAT_SUSPEND, portnum,
- NULL, 0, 1000);
+ ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+ USB_RT_PORT, USB_PORT_FEAT_SUSPEND,
+ portnum, NULL, 0, 1000, GFP_KERNEL);
if (ret < 0)
break;
msleep(15 * 1000);
- ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
- USB_REQ_CLEAR_FEATURE, USB_RT_PORT,
- USB_PORT_FEAT_SUSPEND, portnum,
- NULL, 0, 1000);
+ ret = usb_control_msg_send(hub_udev, 0, USB_REQ_CLEAR_FEATURE,
+ USB_RT_PORT, USB_PORT_FEAT_SUSPEND,
+ portnum, NULL, 0, 1000, GFP_KERNEL);
break;
case TEST_SINGLE_STEP_GET_DEV_DESC:
/* Test: wait for 15secs -> GetDescriptor request */
msleep(15 * 1000);
- buf = kmalloc(USB_DT_DEVICE_SIZE, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
- ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
- USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
- USB_DT_DEVICE << 8, 0,
- buf, USB_DT_DEVICE_SIZE,
- USB_CTRL_GET_TIMEOUT);
- kfree(buf);
+ ret = usb_control_msg_recv(dev, 0, USB_REQ_GET_DESCRIPTOR,
+ USB_DIR_IN, USB_DT_DEVICE << 8, 0,
+ &buf, USB_DT_DEVICE_SIZE,
+ USB_CTRL_GET_TIMEOUT, GFP_KERNEL);
break;
case TEST_SINGLE_STEP_SET_FEATURE:
/*
@@ -100,11 +89,10 @@ static int ehset_probe(struct usb_interface *intf,
break;
}
- ret = usb_control_msg(hub_udev, usb_sndctrlpipe(hub_udev, 0),
- USB_REQ_SET_FEATURE, USB_RT_PORT,
- USB_PORT_FEAT_TEST,
- (6 << 8) | portnum,
- NULL, 0, 60 * 1000);
+ ret = usb_control_msg_send(hub_udev, 0, USB_REQ_SET_FEATURE,
+ USB_RT_PORT, USB_PORT_FEAT_TEST,
+ (6 << 8) | portnum, NULL, 0,
+ 60 * 1000, GFP_KERNEL);
break;
default:
@@ -112,7 +100,7 @@ static int ehset_probe(struct usb_interface *intf,
__func__, test_pid);
}
- return (ret < 0) ? ret : 0;
+ return ret;
}
static void ehset_disconnect(struct usb_interface *intf)
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v4 2/3] usb: misc: ezusb: update to use usb_control_msg_send()
2021-03-26 22:32 [PATCH v4 0/3] drivers: usb: misc: update to use usb_control_msg_{send|recv}() API Anant Thazhemadam
2021-03-26 22:32 ` [PATCH v4 1/3] usb: misc: ehset: update to use the " Anant Thazhemadam
@ 2021-03-26 22:32 ` Anant Thazhemadam
2021-03-26 22:32 ` [PATCH v4 3/3] usb: misc: usbsevseg: " Anant Thazhemadam
2 siblings, 0 replies; 4+ messages in thread
From: Anant Thazhemadam @ 2021-03-26 22:32 UTC (permalink / raw)
To: Greg Kroah-Hartman, Anant Thazhemadam, Johan Hovold
Cc: linux-usb, linux-kernel
The newer usb_control_msg_{send|recv}() API ensures that a short read is
treated as an error, data can be used off the stack, and raw usb pipes need
not be created in the calling functions.
For this reason, the instance of usb_control_msg() has been replaced with
usb_control_msg_send() appropriately.
Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
Reviewed-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/misc/ezusb.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/misc/ezusb.c b/drivers/usb/misc/ezusb.c
index f058d8029761..78aaee56c2b7 100644
--- a/drivers/usb/misc/ezusb.c
+++ b/drivers/usb/misc/ezusb.c
@@ -31,24 +31,12 @@ static const struct ezusb_fx_type ezusb_fx1 = {
static int ezusb_writememory(struct usb_device *dev, int address,
unsigned char *data, int length, __u8 request)
{
- int result;
- unsigned char *transfer_buffer;
-
if (!dev)
return -ENODEV;
- transfer_buffer = kmemdup(data, length, GFP_KERNEL);
- if (!transfer_buffer) {
- dev_err(&dev->dev, "%s - kmalloc(%d) failed.\n",
- __func__, length);
- return -ENOMEM;
- }
- result = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request,
+ return usb_control_msg_send(dev, 0, request,
USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- address, 0, transfer_buffer, length, 3000);
-
- kfree(transfer_buffer);
- return result;
+ address, 0, data, length, 3000, GFP_KERNEL);
}
static int ezusb_set_reset(struct usb_device *dev, unsigned short cpucs_reg,
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v4 3/3] usb: misc: usbsevseg: update to use usb_control_msg_send()
2021-03-26 22:32 [PATCH v4 0/3] drivers: usb: misc: update to use usb_control_msg_{send|recv}() API Anant Thazhemadam
2021-03-26 22:32 ` [PATCH v4 1/3] usb: misc: ehset: update to use the " Anant Thazhemadam
2021-03-26 22:32 ` [PATCH v4 2/3] usb: misc: ezusb: update to use usb_control_msg_send() Anant Thazhemadam
@ 2021-03-26 22:32 ` Anant Thazhemadam
2 siblings, 0 replies; 4+ messages in thread
From: Anant Thazhemadam @ 2021-03-26 22:32 UTC (permalink / raw)
To: Greg Kroah-Hartman, Anant Thazhemadam; +Cc: linux-usb, linux-kernel
The newer usb_control_msg_{send|recv}() API ensures that a short read is
treated as an error, data can be used off the stack, and raw usb pipes
need not be created in the calling functions.
For this reason, instances of usb_control_msg() have been replaced with
usb_control_msg_send() appropriately.
Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com>
---
drivers/usb/misc/usbsevseg.c | 60 ++++++++++--------------------------
1 file changed, 17 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/misc/usbsevseg.c b/drivers/usb/misc/usbsevseg.c
index 551074f5b7ad..4bc816bb09bb 100644
--- a/drivers/usb/misc/usbsevseg.c
+++ b/drivers/usb/misc/usbsevseg.c
@@ -74,15 +74,10 @@ static void update_display_powered(struct usb_sevsegdev *mydev)
if (mydev->shadow_power != 1)
return;
- rc = usb_control_msg(mydev->udev,
- usb_sndctrlpipe(mydev->udev, 0),
- 0x12,
- 0x48,
- (80 * 0x100) + 10, /* (power mode) */
- (0x00 * 0x100) + (mydev->powered ? 1 : 0),
- NULL,
- 0,
- 2000);
+ rc = usb_control_msg_send(mydev->udev, 0, 0x12, 0x48,
+ (80 * 0x100) + 10, /* (power mode) */
+ (0x00 * 0x100) + (mydev->powered ? 1 : 0),
+ NULL, 0, 2000, GFP_KERNEL);
if (rc < 0)
dev_dbg(&mydev->udev->dev, "power retval = %d\n", rc);
@@ -99,15 +94,10 @@ static void update_display_mode(struct usb_sevsegdev *mydev)
if(mydev->shadow_power != 1)
return;
- rc = usb_control_msg(mydev->udev,
- usb_sndctrlpipe(mydev->udev, 0),
- 0x12,
- 0x48,
- (82 * 0x100) + 10, /* (set mode) */
- (mydev->mode_msb * 0x100) + mydev->mode_lsb,
- NULL,
- 0,
- 2000);
+ rc = usb_control_msg_send(mydev->udev, 0, 0x12, 0x48,
+ (82 * 0x100) + 10, /* (set mode) */
+ (mydev->mode_msb * 0x100) + mydev->mode_lsb,
+ NULL, 0, 2000, GFP_NOIO);
if (rc < 0)
dev_dbg(&mydev->udev->dev, "mode retval = %d\n", rc);
@@ -117,48 +107,32 @@ static void update_display_visual(struct usb_sevsegdev *mydev, gfp_t mf)
{
int rc;
int i;
- unsigned char *buffer;
+ unsigned char buffer[MAXLEN] = {0};
u8 decimals = 0;
if(mydev->shadow_power != 1)
return;
- buffer = kzalloc(MAXLEN, mf);
- if (!buffer)
- return;
-
/* The device is right to left, where as you write left to right */
for (i = 0; i < mydev->textlength; i++)
buffer[i] = mydev->text[mydev->textlength-1-i];
- rc = usb_control_msg(mydev->udev,
- usb_sndctrlpipe(mydev->udev, 0),
- 0x12,
- 0x48,
- (85 * 0x100) + 10, /* (write text) */
- (0 * 0x100) + mydev->textmode, /* mode */
- buffer,
- mydev->textlength,
- 2000);
+ rc = usb_control_msg_send(mydev->udev, 0, 0x12, 0x48,
+ (85 * 0x100) + 10, /* (write text) */
+ (0 * 0x100) + mydev->textmode, /* mode */
+ &buffer, mydev->textlength, 2000, mf);
if (rc < 0)
dev_dbg(&mydev->udev->dev, "write retval = %d\n", rc);
- kfree(buffer);
-
/* The device is right to left, where as you write left to right */
for (i = 0; i < sizeof(mydev->decimals); i++)
decimals |= mydev->decimals[i] << i;
- rc = usb_control_msg(mydev->udev,
- usb_sndctrlpipe(mydev->udev, 0),
- 0x12,
- 0x48,
- (86 * 0x100) + 10, /* (set decimal) */
- (0 * 0x100) + decimals, /* decimals */
- NULL,
- 0,
- 2000);
+ rc = usb_control_msg_send(mydev->udev, 0, 0x12, 0x48,
+ (86 * 0x100) + 10, /* (set decimal) */
+ (0 * 0x100) + decimals, /* decimals */
+ NULL, 0, 2000, mf);
if (rc < 0)
dev_dbg(&mydev->udev->dev, "decimal retval = %d\n", rc);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-26 22:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26 22:32 [PATCH v4 0/3] drivers: usb: misc: update to use usb_control_msg_{send|recv}() API Anant Thazhemadam
2021-03-26 22:32 ` [PATCH v4 1/3] usb: misc: ehset: update to use the " Anant Thazhemadam
2021-03-26 22:32 ` [PATCH v4 2/3] usb: misc: ezusb: update to use usb_control_msg_send() Anant Thazhemadam
2021-03-26 22:32 ` [PATCH v4 3/3] usb: misc: usbsevseg: " Anant Thazhemadam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).