From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S969748AbdAIPPa (ORCPT ); Mon, 9 Jan 2017 10:15:30 -0500 Received: from mail-lf0-f68.google.com ([209.85.215.68]:34848 "EHLO mail-lf0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S968224AbdAIPOM (ORCPT ); Mon, 9 Jan 2017 10:14:12 -0500 Date: Mon, 9 Jan 2017 16:14:09 +0100 From: Johan Hovold To: "Ji-Ze Hong (Peter Hong)" Cc: johan@kernel.org, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, tom_tsai@fintek.com.tw, peter_hong@fintek.com.tw, hpeter+linux_kernel@gmail.com Subject: Re: [PATCH V1 1/2] usb:serial: Implement Fintek F81232 break on/off Message-ID: <20170109151409.GQ21536@localhost> References: <1481261974-10914-1-git-send-email-hpeter+linux_kernel@gmail.com> <1481261974-10914-2-git-send-email-hpeter+linux_kernel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1481261974-10914-2-git-send-email-hpeter+linux_kernel@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 Fri, Dec 09, 2016 at 01:39:33PM +0800, Ji-Ze Hong (Peter Hong) wrote: > Implement Fintek F81232 break on/off with LCR register, > it's the same with 16550A LCR register layout. > > Signed-off-by: Ji-Ze Hong (Peter Hong) > --- > drivers/usb/serial/f81232.c | 40 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 34 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c > index 972f5a5..d45a70e 100644 > --- a/drivers/usb/serial/f81232.c > +++ b/drivers/usb/serial/f81232.c > @@ -131,6 +131,21 @@ static int f81232_set_register(struct usb_serial_port *port, u16 reg, u8 val) > return status; > } > > +static int f81232_set_mask_register(struct usb_serial_port *port, u16 reg, > + u8 mask, u8 val) > +{ > + int status; > + u8 tmp; > + > + status = f81232_get_register(port, reg, &tmp); > + if (status) > + return status; > + > + tmp = (tmp & ~mask) | (val & mask); > + > + return f81232_set_register(port, reg, tmp); > +} > + > static void f81232_read_msr(struct usb_serial_port *port) > { > int status; > @@ -335,15 +350,27 @@ static void f81232_process_read_urb(struct urb *urb) > tty_flip_buffer_push(&port->port); > } > > +static void f81232_set_break(struct usb_serial_port *port, bool enable) > +{ > + int status; > + u8 tmp = 0; > + > + if (enable) > + tmp = UART_LCR_SBC; > + > + status = f81232_set_mask_register(port, LINE_CONTROL_REGISTER, > + UART_LCR_SBC, tmp); > + if (status) { > + dev_err(&port->dev, "%s: set break failed: %x\n", __func__, You want %d here. And you can drop the __func__ prefix. > + status); > + } You should also coordinate this with the LCR update in set_termios, otherwise changing line settings would clear break etc. > +} > + > static void f81232_break_ctl(struct tty_struct *tty, int break_state) > { > - /* FIXME - Stubbed out for now */ > + struct usb_serial_port *port = tty->driver_data; > > - /* > - * break_state = -1 to turn on break, and 0 to turn off break > - * see drivers/char/tty_io.c to see it used. > - * last_set_data_urb_value NEVER has the break bit set in it. > - */ > + f81232_set_break(port, break_state); > } > > static void f81232_set_baudrate(struct usb_serial_port *port, speed_t baudrate) > @@ -563,6 +590,7 @@ static void f81232_close(struct usb_serial_port *port) > f81232_port_disable(port); > usb_serial_generic_close(port); > usb_kill_urb(port->interrupt_in_urb); > + f81232_set_break(port, false); I don't think this is needed, and then you can fold set_break into break_ctl above as well. > } > > static void f81232_dtr_rts(struct usb_serial_port *port, int on) Thanks, Johan