Re: [PATCH 07/15] usb: serial: f81534: use usb_control_msg_recv() and usb_control_msg_send()
From: Johan Hovold <johan@kernel.org>
Date: 2020-12-04 09:56:07
Also in:
linux-kernel-mentees, lkml
On Wed, Nov 04, 2020 at 12:16:55PM +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/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