[PATCH 00/36] usb: serial: ti_usb_3410_5052: clean driver

[PATCH 00/36] usb: serial: ti_usb_3410_5052: clean driver

[Mathieu OTHACEHE]

Hi,

The now reverted mxu11x0 turned out to be a copy of ti_usb_3410_5052 driver.
This aim of this serie is to apply all of the cleanups we did in mxu11x0 to
ti_usb_3410_5052.

Thank you,

Mathieu

Mathieu OTHACEHE (36):
  usb: serial: ti_usb_3410_5052: Remove useless comments
  usb: serial: ti_usb_3410_5052: use __packed instead of
    __attribute__((packed))
  usb: serial: ti_usb_3410_5052: Remove ti_usb_3410_5052.h
  usb: serial: ti_usb_3410_5052: Use inline functions rather than macro
  usb: serial: ti_usb_3410_5052: Remove unused data structures
  usb: serial: ti_usb_3410_5052: Do not use __uX types
  usb: serial: ti_usb_3410_5052: Remove closing_wait module parameter
  usb: serial: ti_usb_3410_5052: Remove useless dev_dbg messages
  usb: serial: ti_usb_3410_5052: Use kzalloc instead of kmalloc
  usb: serial: ti_usb_3410_5052: Remove useless NULL-testing
  usb: serial: ti_usb_3410_5052: Use C_X macros instead of c_cflag
    manipulation
  usb: serial: ti_usb_3410_5052: Use generic read/write callbacks
  usb: serial: ti_usb_3410_5052: Remove unused variables
  usb: serial: ti_usb_3410_5052: Use macros instead of magic values
  usb: serial: ti_usb_3410_5052: Remove in_sync and out_sync functions
  usb: serial: ti_usb_3410_5052: Use bulk_out_size in TIOCGSERIAL
  usb: serial: ti_usb_3410_5052: Remove useless tty_wakeup
  usb: serial: ti_usb_3410_5052: Change ti_write_byte function arguments
  usb: serial: ti_usb_3410_5052: Do not modify interrupt context
  usb: serial: ti_usb_3410_5052: Remove usb_serial pointer in ti_port
  usb: serial: ti_usb_3410_5052: Use generic close function
  usb: serial: ti_usb_3410_5052: Change ti_get/set_serial_info function
    arguments
  usb: serial: ti_usb_3410_5052: Do not set shadow mcr in open callback
  usb: serial: ti_usb_3410_5052: Use usb_serial_generic_open
  usb: serial: ti_usb_3410_5052: Check old_termios parameter in
    set_termios
  usb: serial: ti_usb_3410_5052: Fix firmware downloading
  usb: serial: ti_usb_3410_5052: Standardize debug and error messages
  usb: serial: ti_usb_3410_5052: Use variables for vendor and product
  usb: serial: ti_usb_3410_5052: Set shadow msr before waking up waiters
  usb: serial: ti_usb_3410_5052: Remove backpointer in ti_port
  usb: serial: ti_usb_3410_5052: Use a mutex to protect shadow mcr
  usb: serial: ti_usb_3410_5052: Remove prefixes from private structures
  usb: serial: ti_usb_3410_5052: Add CMSPAR support
  usb: serial: ti_usb_3410_5052: Fix indentation problems
  usb: serial: ti_usb_3410_5052: Remove function prototypes
  usb: serial: ti_usb_3410_5052: Add myself as an author

 drivers/usb/serial/ti_usb_3410_5052.c | 2055 +++++++++++++++------------------
 drivers/usb/serial/ti_usb_3410_5052.h |   14 +-
 2 files changed, 937 insertions(+), 1132 deletions(-)

-- 
2.8.2

[PATCH 01/36] usb: serial: ti_usb_3410_5052: Remove useless comments

[Mathieu OTHACEHE]

Remove lines commenting the obvious.
Remove vi related comment.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index e7dbbef2..903b45c 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -1,5 +1,4 @@
-/* vi: ts=8 sw=8
- *
+/*
  * TI 3410/5052 USB Serial Driver
  *
  * Copyright (C) 2004 Texas Instruments
@@ -37,8 +36,6 @@
 
 #include "ti_usb_3410_5052.h"
 
-/* Defines */
-
 #define TI_DRIVER_AUTHOR	"Al Borchers <alborchers@steinerpoint.com>"
 #define TI_DRIVER_DESC		"TI USB 3410/5052 Serial Driver"
 
@@ -58,9 +55,6 @@
 
 #define TI_EXTRA_VID_PID_COUNT	5
 
-
-/* Structures */
-
 struct ti_port {
 	int			tp_is_open;
 	__u8			tp_msr;
@@ -84,9 +78,6 @@ struct ti_device {
 	int			td_urb_error;
 };
 
-
-/* Function Declarations */
-
 static int ti_startup(struct usb_serial *serial);
 static void ti_release(struct usb_serial *serial);
 static int ti_port_probe(struct usb_serial_port *port);
@@ -136,13 +127,8 @@ static int ti_write_byte(struct usb_serial_port *port, struct ti_device *tdev,
 
 static int ti_download_firmware(struct ti_device *tdev);
 
-
-/* Data */
-
-/* module parameters */
 static int closing_wait = TI_DEFAULT_CLOSING_WAIT;
 
-/* supported devices */
 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) },
@@ -174,7 +160,7 @@ static const struct usb_device_id ti_id_table_5052[] = {
 	{ USB_DEVICE(TI_VENDOR_ID, TI_5152_BOOT_PRODUCT_ID) },
 	{ USB_DEVICE(TI_VENDOR_ID, TI_5052_EEPROM_PRODUCT_ID) },
 	{ USB_DEVICE(TI_VENDOR_ID, TI_5052_FIRMWARE_PRODUCT_ID) },
-	{ }	/* terminator */
+	{ }
 };
 
 static const struct usb_device_id ti_id_table_combined[] = {
@@ -275,8 +261,6 @@ static struct usb_serial_driver * const serial_drivers[] = {
 	&ti_1port_device, &ti_2port_device, NULL
 };
 
-/* Module */
-
 MODULE_AUTHOR(TI_DRIVER_AUTHOR);
 MODULE_DESCRIPTION(TI_DRIVER_DESC);
 MODULE_LICENSE("GPL");
@@ -302,8 +286,6 @@ MODULE_DEVICE_TABLE(usb, ti_id_table_combined);
 
 module_usb_serial_driver(serial_drivers, ti_id_table_combined);
 
-/* Functions */
-
 static int ti_startup(struct usb_serial *serial)
 {
 	struct ti_device *tdev;
@@ -319,7 +301,6 @@ static int ti_startup(struct usb_serial *serial)
 		dev->descriptor.bNumConfigurations,
 		dev->actconfig->desc.bConfigurationValue);
 
-	/* create device structure */
 	tdev = kzalloc(sizeof(struct ti_device), GFP_KERNEL);
 	if (!tdev)
 		return -ENOMEM;
-- 
2.8.2

[PATCH 02/36] usb: serial: ti_usb_3410_5052: use __packed instead of __attribute__((packed))

[Mathieu OTHACEHE]

__packed is preferred over __attribute__((packed)) for portability.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.h b/drivers/usb/serial/ti_usb_3410_5052.h
index bbfd3a1..c39b13e 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.h
+++ b/drivers/usb/serial/ti_usb_3410_5052.h
@@ -174,7 +174,7 @@ struct ti_uart_config {
 	char	cXon;
 	char	cXoff;
 	__u8	bUartMode;
-} __attribute__((packed));
+} __packed;
 
 /* Get port status */
 struct ti_port_status {
@@ -183,7 +183,7 @@ struct ti_port_status {
 	__u8	bErrorCode;
 	__u8	bMSR;
 	__u8	bLSR;
-} __attribute__((packed));
+} __packed;
 
 /* Purge modes */
 #define TI_PURGE_OUTPUT			0x00
@@ -211,7 +211,7 @@ struct ti_write_data_bytes {
 	__be16	wBaseAddrHi;
 	__be16	wBaseAddrLo;
 	__u8	bData[0];
-} __attribute__((packed));
+} __packed;
 
 struct ti_read_data_request {
 	__u8	bAddrType;
@@ -219,20 +219,20 @@ struct ti_read_data_request {
 	__u8	bDataCounter;
 	__be16	wBaseAddrHi;
 	__be16	wBaseAddrLo;
-} __attribute__((packed));
+} __packed;
 
 struct ti_read_data_bytes {
 	__u8	bCmdCode;
 	__u8	bModuleId;
 	__u8	bErrorCode;
 	__u8	bData[0];
-} __attribute__((packed));
+} __packed;
 
 /* Interrupt struct */
 struct ti_interrupt {
 	__u8	bICode;
 	__u8	bIInfo;
-} __attribute__((packed));
+} __packed;
 
 /* Interrupt codes */
 #define TI_GET_PORT_FROM_CODE(c)	(((c) >> 4) - 3)
@@ -248,7 +248,7 @@ struct ti_interrupt {
 struct ti_firmware_header {
 	__le16	wLength;
 	__u8	bCheckSum;
-} __attribute__((packed));
+} __packed;
 
 /* UART addresses */
 #define TI_UART1_BASE_ADDR		0xFFA0	/* UART 1 base address */
-- 
2.8.2

[PATCH 03/36] usb: serial: ti_usb_3410_5052: Remove ti_usb_3410_5052.h

[Mathieu OTHACEHE]

The definitions in ti_usb_3410_5052.h are only used in
ti_usb_3410_5052.c.
The content of the header is copied in ti_usb_3410_5052.c.

Also correct a typo in macro TI_PIPE_MODE_CONTINOUS.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 237 +++++++++++++++++++++++++++++++++-
 1 file changed, 235 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 903b45c..6002e8b 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -34,7 +34,240 @@
 #include <linux/usb.h>
 #include <linux/usb/serial.h>
 
-#include "ti_usb_3410_5052.h"
+/* Configuration ids */
+#define TI_BOOT_CONFIG			1
+#define TI_ACTIVE_CONFIG		2
+
+/* Vendor and product ids */
+#define TI_VENDOR_ID			0x0451
+#define IBM_VENDOR_ID			0x04b3
+#define TI_3410_PRODUCT_ID		0x3410
+#define IBM_4543_PRODUCT_ID		0x4543
+#define IBM_454B_PRODUCT_ID		0x454b
+#define IBM_454C_PRODUCT_ID		0x454c
+#define TI_3410_EZ430_ID		0xF430  /* TI ez430 development tool */
+#define TI_5052_BOOT_PRODUCT_ID		0x5052	/* no EEPROM, no firmware */
+#define TI_5152_BOOT_PRODUCT_ID		0x5152	/* no EEPROM, no firmware */
+#define TI_5052_EEPROM_PRODUCT_ID	0x505A	/* EEPROM, no firmware */
+#define TI_5052_FIRMWARE_PRODUCT_ID	0x505F	/* firmware is running */
+#define FRI2_PRODUCT_ID			0x5053  /* Fish River Island II */
+
+/* Multi-Tech vendor and product ids */
+#define MTS_VENDOR_ID			0x06E0
+#define MTS_GSM_NO_FW_PRODUCT_ID	0xF108
+#define MTS_CDMA_NO_FW_PRODUCT_ID	0xF109
+#define MTS_CDMA_PRODUCT_ID		0xF110
+#define MTS_GSM_PRODUCT_ID		0xF111
+#define MTS_EDGE_PRODUCT_ID		0xF112
+#define MTS_MT9234MU_PRODUCT_ID		0xF114
+#define MTS_MT9234ZBA_PRODUCT_ID	0xF115
+#define MTS_MT9234ZBAOLD_PRODUCT_ID	0x0319
+
+/* Abbott Diabetics vendor and product ids */
+#define ABBOTT_VENDOR_ID		0x1a61
+#define ABBOTT_STEREO_PLUG_ID		0x3410
+#define ABBOTT_PRODUCT_ID		ABBOTT_STEREO_PLUG_ID
+#define ABBOTT_STRIP_PORT_ID		0x3420
+
+/* Honeywell vendor and product IDs */
+#define HONEYWELL_VENDOR_ID		0x10ac
+#define HONEYWELL_HGI80_PRODUCT_ID	0x0102  /* Honeywell HGI80 */
+
+/* Moxa UPORT 11x0 vendor and product IDs */
+#define MXU1_VENDOR_ID				0x110a
+#define MXU1_1110_PRODUCT_ID			0x1110
+#define MXU1_1130_PRODUCT_ID			0x1130
+#define MXU1_1150_PRODUCT_ID			0x1150
+#define MXU1_1151_PRODUCT_ID			0x1151
+#define MXU1_1131_PRODUCT_ID			0x1131
+
+/* Commands */
+#define TI_GET_VERSION			0x01
+#define TI_GET_PORT_STATUS		0x02
+#define TI_GET_PORT_DEV_INFO		0x03
+#define TI_GET_CONFIG			0x04
+#define TI_SET_CONFIG			0x05
+#define TI_OPEN_PORT			0x06
+#define TI_CLOSE_PORT			0x07
+#define TI_START_PORT			0x08
+#define TI_STOP_PORT			0x09
+#define TI_TEST_PORT			0x0A
+#define TI_PURGE_PORT			0x0B
+#define TI_RESET_EXT_DEVICE		0x0C
+#define TI_WRITE_DATA			0x80
+#define TI_READ_DATA			0x81
+#define TI_REQ_TYPE_CLASS		0x82
+
+/* Module identifiers */
+#define TI_I2C_PORT			0x01
+#define TI_IEEE1284_PORT		0x02
+#define TI_UART1_PORT			0x03
+#define TI_UART2_PORT			0x04
+#define TI_RAM_PORT			0x05
+
+/* Modem status */
+#define TI_MSR_DELTA_CTS		0x01
+#define TI_MSR_DELTA_DSR		0x02
+#define TI_MSR_DELTA_RI			0x04
+#define TI_MSR_DELTA_CD			0x08
+#define TI_MSR_CTS			0x10
+#define TI_MSR_DSR			0x20
+#define TI_MSR_RI			0x40
+#define TI_MSR_CD			0x80
+#define TI_MSR_DELTA_MASK		0x0F
+#define TI_MSR_MASK			0xF0
+
+/* Line status */
+#define TI_LSR_OVERRUN_ERROR		0x01
+#define TI_LSR_PARITY_ERROR		0x02
+#define TI_LSR_FRAMING_ERROR		0x04
+#define TI_LSR_BREAK			0x08
+#define TI_LSR_ERROR			0x0F
+#define TI_LSR_RX_FULL			0x10
+#define TI_LSR_TX_EMPTY			0x20
+
+/* Line control */
+#define TI_LCR_BREAK			0x40
+
+/* Modem control */
+#define TI_MCR_LOOP			0x04
+#define TI_MCR_DTR			0x10
+#define TI_MCR_RTS			0x20
+
+/* Mask settings */
+#define TI_UART_ENABLE_RTS_IN		0x0001
+#define TI_UART_DISABLE_RTS		0x0002
+#define TI_UART_ENABLE_PARITY_CHECKING	0x0008
+#define TI_UART_ENABLE_DSR_OUT		0x0010
+#define TI_UART_ENABLE_CTS_OUT		0x0020
+#define TI_UART_ENABLE_X_OUT		0x0040
+#define TI_UART_ENABLE_XA_OUT		0x0080
+#define TI_UART_ENABLE_X_IN		0x0100
+#define TI_UART_ENABLE_DTR_IN		0x0800
+#define TI_UART_DISABLE_DTR		0x1000
+#define TI_UART_ENABLE_MS_INTS		0x2000
+#define TI_UART_ENABLE_AUTO_START_DMA	0x4000
+
+/* Parity */
+#define TI_UART_NO_PARITY		0x00
+#define TI_UART_ODD_PARITY		0x01
+#define TI_UART_EVEN_PARITY		0x02
+#define TI_UART_MARK_PARITY		0x03
+#define TI_UART_SPACE_PARITY		0x04
+
+/* Stop bits */
+#define TI_UART_1_STOP_BITS		0x00
+#define TI_UART_1_5_STOP_BITS		0x01
+#define TI_UART_2_STOP_BITS		0x02
+
+/* Bits per character */
+#define TI_UART_5_DATA_BITS		0x00
+#define TI_UART_6_DATA_BITS		0x01
+#define TI_UART_7_DATA_BITS		0x02
+#define TI_UART_8_DATA_BITS		0x03
+
+/* 232/485 modes */
+#define TI_UART_232			0x00
+#define TI_UART_485_RECEIVER_DISABLED	0x01
+#define TI_UART_485_RECEIVER_ENABLED	0x02
+
+/* Pipe transfer mode and timeout */
+#define TI_PIPE_MODE_CONTINUOUS		0x01
+#define TI_PIPE_MODE_MASK		0x03
+#define TI_PIPE_TIMEOUT_MASK		0x7C
+#define TI_PIPE_TIMEOUT_ENABLE		0x80
+
+/* Config struct */
+struct ti_uart_config {
+	__u16	wBaudRate;
+	__u16	wFlags;
+	__u8	bDataBits;
+	__u8	bParity;
+	__u8	bStopBits;
+	char	cXon;
+	char	cXoff;
+	__u8	bUartMode;
+} __packed;
+
+/* Get port status */
+struct ti_port_status {
+	__u8	bCmdCode;
+	__u8	bModuleId;
+	__u8	bErrorCode;
+	__u8	bMSR;
+	__u8	bLSR;
+} __packed;
+
+/* Purge modes */
+#define TI_PURGE_OUTPUT			0x00
+#define TI_PURGE_INPUT			0x80
+
+/* Read/Write data */
+#define TI_RW_DATA_ADDR_SFR		0x10
+#define TI_RW_DATA_ADDR_IDATA		0x20
+#define TI_RW_DATA_ADDR_XDATA		0x30
+#define TI_RW_DATA_ADDR_CODE		0x40
+#define TI_RW_DATA_ADDR_GPIO		0x50
+#define TI_RW_DATA_ADDR_I2C		0x60
+#define TI_RW_DATA_ADDR_FLASH		0x70
+#define TI_RW_DATA_ADDR_DSP		0x80
+
+#define TI_RW_DATA_UNSPECIFIED		0x00
+#define TI_RW_DATA_BYTE			0x01
+#define TI_RW_DATA_WORD			0x02
+#define TI_RW_DATA_DOUBLE_WORD		0x04
+
+struct ti_write_data_bytes {
+	__u8	bAddrType;
+	__u8	bDataType;
+	__u8	bDataCounter;
+	__be16	wBaseAddrHi;
+	__be16	wBaseAddrLo;
+	__u8	bData[0];
+} __packed;
+
+struct ti_read_data_request {
+	__u8	bAddrType;
+	__u8	bDataType;
+	__u8	bDataCounter;
+	__be16	wBaseAddrHi;
+	__be16	wBaseAddrLo;
+} __packed;
+
+struct ti_read_data_bytes {
+	__u8	bCmdCode;
+	__u8	bModuleId;
+	__u8	bErrorCode;
+	__u8	bData[0];
+} __packed;
+
+/* Interrupt struct */
+struct ti_interrupt {
+	__u8	bICode;
+	__u8	bIInfo;
+} __packed;
+
+/* Interrupt codes */
+#define TI_GET_PORT_FROM_CODE(c)	(((c) >> 4) - 3)
+#define TI_GET_FUNC_FROM_CODE(c)	((c) & 0x0f)
+#define TI_CODE_HARDWARE_ERROR		0xFF
+#define TI_CODE_DATA_ERROR		0x03
+#define TI_CODE_MODEM_STATUS		0x04
+
+/* Download firmware max packet size */
+#define TI_DOWNLOAD_MAX_PACKET_SIZE	64
+
+/* Firmware image header */
+struct ti_firmware_header {
+	__le16	wLength;
+	__u8	bCheckSum;
+} __packed;
+
+/* UART addresses */
+#define TI_UART1_BASE_ADDR		0xFFA0	/* UART 1 base address */
+#define TI_UART2_BASE_ADDR		0xFFB0	/* UART 2 base address */
+#define TI_UART_OFFSET_LCR		0x0002	/* UART MCR register offset */
+#define TI_UART_OFFSET_MCR		0x0004	/* UART MCR register offset */
 
 #define TI_DRIVER_AUTHOR	"Al Borchers <alborchers@steinerpoint.com>"
 #define TI_DRIVER_DESC		"TI USB 3410/5052 Serial Driver"
@@ -416,7 +649,7 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	struct urb *urb;
 	int port_number;
 	int status;
-	__u16 open_settings = (__u8)(TI_PIPE_MODE_CONTINOUS |
+	__u16 open_settings = (__u8)(TI_PIPE_MODE_CONTINUOUS |
 			     TI_PIPE_TIMEOUT_ENABLE |
 			     (TI_TRANSFER_TIMEOUT << 2));
 
-- 
2.8.2

Re: [PATCH 03/36] usb: serial: ti_usb_3410_5052: Remove ti_usb_3410_5052.h

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:35AM +0200, Mathieu OTHACEHE wrote:
> The definitions in ti_usb_3410_5052.h are only used in
> ti_usb_3410_5052.c.
> The content of the header is copied in ti_usb_3410_5052.c.
> 
> Also correct a typo in macro TI_PIPE_MODE_CONTINOUS.
> 
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 237 +++++++++++++++++++++++++++++++++-
>  1 file changed, 235 insertions(+), 2 deletions(-)

You forgot actually remove the now unused header file. Fix it up before
applying.

Thanks,
Johan

[PATCH 04/36] usb: serial: ti_usb_3410_5052: Use inline functions rather than macro

[Mathieu OTHACEHE]

Inline functions are preferable to macros resembling functions.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 6002e8b..2fc3ea1 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -248,8 +248,16 @@ struct ti_interrupt {
 } __packed;
 
 /* Interrupt codes */
-#define TI_GET_PORT_FROM_CODE(c)	(((c) >> 4) - 3)
-#define TI_GET_FUNC_FROM_CODE(c)	((c) & 0x0f)
+static inline int ti_get_port_from_code(unsigned char code)
+{
+	return (code >> 4) - 3;
+}
+
+static inline int ti_get_func_from_code(unsigned char code)
+{
+	return code & 0x0f;
+}
+
 #define TI_CODE_HARDWARE_ERROR		0xFF
 #define TI_CODE_DATA_ERROR		0x03
 #define TI_CODE_MODEM_STATUS		0x04
@@ -1209,8 +1217,8 @@ static void ti_interrupt_callback(struct urb *urb)
 		goto exit;
 	}
 
-	port_number = TI_GET_PORT_FROM_CODE(data[0]);
-	function = TI_GET_FUNC_FROM_CODE(data[0]);
+	port_number = ti_get_port_from_code(data[0]);
+	function = ti_get_func_from_code(data[0]);
 
 	dev_dbg(dev, "%s - port_number %d, function %d, data 0x%02X\n",
 		__func__, port_number, function, data[1]);
-- 
2.8.2

Re: [PATCH 04/36] usb: serial: ti_usb_3410_5052: Use inline functions rather than macro

[Sergei Shtylyov]

Hello.

On 5/12/2016 11:48 AM, Mathieu OTHACEHE wrote:

> Inline functions are preferable to macros resembling functions.
>
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
> index 6002e8b..2fc3ea1 100644
> --- a/drivers/usb/serial/ti_usb_3410_5052.c
> +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> @@ -248,8 +248,16 @@ struct ti_interrupt {
>  } __packed;
>
>  /* Interrupt codes */
> -#define TI_GET_PORT_FROM_CODE(c)	(((c) >> 4) - 3)
> -#define TI_GET_FUNC_FROM_CODE(c)	((c) & 0x0f)
> +static inline int ti_get_port_from_code(unsigned char code)
> +{
> +	return (code >> 4) - 3;
> +}
> +
> +static inline int ti_get_func_from_code(unsigned char code)
> +{
> +	return code & 0x0f;
> +}
> +

    We don't specify *inline* in the .c files (only in .h), letting gcc figure 
it out.

[...]

MBR, Sergei

Re: [PATCH 04/36] usb: serial: ti_usb_3410_5052: Use inline functions rather than macro

[Johan Hovold]

On Thu, May 12, 2016 at 03:00:39PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 5/12/2016 11:48 AM, Mathieu OTHACEHE wrote:
> 
> > Inline functions are preferable to macros resembling functions.
> >
> > Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> > ---
> >  drivers/usb/serial/ti_usb_3410_5052.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
> > index 6002e8b..2fc3ea1 100644
> > --- a/drivers/usb/serial/ti_usb_3410_5052.c
> > +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> > @@ -248,8 +248,16 @@ struct ti_interrupt {
> >  } __packed;
> >
> >  /* Interrupt codes */
> > -#define TI_GET_PORT_FROM_CODE(c)	(((c) >> 4) - 3)
> > -#define TI_GET_FUNC_FROM_CODE(c)	((c) & 0x0f)
> > +static inline int ti_get_port_from_code(unsigned char code)
> > +{
> > +	return (code >> 4) - 3;
> > +}
> > +
> > +static inline int ti_get_func_from_code(unsigned char code)
> > +{
> > +	return code & 0x0f;
> > +}
> > +
> 
>     We don't specify *inline* in the .c files (only in .h), letting gcc figure 
> it out.

Indeed. I dropped the inline keywords and also moved the functions above
the interrupt-in completion handler, which is the only place they are
used.

Thanks,
Johan

[PATCH 05/36] usb: serial: ti_usb_3410_5052: Remove unused data structures

[Mathieu OTHACEHE]

ti_read_data_request, ti_read_data_bytes and ti_interrupt are unused.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 2fc3ea1..af4e145 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -226,27 +226,6 @@ struct ti_write_data_bytes {
 	__u8	bData[0];
 } __packed;
 
