linux-kernel - Re: [PATCH 08/36] usb: serial: ti_usb_3410_5052: Remove useless dev

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]

Message-ID: <20160715111255.GG8809@localhost>
Date:	Fri, 15 Jul 2016 13:12:55 +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 08/36] usb: serial: ti_usb_3410_5052: Remove useless
 dev_dbg messages

On Thu, May 12, 2016 at 10:48:40AM +0200, Mathieu OTHACEHE wrote:
> Remove useless or redundant dev_dbg messages.
> Fix debug-message typos.
> 
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@...il.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 28 +++++++++-------------------
>  1 file changed, 9 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
> index 1860a5a..88aacf5 100644

> @@ -846,7 +837,6 @@ static int ti_write_room(struct tty_struct *tty)
>  	room = kfifo_avail(&port->write_fifo);
>  	spin_unlock_irqrestore(&tport->tp_lock, flags);
>  
> -	dev_dbg(&port->dev, "%s - returns %d\n", __func__, room);
>  	return room;
>  }
>  
> @@ -865,7 +855,6 @@ static int ti_chars_in_buffer(struct tty_struct *tty)
>  	chars = kfifo_len(&port->write_fifo);
>  	spin_unlock_irqrestore(&tport->tp_lock, flags);
>  
> -	dev_dbg(&port->dev, "%s - returns %d\n", __func__, chars);
>  	return chars;
>  }

While I agree with most of this, the above two can actually be useful to
keep.

> @@ -924,11 +913,9 @@ static int ti_ioctl(struct tty_struct *tty,
>  
>  	switch (cmd) {
>  	case TIOCGSERIAL:
> -		dev_dbg(&port->dev, "%s - TIOCGSERIAL\n", __func__);
>  		return ti_get_serial_info(tport,
>  				(struct serial_struct __user *)arg);
>  	case TIOCSSERIAL:
> -		dev_dbg(&port->dev, "%s - TIOCSSERIAL\n", __func__);
>  		return ti_set_serial_info(tty, tport,
>  				(struct serial_struct __user *)arg);
>  	}
> @@ -950,9 +937,15 @@ static void ti_set_termios(struct tty_struct *tty,
>  	cflag = tty->termios.c_cflag;
>  	iflag = tty->termios.c_iflag;
>  
> -	dev_dbg(&port->dev, "%s - cflag %08x, iflag %08x\n", __func__, cflag, iflag);
> -	dev_dbg(&port->dev, "%s - old clfag %08x, old iflag %08x\n", __func__,
> -		old_termios->c_cflag, old_termios->c_iflag);
> +	dev_dbg(&port->dev,
> +		"%s - cflag 0x%08x, iflag 0x%08x\n", __func__, cflag, iflag);
> +
> +	if (old_termios) {
> +		dev_dbg(&port->dev, "%s - old clfag 0x%08x, old iflag 0x%08x\n",
> +			__func__,
> +			old_termios->c_cflag,
> +			old_termios->c_iflag);
> +	}

old_termios is not used in the function, why not drop it instead and fix
up the call sites?

Note that it will currently never be NULL.

>  
>  	if (tport == NULL)
>  		return;
> @@ -1140,8 +1133,6 @@ static void ti_break(struct tty_struct *tty, int break_state)
>  	struct ti_port *tport = usb_get_serial_port_data(port);
>  	int status;
>  
> -	dev_dbg(&port->dev, "%s - state = %d\n", __func__, break_state);

I'd keep this one as well.

> -
>  	if (tport == NULL)
>  		return;
>  
> @@ -1220,7 +1211,6 @@ static void ti_interrupt_callback(struct urb *urb)
>  
>  	case TI_CODE_MODEM_STATUS:
>  		msr = data[1];
> -		dev_dbg(dev, "%s - port %d, msr 0x%02X\n", __func__, port_number, msr);
>  		ti_handle_new_msr(tport, msr);
>  		break;

And this one.

Thanks,
Johan