linux-kernel - Re: [PATCH 24/36] usb: serial: ti_usb_3410_5052: Use usb_serial_generic

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:	Fri, 15 Jul 2016 14:12:11 +0200
From:	Johan Hovold <johan@...nel.org>
To:	Mathieu OTHACEHE <m.othacehe@...il.com>
Cc:	johan@...nel.org, gregkh@...uxfoundation.org,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 24/36] usb: serial: ti_usb_3410_5052: Use
 usb_serial_generic_open

On Thu, May 12, 2016 at 10:48:56AM +0200, Mathieu OTHACEHE wrote:
> Use usb_serial_generic_open in open callback to start read urb.
> Also remove useless usb_device pointer.
> 
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@...il.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 21 ++++-----------------
>  1 file changed, 4 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
> index d8bed30..4769c80 100644
> --- a/drivers/usb/serial/ti_usb_3410_5052.c
> +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> @@ -674,7 +674,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
>  	struct ti_port *tport = usb_get_serial_port_data(port);
>  	struct usb_serial *serial = port->serial;
>  	struct ti_device *tdev;
> -	struct usb_device *dev;
>  	struct urb *urb;
>  	int port_number;
>  	int status;
> @@ -684,7 +683,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
>  			 TI_PIPE_TIMEOUT_ENABLE |
>  			 (TI_TRANSFER_TIMEOUT << 2));
>  
> -	dev = port->serial->dev;
>  	tdev = tport->tp_tdev;
>  
>  	/* only one open on any port on a device at a time */
> @@ -748,8 +746,8 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
>  
>  	/* reset the data toggle on the bulk endpoints to work around bug in
>  	 * host controllers where things get out of sync some times */
> -	usb_clear_halt(dev, port->write_urb->pipe);
> -	usb_clear_halt(dev, port->read_urb->pipe);
> +	usb_clear_halt(serial->dev, port->write_urb->pipe);
> +	usb_clear_halt(serial->dev, port->read_urb->pipe);
>  
>  	if (tty)
>  		ti_set_termios(tty, port, &tty->termios);
> @@ -770,20 +768,9 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
>  		goto unlink_int_urb;
>  	}
>  
> -	/* start read urb */
> -	urb = port->read_urb;
> -	if (!urb) {
> -		dev_err(&port->dev, "%s - no read urb\n", __func__);
> -		status = -EINVAL;
> -		goto unlink_int_urb;
> -	}
> -	urb->context = tport;
> -	status = usb_submit_urb(urb, GFP_KERNEL);
> -	if (status) {
> -		dev_err(&port->dev, "%s - submit read urb failed, %d\n",
> -							__func__, status);
> +	status = usb_serial_generic_open(tty, port);
> +	if (status)
>  		goto unlink_int_urb;
> -	}

Ok, you did not submit the second urb until here.

I'd rather you see you start using both the generic open and close
callbacks when you convert the read implementation (followed by a write
conversion).

Thanks,
Johan