Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753582AbcD2NMs (ORCPT ); Fri, 29 Apr 2016 09:12:48 -0400 Received: from mail-lf0-f65.google.com ([209.85.215.65]:34639 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752873AbcD2NLv (ORCPT ); Fri, 29 Apr 2016 09:11:51 -0400 Date: Fri, 29 Apr 2016 15:11:52 +0200 From: Johan Hovold To: Grigori Goronzy Cc: Johan Hovold , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 06/13] USB: ch341: add support for parity, frame length, stop bits Message-ID: <20160429131152.GJ22229@localhost> References: <1460754856-27908-1-git-send-email-greg@chown.ath.cx> <1460754856-27908-7-git-send-email-greg@chown.ath.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1460754856-27908-7-git-send-email-greg@chown.ath.cx> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2876 Lines: 96 On Fri, Apr 15, 2016 at 11:14:09PM +0200, Grigori Goronzy wrote: > With the new reinitialization method, configuring parity, different > frame lengths and different stop bit settings work as expected on > both CH340G and CH341A. This has been extensively tested with a > logic analyzer. > > v2: only set mark/space when parity is enabled, simplifications, > patch termios HW flags. > v3: fix parity odd/even regression. > > Tested-by: Ryan Barber > Signed-off-by: Grigori Goronzy > --- > drivers/usb/serial/ch341.c | 40 ++++++++++++++++++++++++++++++---------- > 1 file changed, 30 insertions(+), 10 deletions(-) > > diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c > index 6181616..2fbec4a 100644 > --- a/drivers/usb/serial/ch341.c > +++ b/drivers/usb/serial/ch341.c > @@ -341,7 +341,6 @@ static void ch341_set_termios(struct tty_struct *tty, > struct usb_serial_port *port, struct ktermios *old_termios) > { > struct ch341_private *priv = usb_get_serial_port_data(port); > - unsigned baud_rate; > unsigned long flags; > unsigned char ctrl; > int r; > @@ -350,13 +349,39 @@ static void ch341_set_termios(struct tty_struct *tty, > if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios)) > return; > > - baud_rate = tty_get_baud_rate(tty); > + priv->baud_rate = tty_get_baud_rate(tty); > > - priv->baud_rate = baud_rate; This is an unrelated change. > + ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX; > > - ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8; > + switch (C_CSIZE(tty)) { > + case CS5: > + ctrl |= CH341_LCR_CS5; > + break; > + case CS6: > + ctrl |= CH341_LCR_CS6; > + break; > + case CS7: > + ctrl |= CH341_LCR_CS7; > + break; > + default: > + tty->termios.c_cflag |= CS8; CSIZE is 2-bits wide, so this shouldn't be needed. (If it were, you'd need to clear the bits first.) > + case CS8: > + ctrl |= CH341_LCR_CS8; > + break; > + } > + > + if (C_PARENB(tty)) { > + ctrl |= CH341_LCR_ENABLE_PAR; > + if (C_PARODD(tty) == 0) > + ctrl |= CH341_LCR_PAR_EVEN; > + if (C_CMSPAR(tty)) > + ctrl |= CH341_LCR_MARK_SPACE; > + } > + > + if (C_CSTOPB(tty)) > + ctrl |= CH341_LCR_STOP_BITS_2; > > - if (baud_rate) { > + if (priv->baud_rate) { > spin_lock_irqsave(&priv->lock, flags); > priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS); > spin_unlock_irqrestore(&priv->lock, flags); > @@ -373,11 +398,6 @@ static void ch341_set_termios(struct tty_struct *tty, > > ch341_set_handshake(port->serial->dev, priv->line_control); > > - /* Unimplemented: > - * (cflag & CSIZE) : data bits [5, 8] > - * (cflag & PARENB) : parity {NONE, EVEN, ODD} > - * (cflag & CSTOPB) : stop bits [1, 2] > - */ > } > > static void ch341_break_ctl(struct tty_struct *tty, int break_state) Looks good otherwise. Thanks, Johan