| Message ID | 20201104064703.15123-11-himadrispandya@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | usb: serial: avoid using usb_control_msg() directly | expand |
On Wed, Nov 04, 2020 at 12:16:58PM +0530, Himadri Pandya wrote: > The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps > usb_control_msg() with proper error check. Hence use the wrappers > instead of calling usb_control_msg() directly > > Signed-off-by: Himadri Pandya <himadrispandya@gmail.com> > --- > drivers/usb/serial/io_ti.c | 28 ++++++++++------------------ > 1 file changed, 10 insertions(+), 18 deletions(-) > > diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c > index c327d4cf7928..ab2370b80b3c 100644 > --- a/drivers/usb/serial/io_ti.c > +++ b/drivers/usb/serial/io_ti.c > @@ -260,16 +260,12 @@ static int ti_vread_sync(struct usb_device *dev, __u8 request, > { > int status; > > - status = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), request, > - (USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN), > - value, index, data, size, 1000); > - if (status < 0) > + status = usb_control_msg_recv(dev, 0, request, USB_TYPE_VENDOR | > + USB_RECIP_DEVICE | USB_DIR_IN, value, > + index, data, size, 1000, GFP_KERNEL); > + if (status) > return status; > - if (status != size) { > - dev_dbg(&dev->dev, "%s - wanted to write %d, but only wrote %d\n", > - __func__, size, status); > - return -ECOMM; > - } > + > return 0; > } > > @@ -278,16 +274,12 @@ static int ti_vsend_sync(struct usb_device *dev, u8 request, u16 value, > { > int status; > > - status = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request, > - (USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT), > - value, index, data, size, timeout); > - if (status < 0) > + status = usb_control_msg_send(dev, 0, request, USB_TYPE_VENDOR | > + USB_RECIP_DEVICE | USB_DIR_OUT, value, > + index, data, size, timeout, GFP_KERNEL); > + if (status) > return status; > - if (status != size) { > - dev_dbg(&dev->dev, "%s - wanted to write %d, but only wrote %d\n", > - __func__, size, status); > - return -ECOMM; > - } > + > return 0; > } These helper are only used with DMA-able buffers so this change introduces an additional redundant allocation and memcpy for every call for no real gain. Please drop this one as well. Johan
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c index c327d4cf7928..ab2370b80b3c 100644 --- a/drivers/usb/serial/io_ti.c +++ b/drivers/usb/serial/io_ti.c @@ -260,16 +260,12 @@ static int ti_vread_sync(struct usb_device *dev, __u8 request, { int status; - status = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), request, - (USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN), - value, index, data, size, 1000); - if (status < 0) + status = usb_control_msg_recv(dev, 0, request, USB_TYPE_VENDOR | + USB_RECIP_DEVICE | USB_DIR_IN, value, + index, data, size, 1000, GFP_KERNEL); + if (status) return status; - if (status != size) { - dev_dbg(&dev->dev, "%s - wanted to write %d, but only wrote %d\n", - __func__, size, status); - return -ECOMM; - } + return 0; } @@ -278,16 +274,12 @@ static int ti_vsend_sync(struct usb_device *dev, u8 request, u16 value, { int status; - status = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request, - (USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT), - value, index, data, size, timeout); - if (status < 0) + status = usb_control_msg_send(dev, 0, request, USB_TYPE_VENDOR | + USB_RECIP_DEVICE | USB_DIR_OUT, value, + index, data, size, timeout, GFP_KERNEL); + if (status) return status; - if (status != size) { - dev_dbg(&dev->dev, "%s - wanted to write %d, but only wrote %d\n", - __func__, size, status); - return -ECOMM; - } + return 0; }
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps usb_control_msg() with proper error check. Hence use the wrappers instead of calling usb_control_msg() directly Signed-off-by: Himadri Pandya <himadrispandya@gmail.com> --- drivers/usb/serial/io_ti.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-)