linux-kernel - Re: [PATCH V2 3/4] USB: serial: f81232: implement break control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 5 Sep 2018 16:41:53 +0200
From:   Johan Hovold <johan@...nel.org>
To:     "Ji-Ze Hong (Peter Hong)" <hpeter@...il.com>
Cc:     johan@...nel.org, gregkh@...uxfoundation.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        peter_hong@...tek.com.tw, hpeter+linux_kernel@...il.com
Subject: Re: [PATCH V2 3/4] USB: serial: f81232: implement break control

On Thu, Aug 30, 2018 at 09:47:13AM +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) <hpeter+linux_kernel@...il.com>
> ---
> v2:
>         1: Fix f81232_break_ctl() for mis-operated set_mask_register().
>         2: Add set break control bit in f81232_set_termios().
> 
>  drivers/usb/serial/f81232.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 56adae8a9ee0..863430d7535f 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -65,6 +65,7 @@ struct f81232_private {
>  	struct mutex lock;
>  	u8 modem_control;
>  	u8 modem_status;
> +	bool is_break;
>  	speed_t baud_base;
>  	struct work_struct lsr_work;
>  	struct work_struct interrupt_work;
> @@ -377,13 +378,24 @@ static void f81232_process_read_urb(struct urb *urb)
>  
>  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;
> +	struct f81232_private *priv = usb_get_serial_port_data(port);
> +	int status;
> +	u8 tmp = 0;
>  
> -	/*
> -	 * 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.
> -	 */
> +	mutex_lock(&priv->lock);
> +
> +	if (break_state) {
> +		priv->is_break = !!break_state;
> +		tmp = UART_LCR_SBC;
> +	}
> +
> +	status = f81232_set_mask_register(port, LINE_CONTROL_REGISTER,
> +						UART_LCR_SBC, tmp);
> +	if (status)
> +		dev_err(&port->dev, "set break failed: %d\n", status);

Why not maintain a shadow lsr value so that you don't have to retrieve
it every time you want to change the break state? It looks like you do
it this way in f81534.

Johan