USB: serial: add Moxa UPORT 11x0 driver
Subsystems (2)
PATCH v4 1/1 USB: serial: add Moxa UPORT 11x0 driver
Add a driver which supports :
- UPort 1110 : 1 port RS-232 USB to Serial Hub.
- UPort 1130 : 1 port RS-422/485 USB to Serial Hub.
- UPort 1130I : 1 port RS-422/485 USB to Serial Hub with Isolation.
- UPort 1150 : 1 port RS-232/422/485 USB to Serial Hub.
- UPort 1150I : 1 port RS-232/422/485 USB to Serial Hub with Isolation.
This driver is based on GPL MOXA driver written by Hen Huang and available on MOXA website. The original driver was based on io_ti serial driver.
--- drivers/usb/serial/Kconfig | 16 + drivers/usb/serial/Makefile | 1 + drivers/usb/serial/mxu11x0.c | 1101 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1118 insertions(+) create mode 100644 drivers/usb/serial/mxu11x0.c
drivers/usb/serial/Kconfig +16 -0
@@ -475,6 +475,22 @@ 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_MXUPORT11 + tristate "USB Moxa UPORT 11x0 Serial Driver" + ---help--- + Say Y here if you want to use a MOXA UPort 11x0 Serial hub. + + This driver supports: + + - UPort 1110 : 1 port RS-232 USB to Serial Hub. + - UPort 1130 : 1 port RS-422/485 USB to Serial Hub. + - UPort 1130I : 1 port RS-422/485 USB to Serial Hub with Isolation. + - UPort 1150 : 1 port RS-232/422/485 USB to Serial Hub. + - UPort 1150I : 1 port RS-232/422/485 USB to Serial Hub with Isolation. + + To compile this driver as a module, choose M here: the + module will be called mxu11x0. + config USB_SERIAL_MXUPORT tristate "USB Moxa UPORT Serial Driver" ---help---
drivers/usb/serial/Makefile +1 -0
@@ -38,6 +38,7 @@ 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_MXUPORT) += mxuport.o +obj-$(CONFIG_USB_SERIAL_MXUPORT11) += mxu11x0.o obj-$(CONFIG_USB_SERIAL_NAVMAN) += navman.o obj-$(CONFIG_USB_SERIAL_OMNINET) += omninet.o obj-$(CONFIG_USB_SERIAL_OPTICON) += opticon.o
drivers/usb/serial/mxu11x0.c new file +1101 -0
1,101 lines not on this page · raw message
Signature · 1 line
2.6.1
Johan Hovold
maintainerQuoted 8 lines · Mathieu OTHACEHE
Add a driver which supports :
- UPort 1110 : 1 port RS-232 USB to Serial Hub.
- UPort 1130 : 1 port RS-422/485 USB to Serial Hub.
- UPort 1130I : 1 port RS-422/485 USB to Serial Hub with Isolation.
- UPort 1150 : 1 port RS-232/422/485 USB to Serial Hub.
- UPort 1150I : 1 port RS-232/422/485 USB to Serial Hub with Isolation.
Quoted 5 lines
This driver is based on GPL MOXA driver written by Hen Huang and available on MOXA website. The original driver was based on
io_tiserial driver.Signed-off-by Mathieu OTHACEHE <[email protected]>---
Thanks for the v4. It'll be a few days still before I have a chance to review it.
What changed since v3 (and v2)?
Please include a summary of the changes below the cutoff line (i.e. here) when you send a new revision of a patch.
Thanks,
Johan
Hi Johan
Ok, I'll include a changelog in future patches. Here is the current changlog :
Changes in v2:
* Use generic implementation for read/write code
* Use TIOCSRS485/TIOCGRS485 instead of moxa specific ioctl
* Other problems pointed out by Johan review
Changes in v3:
* Drop unused defines
* Fix style problems
* Remove port identification code since this driver is for one-port devices only
* Fix debug messages
* Use a mutex to protect shadow_mc
* Use usb_serial_generic_open and close
* Other problems pointed out by Johan review
Changes in v4:
* mxu1_startup return -ENODEV on purpose if the firmware was downloaded. It avoids port_probe to fail since the device is reset.
Thanks,
Mathieu
Quoted 25 lines · Johan Hovold
Quoted 8 lines · Mathieu OTHACEHE
Add a driver which supports :
- UPort 1110 : 1 port RS-232 USB to Serial Hub.
- UPort 1130 : 1 port RS-422/485 USB to Serial Hub.
- UPort 1130I : 1 port RS-422/485 USB to Serial Hub with Isolation.
- UPort 1150 : 1 port RS-232/422/485 USB to Serial Hub.
- UPort 1150I : 1 port RS-232/422/485 USB to Serial Hub with Isolation.Quoted 5 lines
This driver is based on GPL MOXA driver written by Hen Huang and available on MOXA website. The original driver was based on
io_tiserial driver.Signed-off-by Mathieu OTHACEHE <[email protected]>---
Thanks for the v4. It'll be a few days still before I have a chance to review it.
What changed since v3 (and v2)?
Please include a summary of the changes below the cutoff line (i.e. here) when you send a new revision of a patch.
Thanks,
Johan
Johan Hovold
maintainerQuoted 8 lines · Mathieu OTHACEHE
Add a driver which supports :
- UPort 1110 : 1 port RS-232 USB to Serial Hub.
- UPort 1130 : 1 port RS-422/485 USB to Serial Hub.
- UPort 1130I : 1 port RS-422/485 USB to Serial Hub with Isolation.
- UPort 1150 : 1 port RS-232/422/485 USB to Serial Hub.
- UPort 1150I : 1 port RS-232/422/485 USB to Serial Hub with Isolation.
Quoted 5 lines
This driver is based on GPL MOXA driver written by Hen Huang and available on MOXA website. The original driver was based on
io_tiserial driver.Signed-off-by Mathieu OTHACEHE <[email protected]>---
Thanks for the v4, and sorry about the late review.
drivers/usb/serial/mxu11x0.c line 224
Quoted 6 lines
drivers/usb/serial/mxu11x0.c new file +6
-0+struct mxu1_port { +u8 mxp_msr; +u8 mxp_mcr; +u8 mxp_uart_types; +u8 mxp_uart_mode; +struct usb_serial_port *mxp_port;
You should not need a back pointer here. Pass the usb_serial_port as an argument where needed instead.
drivers/usb/serial/mxu11x0.c line 227
Quoted 3 lines
drivers/usb/serial/mxu11x0.c new file +3
-0+spinlock_t mxp_lock;/* Protects mxp_msr */ +struct mutex mxp_mutex;/* Protects mxp_mcr */ +int mxp_send_break;
Why is this not a bool? You never use the value you assign to mxp_send_break other than to compare it with a constant.
Quoted 1 line
+};
Please drop the mxp_ prefix from the field names.
drivers/usb/serial/mxu11x0.c line 232
Quoted 3 lines
drivers/usb/serial/mxu11x0.c new file +3
-0+struct mxu1_device { +struct mutex mxd_lock; +struct usb_serial *mxd_serial;
This lock and back pointer are not needed. You can access the usb_serial from usb_serial_port.
Quoted 9 lines
drivers/usb/serial/mxu11x0.c new file +9
-0+u16 mxd_model; +}; + +static const struct usb_device_id mxu1_idtable[] = { + {USB_DEVICE (MXU1_VENDOR_ID ,MXU1_1110_PRODUCT_ID ) }, + {USB_DEVICE (MXU1_VENDOR_ID ,MXU1_1130_PRODUCT_ID ) }, + {USB_DEVICE (MXU1_VENDOR_ID ,MXU1_1150_PRODUCT_ID ) }, + {USB_DEVICE (MXU1_VENDOR_ID ,MXU1_1151_PRODUCT_ID ) }, + {USB_DEVICE (MXU1_VENDOR_ID ,MXU1_1131_PRODUCT_ID ) },
drivers/usb/serial/mxu11x0.c line 251
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+ { } +}; + +MODULE_DEVICE_TABLE (usb, mxu1_idtable); + +/* Write the given buffer out to the control pipe. */ +static int mxu1_send_ctrl_data_urb (struct usb_serial *serial, +u8 request, +u16 value,u16 index, +u8 *data,size_t size)
Use void * for the buffer and drop the casts at the call sites.
Quoted 41 lines
drivers/usb/serial/mxu11x0.c new file +41
-0+{ +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_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 ; +} + +/* Send a vendor request without any data */ +static int mxu1_send_ctrl_urb (struct usb_serial *serial, +u8 request,u16 value,u16 index) +{ +return mxu1_send_ctrl_data_urb (serial, request, value, index, +NULL ,0 ); +} + +static int mxu1_download_firmware (struct usb_serial *serial, +const struct firmware *fw_p) +{ +int status =0 ; +int buffer_size; +int pos;
drivers/usb/serial/mxu11x0.c line 302
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+int len; +int done; +u8 cs =0 ; +u8 *buffer; +struct usb_device *dev = serial->dev; +struct mxu1_firmware_header *header; +unsigned int pipe; + + pipe =usb_sndbulkpipe (dev, serial->port[0 ]-> + bulk_out_endpointAddress);
Odd line break that is not even needed.
Quoted 28 lines
drivers/usb/serial/mxu11x0.c new file +28
-0+ + buffer_size = fw_p->size +sizeof (*header); + buffer =kmalloc (buffer_size,GFP_KERNEL ); +if (!buffer) +return -ENOMEM ; + +memcpy (buffer, fw_p->data, fw_p->size); +memset (buffer + fw_p->size,0xff , buffer_size - fw_p->size); + +for (pos =sizeof (*header); pos < buffer_size; pos++) + cs = (u8)(cs + buffer[pos]); + + header = (struct mxu1_firmware_header *)buffer; + 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 < buffer_size; pos += done) { + len =min (buffer_size - pos,MXU1_DOWNLOAD_MAX_PACKET_SIZE ); + + status =usb_bulk_msg (dev, pipe, buffer + pos, len, &done, +MXU1_DOWNLOAD_TIMEOUT ); +if (status) +break ; + } + +kfree (buffer);
drivers/usb/serial/mxu11x0.c line 340
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+ +if (status) { +dev_err (&dev->dev,"failed to download firmware: %d\n" , status); +return status; + } + +msleep_interruptible (100 ); +usb_reset_device (dev); + +dev_dbg (&dev->dev,"%s - download successful (%d)\n" , __func__, status);
You can drop status from the message, it's always zero.
Quoted 5 lines
drivers/usb/serial/mxu11x0.c new file +4
-0+ +return 0 ; +} +static int mxu1_startup (struct usb_serial *serial)
drivers/usb/serial/mxu11x0.c line 404
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+{ +struct mxu1_device *mxdev; +struct usb_device *dev = serial->dev; +struct usb_host_interface *cur_altsetting; +char fw_name[32 ]; +const struct firmware *fw_p =NULL ; +int err; +int status =0 ; + + dev_dbg(&dev->dev, "%s - product 0x%4X, num configurations %d, configuration value %d\n",
Use &serial->interface->dev here too, and you want 0x%04x zero-padded.
drivers/usb/serial/mxu11x0.c line 411
Quoted 7 lines
drivers/usb/serial/mxu11x0.c new file +7
-0+ __func__,le16_to_cpu (dev->descriptor.idProduct), + dev->descriptor.bNumConfigurations, + dev->actconfig->desc.bConfigurationValue); + +/* create device structure */ + mxdev =kzalloc (sizeof (struct mxu1_device ),GFP_KERNEL ); +if (mxdev ==NULL )
Use !mxdev.
drivers/usb/serial/mxu11x0.c line 415
Quoted 4 lines
drivers/usb/serial/mxu11x0.c new file +4
-0+return -ENOMEM ; + +mutex_init (&mxdev->mxd_lock); + mxdev->mxd_serial = serial;
These two are not needed as mentioned above.
Quoted 8 lines
drivers/usb/serial/mxu11x0.c new file +8
-0+usb_set_serial_data (serial, mxdev); + + mxdev->mxd_model =le16_to_cpu (dev->descriptor.idProduct); + + cur_altsetting = serial->interface->cur_altsetting; + +/* if we have only 1 configuration, download firmware */ +if (cur_altsetting->desc.bNumEndpoints ==1 ) {
drivers/usb/serial/mxu11x0.c line 433
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+ +snprintf (fw_name, +sizeof (fw_name), +"moxa/moxa-%04x.fw" , + mxdev->mxd_model); + + err =request_firmware (&fw_p, fw_name, &serial->interface->dev); +if (err) { +dev_err (&serial->interface->dev,"firmware %s not found\n" , + fw_name);
Be a little less specific here (e.g. "failed to request firmware: %d\n" and include the errno.
drivers/usb/serial/mxu11x0.c line 441
Quoted 8 lines
drivers/usb/serial/mxu11x0.c new file +8
-0+kfree (mxdev); +return err; + } + + err =mxu1_download_firmware (serial, fw_p); +if (err) { +kfree (mxdev); +return err;
You're leaking the firmware memory here.
Quoted 37 lines
drivers/usb/serial/mxu11x0.c new file +36
-0+ } + + status = -ENODEV ; +release_firmware (fw_p); + } + +return status; +} +static void mxu1_set_termios (struct tty_struct *tty, +struct usb_serial_port *port, +struct ktermios *old_termios) +{ +struct mxu1_port *mxport =usb_get_serial_port_data (port); +struct mxu1_uart_config *config; +tcflag_t cflag, iflag; +speed_t baud; +int status; +unsigned int mcr; + +dev_dbg (&port->dev,"%s\n" , __func__); + + 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 - clfag %08x, iflag %08x\n" , __func__, cflag, iflag); + +if (old_termios) { + dev_dbg(&port->dev, "%s - old clfag %08x, old iflag %08x\n",
drivers/usb/serial/mxu11x0.c line 534
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+ __func__, + old_termios->c_cflag, + old_termios->c_iflag); + } + + config =kzalloc (sizeof (*config),GFP_KERNEL ); +if (!config) +return ; + + config->wFlags =0 ;
You can rely on your kzalloc just above.
drivers/usb/serial/mxu11x0.c line 541
Quoted 7 lines
drivers/usb/serial/mxu11x0.c new file +7
-0+ +/* these flags must be set */ + config->wFlags |=MXU1_UART_ENABLE_MS_INTS ; + config->wFlags |=MXU1_UART_ENABLE_AUTO_START_DMA ; +if (mxport->mxp_send_break ==MXU1_LCR_BREAK ) + config->wFlags |=MXU1_UART_SEND_BREAK_SIGNAL ; + config->bUartMode = (u8)(mxport->mxp_uart_mode);
No need to cast.
Quoted 16 lines
drivers/usb/serial/mxu11x0.c new file +16
-0+static int mxu1_ioctl_set_rs485 (struct mxu1_port *mxport, +struct serial_rs485 __user *rs485_user) +{ +struct serial_rs485 rs485; +struct usb_serial_port *port = mxport->mxp_port; +struct mxu1_device *mxdev; + + mxdev =usb_get_serial_data (port->serial); + +if (copy_from_user (&rs485, rs485_user,sizeof (*rs485_user))) +return -EFAULT ; + +if (mxport->mxp_uart_types & (MXU1_TYPE_RS422 |MXU1_TYPE_RS485 )) { + +if (rs485.flags &SER_RS485_ENABLED ) { + mxport->mxp_uart_mode =MXU1_UART_485_RECEIVER_ENABLED ;
drivers/usb/serial/mxu11x0.c line 678
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+ }else { +if (mxport->mxp_uart_types &MXU1_TYPE_RS232 ) + mxport->mxp_uart_mode =MXU1_UART_232 ; +else + mxport->mxp_uart_mode = +MXU1_UART_485_RECEIVER_DISABLED ; + } + }else { +dev_err (&port->dev,"%s not handled by MOXA UPort %04x\n" , + __func__, mxdev->mxd_model);
Could you just spell this out as something like "RS485 not supported by this device\n"?
Review
Quoted 2 lines
(quoted diff) +2
-0+ return -EINVAL; + }
You also need to revert any bits not supported and return the new config to user space. Take a look at uart_set_rs485_config in serial_core.
Quoted 4 lines
drivers/usb/serial/mxu11x0.c new file +3
-0+ +return 0 ; +}
drivers/usb/serial/mxu11x0.c line 767
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+static int mxu1_tiocmget (struct tty_struct *tty) +{ +struct usb_serial_port *port = tty->driver_data; +struct mxu1_port *mxport =usb_get_serial_port_data (port); +unsigned int result; +unsigned int msr; +unsigned int mcr; +unsigned long flags; + +dev_dbg (&port->dev,"%s\n" , __func__);
Drop this.
Quoted 22 lines
drivers/usb/serial/mxu11x0.c new file +22
-0+ +mutex_lock (&mxport->mxp_mutex); +spin_lock_irqsave (&mxport->mxp_lock, flags); + + msr = mxport->mxp_msr; + mcr = mxport->mxp_mcr; + +spin_unlock_irqrestore (&mxport->mxp_lock, flags); +mutex_unlock (&mxport->mxp_mutex); + + result = ((mcr &MXU1_MCR_DTR ) ?TIOCM_DTR :0 ) | + ((mcr &MXU1_MCR_RTS ) ?TIOCM_RTS :0 ) | + ((mcr &MXU1_MCR_LOOP ) ?TIOCM_LOOP :0 ) | + ((msr &MXU1_MSR_CTS ) ?TIOCM_CTS :0 ) | + ((msr &MXU1_MSR_CD ) ?TIOCM_CAR :0 ) | + ((msr &MXU1_MSR_RI ) ?TIOCM_RI :0 ) | + ((msr &MXU1_MSR_DSR ) ?TIOCM_DSR :0 ); + +dev_dbg (&port->dev,"%s - 0x%04X\n" , __func__, result); + +return result; +}
drivers/usb/serial/mxu11x0.c line 799
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+ +static int mxu1_tiocmset (struct tty_struct *tty, +unsigned int set,unsigned int clear) +{ +struct usb_serial_port *port = tty->driver_data; +struct mxu1_port *mxport =usb_get_serial_port_data (port); +int err; +unsigned int mcr; + +dev_dbg (&port->dev,"%s\n" , __func__);
Ditto.
Quoted 31 lines
drivers/usb/serial/mxu11x0.c new file +30
-0+ +mutex_lock (&mxport->mxp_mutex); + mcr = mxport->mxp_mcr; + +if (set &TIOCM_RTS ) + mcr |=MXU1_MCR_RTS ; +if (set &TIOCM_DTR ) + mcr |=MXU1_MCR_DTR ; +if (set &TIOCM_LOOP ) + mcr |=MXU1_MCR_LOOP ; + +if (clear &TIOCM_RTS ) + mcr &= ~MXU1_MCR_RTS ; +if (clear &TIOCM_DTR ) + mcr &= ~MXU1_MCR_DTR ; +if (clear &TIOCM_LOOP ) + mcr &= ~MXU1_MCR_LOOP ; + + err =mxu1_set_mcr (port, mcr); +if (!err) + mxport->mxp_mcr = mcr; + +mutex_unlock (&mxport->mxp_mutex); + +return err; +} +static int mxu1_open (struct tty_struct *tty,struct usb_serial_port *port) +{ +struct mxu1_device *mxdev; +struct mxu1_port *mxport;
drivers/usb/serial/mxu11x0.c line 855
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+struct usb_device *dev; +struct urb *urb; +int status; +u16 open_settings; + + open_settings = (MXU1_PIPE_MODE_CONTINUOUS | +MXU1_PIPE_TIMEOUT_ENABLE | + (MXU1_TRANSFER_TIMEOUT <<2 )); + + mxdev =usb_get_serial_data (port->serial);
You don't need mxdev; you already have port->serial, and the interrupt urb already has the usb_serial_port set as context.
drivers/usb/serial/mxu11x0.c line 861
Quoted 6 lines
drivers/usb/serial/mxu11x0.c new file +6
-0+ mxport =usb_get_serial_port_data (port); + + dev = port->serial->dev; + + mxport->mxp_msr =0 ; + mxport->mxp_mcr |=MXU1_MCR_RTS |MXU1_MCR_DTR ;
These will be raised by tty core using dtr_rts, so drop this.
drivers/usb/serial/mxu11x0.c line 873
Quoted 12 lines
drivers/usb/serial/mxu11x0.c new file +12
-0+ +dev_dbg (&port->dev,"%s - start interrupt in urb\n" , __func__); + urb = port->interrupt_in_urb; + urb->context = mxdev; + status =usb_submit_urb (urb,GFP_KERNEL ); +if (status) { +dev_err (&port->dev,"failed to submit interrupt urb: %d\n" , + status); +return status; + } + +mxu1_set_termios (tty, port,NULL );
tty may be NULL if this port used as a console, so only call set_termios if tty is non-NULL.
Quoted 70 lines
drivers/usb/serial/mxu11x0.c new file +70
-0+ + status =mxu1_send_ctrl_urb (mxdev->mxd_serial,MXU1_OPEN_PORT , + open_settings,MXU1_UART1_PORT ); +if (status) { +dev_err (&port->dev,"%s - cannot send open command: %d\n" , + __func__, status); + goto unlink_int_urb; + } + + status =mxu1_send_ctrl_urb (mxdev->mxd_serial,MXU1_START_PORT , +0 ,MXU1_UART1_PORT ); +if (status) { +dev_err (&port->dev,"%s - cannot send start command: %d\n" , + __func__, status); + goto unlink_int_urb; + } + + status =mxu1_send_ctrl_urb (mxdev->mxd_serial,MXU1_PURGE_PORT , +MXU1_PURGE_INPUT ,MXU1_UART1_PORT ); +if (status) { +dev_err (&port->dev,"%s - cannot clear input buffers: %d\n" , + __func__, status); + + goto unlink_int_urb; + } + + status =mxu1_send_ctrl_urb (mxdev->mxd_serial,MXU1_PURGE_PORT , +MXU1_PURGE_OUTPUT ,MXU1_UART1_PORT ); +if (status) { +dev_err (&port->dev,"%s - cannot clear output buffers: %d\n" , + __func__, status); + + 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 +*/ +usb_clear_halt (dev, port->write_urb->pipe); +usb_clear_halt (dev, port->read_urb->pipe); + +mxu1_set_termios (tty, port,NULL ); + + status =mxu1_send_ctrl_urb (mxdev->mxd_serial,MXU1_OPEN_PORT , + open_settings,MXU1_UART1_PORT ); +if (status) { +dev_err (&port->dev,"%s - cannot send open command: %d\n" , + __func__, status); + goto unlink_int_urb; + } + + status =mxu1_send_ctrl_urb (mxdev->mxd_serial,MXU1_START_PORT , +0 ,MXU1_UART1_PORT ); +if (status) { +dev_err (&port->dev,"%s - cannot send start command: %d\n" , + __func__, status); + goto unlink_int_urb; + } + + status =usb_serial_generic_open (tty, port); +if (status) { +dev_err (&port->dev,"%s - submit read urb failed: %d\n" , + __func__, status); + goto unlink_int_urb; + } + +return status; + +unlink_int_urb:
drivers/usb/serial/mxu11x0.c line 953
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+usb_kill_urb (port->interrupt_in_urb); + +return status; +} + +static void mxu1_close (struct usb_serial_port *port) +{ +int status; + +dev_dbg (&port->dev,"%s\n" , __func__);
Drop this.
Quoted 11 lines
drivers/usb/serial/mxu11x0.c new file +11
-0+ +usb_serial_generic_close (port); +usb_kill_urb (port->interrupt_in_urb); + + status =mxu1_send_ctrl_urb (port->serial,MXU1_CLOSE_PORT , +0 ,MXU1_UART1_PORT ); +if (status) +dev_err (&port->dev,"failed to send close port command: %d\n" , + status); +} +
drivers/usb/serial/mxu11x0.c line 974
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+static void mxu1_handle_new_msr (struct usb_serial_port *port, +struct mxu1_port *mxport,u8 msr) +{ +struct async_icount *icount; +unsigned long flags; + +dev_dbg (&port->dev,"%s - msr 0x%02X\n" , __func__, msr); + +if (msr &MXU1_MSR_DELTA_MASK ) { + icount = &mxport->mxp_port->icount;
You already have the port structure.
Quoted 5 lines
drivers/usb/serial/mxu11x0.c new file +5
-0+if (msr &MXU1_MSR_DELTA_CTS ) + icount->cts++; +if (msr &MXU1_MSR_DELTA_DSR ) + icount->dsr++; +if (msr &MXU1_MSR_DELTA_CD )
drivers/usb/serial/mxu11x0.c line 989
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+ icount->dcd++; +if (msr &MXU1_MSR_DELTA_RI ) + icount->rng++; + +wake_up_interruptible (&port->port.delta_msr_wait); + } + +spin_lock_irqsave (&mxport->mxp_lock, flags); + mxport->mxp_msr = msr &MXU1_MSR_MASK ; +spin_unlock_irqrestore (&mxport->mxp_lock, flags);
You want to do this before testing the delta and waking up any waiters.
Quoted 8 lines
drivers/usb/serial/mxu11x0.c new file +8
-0+} + +static void mxu1_interrupt_callback (struct urb *urb) +{ +struct mxu1_device *mxdev = urb->context; +struct usb_serial_port *port; +struct usb_serial *serial = mxdev->mxd_serial; +struct mxu1_port *mxport;
drivers/usb/serial/mxu11x0.c line 1007
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+unsigned char *data = urb->transfer_buffer; +int length = urb->actual_length; +int function; +int status; +u8 msr; + + port = serial->port[0 ]; + mxport =usb_get_serial_port_data (port); + +dev_dbg (&port->dev,"%s\n" , __func__);
Drop this.
Quoted 32 lines
drivers/usb/serial/mxu11x0.c new file +32
-0+ +switch (urb->status) { +case 0 : +break ; +case -ECONNRESET : +case -ENOENT : +case -ESHUTDOWN : +dev_dbg (&port->dev,"%s - urb shutting down: %d\n" , + __func__, urb->status); +return ; +default : +dev_dbg (&port->dev,"%s - nonzero urb status: %d\n" , + __func__, urb->status); + goto exit; + } + +if (length !=2 ) { +dev_dbg (&port->dev,"%s - bad packet size: %d\n" , + __func__, length); + goto exit; + } + +if (data[0 ] ==MXU1_CODE_HARDWARE_ERROR ) { +dev_err (&port->dev,"%s - hardware error: %d\n" , + __func__, data[1 ]); + goto exit; + } + + function =mxu1_get_func_from_code (data[0 ]); + +dev_dbg (&port->dev,"%s - function %d, data 0x%02X\n" , + __func__, function, data[1 ]);
drivers/usb/serial/mxu11x0.c line 1049
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+ +switch (function) { +case MXU1_CODE_DATA_ERROR : +dev_dbg (&port->dev,"%s - DATA ERROR, data 0x%02X\n" , + __func__, data[1 ]); +break ; + +case MXU1_CODE_MODEM_STATUS : + msr = data[1 ]; +dev_dbg (&port->dev,"%s - msr 0x%02X\n" , __func__, msr);
You print the same debug info in mxu1_handle_new_msr.
Quoted 4 lines
drivers/usb/serial/mxu11x0.c new file +4
-0+mxu1_handle_new_msr (port, mxport, msr); +break ; + +default :
drivers/usb/serial/mxu11x0.c line 1063
Quoted 10 lines
drivers/usb/serial/mxu11x0.c new file +10
-0+dev_err (&port->dev,"%s - unknown interrupt code: 0x%02X\n" , + __func__, 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);
No need to include the function name.
Quoted 1 line
+}
Overall this looks really good now.
Thanks,
Johan