On Fri, Dec 4, 2020 at 2:38 PM Johan Hovold <johan@xxxxxxxxxx> wrote: > > Hi Himadri, > > and sorry about the late feedback on this one. I'm still trying to dig > myself out of some backlog. > > On Wed, Nov 04, 2020 at 12:16:48PM +0530, Himadri Pandya wrote: > > There are many usages of usb_control_msg() that can use the new wrapper > > functions usb_contro_msg_send() & usb_control_msg_recv() for better > > error checks on short reads and writes. Hence use them whenever possible > > and avoid using usb_control_msg() directly. > > Replacing working code with shiny new helpers unfortunately often ends > up introducing new bugs and I'm afraid there are a few examples of that > in this series as well. > > I'll comment on the patches individually, but my impression is that we > should primarily use these helpers to replace code which allocates a > temporary buffer for each transfer as otherwise there's no clear gain > from using them. > > Some of your patches contains unrelated changes which needs to go in > separate patches if they're to be included at all. > > Please also try to write dedicated commit messages rater than reusing > more or less the same stock message since not everything in these > messages apply to each patch. You never mention that these helpers > nicely hides the allocation of temporary transfer buffers in some cases > for examples. In other places they instead introduce additional > allocations which at least should have been highlighted. > > > Himadri Pandya (15): > > usb: serial: ark3116: use usb_control_msg_recv() and > > usb_control_msg_send() > > Nit: please also use an uppercase "USB" prefix. Hi Johan, Thanks for reviewing this series and sorry for the late reply. I'll soon send a v2 according to your comments. Best regards, Himadri > > > usb: serial: belkin_sa: use usb_control_msg_send() > > usb: serial: ch314: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: cp210x: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: cypress_m8: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: f81232: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: f81534: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: ftdi_sio: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: io_edgeport: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: io_ti: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: ipaq: use usb_control_msg_send() > > usb: serial: ipw: use usb_control_msg_send() > > usb: serial: iuu_phoenix: use usb_control_msg_send() > > usb: serial: keyspan_pda: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: kl5kusb105: use usb_control_msg_recv() and > > usb_control_msg_send() > > > > drivers/usb/serial/ark3116.c | 29 +---- > > drivers/usb/serial/belkin_sa.c | 35 +++--- > > drivers/usb/serial/ch341.c | 45 +++----- > > drivers/usb/serial/cp210x.c | 148 +++++++------------------ > > drivers/usb/serial/cypress_m8.c | 38 ++++--- > > drivers/usb/serial/f81232.c | 88 +++------------ > > drivers/usb/serial/f81534.c | 63 +++-------- > > drivers/usb/serial/ftdi_sio.c | 182 +++++++++++++------------------ > > drivers/usb/serial/io_edgeport.c | 73 +++++-------- > > drivers/usb/serial/io_ti.c | 28 ++--- > > drivers/usb/serial/ipaq.c | 9 +- > > drivers/usb/serial/ipw.c | 107 ++++++------------ > > drivers/usb/serial/iuu_phoenix.c | 5 +- > > drivers/usb/serial/keyspan_pda.c | 172 ++++++++++++----------------- > > drivers/usb/serial/kl5kusb105.c | 94 ++++++++-------- > > 15 files changed, 406 insertions(+), 710 deletions(-) > > Johan
