x86: Fixed MCS7820 device attach problem
Subsystems (2)
PATCH 1/1 x86: Fixed MCS7820 device attach problem
A MCS7820 device supports two serial ports and a MCS7840 device supports four serial ports. Both devices use the same driver, but the attach function in driver was unable to correctly handle the port numbers for MCS7820 device. This problem has been fixed in this patch and this fix has been verified on x86 Linux kernel 3.2.9 with both MCS7820 and MCS7840 devices.
---
drivers/usb/serial/mos7840.c | 83 +++++++++++++++++++++++++++++------------- 1 files changed, 57 insertions(+), 26 deletions(-)
drivers/usb/serial/mos7840.c +1 -0
@@ -174,6 +174,7 @@#define CLK_MULTI_REGISTER ((__u16)(0x02))#define CLK_START_VALUE_REGISTER ((__u16)(0x03)) +#define GPIO_REGISTER ((__u16)(0x07))#define SERIAL_LCR_DLAB ((__u16)(0x0080))
@@ -1103,14 +1104,25 @@ static int mos7840_open(struct tty_struct *tty,
struct usb_serial_port *port)
mos7840_port->read_urb = port->read_urb;
/* set up our bulk in urb */ -
- usb_fill_bulk_urb(mos7840_port->read_urb,
- serial->dev,
- usb_rcvbulkpipe(serial->dev,
- port->bulk_in_endpointAddress),
- port->bulk_in_buffer, - mos7840_port->read_urb->transfer_buffer_length, - mos7840_bulk_in_callback, mos7840_port);
+ if ((serial->num_ports == 2)
+ && ((((__u16)port->number -
+ (__u16)(port->serial->minor)) % 2) != 0)) {
+ usb_fill_bulk_urb(mos7840_port->read_urb,
+ serial->dev,
+ usb_rcvbulkpipe(serial->dev,
+ (port->bulk_in_endpointAddress) + 2),
+ port->bulk_in_buffer, + mos7840_port->read_urb->transfer_buffer_length, + mos7840_bulk_in_callback, mos7840_port);
+ } else {
+ usb_fill_bulk_urb(mos7840_port->read_urb,
+ serial->dev,
+ usb_rcvbulkpipe(serial->dev,
+ port->bulk_in_endpointAddress),
+ port->bulk_in_buffer, + mos7840_port->read_urb->transfer_buffer_length, + mos7840_bulk_in_callback, mos7840_port);
+ }
dbg("mos7840_open: bulkin endpoint is %d",
port->bulk_in_endpointAddress);
@@ -1521,13 +1533,25 @@ static int mos7840_write(struct tty_struct *tty,
struct usb_serial_port *port,
memcpy(urb->transfer_buffer, current_position, transfer_size);
/* fill urb with data and submit */ - usb_fill_bulk_urb(urb,
- serial->dev,
- usb_sndbulkpipe(serial->dev,
- port->bulk_out_endpointAddress),
- urb->transfer_buffer, - transfer_size, - mos7840_bulk_out_data_callback, mos7840_port);
+ if ((serial->num_ports == 2)
+ && ((((__u16)port->number -
+ (__u16)(port->serial->minor)) % 2) != 0)) {
+ usb_fill_bulk_urb(urb,
+ serial->dev,
+ usb_sndbulkpipe(serial->dev,
+ (port->bulk_out_endpointAddress) + 2),
+ urb->transfer_buffer, + transfer_size, + mos7840_bulk_out_data_callback, mos7840_port);
+ } else {
+ usb_fill_bulk_urb(urb,
+ serial->dev,
+ usb_sndbulkpipe(serial->dev,
+ port->bulk_out_endpointAddress),
+ urb->transfer_buffer, + transfer_size, + mos7840_bulk_out_data_callback, mos7840_port);
+ }
data1 = urb->transfer_buffer;
dbg("bulkout endpoint is %d", port->bulk_out_endpointAddress); @@ -1840,7 +1864,7 @@ static int mos7840_send_cmd_write_baud_rate(struct moschip_port *mos7840_port,
} else {
#ifdef HW_flow_control
- / *setting h/w flow control bit to 0 */
+ /* setting h/w flow control bit to 0 */
Data = 0xb;
mos7840_port->shadowMCR = Data;
status = mos7840_set_uart_reg(port, MODEM_CONTROL_REGISTER, @@ -2309,19 +2333,26 @@ static int mos7840_ioctl(struct tty_struct *tty,
static int mos7840_calc_num_ports(struct usb_serial *serial) {
- int mos7840_num_ports = 0;
-
- dbg("numberofendpoints: cur %d, alt %d",
- (int)serial->interface->cur_altsetting->desc.bNumEndpoints,
- (int)serial->interface->altsetting->desc.bNumEndpoints);
- if (serial->interface->cur_altsetting->desc.bNumEndpoints == 5) {
- mos7840_num_ports = serial->num_ports = 2;
- } else if (serial->interface->cur_altsetting->desc.bNumEndpoints ==
9) {
+ __u16 Data = 0x00; + int ret = 0; + int mos7840_num_ports;
+
+ ret = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
+ MCS_RDREQ, MCS_RD_RTYPE, 0, GPIO_REGISTER, &Data,
+ VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT);
+
+ if ((Data&0x01) == 0) {
+ mos7840_num_ports = 2;
+ serial->num_bulk_in = 2;
+ serial->num_bulk_out = 2;
+ serial->num_ports = 2;
+ } else {
+ mos7840_num_ports = 4;
serial->num_bulk_in = 4;
serial->num_bulk_out = 4;
- mos7840_num_ports = serial->num_ports = 4;
+ serial->num_ports = 4;
}
- dbg ("mos7840_num_ports = %d", mos7840_num_ports);
+
return mos7840_num_ports; }
Signature · 1 line
1.7.7.6
'Greg KH'
maintainerQuoted 11 lines · Donald
A MCS7820 device supports two serial ports and a MCS7840 device supports four serial ports. Both devices use the same driver, but the attach function in driver was unable to correctly handle the port numbers for MCS7820 device. This problem has been fixed in this patch and this fix has been verified on x86 Linux kernel 3.2.9 with both MCS7820 and MCS7840 devices.
Signed-off-by Donald Lee <[email protected]>---
drivers/usb/serial/mos7840.c | 83+++++++++++++++++++++++++++++-------------
This is great, but your email client turned the tabs into spaces and line-wrapped the patch, and added trailing spaces on every line, making this impossible to apply :(
Care to read Documentation/email_clients.txt and resend it after fixing things up on your end?
And this isn't limited to x86, so you don't need that in the subject:
line, it's a USB issue for all platforms.
thanks,
greg k-h
Patch "USB: serial: mos7840: Supported MCS7810 device"
Hi Greg,
I am submitting this patch that supports MCS7810 device for the mos7840 driver. If you see any problem regarding this patch, please let me know at any time. Thank you for your help.
Regards,
Donald
Patch Description:
This patch added the support of MCS7810 device for the mos7840 driver. The MCS7810 device supports single USB2.0-to-Serial port with a LED indicator for reflecting transmission or reception activity. Please be noted that part of codes in his patch are relating to the codes in last submitted patch that fixed MCS7820 device attach problem.
--- drivers/usb/serial/mos7840.c | 203 ++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 197 insertions(+), 6 deletions(-)
drivers/usb/serial/mos7840.c +2 -1
@@ -114,6 +114,7 @@#define USB_VENDOR_ID_MOSCHIP 0x9710#define MOSCHIP_DEVICE_ID_7840 0x7840#define MOSCHIP_DEVICE_ID_7820 0x7820 +#define MOSCHIP_DEVICE_ID_7810 0x7810/* The native component can have its vendor/device id's overridden * in vendor-specific implementations. Such devices can be handled * by making a change here, in moschip_port_id_table, and in
@@ -184,10 +185,10 @@#define NUM_URBS 16/* URB Count */ #define URB_TRANSFER_BUFFER_SIZE 32/* URB Size */ -static const struct usb_device_id moschip_port_id_table[] = { {USB_DEVICE (USB_VENDOR_ID_MOSCHIP ,MOSCHIP_DEVICE_ID_7840 )}, {USB_DEVICE (USB_VENDOR_ID_MOSCHIP ,MOSCHIP_DEVICE_ID_7820 )}, + {USB_DEVICE (USB_VENDOR_ID_MOSCHIP ,MOSCHIP_DEVICE_ID_7810 )}, {USB_DEVICE (USB_VENDOR_ID_BANDB ,BANDB_DEVICE_ID_USO9ML2_2 )}, {USB_DEVICE (USB_VENDOR_ID_BANDB ,BANDB_DEVICE_ID_USO9ML2_2P )}, {USB_DEVICE (USB_VENDOR_ID_BANDB ,BANDB_DEVICE_ID_USO9ML2_4 )},
@@ -209,6 +210,7 @@ static const struct usb_device_id
moschip_port_id_table[] = {
static const struct usb_device_id moschip_id_table_combined[]__devinitconst = {
{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7840)}, {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7820)},
+ {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7810)},
{USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_2)}, {USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_2P)}, {USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_4)},
@@ -261,6 +263,12 @@ struct moschip_port {
struct urb *write_urb_pool[NUM_URBS];
char busy[NUM_URBS];
bool read_urb_busy;
+
+ /* For MCS7810 LED */
+ int mos7810_led_flag;
+ struct timer_list mos7810_led_timer1; /* For LED on 500ms */
+ struct timer_list mos7810_led_timer2; /* For LED off 500ms */
+
};
@@ -572,6 +580,68 @@ static int mos7840_get_reg(struct moschip_port *mcs, __u16 Wval, __u16 reg,
return ret;
}
+static void mos7810_control_callback(struct urb *urb)
+{
+ if (!urb)
+ return;
+
+ switch (urb->status) {
+ case 0:
+ /* Success */
+ break;
+ case -ECONNRESET:
+ case -ENOENT:
+ case -ESHUTDOWN:
+ /* This urb is terminated, clean up */
+ dbg("%s - urb shutting down with status: %d", __func__,
+ urb->status);
+ break;
+ default:
+ dbg("%s - nonzero urb status received: %d", __func__,
+ urb->status);
+ }
+}
+
+static int mos7810_set_reg(struct moschip_port *mcs, __u16 Wval, __u16 reg) +{
+ struct usb_device *dev = mcs->port->serial->dev;
+ struct usb_ctrlrequest *dr = mcs->dr;
+
+ dr->bRequestType = MCS_WR_RTYPE;
+ dr->bRequest = MCS_WRREQ;
+ dr->wValue = cpu_to_le16(Wval);
+ dr->wIndex = cpu_to_le16(reg);
+ dr->wLength = cpu_to_le16(0);
+
+ usb_fill_control_urb(mcs->control_urb, dev, usb_sndctrlpipe(dev, 0),
+ (unsigned char *)dr, NULL, 0, mos7810_control_callback,
NULL);
+
+ return usb_submit_urb(mcs->control_urb, GFP_ATOMIC);
+}
+
+static void mos7810_led_off(unsigned long arg)
+{
+ struct moschip_port *mcs = (struct moschip_port *) arg;
+
+ if (!mcs)
+ return;
+
+ /* Turn off MCS7810 LED */
+ mos7810_set_reg(mcs, 0x0300, MODEM_CONTROL_REGISTER);
+ mod_timer(&mcs->mos7810_led_timer2, jiffies + 500);
+}
+
+static void mos7810_led_flag_off(unsigned long arg)
+{
+ struct moschip_port *mcs = (struct moschip_port *) arg;
+
+ if (!mcs)
+ return;
+
+ mcs->mos7810_led_flag = 0;
+
+}
+
/*************************************************************************** **
* mos7840_interrupt_callback
* this is the callback function for when we have received data on the
@@ -792,6 +862,12 @@ static void mos7840_bulk_in_callback(struct urb *urb)
return;
}
+ /* Turn on MCS7810 LED */
+ if (serial->num_ports == 1 && mos7840_port->mos7810_led_flag == 0) {
+ mos7840_port->mos7810_led_flag = 1;
+ mos7810_set_reg(mos7840_port, 0x0301,MODEM_CONTROL_REGISTER);
+ mod_timer(&mos7840_port->mos7810_led_timer1, jiffies + 500);
+ }
mos7840_port->read_urb->dev = serial->dev;
@@ -1556,6 +1632,18 @@ static int mos7840_write(struct tty_struct *tty, struct usb_serial_port *port,
data1 = urb->transfer_buffer;
dbg("bulkout endpoint is %d", port->bulk_out_endpointAddress);
+ /* Turn on MCS7810 LED */
+ if (serial->num_ports == 1 && mos7840_port->mos7810_led_flag == 0) {
+
+ mos7840_port->mos7810_led_flag = 1;
+
+ usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev,
0),
+ MCS_WRREQ, MCS_WR_RTYPE, 0x0301,
+ MODEM_CONTROL_REGISTER, NULL, 0, MOS_WDR_TIMEOUT);
+
+ mod_timer(&mos7840_port->mos7810_led_timer1, jiffies + 500);
+ }
+
/* send it down the pipe */
status = usb_submit_urb(urb, GFP_ATOMIC);
@@ -2331,6 +2419,48 @@ static int mos7840_ioctl(struct tty_struct *tty,
return -ENOIOCTLCMD;
}
+static int mos7810_check(struct usb_serial *serial)
+{
+ int i, pass_count = 0;
+ __u16 Data = 0, MCR_Data = 0;
+ __u16 test_pattern = 0x55AA, write_pattern;
+
+ /* Store MCR setting */
+ usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
+ MCS_RDREQ, MCS_RD_RTYPE, 0x0300, MODEM_CONTROL_REGISTER,
+ &MCR_Data, VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT);
+
+ for (i = 0; i < 16; i++) {
+ write_pattern = 0x0300 | (((test_pattern >> i) & 0x0001) <<
1);
+
+ /* Send the 1-bit test pattern out to RTS pin */
+ usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev,
0),
+ MCS_WRREQ, MCS_WR_RTYPE, write_pattern,
+ MODEM_CONTROL_REGISTER, NULL, 0, MOS_WDR_TIMEOUT);
+
+ /* Read the test pattern back from GPIO1 pin */
+ usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev,
0),
+ MCS_RDREQ, MCS_RD_RTYPE, 0, GPIO_REGISTER, &Data,
+ VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT);
+
+ /* If this a MCS7810 device, both test patterns must match
*/
+ if (((test_pattern >> i) & 0x0001) != ((~(Data >> 1)) &0x0001))
+ break;
+
+ pass_count++;
+ }
+
+ /* Restore MCR setting */
+ usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),MCS_WRREQ,
+ MCS_WR_RTYPE, 0x0300 | MCR_Data, MODEM_CONTROL_REGISTER,
NULL,
+ 0, MOS_WDR_TIMEOUT);
+
+ if (pass_count == 16)
+ return 1;
+
+ return 0;
+}
+
static int mos7840_calc_num_ports(struct usb_serial *serial)
{
__u16 Data = 0x00;
@@ -2341,16 +2471,35 @@ static int mos7840_calc_num_ports(struct usb_serial *serial)
MCS_RDREQ, MCS_RD_RTYPE, 0, GPIO_REGISTER, &Data, VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT);
- if ((Data&0x01) == 0) {
+ if (serial->dev->descriptor.idProduct == MOSCHIP_DEVICE_ID_7810) {
+ mos7840_num_ports = 1;
+ serial->num_bulk_in = 1;
+ serial->num_bulk_out = 1;
+ serial->num_ports = 1;
+ } else if (serial->dev->descriptor.idProduct ==
+ MOSCHIP_DEVICE_ID_7820) {
mos7840_num_ports = 2;
serial->num_bulk_in = 2;
serial->num_bulk_out = 2;
serial->num_ports = 2;
} else {
- mos7840_num_ports = 4;
- serial->num_bulk_in = 4;
- serial->num_bulk_out = 4;
- serial->num_ports = 4;
+ /* For a MCS7840 device GPIO0 must be set to 1*/
+ if ((Data & 0x01) == 1) {
+ mos7840_num_ports = 4;
+ serial->num_bulk_in = 4;
+ serial->num_bulk_out = 4;
+ serial->num_ports = 4;
+ } else if (mos7810_check(serial)) {
+ mos7840_num_ports = 1;
+ serial->num_bulk_in = 1;
+ serial->num_bulk_out = 1;
+ serial->num_ports = 1;
+ } else {
+ mos7840_num_ports = 2;
+ serial->num_bulk_in = 2;
+ serial->num_bulk_out = 2;
+ serial->num_ports = 2;
+ }
}
return mos7840_num_ports;
@@ -2567,6 +2716,33 @@ static int mos7840_startup(struct usb_serial *serial)
status = -ENOMEM;
goto error;
}
+
+ /* Initialize MCS7810 LED timers */
+ if (serial->num_ports == 1) {
+ init_timer(&mos7840_port->mos7810_led_timer1);
+ mos7840_port->mos7810_led_timer1.function =
+ mos7810_led_off;
+ mos7840_port->mos7810_led_timer1.expires =
+ jiffies + 500;
+ mos7840_port->mos7810_led_timer1.data =
+ (unsigned long)mos7840_port;
+
+ init_timer(&mos7840_port->mos7810_led_timer2);
+ mos7840_port->mos7810_led_timer2.function =
+mos7810_led_flag_off;
+ mos7840_port->mos7810_led_timer2.expires =
+ jiffies + 500;
+ mos7840_port->mos7810_led_timer2.data =
+ (unsigned long)mos7840_port;
+
+ mos7840_port->mos7810_led_flag = 0;
+
+ /* Turn off MCS7810 LED */
+ usb_control_msg(serial->dev,
+ usb_sndctrlpipe(serial->dev, 0), MCS_WRREQ,
+ MCS_WR_RTYPE, 0x0300,MODEM_CONTROL_REGISTER,
+ NULL, 0, MOS_WDR_TIMEOUT);
+ }
}
dbg ("mos7840_startup: all ports configured...........");
@@ -2658,6 +2834,21 @@ static void mos7840_release(struct usb_serial *serial)
mos7840_port = mos7840_get_port_private(serial->port[i]); dbg("mos7840_port %d = %p", i, mos7840_port); if (mos7840_port) {
+
+ if (serial->num_ports == 1) {
+ /* Turn off MCS7810 LED */
+ usb_control_msg(serial->dev,
+ usb_sndctrlpipe(serial->dev, 0),
+ MCS_WRREQ, MCS_WR_RTYPE, 0x0300,
+ MODEM_CONTROL_REGISTER, NULL, 0,
+ MOS_WDR_TIMEOUT);
+
+ del_timer_sync(&mos7840_port->
+ mos7810_led_timer1);
+ del_timer_sync(&mos7840_port->
+ mos7810_led_timer2);
+ }
+
kfree(mos7840_port->ctrl_buf); kfree(mos7840_port->dr); kfree(mos7840_port);
Signature · 1 line
1.7.7.6
Re: Patch "USB: serial: mos7840: Supported MCS7810 device"
Review
Quoted 5 lines
(quoted diff) +4
-0+static void mos7810_control_callback(struct urb *urb) +{ + if (!urb) + return;
How can this ever occur. If its not theoretically possible then this check is just going to hide bugs in future.
Review
Quoted 6 lines
(quoted diff) +6
-0+static void mos7810_led_off(unsigned long arg) +{ + struct moschip_port *mcs = (struct moschip_port *) arg; + + if (!mcs) + return;
Ditto
Review
Quoted 7 lines
(quoted diff) +4
-0+ /* Turn on MCS7810 LED */ + if (serial->num_ports == 1 && mos7840_port->mos7810_led_flag == 0) { + mos7840_port->mos7810_led_flag = 1; + mos7810_set_reg(mos7840_port, 0x0301,
MODEM_CONTROL_REGISTER);(quoted diff) +2
-0+ mod_timer(&mos7840_port->mos7810_led_timer1, jiffies + 500); + }
The assumption that serial->num_ports = 1 means "has LED" is going to cause problems later if another deivce appears with 1 port and no LED. I think it would be much clearer to have something like
if (mos7840_port->has_led)
for these tests of the driver
Review
Quoted 5 lines
(quoted diff) +5
-0+static int mos7810_check(struct usb_serial *serial) +{ + int i, pass_count = 0; + __u16 Data = 0, MCR_Data = 0; + __u16 test_pattern = 0x55AA, write_pattern;
generally linux style is all lower case for egister names. Not a big deal
Quoted 4 lines
(quoted diff) +1
-0+ /* If this a MCS7810 device, both test patterns must match*/
(quoted diff) +1
-0+ if (((test_pattern >> i) & 0x0001) != ((~(Data >> 1)) &
0x0001))
if (((test_pattern >> i) ^ (~Data >> 1)) & 0x0001) ?
again trivia...
Review
Quoted 2 lines
(quoted diff) +2
-0+ mos7840_port->mos7810_led_timer1.expires = + jiffies + 500;
This 500 value depends upon the HZ value of the kernel which won't always be 1000 on all platforms. You probably want
jiffies + msecs_to_jiffies(500);
(or indeed HZ/2)
Review
Quoted 7 lines
(quoted diff) +7
-0+ /* Turn off MCS7810 LED */ + usb_control_msg(serial->dev, + usb_sndctrlpipe(serial->dev, 0), + MCS_WRREQ, MCS_WR_RTYPE, 0x0300, + MODEM_CONTROL_REGISTER, NULL, 0, + MOS_WDR_TIMEOUT); +
Might be clearer if these occurences were a separate small function - gcc will inline them if it makes sense, and it will be clearer.
Alan
RE: Patch "USB: serial: mos7840: Supported MCS7810 device"
Hi Alan,
Thank you for your kind comments for this patch. I will refine the patch according to your comments and re-submit it then.
Regards,
Donald
-----Original Message-----
From: Alan Cox [mailto:[email protected]]
Sent: Thursday, March 29, 2012 11:04 PM
To: Donald
Cc: 'Greg KH'; 'open list:USB SUBSYSTEM'; 'open list'
Subject: Re: Patch "USB: serial: mos7840: Supported MCS7810 device"
Review
Quoted 3 lines
(quoted diff) +3
-0+static void mos7810_control_callback(struct urb *urb) { + if (!urb) + return;
How can this ever occur. If its not theoretically possible then this check is just going to hide bugs in future.
Review
Quoted 5 lines
(quoted diff) +5
-0+static void mos7810_led_off(unsigned long arg) { + struct moschip_port *mcs = (struct moschip_port *) arg; + + if (!mcs) + return;
Ditto
Review
Quoted 7 lines
(quoted diff) +4
-0+ /* Turn on MCS7810 LED */ + if (serial->num_ports == 1 && mos7840_port->mos7810_led_flag == 0) { + mos7840_port->mos7810_led_flag = 1; + mos7810_set_reg(mos7840_port, 0x0301,
MODEM_CONTROL_REGISTER);(quoted diff) +2
-0+ mod_timer(&mos7840_port->mos7810_led_timer1, jiffies + 500); + }
The assumption that serial->num_ports = 1 means "has LED" is going to cause problems later if another deivce appears with 1 port and no LED. I think it would be much clearer to have something like
if (mos7840_port->has_led)
for these tests of the driver
Review
Quoted 4 lines
(quoted diff) +4
-0+static int mos7810_check(struct usb_serial *serial) { + int i, pass_count = 0; + __u16 Data = 0, MCR_Data = 0; + __u16 test_pattern = 0x55AA, write_pattern;
generally linux style is all lower case for egister names. Not a big deal
Quoted 4 lines
(quoted diff) +1
-0+ /* If this a MCS7810 device, both test patterns must match*/
(quoted diff) +1
-0+ if (((test_pattern >> i) & 0x0001) != ((~(Data >> 1)) &
0x0001))
if (((test_pattern >> i) ^ (~Data >> 1)) & 0x0001) ?
again trivia...
Review
Quoted 2 lines
(quoted diff) +2
-0+ mos7840_port->mos7810_led_timer1.expires = + jiffies + 500;
This 500 value depends upon the HZ value of the kernel which won't always be 1000 on all platforms. You probably want
jiffies + msecs_to_jiffies(500);
(or indeed HZ/2)
Review
Quoted 7 lines
(quoted diff) +7
-0+ /* Turn off MCS7810 LED */ + usb_control_msg(serial->dev, + usb_sndctrlpipe(serial->dev, 0), + MCS_WRREQ, MCS_WR_RTYPE, 0x0300, + MODEM_CONTROL_REGISTER, NULL, 0, + MOS_WDR_TIMEOUT); +
Might be clearer if these occurences were a separate small function - gcc will inline them if it makes sense, and it will be clearer.
Alan
'Greg KH'
maintainerRe: Patch "USB: serial: mos7840: Supported MCS7810 device"
In reply to DonaldQuoted 9 lines · Donald
Hi Greg,
I am submitting this patch that supports MCS7810 device for the mos7840 driver. If you see any problem regarding this patch, please let me know at any time. Thank you for your help.
Regards,
Donald
Quoted 6 lines
Patch Description:
This patch added the support of MCS7810 device for the mos7840 driver. The MCS7810 device supports single USB2.0-to-Serial port with a LED indicator for reflecting transmission or reception activity. Please be noted that part of codes in his patch are relating to the codes in last submitted patch that fixed MCS7820 device attach problem.
Along with Alan's comments, I need this to be able to be applied, and if you include portions of the patch that I have already accepted, I can't apply it, right?
Can you redo this patch against 3.4-rc1 when it comes out in a few days?
thanks,
greg k-h
RE: Patch "USB: serial: mos7840: Supported MCS7810 device"
Hi Greg,
Thank you for your quick reply. I will refine the patch according to Alan's comments and redo this patch against 3.4-rc1 and re-submit it ASAP.
Regards,
Donald
-----Original Message-----
From: 'Greg KH' [mailto:[email protected]]
Sent: Thursday, March 29, 2012 11:14 PM
To: Donald
Cc: 'open list:USB SUBSYSTEM'; 'open list'
Subject: Re: Patch "USB: serial: mos7840: Supported MCS7810 device"
Quoted 9 lines · Donald
Hi Greg,
I am submitting this patch that supports MCS7810 device for the mos7840 driver. If you see any problem regarding this patch, please let me know at any time. Thank you for your help.
Regards,
Donald
Quoted 6 lines
Patch Description:
This patch added the support of MCS7810 device for the mos7840 driver. The MCS7810 device supports single USB2.0-to-Serial port with a LED indicator for reflecting transmission or reception activity. Please be noted that part of codes in his patch are relating to the codes in last submitted patch that fixed MCS7820 device attach problem.
Along with Alan's comments, I need this to be able to be applied, and if you include portions of the patch that I have already accepted, I can't apply it, right?
Can you redo this patch against 3.4-rc1 when it comes out in a few days?
thanks,
greg k-h
Patch "USB: serial: mos7840: Supported MCS7810 device"
In reply to 'Greg KH'Hi Greg,
I am re-submitting this patch that supports MCS7810 device for the mos7840 driver. This patch was created against 3.4-rc1 and has been verified on 3.4-rc1 also. If you see any problem regarding this patch, please let me know at any time. Thank you for your help.
Besides, I found two things regarding 3.4-rc, firstly, it added a macro to replace module_init() and module_exit(); secondly, the system will crash if I remove module before unplug the dongle; while this issue doesn't happen on Linux kernel 3.2.9. I had been tested mcs7840 and prolific-pl2303, both drivers have the same scenario. I am not sure if this issue is relating to the macro added in 3.4-rc and would like to know if you have any comment on this issue.
Regards,
Donald
Patch Description:
This patch added the support of MCS7810 device for the mos7840 driver. The MCS7810 device supports single USB2.0-to-Serial port with a LED indicator for reflecting transmission or reception activity.
--- drivers/usb/serial/mos7840.c | 215 ++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 208 insertions(+), 7 deletions(-)
drivers/usb/serial/mos7840.c +8 -0
@@ -114,6 +114,7 @@#define USB_VENDOR_ID_MOSCHIP 0x9710#define MOSCHIP_DEVICE_ID_7840 0x7840#define MOSCHIP_DEVICE_ID_7820 0x7820 +#define MOSCHIP_DEVICE_ID_7810 0x7810/* The native component can have its vendor/device id's overridden * in vendor-specific implementations. Such devices can be handled * by making a change here, in moschip_port_id_table, and in
@@ -184,10 +185,17 @@#define NUM_URBS 16/* URB Count */ #define URB_TRANSFER_BUFFER_SIZE 32/* URB Size */ +/* MCS7810 LED support */ +#define MCS7810_HAS_LED + +/* MCS7810 LED on/off milliseconds*/ +#define MCS7810_LED_ON_MS 500 +#define MCS7810_LED_OFF_MS 500static const struct usb_device_id moschip_port_id_table[] = { {USB_DEVICE (USB_VENDOR_ID_MOSCHIP ,MOSCHIP_DEVICE_ID_7840 )}, {USB_DEVICE (USB_VENDOR_ID_MOSCHIP ,MOSCHIP_DEVICE_ID_7820 )}, + {USB_DEVICE (USB_VENDOR_ID_MOSCHIP ,MOSCHIP_DEVICE_ID_7810 )}, {USB_DEVICE (USB_VENDOR_ID_BANDB ,BANDB_DEVICE_ID_USO9ML2_2 )}, {USB_DEVICE (USB_VENDOR_ID_BANDB ,BANDB_DEVICE_ID_USO9ML2_2P )}, {USB_DEVICE (USB_VENDOR_ID_BANDB ,BANDB_DEVICE_ID_USO9ML2_4 )},
@@ -209,6 +217,7 @@ static const struct usb_device_id
moschip_port_id_table[] = {
static const struct usb_device_id moschip_id_table_combined[]__devinitconst = {
{USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7840)}, {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7820)},
+ {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7810)},
{USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_2)}, {USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_2P)}, {USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_4)},
@@ -261,6 +270,13 @@ struct moschip_port {
struct urb *write_urb_pool[NUM_URBS];
char busy[NUM_URBS];
bool read_urb_busy;
+
+#ifdef MCS7810_HAS_LED
+ /* For MCS7810 LED */
+ int mos7810_led_flag;
+ struct timer_list mos7810_led_timer1; /* Timer for LED on */
+ struct timer_list mos7810_led_timer2; /* Timer for LED off */
+#endif
};
@@ -572,6 +588,71 @@ static int mos7840_get_reg(struct moschip_port *mcs, __u16 Wval, __u16 reg,
return ret;
}
+#ifdef MCS7810_HAS_LED
+static void mos7810_control_callback(struct urb *urb)
+{
+ switch (urb->status) {
+ case 0:
+ /* Success */
+ break;
+ case -ECONNRESET:
+ case -ENOENT:
+ case -ESHUTDOWN:
+ /* This urb is terminated, clean up */
+ dbg("%s - urb shutting down with status: %d", __func__,
+ urb->status);
+ break;
+ default:
+ dbg("%s - nonzero urb status received: %d", __func__,
+ urb->status);
+ }
+}
+
+static void mos7810_set_led_async(struct moschip_port *mcs, __u16 wval,
+ __u16 reg)
+{
+ struct usb_device *dev = mcs->port->serial->dev;
+ struct usb_ctrlrequest *dr = mcs->dr;
+
+ dr->bRequestType = MCS_WR_RTYPE;
+ dr->bRequest = MCS_WRREQ;
+ dr->wValue = cpu_to_le16(wval);
+ dr->wIndex = cpu_to_le16(reg);
+ dr->wLength = cpu_to_le16(0);
+
+ usb_fill_control_urb(mcs->control_urb, dev, usb_sndctrlpipe(dev, 0),
+ (unsigned char *)dr, NULL, 0, mos7810_control_callback,
NULL);
+
+ usb_submit_urb(mcs->control_urb, GFP_ATOMIC);
+}
+
+static void mos7810_set_led_sync(struct usb_serial_port *port, __u16 reg,
+ __u16 val)
+{
+ struct usb_device *dev = port->serial->dev;
+
+ usb_control_msg(dev, usb_sndctrlpipe(dev, 0), MCS_WRREQ,MCS_WR_RTYPE,
+ val, reg, NULL, 0, MOS_WDR_TIMEOUT);
+}
+
+static void mos7810_led_off(unsigned long arg)
+{
+ struct moschip_port *mcs = (struct moschip_port *) arg;
+
+ /* Turn off MCS7810 LED */
+ mos7810_set_led_async(mcs, 0x0300, MODEM_CONTROL_REGISTER);
+ mod_timer(&mcs->mos7810_led_timer2,
+ jiffies +msecs_to_jiffies(MCS7810_LED_OFF_MS));
+}
+
+static void mos7810_led_flag_off(unsigned long arg)
+{
+ struct moschip_port *mcs = (struct moschip_port *) arg;
+
+ mcs->mos7810_led_flag = 0;
+}
+#endif
+
/*************************************************************************** **
* mos7840_interrupt_callback
* this is the callback function for when we have received data on the
@@ -792,6 +873,16 @@ static void mos7840_bulk_in_callback(struct urb *urb)
return;
}
+#ifdef MCS7810_HAS_LED
+ /* Turn on MCS7810 LED */
+ if (serial->num_ports == 1 && mos7840_port->mos7810_led_flag == 0) {
+ mos7840_port->mos7810_led_flag = 1;
+ mos7810_set_led_async(mos7840_port, 0x0301,
+ MODEM_CONTROL_REGISTER);
+ mod_timer(&mos7840_port->mos7810_led_timer1,
+ jiffies +msecs_to_jiffies(MCS7810_LED_ON_MS));
+ }
+#endif
mos7840_port->read_urb_busy = true;
retval = usb_submit_urb(mos7840_port->read_urb, GFP_ATOMIC);
@@ -1554,6 +1645,16 @@ static int mos7840_write(struct tty_struct *tty, struct usb_serial_port *port,
data1 = urb->transfer_buffer;
dbg("bulkout endpoint is %d", port->bulk_out_endpointAddress);
+#ifdef MCS7810_HAS_LED
+ /* Turn on MCS7810 LED */
+ if (serial->num_ports == 1 && mos7840_port->mos7810_led_flag == 0) {
+ mos7840_port->mos7810_led_flag = 1;
+ mos7810_set_led_sync(port, MODEM_CONTROL_REGISTER, 0x0301);
+ mod_timer(&mos7840_port->mos7810_led_timer1,
+ jiffies +msecs_to_jiffies(MCS7810_LED_ON_MS));
+ }
+#endif
+
/* send it down the pipe */
status = usb_submit_urb(urb, GFP_ATOMIC);
@@ -2327,26 +2428,86 @@ static int mos7840_ioctl(struct tty_struct *tty,
return -ENOIOCTLCMD;
}
+static int mos7810_check(struct usb_serial *serial) +{
+ int i, pass_count = 0;
+ __u16 data = 0, mcr_data = 0;
+ __u16 test_pattern = 0x55AA;
+
+ /* Store MCR setting */
+ usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
+ MCS_RDREQ, MCS_RD_RTYPE, 0x0300, MODEM_CONTROL_REGISTER,
+ &mcr_data, VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT);
+
+ for (i = 0; i < 16; i++) {
+ /* Send the 1-bit test pattern out to MCS7810 test pin */
+ usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev,
0),
+ MCS_WRREQ, MCS_WR_RTYPE,
+ (0x0300 | (((test_pattern >> i) & 0x0001) << 1)),
+ MODEM_CONTROL_REGISTER, NULL, 0, MOS_WDR_TIMEOUT);
+
+ /* Read the test pattern back */
+ usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev,
0),
+ MCS_RDREQ, MCS_RD_RTYPE, 0, GPIO_REGISTER, &data,
+ VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT);
+
+ /* If this is a MCS7810 device, both test patterns must
match */
+ if (((test_pattern >> i) ^ (~data >> 1)) & 0x0001)
+ break;
+
+ pass_count++;
+ }
+
+ /* Restore MCR setting */
+ usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),MCS_WRREQ,
+ MCS_WR_RTYPE, 0x0300 | mcr_data, MODEM_CONTROL_REGISTER,
NULL,
+ 0, MOS_WDR_TIMEOUT);
+
+ if (pass_count == 16)
+ return 1;
+
+ return 0;
+}
+
static int mos7840_calc_num_ports(struct usb_serial *serial) {
- __u16 Data = 0x00;
+ __u16 data = 0x00;
int ret = 0;
int mos7840_num_ports;
ret = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
- MCS_RDREQ, MCS_RD_RTYPE, 0, GPIO_REGISTER, &Data,
+ MCS_RDREQ, MCS_RD_RTYPE, 0, GPIO_REGISTER, &data,
VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT);
- if ((Data & 0x01) == 0) {
+ if (serial->dev->descriptor.idProduct == MOSCHIP_DEVICE_ID_7810) {
+ mos7840_num_ports = 1;
+ serial->num_bulk_in = 1;
+ serial->num_bulk_out = 1;
+ serial->num_ports = 1;
+ } else if (serial->dev->descriptor.idProduct ==
+ MOSCHIP_DEVICE_ID_7820) {
mos7840_num_ports = 2;
serial->num_bulk_in = 2;
serial->num_bulk_out = 2;
serial->num_ports = 2;
} else {
- mos7840_num_ports = 4;
- serial->num_bulk_in = 4;
- serial->num_bulk_out = 4;
- serial->num_ports = 4;
+ /* For a MCS7840 device GPIO0 must be set to 1*/
+ if ((data & 0x01) == 1) {
+ mos7840_num_ports = 4;
+ serial->num_bulk_in = 4;
+ serial->num_bulk_out = 4;
+ serial->num_ports = 4;
+ } else if (mos7810_check(serial)) {
+ mos7840_num_ports = 1;
+ serial->num_bulk_in = 1;
+ serial->num_bulk_out = 1;
+ serial->num_ports = 1;
+ } else {
+ mos7840_num_ports = 2;
+ serial->num_bulk_in = 2;
+ serial->num_bulk_out = 2;
+ serial->num_ports = 2;
+ }
}
return mos7840_num_ports;
@@ -2563,6 +2724,33 @@ static int mos7840_startup(struct usb_serial *serial)
status = -ENOMEM;
goto error;
}
+
+#ifdef MCS7810_HAS_LED
+ /* Initialize MCS7810 LED timers */
+ if (serial->num_ports == 1) {
+ init_timer(&mos7840_port->mos7810_led_timer1);
+ mos7840_port->mos7810_led_timer1.function =
+ mos7810_led_off;
+ mos7840_port->mos7810_led_timer1.expires =
+ jiffies +msecs_to_jiffies(MCS7810_LED_ON_MS);
+ mos7840_port->mos7810_led_timer1.data =
+ (unsigned long)mos7840_port;
+
+ init_timer(&mos7840_port->mos7810_led_timer2);
+ mos7840_port->mos7810_led_timer2.function =
+mos7810_led_flag_off;
+ mos7840_port->mos7810_led_timer2.expires =
+ jiffies +msecs_to_jiffies(MCS7810_LED_OFF_MS);
+ mos7840_port->mos7810_led_timer2.data =
+ (unsigned long)mos7840_port;
+
+ mos7840_port->mos7810_led_flag = 0;
+
+ /* Turn off MCS7810 LED */
+ mos7810_set_led_sync(serial->port[i],
+ MODEM_CONTROL_REGISTER,0x0300);
+ }
+#endif
}
dbg ("mos7840_startup: all ports configured...........");
@@ -2638,6 +2826,7 @@ static void mos7840_release(struct usb_serial *serial)
{
int i;
struct moschip_port *mos7840_port;
+
dbg("%s", " release :entering..........");
if (!serial) {
@@ -2654,6 +2843,18 @@ static void mos7840_release(struct usb_serial *serial)
mos7840_port = mos7840_get_port_private(serial->port[i]); dbg("mos7840_port %d = %p", i, mos7840_port); if (mos7840_port) {
+#ifdef MCS7810_HAS_LED
+ if (serial->num_ports == 1) {
+ /* Turn off MCS7810 LED */
+ mos7810_set_led_sync(mos7840_port->port,
+ MODEM_CONTROL_REGISTER,0x0300);
+
+ del_timer_sync(&mos7840_port->
+ mos7810_led_timer1);
+ del_timer_sync(&mos7840_port->
+ mos7810_led_timer2);
+ }
+#endif
kfree(mos7840_port->ctrl_buf); kfree(mos7840_port->dr); kfree(mos7840_port);
Signature · 1 line
1.7.7.6
Re: Patch "USB: serial: mos7840: Supported MCS7810 device"
Quoted 7 lines · Donald
Hi Greg,
I am re-submitting this patch that supports MCS7810 device for the mos7840 driver. This patch was created against 3.4-rc1 and has been verified on 3.4-rc1 also. If you see any problem regarding this patch, please let me know at any time. Thank you for your help.
Quoted 7 lines
Besides, I found two things regarding 3.4-rc, firstly, it added a macro to replace
module_init()andmodule_exit(); secondly, the system will crash if I remove module before unplug the dongle; while this issue doesn't happen on Linux kernel 3.2.9. I had been tested mcs7840 and prolific-pl2303, both drivers have the same scenario. I am not sure if this issue is relating to the macro added in 3.4-rc and would like to know if you have any comment on this issue.
This is a known issue which has been fixed by the following patch:
http://marc.info/?l=linux-usb&m=133296552518354&w=2
It should show up in 3.4-rc shortly.
Thanks,
Johan
RE: Patch "USB: serial: mos7840: Supported MCS7810 device"
Hi Johan,
Thank you for your kind and quick response.
Regards,
Donald
-----Original Message-----
From: Johan Hovold [mailto:[email protected]]
Sent: Wednesday, April 11, 2012 7:08 PM
To: Donald
Cc: 'Greg KH'; 'open list:USB SUBSYSTEM'; 'open list'
Subject: Re: Patch "USB: serial: mos7840: Supported MCS7810 device"
Quoted 8 lines · Donald
Hi Greg,
I am re-submitting this patch that supports MCS7810 device for the mos7840 driver. This patch was created against 3.4-rc1 and has been verified on
3.4-rc1 also. If you see any problem regarding this patch, please let me know at any time. Thank you for your help.
Quoted 7 lines
Besides, I found two things regarding 3.4-rc, firstly, it added a macro to replace
module_init()andmodule_exit(); secondly, the system will crash if I remove module before unplug the dongle; while this issue doesn't happen on Linux kernel 3.2.9. I had been tested mcs7840 and prolific-pl2303, both drivers have the same scenario. I am not sure if this issue is relating to the macro added in 3.4-rc and would like to know if you have any comment on this issue.
This is a known issue which has been fixed by the following patch:
http://marc.info/?l=linux-usb&m=133296552518354&w=2
It should show up in 3.4-rc shortly.
Thanks,
Johan
'Greg KH'
maintainerRe: Patch "USB: serial: mos7840: Supported MCS7810 device"
In reply to DonaldQuoted 6 lines · Donald
Hi Greg,
I am re-submitting this patch that supports MCS7810 device for the mos7840 driver. This patch was created against 3.4-rc1 and has been verified on 3.4-rc1 also. If you see any problem regarding this patch, please let me know at any time. Thank you for your help.
The patch is line-wrapped and can not be applied :(
Care to fix your email client (read the file Documentation/email_clients.txt for some hints) and try it again?
thanks,
greg k-h