On Fri, Nov 08, 2013 at 02:19:55PM +0100, Andrew Lunn wrote: > Add a driver which supports the following Moxa USB to serial converters: > * 2 ports : UPort 1250, UPort 1250I > * 4 ports : UPort 1410, UPort 1450, UPort 1450I > * 8 ports : UPort 1610-8, UPort 1650-8 > * 16 ports : UPort 1610-16, UPort 1650-16 > > The UPORT devices don't directy fit the USB serial model. USB serial > assumes a bulk in/out endpoint pair per serial port. Thus a dual port > USB serial device is expected to have two bulk in/out pairs. The Moxa > UPORT only has one pair for data transfer and places a header on each > transfer over the endpoint indicating for which port the transfer > relates to. There is a second endpoint pair for events, such as modem > control lines changing state, setting baud rates etc. Again, a > multiplexing header is used on these endpoints. > > Some ports need to have a kfifo explicitily allocated since the > framework does not allocate one if there is no associated endpoints. > The framework will however free it on unload of the module. > > All data transfers are made on port0, yet the locks are taken on PortN. > urb->context points to PortN, even though the URB is for port0. > > Where possible, code from the generic driver is called. However > mxuport_process_read_urb_data() is mostly a cut/paste of > usb_serial_generic_process_read_urb(). > > The driver will attempt to load firmware from userspace and compare > the available version and the running version. If the available > version is newer, it will be download into RAM of the device and > started. This is optional and the driver appears to work O.K. with > older firmware in the devices ROM. > > This driver is based on the MOXA driver and retains MOXAs copyright. > > Signed-off-by: Andrew Lunn <andrew@xxxxxxx> > > --- > > v1->v2 > > Remove debug module parameter. > Add missing \n to dev_dbg() strings. > Consistent dev_dbg("%s - <message in lowercase>\n", __func__); > Don't log failed allocations. > Remove defensive checks for NULL mx_port. > Use snprintf() instead of sprintf(). > Use port->icount and usb_serial_generic_get_icount(). > Break firmware download into smaller functions. > Don't DMA to/from buffers on the stack. > Use more of the generic write functions. > Make use of port_probe() and port_remove(). > Indent the device structure. > Minor cleanups in mxuport_close() > __u8 -> u8, __u16 -> u16. > remove mxuport_wait_until_sent() and implemented mxuport_tx_empty() > Remove mxuport_write_room() and use the generic equivelent. > Remove unneeded struct mx_uart_config members > Make use of generic functions for receiving data and events. > Name mxport consistently. > Use usb_serial_generic_tiocmiwait() > Let line discipline handle TCFLSH ioctl. > Import contents of mxuport.h into mxuport.c > Use shifts and ORs to create version number. > Use port_number, not minor > > Clarify the meaning of interface in mx_uart_config. > Remove buffer from mxuport_send_async_ctrl_urb(). > No need to multiply USB_CTRL_SET_TIMEOUT by HZ. > Simplify buffer allocation and free. > Swap (un)throttle to synchronous interface, remove async code. > Pass usb_serial to mxuport_[recv|send]_ctrl_urb() > Add missing {} on if else statement. > Use unsigned char type, remove casts. > Replace constants with defines. > Add error handling when disabling HW flow control. > Verify port is open before passing it recieved data > Verify contents of received data & events. > Remove broken support for alternative baud rates. > Fix B0 and modem line handling. Remove uart_cfg structure. > Remove hold_reason, which was set, but never used. > s/is/if/ > Move parts of port open into port probe. > Remove mxport->port and pass usb_serial_port instead. > Remove mxport->flags which is no longer used. > Allocate buffers before starting firmware download. > Remove redundent "detected" debug message. > Use devm_kzalloc() to simply memory handling. > Remove repeated debug message when sending break. > Implement mxuport_dtr_rts(). > Add locking around mcr_state. > Remove unused lsr_state from mxuport_port. > Support 485 via official IOCTL and sysfs. > Use usleep_range() instread of mdelay(). > Simplify the comments. > Use port0 as a template when setting up bulk out endpoints. > Set bulk_in_size for unused endpoints to 0. > Rebase to v3.12-rc2 > > v3->v4 > Remove support for rs485 - No hardware to test it on. > Fix formatting of multi line comments. > Use HEADER_SIZE instead of hard coded 4. > Use EIO instread of ECOMM. > Drop use of spinlocks on mxuport_{un}throttle(). > Remove unneeded parenthesis. > Split mxuport_process_read_urb_event() into a number of functions. > Check for zero bytes of data. > Don't needlessly initialize variables. > Fold mxuport_port_init() into mxuport_port_open() > Correctly handle the on parameter in mxuport_dtr_rts(). > Drop mxuport_get_serial_info and the now empty ioctl handler. > Splitup mxuport_set_termios() into smaller functions > Make use of C_* macros for cflags. > Terminate the firmware download on error. > Remove some unneeded dev_dbg()'s. > Let usb-serial core fill .write function in usb_serial_driver. > Change usb_serial_driver.name to match filename. > Move tiocmget() and tiocmset() next to each other. > Only one declaration per line. > Remove unused udev > Convert a few dev_dbg() to dev_err() for real errors. > Remove unneeded usb_set_serial_data(serial, NULL). > Use RQ_VENDOR_SET_RX_HOST_EN in mxuport_close() > Replace the spinlock protecting msr and lsr state with a mutex. > Add a custom resume function. > > v4->v5 > > Rename C_MSPAR(tty) to C_CMSPAR(tty) > Add the C_CMSPAR(tty) macro patch to the patchset. > Fix name of module in Kconfig. > Remove unneeded include files. > Add missing tab in #define. > Use data & size rather than ch & rcv_len. > unsigned char -> u8 > unsigned short -> u16 > unsigned long -> u32 > Use u8[4] for event, not u8 *. > Exit out of mxuport_msr_event() when rcv_msr_event is 0. > Remove empty lines after breaks. > Move bits of code into else clause. > Drop dev_dbg() statements in mxuport_set_termios > Don't set the hardware to 0 baud, use 9600 instead. > Express number of ports as a quirk. > Pass firmware version around as a u32. > mxuport_break_ctl, drop err, and rely on mxuport_send_ctrl_urb > reporting problems. > Protect mcr_state with a spinlock. > Implement mxuport_dtr_rts() using RQ_VENDOR_SET_MCR. > Add helpers for manipulating RTS and DTS. > Try again with B0, RTS/DTS, HW flow control. Thanks for the update. One more iteration (or two) and I think we could be done. :) Some comments below. > --- > drivers/usb/serial/Kconfig | 29 + > drivers/usb/serial/Makefile | 1 + > drivers/usb/serial/mxuport.c | 1440 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 1470 insertions(+) > create mode 100644 drivers/usb/serial/mxuport.c > > diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig > index ddb9c51f2c99..f023e3221b89 100644 > --- a/drivers/usb/serial/Kconfig > +++ b/drivers/usb/serial/Kconfig > @@ -472,6 +472,35 @@ config USB_SERIAL_MOS7840 > To compile this driver as a module, choose M here: the > module will be called mos7840. If unsure, choose N. > > +config USB_SERIAL_MOXA_UPORT > + tristate "USB Moxa UPORT USB Serial Driver" > + ---help--- > + Say Y here if you want to use a MOXA UPort Serial hub. > + > + This driver supports: > + > + [2 Port] > + - UPort 1250 : 2 Port RS-232/422/485 USB to Serial Hub > + - UPort 1250I : 2 Port RS-232/422/485 USB to Serial Hub with > + Isolation > + > + [4 Port] > + - UPort 1410 : 4 Port RS-232 USB to Serial Hub > + - UPort 1450 : 4 Port RS-232/422/485 USB to Serial Hub > + - UPort 1450I : 4 Port RS-232/422/485 USB to Serial Hub with > + Isolation > + > + [8 Port] > + - UPort 1610-8 : 8 Port RS-232 USB to Serial Hub > + - UPort 1650-8 : 8 Port RS-232/422/485 USB to Serial Hub > + > + [16 Port] > + - UPort 1610-16 : 16 Port RS-232 USB to Serial Hub > + - UPort 1650-16 : 16 Port RS-232/422/485 USB to Serial Hub > + > + To compile this driver as a module, choose M here: the > + module will be called mxuport. > + > config USB_SERIAL_NAVMAN > tristate "USB Navman GPS device" > help > diff --git a/drivers/usb/serial/Makefile b/drivers/usb/serial/Makefile > index 42670f0b5bc0..9b5d6b346737 100644 > --- a/drivers/usb/serial/Makefile > +++ b/drivers/usb/serial/Makefile > @@ -37,6 +37,7 @@ obj-$(CONFIG_USB_SERIAL_MCT_U232) += mct_u232.o > obj-$(CONFIG_USB_SERIAL_METRO) += metro-usb.o > obj-$(CONFIG_USB_SERIAL_MOS7720) += mos7720.o > obj-$(CONFIG_USB_SERIAL_MOS7840) += mos7840.o > +obj-$(CONFIG_USB_SERIAL_MOXA_UPORT) += mxuport.o > obj-$(CONFIG_USB_SERIAL_NAVMAN) += navman.o > obj-$(CONFIG_USB_SERIAL_OMNINET) += omninet.o > obj-$(CONFIG_USB_SERIAL_OPTICON) += opticon.o > diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c > new file mode 100644 > index 000000000000..c444261eda1f > --- /dev/null > +++ b/drivers/usb/serial/mxuport.c > @@ -0,0 +1,1440 @@ > +/* > + * mxuport.c - MOXA UPort series driver > + * > + * Copyright (c) 2006 Moxa Technologies Co., Ltd. > + * Copyright (c) 2013 Andrew Lunn <andrew@xxxxxxx> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * Supports the following Moxa USB to serial converters: > + * 2 ports : UPort 1250, UPort 1250I > + * 4 ports : UPort 1410, UPort 1450, UPort 1450I > + * 8 ports : UPort 1610-8, UPort 1650-8 > + * 16 ports : UPort 1610-16, UPort 1650-16 > + * > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/firmware.h> > +#include <linux/init.h> > +#include <linux/jiffies.h> > +#include <linux/serial.h> > +#include <linux/serial_reg.h> > +#include <linux/slab.h> > +#include <linux/tty.h> > +#include <linux/tty_driver.h> > +#include <linux/tty_flip.h> > +#include <linux/uaccess.h> > +#include <linux/usb.h> > +#include <linux/usb/serial.h> > +#include <asm/unaligned.h> > + > +/* Definitions for driver info */ > +#define DRIVER_AUTHOR \ > + "<support@xxxxxxxx>" \ > + "Andrew Lunn <andrew@xxxxxxx>" > + > +/* Definitions for the vendor ID and device ID */ > +#define MX_USBSERIAL_VID 0x110A > +#define MX_UPORT1250_PID 0x1250 > +#define MX_UPORT1251_PID 0x1251 > +#define MX_UPORT1410_PID 0x1410 > +#define MX_UPORT1450_PID 0x1450 > +#define MX_UPORT1451_PID 0x1451 > +#define MX_UPORT1618_PID 0x1618 > +#define MX_UPORT1658_PID 0x1658 > +#define MX_UPORT1613_PID 0x1613 > +#define MX_UPORT1653_PID 0x1653 > + > +/* Definitions for USB info */ > +#define HEADER_SIZE 4 > +#define MAX_EVENT_LENGTH 8 > +#define DOWN_BLOCK_SIZE 64 > + > +/* Definitions for firmware info */ > +#define VER_ADDR_1 0x20 > +#define VER_ADDR_2 0x24 > +#define VER_ADDR_3 0x28 > + > +/* Definitions for USB vendor request */ > +#define RQ_VENDOR_NONE 0x00 > +#define RQ_VENDOR_SET_BAUD 0x01 /* Set baud rate */ > +#define RQ_VENDOR_SET_LINE 0x02 /* Set line status */ > +#define RQ_VENDOR_SET_CHARS 0x03 /* Set Xon/Xoff chars */ > +#define RQ_VENDOR_SET_RTS 0x04 /* Set RTS */ > +#define RQ_VENDOR_SET_DTR 0x05 /* Set DTR */ > +#define RQ_VENDOR_SET_XONXOFF 0x06 /* Set auto Xon/Xoff */ > +#define RQ_VENDOR_SET_RX_HOST_EN 0x07 /* Set RX host enable */ > +#define RQ_VENDOR_SET_OPEN 0x08 /* Set open/close port */ > +#define RQ_VENDOR_PURGE 0x09 /* Purge Rx/Tx buffer */ > +#define RQ_VENDOR_SET_MCR 0x0A /* Set MCR register */ > +#define RQ_VENDOR_SET_BREAK 0x0B /* Set Break signal */ > + > +#define RQ_VENDOR_START_FW_DOWN 0x0C /* Start firmware download */ > +#define RQ_VENDOR_STOP_FW_DOWN 0x0D /* Stop firmware download */ > +#define RQ_VENDOR_QUERY_FW_READY 0x0E /* Query if new firmware ready */ > + > +#define RQ_VENDOR_SET_FIFO_DISABLE 0x0F /* Set fifo disable */ > +#define RQ_VENDOR_SET_INTERFACE 0x10 /* Set interface */ > +#define RQ_VENDOR_SET_HIGH_PERFOR 0x11 /* Set hi-performance */ > + > +#define RQ_VENDOR_ERASE_BLOCK 0x12 /* Erase flash block */ > +#define RQ_VENDOR_WRITE_PAGE 0x13 /* Write flash page */ > +#define RQ_VENDOR_PREPARE_WRITE 0x14 /* Prepare write flash */ > +#define RQ_VENDOR_CONFIRM_WRITE 0x15 /* Confirm write flash */ > +#define RQ_VENDOR_LOCATE 0x16 /* Locate the device */ > + > +#define RQ_VENDOR_START_ROM_DOWN 0x17 /* Start firmware download */ > +#define RQ_VENDOR_ROM_DATA 0x18 /* Rom file data */ > +#define RQ_VENDOR_STOP_ROM_DOWN 0x19 /* Stop firmware download */ > +#define RQ_VENDOR_FW_DATA 0x20 /* Firmware data */ > + > +#define RQ_VENDOR_RESET_DEVICE 0x23 /* Try to reset the device */ > +#define RQ_VENDOR_QUERY_FW_CONFIG 0x24 > + > +#define RQ_VENDOR_GET_VERSION 0x81 /* Get firmware version */ > +#define RQ_VENDOR_GET_PAGE 0x82 /* Read flash page */ > +#define RQ_VENDOR_GET_ROM_PROC 0x83 /* Get ROM process state */ > + > +#define RQ_VENDOR_GET_INQUEUE 0x84 /* Data in input buffer */ > +#define RQ_VENDOR_GET_OUTQUEUE 0x85 /* Data in output buffer */ > + > +#define RQ_VENDOR_GET_MSR 0x86 /* Get modem status register */ > + > +/* Definitions for UPort event type */ > +#define UPORT_EVENT_NONE 0 /* None */ > +#define UPORT_EVNET_TXBUF_THRESHOLD 1 /* Tx buffer threshold */ > +#define UPORT_EVNET_SEND_NEXT 2 /* Send next */ > +#define UPORT_EVNET_MSR 3 /* Modem status */ > +#define UPORT_EVNET_LSR 4 /* Line status */ > +#define UPORT_EVNET_MCR 5 /* Modem control */ s/EVNET/EVENT/ ? > + > +/* Definitions for serial event type */ > +#define SERIAL_EV_CTS 0x0008 /* CTS changed state */ > +#define SERIAL_EV_DSR 0x0010 /* DSR changed state */ > +#define SERIAL_EV_RLSD 0x0020 /* RLSD changed state */ > + > +/* Definitions for modem control event type */ > +#define SERIAL_EV_XOFF 0x40 /* XOFF received */ > + > +/* Definitions for line control of communication */ > +#define MX_WORDLENGTH_5 5 > +#define MX_WORDLENGTH_6 6 > +#define MX_WORDLENGTH_7 7 > +#define MX_WORDLENGTH_8 8 > + > +#define MX_PARITY_NONE 0 > +#define MX_PARITY_ODD 1 > +#define MX_PARITY_EVEN 2 > +#define MX_PARITY_MARK 3 > +#define MX_PARITY_SPACE 4 > + > +#define MX_STOP_BITS_1 0 > +#define MX_STOP_BITS_1_5 1 > +#define MX_STOP_BITS_2 2 > + > +#define MX_NO_FLOWCTRL 0x0 > +#define MX_HW_FLOWCTRL 0x1 > +#define MX_XON_FLOWCTRL 0x2 > +#define MX_XOFF_FLOWCTRL 0x4 > + > +#define MX_HW_FLOWCTRL_RTS_DISABLE 0x0 > +#define MX_HW_FLOWCTRL_RTS_ENABLE 0x1 > +#define MX_HW_FLOWCTRL_ENABLE 0x2 > + > +#define MX_INT_RS232 0 > +#define MX_INT_2W_RS485 1 > +#define MX_INT_RS422 2 > +#define MX_INT_4W_RS485 3 > + > +/* Definitions for holding reason */ > +#define MX_WAIT_FOR_CTS 0x0001 > +#define MX_WAIT_FOR_DSR 0x0002 > +#define MX_WAIT_FOR_DCD 0x0004 > +#define MX_WAIT_FOR_XON 0x0008 > +#define MX_WAIT_FOR_START_TX 0x0010 > +#define MX_WAIT_FOR_UNTHROTTLE 0x0020 > +#define MX_WAIT_FOR_LOW_WATER 0x0040 > +#define MX_WAIT_FOR_SEND_NEXT 0x0080 > + > +#define MX_UPORT_2_PORT BIT(0) > +#define MX_UPORT_4_PORT BIT(1) > +#define MX_UPORT_8_PORT BIT(2) > +#define MX_UPORT_16_PORT BIT(3) > + > +/* This structure holds all of the local port information */ > +struct mxuport_port { > + u8 mcr_state; /* Last MCR state */ > + u8 msr_state; /* Last MSR state */ > + struct mutex mutex; /* Protects mcr_state */ > + spinlock_t spinlock; /* Protects msr_state */ > +}; > + > +/* Table of devices that work with this driver */ > +static struct usb_device_id mxuport_idtable[] = { > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1250_PID), > + .driver_info = MX_UPORT_2_PORT }, > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1251_PID), > + .driver_info = MX_UPORT_2_PORT }, > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1410_PID), > + .driver_info = MX_UPORT_4_PORT }, > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1450_PID), > + .driver_info = MX_UPORT_4_PORT }, > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1451_PID), > + .driver_info = MX_UPORT_4_PORT }, > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1618_PID), > + .driver_info = MX_UPORT_8_PORT }, > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1658_PID), > + .driver_info = MX_UPORT_8_PORT }, > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1613_PID), > + .driver_info = MX_UPORT_16_PORT }, > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1653_PID), > + .driver_info = MX_UPORT_16_PORT }, > + {} /* Terminating entry */ > +}; > + > +MODULE_DEVICE_TABLE(usb, mxuport_idtable); > + > +/* > + * Add a four byte header containing the port number and the number of > + * bytes of data in the message. Return the number of bytes in the > + * buffer. > + */ > +static int mxuport_prepare_write_buffer(struct usb_serial_port *port, > + void *dest, size_t size) > +{ > + u8 *buf = dest; > + int count; > + > + count = kfifo_out_locked(&port->write_fifo, buf + HEADER_SIZE, > + size - HEADER_SIZE, > + &port->lock); > + > + put_unaligned_be16(port->port_number, buf); > + put_unaligned_be16(count, buf + 2); > + > + dev_dbg(&port->dev, "%s - size %zd count %d\n", __func__, > + size, count); > + > + return count + HEADER_SIZE; > +} > + > +/* Read the given buffer in from the control pipe. */ > +static int mxuport_recv_ctrl_urb(struct usb_serial *serial, > + u8 request, u16 value, u16 index, > + u8 *data, size_t size) > +{ > + int status; > + > + status = usb_control_msg(serial->dev, > + usb_rcvctrlpipe(serial->dev, 0), > + request, > + (USB_DIR_IN | USB_TYPE_VENDOR | > + USB_RECIP_DEVICE), value, index, > + data, size, > + USB_CTRL_GET_TIMEOUT); > + > + if (status < 0) { > + dev_dbg(&serial->interface->dev, > + "%s - recv usb_control_msg failed. (%d)\n", > + __func__, status); > + return status; > + } > + > + if (status != size) { > + dev_dbg(&serial->interface->dev, > + "%s - wanted to recv %zd, but only read %d\n", > + __func__, size, status); > + return -EIO; > + } > + > + return status; > +} > + > +/* Write the given buffer out to the control pipe. */ > +static int mxuport_send_ctrl_data_urb(struct usb_serial *serial, > + u8 request, > + u16 value, u16 index, > + u8 *data, size_t size) > +{ > + int status; > + > + status = usb_control_msg(serial->dev, > + usb_sndctrlpipe(serial->dev, 0), > + request, > + (USB_DIR_OUT | USB_TYPE_VENDOR | > + USB_RECIP_DEVICE), value, index, > + data, size, > + USB_CTRL_SET_TIMEOUT); > + if (status < 0) { > + dev_dbg(&serial->interface->dev, > + "%s - send usb_control_msg failed. (%d)\n", > + __func__, status); > + return status; > + } > + > + if (status != size) { > + dev_dbg(&serial->interface->dev, > + "%s - wanted to write %zd, but only wrote %d\n", > + __func__, size, status); > + return -EIO; > + } > + > + return status; > +} > + > +/* Send a vendor request without any data */ > +static int mxuport_send_ctrl_urb(struct usb_serial *serial, > + u8 request, u16 value, u16 index) > +{ > + return mxuport_send_ctrl_data_urb(serial, request, value, index, > + NULL, 0); > +} > + > +/* > + * mxuport_throttle - throttle function of driver > + * > + * This function is called by the tty driver when it wants to stop the > + * data being read from the port. Since all the data comers over one > + * bulk in endpoint, we cannot stop submitting urbs by setting > + * port->throttle. Instead tell the device to stop sending us data for > + * the port. > + */ > +static void mxuport_throttle(struct tty_struct *tty) > +{ > + struct usb_serial_port *port = tty->driver_data; > + struct usb_serial *serial = port->serial; > + > + dev_dbg(&port->dev, "%s\n", __func__); > + > + mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_RX_HOST_EN, > + 0, port->port_number); > +} > + > +/* > + * mxuport_unthrottle - unthrottle function of driver > + * > + * This function is called by the tty driver when it wants to resume > + * the data being read from the port. Tell the device it can resume > + * sending us received data from the port. > + */ > +static void mxuport_unthrottle(struct tty_struct *tty) > +{ > + > + struct usb_serial_port *port = tty->driver_data; > + struct usb_serial *serial = port->serial; > + > + dev_dbg(&port->dev, "%s\n", __func__); > + > + mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_RX_HOST_EN, > + 1, port->port_number); > +} > + > +/* > + * Processes one chunk of data received for a port. Mostly a copy of > + * usb_serial_generic_process_read_urb(). > + */ > +static void mxuport_process_read_urb_data(struct usb_serial_port *port, > + char *data, int size) > +{ > + int i; > + > + if (!port->port.console || !port->sysrq) { > + tty_insert_flip_string(&port->port, data, size); > + } else { > + for (i = 0; i < size; i++, data++) { > + if (!usb_serial_handle_sysrq_char(port, *data)) > + tty_insert_flip_char(&port->port, *data, > + TTY_NORMAL); > + } > + } > + tty_flip_buffer_push(&port->port); > +} > + > +static void mxuport_msr_event(struct usb_serial_port *port, > + struct mxuport_port *mxport, > + u8 buf[4]) > +{ > + u8 rcv_msr_hold = buf[2] & 0xF0; > + u16 rcv_msr_event = get_unaligned_be16(buf); > + unsigned long flags; > + > + if (rcv_msr_event == 0) > + return; > + > + /* Update hold reason */ > + spin_lock_irqsave(&mxport->spinlock, flags); > + > + dev_dbg(&port->dev, "%s - current MSR status = 0x%x\n", > + __func__, mxport->msr_state); > + > + if (rcv_msr_hold & UART_MSR_CTS) { > + mxport->msr_state |= UART_MSR_CTS; > + dev_dbg(&port->dev, "%s - CTS high\n", > + __func__); No need for line breaks in most of the dev_dbgs in this function anymore. > + } else { > + mxport->msr_state &= ~UART_MSR_CTS; > + dev_dbg(&port->dev, "%s - CTS low\n", > + __func__); > + } > + > + if (rcv_msr_hold & UART_MSR_DSR) { > + mxport->msr_state |= UART_MSR_DSR; > + dev_dbg(&port->dev, "%s - DSR high\n", > + __func__); > + } else { > + mxport->msr_state &= ~UART_MSR_DSR; > + dev_dbg(&port->dev, "%s - DSR low\n", > + __func__); > + } > + > + if (rcv_msr_hold & UART_MSR_DCD) { > + mxport->msr_state |= UART_MSR_DCD; > + dev_dbg(&port->dev, "%s - DCD high\n", > + __func__); > + } else { > + mxport->msr_state &= ~UART_MSR_DCD; > + dev_dbg(&port->dev, "%s - DCD low\n", > + __func__); > + } > + spin_unlock_irqrestore(&mxport->spinlock, flags); > + > + /* Update MSR status */ > + if (rcv_msr_event & > + (SERIAL_EV_CTS | SERIAL_EV_DSR | SERIAL_EV_RLSD)) { > + > + if (rcv_msr_event & SERIAL_EV_CTS) { > + port->icount.cts++; > + dev_dbg(&port->dev, "%s - CTS change\n", > + __func__); > + } > + > + if (rcv_msr_event & SERIAL_EV_DSR) { > + port->icount.dsr++; > + dev_dbg(&port->dev, "%s - DSR change\n", > + __func__); > + } > + > + if (rcv_msr_event & SERIAL_EV_RLSD) { > + port->icount.dcd++; > + dev_dbg(&port->dev, "%s - DCD change\n", > + __func__); > + } > + wake_up_interruptible(&port->port.delta_msr_wait); > + } > +} > + > +static void mxuport_lsr_event(struct usb_serial_port *port, > + u8 buf[4]) No line break needed anymore. > +{ > + u8 lsr_event = buf[2]; > + > + if (lsr_event & UART_LSR_BI) { > + port->icount.brk++; > + dev_dbg(&port->dev, "%s - break error\n", __func__); > + } > + > + if (lsr_event & UART_LSR_FE) { > + port->icount.frame++; > + dev_dbg(&port->dev, "%s - frame error\n", __func__); > + } > + > + if (lsr_event & UART_LSR_PE) { > + port->icount.parity++; > + dev_dbg(&port->dev, "%s - parity error\n", __func__); > + } > + > + if (lsr_event & UART_LSR_OE) { > + port->icount.overrun++; > + dev_dbg(&port->dev, "%s - overrun error\n", __func__); > + } > +} > + > +/* > + * When something interesting happens, modem control lines XON/XOFF > + * etc, the device sends an event. Process these events. > + */ > +static void mxuport_process_read_urb_event(struct usb_serial_port *port, > + u8 buf[4], > + u32 event) No line break? > +{ > + struct mxuport_port *mxport = usb_get_serial_port_data(port); > + > + dev_dbg(&port->dev, "%s - receive event : %d\n", Why not use %04x here? And no line break needed. > + __func__, event); > + > + switch (event) { > + case UPORT_EVNET_SEND_NEXT: > + /* > + * Sent as part of the flow control on device buffers. > + * Not currently used. > + */ > + break; > + case UPORT_EVNET_MSR: > + mxuport_msr_event(port, mxport, buf); > + break; > + case UPORT_EVNET_LSR: > + mxuport_lsr_event(port, buf); > + break; > + case UPORT_EVNET_MCR: > + /* > + * Event to indicate a change in XON/XOFF from the > + * peer. Currently not used. We just continue > + * sending the device data and it will buffer it if > + * needed. This event could be used for flow control > + * between the host and the device. > + */ > + break; > + default: > + break; > + } > +} > + > +/* > + * One URB can contain data for multiple ports. Demultiplex the data, > + * checking the port exists, is opened and the message is valid. > + */ > +static void mxuport_process_read_urb_demux_data(struct urb *urb) > +{ > + struct usb_serial_port *port = urb->context; > + struct usb_serial *serial = port->serial; > + u8 *data = urb->transfer_buffer; > + u8 *end = data + urb->actual_length; > + struct usb_serial_port *demux_port; > + u8 *ch; > + u16 rcv_port; > + u16 rcv_len; > + > + while (data < end) { > + if (data + HEADER_SIZE > end) { > + dev_warn(&port->dev, "%s - message with short header\n", > + __func__); > + return; > + } > + > + rcv_port = get_unaligned_be16(data); > + if (rcv_port >= serial->num_ports) { > + dev_warn(&port->dev, "%s - message for invalid port\n", > + __func__); > + return; > + } > + > + demux_port = serial->port[rcv_port]; > + rcv_len = get_unaligned_be16(data + 2); > + if (!rcv_len || data + HEADER_SIZE + rcv_len > end) { > + dev_warn(&port->dev, "%s - short data\n", > + __func__); > + return; > + } > + > + if (test_bit(ASYNCB_INITIALIZED, &demux_port->port.flags)) { > + ch = data + HEADER_SIZE; > + mxuport_process_read_urb_data(demux_port, ch, rcv_len); > + } else { > + dev_dbg(&demux_port->dev, "%s - data for closed port\n", > + __func__); > + } > + data += HEADER_SIZE + rcv_len; > + } > +} > + > +/* > + * One URB can contain events for multiple ports. Demultiplex the event, > + * checking the port exists, and is opened. > + */ > +static void mxuport_process_read_urb_demux_event(struct urb *urb) > +{ > + struct usb_serial_port *port = urb->context; > + struct usb_serial *serial = port->serial; > + u8 *data = urb->transfer_buffer; > + u8 *end = data + urb->actual_length; > + struct usb_serial_port *demux_port; > + u8 *ch; > + u16 rcv_port; > + u16 rcv_event; > + > + while (data < end) { > + if (data + MAX_EVENT_LENGTH > end) { > + dev_warn(&port->dev, "%s - message with short event\n", > + __func__); > + return; > + } > + > + rcv_port = get_unaligned_be16(data); > + if (rcv_port >= serial->num_ports) { > + dev_warn(&port->dev, "%s - message for invalid port\n", > + __func__); > + return; > + } > + > + demux_port = serial->port[rcv_port]; > + if (test_bit(ASYNCB_INITIALIZED, &demux_port->port.flags)) { > + ch = data + HEADER_SIZE; > + rcv_event = get_unaligned_be16(data + 2); > + mxuport_process_read_urb_event(demux_port, ch, > + rcv_event); > + } else { > + dev_dbg(&demux_port->dev, > + "%s - event for closed port\n", __func__); > + } > + data += MAX_EVENT_LENGTH; > + } > +} > + > +/* > + * This is called when we have received data on the bulk in > + * endpoint. Depending on which port it was received on, it can > + * contain serial data or events. > + */ > +static void mxuport_process_read_urb(struct urb *urb) > +{ > + struct usb_serial_port *port = urb->context; > + struct usb_serial *serial = port->serial; > + > + if (port == serial->port[0]) > + mxuport_process_read_urb_demux_data(urb); > + > + if (port == serial->port[1]) > + mxuport_process_read_urb_demux_event(urb); > +} > + > +/* > + * Ask the device how many bytes it has queued to be sent out. If > + * there are none, return true. > + */ > +static bool mxuport_tx_empty(struct usb_serial_port *port) > +{ > + struct usb_serial *serial = port->serial; > + bool is_empty = true; > + u32 txlen; > + u8 *len_buf; > + int err; > + > + len_buf = kzalloc(4, GFP_KERNEL); > + if (!len_buf) > + goto out; > + > + err = mxuport_recv_ctrl_urb(serial, > + RQ_VENDOR_GET_OUTQUEUE, > + 0, port->port_number, > + len_buf, 4); > + if (err < 0) > + goto out; > + > + txlen = get_unaligned_be32(len_buf); > + dev_dbg(&port->dev, "%s - tx len = %d\n", __func__, txlen); Use %u? > + > + if (txlen != 0) > + is_empty = false; > + > +out: > + kfree(len_buf); > + return is_empty; > +} > + > +static int mxuport_set_mcr(struct usb_serial_port *port, u8 mcr_state) > +{ > + int err; > + struct usb_serial *serial = port->serial; > + > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_MCR, > + mcr_state, port->port_number); > + > + if (err) > + dev_dbg(&port->dev, "%s - fail to change MCR\n", __func__); > + > + return err; > +} > + > +static int mxuport_dtr(struct usb_serial_port *port, int on) > +{ > + struct mxuport_port *mxport = usb_get_serial_port_data(port); > + int err; > + > + mutex_lock(&mxport->mutex); > + > + if (on) > + mxport->mcr_state |= UART_MCR_DTR; > + else > + mxport->mcr_state &= ~UART_MCR_DTR; > + > + err = mxuport_set_mcr(port, mxport->mcr_state); > + > + mutex_unlock(&mxport->mutex); > + > + return err; > +} You should probably use SET_DTR (and rename the function mxuport_set_dtr). > + > +static int mxuport_rts(struct usb_serial_port *port, int on) > +{ > + struct mxuport_port *mxport = usb_get_serial_port_data(port); > + int err; > + > + mutex_lock(&mxport->mutex); > + > + if (on) > + mxport->mcr_state |= UART_MCR_RTS; > + else > + mxport->mcr_state &= ~UART_MCR_RTS; > + > + err = mxuport_set_mcr(port, mxport->mcr_state); > + > + mutex_unlock(&mxport->mutex); > + > + return err; > +} I think you need to use SET_RTS and pass in a tri-state argument (e.g. u8) for off, on, and hw-flow_enable (auto). More details below. > + > +static void mxuport_dtr_rts(struct usb_serial_port *port, int on) > +{ > + struct mxuport_port *mxport = usb_get_serial_port_data(port); > + > + mutex_lock(&mxport->mutex); > + > + if (on) > + mxport->mcr_state |= (UART_MCR_RTS | UART_MCR_DTR); > + else > + mxport->mcr_state &= ~(UART_MCR_RTS | UART_MCR_DTR); > + > + mxuport_set_mcr(port, mxport->mcr_state); > + > + mutex_unlock(&mxport->mutex); > +} Nice and clean. :) > + > +static int mxuport_tiocmset(struct tty_struct *tty, unsigned int set, > + unsigned int clear) > +{ > + struct usb_serial_port *port = tty->driver_data; > + int err = 0; > + > + dev_dbg(&port->dev, "%s\n", __func__); > + > + /* Set MCR status */ > + if (set & TIOCM_RTS) { > + err = mxuport_rts(port, 1); > + if (err) > + goto out; > + } > + > + if (set & TIOCM_DTR) { > + err = mxuport_dtr(port, 1); > + if (err) > + goto out; > + } > + > + /* Clear MCR status */ > + if (clear & TIOCM_RTS) { > + err = mxuport_rts(port, 0); > + if (err) > + goto out; > + } > + > + if (clear & TIOCM_DTR) > + err = mxuport_dtr(port, 0); > + > +out: > + return err; > +} I think you should do the requested change in one request rather than use the new helper functions. Grab the mutex once, update a copy of mxport->mcr_state depending on which bits are set, and then do one call to SET_MCR (mxuport_set_mcr) and if it succeeds you update the private mcr_state. That is basically what you did before but use one SET_MCR instead of multiple SET_RTS and SET_DTS. > + > +static int mxuport_tiocmget(struct tty_struct *tty) > +{ > + struct mxuport_port *mxport; > + struct usb_serial_port *port = tty->driver_data; > + unsigned int result = 0; No need to initialise. > + unsigned long flags; > + unsigned int msr; > + unsigned int mcr; > + > + mxport = usb_get_serial_port_data(port); > + > + mutex_lock(&mxport->mutex); > + spin_lock_irqsave(&mxport->spinlock, flags); > + > + msr = mxport->msr_state; > + mcr = mxport->mcr_state; > + > + spin_unlock_irqrestore(&mxport->spinlock, flags); > + mutex_unlock(&mxport->mutex); > + > + result = (((mcr & UART_MCR_DTR) ? TIOCM_DTR : 0) | /* 0x002 */ > + ((mcr & UART_MCR_RTS) ? TIOCM_RTS : 0) | /* 0x004 */ > + ((msr & UART_MSR_CTS) ? TIOCM_CTS : 0) | /* 0x020 */ > + ((msr & UART_MSR_DCD) ? TIOCM_CAR : 0) | /* 0x040 */ > + ((msr & UART_MSR_RI) ? TIOCM_RI : 0) | /* 0x080 */ > + ((msr & UART_MSR_DSR) ? TIOCM_DSR : 0)); /* 0x100 */ > + > + dev_dbg(&port->dev, "%s - MSR return 0x%x\n", __func__, result); > + > + return result; > +} > + > +static int mxuport_set_termios_flow(struct tty_struct *tty, > + struct usb_serial_port *port, > + struct usb_serial *serial, > + char *buf) Pass the buffer as u8[4] (or u8[2]). Or perhaps simply doing a new buffer allocation for the RQ_VENDOR_SET_CHARS if sw-flow is enabled is better now that flow-control handling lives in its own function? > +{ > + u32 flow_ctrl = 0; > + char xon = START_CHAR(tty); > + char xoff = STOP_CHAR(tty); Use unsigned char here and drop the casts below. > + int err; > + int enable; > + > + /* S/W flow control settings */ > + if (I_IXOFF(tty) || I_IXON(tty)) { > + buf[0] = (u8)xon; > + buf[1] = (u8)xoff; Drop casts here. > + > + err = mxuport_send_ctrl_data_urb(serial, RQ_VENDOR_SET_CHARS, > + 0, port->port_number, > + buf, 2); > + > + if (err != 2) > + goto out; > + } > + > + /* If we are implementing outbound XON/XOFF */ > + if (I_IXON(tty)) { > + flow_ctrl |= MX_XON_FLOWCTRL; > + dev_dbg(&port->dev, > + "%s - outbound XON/XOFF is enabled, XON = 0x%2x," > + "XOFF = 0x%2x\n", __func__, xon, xoff); > + } else { > + flow_ctrl &= ~MX_XON_FLOWCTRL; > + dev_dbg(&port->dev, "%s - outbound XON/XOFF is disabled\n", > + __func__); > + } > + > + /* If we are implementing inbound XON/XOFF */ > + if (I_IXOFF(tty)) { > + flow_ctrl |= MX_XOFF_FLOWCTRL; > + dev_dbg(&port->dev, > + "%s - inbound XON/XOFF is enabled, XON = 0x%2x," > + " XOFF = 0x%2x\n", __func__, xon, xoff); > + } else { > + flow_ctrl &= ~MX_XOFF_FLOWCTRL; > + dev_dbg(&port->dev, "%s - inbound XON/XOFF is disabled\n", > + __func__); > + } > + > + if (flow_ctrl & (MX_XON_FLOWCTRL | MX_XOFF_FLOWCTRL)) > + enable = 1; > + else > + enable = 0; > + > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_XONXOFF, > + enable, port->port_number); > + if (err) > + goto out; Ok, it seems the above could simplified quite a bit. The chip seems to only have two settings for sw-flow (on or off) so there's no need to manipulate the flow_control mask (which could be dropped). So you'd end up with something like if (I_IXOFF(tty) || I_IXON(tty)) { ... mxuport_send_ctrl_data_urb(serial, RQ_VENDOR_SET_CHARS, ... dev_dbg(&port->dev, "XON = %02x, XOFF = %02x\n", ... sw_flow_enable = 1; } else { sw_flow_enable = 0; } err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_XONXOFF, sw_flow_enable, port->port_number); > + > + /* H/W flow control settings */ > + if (C_BAUD(tty) && C_CRTSCTS(tty)) > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_RTS, > + MX_HW_FLOWCTRL_ENABLE, > + port->port_number); > + else > + if (C_BAUD(tty)) > + err = mxuport_rts(port, 1); > + else > + err = mxuport_rts(port, 0); Hmm. This isn't quite right yet. If you move the B0 handling from set_termios to this function as I suggested, it would be easier to see their dependencies. You want something along the following lines: #define MX_RTS_OFF 0 #define MX_RTS_ON 1 #define MX_RTS_HW 2 #define MX_RTS_NO_CHANGE 3 unsigned rts = MX_RTS_NO_CHANGE; /* H/W flow control settings */ if (!old_termios || C_CRTCTS(tty) != old_termios->c_cflag & C_CRTSCTS) { if (C_CRTSCTS(tty)) rts = MX_RTS_HW; else rts = MX_RTS_ON; } if (C_BAUD(tty)) { if (old_termios && (old_termios->c_cflag & CBAUD) == B0) { /* Raise DTR and RTS */ if (C_CRTSCTS(tty)) rts = MX_RTS_HW; else rts = MX_RTS_ON; mxuport_set_dtr(port, 1); } } else { /* Drop DTR and RTS */ rts = MX_RTS_OFF; mxuport_set_dtr(port, 0); } if (rts != MX_RTS_NO_CHANGE) err = mxport_set_rts(port, rts); That ought to pretty much cover it, right? Note that mxport_set_rts should only update last_mcr if rts != MX_RTS_HW. > + > +out: > + return err; > +} > + > +static void mxuport_set_termios(struct tty_struct *tty, > + struct usb_serial_port *port, > + struct ktermios *old_termios) > +{ > + struct ktermios *termios = &tty->termios; > + struct usb_serial *serial = port->serial; > + unsigned int iflag; > + u8 *buf; > + u8 data_bits; > + u8 stop_bits; > + u8 parity; > + int baud; > + int err; > + > + dev_dbg(&port->dev, "%s\n", __func__); > + > + /* Check that they really want us to change something */ > + iflag = 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; > + } > + > + buf = kmalloc(4, GFP_KERNEL); > + if (!buf) > + return; > + > + /* Set data bit of termios */ > + switch (C_CSIZE(tty)) { > + case CS5: > + data_bits = MX_WORDLENGTH_5; > + break; > + case CS6: > + data_bits = MX_WORDLENGTH_6; > + break; > + case CS7: > + data_bits = MX_WORDLENGTH_7; > + break; > + case CS8: > + default: > + data_bits = MX_WORDLENGTH_8; > + break; > + } > + > + /* Set parity of termios */ > + if (C_PARENB(tty)) { > + if (C_CMSPAR(tty)) { > + if (C_PARODD(tty)) > + parity = MX_PARITY_MARK; > + else > + parity = MX_PARITY_SPACE; > + } else if (C_PARODD(tty)) { > + parity = MX_PARITY_ODD; > + } else { > + parity = MX_PARITY_EVEN; > + } > + } else { > + parity = MX_PARITY_NONE; > + } > + > + /* Set stop bit of termios */ > + if (C_CSTOPB(tty)) > + stop_bits = MX_STOP_BITS_2; > + else > + stop_bits = MX_STOP_BITS_1; > + > + /* Submit the vendor request */ > + buf[0] = data_bits; > + buf[1] = parity; > + buf[2] = stop_bits; > + buf[3] = 0; > + > + err = mxuport_send_ctrl_data_urb(serial, RQ_VENDOR_SET_LINE, > + 0, port->port_number, buf, 4); > + > + if (err != 4) > + goto out; > + > + if (C_BAUD(tty)) { > + if (old_termios && (old_termios->c_cflag & CBAUD) == B0) { > + if (!C_CRTSCTS(tty)) > + mxuport_rts(port, 1); > + mxuport_dtr(port, 1); > + } else { > + /* Drop DTR/RTS */ > + mxuport_dtr_rts(port, 0); > + } > + } So move this bit to set_termios_flow as discussed above. > + > + err = mxuport_set_termios_flow(tty, port, serial, buf); > + if (err) > + goto out; > + > + baud = tty_get_baud_rate(tty); > + if (!baud) > + baud = 9600; > + > + /* Submit the vendor request - Little Endian */ > + put_unaligned_le32(baud, buf); > + > + err = mxuport_send_ctrl_data_urb(serial, RQ_VENDOR_SET_BAUD, > + 0, port->port_number, > + buf, 4); > + > + if (err != 4) > + goto out; > + > + /* Dump serial settings */ > + dev_dbg(&port->dev, "baud_rate : %d\n", baud); > + dev_dbg(&port->dev, "data_bits : %d\n", data_bits); > + dev_dbg(&port->dev, "parity : %d\n", parity); > + dev_dbg(&port->dev, "stop_bits : %d\n", stop_bits); > + > +out: > + kfree(buf); > +} > + > +/* > + * Determine how many ports this device has dynamically. It will be > + * called after the probe() callback is called, but before attach(). > + */ > +static int mxuport_calc_num_ports(struct usb_serial *serial) > +{ > + u32 quirks = (u32) usb_get_serial_data(serial); You have to cast via (unsigned long) when using u32 (for 64-bit machines), so better to just stick to unsigned long. Minor knit: perhaps we should reserve the word "quirks" for actual quirks (odd driver behaviour) and use "features" here instead. Then this would be: unsigned long features = (unsigned long)usb_get_serial_data(serial); > + > + if (quirks & MX_UPORT_2_PORT) > + return 2; > + Drop the empty line. > + if (quirks & MX_UPORT_4_PORT) > + return 4; > + if (quirks & MX_UPORT_8_PORT) > + return 8; > + if (quirks & MX_UPORT_16_PORT) > + return 16; And add one here instead. > + return 0; > +} > + > +/* Get the version of the firmware currently running. */ > +static int mxuport_get_fw_version(struct usb_serial *serial, u32 *version) > +{ > + u8 *ver_buf; > + int err; > + > + ver_buf = kzalloc(4, GFP_KERNEL); > + if (!ver_buf) > + return -ENOMEM; > + > + /* Send vendor request - Get firmware version from SDRAM */ > + err = mxuport_recv_ctrl_urb(serial, RQ_VENDOR_GET_VERSION, 0, 0, > + ver_buf, 4); > + if (err != 4) > + goto out; > + > + *version = (ver_buf[0] << 16) | (ver_buf[1] << 8) | ver_buf[2]; > + err = 0; > +out: > + kfree(ver_buf); > + return err; > +} > + > +/* Given a firmware blob, download it to the device. */ > +static int mxuport_download_fw(struct usb_serial *serial, > + const struct firmware *fw_p) > +{ > + u8 *fw_buf = NULL; > + size_t txlen; > + size_t fwidx; > + int err; > + > + fw_buf = kmalloc(DOWN_BLOCK_SIZE, GFP_KERNEL); > + if (fw_buf == NULL) > + return -ENOMEM; > + > + dev_dbg(&serial->interface->dev, "Starting download firmware ...\n"); > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_START_FW_DOWN, 0, 0); > + if (err) > + goto out; > + > + fwidx = 0; > + do { > + txlen = min_t(size_t, (fw_p->size - fwidx), DOWN_BLOCK_SIZE); > + > + memcpy(fw_buf, &fw_p->data[fwidx], txlen); > + err = mxuport_send_ctrl_data_urb(serial, RQ_VENDOR_FW_DATA, > + 0, 0, fw_buf, txlen); > + if (err != txlen) { > + mxuport_send_ctrl_urb(serial, RQ_VENDOR_STOP_FW_DOWN, > + 0, 0); > + goto out; > + } > + > + fwidx += txlen; > + usleep_range(1000, 2000); > + > + } while (fwidx < fw_p->size); > + > + msleep(1000); > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_STOP_FW_DOWN, 0, 0); > + if (err) > + goto out; > + > + msleep(1000); > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_QUERY_FW_READY, 0, 0); > + > +out: > + kfree(fw_buf); > + return err; > +} > + > +static int mxuport_probe(struct usb_serial *serial, > + const struct usb_device_id *id) > +{ > + u16 productid = le16_to_cpu(serial->dev->descriptor.idProduct); > + const struct firmware *fw_p = NULL; > + u32 version; > + int local_ver; > + char buf[32]; > + int err; > + > + /* Load our firmware */ > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_QUERY_FW_CONFIG, 0, 0); > + if (err) { > + mxuport_send_ctrl_urb(serial, RQ_VENDOR_RESET_DEVICE, 0, 0); > + return err; > + } > + > + err = mxuport_get_fw_version(serial, &version); > + if (err < 0) > + goto out; > + > + dev_dbg(&serial->interface->dev, "Device firmware version v%x.%x.%x\n", > + (version & 0xff0000) >> 16, > + (version & 0xff00) >> 8, > + (version & 0xff)); > + > + snprintf(buf, sizeof(buf) - 1, "moxa/moxa-%04x.fw", productid); > + > + err = request_firmware(&fw_p, buf, &serial->interface->dev); > + if (err) { > + dev_warn(&serial->interface->dev, "Firmware %s not found\n", > + buf); > + > + /* Use the firmware already in the device */ > + err = 0; > + } else { > + local_ver = ((fw_p->data[VER_ADDR_1] << 16) | > + (fw_p->data[VER_ADDR_2] << 8) | > + fw_p->data[VER_ADDR_3]); > + dev_dbg(&serial->interface->dev, > + "Available firmware version v%x.%x.%x\n", > + fw_p->data[VER_ADDR_1], fw_p->data[VER_ADDR_2], > + fw_p->data[VER_ADDR_3]); > + if (local_ver > version) { > + err = mxuport_download_fw(serial, fw_p); > + if (err) > + goto out; > + err = mxuport_get_fw_version(serial, &version); > + if (err < 0) > + goto out; > + } > + } > + > + dev_info(&serial->interface->dev, > + "Using device firmware version v%x.%x.%x\n", > + (version & 0xff0000) >> 16, > + (version & 0xff00) >> 8, > + (version & 0xff)); > + Perhaps you could just add a comment here about storing driver info for later use. > + usb_set_serial_data(serial, (void *)id->driver_info); > +out: > + if (fw_p) > + release_firmware(fw_p); > + return err; > +} > + > + > +static int mxuport_port_probe(struct usb_serial_port *port) > +{ > + struct usb_serial *serial = port->serial; > + struct mxuport_port *mxport; > + int err; > + > + mxport = devm_kzalloc(&port->dev, sizeof(struct mxuport_port), > + GFP_KERNEL); > + if (!mxport) > + return -ENOMEM; > + > + mutex_init(&mxport->mutex); > + spin_lock_init(&mxport->spinlock); > + > + /* Set the port private data */ > + usb_set_serial_port_data(port, mxport); > + > + /* Send vendor request - set FIFO (Enable) */ The "Send vendor request" part of these comments doesn't add much information. Just remove that bit here (and elsewhere). > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_FIFO_DISABLE, > + 0, port->port_number); > + if (err) > + return err; > + > + /* Send vendor request - set transmission mode (Hi-Performance) */ > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_HIGH_PERFOR, > + 0, port->port_number); > + if (err) > + return err; > + > + /* Send vendor request - set interface (RS-232) */ > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_INTERFACE, > + MX_INT_RS232, > + port->port_number); > + if (err) > + return err; > + > + return err; > +} > + > +static int mxuport_alloc_write_urb(struct usb_serial *serial, > + struct usb_serial_port *port, > + struct usb_serial_port *port0, > + int j) > +{ > + struct usb_device *dev = interface_to_usbdev(serial->interface); > + > + set_bit(j, &port->write_urbs_free); > + port->write_urbs[j] = usb_alloc_urb(0, GFP_KERNEL); > + if (!port->write_urbs[j]) > + return -ENOMEM; > + > + port->bulk_out_buffers[j] = kmalloc(port0->bulk_out_size, GFP_KERNEL); > + if (!port->bulk_out_buffers[j]) > + return -ENOMEM; > + > + usb_fill_bulk_urb(port->write_urbs[j], dev, > + usb_sndbulkpipe(dev, port->bulk_out_endpointAddress), > + port->bulk_out_buffers[j], > + port->bulk_out_size, > + serial->type->write_bulk_callback, > + port); > + return 0; > +} > + > + > +static int mxuport_alloc_write_urbs(struct usb_serial *serial, > + struct usb_serial_port *port, > + struct usb_serial_port *port0) > +{ > + int j; > + int ret; > + > + for (j = 0; j < ARRAY_SIZE(port->write_urbs); ++j) { > + ret = mxuport_alloc_write_urb(serial, port, port0, j); > + if (ret) > + return ret; > + } > + return 0; > +} > + > + > +static int mxuport_attach(struct usb_serial *serial) > +{ > + struct usb_serial_port *port0 = serial->port[0]; > + struct usb_serial_port *port1 = serial->port[1]; > + struct usb_serial_port *port; > + int err; > + int i; > + int j; > + > + /* > + * Throw away all but the first allocated write URBs so we can > + * set them up again to fit the multiplexing scheme. > + */ > + for (i = 1; i < serial->num_bulk_out; ++i) { > + port = serial->port[i]; > + for (j = 0; j < ARRAY_SIZE(port->write_urbs); ++j) { > + usb_free_urb(port->write_urbs[j]); > + kfree(port->bulk_out_buffers[j]); > + port->write_urbs[j] = NULL; > + port->bulk_out_buffers[j] = NULL; > + } > + port->write_urbs_free = 0; > + } > + > + /* > + * All write data is sent over the first bulk out endpoint, > + * with an added header to indicate the port. Allocate URBs > + * for each port to the first bulk out endpoint. > + */ > + for (i = 1; i < serial->num_ports; ++i) { > + port = serial->port[i]; > + port->bulk_out_size = port0->bulk_out_size; > + port->bulk_out_endpointAddress = > + port0->bulk_out_endpointAddress; > + > + err = mxuport_alloc_write_urbs(serial, port, port0); > + if (err) > + goto out; > + > + port->write_urb = port->write_urbs[0]; > + port->bulk_out_buffer = port->bulk_out_buffers[0]; > + > + /* > + * Ensure each port has a fifo. The framework only > + * allocates a fifo to ports with a bulk out endpoint, > + * where as we need one for every port. > + */ > + if (!kfifo_initialized(&port->write_fifo)) { > + err = kfifo_alloc(&port->write_fifo, PAGE_SIZE, > + GFP_KERNEL); > + if (err) > + goto out; > + } > + } > + > + /* > + * Add data from the ports is received on the first bulk in > + * endpoint, with a multiplex header. The second bulk in is > + * used for events. Throw away all but the first two bulk in > + * urbs. > + */ > + for (i = 2; i < serial->num_bulk_in; ++i) { > + port = serial->port[i]; > + for (j = 0; j < ARRAY_SIZE(port->read_urbs); ++j) { > + usb_free_urb(port->read_urbs[j]); > + kfree(port->bulk_in_buffers[j]); > + port->read_urbs[j] = NULL; > + port->bulk_in_buffers[j] = NULL; > + port->bulk_in_size = 0; > + } > + } > + > + /* Start to read serial data from the device */ > + err = usb_serial_generic_submit_read_urbs(port0, GFP_KERNEL); > + if (err) { > + dev_err(&port0->dev, > + "%s - USB submit read bulk urb failed. (status = %d)\n", > + __func__, err); > + goto out; > + } > + > + /* Endpoints on Port1 is used for events */ > + err = usb_serial_generic_submit_read_urbs(port1, GFP_KERNEL); > + if (err) { > + dev_err(&port1->dev, > + "%s - USB submit read bulk urb failed. (status = %d)\n", > + __func__, err); > + goto out; > + } > + return 0; > + > +out: > + /* Shutdown any bulk transfers that might be going on */ > + usb_serial_generic_close(port1); > + usb_serial_generic_close(port0); > + > + return err; > +} > + > +static int mxuport_open(struct tty_struct *tty, struct usb_serial_port *port) > +{ > + struct mxuport_port *mxport = usb_get_serial_port_data(port); > + struct usb_serial *serial = port->serial; > + int err; > + > + /* Send vendor request - set receive host (enable) */ > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_RX_HOST_EN, > + 1, port->port_number); > + if (err) > + return err; > + > + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_OPEN, > + 1, port->port_number); > + No empty line. > + if (err) { > + mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_RX_HOST_EN, > + 0, port->port_number); > + return err; > + } > + > + /* Initial port termios */ > + mxuport_set_termios(tty, port, NULL); > + > + /* > + * TODO: use RQ_VENDOR_GET_MSR, once we know what it > + * returns. > + */ > + mxport->msr_state = 0; > + > + return err; > +} > + > +static void mxuport_close(struct usb_serial_port *port) > +{ > + struct usb_serial *serial = port->serial; > + > + mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_OPEN, 0, > + port->port_number); > + > + mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_RX_HOST_EN, 0, > + port->port_number); > + > +} > + > +/* Send a break to the port. */ > +static void mxuport_break_ctl(struct tty_struct *tty, int break_state) > +{ > + struct usb_serial_port *port = tty->driver_data; > + struct usb_serial *serial = port->serial; > + struct mxuport_port *mxport; > + int enable; > + > + mxport = usb_get_serial_port_data(port); > + > + if (break_state == -1) { > + enable = 1; > + dev_dbg(&port->dev, "%s - sending break\n", __func__); > + } else { > + enable = 0; > + dev_dbg(&port->dev, "%s - clearing break\n", __func__); > + } > + > + mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_BREAK, > + enable, port->port_number); > +} > + > +static int mxuport_resume(struct usb_serial *serial) > +{ > + struct usb_serial_port *port; > + int c = 0; > + int i; > + int r; > + > + for (i = 0; i < 2; i++) { > + port = serial->port[i]; > + > + r = usb_serial_generic_submit_read_urbs(port, GFP_NOIO); > + if (r < 0) > + c++; > + } > + > + for (i = 0; i < serial->num_ports; i++) { > + port = serial->port[i]; > + if (!test_bit(ASYNCB_INITIALIZED, &port->port.flags)) > + continue; > + > + r = usb_serial_generic_write_start(port, GFP_NOIO); > + if (r < 0) > + c++; > + } > + > + return c ? -EIO : 0; > +} > + > +static struct usb_serial_driver mxuport_device = { > + .driver = { > + .owner = THIS_MODULE, > + .name = "mxuport", > + }, > + .description = "MOXA UPort", > + .id_table = mxuport_idtable, > + .num_ports = 0, > + .probe = mxuport_probe, > + .port_probe = mxuport_port_probe, > + .attach = mxuport_attach, > + .calc_num_ports = mxuport_calc_num_ports, > + .open = mxuport_open, > + .close = mxuport_close, > + .set_termios = mxuport_set_termios, > + .break_ctl = mxuport_break_ctl, > + .tx_empty = mxuport_tx_empty, > + .tiocmiwait = usb_serial_generic_tiocmiwait, > + .get_icount = usb_serial_generic_get_icount, > + .throttle = mxuport_throttle, > + .unthrottle = mxuport_unthrottle, > + .tiocmget = mxuport_tiocmget, > + .tiocmset = mxuport_tiocmset, > + .dtr_rts = mxuport_dtr_rts, > + .process_read_urb = mxuport_process_read_urb, > + .prepare_write_buffer = mxuport_prepare_write_buffer, > + .resume = mxuport_resume, > +}; > + > +static struct usb_serial_driver *const serial_drivers[] = { > + &mxuport_device, NULL > +}; > + > +module_usb_serial_driver(serial_drivers, mxuport_idtable); > + > +/* > + * Module Information > + */ > +MODULE_AUTHOR(DRIVER_AUTHOR); > +MODULE_LICENSE("GPL"); > -- > 1.8.4.rc3 > Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html