On Wed, Nov 04, 2020 at 12:17:00PM +0530, Himadri Pandya wrote: > The new usb_control_msg_send() nicely wraps usb_control_msg() with proper > error check. Hence use the wrapper instead of calling usb_control_msg() > directly. > > Signed-off-by: Himadri Pandya <himadrispandya@xxxxxxxxx> > --- > drivers/usb/serial/ipw.c | 107 +++++++++++++-------------------------- > 1 file changed, 36 insertions(+), 71 deletions(-) > > diff --git a/drivers/usb/serial/ipw.c b/drivers/usb/serial/ipw.c > index d04c7cc5c1c2..3c3895b4dff8 100644 > --- a/drivers/usb/serial/ipw.c > +++ b/drivers/usb/serial/ipw.c > @@ -134,26 +134,16 @@ static int ipw_open(struct tty_struct *tty, struct usb_serial_port *port) > struct usb_device *udev = port->serial->dev; > struct device *dev = &port->dev; > u8 buf_flow_static[16] = IPW_BYTES_FLOWINIT; > - u8 *buf_flow_init; > int result; > > - buf_flow_init = kmemdup(buf_flow_static, 16, GFP_KERNEL); > - if (!buf_flow_init) > - return -ENOMEM; > - > /* --1: Tell the modem to initialize (we think) From sniffs this is > * always the first thing that gets sent to the modem during > * opening of the device */ > dev_dbg(dev, "%s: Sending SIO_INIT (we guess)\n", __func__); > - result = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), > - IPW_SIO_INIT, > - USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT, > - 0, > - 0, /* index */ > - NULL, > - 0, > - 100000); > - if (result < 0) > + result = usb_control_msg_send(udev, 0, IPW_SIO_INIT, USB_TYPE_VENDOR | > + USB_RECIP_INTERFACE | USB_DIR_OUT, 0, > + 0, NULL, 0, 100000, GFP_KERNEL); Try to keep the existing indentation style and also avoid breaking the request argument (USB_TYPE_VENDOR | ...) over multiple lines. > + if (result) > dev_err(dev, "Init of modem failed (error = %d)\n", result); > > /* reset the bulk pipes */ > @@ -166,31 +156,22 @@ static int ipw_open(struct tty_struct *tty, struct usb_serial_port *port) > > /*--3: Tell the modem to open the floodgates on the rx bulk channel */ > dev_dbg(dev, "%s:asking modem for RxRead (RXBULK_ON)\n", __func__); > - result = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), > - IPW_SIO_RXCTL, > - USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT, > - IPW_RXBULK_ON, > - 0, /* index */ > - NULL, > - 0, > - 100000); > - if (result < 0) > + result = usb_control_msg_send(udev, 0, IPW_SIO_RXCTL, USB_TYPE_VENDOR | > + USB_RECIP_INTERFACE | USB_DIR_OUT, > + IPW_RXBULK_ON, 0, NULL, 0, 100000, > + GFP_KERNEL); > + if (result) > dev_err(dev, "Enabling bulk RxRead failed (error = %d)\n", result); > > /*--4: setup the initial flowcontrol */ > - dev_dbg(dev, "%s:setting init flowcontrol (%s)\n", __func__, buf_flow_init); > - result = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), > - IPW_SIO_HANDFLOW, > - USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT, > - 0, > - 0, > - buf_flow_init, > - 0x10, > - 200000); > - if (result < 0) > + dev_dbg(dev, "%s:setting init flowcontrol (%s)\n", __func__, buf_flow_static); > + result = usb_control_msg_send(udev, 0, IPW_SIO_HANDFLOW, > + USB_TYPE_VENDOR | USB_RECIP_INTERFACE | > + USB_DIR_OUT, 0, 0, &buf_flow_static, 0x10, > + 200000, GFP_KERNEL); > + if (result) > dev_err(dev, "initial flowcontrol failed (error = %d)\n", result); Note that sending a short control message is always an error so there's nothing wrong with the current errors checks. In fact, since all but this control request lacks a data stage, I'd just leave this one as is as well. Johan