-struct ti_read_data_request {
-	__u8	bAddrType;
-	__u8	bDataType;
-	__u8	bDataCounter;
-	__be16	wBaseAddrHi;
-	__be16	wBaseAddrLo;
-} __packed;
-
-struct ti_read_data_bytes {
-	__u8	bCmdCode;
-	__u8	bModuleId;
-	__u8	bErrorCode;
-	__u8	bData[0];
-} __packed;
-
-/* Interrupt struct */
-struct ti_interrupt {
-	__u8	bICode;
-	__u8	bIInfo;
-} __packed;
-
 /* Interrupt codes */
 static inline int ti_get_port_from_code(unsigned char code)
 {
-- 
2.8.2

Re: [PATCH 05/36] usb: serial: ti_usb_3410_5052: Remove unused data structures

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:37AM +0200, Mathieu OTHACEHE wrote:
> ti_read_data_request, ti_read_data_bytes and ti_interrupt are unused.
> 
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 21 ---------------------
>  1 file changed, 21 deletions(-)
> 
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
> index 2fc3ea1..af4e145 100644
> --- a/drivers/usb/serial/ti_usb_3410_5052.c
> +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> @@ -226,27 +226,6 @@ struct ti_write_data_bytes {
>  	__u8	bData[0];
>  } __packed;
>  
> -struct ti_read_data_request {
> -	__u8	bAddrType;
> -	__u8	bDataType;
> -	__u8	bDataCounter;
> -	__be16	wBaseAddrHi;
> -	__be16	wBaseAddrLo;
> -} __packed;
> -
> -struct ti_read_data_bytes {
> -	__u8	bCmdCode;
> -	__u8	bModuleId;
> -	__u8	bErrorCode;
> -	__u8	bData[0];
> -} __packed;
> -
> -/* Interrupt struct */
> -struct ti_interrupt {
> -	__u8	bICode;
> -	__u8	bIInfo;
> -} __packed;

I'm not sure this is a good idea as we lose protocol information this
way. The ti_interrupt message is in fact used in the interrupt
completion callback by accessing bICode through data[0] for example.

Why not put the structs to use instead?

Thanks,
Johan

[PATCH 06/36] usb: serial: ti_usb_3410_5052: Do not use __uX types

[Mathieu OTHACEHE]

__uX types should only be used for user-space interactions.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 66 ++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index af4e145..164e07b 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -179,23 +179,23 @@
 
 /* Config struct */
 struct ti_uart_config {
-	__u16	wBaudRate;
-	__u16	wFlags;
-	__u8	bDataBits;
-	__u8	bParity;
-	__u8	bStopBits;
+	__be16	wBaudRate;
+	__be16	wFlags;
+	u8	bDataBits;
+	u8	bParity;
+	u8	bStopBits;
 	char	cXon;
 	char	cXoff;
-	__u8	bUartMode;
+	u8	bUartMode;
 } __packed;
 
 /* Get port status */
 struct ti_port_status {
-	__u8	bCmdCode;
-	__u8	bModuleId;
-	__u8	bErrorCode;
-	__u8	bMSR;
-	__u8	bLSR;
+	u8 bCmdCode;
+	u8 bModuleId;
+	u8 bErrorCode;
+	u8 bMSR;
+	u8 bLSR;
 } __packed;
 
 /* Purge modes */
@@ -218,12 +218,12 @@ struct ti_port_status {
 #define TI_RW_DATA_DOUBLE_WORD		0x04
 
 struct ti_write_data_bytes {
-	__u8	bAddrType;
-	__u8	bDataType;
-	__u8	bDataCounter;
+	u8	bAddrType;
+	u8	bDataType;
+	u8	bDataCounter;
 	__be16	wBaseAddrHi;
 	__be16	wBaseAddrLo;
-	__u8	bData[0];
+	u8	bData[0];
 } __packed;
 
 /* Interrupt codes */
@@ -247,7 +247,7 @@ static inline int ti_get_func_from_code(unsigned char code)
 /* Firmware image header */
 struct ti_firmware_header {
 	__le16	wLength;
-	__u8	bCheckSum;
+	u8	bCheckSum;
 } __packed;
 
 /* UART addresses */
@@ -277,9 +277,9 @@ struct ti_firmware_header {
 
 struct ti_port {
 	int			tp_is_open;
-	__u8			tp_msr;
-	__u8			tp_shadow_mcr;
-	__u8			tp_uart_mode;	/* 232 or 485 modes */
+	u8			tp_msr;
+	u8			tp_shadow_mcr;
+	u8			tp_uart_mode;	/* 232 or 485 modes */
 	unsigned int		tp_uart_base_addr;
 	int			tp_flags;
 	struct ti_device	*tp_tdev;
@@ -332,7 +332,7 @@ static int ti_get_serial_info(struct ti_port *tport,
 	struct serial_struct __user *ret_arg);
 static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
 	struct serial_struct __user *new_arg);
-static void ti_handle_new_msr(struct ti_port *tport, __u8 msr);
+static void ti_handle_new_msr(struct ti_port *tport, u8 msr);
 
 static void ti_stop_read(struct ti_port *tport, struct tty_struct *tty);
 static int ti_restart_read(struct ti_port *tport, struct tty_struct *tty);
@@ -343,7 +343,7 @@ static int ti_command_in_sync(struct ti_device *tdev, __u8 command,
 	__u16 moduleid, __u16 value, __u8 *data, int size);
 
 static int ti_write_byte(struct usb_serial_port *port, struct ti_device *tdev,
-			 unsigned long addr, __u8 mask, __u8 byte);
+			 unsigned long addr, u8 mask, u8 byte);
 
 static int ti_download_firmware(struct ti_device *tdev);
 
@@ -636,9 +636,11 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	struct urb *urb;
 	int port_number;
 	int status;
-	__u16 open_settings = (__u8)(TI_PIPE_MODE_CONTINUOUS |
-			     TI_PIPE_TIMEOUT_ENABLE |
-			     (TI_TRANSFER_TIMEOUT << 2));
+	u16 open_settings;
+
+	open_settings = (TI_PIPE_MODE_CONTINUOUS |
+			 TI_PIPE_TIMEOUT_ENABLE |
+			 (TI_TRANSFER_TIMEOUT << 2));
 
 	if (tport == NULL)
 		return -ENODEV;
@@ -968,7 +970,7 @@ static void ti_set_termios(struct tty_struct *tty,
 	/* these flags must be set */
 	config->wFlags |= TI_UART_ENABLE_MS_INTS;
 	config->wFlags |= TI_UART_ENABLE_AUTO_START_DMA;
-	config->bUartMode = (__u8)(tport->tp_uart_mode);
+	config->bUartMode = tport->tp_uart_mode;
 
 	switch (cflag & CSIZE) {
 	case CS5:
@@ -1169,7 +1171,7 @@ static void ti_interrupt_callback(struct urb *urb)
 	int function;
 	int status = urb->status;
 	int retval;
-	__u8 msr;
+	u8 msr;
 
 	switch (status) {
 	case 0:
@@ -1502,7 +1504,7 @@ static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
 }
 
 
-static void ti_handle_new_msr(struct ti_port *tport, __u8 msr)
+static void ti_handle_new_msr(struct ti_port *tport, u8 msr)
 {
 	struct async_icount *icount;
 	struct tty_struct *tty;
@@ -1614,8 +1616,8 @@ static int ti_command_in_sync(struct ti_device *tdev, __u8 command,
 
 
 static int ti_write_byte(struct usb_serial_port *port,
-			struct ti_device *tdev, unsigned long addr,
-			__u8 mask, __u8 byte)
+			 struct ti_device *tdev, unsigned long addr,
+			 u8 mask, u8 byte)
 {
 	int status;
 	unsigned int size;
@@ -1659,10 +1661,10 @@ static int ti_do_download(struct usb_device *dev, int pipe,
 	int len;
 
 	for (pos = sizeof(struct ti_firmware_header); pos < size; pos++)
-		cs = (__u8)(cs + buffer[pos]);
+		cs = (u8)(cs + buffer[pos]);
 
 	header = (struct ti_firmware_header *)buffer;
-	header->wLength = cpu_to_le16((__u16)(size
+	header->wLength = cpu_to_le16((u16)(size
 					- sizeof(struct ti_firmware_header)));
 	header->bCheckSum = cs;
 
@@ -1681,7 +1683,7 @@ static int ti_download_firmware(struct ti_device *tdev)
 {
 	int status;
 	int buffer_size;
-	__u8 *buffer;
+	u8 *buffer;
 	struct usb_device *dev = tdev->td_serial->dev;
 	unsigned int pipe = usb_sndbulkpipe(dev,
 		tdev->td_serial->port[0]->bulk_out_endpointAddress);
-- 
2.8.2

Re: [PATCH 06/36] usb: serial: ti_usb_3410_5052: Do not use __uX types

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:38AM +0200, Mathieu OTHACEHE wrote:
> __uX types should only be used for user-space interactions.
> 
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 66 ++++++++++++++++++-----------------
>  1 file changed, 34 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
> index af4e145..164e07b 100644
> --- a/drivers/usb/serial/ti_usb_3410_5052.c
> +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> @@ -179,23 +179,23 @@
>  
>  /* Config struct */
>  struct ti_uart_config {
> -	__u16	wBaudRate;
> -	__u16	wFlags;
> -	__u8	bDataBits;
> -	__u8	bParity;
> -	__u8	bStopBits;
> +	__be16	wBaudRate;
> +	__be16	wFlags;

While I have nothing against replacing the __u<width>, this makes me
worried. How, if at all, is the endianess of these fields handled in the
code?

Ok, I see now it's using cpu_to_be16s after writing host-endian data
into it. Could you clean that to use cpu_to_be16 instead (you may need
to update an error message too)?

Note that the wBaudRate assignment was still using __u16 after this
patch.

> +	u8	bDataBits;
> +	u8	bParity;
> +	u8	bStopBits;
>  	char	cXon;
>  	char	cXoff;
> -	__u8	bUartMode;
> +	u8	bUartMode;
>  } __packed;
 
>  static int ti_write_byte(struct usb_serial_port *port,
> -			struct ti_device *tdev, unsigned long addr,
> -			__u8 mask, __u8 byte)
> +			 struct ti_device *tdev, unsigned long addr,
> +			 u8 mask, u8 byte)
>  {
>  	int status;
>  	unsigned int size;
> @@ -1659,10 +1661,10 @@ static int ti_do_download(struct usb_device *dev, int pipe,
>  	int len;
>  
>  	for (pos = sizeof(struct ti_firmware_header); pos < size; pos++)
> -		cs = (__u8)(cs + buffer[pos]);
> +		cs = (u8)(cs + buffer[pos]);
>  
>  	header = (struct ti_firmware_header *)buffer;
> -	header->wLength = cpu_to_le16((__u16)(size
> +	header->wLength = cpu_to_le16((u16)(size
>  					- sizeof(struct ti_firmware_header)));

Cast not needed.

Thanks,
Johan

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

[Mathieu OTHACEHE]

Closing wait delay is configurable per device using TIOCSSERIAL.

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");
-
 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);
 	port->port.drain_delay = 3;
 
 	return 0;
-- 
2.8.2

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

[Johan Hovold]

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

[PATCH 08/36] usb: serial: ti_usb_3410_5052: Remove useless dev_dbg messages

[Mathieu OTHACEHE]

Remove useless or redundant dev_dbg messages.
Fix debug-message typos.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.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
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -673,7 +673,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	if (tty)
 		ti_set_termios(tty, port, &tty->termios);
 
-	dev_dbg(&port->dev, "%s - sending TI_OPEN_PORT\n", __func__);
 	status = ti_command_out_sync(tdev, TI_OPEN_PORT,
 		(__u8)(TI_UART1_PORT + port_number), open_settings, NULL, 0);
 	if (status) {
@@ -682,7 +681,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 		goto unlink_int_urb;
 	}
 
-	dev_dbg(&port->dev, "%s - sending TI_START_PORT\n", __func__);
 	status = ti_command_out_sync(tdev, TI_START_PORT,
 		(__u8)(TI_UART1_PORT + port_number), 0, NULL, 0);
 	if (status) {
@@ -691,7 +689,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 		goto unlink_int_urb;
 	}
 
-	dev_dbg(&port->dev, "%s - sending TI_PURGE_PORT\n", __func__);
 	status = ti_command_out_sync(tdev, TI_PURGE_PORT,
 		(__u8)(TI_UART1_PORT + port_number), TI_PURGE_INPUT, NULL, 0);
 	if (status) {
@@ -715,7 +712,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	if (tty)
 		ti_set_termios(tty, port, &tty->termios);
 
-	dev_dbg(&port->dev, "%s - sending TI_OPEN_PORT (2)\n", __func__);
 	status = ti_command_out_sync(tdev, TI_OPEN_PORT,
 		(__u8)(TI_UART1_PORT + port_number), open_settings, NULL, 0);
 	if (status) {
@@ -724,7 +720,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 		goto unlink_int_urb;
 	}
 
-	dev_dbg(&port->dev, "%s - sending TI_START_PORT (2)\n", __func__);
 	status = ti_command_out_sync(tdev, TI_START_PORT,
 		(__u8)(TI_UART1_PORT + port_number), 0, NULL, 0);
 	if (status) {
@@ -734,7 +729,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	}
 
 	/* start read urb */
-	dev_dbg(&port->dev, "%s - start read urb\n", __func__);
 	urb = port->read_urb;
 	if (!urb) {
 		dev_err(&port->dev, "%s - no read urb\n", __func__);
@@ -760,7 +754,6 @@ unlink_int_urb:
 		usb_kill_urb(port->serial->port[0]->interrupt_in_urb);
 release_lock:
 	mutex_unlock(&tdev->td_open_close_lock);
-	dev_dbg(&port->dev, "%s - exit %d\n", __func__, status);
 	return status;
 }
 
@@ -790,7 +783,6 @@ static void ti_close(struct usb_serial_port *port)
 
 	port_number = port->port_number;
 
-	dev_dbg(&port->dev, "%s - sending TI_CLOSE_PORT\n", __func__);
 	status = ti_command_out_sync(tdev, TI_CLOSE_PORT,
 		     (__u8)(TI_UART1_PORT + port_number), 0, NULL, 0);
 	if (status)
@@ -817,7 +809,6 @@ static int ti_write(struct tty_struct *tty, struct usb_serial_port *port,
 	struct ti_port *tport = usb_get_serial_port_data(port);
 
 	if (count == 0) {
-		dev_dbg(&port->dev, "%s - write request of 0 bytes\n", __func__);
 		return 0;
 	}
 
@@ -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;
 }
 
@@ -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);
+	}
 
 	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);
-
 	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;
 
-- 
2.8.2

Re: [PATCH 08/36] usb: serial: ti_usb_3410_5052: Remove useless dev_dbg messages

[Johan Hovold]

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@gmail.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

[PATCH 09/36] usb: serial: ti_usb_3410_5052: Use kzalloc instead of kmalloc

[Mathieu OTHACEHE]

Use kzalloc instead of kmalloc to avoid field initialisation to 0.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 88aacf5..c9e2345 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -950,12 +950,10 @@ static void ti_set_termios(struct tty_struct *tty,
 	if (tport == NULL)
 		return;
 
-	config = kmalloc(sizeof(*config), GFP_KERNEL);
+	config = kzalloc(sizeof(*config), GFP_KERNEL);
 	if (!config)
 		return;
 
-	config->wFlags = 0;
-
 	/* these flags must be set */
 	config->wFlags |= TI_UART_ENABLE_MS_INTS;
 	config->wFlags |= TI_UART_ENABLE_AUTO_START_DMA;
-- 
2.8.2

[PATCH 10/36] usb: serial: ti_usb_3410_5052: Remove useless NULL-testing

[Mathieu OTHACEHE]

It is useless to check the return of usb_get_serial_port_data.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 34 +---------------------------------
 1 file changed, 1 insertion(+), 33 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index c9e2345..ef6789b 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -638,9 +638,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 			 TI_PIPE_TIMEOUT_ENABLE |
 			 (TI_TRANSFER_TIMEOUT << 2));
 
-	if (tport == NULL)
-		return -ENODEV;
-
 	dev = port->serial->dev;
 	tdev = tport->tp_tdev;
 
@@ -769,8 +766,6 @@ static void ti_close(struct usb_serial_port *port)
 
 	tdev = usb_get_serial_data(port->serial);
 	tport = usb_get_serial_port_data(port);
-	if (tdev == NULL || tport == NULL)
-		return;
 
 	tport->tp_is_open = 0;
 
@@ -812,7 +807,7 @@ static int ti_write(struct tty_struct *tty, struct usb_serial_port *port,
 		return 0;
 	}
 
-	if (tport == NULL || !tport->tp_is_open)
+	if (!tport->tp_is_open)
 		return -ENODEV;
 
 	count = kfifo_in_locked(&port->write_fifo, data, count,
@@ -830,9 +825,6 @@ static int ti_write_room(struct tty_struct *tty)
 	int room = 0;
 	unsigned long flags;
 
-	if (tport == NULL)
-		return 0;
-
 	spin_lock_irqsave(&tport->tp_lock, flags);
 	room = kfifo_avail(&port->write_fifo);
 	spin_unlock_irqrestore(&tport->tp_lock, flags);
@@ -848,9 +840,6 @@ static int ti_chars_in_buffer(struct tty_struct *tty)
 	int chars = 0;
 	unsigned long flags;
 
-	if (tport == NULL)
-		return 0;
-
 	spin_lock_irqsave(&tport->tp_lock, flags);
 	chars = kfifo_len(&port->write_fifo);
 	spin_unlock_irqrestore(&tport->tp_lock, flags);
@@ -876,9 +865,6 @@ static void ti_throttle(struct tty_struct *tty)
 	struct usb_serial_port *port = tty->driver_data;
 	struct ti_port *tport = usb_get_serial_port_data(port);
 
-	if (tport == NULL)
-		return;
-
 	if (I_IXOFF(tty) || C_CRTSCTS(tty))
 		ti_stop_read(tport, tty);
 
@@ -891,9 +877,6 @@ static void ti_unthrottle(struct tty_struct *tty)
 	struct ti_port *tport = usb_get_serial_port_data(port);
 	int status;
 
-	if (tport == NULL)
-		return;
-
 	if (I_IXOFF(tty) || C_CRTSCTS(tty)) {
 		status = ti_restart_read(tport, tty);
 		if (status)
@@ -908,9 +891,6 @@ static int ti_ioctl(struct tty_struct *tty,
 	struct usb_serial_port *port = tty->driver_data;
 	struct ti_port *tport = usb_get_serial_port_data(port);
 
-	if (tport == NULL)
-		return -ENODEV;
-
 	switch (cmd) {
 	case TIOCGSERIAL:
 		return ti_get_serial_info(tport,
@@ -947,9 +927,6 @@ static void ti_set_termios(struct tty_struct *tty,
 			old_termios->c_iflag);
 	}
 
-	if (tport == NULL)
-		return;
-
 	config = kzalloc(sizeof(*config), GFP_KERNEL);
 	if (!config)
 		return;
@@ -1070,9 +1047,6 @@ static int ti_tiocmget(struct tty_struct *tty)
 	unsigned int mcr;
 	unsigned long flags;
 
-	if (tport == NULL)
-		return -ENODEV;
-
 	spin_lock_irqsave(&tport->tp_lock, flags);
 	msr = tport->tp_msr;
 	mcr = tport->tp_shadow_mcr;
@@ -1100,9 +1074,6 @@ static int ti_tiocmset(struct tty_struct *tty,
 	unsigned int mcr;
 	unsigned long flags;
 
-	if (tport == NULL)
-		return -ENODEV;
-
 	spin_lock_irqsave(&tport->tp_lock, flags);
 	mcr = tport->tp_shadow_mcr;
 
@@ -1131,9 +1102,6 @@ static void ti_break(struct tty_struct *tty, int break_state)
 	struct ti_port *tport = usb_get_serial_port_data(port);
 	int status;
 
-	if (tport == NULL)
-		return;
-
 	status = ti_write_byte(port, tport->tp_tdev,
 		tport->tp_uart_base_addr + TI_UART_OFFSET_LCR,
 		TI_LCR_BREAK, break_state == -1 ? TI_LCR_BREAK : 0);
-- 
2.8.2

[PATCH 11/36] usb: serial: ti_usb_3410_5052: Use C_X macros instead of c_cflag manipulation

[Mathieu OTHACEHE]

Use C_X tty.h macros to avoid direct manipulation of termios
c_cflag variable.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index ef6789b..601fab6 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -936,7 +936,7 @@ static void ti_set_termios(struct tty_struct *tty,
 	config->wFlags |= TI_UART_ENABLE_AUTO_START_DMA;
 	config->bUartMode = tport->tp_uart_mode;
 
-	switch (cflag & CSIZE) {
+	switch (C_CSIZE(tty)) {
 	case CS5:
 		    config->bDataBits = TI_UART_5_DATA_BITS;
 		    break;
@@ -955,8 +955,8 @@ static void ti_set_termios(struct tty_struct *tty,
 	/* CMSPAR isn't supported by this driver */
 	tty->termios.c_cflag &= ~CMSPAR;
 
-	if (cflag & PARENB) {
-		if (cflag & PARODD) {
+	if (C_PARENB(tty)) {
+		if (C_PARODD(tty)) {
 			config->wFlags |= TI_UART_ENABLE_PARITY_CHECKING;
 			config->bParity = TI_UART_ODD_PARITY;
 		} else {
@@ -968,14 +968,14 @@ static void ti_set_termios(struct tty_struct *tty,
 		config->bParity = TI_UART_NO_PARITY;
 	}
 
-	if (cflag & CSTOPB)
+	if (C_CSTOPB(tty))
 		config->bStopBits = TI_UART_2_STOP_BITS;
 	else
 		config->bStopBits = TI_UART_1_STOP_BITS;
 
-	if (cflag & CRTSCTS) {
+	if (C_CRTSCTS(tty)) {
 		/* RTS flow control must be off to drop RTS for baud rate B0 */
-		if ((cflag & CBAUD) != B0)
+		if ((C_BAUD(tty)) != B0)
 			config->wFlags |= TI_UART_ENABLE_RTS_IN;
 		config->wFlags |= TI_UART_ENABLE_CTS_OUT;
 	} else {
@@ -1004,7 +1004,7 @@ static void ti_set_termios(struct tty_struct *tty,
 		config->wBaudRate = (__u16)((461538 + baud/2) / baud);
 
 	/* FIXME: Should calculate resulting baud here and report it back */
-	if ((cflag & CBAUD) != B0)
+	if ((C_BAUD(tty)) != B0)
 		tty_encode_baud_rate(tty, baud, baud);
 
 	dev_dbg(&port->dev,
@@ -1026,7 +1026,7 @@ static void ti_set_termios(struct tty_struct *tty,
 	/* SET_CONFIG asserts RTS and DTR, reset them correctly */
 	mcr = tport->tp_shadow_mcr;
 	/* if baud rate is B0, clear RTS and DTR */
-	if ((cflag & CBAUD) == B0)
+	if (C_BAUD(tty) == B0)
 		mcr &= ~(TI_MCR_DTR | TI_MCR_RTS);
 	status = ti_set_mcr(tport, mcr);
 	if (status)
-- 
2.8.2

[PATCH 12/36] usb: serial: ti_usb_3410_5052: Use generic read/write callbacks

[Mathieu OTHACEHE]

Remove read_bulk_callback, write_bulk_callback, write, write_room,
chars_in_buffer, throttle and unthrottle callbacks who uselessly
reimplements generic functions.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 315 ----------------------------------
 1 file changed, 315 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 601fab6..58df42d 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -268,11 +268,6 @@ struct ti_firmware_header {
 /* supported setserial flags */
 #define TI_SET_SERIAL_FLAGS	0
 
-/* read urb states */
-#define TI_READ_URB_RUNNING	0
-#define TI_READ_URB_STOPPING	1
-#define TI_READ_URB_STOPPED	2
-
 #define TI_EXTRA_VID_PID_COUNT	5
 
 struct ti_port {
@@ -285,8 +280,6 @@ struct ti_port {
 	struct ti_device	*tp_tdev;
 	struct usb_serial_port	*tp_port;
 	spinlock_t		tp_lock;
-	int			tp_read_urb_state;
-	int			tp_write_urb_in_use;
 };
 
 struct ti_device {
@@ -304,13 +297,7 @@ static int ti_port_probe(struct usb_serial_port *port);
 static int ti_port_remove(struct usb_serial_port *port);
 static int ti_open(struct tty_struct *tty, struct usb_serial_port *port);
 static void ti_close(struct usb_serial_port *port);
-static int ti_write(struct tty_struct *tty, struct usb_serial_port *port,
-		const unsigned char *data, int count);
-static int ti_write_room(struct tty_struct *tty);
-static int ti_chars_in_buffer(struct tty_struct *tty);
 static bool ti_tx_empty(struct usb_serial_port *port);
-static void ti_throttle(struct tty_struct *tty);
-static void ti_unthrottle(struct tty_struct *tty);
 static int ti_ioctl(struct tty_struct *tty,
 		unsigned int cmd, unsigned long arg);
 static void ti_set_termios(struct tty_struct *tty,
@@ -320,12 +307,7 @@ static int ti_tiocmset(struct tty_struct *tty,
 		unsigned int set, unsigned int clear);
 static void ti_break(struct tty_struct *tty, int break_state);
 static void ti_interrupt_callback(struct urb *urb);
-static void ti_bulk_in_callback(struct urb *urb);
-static void ti_bulk_out_callback(struct urb *urb);
 
-static void ti_recv(struct usb_serial_port *port, unsigned char *data,
-		int length);
-static void ti_send(struct ti_port *tport);
 static int ti_set_mcr(struct ti_port *tport, unsigned int mcr);
 static int ti_get_lsr(struct ti_port *tport, u8 *lsr);
 static int ti_get_serial_info(struct ti_port *tport,
@@ -334,9 +316,6 @@ static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
 	struct serial_struct __user *new_arg);
 static void ti_handle_new_msr(struct ti_port *tport, u8 msr);
 
-static void ti_stop_read(struct ti_port *tport, struct tty_struct *tty);
-static int ti_restart_read(struct ti_port *tport, struct tty_struct *tty);
-
 static int ti_command_out_sync(struct ti_device *tdev, __u8 command,
 	__u16 moduleid, __u16 value, __u8 *data, int size);
 static int ti_command_in_sync(struct ti_device *tdev, __u8 command,
@@ -425,12 +404,7 @@ static struct usb_serial_driver ti_1port_device = {
 	.port_remove		= ti_port_remove,
 	.open			= ti_open,
 	.close			= ti_close,
-	.write			= ti_write,
-	.write_room		= ti_write_room,
-	.chars_in_buffer	= ti_chars_in_buffer,
 	.tx_empty		= ti_tx_empty,
-	.throttle		= ti_throttle,
-	.unthrottle		= ti_unthrottle,
 	.ioctl			= ti_ioctl,
 	.set_termios		= ti_set_termios,
 	.tiocmget		= ti_tiocmget,
@@ -439,8 +413,6 @@ static struct usb_serial_driver ti_1port_device = {
 	.get_icount		= usb_serial_generic_get_icount,
 	.break_ctl		= ti_break,
 	.read_int_callback	= ti_interrupt_callback,
-	.read_bulk_callback	= ti_bulk_in_callback,
-	.write_bulk_callback	= ti_bulk_out_callback,
 };
 
 static struct usb_serial_driver ti_2port_device = {
@@ -457,12 +429,7 @@ static struct usb_serial_driver ti_2port_device = {
 	.port_remove		= ti_port_remove,
 	.open			= ti_open,
 	.close			= ti_close,
-	.write			= ti_write,
-	.write_room		= ti_write_room,
-	.chars_in_buffer	= ti_chars_in_buffer,
 	.tx_empty		= ti_tx_empty,
-	.throttle		= ti_throttle,
-	.unthrottle		= ti_unthrottle,
 	.ioctl			= ti_ioctl,
 	.set_termios		= ti_set_termios,
 	.tiocmget		= ti_tiocmget,
@@ -471,8 +438,6 @@ static struct usb_serial_driver ti_2port_device = {
 	.get_icount		= usb_serial_generic_get_icount,
 	.break_ctl		= ti_break,
 	.read_int_callback	= ti_interrupt_callback,
-	.read_bulk_callback	= ti_bulk_in_callback,
-	.write_bulk_callback	= ti_bulk_out_callback,
 };
 
 static struct usb_serial_driver * const serial_drivers[] = {
@@ -732,7 +697,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 		status = -EINVAL;
 		goto unlink_int_urb;
 	}
-	tport->tp_read_urb_state = TI_READ_URB_RUNNING;
 	urb->context = tport;
 	status = usb_submit_urb(urb, GFP_KERNEL);
 	if (status) {
@@ -771,7 +735,6 @@ static void ti_close(struct usb_serial_port *port)
 
 	usb_kill_urb(port->read_urb);
 	usb_kill_urb(port->write_urb);
-	tport->tp_write_urb_in_use = 0;
 	spin_lock_irqsave(&tport->tp_lock, flags);
 	kfifo_reset_out(&port->write_fifo);
 	spin_unlock_irqrestore(&tport->tp_lock, flags);
@@ -797,56 +760,6 @@ static void ti_close(struct usb_serial_port *port)
 		mutex_unlock(&tdev->td_open_close_lock);
 }
 
-
-static int ti_write(struct tty_struct *tty, struct usb_serial_port *port,
-			const unsigned char *data, int count)
-{
-	struct ti_port *tport = usb_get_serial_port_data(port);
-
-	if (count == 0) {
-		return 0;
-	}
-
-	if (!tport->tp_is_open)
-		return -ENODEV;
-
-	count = kfifo_in_locked(&port->write_fifo, data, count,
-							&tport->tp_lock);
-	ti_send(tport);
-
-	return count;
-}
-
-
-static int ti_write_room(struct tty_struct *tty)
-{
-	struct usb_serial_port *port = tty->driver_data;
-	struct ti_port *tport = usb_get_serial_port_data(port);
-	int room = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&tport->tp_lock, flags);
-	room = kfifo_avail(&port->write_fifo);
-	spin_unlock_irqrestore(&tport->tp_lock, flags);
-
-	return room;
-}
-
-
-static int ti_chars_in_buffer(struct tty_struct *tty)
-{
-	struct usb_serial_port *port = tty->driver_data;
-	struct ti_port *tport = usb_get_serial_port_data(port);
-	int chars = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&tport->tp_lock, flags);
-	chars = kfifo_len(&port->write_fifo);
-	spin_unlock_irqrestore(&tport->tp_lock, flags);
-
-	return chars;
-}
-
 static bool ti_tx_empty(struct usb_serial_port *port)
 {
 	struct ti_port *tport = usb_get_serial_port_data(port);
@@ -860,31 +773,6 @@ static bool ti_tx_empty(struct usb_serial_port *port)
 	return true;
 }
 
-static void ti_throttle(struct tty_struct *tty)
-{
-	struct usb_serial_port *port = tty->driver_data;
-	struct ti_port *tport = usb_get_serial_port_data(port);
-
-	if (I_IXOFF(tty) || C_CRTSCTS(tty))
-		ti_stop_read(tport, tty);
-
-}
-
-
-static void ti_unthrottle(struct tty_struct *tty)
-{
-	struct usb_serial_port *port = tty->driver_data;
-	struct ti_port *tport = usb_get_serial_port_data(port);
-	int status;
-
-	if (I_IXOFF(tty) || C_CRTSCTS(tty)) {
-		status = ti_restart_read(tport, tty);
-		if (status)
-			dev_err(&port->dev, "%s - cannot restart read, %d\n",
-							__func__, status);
-	}
-}
-
 static int ti_ioctl(struct tty_struct *tty,
 	unsigned int cmd, unsigned long arg)
 {
@@ -978,8 +866,6 @@ static void ti_set_termios(struct tty_struct *tty,
 		if ((C_BAUD(tty)) != B0)
 			config->wFlags |= TI_UART_ENABLE_RTS_IN;
 		config->wFlags |= TI_UART_ENABLE_CTS_OUT;
-	} else {
-		ti_restart_read(tport, tty);
 	}
 
 	if (I_IXOFF(tty) || I_IXON(tty)) {
@@ -988,8 +874,6 @@ static void ti_set_termios(struct tty_struct *tty,
 
 		if (I_IXOFF(tty))
 			config->wFlags |= TI_UART_ENABLE_X_IN;
-		else
-			ti_restart_read(tport, tty);
 
 		if (I_IXON(tty))
 			config->wFlags |= TI_UART_ENABLE_X_OUT;
@@ -1193,168 +1077,6 @@ exit:
 			__func__, retval);
 }
 
-
-static void ti_bulk_in_callback(struct urb *urb)
-{
-	struct ti_port *tport = urb->context;
-	struct usb_serial_port *port = tport->tp_port;
-	struct device *dev = &urb->dev->dev;
-	int status = urb->status;
-	int retval = 0;
-
-	switch (status) {
-	case 0:
-		break;
-	case -ECONNRESET:
-	case -ENOENT:
-	case -ESHUTDOWN:
-		dev_dbg(dev, "%s - urb shutting down, %d\n", __func__, status);
-		tport->tp_tdev->td_urb_error = 1;
-		return;
-	default:
-		dev_err(dev, "%s - nonzero urb status, %d\n",
-			__func__, status);
-		tport->tp_tdev->td_urb_error = 1;
-	}
-
-	if (status == -EPIPE)
-		goto exit;
-
-	if (status) {
-		dev_err(dev, "%s - stopping read!\n", __func__);
-		return;
-	}
-
-	if (urb->actual_length) {
-		usb_serial_debug_data(dev, __func__, urb->actual_length,
-				      urb->transfer_buffer);
-
-		if (!tport->tp_is_open)
-			dev_dbg(dev, "%s - port closed, dropping data\n",
-				__func__);
-		else
-			ti_recv(port, urb->transfer_buffer, urb->actual_length);
-		spin_lock(&tport->tp_lock);
-		port->icount.rx += urb->actual_length;
-		spin_unlock(&tport->tp_lock);
-	}
-
-exit:
-	/* continue to read unless stopping */
-	spin_lock(&tport->tp_lock);
-	if (tport->tp_read_urb_state == TI_READ_URB_RUNNING)
-		retval = usb_submit_urb(urb, GFP_ATOMIC);
-	else if (tport->tp_read_urb_state == TI_READ_URB_STOPPING)
-		tport->tp_read_urb_state = TI_READ_URB_STOPPED;
-
-	spin_unlock(&tport->tp_lock);
-	if (retval)
-		dev_err(dev, "%s - resubmit read urb failed, %d\n",
-			__func__, retval);
-}
-
-
-static void ti_bulk_out_callback(struct urb *urb)
-{
-	struct ti_port *tport = urb->context;
-	struct usb_serial_port *port = tport->tp_port;
-	int status = urb->status;
-
-	tport->tp_write_urb_in_use = 0;
-
-	switch (status) {
-	case 0:
-		break;
-	case -ECONNRESET:
-	case -ENOENT:
-	case -ESHUTDOWN:
-		dev_dbg(&port->dev, "%s - urb shutting down, %d\n", __func__, status);
-		tport->tp_tdev->td_urb_error = 1;
-		return;
-	default:
-		dev_err_console(port, "%s - nonzero urb status, %d\n",
-			__func__, status);
-		tport->tp_tdev->td_urb_error = 1;
-	}
-
-	/* send any buffered data */
-	ti_send(tport);
-}
-
-
-static void ti_recv(struct usb_serial_port *port, unsigned char *data,
-		int length)
-{
-	int cnt;
-
-	do {
-		cnt = tty_insert_flip_string(&port->port, data, length);
-		if (cnt < length) {
-			dev_err(&port->dev, "%s - dropping data, %d bytes lost\n",
-						__func__, length - cnt);
-			if (cnt == 0)
-				break;
-		}
-		tty_flip_buffer_push(&port->port);
-		data += cnt;
-		length -= cnt;
-	} while (length > 0);
-}
-
-
-static void ti_send(struct ti_port *tport)
-{
-	int count, result;
-	struct usb_serial_port *port = tport->tp_port;
-	unsigned long flags;
-
-	spin_lock_irqsave(&tport->tp_lock, flags);
-
-	if (tport->tp_write_urb_in_use)
-		goto unlock;
-
-	count = kfifo_out(&port->write_fifo,
-				port->write_urb->transfer_buffer,
-				port->bulk_out_size);
-
-	if (count == 0)
-		goto unlock;
-
-	tport->tp_write_urb_in_use = 1;
-
-	spin_unlock_irqrestore(&tport->tp_lock, flags);
-
-	usb_serial_debug_data(&port->dev, __func__, count,
-			      port->write_urb->transfer_buffer);
-
-	usb_fill_bulk_urb(port->write_urb, port->serial->dev,
-			   usb_sndbulkpipe(port->serial->dev,
-					    port->bulk_out_endpointAddress),
-			   port->write_urb->transfer_buffer, count,
-			   ti_bulk_out_callback, tport);
-
-	result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
-	if (result) {
-		dev_err_console(port, "%s - submit write urb failed, %d\n",
-							__func__, result);
-		tport->tp_write_urb_in_use = 0;
-		/* TODO: reschedule ti_send */
-	} else {
-		spin_lock_irqsave(&tport->tp_lock, flags);
-		port->icount.tx += count;
-		spin_unlock_irqrestore(&tport->tp_lock, flags);
-	}
-
-	/* more room in the buffer for new writes, wakeup */
-	tty_port_tty_wakeup(&port->port);
-
-	return;
-unlock:
-	spin_unlock_irqrestore(&tport->tp_lock, flags);
-	return;
-}
-
-
 static int ti_set_mcr(struct ti_port *tport, unsigned int mcr)
 {
 	unsigned long flags;
@@ -1490,43 +1212,6 @@ static void ti_handle_new_msr(struct ti_port *tport, u8 msr)
 	tty_kref_put(tty);
 }
 
-
-static void ti_stop_read(struct ti_port *tport, struct tty_struct *tty)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&tport->tp_lock, flags);
-
-	if (tport->tp_read_urb_state == TI_READ_URB_RUNNING)
-		tport->tp_read_urb_state = TI_READ_URB_STOPPING;
-
-	spin_unlock_irqrestore(&tport->tp_lock, flags);
-}
-
-
-static int ti_restart_read(struct ti_port *tport, struct tty_struct *tty)
-{
-	struct urb *urb;
-	int status = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&tport->tp_lock, flags);
-
-	if (tport->tp_read_urb_state == TI_READ_URB_STOPPED) {
-		tport->tp_read_urb_state = TI_READ_URB_RUNNING;
-		urb = tport->tp_port->read_urb;
-		spin_unlock_irqrestore(&tport->tp_lock, flags);
-		urb->context = tport;
-		status = usb_submit_urb(urb, GFP_KERNEL);
-	} else  {
-		tport->tp_read_urb_state = TI_READ_URB_RUNNING;
-		spin_unlock_irqrestore(&tport->tp_lock, flags);
-	}
-
-	return status;
-}
-
-
 static int ti_command_out_sync(struct ti_device *tdev, __u8 command,
 	__u16 moduleid, __u16 value, __u8 *data, int size)
 {
-- 
2.8.2

Re: [PATCH 12/36] usb: serial: ti_usb_3410_5052: Use generic read/write callbacks

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:44AM +0200, Mathieu OTHACEHE wrote:
> Remove read_bulk_callback, write_bulk_callback, write, write_room,
> chars_in_buffer, throttle and unthrottle callbacks who uselessly
> reimplements generic functions.
> 
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 315 ----------------------------------
>  1 file changed, 315 deletions(-)
> 

> -static void ti_throttle(struct tty_struct *tty)
> -{
> -	struct usb_serial_port *port = tty->driver_data;
> -	struct ti_port *tport = usb_get_serial_port_data(port);
> -
> -	if (I_IXOFF(tty) || C_CRTSCTS(tty))
> -		ti_stop_read(tport, tty);
> -
>-}
> -
> -
> -static void ti_unthrottle(struct tty_struct *tty)
> -{
> -	struct usb_serial_port *port = tty->driver_data;
> -	struct ti_port *tport = usb_get_serial_port_data(port);
> -	int status;
> -
> -	if (I_IXOFF(tty) || C_CRTSCTS(tty)) {
> -		status = ti_restart_read(tport, tty);
> -		if (status)
> -			dev_err(&port->dev, "%s - cannot restart read, %d\n",
> -							__func__, status);
> -	}
> -}
> -
>  static int ti_ioctl(struct tty_struct *tty,
>  	unsigned int cmd, unsigned long arg)
>  {
> @@ -978,8 +866,6 @@ static void ti_set_termios(struct tty_struct *tty,
>  		if ((C_BAUD(tty)) != B0)
>  			config->wFlags |= TI_UART_ENABLE_RTS_IN;
>  		config->wFlags |= TI_UART_ENABLE_CTS_OUT;
> -	} else {
> -		ti_restart_read(tport, tty);
>  	}
>  
>  	if (I_IXOFF(tty) || I_IXON(tty)) {
> @@ -988,8 +874,6 @@ static void ti_set_termios(struct tty_struct *tty,
>  
>  		if (I_IXOFF(tty))
>  			config->wFlags |= TI_UART_ENABLE_X_IN;
> -		else
> -			ti_restart_read(tport, tty);
>  
>  		if (I_IXON(tty))
>  			config->wFlags |= TI_UART_ENABLE_X_OUT;
> @@ -1193,168 +1077,6 @@ exit:
>  			__func__, retval);
>  }

The interactions with software flow control here needs to be looked at
more closely, as the generic implementation ignores them.

>  
> -
> -static void ti_bulk_in_callback(struct urb *urb)
> -{
> -	struct ti_port *tport = urb->context;
> -	struct usb_serial_port *port = tport->tp_port;
> -	struct device *dev = &urb->dev->dev;
> -	int status = urb->status;
> -	int retval = 0;
> -
> -	switch (status) {
> -	case 0:
> -		break;
> -	case -ECONNRESET:
> -	case -ENOENT:
> -	case -ESHUTDOWN:
> -		dev_dbg(dev, "%s - urb shutting down, %d\n", __func__, status);
> -		tport->tp_tdev->td_urb_error = 1;
> -		return;
> -	default:
> -		dev_err(dev, "%s - nonzero urb status, %d\n",
> -			__func__, status);
> -		tport->tp_tdev->td_urb_error = 1;
> -	}
> -
> -	if (status == -EPIPE)
> -		goto exit;
> -
> -	if (status) {
> -		dev_err(dev, "%s - stopping read!\n", __func__);
> -		return;
> -	}
> -
> -	if (urb->actual_length) {
> -		usb_serial_debug_data(dev, __func__, urb->actual_length,
> -				      urb->transfer_buffer);
> -
> -		if (!tport->tp_is_open)
> -			dev_dbg(dev, "%s - port closed, dropping data\n",
> -				__func__);
> -		else
> -			ti_recv(port, urb->transfer_buffer, urb->actual_length);
> -		spin_lock(&tport->tp_lock);
> -		port->icount.rx += urb->actual_length;

icount.tx/rx is not updated by the generic implementations either (there
are a few reasons why this driver has not simply been converted to use
the generic implementation already).

A bit too much is going on here at once, and we risk introducing
regression such as the issues raised above.

Please at least try to do the conversion in two steps for the rx and tx
paths.

Thanks,
Johan

[PATCH 13/36] usb: serial: ti_usb_3410_5052: Remove unused variables

[Mathieu OTHACEHE]

Remove variables affected but never read.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 58df42d..00ae52e 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -265,18 +265,13 @@ struct ti_firmware_header {
 
 #define TI_DEFAULT_CLOSING_WAIT	4000		/* in .01 secs */
 
-/* supported setserial flags */
-#define TI_SET_SERIAL_FLAGS	0
-
 #define TI_EXTRA_VID_PID_COUNT	5
 
 struct ti_port {
-	int			tp_is_open;
 	u8			tp_msr;
 	u8			tp_shadow_mcr;
 	u8			tp_uart_mode;	/* 232 or 485 modes */
 	unsigned int		tp_uart_base_addr;
-	int			tp_flags;
 	struct ti_device	*tp_tdev;
 	struct usb_serial_port	*tp_port;
 	spinlock_t		tp_lock;
@@ -705,7 +700,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 		goto unlink_int_urb;
 	}
 
-	tport->tp_is_open = 1;
 	++tdev->td_open_port_count;
 
 	goto release_lock;
@@ -731,8 +725,6 @@ static void ti_close(struct usb_serial_port *port)
 	tdev = usb_get_serial_data(port->serial);
 	tport = usb_get_serial_port_data(port);
 
-	tport->tp_is_open = 0;
-
 	usb_kill_urb(port->read_urb);
 	usb_kill_urb(port->write_urb);
 	spin_lock_irqsave(&tport->tp_lock, flags);
@@ -1017,11 +1009,9 @@ static void ti_interrupt_callback(struct urb *urb)
 	case -ENOENT:
 	case -ESHUTDOWN:
 		dev_dbg(dev, "%s - urb shutting down, %d\n", __func__, status);
-		tdev->td_urb_error = 1;
 		return;
 	default:
 		dev_err(dev, "%s - nonzero urb status, %d\n", __func__, status);
-		tdev->td_urb_error = 1;
 		goto exit;
 	}
 
@@ -1146,7 +1136,6 @@ static int ti_get_serial_info(struct ti_port *tport,
 	ret_serial.type = PORT_16550A;
 	ret_serial.line = port->minor;
 	ret_serial.port = port->port_number;
-	ret_serial.flags = tport->tp_flags;
 	ret_serial.xmit_fifo_size = kfifo_size(&port->write_fifo);
 	ret_serial.baud_base = tport->tp_tdev->td_is_3410 ? 921600 : 460800;
 	ret_serial.closing_wait = cwait;
@@ -1171,7 +1160,6 @@ static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
 	if (cwait != ASYNC_CLOSING_WAIT_NONE)
 		cwait = msecs_to_jiffies(10 * new_serial.closing_wait);
 
-	tport->tp_flags = new_serial.flags & TI_SET_SERIAL_FLAGS;
 	tport->tp_port->port.closing_wait = cwait;
 
 	return 0;
-- 
2.8.2

[PATCH 14/36] usb: serial: ti_usb_3410_5052: Use macros instead of magic values

[Mathieu OTHACEHE]

Use macros to define 3410 and 5052 baud bases and remove useless casting.
Use macro to define usb download timeout.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 00ae52e..91ea1b5 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -259,10 +259,12 @@ struct ti_firmware_header {
 #define TI_DRIVER_AUTHOR	"Al Borchers <alborchers@steinerpoint.com>"
 #define TI_DRIVER_DESC		"TI USB 3410/5052 Serial Driver"
 
-#define TI_FIRMWARE_BUF_SIZE	16284
+#define TI_3410_BAUD_BASE       923077
+#define TI_5052_BAUD_BASE       461538
 
+#define TI_FIRMWARE_BUF_SIZE	16284
 #define TI_TRANSFER_TIMEOUT	2
-
+#define TI_DOWNLOAD_TIMEOUT	1000
 #define TI_DEFAULT_CLOSING_WAIT	4000		/* in .01 secs */
 
 #define TI_EXTRA_VID_PID_COUNT	5
@@ -875,9 +877,9 @@ static void ti_set_termios(struct tty_struct *tty,
 	if (!baud)
 		baud = 9600;
 	if (tport->tp_tdev->td_is_3410)
-		config->wBaudRate = (__u16)((923077 + baud/2) / baud);
+		config->wBaudRate = (TI_3410_BAUD_BASE + baud / 2) / baud;
 	else
-		config->wBaudRate = (__u16)((461538 + baud/2) / baud);
+		config->wBaudRate = (TI_5052_BAUD_BASE + baud / 2) / baud;
 
 	/* FIXME: Should calculate resulting baud here and report it back */
 	if ((C_BAUD(tty)) != B0)
@@ -1123,6 +1125,7 @@ static int ti_get_serial_info(struct ti_port *tport,
 	struct usb_serial_port *port = tport->tp_port;
 	struct serial_struct ret_serial;
 	unsigned cwait;
+	int baud_base;
 
 	if (!ret_arg)
 		return -EFAULT;
@@ -1133,11 +1136,16 @@ static int ti_get_serial_info(struct ti_port *tport,
 
 	memset(&ret_serial, 0, sizeof(ret_serial));
 
+	if (tport->tp_tdev->td_is_3410)
+		baud_base = TI_3410_BAUD_BASE;
+	else
+		baud_base = TI_5052_BAUD_BASE;
+
 	ret_serial.type = PORT_16550A;
 	ret_serial.line = port->minor;
 	ret_serial.port = port->port_number;
 	ret_serial.xmit_fifo_size = kfifo_size(&port->write_fifo);
-	ret_serial.baud_base = tport->tp_tdev->td_is_3410 ? 921600 : 460800;
+	ret_serial.baud_base = baud_base;
 	ret_serial.closing_wait = cwait;
 
 	if (copy_to_user(ret_arg, &ret_serial, sizeof(*ret_arg)))
@@ -1296,8 +1304,8 @@ static int ti_do_download(struct usb_device *dev, int pipe,
 	dev_dbg(&dev->dev, "%s - downloading firmware\n", __func__);
 	for (pos = 0; pos < size; pos += done) {
 		len = min(size - pos, TI_DOWNLOAD_MAX_PACKET_SIZE);
-		status = usb_bulk_msg(dev, pipe, buffer + pos, len,
-								&done, 1000);
+		status = usb_bulk_msg(dev, pipe, buffer + pos, len, &done,
+				      TI_DOWNLOAD_TIMEOUT);
 		if (status)
 			break;
 	}
-- 
2.8.2

[PATCH 15/36] usb: serial: ti_usb_3410_5052: Remove in_sync and out_sync functions

[Mathieu OTHACEHE]

ti_command_in_sync and ti_command_out_sync shouldn't use userspace
datatypes (__uX), data should be void* to avoid casting and size
should be size_t.

This patch rewrite those functions with new names: ti_send_ctrl_data_urb
and ti_recv_ctrl_urb. Also add a ti_send_ctrl_urb to simplify command
sending.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 156 +++++++++++++++++++---------------
 1 file changed, 88 insertions(+), 68 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 91ea1b5..eb4df1e 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -313,11 +313,6 @@ static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
 	struct serial_struct __user *new_arg);
 static void ti_handle_new_msr(struct ti_port *tport, u8 msr);
 
-static int ti_command_out_sync(struct ti_device *tdev, __u8 command,
-	__u16 moduleid, __u16 value, __u8 *data, int size);
-static int ti_command_in_sync(struct ti_device *tdev, __u8 command,
-	__u16 moduleid, __u16 value, __u8 *data, int size);
-
 static int ti_write_byte(struct usb_serial_port *port, struct ti_device *tdev,
 			 unsigned long addr, u8 mask, u8 byte);
 
@@ -462,6 +457,71 @@ MODULE_DEVICE_TABLE(usb, ti_id_table_combined);
 
 module_usb_serial_driver(serial_drivers, ti_id_table_combined);
 
+static int ti_send_ctrl_data_urb(struct usb_serial *serial, u8 request,
+				 u16 value, u16 index, void *data, size_t size)
+{
+	int status;
+
+	status = usb_control_msg(serial->dev,
+				 usb_sndctrlpipe(serial->dev, 0),
+				 request,
+				 (USB_TYPE_VENDOR | USB_RECIP_DEVICE |
+				  USB_DIR_OUT), value, index,
+				 data, size,
+				 USB_CTRL_SET_TIMEOUT);
+	if (status < 0) {
+		dev_err(&serial->interface->dev,
+			"%s - usb_control_msg failed: %d\n",
+			__func__, status);
+		return status;
+	}
+
+	if (status != size) {
+		dev_err(&serial->interface->dev,
+			"%s - short write (%d / %zd)\n",
+			__func__, status, size);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int ti_send_ctrl_urb(struct usb_serial *serial,
+			    u8 request, u16 value, u16 index)
+{
+	return ti_send_ctrl_data_urb(serial, request, value, index,
+				     NULL, 0);
+}
+
+static int ti_recv_ctrl_data_urb(struct usb_serial *serial, u8 request,
+				 u16 value, u16 index, void *data, size_t size)
+{
+	int status;
+
+	status = usb_control_msg(serial->dev,
+				 usb_rcvctrlpipe(serial->dev, 0),
+				 request,
+				 (USB_TYPE_VENDOR | USB_RECIP_DEVICE |
+				  USB_DIR_IN), value, index,
+				 data, size,
+				 USB_CTRL_SET_TIMEOUT);
+	if (status < 0) {
+		dev_err(&serial->interface->dev,
+			"%s - usb_control_msg failed: %d\n",
+			__func__, status);
+		return status;
+	}
+
+	if (status != size) {
+		dev_err(&serial->interface->dev,
+			"%s - short read (%d / %zd)\n",
+			__func__, status, size);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static int ti_startup(struct usb_serial *serial)
 {
 	struct ti_device *tdev;
@@ -589,6 +649,7 @@ static int ti_port_remove(struct usb_serial_port *port)
 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;
@@ -632,31 +693,32 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	if (tty)
 		ti_set_termios(tty, port, &tty->termios);
 
-	status = ti_command_out_sync(tdev, TI_OPEN_PORT,
-		(__u8)(TI_UART1_PORT + port_number), open_settings, NULL, 0);
+	status = ti_send_ctrl_urb(serial, TI_OPEN_PORT, open_settings,
+				  TI_UART1_PORT + port_number);
 	if (status) {
 		dev_err(&port->dev, "%s - cannot send open command, %d\n",
 			__func__, status);
 		goto unlink_int_urb;
 	}
 
-	status = ti_command_out_sync(tdev, TI_START_PORT,
-		(__u8)(TI_UART1_PORT + port_number), 0, NULL, 0);
+	status = ti_send_ctrl_urb(serial, TI_START_PORT, 0,
+				  TI_UART1_PORT + port_number);
 	if (status) {
 		dev_err(&port->dev, "%s - cannot send start command, %d\n",
 							__func__, status);
 		goto unlink_int_urb;
 	}
 
-	status = ti_command_out_sync(tdev, TI_PURGE_PORT,
-		(__u8)(TI_UART1_PORT + port_number), TI_PURGE_INPUT, NULL, 0);
+	status = ti_send_ctrl_urb(serial, TI_PURGE_PORT, TI_PURGE_INPUT,
+				  TI_UART1_PORT + port_number);
 	if (status) {
 		dev_err(&port->dev, "%s - cannot clear input buffers, %d\n",
 							__func__, status);
 		goto unlink_int_urb;
 	}
-	status = ti_command_out_sync(tdev, TI_PURGE_PORT,
-		(__u8)(TI_UART1_PORT + port_number), TI_PURGE_OUTPUT, NULL, 0);
+
+	status = ti_send_ctrl_urb(serial, TI_PURGE_PORT, TI_PURGE_OUTPUT,
+				  TI_UART1_PORT + port_number);
 	if (status) {
 		dev_err(&port->dev, "%s - cannot clear output buffers, %d\n",
 							__func__, status);
@@ -671,16 +733,16 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	if (tty)
 		ti_set_termios(tty, port, &tty->termios);
 
-	status = ti_command_out_sync(tdev, TI_OPEN_PORT,
-		(__u8)(TI_UART1_PORT + port_number), open_settings, NULL, 0);
+	status = ti_send_ctrl_urb(serial, TI_OPEN_PORT, open_settings,
+				  TI_UART1_PORT + port_number);
 	if (status) {
 		dev_err(&port->dev, "%s - cannot send open command (2), %d\n",
 							__func__, status);
 		goto unlink_int_urb;
 	}
 
-	status = ti_command_out_sync(tdev, TI_START_PORT,
-		(__u8)(TI_UART1_PORT + port_number), 0, NULL, 0);
+	status = ti_send_ctrl_urb(serial, TI_START_PORT, 0,
+				  TI_UART1_PORT + port_number);
 	if (status) {
 		dev_err(&port->dev, "%s - cannot send start command (2), %d\n",
 							__func__, status);
@@ -735,8 +797,8 @@ static void ti_close(struct usb_serial_port *port)
 
 	port_number = port->port_number;
 
-	status = ti_command_out_sync(tdev, TI_CLOSE_PORT,
-		     (__u8)(TI_UART1_PORT + port_number), 0, NULL, 0);
+	status = ti_send_ctrl_urb(port->serial, TI_CLOSE_PORT, 0,
+				  TI_UART1_PORT + port_number);
 	if (status)
 		dev_err(&port->dev,
 			"%s - cannot send close port command, %d\n"
@@ -894,9 +956,9 @@ static void ti_set_termios(struct tty_struct *tty,
 	cpu_to_be16s(&config->wBaudRate);
 	cpu_to_be16s(&config->wFlags);
 
-	status = ti_command_out_sync(tport->tp_tdev, TI_SET_CONFIG,
-		(__u8)(TI_UART1_PORT + port_number), 0, (__u8 *)config,
-		sizeof(*config));
+	status = ti_send_ctrl_data_urb(port->serial, TI_SET_CONFIG, 0,
+				       TI_UART1_PORT + port_number, config,
+				       sizeof(*config));
 	if (status)
 		dev_err(&port->dev, "%s - cannot set config on port %d, %d\n",
 					__func__, port_number, status);
@@ -1090,7 +1152,6 @@ static int ti_set_mcr(struct ti_port *tport, unsigned int mcr)
 static int ti_get_lsr(struct ti_port *tport, u8 *lsr)
 {
 	int size, status;
-	struct ti_device *tdev = tport->tp_tdev;
 	struct usb_serial_port *port = tport->tp_port;
 	int port_number = port->port_number;
 	struct ti_port_status *data;
@@ -1100,8 +1161,8 @@ static int ti_get_lsr(struct ti_port *tport, u8 *lsr)
 	if (!data)
 		return -ENOMEM;
 
-	status = ti_command_in_sync(tdev, TI_GET_PORT_STATUS,
-		(__u8)(TI_UART1_PORT+port_number), 0, (__u8 *)data, size);
+	status = ti_recv_ctrl_data_urb(port->serial, TI_GET_PORT_STATUS, 0,
+				       TI_UART1_PORT + port_number, data, size);
 	if (status) {
 		dev_err(&port->dev,
 			"%s - get port status command failed, %d\n",
@@ -1208,46 +1269,6 @@ static void ti_handle_new_msr(struct ti_port *tport, u8 msr)
 	tty_kref_put(tty);
 }
 
-static int ti_command_out_sync(struct ti_device *tdev, __u8 command,
-	__u16 moduleid, __u16 value, __u8 *data, int size)
-{
-	int status;
-
-	status = usb_control_msg(tdev->td_serial->dev,
-		usb_sndctrlpipe(tdev->td_serial->dev, 0), command,
-		(USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT),
-		value, moduleid, data, size, 1000);
-
-	if (status == size)
-		status = 0;
-
-	if (status > 0)
-		status = -ECOMM;
-
-	return status;
-}
-
-
-static int ti_command_in_sync(struct ti_device *tdev, __u8 command,
-	__u16 moduleid, __u16 value, __u8 *data, int size)
-{
-	int status;
-
-	status = usb_control_msg(tdev->td_serial->dev,
-		usb_rcvctrlpipe(tdev->td_serial->dev, 0), command,
-		(USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN),
-		value, moduleid, data, size, 1000);
-
-	if (status == size)
-		status = 0;
-
-	if (status > 0)
-		status = -ECOMM;
-
-	return status;
-}
-
-
 static int ti_write_byte(struct usb_serial_port *port,
 			 struct ti_device *tdev, unsigned long addr,
 			 u8 mask, u8 byte)
@@ -1272,9 +1293,8 @@ static int ti_write_byte(struct usb_serial_port *port,
 	data->bData[0] = mask;
 	data->bData[1] = byte;
 
-	status = ti_command_out_sync(tdev, TI_WRITE_DATA, TI_RAM_PORT, 0,
-		(__u8 *)data, size);
-
+	status = ti_send_ctrl_data_urb(port->serial, TI_WRITE_DATA, 0,
+				       TI_RAM_PORT, data, size);
 	if (status < 0)
 		dev_err(&port->dev, "%s - failed, %d\n", __func__, status);
 
-- 
2.8.2

[PATCH 16/36] usb: serial: ti_usb_3410_5052: Use bulk_out_size in TIOCGSERIAL

[Mathieu OTHACEHE]

Use bulk_out_size instead of recalculate it with kfifo_size

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index eb4df1e..5ef721c 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -1205,7 +1205,7 @@ static int ti_get_serial_info(struct ti_port *tport,
 	ret_serial.type = PORT_16550A;
 	ret_serial.line = port->minor;
 	ret_serial.port = port->port_number;
-	ret_serial.xmit_fifo_size = kfifo_size(&port->write_fifo);
+	ret_serial.xmit_fifo_size = port->bulk_out_size;
 	ret_serial.baud_base = baud_base;
 	ret_serial.closing_wait = cwait;
 
-- 
2.8.2

Re: [PATCH 16/36] usb: serial: ti_usb_3410_5052: Use bulk_out_size in TIOCGSERIAL

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:48AM +0200, Mathieu OTHACEHE wrote:
> Use bulk_out_size instead of recalculate it with kfifo_size
> 
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
> index eb4df1e..5ef721c 100644
> --- a/drivers/usb/serial/ti_usb_3410_5052.c
> +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> @@ -1205,7 +1205,7 @@ static int ti_get_serial_info(struct ti_port *tport,
>  	ret_serial.type = PORT_16550A;
>  	ret_serial.line = port->minor;
>  	ret_serial.port = port->port_number;
> -	ret_serial.xmit_fifo_size = kfifo_size(&port->write_fifo);
> +	ret_serial.xmit_fifo_size = port->bulk_out_size;

These two are in not equivalent. bulk_out_size holds the urb buffer size
(which coincides with the endpoint size by default, but need to do so),
while the fifo size is the size of the write fifo, which is typically
always larger.

This field is supposed to reflect the hardware fifo size, and we don't
really use it consistently for usb-serial drivers. I'd just leave it as
is for now.

Thanks,
Johan

[PATCH 17/36] usb: serial: ti_usb_3410_5052: Remove useless tty_wakeup

[Mathieu OTHACEHE]

The generic driver doesn't call tty_wakeup in
usb_serial_generic_msr_changed so this tty_wakeup seems useless.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 5ef721c..3f2372e 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -1238,7 +1238,6 @@ static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
 static void ti_handle_new_msr(struct ti_port *tport, u8 msr)
 {
 	struct async_icount *icount;
-	struct tty_struct *tty;
 	unsigned long flags;
 
 	dev_dbg(&tport->tp_port->dev, "%s - msr 0x%02X\n", __func__, msr);
@@ -1259,14 +1258,6 @@ static void ti_handle_new_msr(struct ti_port *tport, u8 msr)
 	}
 
 	tport->tp_msr = msr & TI_MSR_MASK;
-
-	/* handle CTS flow control */
-	tty = tty_port_tty_get(&tport->tp_port->port);
-	if (tty && C_CRTSCTS(tty)) {
-		if (msr & TI_MSR_CTS)
-			tty_wakeup(tty);
-	}
-	tty_kref_put(tty);
 }
 
 static int ti_write_byte(struct usb_serial_port *port,
-- 
2.8.2

Re: [PATCH 17/36] usb: serial: ti_usb_3410_5052: Remove useless tty_wakeup

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:49AM +0200, Mathieu OTHACEHE wrote:
> The generic driver doesn't call tty_wakeup in
> usb_serial_generic_msr_changed so this tty_wakeup seems useless.

While we strive to have the generic implementation as complete as
possible, it not doing something does not mean it must not be done. :)

> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
> index 5ef721c..3f2372e 100644
> --- a/drivers/usb/serial/ti_usb_3410_5052.c
> +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> @@ -1238,7 +1238,6 @@ static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
>  static void ti_handle_new_msr(struct ti_port *tport, u8 msr)
>  {
>  	struct async_icount *icount;
> -	struct tty_struct *tty;
>  	unsigned long flags;
>  
>  	dev_dbg(&tport->tp_port->dev, "%s - msr 0x%02X\n", __func__, msr);
> @@ -1259,14 +1258,6 @@ static void ti_handle_new_msr(struct ti_port *tport, u8 msr)
>  	}
>  
>  	tport->tp_msr = msr & TI_MSR_MASK;
> -
> -	/* handle CTS flow control */
> -	tty = tty_port_tty_get(&tport->tp_port->port);
> -	if (tty && C_CRTSCTS(tty)) {
> -		if (msr & TI_MSR_CTS)
> -			tty_wakeup(tty);
> -	}
> -	tty_kref_put(tty);

That said, this particular change does seem correct, as any writers
would have been woken up once any blocked (nacked) bulk-out transfers
complete.

Thanks,
Johan

[PATCH 18/36] usb: serial: ti_usb_3410_5052: Change ti_write_byte function arguments

[Mathieu OTHACEHE]

Remove useless ti_device pointer, and change addr to u32.
Move function upper to avoid function prototyping.

Also change size variable in function from int to size_t.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 76 ++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 41 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 3f2372e..9654fa3 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -312,10 +312,6 @@ static int ti_get_serial_info(struct ti_port *tport,
 static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
 	struct serial_struct __user *new_arg);
 static void ti_handle_new_msr(struct ti_port *tport, u8 msr);
-
-static int ti_write_byte(struct usb_serial_port *port, struct ti_device *tdev,
-			 unsigned long addr, u8 mask, u8 byte);
-
 static int ti_download_firmware(struct ti_device *tdev);
 
 static const struct usb_device_id ti_id_table_3410[] = {
@@ -522,6 +518,39 @@ static int ti_recv_ctrl_data_urb(struct usb_serial *serial, u8 request,
 	return 0;
 }
 
+static int ti_write_byte(struct usb_serial_port *port, u32 addr,
+			u8 mask, u8 byte)
+{
+	int status;
+	size_t size;
+	struct ti_write_data_bytes *data;
+
+	dev_dbg(&port->dev, "%s - addr 0x%08X, mask 0x%02X, byte 0x%02X\n",
+		__func__, addr, mask, byte);
+
+	size = sizeof(struct ti_write_data_bytes) + 2;
+	data = kmalloc(size, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->bAddrType = TI_RW_DATA_ADDR_XDATA;
+	data->bDataType = TI_RW_DATA_BYTE;
+	data->bDataCounter = 1;
+	data->wBaseAddrHi = cpu_to_be16(addr >> 16);
+	data->wBaseAddrLo = cpu_to_be16(addr);
+	data->bData[0] = mask;
+	data->bData[1] = byte;
+
+	status = ti_send_ctrl_data_urb(port->serial, TI_WRITE_DATA, 0,
+				       TI_RAM_PORT, data, size);
+	if (status < 0)
+		dev_err(&port->dev, "%s - failed, %d\n", __func__, status);
+
+	kfree(data);
+
+	return status;
+}
+
 static int ti_startup(struct usb_serial *serial)
 {
 	struct ti_device *tdev;
@@ -1042,10 +1071,9 @@ static void ti_break(struct tty_struct *tty, int break_state)
 	struct ti_port *tport = usb_get_serial_port_data(port);
 	int status;
 
-	status = ti_write_byte(port, tport->tp_tdev,
+	status = ti_write_byte(port,
 		tport->tp_uart_base_addr + TI_UART_OFFSET_LCR,
 		TI_LCR_BREAK, break_state == -1 ? TI_LCR_BREAK : 0);
-
 	if (status)
 		dev_dbg(&port->dev, "%s - error setting break, %d\n", __func__, status);
 }
@@ -1136,7 +1164,7 @@ static int ti_set_mcr(struct ti_port *tport, unsigned int mcr)
 	unsigned long flags;
 	int status;
 
-	status = ti_write_byte(tport->tp_port, tport->tp_tdev,
+	status = ti_write_byte(tport->tp_port,
 		tport->tp_uart_base_addr + TI_UART_OFFSET_MCR,
 		TI_MCR_RTS | TI_MCR_DTR | TI_MCR_LOOP, mcr);
 
@@ -1260,40 +1288,6 @@ static void ti_handle_new_msr(struct ti_port *tport, u8 msr)
 	tport->tp_msr = msr & TI_MSR_MASK;
 }
 
-static int ti_write_byte(struct usb_serial_port *port,
-			 struct ti_device *tdev, unsigned long addr,
-			 u8 mask, u8 byte)
-{
-	int status;
-	unsigned int size;
-	struct ti_write_data_bytes *data;
-
-	dev_dbg(&port->dev, "%s - addr 0x%08lX, mask 0x%02X, byte 0x%02X\n", __func__,
-		addr, mask, byte);
-
-	size = sizeof(struct ti_write_data_bytes) + 2;
-	data = kmalloc(size, GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->bAddrType = TI_RW_DATA_ADDR_XDATA;
-	data->bDataType = TI_RW_DATA_BYTE;
-	data->bDataCounter = 1;
-	data->wBaseAddrHi = cpu_to_be16(addr>>16);
-	data->wBaseAddrLo = cpu_to_be16(addr);
-	data->bData[0] = mask;
-	data->bData[1] = byte;
-
-	status = ti_send_ctrl_data_urb(port->serial, TI_WRITE_DATA, 0,
-				       TI_RAM_PORT, data, size);
-	if (status < 0)
-		dev_err(&port->dev, "%s - failed, %d\n", __func__, status);
-
-	kfree(data);
-
-	return status;
-}
-
 static int ti_do_download(struct usb_device *dev, int pipe,
 						u8 *buffer, int size)
 {
-- 
2.8.2

Re: [PATCH 18/36] usb: serial: ti_usb_3410_5052: Change ti_write_byte function arguments

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:50AM +0200, Mathieu OTHACEHE wrote:
> Remove useless ti_device pointer, and change addr to u32.
> Move function upper to avoid function prototyping.

That's just noise. Having an occasional prototype is just fine. You may
want to consider reordering functions for the first submission, but no
need to move things around after that.

> Also change size variable in function from int to size_t.

Combining changes with moving code, makes it harder to review for no
good reason.

Thanks,
Johan

[PATCH 19/36] usb: serial: ti_usb_3410_5052: Do not modify interrupt context

[Mathieu OTHACEHE]

It is useless to pass a specific context (ti_device) to the interrupt
callback.

So use the default context (usb_serial_port).

Remove useless variables in ti_interrupt_callback.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 55 +++++++++++++++--------------------
 1 file changed, 24 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 9654fa3..48f9253 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -311,7 +311,7 @@ static int ti_get_serial_info(struct ti_port *tport,
 	struct serial_struct __user *ret_arg);
 static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
 	struct serial_struct __user *new_arg);
-static void ti_handle_new_msr(struct ti_port *tport, u8 msr);
+static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr);
 static int ti_download_firmware(struct ti_device *tdev);
 
 static const struct usb_device_id ti_id_table_3410[] = {
@@ -711,7 +711,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 			status = -EINVAL;
 			goto release_lock;
 		}
-		urb->context = tdev;
 		status = usb_submit_urb(urb, GFP_KERNEL);
 		if (status) {
 			dev_err(&port->dev, "%s - submit interrupt urb failed, %d\n", __func__, status);
@@ -1081,17 +1080,12 @@ static void ti_break(struct tty_struct *tty, int break_state)
 
 static void ti_interrupt_callback(struct urb *urb)
 {
-	struct ti_device *tdev = urb->context;
-	struct usb_serial_port *port;
-	struct usb_serial *serial = tdev->td_serial;
-	struct ti_port *tport;
-	struct device *dev = &urb->dev->dev;
+	struct usb_serial_port *port = urb->context;
 	unsigned char *data = urb->transfer_buffer;
 	int length = urb->actual_length;
 	int port_number;
 	int function;
 	int status = urb->status;
-	int retval;
 	u8 msr;
 
 	switch (status) {
@@ -1100,63 +1094,61 @@ static void ti_interrupt_callback(struct urb *urb)
 	case -ECONNRESET:
 	case -ENOENT:
 	case -ESHUTDOWN:
-		dev_dbg(dev, "%s - urb shutting down, %d\n", __func__, status);
+		dev_dbg(&port->dev, "%s - urb shutting down, %d\n",
+			__func__, status);
 		return;
 	default:
-		dev_err(dev, "%s - nonzero urb status, %d\n", __func__, status);
+		dev_err(&port->dev, "%s - nonzero urb status, %d\n",
+			__func__, status);
 		goto exit;
 	}
 
 	if (length != 2) {
-		dev_dbg(dev, "%s - bad packet size, %d\n", __func__, length);
+		dev_dbg(&port->dev, "%s - bad packet size, %d\n",
+			__func__, length);
 		goto exit;
 	}
 
 	if (data[0] == TI_CODE_HARDWARE_ERROR) {
-		dev_err(dev, "%s - hardware error, %d\n", __func__, data[1]);
+		dev_err(&port->dev, "%s - hardware error, %d\n",
+			__func__, data[1]);
 		goto exit;
 	}
 
 	port_number = ti_get_port_from_code(data[0]);
 	function = ti_get_func_from_code(data[0]);
 
-	dev_dbg(dev, "%s - port_number %d, function %d, data 0x%02X\n",
+	dev_dbg(&port->dev, "%s - port_number %d, function %d, data 0x%02X\n",
 		__func__, port_number, function, data[1]);
 
-	if (port_number >= serial->num_ports) {
-		dev_err(dev, "%s - bad port number, %d\n",
+	if (port_number >= port->serial->num_ports) {
+		dev_err(&port->dev, "%s - bad port number, %d\n",
 						__func__, port_number);
 		goto exit;
 	}
 
-	port = serial->port[port_number];
-
-	tport = usb_get_serial_port_data(port);
-	if (!tport)
-		goto exit;
-
 	switch (function) {
 	case TI_CODE_DATA_ERROR:
-		dev_err(dev, "%s - DATA ERROR, port %d, data 0x%02X\n",
+		dev_err(&port->dev, "%s - DATA ERROR, port %d, data 0x%02X\n",
 			__func__, port_number, data[1]);
 		break;
 
 	case TI_CODE_MODEM_STATUS:
 		msr = data[1];
-		ti_handle_new_msr(tport, msr);
+		ti_handle_new_msr(port, msr);
 		break;
 
 	default:
-		dev_err(dev, "%s - unknown interrupt code, 0x%02X\n",
+		dev_err(&port->dev, "%s - unknown interrupt code, 0x%02X\n",
 							__func__, data[1]);
 		break;
 	}
 
 exit:
-	retval = usb_submit_urb(urb, GFP_ATOMIC);
-	if (retval)
-		dev_err(dev, "%s - resubmit interrupt urb failed, %d\n",
-			__func__, retval);
+	status = usb_submit_urb(urb, GFP_ATOMIC);
+	if (status)
+		dev_err(&port->dev, "%s - resubmit interrupt urb failed, %d\n",
+			__func__, status);
 }
 
 static int ti_set_mcr(struct ti_port *tport, unsigned int mcr)
@@ -1263,12 +1255,13 @@ static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
 }
 
 
-static void ti_handle_new_msr(struct ti_port *tport, u8 msr)
+static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr)
 {
+	struct ti_port *tport = usb_get_serial_port_data(port);
 	struct async_icount *icount;
 	unsigned long flags;
 
-	dev_dbg(&tport->tp_port->dev, "%s - msr 0x%02X\n", __func__, msr);
+	dev_dbg(&port->dev, "%s - msr 0x%02X\n", __func__, msr);
 
 	if (msr & TI_MSR_DELTA_MASK) {
 		spin_lock_irqsave(&tport->tp_lock, flags);
@@ -1281,7 +1274,7 @@ static void ti_handle_new_msr(struct ti_port *tport, u8 msr)
 			icount->dcd++;
 		if (msr & TI_MSR_DELTA_RI)
 			icount->rng++;
-		wake_up_interruptible(&tport->tp_port->port.delta_msr_wait);
+		wake_up_interruptible(&port->port.delta_msr_wait);
 		spin_unlock_irqrestore(&tport->tp_lock, flags);
 	}
 
-- 
2.8.2

[PATCH 20/36] usb: serial: ti_usb_3410_5052: Remove usb_serial pointer in ti_port

[Mathieu OTHACEHE]

There is no need to keep a pointer to usb_serial in ti_port
structure.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 48f9253..8350c6b 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -282,7 +282,6 @@ struct ti_port {
 struct ti_device {
 	struct mutex		td_open_close_lock;
 	int			td_open_port_count;
-	struct usb_serial	*td_serial;
 	int			td_is_3410;
 	bool			td_rs485_only;
 	int			td_urb_error;
@@ -312,7 +311,7 @@ static int ti_get_serial_info(struct ti_port *tport,
 static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
 	struct serial_struct __user *new_arg);
 static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr);
-static int ti_download_firmware(struct ti_device *tdev);
+static int ti_download_firmware(struct usb_serial *serial);
 
 static const struct usb_device_id ti_id_table_3410[] = {
 	{ USB_DEVICE(TI_VENDOR_ID, TI_3410_PRODUCT_ID) },
@@ -571,7 +570,6 @@ static int ti_startup(struct usb_serial *serial)
 		return -ENOMEM;
 
 	mutex_init(&tdev->td_open_close_lock);
-	tdev->td_serial = serial;
 	usb_set_serial_data(serial, tdev);
 
 	/* determine device type */
@@ -596,7 +594,7 @@ static int ti_startup(struct usb_serial *serial)
 
 	/* if we have only 1 configuration and 1 endpoint, download firmware */
 	if (dev->descriptor.bNumConfigurations == 1 && num_endpoints == 1) {
-		status = ti_download_firmware(tdev);
+		status = ti_download_firmware(serial);
 
 		if (status != 0)
 			goto free_tdev;
@@ -705,7 +703,7 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	/* start interrupt urb the first time a port is opened on this device */
 	if (tdev->td_open_port_count == 0) {
 		dev_dbg(&port->dev, "%s - start interrupt in urb\n", __func__);
-		urb = tdev->td_serial->port[0]->interrupt_in_urb;
+		urb = serial->port[0]->interrupt_in_urb;
 		if (!urb) {
 			dev_err(&port->dev, "%s - no interrupt urb\n", __func__);
 			status = -EINVAL;
@@ -798,7 +796,7 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 
 unlink_int_urb:
 	if (tdev->td_open_port_count == 0)
-		usb_kill_urb(port->serial->port[0]->interrupt_in_urb);
+		usb_kill_urb(serial->port[0]->interrupt_in_urb);
 release_lock:
 	mutex_unlock(&tdev->td_open_close_lock);
 	return status;
@@ -1310,17 +1308,19 @@ static int ti_do_download(struct usb_device *dev, int pipe,
 	return status;
 }
 
-static int ti_download_firmware(struct ti_device *tdev)
+static int ti_download_firmware(struct usb_serial *serial)
 {
 	int status;
 	int buffer_size;
 	u8 *buffer;
-	struct usb_device *dev = tdev->td_serial->dev;
-	unsigned int pipe = usb_sndbulkpipe(dev,
-		tdev->td_serial->port[0]->bulk_out_endpointAddress);
+	struct usb_device *dev = serial->dev;
+	struct ti_device *tdev = usb_get_serial_data(serial);
+	unsigned int pipe;
 	const struct firmware *fw_p;
 	char buf[32];
 
+	pipe = usb_sndbulkpipe(dev, serial->port[0]->bulk_out_endpointAddress);
+
 	if (le16_to_cpu(dev->descriptor.idVendor) == MXU1_VENDOR_ID) {
 		snprintf(buf,
 			sizeof(buf),
-- 
2.8.2

[PATCH 21/36] usb: serial: ti_usb_3410_5052: Use generic close function

[Mathieu OTHACEHE]

Use usb_serial_generic_close in close callback.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 8350c6b..3d36ae7 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -807,24 +807,16 @@ static void ti_close(struct usb_serial_port *port)
 {
 	struct ti_device *tdev;
 	struct ti_port *tport;
-	int port_number;
 	int status;
 	int do_unlock;
-	unsigned long flags;
 
 	tdev = usb_get_serial_data(port->serial);
 	tport = usb_get_serial_port_data(port);
 
-	usb_kill_urb(port->read_urb);
-	usb_kill_urb(port->write_urb);
-	spin_lock_irqsave(&tport->tp_lock, flags);
-	kfifo_reset_out(&port->write_fifo);
-	spin_unlock_irqrestore(&tport->tp_lock, flags);
-
-	port_number = port->port_number;
+	usb_serial_generic_close(port);
 
 	status = ti_send_ctrl_urb(port->serial, TI_CLOSE_PORT, 0,
-				  TI_UART1_PORT + port_number);
+				  TI_UART1_PORT + port->port_number);
 	if (status)
 		dev_err(&port->dev,
 			"%s - cannot send close port command, %d\n"
-- 
2.8.2

Re: [PATCH 21/36] usb: serial: ti_usb_3410_5052: Use generic close function

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:53AM +0200, Mathieu OTHACEHE wrote:
> Use usb_serial_generic_close in close callback.
>
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---
>  drivers/usb/serial/ti_usb_3410_5052.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
> index 8350c6b..3d36ae7 100644
> --- a/drivers/usb/serial/ti_usb_3410_5052.c
> +++ b/drivers/usb/serial/ti_usb_3410_5052.c
> @@ -807,24 +807,16 @@ static void ti_close(struct usb_serial_port *port)
>  {
>  	struct ti_device *tdev;
>  	struct ti_port *tport;
> -	int port_number;
>  	int status;
>  	int do_unlock;
> -	unsigned long flags;
>  
>  	tdev = usb_get_serial_data(port->serial);
>  	tport = usb_get_serial_port_data(port);
>  
> -	usb_kill_urb(port->read_urb);
> -	usb_kill_urb(port->write_urb);
> -	spin_lock_irqsave(&tport->tp_lock, flags);
> -	kfifo_reset_out(&port->write_fifo);
> -	spin_unlock_irqrestore(&tport->tp_lock, flags);
> -
> -	port_number = port->port_number;
> +	usb_serial_generic_close(port);

This change must go with the conversion to use the generic
implementations (e.g. in order to kill both read urbs at close, etc).

Thanks,
Johan

[PATCH 22/36] usb: serial: ti_usb_3410_5052: Change ti_get/set_serial_info function arguments

[Mathieu OTHACEHE]

It is sufficient to pass usb_serial_port structure to ti_get_serial_info
and ti_set_serial_info.

Also move functions above ioctl to avoid function prototyping
and use unsigned int instead of unsigned for cwait variable.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 122 ++++++++++++++++------------------
 1 file changed, 57 insertions(+), 65 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 3d36ae7..2a993a2 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -306,10 +306,6 @@ static void ti_interrupt_callback(struct urb *urb);
 
 static int ti_set_mcr(struct ti_port *tport, unsigned int mcr);
 static int ti_get_lsr(struct ti_port *tport, u8 *lsr);
-static int ti_get_serial_info(struct ti_port *tport,
-	struct serial_struct __user *ret_arg);
-static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
-	struct serial_struct __user *new_arg);
 static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr);
 static int ti_download_firmware(struct usb_serial *serial);
 
@@ -847,19 +843,71 @@ static bool ti_tx_empty(struct usb_serial_port *port)
 	return true;
 }
 
+static int ti_get_serial_info(struct usb_serial_port *port,
+			      struct serial_struct __user *ret_arg)
+{
+	struct ti_device *tdev = usb_get_serial_data(port->serial);
+	struct serial_struct ret_serial;
+	unsigned int cwait;
+	int baud_base;
+
+	if (!ret_arg)
+		return -EFAULT;
+
+	cwait = port->port.closing_wait;
+	if (cwait != ASYNC_CLOSING_WAIT_NONE)
+		cwait = jiffies_to_msecs(cwait) / 10;
+
+	memset(&ret_serial, 0, sizeof(ret_serial));
+
+	if (tdev->td_is_3410)
+		baud_base = TI_3410_BAUD_BASE;
+	else
+		baud_base = TI_5052_BAUD_BASE;
+
+	ret_serial.type = PORT_16550A;
+	ret_serial.line = port->minor;
+	ret_serial.port = port->port_number;
+	ret_serial.xmit_fifo_size = port->bulk_out_size;
+	ret_serial.baud_base = baud_base;
+	ret_serial.closing_wait = cwait;
+
+	if (copy_to_user(ret_arg, &ret_serial, sizeof(*ret_arg)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int ti_set_serial_info(struct usb_serial_port *port,
+			      struct serial_struct __user *new_arg)
+{
+	struct serial_struct new_serial;
+	unsigned int cwait;
+
+	if (copy_from_user(&new_serial, new_arg, sizeof(new_serial)))
+		return -EFAULT;
+
+	cwait = new_serial.closing_wait;
+	if (cwait != ASYNC_CLOSING_WAIT_NONE)
+		cwait = msecs_to_jiffies(10 * new_serial.closing_wait);
+
+	port->port.closing_wait = cwait;
+
+	return 0;
+}
+
 static int ti_ioctl(struct tty_struct *tty,
 	unsigned int cmd, unsigned long arg)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	struct ti_port *tport = usb_get_serial_port_data(port);
 
 	switch (cmd) {
 	case TIOCGSERIAL:
-		return ti_get_serial_info(tport,
-				(struct serial_struct __user *)arg);
+		return ti_get_serial_info(port,
+					  (struct serial_struct __user *)arg);
 	case TIOCSSERIAL:
-		return ti_set_serial_info(tty, tport,
-				(struct serial_struct __user *)arg);
+		return ti_set_serial_info(port,
+					  (struct serial_struct __user *)arg);
 	}
 	return -ENOIOCTLCMD;
 }
@@ -1189,62 +1237,6 @@ free_data:
 	return status;
 }
 
-
-static int ti_get_serial_info(struct ti_port *tport,
-	struct serial_struct __user *ret_arg)
-{
-	struct usb_serial_port *port = tport->tp_port;
-	struct serial_struct ret_serial;
-	unsigned cwait;
-	int baud_base;
-
-	if (!ret_arg)
-		return -EFAULT;
-
-	cwait = port->port.closing_wait;
-	if (cwait != ASYNC_CLOSING_WAIT_NONE)
-		cwait = jiffies_to_msecs(cwait) / 10;
-
-	memset(&ret_serial, 0, sizeof(ret_serial));
-
-	if (tport->tp_tdev->td_is_3410)
-		baud_base = TI_3410_BAUD_BASE;
-	else
-		baud_base = TI_5052_BAUD_BASE;
-
-	ret_serial.type = PORT_16550A;
-	ret_serial.line = port->minor;
-	ret_serial.port = port->port_number;
-	ret_serial.xmit_fifo_size = port->bulk_out_size;
-	ret_serial.baud_base = baud_base;
-	ret_serial.closing_wait = cwait;
-
-	if (copy_to_user(ret_arg, &ret_serial, sizeof(*ret_arg)))
-		return -EFAULT;
-
-	return 0;
-}
-
-
-static int ti_set_serial_info(struct tty_struct *tty, struct ti_port *tport,
-	struct serial_struct __user *new_arg)
-{
-	struct serial_struct new_serial;
-	unsigned cwait;
-
-	if (copy_from_user(&new_serial, new_arg, sizeof(new_serial)))
-		return -EFAULT;
-
-	cwait = new_serial.closing_wait;
-	if (cwait != ASYNC_CLOSING_WAIT_NONE)
-		cwait = msecs_to_jiffies(10 * new_serial.closing_wait);
-
-	tport->tp_port->port.closing_wait = cwait;
-
-	return 0;
-}
-
-
 static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr)
 {
 	struct ti_port *tport = usb_get_serial_port_data(port);
-- 
2.8.2

Re: [PATCH 22/36] usb: serial: ti_usb_3410_5052: Change ti_get/set_serial_info function arguments

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:54AM +0200, Mathieu OTHACEHE wrote:
> It is sufficient to pass usb_serial_port structure to ti_get_serial_info
> and ti_set_serial_info.
> 
> Also move functions above ioctl to avoid function prototyping
> and use unsigned int instead of unsigned for cwait variable.

Avoid changing things in the same patch as you move something, if moving
is at all needed.

Thanks,
Johan

[PATCH 23/36] usb: serial: ti_usb_3410_5052: Do not set shadow mcr in open callback

[Mathieu OTHACEHE]

Setting DTR/RTS is handled using dtr_rts in tty_core.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 2a993a2..d8bed30 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -694,7 +694,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	port_number = port->port_number;
 
 	tport->tp_msr = 0;
-	tport->tp_shadow_mcr |= (TI_MCR_RTS | TI_MCR_DTR);
 
 	/* start interrupt urb the first time a port is opened on this device */
 	if (tdev->td_open_port_count == 0) {
-- 
2.8.2

[PATCH 24/36] usb: serial: ti_usb_3410_5052: Use usb_serial_generic_open

[Mathieu OTHACEHE]

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@gmail.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;
-	}
 
 	++tdev->td_open_port_count;
 
-- 
2.8.2

Re: [PATCH 24/36] usb: serial: ti_usb_3410_5052: Use usb_serial_generic_open

[Johan Hovold]

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@gmail.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

[PATCH 25/36] usb: serial: ti_usb_3410_5052: Check old_termios parameter in set_termios

[Mathieu OTHACEHE]

The old_termios parameter is never used in set_termios callback.

Add a check to old_termios to see if we can return right away because
there is nothing to change.
Add a check to old_termios CBAUD to see if we can set DTR/RTS because
last speed was B0.

Also pass NULL for old_termios in open callback because it is the
initial call to set_termios.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 4769c80..b50f52f 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -710,7 +710,7 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	}
 
 	if (tty)
-		ti_set_termios(tty, port, &tty->termios);
+		ti_set_termios(tty, port, NULL);
 
 	status = ti_send_ctrl_urb(serial, TI_OPEN_PORT, open_settings,
 				  TI_UART1_PORT + port_number);
@@ -750,7 +750,7 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	usb_clear_halt(serial->dev, port->read_urb->pipe);
 
 	if (tty)
-		ti_set_termios(tty, port, &tty->termios);
+		ti_set_termios(tty, port, NULL);
 
 	status = ti_send_ctrl_urb(serial, TI_OPEN_PORT, open_settings,
 				  TI_UART1_PORT + port_number);
@@ -913,6 +913,13 @@ static void ti_set_termios(struct tty_struct *tty,
 	cflag = tty->termios.c_cflag;
 	iflag = tty->termios.c_iflag;
 
+	if (old_termios &&
+		!tty_termios_hw_change(&tty->termios, old_termios) &&
+		tty->termios.c_iflag == old_termios->c_iflag) {
+		dev_dbg(&port->dev, "%s - nothing to change\n", __func__);
+		return;
+	}
+
 	dev_dbg(&port->dev,
 		"%s - cflag 0x%08x, iflag 0x%08x\n", __func__, cflag, iflag);
 
@@ -1020,6 +1027,9 @@ static void ti_set_termios(struct tty_struct *tty,
 	/* if baud rate is B0, clear RTS and DTR */
 	if (C_BAUD(tty) == B0)
 		mcr &= ~(TI_MCR_DTR | TI_MCR_RTS);
+	else if (old_termios && (old_termios->c_cflag & CBAUD) == B0)
+		mcr |= TI_MCR_DTR | TI_MCR_RTS;
+
 	status = ti_set_mcr(tport, mcr);
 	if (status)
 		dev_err(&port->dev,
-- 
2.8.2

Re: [PATCH 25/36] usb: serial: ti_usb_3410_5052: Check old_termios parameter in set_termios

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:57AM +0200, Mathieu OTHACEHE wrote:
> The old_termios parameter is never used in set_termios callback.

There we go. :)

> Add a check to old_termios to see if we can return right away because
> there is nothing to change.
> Add a check to old_termios CBAUD to see if we can set DTR/RTS because
> last speed was B0.

Try splitting that up in two patches.

> Also pass NULL for old_termios in open callback because it is the
> initial call to set_termios.

That's what you need to do of course (not change the prototype as I said
earlier).

Thanks,
Johan

[PATCH 26/36] usb: serial: ti_usb_3410_5052: Fix firmware downloading

[Mathieu OTHACEHE]

The buffer used to store firmware is allocated to maximum firmware
size (TI_FIRMWARE_BUF_SIZE) + header size.
This buffer is filled with requested firmware (fw_p->size) and padded
with 0xff bytes. The header is written over the 3 first bytes of the
buffer (overwritting the 3 first bytes of the requested firmware)

Then, ti_do_download function is called to send only the first
fw_p->size bytes from the buffer to the device.
The rest of the buffer is never used.

So it is sufficient to allocate a buffer of fw_p->size bytes to store
only the requested firmware. There is no need to do any padding.

This patch also move firmware buffer manipulation to ti_do_download
function.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 62 +++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index b50f52f..85f66f0 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -1259,47 +1259,65 @@ static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr)
 	tport->tp_msr = msr & TI_MSR_MASK;
 }
 
-static int ti_do_download(struct usb_device *dev, int pipe,
-						u8 *buffer, int size)
+static int ti_do_download(struct usb_serial *serial,
+			  const struct firmware *fw_p)
 {
 	int pos;
 	u8 cs = 0;
 	int done;
+	struct usb_device *dev = serial->dev;
 	struct ti_firmware_header *header;
 	int status = 0;
+	u8 *buffer;
+	int buffer_size;
 	int len;
+	unsigned int pipe;
+
+	pipe = usb_sndbulkpipe(dev, serial->port[0]->bulk_out_endpointAddress);
+
+	buffer_size = fw_p->size;
+	buffer = kmalloc(buffer_size, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
 
-	for (pos = sizeof(struct ti_firmware_header); pos < size; pos++)
+	memcpy(buffer, fw_p->data, fw_p->size);
+
+	for (pos = sizeof(*header); pos < buffer_size; pos++)
 		cs = (u8)(cs + buffer[pos]);
 
 	header = (struct ti_firmware_header *)buffer;
-	header->wLength = cpu_to_le16((u16)(size
-					- sizeof(struct ti_firmware_header)));
+	header->wLength = cpu_to_le16(buffer_size - sizeof(*header));
 	header->bCheckSum = cs;
 
 	dev_dbg(&dev->dev, "%s - downloading firmware\n", __func__);
-	for (pos = 0; pos < size; pos += done) {
-		len = min(size - pos, TI_DOWNLOAD_MAX_PACKET_SIZE);
+	for (pos = 0; pos < buffer_size; pos += done) {
+		len = min(buffer_size - pos, TI_DOWNLOAD_MAX_PACKET_SIZE);
 		status = usb_bulk_msg(dev, pipe, buffer + pos, len, &done,
 				      TI_DOWNLOAD_TIMEOUT);
 		if (status)
 			break;
 	}
-	return status;
+
+	kfree(buffer);
+
+	if (status) {
+		dev_err(&dev->dev, "failed to download firmware: %d\n", status);
+		return status;
+	}
+
+	dev_dbg(&dev->dev, "%s - download successful\n", __func__);
+
+	return 0;
 }
 
 static int ti_download_firmware(struct usb_serial *serial)
 {
 	int status;
-	int buffer_size;
-	u8 *buffer;
 	struct usb_device *dev = serial->dev;
 	struct ti_device *tdev = usb_get_serial_data(serial);
-	unsigned int pipe;
 	const struct firmware *fw_p;
 	char buf[32];
 
-	pipe = usb_sndbulkpipe(dev, serial->port[0]->bulk_out_endpointAddress);
 
 	if (le16_to_cpu(dev->descriptor.idVendor) == MXU1_VENDOR_ID) {
 		snprintf(buf,
@@ -1354,30 +1372,16 @@ check_firmware:
 		dev_err(&dev->dev, "%s - firmware not found\n", __func__);
 		return -ENOENT;
 	}
+
 	if (fw_p->size > TI_FIRMWARE_BUF_SIZE) {
 		dev_err(&dev->dev, "%s - firmware too large %zu\n", __func__, fw_p->size);
 		release_firmware(fw_p);
 		return -ENOENT;
 	}
 
-	buffer_size = TI_FIRMWARE_BUF_SIZE + sizeof(struct ti_firmware_header);
-	buffer = kmalloc(buffer_size, GFP_KERNEL);
-	if (buffer) {
-		memcpy(buffer, fw_p->data, fw_p->size);
-		memset(buffer + fw_p->size, 0xff, buffer_size - fw_p->size);
-		status = ti_do_download(dev, pipe, buffer, fw_p->size);
-		kfree(buffer);
-	} else {
-		status = -ENOMEM;
-	}
-	release_firmware(fw_p);
-	if (status) {
-		dev_err(&dev->dev, "%s - error downloading firmware, %d\n",
-							__func__, status);
-		return status;
-	}
+	ti_do_download(serial, fw_p);
 
-	dev_dbg(&dev->dev, "%s - download successful\n", __func__);
+	release_firmware(fw_p);
 
 	return 0;
 }
-- 
2.8.2

[PATCH 27/36] usb: serial: ti_usb_3410_5052: Standardize debug and error messages

[Mathieu OTHACEHE]

Use the format "error text: error value\n" when possible.
Drop redundant function names from error messages.
Also move a couple err messages to dbg messages.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 83 +++++++++++++++++------------------
 1 file changed, 41 insertions(+), 42 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 85f66f0..1b62ffb 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -539,7 +539,7 @@ static int ti_write_byte(struct usb_serial_port *port, u32 addr,
 	status = ti_send_ctrl_data_urb(port->serial, TI_WRITE_DATA, 0,
 				       TI_RAM_PORT, data, size);
 	if (status < 0)
-		dev_err(&port->dev, "%s - failed, %d\n", __func__, status);
+		dev_err(&port->dev, "%s - failed: %d\n", __func__, status);
 
 	kfree(data);
 
@@ -571,7 +571,7 @@ static int ti_startup(struct usb_serial *serial)
 	/* determine device type */
 	if (serial->type == &ti_1port_device)
 		tdev->td_is_3410 = 1;
-	dev_dbg(&dev->dev, "%s - device type is %s\n", __func__,
+	dev_dbg(&dev->dev, "%s - device type is: %s\n", __func__,
 		tdev->td_is_3410 ? "3410" : "5052");
 
 	vid = le16_to_cpu(dev->descriptor.idVendor);
@@ -698,13 +698,14 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 		dev_dbg(&port->dev, "%s - start interrupt in urb\n", __func__);
 		urb = serial->port[0]->interrupt_in_urb;
 		if (!urb) {
-			dev_err(&port->dev, "%s - no interrupt urb\n", __func__);
+			dev_err(&port->dev, "no interrupt endpoint\n");
 			status = -EINVAL;
 			goto release_lock;
 		}
 		status = usb_submit_urb(urb, GFP_KERNEL);
 		if (status) {
-			dev_err(&port->dev, "%s - submit interrupt urb failed, %d\n", __func__, status);
+			dev_err(&port->dev, "failed to submit interrupt urb: %d\n",
+				status);
 			goto release_lock;
 		}
 	}
@@ -715,32 +716,29 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	status = ti_send_ctrl_urb(serial, TI_OPEN_PORT, open_settings,
 				  TI_UART1_PORT + port_number);
 	if (status) {
-		dev_err(&port->dev, "%s - cannot send open command, %d\n",
-			__func__, status);
+		dev_err(&port->dev, "cannot send open command: %d\n", status);
 		goto unlink_int_urb;
 	}
 
 	status = ti_send_ctrl_urb(serial, TI_START_PORT, 0,
 				  TI_UART1_PORT + port_number);
 	if (status) {
-		dev_err(&port->dev, "%s - cannot send start command, %d\n",
-							__func__, status);
+		dev_err(&port->dev, "cannot send start command: %d\n", status);
 		goto unlink_int_urb;
 	}
 
 	status = ti_send_ctrl_urb(serial, TI_PURGE_PORT, TI_PURGE_INPUT,
 				  TI_UART1_PORT + port_number);
 	if (status) {
-		dev_err(&port->dev, "%s - cannot clear input buffers, %d\n",
-							__func__, status);
+		dev_err(&port->dev, "cannot clear input buffers: %d\n", status);
 		goto unlink_int_urb;
 	}
 
 	status = ti_send_ctrl_urb(serial, TI_PURGE_PORT, TI_PURGE_OUTPUT,
 				  TI_UART1_PORT + port_number);
 	if (status) {
-		dev_err(&port->dev, "%s - cannot clear output buffers, %d\n",
-							__func__, status);
+		dev_err(&port->dev, "cannot clear output buffers: %d\n",
+			status);
 		goto unlink_int_urb;
 	}
 
@@ -755,16 +753,16 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	status = ti_send_ctrl_urb(serial, TI_OPEN_PORT, open_settings,
 				  TI_UART1_PORT + port_number);
 	if (status) {
-		dev_err(&port->dev, "%s - cannot send open command (2), %d\n",
-							__func__, status);
+		dev_err(&port->dev, "cannot send open command (2): %d\n",
+			status);
 		goto unlink_int_urb;
 	}
 
 	status = ti_send_ctrl_urb(serial, TI_START_PORT, 0,
 				  TI_UART1_PORT + port_number);
 	if (status) {
-		dev_err(&port->dev, "%s - cannot send start command (2), %d\n",
-							__func__, status);
+		dev_err(&port->dev, "cannot send start command (2): %d\n",
+			status);
 		goto unlink_int_urb;
 	}
 
@@ -799,10 +797,10 @@ static void ti_close(struct usb_serial_port *port)
 
 	status = ti_send_ctrl_urb(port->serial, TI_CLOSE_PORT, 0,
 				  TI_UART1_PORT + port->port_number);
-	if (status)
-		dev_err(&port->dev,
-			"%s - cannot send close port command, %d\n"
-							, __func__, status);
+	if (status) {
+		dev_err(&port->dev, "failed to send close port command: %d\n",
+			status);
+	}
 
 	/* if mutex_lock is interrupted, continue anyway */
 	do_unlock = !mutex_lock_interruptible(&tdev->td_open_close_lock);
@@ -1018,9 +1016,10 @@ static void ti_set_termios(struct tty_struct *tty,
 	status = ti_send_ctrl_data_urb(port->serial, TI_SET_CONFIG, 0,
 				       TI_UART1_PORT + port_number, config,
 				       sizeof(*config));
-	if (status)
-		dev_err(&port->dev, "%s - cannot set config on port %d, %d\n",
-					__func__, port_number, status);
+	if (status) {
+		dev_err(&port->dev, "cannot set config on port %d: %d\n",
+			port_number, status);
+	}
 
 	/* SET_CONFIG asserts RTS and DTR, reset them correctly */
 	mcr = tport->tp_shadow_mcr;
@@ -1031,10 +1030,11 @@ static void ti_set_termios(struct tty_struct *tty,
 		mcr |= TI_MCR_DTR | TI_MCR_RTS;
 
 	status = ti_set_mcr(tport, mcr);
-	if (status)
+	if (status) {
 		dev_err(&port->dev,
-			"%s - cannot set modem control on port %d, %d\n",
-						__func__, port_number, status);
+			"cannot set modem control on port %d: %d\n",
+			port_number, status);
+	}
 
 	kfree(config);
 }
@@ -1108,7 +1108,7 @@ static void ti_break(struct tty_struct *tty, int break_state)
 		tport->tp_uart_base_addr + TI_UART_OFFSET_LCR,
 		TI_LCR_BREAK, break_state == -1 ? TI_LCR_BREAK : 0);
 	if (status)
-		dev_dbg(&port->dev, "%s - error setting break, %d\n", __func__, status);
+		dev_dbg(&port->dev, "failed to set break: %d\n", status);
 }
 
 
@@ -1128,23 +1128,23 @@ static void ti_interrupt_callback(struct urb *urb)
 	case -ECONNRESET:
 	case -ENOENT:
 	case -ESHUTDOWN:
-		dev_dbg(&port->dev, "%s - urb shutting down, %d\n",
+		dev_dbg(&port->dev, "%s - urb shutting down: %d\n",
 			__func__, status);
 		return;
 	default:
-		dev_err(&port->dev, "%s - nonzero urb status, %d\n",
+		dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
 			__func__, status);
 		goto exit;
 	}
 
 	if (length != 2) {
-		dev_dbg(&port->dev, "%s - bad packet size, %d\n",
+		dev_dbg(&port->dev, "%s - bad packet size: %d\n",
 			__func__, length);
 		goto exit;
 	}
 
 	if (data[0] == TI_CODE_HARDWARE_ERROR) {
-		dev_err(&port->dev, "%s - hardware error, %d\n",
+		dev_err(&port->dev, "%s - hardware error: %d\n",
 			__func__, data[1]);
 		goto exit;
 	}
@@ -1156,14 +1156,13 @@ static void ti_interrupt_callback(struct urb *urb)
 		__func__, port_number, function, data[1]);
 
 	if (port_number >= port->serial->num_ports) {
-		dev_err(&port->dev, "%s - bad port number, %d\n",
-						__func__, port_number);
+		dev_err(&port->dev, "bad port number: %d\n", port_number);
 		goto exit;
 	}
 
 	switch (function) {
 	case TI_CODE_DATA_ERROR:
-		dev_err(&port->dev, "%s - DATA ERROR, port %d, data 0x%02X\n",
+		dev_dbg(&port->dev, "%s - DATA ERROR, port %d, data 0x%02X\n",
 			__func__, port_number, data[1]);
 		break;
 
@@ -1173,16 +1172,16 @@ static void ti_interrupt_callback(struct urb *urb)
 		break;
 
 	default:
-		dev_err(&port->dev, "%s - unknown interrupt code, 0x%02X\n",
-							__func__, data[1]);
+		dev_err(&port->dev, "unknown interrupt code: 0x%02X\n",
+			data[1]);
 		break;
 	}
 
 exit:
 	status = usb_submit_urb(urb, GFP_ATOMIC);
 	if (status)
-		dev_err(&port->dev, "%s - resubmit interrupt urb failed, %d\n",
-			__func__, status);
+		dev_err(&port->dev, "resubmit interrupt urb failed: %d\n",
+			status);
 }
 
 static int ti_set_mcr(struct ti_port *tport, unsigned int mcr)
@@ -1219,8 +1218,8 @@ static int ti_get_lsr(struct ti_port *tport, u8 *lsr)
 				       TI_UART1_PORT + port_number, data, size);
 	if (status) {
 		dev_err(&port->dev,
-			"%s - get port status command failed, %d\n",
-							__func__, status);
+			"get port status command failed: %d\n",
+			status);
 		goto free_data;
 	}
 
@@ -1369,12 +1368,12 @@ static int ti_download_firmware(struct usb_serial *serial)
 
 check_firmware:
 	if (status) {
-		dev_err(&dev->dev, "%s - firmware not found\n", __func__);
+		dev_err(&dev->dev, "failed to request firmware: %d\n", status);
 		return -ENOENT;
 	}
 
 	if (fw_p->size > TI_FIRMWARE_BUF_SIZE) {
-		dev_err(&dev->dev, "%s - firmware too large %zu\n", __func__, fw_p->size);
+		dev_err(&dev->dev, "firmware too large: %zu\n", fw_p->size);
 		release_firmware(fw_p);
 		return -ENOENT;
 	}
-- 
2.8.2

[PATCH 28/36] usb: serial: ti_usb_3410_5052: Use variables for vendor and product

[Mathieu OTHACEHE]

Use variables for vendor and product in download_firmware to improve
readability.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 1b62ffb..72cb675 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -1316,7 +1316,10 @@ static int ti_download_firmware(struct usb_serial *serial)
 	struct ti_device *tdev = usb_get_serial_data(serial);
 	const struct firmware *fw_p;
 	char buf[32];
+	__le16 vendor, product;
 
+	vendor = le16_to_cpu(dev->descriptor.idVendor);
+	product = le16_to_cpu(dev->descriptor.idProduct);
 
 	if (le16_to_cpu(dev->descriptor.idVendor) == MXU1_VENDOR_ID) {
 		snprintf(buf,
@@ -1329,15 +1332,13 @@ static int ti_download_firmware(struct usb_serial *serial)
 	}
 
 	/* try ID specific firmware first, then try generic firmware */
-	sprintf(buf, "ti_usb-v%04x-p%04x.fw",
-			le16_to_cpu(dev->descriptor.idVendor),
-			le16_to_cpu(dev->descriptor.idProduct));
+	sprintf(buf, "ti_usb-v%04x-p%04x.fw", vendor, product);
 	status = request_firmware(&fw_p, buf, &dev->dev);
 
 	if (status != 0) {
 		buf[0] = '\0';
-		if (le16_to_cpu(dev->descriptor.idVendor) == MTS_VENDOR_ID) {
-			switch (le16_to_cpu(dev->descriptor.idProduct)) {
+		if (vendor == MTS_VENDOR_ID) {
+			switch (product) {
 			case MTS_CDMA_PRODUCT_ID:
 				strcpy(buf, "mts_cdma.fw");
 				break;
@@ -1357,6 +1358,7 @@ static int ti_download_firmware(struct usb_serial *serial)
 				strcpy(buf, "mts_mt9234zba.fw");
 				break;			}
 		}
+
 		if (buf[0] == '\0') {
 			if (tdev->td_is_3410)
 				strcpy(buf, "ti_3410.fw");
-- 
2.8.2

[PATCH 29/36] usb: serial: ti_usb_3410_5052: Set shadow msr before waking up waiters

[Mathieu OTHACEHE]

Save msr before testing the delta and waking up any waiters.
Also use port directly instead of tport->tp_port.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 72cb675..1d8d8ef 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -1240,9 +1240,12 @@ static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr)
 
 	dev_dbg(&port->dev, "%s - msr 0x%02X\n", __func__, msr);
 
+	spin_lock_irqsave(&tport->tp_lock, flags);
+	tport->tp_msr = msr & TI_MSR_MASK;
+	spin_unlock_irqrestore(&tport->tp_lock, flags);
+
 	if (msr & TI_MSR_DELTA_MASK) {
-		spin_lock_irqsave(&tport->tp_lock, flags);
-		icount = &tport->tp_port->icount;
+		icount = &port->icount;
 		if (msr & TI_MSR_DELTA_CTS)
 			icount->cts++;
 		if (msr & TI_MSR_DELTA_DSR)
@@ -1251,11 +1254,9 @@ static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr)
 			icount->dcd++;
 		if (msr & TI_MSR_DELTA_RI)
 			icount->rng++;
+
 		wake_up_interruptible(&port->port.delta_msr_wait);
-		spin_unlock_irqrestore(&tport->tp_lock, flags);
 	}
-
-	tport->tp_msr = msr & TI_MSR_MASK;
 }
 
 static int ti_do_download(struct usb_serial *serial,
-- 
2.8.2

[PATCH 30/36] usb: serial: ti_usb_3410_5052: Remove backpointer in ti_port

[Mathieu OTHACEHE]

In ti_port structure, remove useless tp_tdev backpointer.
Also remove pointer to usb_serial_port.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 44 ++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 1d8d8ef..ae77084 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -274,8 +274,6 @@ struct ti_port {
 	u8			tp_shadow_mcr;
 	u8			tp_uart_mode;	/* 232 or 485 modes */
 	unsigned int		tp_uart_base_addr;
-	struct ti_device	*tp_tdev;
-	struct usb_serial_port	*tp_port;
 	spinlock_t		tp_lock;
 };
 
@@ -304,8 +302,8 @@ static int ti_tiocmset(struct tty_struct *tty,
 static void ti_break(struct tty_struct *tty, int break_state);
 static void ti_interrupt_callback(struct urb *urb);
 
-static int ti_set_mcr(struct ti_port *tport, unsigned int mcr);
-static int ti_get_lsr(struct ti_port *tport, u8 *lsr);
+static int ti_set_mcr(struct usb_serial_port *port, unsigned int mcr);
+static int ti_get_lsr(struct usb_serial_port *port, u8 *lsr);
 static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr);
 static int ti_download_firmware(struct usb_serial *serial);
 
@@ -631,6 +629,7 @@ static void ti_release(struct usb_serial *serial)
 static int ti_port_probe(struct usb_serial_port *port)
 {
 	struct ti_port *tport;
+	struct ti_device *tdev;
 
 	tport = kzalloc(sizeof(*tport), GFP_KERNEL);
 	if (!tport)
@@ -642,10 +641,9 @@ static int ti_port_probe(struct usb_serial_port *port)
 	else
 		tport->tp_uart_base_addr = TI_UART2_BASE_ADDR;
 
-	tport->tp_port = port;
-	tport->tp_tdev = usb_get_serial_data(port->serial);
+	tdev = usb_get_serial_data(port->serial);
 
-	if (tport->tp_tdev->td_rs485_only)
+	if (tdev->td_rs485_only)
 		tport->tp_uart_mode = TI_UART_485_RECEIVER_DISABLED;
 	else
 		tport->tp_uart_mode = TI_UART_232;
@@ -672,8 +670,8 @@ static int ti_port_remove(struct usb_serial_port *port)
 static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
 	struct ti_port *tport = usb_get_serial_port_data(port);
+	struct ti_device *tdev = usb_get_serial_data(port->serial);
 	struct usb_serial *serial = port->serial;
-	struct ti_device *tdev;
 	struct urb *urb;
 	int port_number;
 	int status;
@@ -683,8 +681,6 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 			 TI_PIPE_TIMEOUT_ENABLE |
 			 (TI_TRANSFER_TIMEOUT << 2));
 
-	tdev = tport->tp_tdev;
-
 	/* only one open on any port on a device at a time */
 	if (mutex_lock_interruptible(&tdev->td_open_close_lock))
 		return -ERESTARTSYS;
@@ -804,11 +800,11 @@ static void ti_close(struct usb_serial_port *port)
 
 	/* if mutex_lock is interrupted, continue anyway */
 	do_unlock = !mutex_lock_interruptible(&tdev->td_open_close_lock);
-	--tport->tp_tdev->td_open_port_count;
-	if (tport->tp_tdev->td_open_port_count <= 0) {
+	tdev->td_open_port_count--;
+	if (tdev->td_open_port_count <= 0) {
 		/* last port is closed, shut down interrupt urb */
 		usb_kill_urb(port->serial->port[0]->interrupt_in_urb);
-		tport->tp_tdev->td_open_port_count = 0;
+		tdev->td_open_port_count = 0;
 	}
 	if (do_unlock)
 		mutex_unlock(&tdev->td_open_close_lock);
@@ -816,11 +812,10 @@ static void ti_close(struct usb_serial_port *port)
 
 static bool ti_tx_empty(struct usb_serial_port *port)
 {
-	struct ti_port *tport = usb_get_serial_port_data(port);
 	int ret;
 	u8 lsr;
 
-	ret = ti_get_lsr(tport, &lsr);
+	ret = ti_get_lsr(port, &lsr);
 	if (!ret && !(lsr & TI_LSR_TX_EMPTY))
 		return false;
 
@@ -901,6 +896,7 @@ static void ti_set_termios(struct tty_struct *tty,
 		struct usb_serial_port *port, struct ktermios *old_termios)
 {
 	struct ti_port *tport = usb_get_serial_port_data(port);
+	struct ti_device *tdev = usb_get_serial_data(port->serial);
 	struct ti_uart_config *config;
 	tcflag_t cflag, iflag;
 	int baud;
@@ -995,7 +991,7 @@ static void ti_set_termios(struct tty_struct *tty,
 	baud = tty_get_baud_rate(tty);
 	if (!baud)
 		baud = 9600;
-	if (tport->tp_tdev->td_is_3410)
+	if (tdev->td_is_3410)
 		config->wBaudRate = (TI_3410_BAUD_BASE + baud / 2) / baud;
 	else
 		config->wBaudRate = (TI_5052_BAUD_BASE + baud / 2) / baud;
@@ -1029,7 +1025,7 @@ static void ti_set_termios(struct tty_struct *tty,
 	else if (old_termios && (old_termios->c_cflag & CBAUD) == B0)
 		mcr |= TI_MCR_DTR | TI_MCR_RTS;
 
-	status = ti_set_mcr(tport, mcr);
+	status = ti_set_mcr(port, mcr);
 	if (status) {
 		dev_err(&port->dev,
 			"cannot set modem control on port %d: %d\n",
@@ -1094,7 +1090,7 @@ static int ti_tiocmset(struct tty_struct *tty,
 		mcr &= ~TI_MCR_LOOP;
 	spin_unlock_irqrestore(&tport->tp_lock, flags);
 
-	return ti_set_mcr(tport, mcr);
+	return ti_set_mcr(port, mcr);
 }
 
 
@@ -1184,14 +1180,15 @@ exit:
 			status);
 }
 
-static int ti_set_mcr(struct ti_port *tport, unsigned int mcr)
+static int ti_set_mcr(struct usb_serial_port *port, unsigned int mcr)
 {
+	struct ti_port *tport = usb_get_serial_port_data(port);
 	unsigned long flags;
 	int status;
 
-	status = ti_write_byte(tport->tp_port,
-		tport->tp_uart_base_addr + TI_UART_OFFSET_MCR,
-		TI_MCR_RTS | TI_MCR_DTR | TI_MCR_LOOP, mcr);
+	status = ti_write_byte(port,
+			       tport->tp_uart_base_addr + TI_UART_OFFSET_MCR,
+			       TI_MCR_RTS | TI_MCR_DTR | TI_MCR_LOOP, mcr);
 
 	spin_lock_irqsave(&tport->tp_lock, flags);
 	if (!status)
@@ -1202,10 +1199,9 @@ static int ti_set_mcr(struct ti_port *tport, unsigned int mcr)
 }
 
 
-static int ti_get_lsr(struct ti_port *tport, u8 *lsr)
+static int ti_get_lsr(struct usb_serial_port *port, u8 *lsr)
 {
 	int size, status;
-	struct usb_serial_port *port = tport->tp_port;
 	int port_number = port->port_number;
 	struct ti_port_status *data;
 
-- 
2.8.2

[PATCH 31/36] usb: serial: ti_usb_3410_5052: Use a mutex to protect shadow mcr

[Mathieu OTHACEHE]

Only shadow msr is accessed from interrupt context. So use the ti_port
spinlock to protect only shadow msr.

Add a mutex in ti_port to protect mcr from concurrent access.
Also move shadow mcr setting out of ti_set_mcr function.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 36 +++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index ae77084..0192be9 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -274,7 +274,8 @@ struct ti_port {
 	u8			tp_shadow_mcr;
 	u8			tp_uart_mode;	/* 232 or 485 modes */
 	unsigned int		tp_uart_base_addr;
-	spinlock_t		tp_lock;
+	spinlock_t		tp_lock; /* Protects tp_msr */
+	struct mutex            tp_mutex; /* Protects tp_shadow_mcr */
 };
 
 struct ti_device {
@@ -636,6 +637,8 @@ static int ti_port_probe(struct usb_serial_port *port)
 		return -ENOMEM;
 
 	spin_lock_init(&tport->tp_lock);
+	mutex_init(&tport->tp_mutex);
+
 	if (port == port->serial->port[0])
 		tport->tp_uart_base_addr = TI_UART1_BASE_ADDR;
 	else
@@ -1017,9 +1020,9 @@ static void ti_set_termios(struct tty_struct *tty,
 			port_number, status);
 	}
 
-	/* SET_CONFIG asserts RTS and DTR, reset them correctly */
+	mutex_lock(&tport->tp_mutex);
 	mcr = tport->tp_shadow_mcr;
-	/* if baud rate is B0, clear RTS and DTR */
+
 	if (C_BAUD(tty) == B0)
 		mcr &= ~(TI_MCR_DTR | TI_MCR_RTS);
 	else if (old_termios && (old_termios->c_cflag & CBAUD) == B0)
@@ -1030,7 +1033,10 @@ static void ti_set_termios(struct tty_struct *tty,
 		dev_err(&port->dev,
 			"cannot set modem control on port %d: %d\n",
 			port_number, status);
+	} else {
+		tport->tp_shadow_mcr = mcr;
 	}
+	mutex_unlock(&tport->tp_mutex);
 
 	kfree(config);
 }
@@ -1045,10 +1051,14 @@ static int ti_tiocmget(struct tty_struct *tty)
 	unsigned int mcr;
 	unsigned long flags;
 
+	mutex_lock(&tport->tp_mutex);
 	spin_lock_irqsave(&tport->tp_lock, flags);
+
 	msr = tport->tp_msr;
 	mcr = tport->tp_shadow_mcr;
+
 	spin_unlock_irqrestore(&tport->tp_lock, flags);
+	mutex_unlock(&tport->tp_mutex);
 
 	result = ((mcr & TI_MCR_DTR) ? TIOCM_DTR : 0)
 		| ((mcr & TI_MCR_RTS) ? TIOCM_RTS : 0)
@@ -1069,10 +1079,10 @@ static int ti_tiocmset(struct tty_struct *tty,
 {
 	struct usb_serial_port *port = tty->driver_data;
 	struct ti_port *tport = usb_get_serial_port_data(port);
+	int err;
 	unsigned int mcr;
-	unsigned long flags;
 
-	spin_lock_irqsave(&tport->tp_lock, flags);
+	mutex_lock(&tport->tp_mutex);
 	mcr = tport->tp_shadow_mcr;
 
 	if (set & TIOCM_RTS)
@@ -1088,9 +1098,14 @@ static int ti_tiocmset(struct tty_struct *tty,
 		mcr &= ~TI_MCR_DTR;
 	if (clear & TIOCM_LOOP)
 		mcr &= ~TI_MCR_LOOP;
-	spin_unlock_irqrestore(&tport->tp_lock, flags);
 
-	return ti_set_mcr(port, mcr);
+	err = ti_set_mcr(port, mcr);
+	if (!err)
+		tport->tp_shadow_mcr = mcr;
+
+	mutex_unlock(&tport->tp_mutex);
+
+	return err;
 }
 
 
@@ -1183,18 +1198,11 @@ exit:
 static int ti_set_mcr(struct usb_serial_port *port, unsigned int mcr)
 {
 	struct ti_port *tport = usb_get_serial_port_data(port);
-	unsigned long flags;
 	int status;
 
 	status = ti_write_byte(port,
 			       tport->tp_uart_base_addr + TI_UART_OFFSET_MCR,
 			       TI_MCR_RTS | TI_MCR_DTR | TI_MCR_LOOP, mcr);
-
-	spin_lock_irqsave(&tport->tp_lock, flags);
-	if (!status)
-		tport->tp_shadow_mcr = mcr;
-	spin_unlock_irqrestore(&tport->tp_lock, flags);
-
 	return status;
 }
 
-- 
2.8.2

[PATCH 32/36] usb: serial: ti_usb_3410_5052: Remove prefixes from private structures

[Mathieu OTHACEHE]

Remove prefixes from ti_port and ti_device structures.
Also change type of is_3410 from int to bool.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 114 +++++++++++++++++-----------------
 1 file changed, 57 insertions(+), 57 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 0192be9..451dab4 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -270,20 +270,20 @@ struct ti_firmware_header {
 #define TI_EXTRA_VID_PID_COUNT	5
 
 struct ti_port {
-	u8			tp_msr;
-	u8			tp_shadow_mcr;
-	u8			tp_uart_mode;	/* 232 or 485 modes */
-	unsigned int		tp_uart_base_addr;
-	spinlock_t		tp_lock; /* Protects tp_msr */
-	struct mutex            tp_mutex; /* Protects tp_shadow_mcr */
+	u8 msr;
+	u8 mcr;
+	u8 uart_mode;	/* 232 or 485 modes */
+	unsigned int uart_base_addr;
+	spinlock_t spinlock; /* Protects msr */
+	struct mutex mutex; /* Protects mcr */
 };
 
 struct ti_device {
-	struct mutex		td_open_close_lock;
-	int			td_open_port_count;
-	int			td_is_3410;
-	bool			td_rs485_only;
-	int			td_urb_error;
+	struct mutex open_close_lock;
+	int open_port_count;
+	bool is_3410;
+	bool rs485_only;
+	int model;
 };
 
 static int ti_startup(struct usb_serial *serial);
@@ -564,14 +564,14 @@ static int ti_startup(struct usb_serial *serial)
 	if (!tdev)
 		return -ENOMEM;
 
-	mutex_init(&tdev->td_open_close_lock);
+	mutex_init(&tdev->open_close_lock);
 	usb_set_serial_data(serial, tdev);
 
 	/* determine device type */
 	if (serial->type == &ti_1port_device)
-		tdev->td_is_3410 = 1;
+		tdev->is_3410 = true;
 	dev_dbg(&dev->dev, "%s - device type is: %s\n", __func__,
-		tdev->td_is_3410 ? "3410" : "5052");
+		tdev->is_3410 ? "3410" : "5052");
 
 	vid = le16_to_cpu(dev->descriptor.idVendor);
 	pid = le16_to_cpu(dev->descriptor.idProduct);
@@ -579,7 +579,7 @@ static int ti_startup(struct usb_serial *serial)
 		switch (pid) {
 		case MXU1_1130_PRODUCT_ID:
 		case MXU1_1131_PRODUCT_ID:
-			tdev->td_rs485_only = true;
+			tdev->rs485_only = true;
 			break;
 		}
 	}
@@ -595,7 +595,7 @@ static int ti_startup(struct usb_serial *serial)
 			goto free_tdev;
 
 		/* 3410 must be reset, 5052 resets itself */
-		if (tdev->td_is_3410) {
+		if (tdev->is_3410) {
 			msleep_interruptible(100);
 			usb_reset_device(dev);
 		}
@@ -636,20 +636,20 @@ static int ti_port_probe(struct usb_serial_port *port)
 	if (!tport)
 		return -ENOMEM;
 
-	spin_lock_init(&tport->tp_lock);
-	mutex_init(&tport->tp_mutex);
+	spin_lock_init(&tport->spinlock);
+	mutex_init(&tport->mutex);
 
 	if (port == port->serial->port[0])
-		tport->tp_uart_base_addr = TI_UART1_BASE_ADDR;
+		tport->uart_base_addr = TI_UART1_BASE_ADDR;
 	else
-		tport->tp_uart_base_addr = TI_UART2_BASE_ADDR;
+		tport->uart_base_addr = TI_UART2_BASE_ADDR;
 
 	tdev = usb_get_serial_data(port->serial);
 
-	if (tdev->td_rs485_only)
-		tport->tp_uart_mode = TI_UART_485_RECEIVER_DISABLED;
+	if (tdev->rs485_only)
+		tport->uart_mode = TI_UART_485_RECEIVER_DISABLED;
 	else
-		tport->tp_uart_mode = TI_UART_232;
+		tport->uart_mode = TI_UART_232;
 
 	usb_set_serial_port_data(port, tport);
 
@@ -685,15 +685,15 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 			 (TI_TRANSFER_TIMEOUT << 2));
 
 	/* only one open on any port on a device at a time */
-	if (mutex_lock_interruptible(&tdev->td_open_close_lock))
+	if (mutex_lock_interruptible(&tdev->open_close_lock))
 		return -ERESTARTSYS;
 
 	port_number = port->port_number;
 
-	tport->tp_msr = 0;
+	tport->msr = 0;
 
 	/* start interrupt urb the first time a port is opened on this device */
-	if (tdev->td_open_port_count == 0) {
+	if (tdev->open_port_count == 0) {
 		dev_dbg(&port->dev, "%s - start interrupt in urb\n", __func__);
 		urb = serial->port[0]->interrupt_in_urb;
 		if (!urb) {
@@ -769,15 +769,15 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 	if (status)
 		goto unlink_int_urb;
 
-	++tdev->td_open_port_count;
+	++tdev->open_port_count;
 
 	goto release_lock;
 
 unlink_int_urb:
-	if (tdev->td_open_port_count == 0)
+	if (tdev->open_port_count == 0)
 		usb_kill_urb(serial->port[0]->interrupt_in_urb);
 release_lock:
-	mutex_unlock(&tdev->td_open_close_lock);
+	mutex_unlock(&tdev->open_close_lock);
 	return status;
 }
 
@@ -802,15 +802,15 @@ static void ti_close(struct usb_serial_port *port)
 	}
 
 	/* if mutex_lock is interrupted, continue anyway */
-	do_unlock = !mutex_lock_interruptible(&tdev->td_open_close_lock);
-	tdev->td_open_port_count--;
-	if (tdev->td_open_port_count <= 0) {
+	do_unlock = !mutex_lock_interruptible(&tdev->open_close_lock);
+	tdev->open_port_count--;
+	if (tdev->open_port_count <= 0) {
 		/* last port is closed, shut down interrupt urb */
 		usb_kill_urb(port->serial->port[0]->interrupt_in_urb);
-		tdev->td_open_port_count = 0;
+		tdev->open_port_count = 0;
 	}
 	if (do_unlock)
-		mutex_unlock(&tdev->td_open_close_lock);
+		mutex_unlock(&tdev->open_close_lock);
 }
 
 static bool ti_tx_empty(struct usb_serial_port *port)
@@ -842,7 +842,7 @@ static int ti_get_serial_info(struct usb_serial_port *port,
 
 	memset(&ret_serial, 0, sizeof(ret_serial));
 
-	if (tdev->td_is_3410)
+	if (tdev->is_3410)
 		baud_base = TI_3410_BAUD_BASE;
 	else
 		baud_base = TI_5052_BAUD_BASE;
@@ -934,7 +934,7 @@ static void ti_set_termios(struct tty_struct *tty,
 	/* these flags must be set */
 	config->wFlags |= TI_UART_ENABLE_MS_INTS;
 	config->wFlags |= TI_UART_ENABLE_AUTO_START_DMA;
-	config->bUartMode = tport->tp_uart_mode;
+	config->bUartMode = tport->uart_mode;
 
 	switch (C_CSIZE(tty)) {
 	case CS5:
@@ -994,7 +994,7 @@ static void ti_set_termios(struct tty_struct *tty,
 	baud = tty_get_baud_rate(tty);
 	if (!baud)
 		baud = 9600;
-	if (tdev->td_is_3410)
+	if (tdev->is_3410)
 		config->wBaudRate = (TI_3410_BAUD_BASE + baud / 2) / baud;
 	else
 		config->wBaudRate = (TI_5052_BAUD_BASE + baud / 2) / baud;
@@ -1020,8 +1020,8 @@ static void ti_set_termios(struct tty_struct *tty,
 			port_number, status);
 	}
 
-	mutex_lock(&tport->tp_mutex);
-	mcr = tport->tp_shadow_mcr;
+	mutex_lock(&tport->mutex);
+	mcr = tport->mcr;
 
 	if (C_BAUD(tty) == B0)
 		mcr &= ~(TI_MCR_DTR | TI_MCR_RTS);
@@ -1034,9 +1034,9 @@ static void ti_set_termios(struct tty_struct *tty,
 			"cannot set modem control on port %d: %d\n",
 			port_number, status);
 	} else {
-		tport->tp_shadow_mcr = mcr;
+		tport->mcr = mcr;
 	}
-	mutex_unlock(&tport->tp_mutex);
+	mutex_unlock(&tport->mutex);
 
 	kfree(config);
 }
@@ -1051,14 +1051,14 @@ static int ti_tiocmget(struct tty_struct *tty)
 	unsigned int mcr;
 	unsigned long flags;
 
-	mutex_lock(&tport->tp_mutex);
-	spin_lock_irqsave(&tport->tp_lock, flags);
+	mutex_lock(&tport->mutex);
+	spin_lock_irqsave(&tport->spinlock, flags);
 
-	msr = tport->tp_msr;
-	mcr = tport->tp_shadow_mcr;
+	msr = tport->msr;
+	mcr = tport->mcr;
 
-	spin_unlock_irqrestore(&tport->tp_lock, flags);
-	mutex_unlock(&tport->tp_mutex);
+	spin_unlock_irqrestore(&tport->spinlock, flags);
+	mutex_unlock(&tport->mutex);
 
 	result = ((mcr & TI_MCR_DTR) ? TIOCM_DTR : 0)
 		| ((mcr & TI_MCR_RTS) ? TIOCM_RTS : 0)
@@ -1082,8 +1082,8 @@ static int ti_tiocmset(struct tty_struct *tty,
 	int err;
 	unsigned int mcr;
 
-	mutex_lock(&tport->tp_mutex);
-	mcr = tport->tp_shadow_mcr;
+	mutex_lock(&tport->mutex);
+	mcr = tport->mcr;
 
 	if (set & TIOCM_RTS)
 		mcr |= TI_MCR_RTS;
@@ -1101,9 +1101,9 @@ static int ti_tiocmset(struct tty_struct *tty,
 
 	err = ti_set_mcr(port, mcr);
 	if (!err)
-		tport->tp_shadow_mcr = mcr;
+		tport->mcr = mcr;
 
-	mutex_unlock(&tport->tp_mutex);
+	mutex_unlock(&tport->mutex);
 
 	return err;
 }
@@ -1116,7 +1116,7 @@ static void ti_break(struct tty_struct *tty, int break_state)
 	int status;
 
 	status = ti_write_byte(port,
-		tport->tp_uart_base_addr + TI_UART_OFFSET_LCR,
+		tport->uart_base_addr + TI_UART_OFFSET_LCR,
 		TI_LCR_BREAK, break_state == -1 ? TI_LCR_BREAK : 0);
 	if (status)
 		dev_dbg(&port->dev, "failed to set break: %d\n", status);
@@ -1201,7 +1201,7 @@ static int ti_set_mcr(struct usb_serial_port *port, unsigned int mcr)
 	int status;
 
 	status = ti_write_byte(port,
-			       tport->tp_uart_base_addr + TI_UART_OFFSET_MCR,
+			       tport->uart_base_addr + TI_UART_OFFSET_MCR,
 			       TI_MCR_RTS | TI_MCR_DTR | TI_MCR_LOOP, mcr);
 	return status;
 }
@@ -1244,9 +1244,9 @@ static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr)
 
 	dev_dbg(&port->dev, "%s - msr 0x%02X\n", __func__, msr);
 
-	spin_lock_irqsave(&tport->tp_lock, flags);
-	tport->tp_msr = msr & TI_MSR_MASK;
-	spin_unlock_irqrestore(&tport->tp_lock, flags);
+	spin_lock_irqsave(&tport->spinlock, flags);
+	tport->msr = msr & TI_MSR_MASK;
+	spin_unlock_irqrestore(&tport->spinlock, flags);
 
 	if (msr & TI_MSR_DELTA_MASK) {
 		icount = &port->icount;
@@ -1365,7 +1365,7 @@ static int ti_download_firmware(struct usb_serial *serial)
 		}
 
 		if (buf[0] == '\0') {
-			if (tdev->td_is_3410)
+			if (tdev->is_3410)
 				strcpy(buf, "ti_3410.fw");
 			else
 				strcpy(buf, "ti_5052.fw");
-- 
2.8.2

[PATCH 33/36] usb: serial: ti_usb_3410_5052: Add CMSPAR support

[Mathieu OTHACEHE]

Add CMSPAR support in set_termios callback.
Move TI_UART_ENABLE_PARITY_CHECKING setting in the upper
block to avoid doing it twice.
Delete useless TI_UART_ENABLE_PARITY_CHECKING unsetting.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 451dab4..94e7edd 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -952,19 +952,20 @@ static void ti_set_termios(struct tty_struct *tty,
 		    break;
 	}
 
-	/* CMSPAR isn't supported by this driver */
-	tty->termios.c_cflag &= ~CMSPAR;
-
 	if (C_PARENB(tty)) {
-		if (C_PARODD(tty)) {
-			config->wFlags |= TI_UART_ENABLE_PARITY_CHECKING;
-			config->bParity = TI_UART_ODD_PARITY;
+		config->wFlags |= TI_UART_ENABLE_PARITY_CHECKING;
+		if (C_CMSPAR(tty)) {
+			if (C_PARODD(tty))
+				config->bParity = TI_UART_MARK_PARITY;
+			else
+				config->bParity = TI_UART_SPACE_PARITY;
 		} else {
-			config->wFlags |= TI_UART_ENABLE_PARITY_CHECKING;
-			config->bParity = TI_UART_EVEN_PARITY;
+			if (C_PARODD(tty))
+				config->bParity = TI_UART_ODD_PARITY;
+			else
+				config->bParity = TI_UART_EVEN_PARITY;
 		}
 	} else {
-		config->wFlags &= ~TI_UART_ENABLE_PARITY_CHECKING;
 		config->bParity = TI_UART_NO_PARITY;
 	}
 
-- 
2.8.2

[PATCH 34/36] usb: serial: ti_usb_3410_5052: Fix indentation problems

[Mathieu OTHACEHE]

Fix some minor indentation problems.
Also correct a multi-line comment.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 94e7edd..1e36c46 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -741,8 +741,10 @@ static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 		goto unlink_int_urb;
 	}
 
-	/* reset the data toggle on the bulk endpoints to work around bug in
-	 * host controllers where things get out of sync some times */
+	/*
+	 * 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(serial->dev, port->write_urb->pipe);
 	usb_clear_halt(serial->dev, port->read_urb->pipe);
 
@@ -879,7 +881,7 @@ static int ti_set_serial_info(struct usb_serial_port *port,
 }
 
 static int ti_ioctl(struct tty_struct *tty,
-	unsigned int cmd, unsigned long arg)
+		    unsigned int cmd, unsigned long arg)
 {
 	struct usb_serial_port *port = tty->driver_data;
 
@@ -896,7 +898,8 @@ static int ti_ioctl(struct tty_struct *tty,
 
 
 static void ti_set_termios(struct tty_struct *tty,
-		struct usb_serial_port *port, struct ktermios *old_termios)
+			   struct usb_serial_port *port,
+			   struct ktermios *old_termios)
 {
 	struct ti_port *tport = usb_get_serial_port_data(port);
 	struct ti_device *tdev = usb_get_serial_data(port->serial);
@@ -1061,13 +1064,13 @@ static int ti_tiocmget(struct tty_struct *tty)
 	spin_unlock_irqrestore(&tport->spinlock, flags);
 	mutex_unlock(&tport->mutex);
 
-	result = ((mcr & TI_MCR_DTR) ? TIOCM_DTR : 0)
-		| ((mcr & TI_MCR_RTS) ? TIOCM_RTS : 0)
-		| ((mcr & TI_MCR_LOOP) ? TIOCM_LOOP : 0)
-		| ((msr & TI_MSR_CTS) ? TIOCM_CTS : 0)
-		| ((msr & TI_MSR_CD) ? TIOCM_CAR : 0)
-		| ((msr & TI_MSR_RI) ? TIOCM_RI : 0)
-		| ((msr & TI_MSR_DSR) ? TIOCM_DSR : 0);
+	result = ((mcr & TI_MCR_DTR)	? TIOCM_DTR	: 0) |
+		 ((mcr & TI_MCR_RTS)	? TIOCM_RTS	: 0) |
+		 ((mcr & TI_MCR_LOOP)   ? TIOCM_LOOP	: 0) |
+		 ((msr & TI_MSR_CTS)	? TIOCM_CTS	: 0) |
+		 ((msr & TI_MSR_CD)	? TIOCM_CAR	: 0) |
+		 ((msr & TI_MSR_RI)	? TIOCM_RI	: 0) |
+		 ((msr & TI_MSR_DSR)	? TIOCM_DSR	: 0);
 
 	dev_dbg(&port->dev, "%s - 0x%04X\n", __func__, result);
 
@@ -1076,7 +1079,7 @@ static int ti_tiocmget(struct tty_struct *tty)
 
 
 static int ti_tiocmset(struct tty_struct *tty,
-				unsigned int set, unsigned int clear)
+		       unsigned int set, unsigned int clear)
 {
 	struct usb_serial_port *port = tty->driver_data;
 	struct ti_port *tport = usb_get_serial_port_data(port);
-- 
2.8.2

[PATCH 35/36] usb: serial: ti_usb_3410_5052: Remove function prototypes

[Mathieu OTHACEHE]

Declare functions in a the right order to avoid prototyping.
There is no functional change here.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 1112 ++++++++++++++++-----------------
 1 file changed, 544 insertions(+), 568 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 1e36c46..95bdcca 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -286,28 +286,6 @@ struct ti_device {
 	int model;
 };
 
-static int ti_startup(struct usb_serial *serial);
-static void ti_release(struct usb_serial *serial);
-static int ti_port_probe(struct usb_serial_port *port);
-static int ti_port_remove(struct usb_serial_port *port);
-static int ti_open(struct tty_struct *tty, struct usb_serial_port *port);
-static void ti_close(struct usb_serial_port *port);
-static bool ti_tx_empty(struct usb_serial_port *port);
-static int ti_ioctl(struct tty_struct *tty,
-		unsigned int cmd, unsigned long arg);
-static void ti_set_termios(struct tty_struct *tty,
-		struct usb_serial_port *port, struct ktermios *old_termios);
-static int ti_tiocmget(struct tty_struct *tty);
-static int ti_tiocmset(struct tty_struct *tty,
-		unsigned int set, unsigned int clear);
-static void ti_break(struct tty_struct *tty, int break_state);
-static void ti_interrupt_callback(struct urb *urb);
-
-static int ti_set_mcr(struct usb_serial_port *port, unsigned int mcr);
-static int ti_get_lsr(struct usb_serial_port *port, u8 *lsr);
-static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr);
-static int ti_download_firmware(struct usb_serial *serial);
-
 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) },
@@ -372,80 +350,7 @@ static const struct usb_device_id ti_id_table_combined[] = {
 	{ }	/* terminator */
 };
 
-static struct usb_serial_driver ti_1port_device = {
-	.driver = {
-		.owner		= THIS_MODULE,
-		.name		= "ti_usb_3410_5052_1",
-	},
-	.description		= "TI USB 3410 1 port adapter",
-	.id_table		= ti_id_table_3410,
-	.num_ports		= 1,
-	.attach			= ti_startup,
-	.release		= ti_release,
-	.port_probe		= ti_port_probe,
-	.port_remove		= ti_port_remove,
-	.open			= ti_open,
-	.close			= ti_close,
-	.tx_empty		= ti_tx_empty,
-	.ioctl			= ti_ioctl,
-	.set_termios		= ti_set_termios,
-	.tiocmget		= ti_tiocmget,
-	.tiocmset		= ti_tiocmset,
-	.tiocmiwait		= usb_serial_generic_tiocmiwait,
-	.get_icount		= usb_serial_generic_get_icount,
-	.break_ctl		= ti_break,
-	.read_int_callback	= ti_interrupt_callback,
-};
-
-static struct usb_serial_driver ti_2port_device = {
-	.driver = {
-		.owner		= THIS_MODULE,
-		.name		= "ti_usb_3410_5052_2",
-	},
-	.description		= "TI USB 5052 2 port adapter",
-	.id_table		= ti_id_table_5052,
-	.num_ports		= 2,
-	.attach			= ti_startup,
-	.release		= ti_release,
-	.port_probe		= ti_port_probe,
-	.port_remove		= ti_port_remove,
-	.open			= ti_open,
-	.close			= ti_close,
-	.tx_empty		= ti_tx_empty,
-	.ioctl			= ti_ioctl,
-	.set_termios		= ti_set_termios,
-	.tiocmget		= ti_tiocmget,
-	.tiocmset		= ti_tiocmset,
-	.tiocmiwait		= usb_serial_generic_tiocmiwait,
-	.get_icount		= usb_serial_generic_get_icount,
-	.break_ctl		= ti_break,
-	.read_int_callback	= ti_interrupt_callback,
-};
-
-static struct usb_serial_driver * const serial_drivers[] = {
-	&ti_1port_device, &ti_2port_device, NULL
-};
-
-MODULE_AUTHOR(TI_DRIVER_AUTHOR);
-MODULE_DESCRIPTION(TI_DRIVER_DESC);
-MODULE_LICENSE("GPL");
-
-MODULE_FIRMWARE("ti_3410.fw");
-MODULE_FIRMWARE("ti_5052.fw");
-MODULE_FIRMWARE("mts_cdma.fw");
-MODULE_FIRMWARE("mts_gsm.fw");
-MODULE_FIRMWARE("mts_edge.fw");
-MODULE_FIRMWARE("mts_mt9234mu.fw");
-MODULE_FIRMWARE("mts_mt9234zba.fw");
-MODULE_FIRMWARE("moxa/moxa-1110.fw");
-MODULE_FIRMWARE("moxa/moxa-1130.fw");
-MODULE_FIRMWARE("moxa/moxa-1131.fw");
-MODULE_FIRMWARE("moxa/moxa-1150.fw");
-MODULE_FIRMWARE("moxa/moxa-1151.fw");
-
-MODULE_DEVICE_TABLE(usb, ti_id_table_combined);
-
-module_usb_serial_driver(serial_drivers, ti_id_table_combined);
+static struct usb_serial_driver ti_1port_device;
 
 static int ti_send_ctrl_data_urb(struct usb_serial *serial, u8 request,
 				 u16 value, u16 index, void *data, size_t size)
@@ -512,120 +417,134 @@ static int ti_recv_ctrl_data_urb(struct usb_serial *serial, u8 request,
 	return 0;
 }
 
-static int ti_write_byte(struct usb_serial_port *port, u32 addr,
-			u8 mask, u8 byte)
+static int ti_do_download(struct usb_serial *serial,
+			  const struct firmware *fw_p)
 {
-	int status;
-	size_t size;
-	struct ti_write_data_bytes *data;
+	int pos;
+	u8 cs = 0;
+	int done;
+	struct usb_device *dev = serial->dev;
+	struct ti_firmware_header *header;
+	int status = 0;
+	u8 *buffer;
+	int buffer_size;
+	int len;
+	unsigned int pipe;
 
-	dev_dbg(&port->dev, "%s - addr 0x%08X, mask 0x%02X, byte 0x%02X\n",
-		__func__, addr, mask, byte);
+	pipe = usb_sndbulkpipe(dev, serial->port[0]->bulk_out_endpointAddress);
 
-	size = sizeof(struct ti_write_data_bytes) + 2;
-	data = kmalloc(size, GFP_KERNEL);
-	if (!data)
+	buffer_size = fw_p->size;
+	buffer = kmalloc(buffer_size, GFP_KERNEL);
+	if (!buffer)
 		return -ENOMEM;
 
-	data->bAddrType = TI_RW_DATA_ADDR_XDATA;
-	data->bDataType = TI_RW_DATA_BYTE;
-	data->bDataCounter = 1;
-	data->wBaseAddrHi = cpu_to_be16(addr >> 16);
-	data->wBaseAddrLo = cpu_to_be16(addr);
-	data->bData[0] = mask;
-	data->bData[1] = byte;
+	memcpy(buffer, fw_p->data, fw_p->size);
 
-	status = ti_send_ctrl_data_urb(port->serial, TI_WRITE_DATA, 0,
-				       TI_RAM_PORT, data, size);
-	if (status < 0)
-		dev_err(&port->dev, "%s - failed: %d\n", __func__, status);
+	for (pos = sizeof(*header); pos < buffer_size; pos++)
+		cs = (u8)(cs + buffer[pos]);
 
-	kfree(data);
+	header = (struct ti_firmware_header *)buffer;
+	header->wLength = cpu_to_le16(buffer_size - sizeof(*header));
+	header->bCheckSum = cs;
 
-	return status;
+	dev_dbg(&dev->dev, "%s - downloading firmware\n", __func__);
+	for (pos = 0; pos < buffer_size; pos += done) {
+		len = min(buffer_size - pos, TI_DOWNLOAD_MAX_PACKET_SIZE);
+		status = usb_bulk_msg(dev, pipe, buffer + pos, len, &done,
+				      TI_DOWNLOAD_TIMEOUT);
+		if (status)
+			break;
+	}
+
+	kfree(buffer);
+
+	if (status) {
+		dev_err(&dev->dev, "failed to download firmware: %d\n", status);
+		return status;
+	}
+
+	dev_dbg(&dev->dev, "%s - download successful\n", __func__);
+
+	return 0;
 }
 
-static int ti_startup(struct usb_serial *serial)
+static int ti_download_firmware(struct usb_serial *serial)
 {
-	struct ti_device *tdev;
-	struct usb_device *dev = serial->dev;
-	struct usb_host_interface *cur_altsetting;
-	int num_endpoints;
-	u16 vid, pid;
 	int status;
+	struct usb_device *dev = serial->dev;
+	struct ti_device *tdev = usb_get_serial_data(serial);
+	const struct firmware *fw_p;
+	char buf[32];
+	__le16 vendor, product;
 
-	dev_dbg(&dev->dev,
-		"%s - product 0x%4X, num configurations %d, configuration value %d\n",
-		__func__, le16_to_cpu(dev->descriptor.idProduct),
-		dev->descriptor.bNumConfigurations,
-		dev->actconfig->desc.bConfigurationValue);
-
-	tdev = kzalloc(sizeof(struct ti_device), GFP_KERNEL);
-	if (!tdev)
-		return -ENOMEM;
-
-	mutex_init(&tdev->open_close_lock);
-	usb_set_serial_data(serial, tdev);
+	vendor = le16_to_cpu(dev->descriptor.idVendor);
+	product = le16_to_cpu(dev->descriptor.idProduct);
 
-	/* determine device type */
-	if (serial->type == &ti_1port_device)
-		tdev->is_3410 = true;
-	dev_dbg(&dev->dev, "%s - device type is: %s\n", __func__,
-		tdev->is_3410 ? "3410" : "5052");
+	if (le16_to_cpu(dev->descriptor.idVendor) == MXU1_VENDOR_ID) {
+		snprintf(buf,
+			sizeof(buf),
+			"moxa/moxa-%04x.fw",
+			le16_to_cpu(dev->descriptor.idProduct));
 
-	vid = le16_to_cpu(dev->descriptor.idVendor);
-	pid = le16_to_cpu(dev->descriptor.idProduct);
-	if (vid == MXU1_VENDOR_ID) {
-		switch (pid) {
-		case MXU1_1130_PRODUCT_ID:
-		case MXU1_1131_PRODUCT_ID:
-			tdev->rs485_only = true;
-			break;
-		}
+		status = request_firmware(&fw_p, buf, &dev->dev);
+		goto check_firmware;
 	}
 
-	cur_altsetting = serial->interface->cur_altsetting;
-	num_endpoints = cur_altsetting->desc.bNumEndpoints;
-
-	/* if we have only 1 configuration and 1 endpoint, download firmware */
-	if (dev->descriptor.bNumConfigurations == 1 && num_endpoints == 1) {
-		status = ti_download_firmware(serial);
+	/* try ID specific firmware first, then try generic firmware */
+	sprintf(buf, "ti_usb-v%04x-p%04x.fw", vendor, product);
+	status = request_firmware(&fw_p, buf, &dev->dev);
 
-		if (status != 0)
-			goto free_tdev;
+	if (status != 0) {
+		buf[0] = '\0';
+		if (vendor == MTS_VENDOR_ID) {
+			switch (product) {
+			case MTS_CDMA_PRODUCT_ID:
+				strcpy(buf, "mts_cdma.fw");
+				break;
+			case MTS_GSM_PRODUCT_ID:
+				strcpy(buf, "mts_gsm.fw");
+				break;
+			case MTS_EDGE_PRODUCT_ID:
+				strcpy(buf, "mts_edge.fw");
+				break;
+			case MTS_MT9234MU_PRODUCT_ID:
+				strcpy(buf, "mts_mt9234mu.fw");
+				break;
+			case MTS_MT9234ZBA_PRODUCT_ID:
+				strcpy(buf, "mts_mt9234zba.fw");
+				break;
+			case MTS_MT9234ZBAOLD_PRODUCT_ID:
+				strcpy(buf, "mts_mt9234zba.fw");
+				break;			}
+		}
 
-		/* 3410 must be reset, 5052 resets itself */
-		if (tdev->is_3410) {
-			msleep_interruptible(100);
-			usb_reset_device(dev);
+		if (buf[0] == '\0') {
+			if (tdev->is_3410)
+				strcpy(buf, "ti_3410.fw");
+			else
+				strcpy(buf, "ti_5052.fw");
 		}
+		status = request_firmware(&fw_p, buf, &dev->dev);
+	}
 
-		status = -ENODEV;
-		goto free_tdev;
+check_firmware:
+	if (status) {
+		dev_err(&dev->dev, "failed to request firmware: %d\n", status);
+		return -ENOENT;
 	}
 
-	/* the second configuration must be set */
-	if (dev->actconfig->desc.bConfigurationValue == TI_BOOT_CONFIG) {
-		status = usb_driver_set_configuration(dev, TI_ACTIVE_CONFIG);
-		status = status ? status : -ENODEV;
-		goto free_tdev;
+	if (fw_p->size > TI_FIRMWARE_BUF_SIZE) {
+		dev_err(&dev->dev, "firmware too large: %zu\n", fw_p->size);
+		release_firmware(fw_p);
+		return -ENOENT;
 	}
 
+	ti_do_download(serial, fw_p);
+
+	release_firmware(fw_p);
+
 	return 0;
-
-free_tdev:
-	kfree(tdev);
-	usb_set_serial_data(serial, NULL);
-	return status;
-}
-
-
-static void ti_release(struct usb_serial *serial)
-{
-	struct ti_device *tdev = usb_get_serial_data(serial);
-
-	kfree(tdev);
-}
+}
 
 static int ti_port_probe(struct usb_serial_port *port)
 {
@@ -670,258 +589,157 @@ static int ti_port_remove(struct usb_serial_port *port)
 	return 0;
 }
 
-static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
+static int ti_startup(struct usb_serial *serial)
 {
-	struct ti_port *tport = usb_get_serial_port_data(port);
-	struct ti_device *tdev = usb_get_serial_data(port->serial);
-	struct usb_serial *serial = port->serial;
-	struct urb *urb;
-	int port_number;
+	struct ti_device *tdev;
+	struct usb_device *dev = serial->dev;
+	struct usb_host_interface *cur_altsetting;
+	int num_endpoints;
+	u16 vid, pid;
 	int status;
-	u16 open_settings;
 
-	open_settings = (TI_PIPE_MODE_CONTINUOUS |
-			 TI_PIPE_TIMEOUT_ENABLE |
-			 (TI_TRANSFER_TIMEOUT << 2));
+	dev_dbg(&dev->dev,
+		"%s - product 0x%4X, num configurations %d, configuration value %d\n",
+		__func__, le16_to_cpu(dev->descriptor.idProduct),
+		dev->descriptor.bNumConfigurations,
+		dev->actconfig->desc.bConfigurationValue);
 
-	/* only one open on any port on a device at a time */
-	if (mutex_lock_interruptible(&tdev->open_close_lock))
-		return -ERESTARTSYS;
+	tdev = kzalloc(sizeof(struct ti_device), GFP_KERNEL);
+	if (!tdev)
+		return -ENOMEM;
 
-	port_number = port->port_number;
+	mutex_init(&tdev->open_close_lock);
+	usb_set_serial_data(serial, tdev);
 
-	tport->msr = 0;
+	/* determine device type */
+	if (serial->type == &ti_1port_device)
+		tdev->is_3410 = true;
+	dev_dbg(&dev->dev, "%s - device type is: %s\n", __func__,
+		tdev->is_3410 ? "3410" : "5052");
 
-	/* start interrupt urb the first time a port is opened on this device */
-	if (tdev->open_port_count == 0) {
-		dev_dbg(&port->dev, "%s - start interrupt in urb\n", __func__);
-		urb = serial->port[0]->interrupt_in_urb;
-		if (!urb) {
-			dev_err(&port->dev, "no interrupt endpoint\n");
-			status = -EINVAL;
-			goto release_lock;
-		}
-		status = usb_submit_urb(urb, GFP_KERNEL);
-		if (status) {
-			dev_err(&port->dev, "failed to submit interrupt urb: %d\n",
-				status);
-			goto release_lock;
+	vid = le16_to_cpu(dev->descriptor.idVendor);
+	pid = le16_to_cpu(dev->descriptor.idProduct);
+	if (vid == MXU1_VENDOR_ID) {
+		switch (pid) {
+		case MXU1_1130_PRODUCT_ID:
+		case MXU1_1131_PRODUCT_ID:
+			tdev->rs485_only = true;
+			break;
 		}
 	}
 
-	if (tty)
-		ti_set_termios(tty, port, NULL);
+	cur_altsetting = serial->interface->cur_altsetting;
+	num_endpoints = cur_altsetting->desc.bNumEndpoints;
 
-	status = ti_send_ctrl_urb(serial, TI_OPEN_PORT, open_settings,
-				  TI_UART1_PORT + port_number);
-	if (status) {
-		dev_err(&port->dev, "cannot send open command: %d\n", status);
-		goto unlink_int_urb;
-	}
+	/* if we have only 1 configuration and 1 endpoint, download firmware */
+	if (dev->descriptor.bNumConfigurations == 1 && num_endpoints == 1) {
+		status = ti_download_firmware(serial);
 
-	status = ti_send_ctrl_urb(serial, TI_START_PORT, 0,
-				  TI_UART1_PORT + port_number);
-	if (status) {
-		dev_err(&port->dev, "cannot send start command: %d\n", status);
-		goto unlink_int_urb;
-	}
+		if (status != 0)
+			goto free_tdev;
 
-	status = ti_send_ctrl_urb(serial, TI_PURGE_PORT, TI_PURGE_INPUT,
-				  TI_UART1_PORT + port_number);
-	if (status) {
-		dev_err(&port->dev, "cannot clear input buffers: %d\n", status);
-		goto unlink_int_urb;
-	}
+		/* 3410 must be reset, 5052 resets itself */
+		if (tdev->is_3410) {
+			msleep_interruptible(100);
+			usb_reset_device(dev);
+		}
 
-	status = ti_send_ctrl_urb(serial, TI_PURGE_PORT, TI_PURGE_OUTPUT,
-				  TI_UART1_PORT + port_number);
-	if (status) {
-		dev_err(&port->dev, "cannot clear output buffers: %d\n",
-			status);
-		goto unlink_int_urb;
+		status = -ENODEV;
+		goto free_tdev;
 	}
 
-	/*
-	 * 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(serial->dev, port->write_urb->pipe);
-	usb_clear_halt(serial->dev, port->read_urb->pipe);
-
-	if (tty)
-		ti_set_termios(tty, port, NULL);
-
-	status = ti_send_ctrl_urb(serial, TI_OPEN_PORT, open_settings,
-				  TI_UART1_PORT + port_number);
-	if (status) {
-		dev_err(&port->dev, "cannot send open command (2): %d\n",
-			status);
-		goto unlink_int_urb;
+	/* the second configuration must be set */
+	if (dev->actconfig->desc.bConfigurationValue == TI_BOOT_CONFIG) {
+		status = usb_driver_set_configuration(dev, TI_ACTIVE_CONFIG);
+		status = status ? status : -ENODEV;
+		goto free_tdev;
 	}
 
-	status = ti_send_ctrl_urb(serial, TI_START_PORT, 0,
-				  TI_UART1_PORT + port_number);
-	if (status) {
-		dev_err(&port->dev, "cannot send start command (2): %d\n",
-			status);
-		goto unlink_int_urb;
-	}
+	return 0;
 
-	status = usb_serial_generic_open(tty, port);
-	if (status)
-		goto unlink_int_urb;
+free_tdev:
+	kfree(tdev);
+	usb_set_serial_data(serial, NULL);
+	return status;
+}
 
-	++tdev->open_port_count;
 
-	goto release_lock;
+static void ti_release(struct usb_serial *serial)
+{
+	struct ti_device *tdev = usb_get_serial_data(serial);
 
-unlink_int_urb:
-	if (tdev->open_port_count == 0)
-		usb_kill_urb(serial->port[0]->interrupt_in_urb);
-release_lock:
-	mutex_unlock(&tdev->open_close_lock);
-	return status;
+	kfree(tdev);
 }
 
-
-static void ti_close(struct usb_serial_port *port)
+static int ti_write_byte(struct usb_serial_port *port, u32 addr,
+			u8 mask, u8 byte)
 {
-	struct ti_device *tdev;
-	struct ti_port *tport;
 	int status;
-	int do_unlock;
+	size_t size;
+	struct ti_write_data_bytes *data;
 
-	tdev = usb_get_serial_data(port->serial);
-	tport = usb_get_serial_port_data(port);
+	dev_dbg(&port->dev, "%s - addr 0x%08X, mask 0x%02X, byte 0x%02X\n",
+		__func__, addr, mask, byte);
 
-	usb_serial_generic_close(port);
+	size = sizeof(struct ti_write_data_bytes) + 2;
+	data = kmalloc(size, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
 
-	status = ti_send_ctrl_urb(port->serial, TI_CLOSE_PORT, 0,
-				  TI_UART1_PORT + port->port_number);
-	if (status) {
-		dev_err(&port->dev, "failed to send close port command: %d\n",
-			status);
-	}
+	data->bAddrType = TI_RW_DATA_ADDR_XDATA;
+	data->bDataType = TI_RW_DATA_BYTE;
+	data->bDataCounter = 1;
+	data->wBaseAddrHi = cpu_to_be16(addr >> 16);
+	data->wBaseAddrLo = cpu_to_be16(addr);
+	data->bData[0] = mask;
+	data->bData[1] = byte;
 
-	/* if mutex_lock is interrupted, continue anyway */
-	do_unlock = !mutex_lock_interruptible(&tdev->open_close_lock);
-	tdev->open_port_count--;
-	if (tdev->open_port_count <= 0) {
-		/* last port is closed, shut down interrupt urb */
-		usb_kill_urb(port->serial->port[0]->interrupt_in_urb);
-		tdev->open_port_count = 0;
-	}
-	if (do_unlock)
-		mutex_unlock(&tdev->open_close_lock);
+	status = ti_send_ctrl_data_urb(port->serial, TI_WRITE_DATA, 0,
+				       TI_RAM_PORT, data, size);
+	if (status < 0)
+		dev_err(&port->dev, "%s - failed: %d\n", __func__, status);
+
+	kfree(data);
+
+	return status;
 }
 
-static bool ti_tx_empty(struct usb_serial_port *port)
+static int ti_set_mcr(struct usb_serial_port *port, unsigned int mcr)
 {
-	int ret;
-	u8 lsr;
-
-	ret = ti_get_lsr(port, &lsr);
-	if (!ret && !(lsr & TI_LSR_TX_EMPTY))
-		return false;
+	struct ti_port *tport = usb_get_serial_port_data(port);
+	int status;
 
-	return true;
+	status = ti_write_byte(port,
+			       tport->uart_base_addr + TI_UART_OFFSET_MCR,
+			       TI_MCR_RTS | TI_MCR_DTR | TI_MCR_LOOP, mcr);
+	return status;
 }
 
-static int ti_get_serial_info(struct usb_serial_port *port,
-			      struct serial_struct __user *ret_arg)
+static void ti_set_termios(struct tty_struct *tty,
+			   struct usb_serial_port *port,
+			   struct ktermios *old_termios)
 {
+	struct ti_port *tport = usb_get_serial_port_data(port);
 	struct ti_device *tdev = usb_get_serial_data(port->serial);
-	struct serial_struct ret_serial;
-	unsigned int cwait;
-	int baud_base;
+	struct ti_uart_config *config;
+	tcflag_t cflag, iflag;
+	int baud;
+	int status;
+	int port_number = port->port_number;
+	unsigned int mcr;
 
-	if (!ret_arg)
-		return -EFAULT;
+	cflag = tty->termios.c_cflag;
+	iflag = tty->termios.c_iflag;
 
-	cwait = port->port.closing_wait;
-	if (cwait != ASYNC_CLOSING_WAIT_NONE)
-		cwait = jiffies_to_msecs(cwait) / 10;
+	if (old_termios &&
+		!tty_termios_hw_change(&tty->termios, old_termios) &&
+		tty->termios.c_iflag == old_termios->c_iflag) {
+		dev_dbg(&port->dev, "%s - nothing to change\n", __func__);
+		return;
+	}
 
-	memset(&ret_serial, 0, sizeof(ret_serial));
-
-	if (tdev->is_3410)
-		baud_base = TI_3410_BAUD_BASE;
-	else
-		baud_base = TI_5052_BAUD_BASE;
-
-	ret_serial.type = PORT_16550A;
-	ret_serial.line = port->minor;
-	ret_serial.port = port->port_number;
-	ret_serial.xmit_fifo_size = port->bulk_out_size;
-	ret_serial.baud_base = baud_base;
-	ret_serial.closing_wait = cwait;
-
-	if (copy_to_user(ret_arg, &ret_serial, sizeof(*ret_arg)))
-		return -EFAULT;
-
-	return 0;
-}
-
-static int ti_set_serial_info(struct usb_serial_port *port,
-			      struct serial_struct __user *new_arg)
-{
-	struct serial_struct new_serial;
-	unsigned int cwait;
-
-	if (copy_from_user(&new_serial, new_arg, sizeof(new_serial)))
-		return -EFAULT;
-
-	cwait = new_serial.closing_wait;
-	if (cwait != ASYNC_CLOSING_WAIT_NONE)
-		cwait = msecs_to_jiffies(10 * new_serial.closing_wait);
-
-	port->port.closing_wait = cwait;
-
-	return 0;
-}
-
-static int ti_ioctl(struct tty_struct *tty,
-		    unsigned int cmd, unsigned long arg)
-{
-	struct usb_serial_port *port = tty->driver_data;
-
-	switch (cmd) {
-	case TIOCGSERIAL:
-		return ti_get_serial_info(port,
-					  (struct serial_struct __user *)arg);
-	case TIOCSSERIAL:
-		return ti_set_serial_info(port,
-					  (struct serial_struct __user *)arg);
-	}
-	return -ENOIOCTLCMD;
-}
-
-
-static void ti_set_termios(struct tty_struct *tty,
-			   struct usb_serial_port *port,
-			   struct ktermios *old_termios)
-{
-	struct ti_port *tport = usb_get_serial_port_data(port);
-	struct ti_device *tdev = usb_get_serial_data(port->serial);
-	struct ti_uart_config *config;
-	tcflag_t cflag, iflag;
-	int baud;
-	int status;
-	int port_number = port->port_number;
-	unsigned int mcr;
-
-	cflag = tty->termios.c_cflag;
-	iflag = tty->termios.c_iflag;
-
-	if (old_termios &&
-		!tty_termios_hw_change(&tty->termios, old_termios) &&
-		tty->termios.c_iflag == old_termios->c_iflag) {
-		dev_dbg(&port->dev, "%s - nothing to change\n", __func__);
-		return;
-	}
-
-	dev_dbg(&port->dev,
-		"%s - cflag 0x%08x, iflag 0x%08x\n", __func__, cflag, 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",
@@ -1045,6 +863,74 @@ static void ti_set_termios(struct tty_struct *tty,
 	kfree(config);
 }
 
+static int ti_get_serial_info(struct usb_serial_port *port,
+			      struct serial_struct __user *ret_arg)
+{
+	struct ti_device *tdev = usb_get_serial_data(port->serial);
+	struct serial_struct ret_serial;
+	unsigned int cwait;
+	int baud_base;
+
+	if (!ret_arg)
+		return -EFAULT;
+
+	cwait = port->port.closing_wait;
+	if (cwait != ASYNC_CLOSING_WAIT_NONE)
+		cwait = jiffies_to_msecs(cwait) / 10;
+
+	memset(&ret_serial, 0, sizeof(ret_serial));
+
+	if (tdev->is_3410)
+		baud_base = TI_3410_BAUD_BASE;
+	else
+		baud_base = TI_5052_BAUD_BASE;
+
+	ret_serial.type = PORT_16550A;
+	ret_serial.line = port->minor;
+	ret_serial.port = port->port_number;
+	ret_serial.xmit_fifo_size = port->bulk_out_size;
+	ret_serial.baud_base = baud_base;
+	ret_serial.closing_wait = cwait;
+
+	if (copy_to_user(ret_arg, &ret_serial, sizeof(*ret_arg)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int ti_set_serial_info(struct usb_serial_port *port,
+			      struct serial_struct __user *new_arg)
+{
+	struct serial_struct new_serial;
+	unsigned int cwait;
+
+	if (copy_from_user(&new_serial, new_arg, sizeof(new_serial)))
+		return -EFAULT;
+
+	cwait = new_serial.closing_wait;
+	if (cwait != ASYNC_CLOSING_WAIT_NONE)
+		cwait = msecs_to_jiffies(10 * new_serial.closing_wait);
+
+	port->port.closing_wait = cwait;
+
+	return 0;
+}
+
+static int ti_ioctl(struct tty_struct *tty,
+		    unsigned int cmd, unsigned long arg)
+{
+	struct usb_serial_port *port = tty->driver_data;
+
+	switch (cmd) {
+	case TIOCGSERIAL:
+		return ti_get_serial_info(port,
+					  (struct serial_struct __user *)arg);
+	case TIOCSSERIAL:
+		return ti_set_serial_info(port,
+					  (struct serial_struct __user *)arg);
+	}
+	return -ENOIOCTLCMD;
+}
 
 static int ti_tiocmget(struct tty_struct *tty)
 {
@@ -1126,91 +1012,151 @@ static void ti_break(struct tty_struct *tty, int break_state)
 		dev_dbg(&port->dev, "failed to set break: %d\n", status);
 }
 
-
-static void ti_interrupt_callback(struct urb *urb)
+static int ti_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
-	struct usb_serial_port *port = urb->context;
-	unsigned char *data = urb->transfer_buffer;
-	int length = urb->actual_length;
+	struct ti_port *tport = usb_get_serial_port_data(port);
+	struct ti_device *tdev = usb_get_serial_data(port->serial);
+	struct usb_serial *serial = port->serial;
+	struct urb *urb;
 	int port_number;
-	int function;
-	int status = urb->status;
-	u8 msr;
-
-	switch (status) {
-	case 0:
-		break;
-	case -ECONNRESET:
-	case -ENOENT:
-	case -ESHUTDOWN:
-		dev_dbg(&port->dev, "%s - urb shutting down: %d\n",
-			__func__, status);
-		return;
-	default:
-		dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
-			__func__, status);
-		goto exit;
-	}
+	int status;
+	u16 open_settings;
 
-	if (length != 2) {
-		dev_dbg(&port->dev, "%s - bad packet size: %d\n",
-			__func__, length);
-		goto exit;
-	}
+	open_settings = (TI_PIPE_MODE_CONTINUOUS |
+			 TI_PIPE_TIMEOUT_ENABLE |
+			 (TI_TRANSFER_TIMEOUT << 2));
 
-	if (data[0] == TI_CODE_HARDWARE_ERROR) {
-		dev_err(&port->dev, "%s - hardware error: %d\n",
-			__func__, data[1]);
-		goto exit;
-	}
+	/* only one open on any port on a device at a time */
+	if (mutex_lock_interruptible(&tdev->open_close_lock))
+		return -ERESTARTSYS;
 
-	port_number = ti_get_port_from_code(data[0]);
-	function = ti_get_func_from_code(data[0]);
+	port_number = port->port_number;
 
-	dev_dbg(&port->dev, "%s - port_number %d, function %d, data 0x%02X\n",
-		__func__, port_number, function, data[1]);
+	tport->msr = 0;
 
-	if (port_number >= port->serial->num_ports) {
-		dev_err(&port->dev, "bad port number: %d\n", port_number);
-		goto exit;
+	/* start interrupt urb the first time a port is opened on this device */
+	if (tdev->open_port_count == 0) {
+		dev_dbg(&port->dev, "%s - start interrupt in urb\n", __func__);
+		urb = serial->port[0]->interrupt_in_urb;
+		if (!urb) {
+			dev_err(&port->dev, "no interrupt endpoint\n");
+			status = -EINVAL;
+			goto release_lock;
+		}
+		status = usb_submit_urb(urb, GFP_KERNEL);
+		if (status) {
+			dev_err(&port->dev, "failed to submit interrupt urb: %d\n",
+				status);
+			goto release_lock;
+		}
 	}
 
-	switch (function) {
-	case TI_CODE_DATA_ERROR:
-		dev_dbg(&port->dev, "%s - DATA ERROR, port %d, data 0x%02X\n",
-			__func__, port_number, data[1]);
-		break;
+	if (tty)
+		ti_set_termios(tty, port, NULL);
 
-	case TI_CODE_MODEM_STATUS:
-		msr = data[1];
-		ti_handle_new_msr(port, msr);
-		break;
+	status = ti_send_ctrl_urb(serial, TI_OPEN_PORT, open_settings,
+				  TI_UART1_PORT + port_number);
+	if (status) {
+		dev_err(&port->dev, "cannot send open command: %d\n", status);
+		goto unlink_int_urb;
+	}
 
-	default:
-		dev_err(&port->dev, "unknown interrupt code: 0x%02X\n",
-			data[1]);
-		break;
+	status = ti_send_ctrl_urb(serial, TI_START_PORT, 0,
+				  TI_UART1_PORT + port_number);
+	if (status) {
+		dev_err(&port->dev, "cannot send start command: %d\n", status);
+		goto unlink_int_urb;
 	}
 
-exit:
-	status = usb_submit_urb(urb, GFP_ATOMIC);
-	if (status)
-		dev_err(&port->dev, "resubmit interrupt urb failed: %d\n",
-			status);
-}
+	status = ti_send_ctrl_urb(serial, TI_PURGE_PORT, TI_PURGE_INPUT,
+				  TI_UART1_PORT + port_number);
+	if (status) {
+		dev_err(&port->dev, "cannot clear input buffers: %d\n", status);
+		goto unlink_int_urb;
+	}
 
-static int ti_set_mcr(struct usb_serial_port *port, unsigned int mcr)
-{
-	struct ti_port *tport = usb_get_serial_port_data(port);
-	int status;
+	status = ti_send_ctrl_urb(serial, TI_PURGE_PORT, TI_PURGE_OUTPUT,
+				  TI_UART1_PORT + port_number);
+	if (status) {
+		dev_err(&port->dev, "cannot clear output buffers: %d\n",
+			status);
+		goto unlink_int_urb;
+	}
 
-	status = ti_write_byte(port,
-			       tport->uart_base_addr + TI_UART_OFFSET_MCR,
-			       TI_MCR_RTS | TI_MCR_DTR | TI_MCR_LOOP, mcr);
+	/*
+	 * 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(serial->dev, port->write_urb->pipe);
+	usb_clear_halt(serial->dev, port->read_urb->pipe);
+
+	if (tty)
+		ti_set_termios(tty, port, NULL);
+
+	status = ti_send_ctrl_urb(serial, TI_OPEN_PORT, open_settings,
+				  TI_UART1_PORT + port_number);
+	if (status) {
+		dev_err(&port->dev, "cannot send open command (2): %d\n",
+			status);
+		goto unlink_int_urb;
+	}
+
+	status = ti_send_ctrl_urb(serial, TI_START_PORT, 0,
+				  TI_UART1_PORT + port_number);
+	if (status) {
+		dev_err(&port->dev, "cannot send start command (2): %d\n",
+			status);
+		goto unlink_int_urb;
+	}
+
+	status = usb_serial_generic_open(tty, port);
+	if (status)
+		goto unlink_int_urb;
+
+	++tdev->open_port_count;
+
+	goto release_lock;
+
+unlink_int_urb:
+	if (tdev->open_port_count == 0)
+		usb_kill_urb(serial->port[0]->interrupt_in_urb);
+release_lock:
+	mutex_unlock(&tdev->open_close_lock);
 	return status;
 }
 
 
+static void ti_close(struct usb_serial_port *port)
+{
+	struct ti_device *tdev;
+	struct ti_port *tport;
+	int status;
+	int do_unlock;
+
+	tdev = usb_get_serial_data(port->serial);
+	tport = usb_get_serial_port_data(port);
+
+	usb_serial_generic_close(port);
+
+	status = ti_send_ctrl_urb(port->serial, TI_CLOSE_PORT, 0,
+				  TI_UART1_PORT + port->port_number);
+	if (status) {
+		dev_err(&port->dev, "failed to send close port command: %d\n",
+			status);
+	}
+
+	/* if mutex_lock is interrupted, continue anyway */
+	do_unlock = !mutex_lock_interruptible(&tdev->open_close_lock);
+	tdev->open_port_count--;
+	if (tdev->open_port_count <= 0) {
+		/* last port is closed, shut down interrupt urb */
+		usb_kill_urb(port->serial->port[0]->interrupt_in_urb);
+		tdev->open_port_count = 0;
+	}
+	if (do_unlock)
+		mutex_unlock(&tdev->open_close_lock);
+}
+
 static int ti_get_lsr(struct usb_serial_port *port, u8 *lsr)
 {
 	int size, status;
@@ -1240,6 +1186,18 @@ free_data:
 	return status;
 }
 
+static bool ti_tx_empty(struct usb_serial_port *port)
+{
+	int ret;
+	u8 lsr;
+
+	ret = ti_get_lsr(port, &lsr);
+	if (!ret && !(lsr & TI_LSR_TX_EMPTY))
+		return false;
+
+	return true;
+}
+
 static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr)
 {
 	struct ti_port *tport = usb_get_serial_port_data(port);
@@ -1267,131 +1225,149 @@ static void ti_handle_new_msr(struct usb_serial_port *port, u8 msr)
 	}
 }
 
-static int ti_do_download(struct usb_serial *serial,
-			  const struct firmware *fw_p)
+static void ti_interrupt_callback(struct urb *urb)
 {
-	int pos;
-	u8 cs = 0;
-	int done;
-	struct usb_device *dev = serial->dev;
-	struct ti_firmware_header *header;
-	int status = 0;
-	u8 *buffer;
-	int buffer_size;
-	int len;
-	unsigned int pipe;
-
-	pipe = usb_sndbulkpipe(dev, serial->port[0]->bulk_out_endpointAddress);
-
-	buffer_size = fw_p->size;
-	buffer = kmalloc(buffer_size, GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
-
-	memcpy(buffer, fw_p->data, fw_p->size);
-
-	for (pos = sizeof(*header); pos < buffer_size; pos++)
-		cs = (u8)(cs + buffer[pos]);
-
-	header = (struct ti_firmware_header *)buffer;
-	header->wLength = cpu_to_le16(buffer_size - sizeof(*header));
-	header->bCheckSum = cs;
+	struct usb_serial_port *port = urb->context;
+	unsigned char *data = urb->transfer_buffer;
+	int length = urb->actual_length;
+	int port_number;
+	int function;
+	int status = urb->status;
+	u8 msr;
 
-	dev_dbg(&dev->dev, "%s - downloading firmware\n", __func__);
-	for (pos = 0; pos < buffer_size; pos += done) {
-		len = min(buffer_size - pos, TI_DOWNLOAD_MAX_PACKET_SIZE);
-		status = usb_bulk_msg(dev, pipe, buffer + pos, len, &done,
-				      TI_DOWNLOAD_TIMEOUT);
-		if (status)
-			break;
+	switch (status) {
+	case 0:
+		break;
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+		dev_dbg(&port->dev, "%s - urb shutting down: %d\n",
+			__func__, status);
+		return;
+	default:
+		dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
+			__func__, status);
+		goto exit;
 	}
 
-	kfree(buffer);
+	if (length != 2) {
+		dev_dbg(&port->dev, "%s - bad packet size: %d\n",
+			__func__, length);
+		goto exit;
+	}
 
-	if (status) {
-		dev_err(&dev->dev, "failed to download firmware: %d\n", status);
-		return status;
+	if (data[0] == TI_CODE_HARDWARE_ERROR) {
+		dev_err(&port->dev, "%s - hardware error: %d\n",
+			__func__, data[1]);
+		goto exit;
 	}
 
-	dev_dbg(&dev->dev, "%s - download successful\n", __func__);
+	port_number = ti_get_port_from_code(data[0]);
+	function = ti_get_func_from_code(data[0]);
 
-	return 0;
-}
+	dev_dbg(&port->dev, "%s - port_number %d, function %d, data 0x%02X\n",
+		__func__, port_number, function, data[1]);
 
-static int ti_download_firmware(struct usb_serial *serial)
-{
-	int status;
-	struct usb_device *dev = serial->dev;
-	struct ti_device *tdev = usb_get_serial_data(serial);
-	const struct firmware *fw_p;
-	char buf[32];
-	__le16 vendor, product;
+	if (port_number >= port->serial->num_ports) {
+		dev_err(&port->dev, "bad port number: %d\n", port_number);
+		goto exit;
+	}
 
-	vendor = le16_to_cpu(dev->descriptor.idVendor);
-	product = le16_to_cpu(dev->descriptor.idProduct);
+	switch (function) {
+	case TI_CODE_DATA_ERROR:
+		dev_dbg(&port->dev, "%s - DATA ERROR, port %d, data 0x%02X\n",
+			__func__, port_number, data[1]);
+		break;
 
-	if (le16_to_cpu(dev->descriptor.idVendor) == MXU1_VENDOR_ID) {
-		snprintf(buf,
-			sizeof(buf),
-			"moxa/moxa-%04x.fw",
-			le16_to_cpu(dev->descriptor.idProduct));
+	case TI_CODE_MODEM_STATUS:
+		msr = data[1];
+		ti_handle_new_msr(port, msr);
+		break;
 
-		status = request_firmware(&fw_p, buf, &dev->dev);
-		goto check_firmware;
+	default:
+		dev_err(&port->dev, "unknown interrupt code: 0x%02X\n",
+			data[1]);
+		break;
 	}
 
-	/* try ID specific firmware first, then try generic firmware */
-	sprintf(buf, "ti_usb-v%04x-p%04x.fw", vendor, product);
-	status = request_firmware(&fw_p, buf, &dev->dev);
+exit:
+	status = usb_submit_urb(urb, GFP_ATOMIC);
+	if (status)
+		dev_err(&port->dev, "resubmit interrupt urb failed: %d\n",
+			status);
+}
 
-	if (status != 0) {
-		buf[0] = '\0';
-		if (vendor == MTS_VENDOR_ID) {
-			switch (product) {
-			case MTS_CDMA_PRODUCT_ID:
-				strcpy(buf, "mts_cdma.fw");
-				break;
-			case MTS_GSM_PRODUCT_ID:
-				strcpy(buf, "mts_gsm.fw");
-				break;
-			case MTS_EDGE_PRODUCT_ID:
-				strcpy(buf, "mts_edge.fw");
-				break;
-			case MTS_MT9234MU_PRODUCT_ID:
-				strcpy(buf, "mts_mt9234mu.fw");
-				break;
-			case MTS_MT9234ZBA_PRODUCT_ID:
-				strcpy(buf, "mts_mt9234zba.fw");
-				break;
-			case MTS_MT9234ZBAOLD_PRODUCT_ID:
-				strcpy(buf, "mts_mt9234zba.fw");
-				break;			}
-		}
+static struct usb_serial_driver ti_1port_device = {
+	.driver = {
+		.owner		= THIS_MODULE,
+		.name		= "ti_usb_3410_5052_1",
+	},
+	.description		= "TI USB 3410 1 port adapter",
+	.id_table		= ti_id_table_3410,
+	.num_ports		= 1,
+	.attach			= ti_startup,
+	.release		= ti_release,
+	.port_probe		= ti_port_probe,
+	.port_remove		= ti_port_remove,
+	.open			= ti_open,
+	.close			= ti_close,
+	.tx_empty		= ti_tx_empty,
+	.ioctl			= ti_ioctl,
+	.set_termios		= ti_set_termios,
+	.tiocmget		= ti_tiocmget,
+	.tiocmset		= ti_tiocmset,
+	.tiocmiwait		= usb_serial_generic_tiocmiwait,
+	.get_icount		= usb_serial_generic_get_icount,
+	.break_ctl		= ti_break,
+	.read_int_callback	= ti_interrupt_callback,
+};
 
-		if (buf[0] == '\0') {
-			if (tdev->is_3410)
-				strcpy(buf, "ti_3410.fw");
-			else
-				strcpy(buf, "ti_5052.fw");
-		}
-		status = request_firmware(&fw_p, buf, &dev->dev);
-	}
+static struct usb_serial_driver ti_2port_device = {
+	.driver = {
+		.owner		= THIS_MODULE,
+		.name		= "ti_usb_3410_5052_2",
+	},
+	.description		= "TI USB 5052 2 port adapter",
+	.id_table		= ti_id_table_5052,
+	.num_ports		= 2,
+	.attach			= ti_startup,
+	.release		= ti_release,
+	.port_probe		= ti_port_probe,
+	.port_remove		= ti_port_remove,
+	.open			= ti_open,
+	.close			= ti_close,
+	.tx_empty		= ti_tx_empty,
+	.ioctl			= ti_ioctl,
+	.set_termios		= ti_set_termios,
+	.tiocmget		= ti_tiocmget,
+	.tiocmset		= ti_tiocmset,
+	.tiocmiwait		= usb_serial_generic_tiocmiwait,
+	.get_icount		= usb_serial_generic_get_icount,
+	.break_ctl		= ti_break,
+	.read_int_callback	= ti_interrupt_callback,
+};
 
-check_firmware:
-	if (status) {
-		dev_err(&dev->dev, "failed to request firmware: %d\n", status);
-		return -ENOENT;
-	}
+static struct usb_serial_driver * const serial_drivers[] = {
+	&ti_1port_device, &ti_2port_device, NULL
+};
 
-	if (fw_p->size > TI_FIRMWARE_BUF_SIZE) {
-		dev_err(&dev->dev, "firmware too large: %zu\n", fw_p->size);
-		release_firmware(fw_p);
-		return -ENOENT;
-	}
+MODULE_AUTHOR(TI_DRIVER_AUTHOR);
+MODULE_DESCRIPTION(TI_DRIVER_DESC);
+MODULE_LICENSE("GPL");
 
-	ti_do_download(serial, fw_p);
+MODULE_FIRMWARE("ti_3410.fw");
+MODULE_FIRMWARE("ti_5052.fw");
+MODULE_FIRMWARE("mts_cdma.fw");
+MODULE_FIRMWARE("mts_gsm.fw");
+MODULE_FIRMWARE("mts_edge.fw");
+MODULE_FIRMWARE("mts_mt9234mu.fw");
+MODULE_FIRMWARE("mts_mt9234zba.fw");
+MODULE_FIRMWARE("moxa/moxa-1110.fw");
+MODULE_FIRMWARE("moxa/moxa-1130.fw");
+MODULE_FIRMWARE("moxa/moxa-1131.fw");
+MODULE_FIRMWARE("moxa/moxa-1150.fw");
+MODULE_FIRMWARE("moxa/moxa-1151.fw");
 
-	release_firmware(fw_p);
+MODULE_DEVICE_TABLE(usb, ti_id_table_combined);
 
-	return 0;
-}
+module_usb_serial_driver(serial_drivers, ti_id_table_combined);
-- 
2.8.2

Re: [PATCH 35/36] usb: serial: ti_usb_3410_5052: Remove function prototypes

[Johan Hovold]

On Thu, May 12, 2016 at 10:49:07AM +0200, Mathieu OTHACEHE wrote:
> Declare functions in a the right order to avoid prototyping.
> There is no functional change here.

I'm not sure this is needed.

Thanks,
Johan

[PATCH 36/36] usb: serial: ti_usb_3410_5052: Add myself as an author

[Mathieu OTHACEHE]

Add myself in the copyright section and as an author of the driver.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 95bdcca..90f80b6 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -1,6 +1,7 @@
 /*
  * TI 3410/5052 USB Serial Driver
  *
+ * Copyright (C) 2016 Mathieu Othacehe <m.othacehe@gmail.com>
  * Copyright (C) 2004 Texas Instruments
  *
  * This driver is based on the Linux io_ti driver, which is
@@ -256,7 +257,7 @@ struct ti_firmware_header {
 #define TI_UART_OFFSET_LCR		0x0002	/* UART MCR register offset */
 #define TI_UART_OFFSET_MCR		0x0004	/* UART MCR register offset */
 
-#define TI_DRIVER_AUTHOR	"Al Borchers <alborchers@steinerpoint.com>"
+#define TI_DRIVER_AUTHOR	"Al Borchers <alborchers@steinerpoint.com>, Mathieu OTHACEHE <m.othacehe@gmail.com>"
 #define TI_DRIVER_DESC		"TI USB 3410/5052 Serial Driver"
 
 #define TI_3410_BAUD_BASE       923077
-- 
2.8.2

Re: [PATCH 00/36] usb: serial: ti_usb_3410_5052: clean driver

[Johan Hovold]

On Thu, May 12, 2016 at 10:48:32AM +0200, Mathieu OTHACEHE wrote:
> Hi,
> 
> The now reverted mxu11x0 turned out to be a copy of ti_usb_3410_5052 driver.
> This aim of this serie is to apply all of the cleanups we did in mxu11x0 to
> ti_usb_3410_5052.

I apologise for the late review of this one. I've applied the first four
now (with some tweaks) and will comment on the rest.

Thanks,
Johan

Re: [PATCH 00/36] usb: serial: ti_usb_3410_5052: clean driver

[Johan Hovold]

On Fri, Jul 15, 2016 at 12:48:25PM +0200, Johan Hovold wrote:
> On Thu, May 12, 2016 at 10:48:32AM +0200, Mathieu OTHACEHE wrote:
> > Hi,
> > 
> > The now reverted mxu11x0 turned out to be a copy of ti_usb_3410_5052 driver.
> > This aim of this serie is to apply all of the cleanups we did in mxu11x0 to
> > ti_usb_3410_5052.
> 
> I apologise for the late review of this one. I've applied the first four
> now (with some tweaks) and will comment on the rest.

These overall look really good. I had few comments to some of the
patches though.

Thanks again for doing this.

Johan