From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754429AbcDYKMn (ORCPT ); Mon, 25 Apr 2016 06:12:43 -0400 Received: from mail-lb0-f194.google.com ([209.85.217.194]:33998 "EHLO mail-lb0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753825AbcDYKMl (ORCPT ); Mon, 25 Apr 2016 06:12:41 -0400 Date: Mon, 25 Apr 2016 12:12:39 +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 2/5] USB: serial: cp210x: Made sure SERIAL_XOFF_CONTINUE flag is clear. Message-ID: <20160425101239.GH18866@localhost> References: <1461517750-6205-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: <1461517750-6205-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:09:10PM -0500, Konstantin Shkolnyy wrote: > The CRTCTS flag code intended to clear the SERIAL_XOFF_CONTINUE flag, but > did it inconsistently. This change is non-functional for existing chips > because the driver never set the flag and it's clear by default. > > Signed-off-by: Konstantin Shkolnyy > --- > drivers/usb/serial/cp210x.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c > index de1606f..ede5c52 100644 > --- a/drivers/usb/serial/cp210x.c > +++ b/drivers/usb/serial/cp210x.c > @@ -955,7 +955,6 @@ static void cp210x_set_termios(struct tty_struct *tty, > modem_ctl[0] &= ~0x7B; > modem_ctl[0] |= 0x09; > modem_ctl[4] = 0x80; > - modem_ctl[7] = 0; > dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__); > } else { > modem_ctl[0] &= ~0x7B; > @@ -963,6 +962,7 @@ static void cp210x_set_termios(struct tty_struct *tty, > modem_ctl[4] = 0x40; > dev_dbg(dev, "%s - flow control = NONE\n", __func__); > } > + modem_ctl[7] = 0; I'd rather see this done using bit operations and a define (see my comments on the next patch). > > dev_dbg(dev, "%s - write modem controls = %02x .. .. .. %02x .. .. %02x\n", > __func__, modem_ctl[0], modem_ctl[4], modem_ctl[7]); Thanks, Johan