Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753288AbeABPAE (ORCPT + 1 other); Tue, 2 Jan 2018 10:00:04 -0500 Received: from mail-lf0-f67.google.com ([209.85.215.67]:44175 "EHLO mail-lf0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750726AbeABPAC (ORCPT ); Tue, 2 Jan 2018 10:00:02 -0500 X-Google-Smtp-Source: ACJfBotSSeB9scTFBxasEGvFCE1J4+vyXj2EfPgLO8WmsePMjxiW7GFsJAxpNI7dRAH7WVFB+UO0wg== Date: Tue, 2 Jan 2018 16:00:02 +0100 From: Johan Hovold To: Mikhail Zaytsev Cc: Johan Hovold , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] USB: serial: ark3116.c: Move TIOCGSERIAL ioctl case to function. Message-ID: <20180102150002.GM16993@localhost> References: <20171213123004.4619000a@zaytsev.tver.pg> <20171213112445.GB3831@localhost> <20171213164506.79ef2eaa@zaytsev.tver.pg> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171213164506.79ef2eaa@zaytsev.tver.pg> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Wed, Dec 13, 2017 at 04:45:06PM +0300, Mikhail Zaytsev wrote: > The patch moves TIOCGSERIAL ioctl case to get_serial_info function. > > Signed-off-by: Mikhail Zaytsev > --- > drivers/usb/serial/ark3116.c | 35 +++++++++++++++++++++-------------- > 1 file changed, 21 insertions(+), 14 deletions(-) > > diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c > index 2e957c76f..2ce8fe10e 100644 > --- a/drivers/usb/serial/ark3116.c > +++ b/drivers/usb/serial/ark3116.c > @@ -36,6 +36,7 @@ > #define DRIVER_DESC "USB ARK3116 serial/IrDA driver" > #define DRIVER_DEV_DESC "ARK3116 RS232/IrDA" > #define DRIVER_NAME "ark3116" > +#define ARK3116_BAUDRATE 460800 I thought you and Oliver agreed this wasn't needed (wanted). Just hard-code this number in the new helper as before. > /* usb timeout of 1 second */ > #define ARK_TIMEOUT 1000 > @@ -397,27 +398,33 @@ static int ark3116_open(struct tty_struct *tty, struct usb_serial_port *port) > return result; > } > > +static int ark3116_get_serial_info(struct usb_serial_port *port, > + unsigned long arg) Keep the (void __user *) cast in ark3116_ioctl below and pass a struct serial_struct __user pointer here. > +{ > + struct serial_struct ser; > + > + memset(&ser, 0, sizeof(ser)); > + > + ser.type = PORT_16654; > + ser.line = port->minor; > + ser.port = port->port_number; > + ser.custom_divisor = 0; Might as well drop this redundant assignment as well. > + ser.baud_base = ARK3116_BAUDRATE; > + > + if (copy_to_user((void __user *)arg, &ser, sizeof(ser))) > + return -EFAULT; > + > + return 0; > +} > + > static int ark3116_ioctl(struct tty_struct *tty, > unsigned int cmd, unsigned long arg) > { > struct usb_serial_port *port = tty->driver_data; > - struct serial_struct serstruct; > - void __user *user_arg = (void __user *)arg; > > switch (cmd) { > case TIOCGSERIAL: > - /* XXX: Some of these values are probably wrong. */ > - memset(&serstruct, 0, sizeof(serstruct)); > - serstruct.type = PORT_16654; > - serstruct.line = port->minor; > - serstruct.port = port->port_number; > - serstruct.custom_divisor = 0; > - serstruct.baud_base = 460800; > - > - if (copy_to_user(user_arg, &serstruct, sizeof(serstruct))) > - return -EFAULT; > - > - return 0; > + return ark3116_get_serial_info(port, arg); > default: > break; > } Thanks, Johan