On Wed, Nov 04, 2020 at 12:17:03PM +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 <[email protected]>
> ---
> 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.
> @@ -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
