From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754122AbcDYKLG (ORCPT ); Mon, 25 Apr 2016 06:11:06 -0400 Received: from mail-lf0-f66.google.com ([209.85.215.66]:35031 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752299AbcDYKLE (ORCPT ); Mon, 25 Apr 2016 06:11:04 -0400 Date: Mon, 25 Apr 2016 12:11:02 +0200 From: Johan Hovold To: Konstantin Shkolnyy Cc: johan@kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH RESEND 1/5] USB: serial: cp210x: Fixed RTS mode setting by the CRTSCTS flag. Message-ID: <20160425101102.GG18866@localhost> References: <1461517735-6170-1-git-send-email-konstantin.shkolnyy@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1461517735-6170-1-git-send-email-konstantin.shkolnyy@gmail.com> 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 On Sun, Apr 24, 2016 at 12:08:55PM -0500, Konstantin Shkolnyy wrote: > A bug in the CRTSCT handling caused RTS to alternate between > CRTSCTS=0 => "RTS transmits active signal" and > CRTSCTS=1 => "RTS receives flow control" > instead of > CRTSCTS=0 => "RTS is statically active" and > CRTSCTS=1 => "RTS receives flow control" Perhaps you should mention that this only happens after first having enabled CRTSCTS. > Signed-off-by: Konstantin Shkolnyy > --- > drivers/usb/serial/cp210x.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c > index c740592..de1606f 100644 > --- a/drivers/usb/serial/cp210x.c > +++ b/drivers/usb/serial/cp210x.c > @@ -955,16 +955,12 @@ static void cp210x_set_termios(struct tty_struct *tty, > modem_ctl[0] &= ~0x7B; > modem_ctl[0] |= 0x09; > modem_ctl[4] = 0x80; > - /* FIXME - why clear reserved bits just read? */ > - modem_ctl[5] = 0; > - modem_ctl[6] = 0; This is an unrelated chunk. Perhaps include it when addressing the XOFF_CONTINUE bit. > modem_ctl[7] = 0; > dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__); > } else { > modem_ctl[0] &= ~0x7B; > modem_ctl[0] |= 0x01; > - /* FIXME - OR here instead of assignment looks wrong */ > - modem_ctl[4] |= 0x40; > + modem_ctl[4] = 0x40; > dev_dbg(dev, "%s - flow control = NONE\n", __func__); > } Ideally modifying the RTS mask should not clear unrelated flags as break char insertion. But as we don't use these currently so this would be fine as a fix for now. Thanks, Johan