Re: [Linux-kernel-mentees] [PATCH 15/15] usb: serial: kl5kusb105: use usb_control_msg_recv() and usb_control_msg_send()
From: Johan Hovold <johan@kernel.org>
Date: 2020-12-04 10:37:45
Also in:
linux-usb, lkml
On Wed, Nov 04, 2020 at 12:17:03PM +0530, Himadri Pandya wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- drivers/usb/serial/kl5kusb105.c | 94 +++++++++++++++------------------ 1 file changed, 44 insertions(+), 50 deletions(-)diff --git a/drivers/usb/serial/kl5kusb105.c b/drivers/usb/serial/kl5kusb105.c index 5ee48b0650c4..75cfd1c907f3 100644 --- a/drivers/usb/serial/kl5kusb105.c +++ b/drivers/usb/serial/kl5kusb105.c@@ -124,16 +124,17 @@ static int klsi_105_chg_port_settings(struct usb_serial_port *port, { int rc; - rc = usb_control_msg(port->serial->dev, - usb_sndctrlpipe(port->serial->dev, 0), - KL5KUSB105A_SIO_SET_DATA, - USB_TYPE_VENDOR | USB_DIR_OUT | USB_RECIP_INTERFACE, - 0, /* value */ - 0, /* index */ - settings, - sizeof(struct klsi_105_port_settings), - KLSI_TIMEOUT); - if (rc < 0) + rc = usb_control_msg_send(port->serial->dev, 0, + KL5KUSB105A_SIO_SET_DATA, + USB_TYPE_VENDOR | USB_DIR_OUT | + USB_RECIP_INTERFACE, + 0, /* value */ + 0, /* index */ + settings, + sizeof(struct klsi_105_port_settings), + KLSI_TIMEOUT, + GFP_KERNEL); + if (rc) dev_err(&port->dev, "Change port settings failed (error = %d)\n", rc);
The callers of this function already allocates a transfer-buffer which you should now remove to avoid introducing an additional memdup() per call. Note that there was a related bug in open() which failed to release it's transfer buffer on successful open. I CCed you on the fix which we should get merged and backported before converting to using the new helper.
quoted hunk ↗ jump to hunk
@@ -283,16 +276,17 @@ static int klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port) goto err_free_cfg; } - rc = usb_control_msg(port->serial->dev, - usb_sndctrlpipe(port->serial->dev, 0), - KL5KUSB105A_SIO_CONFIGURE, - USB_TYPE_VENDOR|USB_DIR_OUT|USB_RECIP_INTERFACE, - KL5KUSB105A_SIO_CONFIGURE_READ_ON, - 0, /* index */ - NULL, - 0, - KLSI_TIMEOUT); - if (rc < 0) { + rc = usb_control_msg_send(port->serial->dev, 0, + KL5KUSB105A_SIO_CONFIGURE, + USB_TYPE_VENDOR | USB_DIR_OUT | + USB_RECIP_INTERFACE, + KL5KUSB105A_SIO_CONFIGURE_READ_ON, + 0, /* index */ + NULL, + 0, + KLSI_TIMEOUT, + GFP_KERNEL); + if (rc) {
And again there's no need to change the control transfers without a data stage. Johan _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees