[PATCH v3 2/2] USB: serial: cp210x: use usb_control_msg_recv() and usb_control_msg_send()

From: Himadri Pandya
Date: Fri Oct 01 2021 - 03:00:11 EST


The new wrapper functions for usb_control_msg() can accept data from
stack and treat short reads as error. Hence use the wrappers functions.
Please note that because of this change, cp210x_read_reg_block() will no
longer log the length of short reads.

Signed-off-by: Himadri Pandya <himadrispandya@xxxxxxxxx>
---
Changes in v3:
- Rephrase the commit message
- Explicitly mention that short reads don't log length now

Changes in v2:
- Drop unrelated style fixes
---
drivers/usb/serial/cp210x.c | 106 ++++++++++--------------------------
1 file changed, 30 insertions(+), 76 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 189279869a8b..3c3ca46b0b82 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -631,29 +631,19 @@ static int cp210x_read_reg_block(struct usb_serial_port *port, u8 req,
{
struct usb_serial *serial = port->serial;
struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
- void *dmabuf;
int result;

- dmabuf = kmalloc(bufsize, GFP_KERNEL);
- if (!dmabuf)
- return -ENOMEM;

- result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
- req, REQTYPE_INTERFACE_TO_HOST, 0,
- port_priv->bInterfaceNumber, dmabuf, bufsize,
- USB_CTRL_GET_TIMEOUT);
- if (result == bufsize) {
- memcpy(buf, dmabuf, bufsize);
- result = 0;
- } else {
+ result = usb_control_msg_recv(serial->dev, 0, req,
+ REQTYPE_INTERFACE_TO_HOST, 0,
+ port_priv->bInterfaceNumber, buf,
+ bufsize, USB_CTRL_SET_TIMEOUT,
+ GFP_KERNEL);
+ if (result) {
dev_err(&port->dev, "failed get req 0x%x size %d status: %d\n",
req, bufsize, result);
- if (result >= 0)
- result = -EIO;
}

- kfree(dmabuf);
-
return result;
}

@@ -672,30 +662,17 @@ static int cp210x_read_u8_reg(struct usb_serial_port *port, u8 req, u8 *val)
static int cp210x_read_vendor_block(struct usb_serial *serial, u8 type, u16 val,
void *buf, int bufsize)
{
- void *dmabuf;
int result;

- dmabuf = kmalloc(bufsize, GFP_KERNEL);
- if (!dmabuf)
- return -ENOMEM;
-
- result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
- CP210X_VENDOR_SPECIFIC, type, val,
- cp210x_interface_num(serial), dmabuf, bufsize,
- USB_CTRL_GET_TIMEOUT);
- if (result == bufsize) {
- memcpy(buf, dmabuf, bufsize);
- result = 0;
- } else {
+ result = usb_control_msg_recv(serial->dev, 0, CP210X_VENDOR_SPECIFIC,
+ type, val, cp210x_interface_num(serial),
+ buf, bufsize, USB_CTRL_GET_TIMEOUT,
+ GFP_KERNEL);
+ if (result) {
dev_err(&serial->interface->dev,
"failed to get vendor val 0x%04x size %d: %d\n", val,
bufsize, result);
- if (result >= 0)
- result = -EIO;
}
-
- kfree(dmabuf);
-
return result;
}

@@ -730,21 +707,14 @@ static int cp210x_write_reg_block(struct usb_serial_port *port, u8 req,
{
struct usb_serial *serial = port->serial;
struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
- void *dmabuf;
int result;

- dmabuf = kmemdup(buf, bufsize, GFP_KERNEL);
- if (!dmabuf)
- return -ENOMEM;
-
- result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
- req, REQTYPE_HOST_TO_INTERFACE, 0,
- port_priv->bInterfaceNumber, dmabuf, bufsize,
- USB_CTRL_SET_TIMEOUT);
-
- kfree(dmabuf);
+ result = usb_control_msg_send(serial->dev, 0, req,
+ REQTYPE_HOST_TO_INTERFACE, 0,
+ port_priv->bInterfaceNumber, buf, bufsize,
+ USB_CTRL_SET_TIMEOUT, GFP_KERNEL);

- if (result < 0) {
+ if (result) {
dev_err(&port->dev, "failed set req 0x%x size %d status: %d\n",
req, bufsize, result);
return result;
@@ -773,21 +743,14 @@ static int cp210x_write_u32_reg(struct usb_serial_port *port, u8 req, u32 val)
static int cp210x_write_vendor_block(struct usb_serial *serial, u8 type,
u16 val, void *buf, int bufsize)
{
- void *dmabuf;
int result;

- dmabuf = kmemdup(buf, bufsize, GFP_KERNEL);
- if (!dmabuf)
- return -ENOMEM;
+ result = usb_control_msg_send(serial->dev, 0, CP210X_VENDOR_SPECIFIC,
+ type, val, cp210x_interface_num(serial),
+ buf, bufsize, USB_CTRL_SET_TIMEOUT,
+ GFP_KERNEL);

- result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
- CP210X_VENDOR_SPECIFIC, type, val,
- cp210x_interface_num(serial), dmabuf, bufsize,
- USB_CTRL_SET_TIMEOUT);
-
- kfree(dmabuf);
-
- if (result < 0) {
+ if (result) {
dev_err(&serial->interface->dev,
"failed to set vendor val 0x%04x size %d: %d\n", val,
bufsize, result);
@@ -952,27 +915,18 @@ static int cp210x_get_tx_queue_byte_count(struct usb_serial_port *port,
{
struct usb_serial *serial = port->serial;
struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
- struct cp210x_comm_status *sts;
+ struct cp210x_comm_status sts;
int result;

- sts = kmalloc(sizeof(*sts), GFP_KERNEL);
- if (!sts)
- return -ENOMEM;
-
- result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
- CP210X_GET_COMM_STATUS, REQTYPE_INTERFACE_TO_HOST,
- 0, port_priv->bInterfaceNumber, sts, sizeof(*sts),
- USB_CTRL_GET_TIMEOUT);
- if (result == sizeof(*sts)) {
- *count = le32_to_cpu(sts->ulAmountInOutQueue);
- result = 0;
- } else {
+ result = usb_control_msg_recv(serial->dev, 0, CP210X_GET_COMM_STATUS,
+ REQTYPE_INTERFACE_TO_HOST, 0,
+ port_priv->bInterfaceNumber, &sts,
+ sizeof(sts), USB_CTRL_GET_TIMEOUT,
+ GFP_KERNEL);
+ if (result == 0)
+ *count = le32_to_cpu(sts.ulAmountInOutQueue);
+ else
dev_err(&port->dev, "failed to get comm status: %d\n", result);
- if (result >= 0)
- result = -EIO;
- }
-
- kfree(sts);

return result;
}
--
2.17.1