From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751595AbcACQcm (ORCPT ); Sun, 3 Jan 2016 11:32:42 -0500 Received: from mail-lf0-f52.google.com ([209.85.215.52]:34131 "EHLO mail-lf0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750792AbcACQci (ORCPT ); Sun, 3 Jan 2016 11:32:38 -0500 Date: Sun, 3 Jan 2016 17:33:13 +0100 From: Johan Hovold To: Mathieu OTHACEHE Cc: johan@kernel.org, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [PATCH 2/3] USB: mxu11x0: clean device control commands Message-ID: <20160103163313.GE25304@localhost> References: <1451831161-26792-1-git-send-email-m.othacehe@gmail.com> <1451831161-26792-3-git-send-email-m.othacehe@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1451831161-26792-3-git-send-email-m.othacehe@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Jan 03, 2016 at 03:26:00PM +0100, Mathieu OTHACEHE wrote: > Sending OPEN and START commands twice is not necessary for this driver. > Also send STOP command at close. > > Signed-off-by: Mathieu OTHACEHE > --- > drivers/usb/serial/mxu11x0.c | 31 +++++++------------------------ > 1 file changed, 7 insertions(+), 24 deletions(-) > > diff --git a/drivers/usb/serial/mxu11x0.c b/drivers/usb/serial/mxu11x0.c > index 0fe7eab..354fcb5 100644 > --- a/drivers/usb/serial/mxu11x0.c > +++ b/drivers/usb/serial/mxu11x0.c > @@ -823,30 +823,6 @@ static int mxu1_open(struct tty_struct *tty, struct usb_serial_port *port) > goto unlink_int_urb; > } > > - /* > - * reset the data toggle on the bulk endpoints to work around bug in > - * host controllers where things get out of sync some times > - */ > - usb_clear_halt(serial->dev, port->write_urb->pipe); > - usb_clear_halt(serial->dev, port->read_urb->pipe); This is an unrelated change. You can remove it, but then do so in a separate patch. > - > - if (tty) > - mxu1_set_termios(tty, port, NULL); But you should definitely not be removing this. > - > - status = mxu1_send_ctrl_urb(serial, MXU1_OPEN_PORT, > - open_settings, MXU1_UART1_PORT); > - if (status) { > - dev_err(&port->dev, "cannot send open command: %d\n", status); > - goto unlink_int_urb; > - } > - > - status = mxu1_send_ctrl_urb(serial, MXU1_START_PORT, > - 0, MXU1_UART1_PORT); > - if (status) { > - dev_err(&port->dev, "cannot send start command: %d\n", status); > - goto unlink_int_urb; > - } > - Also did you not say that your sniffed traffic showed the following sequence OPEN, CONFIG (e.g. purge, set_termios), START? Johan