From 29d15361385f0792bd9e8abb10604ebc4b96cf5b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 10 Oct 2017 11:25:22 -0700 Subject: [PATCH 01/14] USB: serial: garmin_gps: convert timers to use timer_setup() In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Cc: Johan Hovold Cc: Allen Pais Cc: Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org Signed-off-by: Kees Cook Signed-off-by: Johan Hovold --- drivers/usb/serial/garmin_gps.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c index b2f2e87aed94..1f439b6e7e6f 100644 --- a/drivers/usb/serial/garmin_gps.c +++ b/drivers/usb/serial/garmin_gps.c @@ -1370,9 +1370,9 @@ static void garmin_unthrottle(struct tty_struct *tty) * the tty in cases where the protocol provides no own handshaking * to initiate the transfer. */ -static void timeout_handler(unsigned long data) +static void timeout_handler(struct timer_list *t) { - struct garmin_data *garmin_data_p = (struct garmin_data *) data; + struct garmin_data *garmin_data_p = from_timer(garmin_data_p, t, timer); /* send the next queued packet to the tty port */ if (garmin_data_p->mode == MODE_NATIVE) @@ -1391,12 +1391,10 @@ static int garmin_port_probe(struct usb_serial_port *port) if (!garmin_data_p) return -ENOMEM; - init_timer(&garmin_data_p->timer); + timer_setup(&garmin_data_p->timer, timeout_handler, 0); spin_lock_init(&garmin_data_p->lock); INIT_LIST_HEAD(&garmin_data_p->pktlist); /* garmin_data_p->timer.expires = jiffies + session_timeout; */ - garmin_data_p->timer.data = (unsigned long)garmin_data_p; - garmin_data_p->timer.function = timeout_handler; garmin_data_p->port = port; garmin_data_p->state = 0; garmin_data_p->flags = 0; From 19a565d9af6e0d828bd0d521d3bafd5017f4ce52 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 11 Oct 2017 14:02:57 +0200 Subject: [PATCH 02/14] USB: serial: garmin_gps: fix I/O after failed probe and remove Make sure to stop any submitted interrupt and bulk-out URBs before returning after failed probe and when the port is being unbound to avoid later NULL-pointer dereferences in the completion callbacks. Also fix up the related and broken I/O cancellation on failed open and on close. (Note that port->write_urb was never submitted.) Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable # 51a2f077 ("USB: introduce usb_anchor") Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/garmin_gps.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c index 1f439b6e7e6f..8967de6623c4 100644 --- a/drivers/usb/serial/garmin_gps.c +++ b/drivers/usb/serial/garmin_gps.c @@ -138,6 +138,7 @@ struct garmin_data { __u8 privpkt[4*6]; spinlock_t lock; struct list_head pktlist; + struct usb_anchor write_urbs; }; @@ -905,13 +906,19 @@ static int garmin_init_session(struct usb_serial_port *port) sizeof(GARMIN_START_SESSION_REQ), 0); if (status < 0) - break; + goto err_kill_urbs; } if (status > 0) status = 0; } + return status; + +err_kill_urbs: + usb_kill_anchored_urbs(&garmin_data_p->write_urbs); + usb_kill_urb(port->interrupt_in_urb); + return status; } @@ -930,7 +937,6 @@ static int garmin_open(struct tty_struct *tty, struct usb_serial_port *port) spin_unlock_irqrestore(&garmin_data_p->lock, flags); /* shutdown any bulk reads that might be going on */ - usb_kill_urb(port->write_urb); usb_kill_urb(port->read_urb); if (garmin_data_p->state == STATE_RESET) @@ -953,7 +959,7 @@ static void garmin_close(struct usb_serial_port *port) /* shutdown our urbs */ usb_kill_urb(port->read_urb); - usb_kill_urb(port->write_urb); + usb_kill_anchored_urbs(&garmin_data_p->write_urbs); /* keep reset state so we know that we must start a new session */ if (garmin_data_p->state != STATE_RESET) @@ -1037,12 +1043,14 @@ static int garmin_write_bulk(struct usb_serial_port *port, } /* send it down the pipe */ + usb_anchor_urb(urb, &garmin_data_p->write_urbs); status = usb_submit_urb(urb, GFP_ATOMIC); if (status) { dev_err(&port->dev, "%s - usb_submit_urb(write bulk) failed with status = %d\n", __func__, status); count = status; + usb_unanchor_urb(urb); kfree(buffer); } @@ -1399,6 +1407,7 @@ static int garmin_port_probe(struct usb_serial_port *port) garmin_data_p->state = 0; garmin_data_p->flags = 0; garmin_data_p->count = 0; + init_usb_anchor(&garmin_data_p->write_urbs); usb_set_serial_port_data(port, garmin_data_p); status = garmin_init_session(port); @@ -1411,6 +1420,7 @@ static int garmin_port_remove(struct usb_serial_port *port) { struct garmin_data *garmin_data_p = usb_get_serial_port_data(port); + usb_kill_anchored_urbs(&garmin_data_p->write_urbs); usb_kill_urb(port->interrupt_in_urb); del_timer_sync(&garmin_data_p->timer); kfree(garmin_data_p); From 74d471b598444b7f2d964930f7234779c80960a0 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 11 Oct 2017 14:02:58 +0200 Subject: [PATCH 03/14] USB: serial: garmin_gps: fix memory leak on probe errors Make sure to free the port private data before returning after a failed probe attempt. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/garmin_gps.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c index 8967de6623c4..677558c99a97 100644 --- a/drivers/usb/serial/garmin_gps.c +++ b/drivers/usb/serial/garmin_gps.c @@ -1411,6 +1411,12 @@ static int garmin_port_probe(struct usb_serial_port *port) usb_set_serial_port_data(port, garmin_data_p); status = garmin_init_session(port); + if (status) + goto err_free; + + return 0; +err_free: + kfree(garmin_data_p); return status; } From b943cee8a5294d4d0e30528ed5f776aefd2b6fc3 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 11 Oct 2017 14:02:59 +0200 Subject: [PATCH 04/14] USB: serial: garmin_gps: drop unused timer initialisation Drop out-commented timer expiry initialisation which would not even compile anymore. Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/garmin_gps.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c index 677558c99a97..d22a84c95c07 100644 --- a/drivers/usb/serial/garmin_gps.c +++ b/drivers/usb/serial/garmin_gps.c @@ -1402,7 +1402,6 @@ static int garmin_port_probe(struct usb_serial_port *port) timer_setup(&garmin_data_p->timer, timeout_handler, 0); spin_lock_init(&garmin_data_p->lock); INIT_LIST_HEAD(&garmin_data_p->pktlist); - /* garmin_data_p->timer.expires = jiffies + session_timeout; */ garmin_data_p->port = port; garmin_data_p->state = 0; garmin_data_p->flags = 0; From c1a357b68614620c976a94260749a979f9c46d29 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 11 Oct 2017 14:03:00 +0200 Subject: [PATCH 05/14] USB: serial: garmin_gps: clean up init-session messages Use the port device for any init-session error and debug messages, remove one redundant debug message and simplify one error message. Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/garmin_gps.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c index d22a84c95c07..5f7832adf6af 100644 --- a/drivers/usb/serial/garmin_gps.c +++ b/drivers/usb/serial/garmin_gps.c @@ -876,7 +876,6 @@ static int garmin_clear(struct garmin_data *garmin_data_p) static int garmin_init_session(struct usb_serial_port *port) { - struct usb_serial *serial = port->serial; struct garmin_data *garmin_data_p = usb_get_serial_port_data(port); int status = 0; int i = 0; @@ -884,12 +883,12 @@ static int garmin_init_session(struct usb_serial_port *port) if (status == 0) { usb_kill_urb(port->interrupt_in_urb); - dev_dbg(&serial->dev->dev, "%s - adding interrupt input\n", __func__); status = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); - if (status) - dev_err(&serial->dev->dev, - "%s - failed submitting interrupt urb, error %d\n", - __func__, status); + if (status) { + dev_err(&port->dev, + "failed to submit interrupt urb: %d\n", + status); + } } /* @@ -897,7 +896,7 @@ static int garmin_init_session(struct usb_serial_port *port) * gpsbabel/jeeps/gpslibusb.c gusb_reset_toggles() */ if (status == 0) { - dev_dbg(&serial->dev->dev, "%s - starting session ...\n", __func__); + dev_dbg(&port->dev, "%s - starting session ...\n", __func__); garmin_data_p->state = STATE_ACTIVE; for (i = 0; i < 3; i++) { From 9eabd28e8baf3bd7cfa729e74c664eed060958bc Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 11 Oct 2017 14:03:01 +0200 Subject: [PATCH 06/14] USB: serial: garmin_gps: simplify init-session logic Clean up the somewhat convoluted init-session logic to improve readability. Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/garmin_gps.c | 41 +++++++++++++-------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c index 5f7832adf6af..4f793c86978e 100644 --- a/drivers/usb/serial/garmin_gps.c +++ b/drivers/usb/serial/garmin_gps.c @@ -877,42 +877,33 @@ static int garmin_clear(struct garmin_data *garmin_data_p) static int garmin_init_session(struct usb_serial_port *port) { struct garmin_data *garmin_data_p = usb_get_serial_port_data(port); - int status = 0; - int i = 0; + int status; + int i; - if (status == 0) { - usb_kill_urb(port->interrupt_in_urb); + usb_kill_urb(port->interrupt_in_urb); - status = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); - if (status) { - dev_err(&port->dev, - "failed to submit interrupt urb: %d\n", - status); - } + status = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); + if (status) { + dev_err(&port->dev, "failed to submit interrupt urb: %d\n", + status); + return status; } /* * using the initialization method from gpsbabel. See comments in * gpsbabel/jeeps/gpslibusb.c gusb_reset_toggles() */ - if (status == 0) { - dev_dbg(&port->dev, "%s - starting session ...\n", __func__); - garmin_data_p->state = STATE_ACTIVE; + dev_dbg(&port->dev, "%s - starting session ...\n", __func__); + garmin_data_p->state = STATE_ACTIVE; - for (i = 0; i < 3; i++) { - status = garmin_write_bulk(port, - GARMIN_START_SESSION_REQ, - sizeof(GARMIN_START_SESSION_REQ), 0); - - if (status < 0) - goto err_kill_urbs; - } - - if (status > 0) - status = 0; + for (i = 0; i < 3; i++) { + status = garmin_write_bulk(port, GARMIN_START_SESSION_REQ, + sizeof(GARMIN_START_SESSION_REQ), 0); + if (status < 0) + goto err_kill_urbs; } - return status; + return 0; err_kill_urbs: usb_kill_anchored_urbs(&garmin_data_p->write_urbs); From 2339536d229df25c71c0900fc619289229bfecf6 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Thu, 12 Oct 2017 10:54:21 +0200 Subject: [PATCH 07/14] USB: serial: metro-usb: stop I/O after failed open Make sure to kill the interrupt-in URB after a failed open request. Apart from saving power (and avoiding stale input after a later successful open), this also prevents a NULL-deref in the completion handler if the port is manually unbound. Reviewed-by: Greg Kroah-Hartman Fixes: 704577861d5e ("USB: serial: metro-usb: get data from device in Uni-Directional mode.") Cc: stable # 3.5 Signed-off-by: Johan Hovold --- drivers/usb/serial/metro-usb.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/usb/serial/metro-usb.c b/drivers/usb/serial/metro-usb.c index cc84da8dbb84..4bb29e03dae4 100644 --- a/drivers/usb/serial/metro-usb.c +++ b/drivers/usb/serial/metro-usb.c @@ -188,7 +188,7 @@ static int metrousb_open(struct tty_struct *tty, struct usb_serial_port *port) dev_err(&port->dev, "%s - failed submitting interrupt in urb, error code=%d\n", __func__, result); - goto exit; + return result; } /* Send activate cmd to device */ @@ -197,9 +197,14 @@ static int metrousb_open(struct tty_struct *tty, struct usb_serial_port *port) dev_err(&port->dev, "%s - failed to configure device, error code=%d\n", __func__, result); - goto exit; + goto err_kill_urb; } -exit: + + return 0; + +err_kill_urb: + usb_kill_urb(port->interrupt_in_urb); + return result; } From 6f792f471fad3ed16aca9ccedacdcd48fbedf7a9 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Thu, 12 Oct 2017 10:54:22 +0200 Subject: [PATCH 08/14] USB: serial: metro-usb: simplify endpoint check Let usb-serial core verify that the interrupt-in endpoint is present when binding the interface instead of the driver verifying this at every open. Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/metro-usb.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/usb/serial/metro-usb.c b/drivers/usb/serial/metro-usb.c index 4bb29e03dae4..a64940975ac6 100644 --- a/drivers/usb/serial/metro-usb.c +++ b/drivers/usb/serial/metro-usb.c @@ -160,13 +160,6 @@ static int metrousb_open(struct tty_struct *tty, struct usb_serial_port *port) unsigned long flags = 0; int result = 0; - /* Make sure the urb is initialized. */ - if (!port->interrupt_in_urb) { - dev_err(&port->dev, "%s - interrupt urb not initialized\n", - __func__); - return -ENODEV; - } - /* Set the private data information for the port. */ spin_lock_irqsave(&metro_priv->lock, flags); metro_priv->control_state = 0; @@ -342,6 +335,7 @@ static struct usb_serial_driver metrousb_device = { .description = "Metrologic USB to Serial", .id_table = id_table, .num_ports = 1, + .num_interrupt_in = 1, .open = metrousb_open, .close = metrousb_cleanup, .read_int_callback = metrousb_read_int_callback, From ee13a25fc355650b3acf43857b7a496eb6b07f0b Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Thu, 12 Oct 2017 10:54:23 +0200 Subject: [PATCH 09/14] USB: serial: metro-usb: add missing interrupt-out endpoint check One class of "unidirectional" devices managed by this driver uses an interrupt-out endpoint to send control messages at open and close. Due to a missing endpoint sanity check, this could result in an interrupt URB being submitted to endpoint 0 instead. This would be caught by USB core (without a WARN dump), but let's verify that the expected endpoints are present at probe rather than when a port is later opened. Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/metro-usb.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/usb/serial/metro-usb.c b/drivers/usb/serial/metro-usb.c index a64940975ac6..f16915b457c9 100644 --- a/drivers/usb/serial/metro-usb.c +++ b/drivers/usb/serial/metro-usb.c @@ -53,21 +53,33 @@ MODULE_DEVICE_TABLE(usb, id_table); #define UNI_CMD_OPEN 0x80 #define UNI_CMD_CLOSE 0xFF -static inline int metrousb_is_unidirectional_mode(struct usb_serial_port *port) +static int metrousb_is_unidirectional_mode(struct usb_serial *serial) { - __u16 product_id = le16_to_cpu( - port->serial->dev->descriptor.idProduct); + u16 product_id = le16_to_cpu(serial->dev->descriptor.idProduct); return product_id == FOCUS_PRODUCT_ID_UNI; } +static int metrousb_calc_num_ports(struct usb_serial *serial, + struct usb_serial_endpoints *epds) +{ + if (metrousb_is_unidirectional_mode(serial)) { + if (epds->num_interrupt_out == 0) { + dev_err(&serial->interface->dev, "interrupt-out endpoint missing\n"); + return -ENODEV; + } + } + + return 1; +} + static int metrousb_send_unidirectional_cmd(u8 cmd, struct usb_serial_port *port) { int ret; int actual_len; u8 *buffer_cmd = NULL; - if (!metrousb_is_unidirectional_mode(port)) + if (!metrousb_is_unidirectional_mode(port->serial)) return 0; buffer_cmd = kzalloc(sizeof(cmd), GFP_KERNEL); @@ -334,8 +346,8 @@ static struct usb_serial_driver metrousb_device = { }, .description = "Metrologic USB to Serial", .id_table = id_table, - .num_ports = 1, .num_interrupt_in = 1, + .calc_num_ports = metrousb_calc_num_ports, .open = metrousb_open, .close = metrousb_cleanup, .read_int_callback = metrousb_read_int_callback, From 2944fd27d1c0e3f8d89287c2d41e7aa640b68d65 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Thu, 12 Oct 2017 17:22:28 +0200 Subject: [PATCH 10/14] USB: serial: drop unused core USB driver Drop the usb-serial-core USB driver that was registered at module init but then never used. This was a remnant dating back to 2004 (!) when this struct usb_driver was used for the generic driver; see commit bbc53b7d7322 ("USB: fix bug where removing usb-serial modules or usb serial devices could oops") in the tglx bitkeeper-history archive. Note that every usb-serial driver (including the generic one) registers its own USB (interface) driver along with its usb-serial bus drivers. Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/usb-serial.c | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index bb34f9f7eaf4..20639d5b8886 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -1201,17 +1201,6 @@ static const struct tty_operations serial_ops = { struct tty_driver *usb_serial_tty_driver; -/* Driver structure we register with the USB core */ -static struct usb_driver usb_serial_driver = { - .name = "usbserial", - .probe = usb_serial_probe, - .disconnect = usb_serial_disconnect, - .suspend = usb_serial_suspend, - .resume = usb_serial_resume, - .no_dynamic_id = 1, - .supports_autosuspend = 1, -}; - static int __init usb_serial_init(void) { int result; @@ -1247,13 +1236,6 @@ static int __init usb_serial_init(void) goto exit_reg_driver; } - /* register the USB driver */ - result = usb_register(&usb_serial_driver); - if (result < 0) { - pr_err("%s - usb_register failed\n", __func__); - goto exit_tty; - } - /* register the generic driver, if we should */ result = usb_serial_generic_register(); if (result < 0) { @@ -1264,9 +1246,6 @@ static int __init usb_serial_init(void) return result; exit_generic: - usb_deregister(&usb_serial_driver); - -exit_tty: tty_unregister_driver(usb_serial_tty_driver); exit_reg_driver: @@ -1285,7 +1264,6 @@ static void __exit usb_serial_exit(void) usb_serial_generic_deregister(); - usb_deregister(&usb_serial_driver); tty_unregister_driver(usb_serial_tty_driver); put_tty_driver(usb_serial_tty_driver); bus_unregister(&usb_serial_bus_type); From fe1f68a08fbffd8c84bf8e7f72d2d531a3fe8f50 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Tue, 24 Oct 2017 11:52:46 -0500 Subject: [PATCH 11/14] USB: serial: kobil_sct: mark expected switch fall-through In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 115014 Signed-off-by: Gustavo A. R. Silva Signed-off-by: Johan Hovold --- drivers/usb/serial/kobil_sct.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c index 3024b9b25360..2f8fa3589b20 100644 --- a/drivers/usb/serial/kobil_sct.c +++ b/drivers/usb/serial/kobil_sct.c @@ -491,6 +491,7 @@ static void kobil_set_termios(struct tty_struct *tty, break; default: speed = 9600; + /* fall through */ case 9600: urb_val = SUSBCR_SBR_9600; break; From 771394a54148f18926ca86414e51c69eda27d0cd Mon Sep 17 00:00:00 2001 From: Douglas Fischer Date: Sun, 29 Oct 2017 23:29:55 +0000 Subject: [PATCH 12/14] USB: serial: qcserial: add pid/vid for Sierra Wireless EM7355 fw update Add USB PID/VID for Sierra Wireless EM7355 LTE modem QDL firmware update mode. Signed-off-by: Douglas Fischer Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/qcserial.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c index ebc0beea69d6..ffac081e650d 100644 --- a/drivers/usb/serial/qcserial.c +++ b/drivers/usb/serial/qcserial.c @@ -148,6 +148,7 @@ static const struct usb_device_id id_table[] = { {DEVICE_SWI(0x1199, 0x68a2)}, /* Sierra Wireless MC7710 */ {DEVICE_SWI(0x1199, 0x68c0)}, /* Sierra Wireless MC7304/MC7354 */ {DEVICE_SWI(0x1199, 0x901c)}, /* Sierra Wireless EM7700 */ + {DEVICE_SWI(0x1199, 0x901e)}, /* Sierra Wireless EM7355 QDL */ {DEVICE_SWI(0x1199, 0x901f)}, /* Sierra Wireless EM7355 */ {DEVICE_SWI(0x1199, 0x9040)}, /* Sierra Wireless Modem */ {DEVICE_SWI(0x1199, 0x9041)}, /* Sierra Wireless MC7305/MC7355 */ From dea744bae7a55cc71dd8d54cdb76639907fce1ec Mon Sep 17 00:00:00 2001 From: "Ji-Ze Hong (Peter Hong)" Date: Fri, 13 Oct 2017 10:21:34 +0800 Subject: [PATCH 13/14] USB: serial: f81534: fix hang-up on overrun The F81532/534 without this patch will hang-up on data overrun. It's caused by enable LSR interrupt in IER by default and occur data overrun, the chip will busy for process LSR interrupt but not read LSR internally. It will not responed for USB control endpoint0 and we can't read LSR from driver in this situration. So we'll disable the LSR interrupt in probe() and submit the LSR worker to clear LSR state when reported LSR error bit with bulk-in data in f81534_process_per_serial_block(). Signed-off-by: Ji-Ze Hong (Peter Hong) Signed-off-by: Johan Hovold --- drivers/usb/serial/f81534.c | 43 +++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c index 3d616a2a9f96..4ced064dbcc4 100644 --- a/drivers/usb/serial/f81534.c +++ b/drivers/usb/serial/f81534.c @@ -39,9 +39,11 @@ #define F81534_UART_OFFSET 0x10 #define F81534_DIVISOR_LSB_REG (0x00 + F81534_UART_BASE_ADDRESS) #define F81534_DIVISOR_MSB_REG (0x01 + F81534_UART_BASE_ADDRESS) +#define F81534_INTERRUPT_ENABLE_REG (0x01 + F81534_UART_BASE_ADDRESS) #define F81534_FIFO_CONTROL_REG (0x02 + F81534_UART_BASE_ADDRESS) #define F81534_LINE_CONTROL_REG (0x03 + F81534_UART_BASE_ADDRESS) #define F81534_MODEM_CONTROL_REG (0x04 + F81534_UART_BASE_ADDRESS) +#define F81534_LINE_STATUS_REG (0x05 + F81534_UART_BASE_ADDRESS) #define F81534_MODEM_STATUS_REG (0x06 + F81534_UART_BASE_ADDRESS) #define F81534_CONFIG1_REG (0x09 + F81534_UART_BASE_ADDRESS) @@ -126,6 +128,8 @@ struct f81534_serial_private { struct f81534_port_private { struct mutex mcr_mutex; + struct work_struct lsr_work; + struct usb_serial_port *port; unsigned long tx_empty; spinlock_t msr_lock; u8 shadow_mcr; @@ -1015,6 +1019,8 @@ static void f81534_process_per_serial_block(struct usb_serial_port *port, tty_insert_flip_char(&port->port, 0, TTY_OVERRUN); } + + schedule_work(&port_priv->lsr_work); } if (port->port.console && port->sysrq) { @@ -1162,6 +1168,21 @@ static int f81534_attach(struct usb_serial *serial) return 0; } +static void f81534_lsr_worker(struct work_struct *work) +{ + struct f81534_port_private *port_priv; + struct usb_serial_port *port; + int status; + u8 tmp; + + port_priv = container_of(work, struct f81534_port_private, lsr_work); + port = port_priv->port; + + status = f81534_get_port_register(port, F81534_LINE_STATUS_REG, &tmp); + if (status) + dev_warn(&port->dev, "read LSR failed: %d\n", status); +} + static int f81534_port_probe(struct usb_serial_port *port) { struct f81534_port_private *port_priv; @@ -1173,6 +1194,7 @@ static int f81534_port_probe(struct usb_serial_port *port) spin_lock_init(&port_priv->msr_lock); mutex_init(&port_priv->mcr_mutex); + INIT_WORK(&port_priv->lsr_work, f81534_lsr_worker); /* Assign logic-to-phy mapping */ ret = f81534_logic_to_phy_port(port->serial, port); @@ -1180,10 +1202,30 @@ static int f81534_port_probe(struct usb_serial_port *port) return ret; port_priv->phy_num = ret; + port_priv->port = port; usb_set_serial_port_data(port, port_priv); dev_dbg(&port->dev, "%s: port_number: %d, phy_num: %d\n", __func__, port->port_number, port_priv->phy_num); + /* + * The F81532/534 will hang-up when enable LSR interrupt in IER and + * occur data overrun. So we'll disable the LSR interrupt in probe() + * and submit the LSR worker to clear LSR state when reported LSR error + * bit with bulk-in data in f81534_process_per_serial_block(). + */ + ret = f81534_set_port_register(port, F81534_INTERRUPT_ENABLE_REG, + UART_IER_RDI | UART_IER_THRI | UART_IER_MSI); + if (ret) + return ret; + + return 0; +} + +static int f81534_port_remove(struct usb_serial_port *port) +{ + struct f81534_port_private *port_priv = usb_get_serial_port_data(port); + + flush_work(&port_priv->lsr_work); return 0; } @@ -1317,6 +1359,7 @@ static struct usb_serial_driver f81534_device = { .calc_num_ports = f81534_calc_num_ports, .attach = f81534_attach, .port_probe = f81534_port_probe, + .port_remove = f81534_port_remove, .dtr_rts = f81534_dtr_rts, .process_read_urb = f81534_process_read_urb, .ioctl = f81534_ioctl, From 7c36e6e14da53a0a9c0893c363163afe629b2ee5 Mon Sep 17 00:00:00 2001 From: "Ji-Ze Hong (Peter Hong)" Date: Fri, 13 Oct 2017 10:21:35 +0800 Subject: [PATCH 14/14] USB: serial: f81534: implement break control Implement Fintek f81534 break on/off with LCR register. It's the same with 16550A LCR register layout. Signed-off-by: Ji-Ze Hong (Peter Hong) Signed-off-by: Johan Hovold --- drivers/usb/serial/f81534.c | 47 ++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c index 4ced064dbcc4..cb8214860192 100644 --- a/drivers/usb/serial/f81534.c +++ b/drivers/usb/serial/f81534.c @@ -128,11 +128,13 @@ struct f81534_serial_private { struct f81534_port_private { struct mutex mcr_mutex; + struct mutex lcr_mutex; struct work_struct lsr_work; struct usb_serial_port *port; unsigned long tx_empty; spinlock_t msr_lock; u8 shadow_mcr; + u8 shadow_lcr; u8 shadow_msr; u8 phy_num; }; @@ -465,6 +467,7 @@ static u32 f81534_calc_baud_divisor(u32 baudrate, u32 clockrate) static int f81534_set_port_config(struct usb_serial_port *port, u32 baudrate, u8 lcr) { + struct f81534_port_private *port_priv = usb_get_serial_port_data(port); u32 divisor; int status; u8 value; @@ -493,35 +496,65 @@ static int f81534_set_port_config(struct usb_serial_port *port, u32 baudrate, } divisor = f81534_calc_baud_divisor(baudrate, F81534_MAX_BAUDRATE); + + mutex_lock(&port_priv->lcr_mutex); + value = UART_LCR_DLAB; status = f81534_set_port_register(port, F81534_LINE_CONTROL_REG, value); if (status) { dev_err(&port->dev, "%s: set LCR failed\n", __func__); - return status; + goto out_unlock; } value = divisor & 0xff; status = f81534_set_port_register(port, F81534_DIVISOR_LSB_REG, value); if (status) { dev_err(&port->dev, "%s: set DLAB LSB failed\n", __func__); - return status; + goto out_unlock; } value = (divisor >> 8) & 0xff; status = f81534_set_port_register(port, F81534_DIVISOR_MSB_REG, value); if (status) { dev_err(&port->dev, "%s: set DLAB MSB failed\n", __func__); - return status; + goto out_unlock; } - status = f81534_set_port_register(port, F81534_LINE_CONTROL_REG, lcr); + value = lcr | (port_priv->shadow_lcr & UART_LCR_SBC); + status = f81534_set_port_register(port, F81534_LINE_CONTROL_REG, + value); if (status) { dev_err(&port->dev, "%s: set LCR failed\n", __func__); - return status; + goto out_unlock; } - return 0; + port_priv->shadow_lcr = value; +out_unlock: + mutex_unlock(&port_priv->lcr_mutex); + + return status; +} + +static void f81534_break_ctl(struct tty_struct *tty, int break_state) +{ + struct usb_serial_port *port = tty->driver_data; + struct f81534_port_private *port_priv = usb_get_serial_port_data(port); + int status; + + mutex_lock(&port_priv->lcr_mutex); + + if (break_state) + port_priv->shadow_lcr |= UART_LCR_SBC; + else + port_priv->shadow_lcr &= ~UART_LCR_SBC; + + status = f81534_set_port_register(port, F81534_LINE_CONTROL_REG, + port_priv->shadow_lcr); + if (status) + dev_err(&port->dev, "set break failed: %d\n", status); + + mutex_unlock(&port_priv->lcr_mutex); } static int f81534_update_mctrl(struct usb_serial_port *port, unsigned int set, @@ -1194,6 +1227,7 @@ static int f81534_port_probe(struct usb_serial_port *port) spin_lock_init(&port_priv->msr_lock); mutex_init(&port_priv->mcr_mutex); + mutex_init(&port_priv->lcr_mutex); INIT_WORK(&port_priv->lsr_work, f81534_lsr_worker); /* Assign logic-to-phy mapping */ @@ -1360,6 +1394,7 @@ static struct usb_serial_driver f81534_device = { .attach = f81534_attach, .port_probe = f81534_port_probe, .port_remove = f81534_port_remove, + .break_ctl = f81534_break_ctl, .dtr_rts = f81534_dtr_rts, .process_read_urb = f81534_process_read_urb, .ioctl = f81534_ioctl,