Re: [PATCH 07/36] usb: serial: ti_usb_3410_5052: Remove closing_wait module parameter

From: Johan Hovold <johan@kernel.org>
To: Mathieu OTHACEHE <m.othacehe@gmail.com>
Cc: johan@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 07/36] usb: serial: ti_usb_3410_5052: Remove closing_wait module parameter
Date: Fri, 15 Jul 2016 13:08:03 +0200	[thread overview]
Message-ID: <20160715110803.GF8809@localhost> (raw)
In-Reply-To: <1463042948-12205-8-git-send-email-m.othacehe@gmail.com>

On Thu, May 12, 2016 at 10:48:39AM +0200, Mathieu OTHACEHE wrote:
> Closing wait delay is configurable per device using TIOCSSERIAL.

Please try to make the commit messages self-contained and not rely on
the commit summary to make sense.

> Also initialise tty_port closing_wait in port_probe with default value.
> 
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
> index 164e07b..1860a5a 100644
> --- a/drivers/usb/serial/ti_usb_3410_5052.c
> +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> @@ -347,8 +347,6 @@ static int ti_write_byte(struct usb_serial_port *port, struct ti_device *tdev,
>  
>  static int ti_download_firmware(struct ti_device *tdev);
>  
> -static int closing_wait = TI_DEFAULT_CLOSING_WAIT;
> -
>  static const struct usb_device_id ti_id_table_3410[] = {
>  	{ USB_DEVICE(TI_VENDOR_ID, TI_3410_PRODUCT_ID) },
>  	{ USB_DEVICE(TI_VENDOR_ID, TI_3410_EZ430_ID) },
> @@ -498,10 +496,6 @@ MODULE_FIRMWARE("moxa/moxa-1131.fw");
>  MODULE_FIRMWARE("moxa/moxa-1150.fw");
>  MODULE_FIRMWARE("moxa/moxa-1151.fw");
>  
> -module_param(closing_wait, int, S_IRUGO | S_IWUSR);
> -MODULE_PARM_DESC(closing_wait,
> -    "Maximum wait for data to drain in close, in .01 secs, default is 4000");
> -

As there may systems out there relying on this module-parameter still,
I'm not sure it's a good idea to simply remove it.

>  MODULE_DEVICE_TABLE(usb, ti_id_table_combined);
>  
>  module_usb_serial_driver(serial_drivers, ti_id_table_combined);
> @@ -602,7 +596,7 @@ static int ti_port_probe(struct usb_serial_port *port)
>  		tport->tp_uart_base_addr = TI_UART1_BASE_ADDR;
>  	else
>  		tport->tp_uart_base_addr = TI_UART2_BASE_ADDR;
> -	port->port.closing_wait = msecs_to_jiffies(10 * closing_wait);
> +
>  	tport->tp_port = port;
>  	tport->tp_tdev = usb_get_serial_data(port->serial);
>  
> @@ -613,6 +607,8 @@ static int ti_port_probe(struct usb_serial_port *port)
>  
>  	usb_set_serial_port_data(port, tport);
>  
> +	port->port.closing_wait =
> +			msecs_to_jiffies(TI_DEFAULT_CLOSING_WAIT * 10);

Moving initialisation here makes sense though.

>  	port->port.drain_delay = 3;
>  
>  	return 0;

Thanks,
Johan