On Wed, Nov 04, 2020 at 12:16:55PM +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/f81534.c | 63 +++++++++++-------------------------- > 1 file changed, 18 insertions(+), 45 deletions(-) > > diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c > index 5661fd03e545..23eb17a2c052 100644 > --- a/drivers/usb/serial/f81534.c > +++ b/drivers/usb/serial/f81534.c > @@ -217,38 +217,26 @@ static int f81534_set_register(struct usb_serial > *serial, u16 reg, u8 data) > struct usb_device *dev = serial->dev; > size_t count = F81534_USB_MAX_RETRY; > int status; > - u8 *tmp; > - > - tmp = kmalloc(sizeof(u8), GFP_KERNEL); > - if (!tmp) > - return -ENOMEM; > - > - *tmp = data; > > /* > * Our device maybe not reply when heavily loading, We'll retry for > * F81534_USB_MAX_RETRY times. > */ > while (count--) { > - status = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), > - F81534_SET_GET_REGISTER, > - USB_TYPE_VENDOR | USB_DIR_OUT, > - reg, 0, tmp, sizeof(u8), > - F81534_USB_TIMEOUT); > - if (status > 0) { > - status = 0; > - break; > - } else if (status == 0) { > - status = -EIO; > + status = usb_control_msg_send(dev, 0, F81534_SET_GET_REGISTER, > + USB_TYPE_VENDOR | USB_DIR_OUT, > + reg, 0, &data, sizeof(u8), > + F81534_USB_TIMEOUT, GFP_KERNEL); > + if (status) { > + /* Try again */ > + continue; > } > }
Here too this change breaks the logic and the control transfer is now repeated also after successful transfer (ten times!). This change would also introduce an additional malloc + memcpy for every retry. As this is a function that is used often and the comment suggest that having to retry isn't that rare, I suggest dropping this patch as well. Johan
