From 1fcf689181e95f3a289f42806fff38fb15a66b61 Mon Sep 17 00:00:00 2001 From: "Ahmed S. Darwish" Date: Mon, 26 Oct 2020 15:03:13 +0100 Subject: USB: serial: digi_acceleport: remove in_interrupt() usage The usage of in_interrupt() in drivers is phased out and Linus clearly requested that code which changes behaviour depending on context should either be separated or the context be conveyed in an argument passed by the caller, which usually knows the context. The debug printk() in digi_write() prints in_interrupt() as context information. This information is imprecise as it does not distinguish between hard-IRQ or disabled bottom half and it does not consider disabled interrupts or preemption. It is not really helpful. Remove the in_interrupt() printout. Signed-off-by: Ahmed S. Darwish Signed-off-by: Thomas Gleixner Signed-off-by: Sebastian Andrzej Siewior Link: https://lore.kernel.org/r/20201026140313.dpg3hkhkje2os4hw@linutronix.de [ johan: amend commit message ] Signed-off-by: Johan Hovold --- drivers/usb/serial/digi_acceleport.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c index 91055a191995..016e7dec3196 100644 --- a/drivers/usb/serial/digi_acceleport.c +++ b/drivers/usb/serial/digi_acceleport.c @@ -911,9 +911,8 @@ static int digi_write(struct tty_struct *tty, struct usb_serial_port *port, unsigned char *data = port->write_urb->transfer_buffer; unsigned long flags = 0; - dev_dbg(&port->dev, - "digi_write: TOP: port=%d, count=%d, in_interrupt=%ld\n", - priv->dp_port_num, count, in_interrupt()); + dev_dbg(&port->dev, "digi_write: TOP: port=%d, count=%d\n", + priv->dp_port_num, count); /* copy user data (which can sleep) before getting spin lock */ count = min(count, port->bulk_out_size-2); -- cgit v1.2.3-70-g09d2 From d1849b9ff9f442b3720e07b2d6f6c5cf1b97b1a3 Mon Sep 17 00:00:00 2001 From: Tom Rix Date: Mon, 26 Oct 2020 12:14:50 -0700 Subject: USB: serial: iuu_phoenix: remove unneeded break A break is not needed if it is preceded by a return. Signed-off-by: Tom Rix Signed-off-by: Johan Hovold --- drivers/usb/serial/iuu_phoenix.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/serial/iuu_phoenix.c b/drivers/usb/serial/iuu_phoenix.c index b4ba79123d9d..f1201d4de297 100644 --- a/drivers/usb/serial/iuu_phoenix.c +++ b/drivers/usb/serial/iuu_phoenix.c @@ -850,7 +850,6 @@ static int iuu_uart_baud(struct usb_serial_port *port, u32 baud_base, default: kfree(dataout); return IUU_INVALID_PARAMETER; - break; } switch (parity & 0xF0) { @@ -864,7 +863,6 @@ static int iuu_uart_baud(struct usb_serial_port *port, u32 baud_base, default: kfree(dataout); return IUU_INVALID_PARAMETER; - break; } status = bulk_immediate(port, dataout, DataCount); -- cgit v1.2.3-70-g09d2 From 696c541c8c6cfa05d65aa24ae2b9e720fc01766e Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:47 +0100 Subject: USB: serial: keyspan_pda: fix dropped unthrottle interrupts Commit c528fcb116e6 ("USB: serial: keyspan_pda: fix receive sanity checks") broke write-unthrottle handling by dropping well-formed unthrottle-interrupt packets which are precisely two bytes long. This could lead to blocked writers not being woken up when buffer space again becomes available. Instead, stop unconditionally printing the third byte which is (presumably) only valid on modem-line changes. Fixes: c528fcb116e6 ("USB: serial: keyspan_pda: fix receive sanity checks") Cc: stable # 4.11 Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index c1333919716b..2d5ad579475a 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -172,11 +172,11 @@ static void keyspan_pda_rx_interrupt(struct urb *urb) break; case 1: /* status interrupt */ - if (len < 3) { + if (len < 2) { dev_warn(&port->dev, "short interrupt message received\n"); break; } - dev_dbg(&port->dev, "rx int, d1=%d, d2=%d\n", data[1], data[2]); + dev_dbg(&port->dev, "rx int, d1=%d\n", data[1]); switch (data[1]) { case 1: /* modemline change */ break; -- cgit v1.2.3-70-g09d2 From 7353cad7ee4deaefc16e94727e69285563e219f6 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:48 +0100 Subject: USB: serial: keyspan_pda: fix write deadlock The write() callback can be called in interrupt context (e.g. when used as a console) so interrupts must be disabled while holding the port lock to prevent a possible deadlock. Fixes: e81ee637e4ae ("usb-serial: possible irq lock inversion (PPP vs. usb/serial)") Fixes: 507ca9bc0476 ("[PATCH] USB: add ability for usb-serial drivers to determine if their write urb is currently being used.") Cc: stable # 2.6.19 Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index 2d5ad579475a..17b60e5a9f1f 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -443,6 +443,7 @@ static int keyspan_pda_write(struct tty_struct *tty, int request_unthrottle = 0; int rc = 0; struct keyspan_pda_private *priv; + unsigned long flags; priv = usb_get_serial_port_data(port); /* guess how much room is left in the device's ring buffer, and if we @@ -462,13 +463,13 @@ static int keyspan_pda_write(struct tty_struct *tty, the TX urb is in-flight (wait until it completes) the device is full (wait until it says there is room) */ - spin_lock_bh(&port->lock); + spin_lock_irqsave(&port->lock, flags); if (!test_bit(0, &port->write_urbs_free) || priv->tx_throttled) { - spin_unlock_bh(&port->lock); + spin_unlock_irqrestore(&port->lock, flags); return 0; } clear_bit(0, &port->write_urbs_free); - spin_unlock_bh(&port->lock); + spin_unlock_irqrestore(&port->lock, flags); /* At this point the URB is in our control, nobody else can submit it again (the only sudden transition was the one from EINPROGRESS to -- cgit v1.2.3-70-g09d2 From c01d2c58698f710c9e13ba3e2d296328606f74fd Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:49 +0100 Subject: USB: serial: keyspan_pda: fix stalled writes Make sure to clear the write-busy flag also in case no new data was submitted due to lack of device buffer space so that writing is resumed once space again becomes available. Fixes: 507ca9bc0476 ("[PATCH] USB: add ability for usb-serial drivers to determine if their write urb is currently being used.") Cc: stable # 2.6.13 Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index 17b60e5a9f1f..d6ebde779e85 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -548,7 +548,7 @@ static int keyspan_pda_write(struct tty_struct *tty, rc = count; exit: - if (rc < 0) + if (rc <= 0) set_bit(0, &port->write_urbs_free); return rc; } -- cgit v1.2.3-70-g09d2 From 37faf50615412947868c49aee62f68233307f4e4 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:50 +0100 Subject: USB: serial: keyspan_pda: fix write-wakeup use-after-free The driver's deferred write wakeup was never flushed on disconnect, something which could lead to the driver port data being freed while the wakeup work is still scheduled. Fix this by using the usb-serial write wakeup which gets cancelled properly on disconnect. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index d6ebde779e85..d91180ab5f3b 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -43,8 +43,7 @@ struct keyspan_pda_private { int tx_room; int tx_throttled; - struct work_struct wakeup_work; - struct work_struct unthrottle_work; + struct work_struct unthrottle_work; struct usb_serial *serial; struct usb_serial_port *port; }; @@ -97,15 +96,6 @@ static const struct usb_device_id id_table_fake_xircom[] = { }; #endif -static void keyspan_pda_wakeup_write(struct work_struct *work) -{ - struct keyspan_pda_private *priv = - container_of(work, struct keyspan_pda_private, wakeup_work); - struct usb_serial_port *port = priv->port; - - tty_port_tty_wakeup(&port->port); -} - static void keyspan_pda_request_unthrottle(struct work_struct *work) { struct keyspan_pda_private *priv = @@ -183,7 +173,7 @@ static void keyspan_pda_rx_interrupt(struct urb *urb) case 2: /* tx unthrottle interrupt */ priv->tx_throttled = 0; /* queue up a wakeup at scheduler time */ - schedule_work(&priv->wakeup_work); + usb_serial_port_softint(port); break; default: break; @@ -563,7 +553,7 @@ static void keyspan_pda_write_bulk_callback(struct urb *urb) priv = usb_get_serial_port_data(port); /* queue up a wakeup at scheduler time */ - schedule_work(&priv->wakeup_work); + usb_serial_port_softint(port); } @@ -715,7 +705,6 @@ static int keyspan_pda_port_probe(struct usb_serial_port *port) if (!priv) return -ENOMEM; - INIT_WORK(&priv->wakeup_work, keyspan_pda_wakeup_write); INIT_WORK(&priv->unthrottle_work, keyspan_pda_request_unthrottle); priv->serial = port->serial; priv->port = port; -- cgit v1.2.3-70-g09d2 From 49fbb8e37a961396a5b6c82937c70df91de45e9d Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:51 +0100 Subject: USB: serial: keyspan_pda: fix tx-unthrottle use-after-free The driver's transmit-unthrottle work was never flushed on disconnect, something which could lead to the driver port data being freed while the unthrottle work is still scheduled. Fix this by cancelling the unthrottle work when shutting down the port. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index d91180ab5f3b..781b6723379f 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -647,8 +647,12 @@ error: } static void keyspan_pda_close(struct usb_serial_port *port) { + struct keyspan_pda_private *priv = usb_get_serial_port_data(port); + usb_kill_urb(port->write_urb); usb_kill_urb(port->interrupt_in_urb); + + cancel_work_sync(&priv->unthrottle_work); } -- cgit v1.2.3-70-g09d2 From 320f9028c7873c3c7710e8e93e5c979f4c857490 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:52 +0100 Subject: USB: serial: keyspan_pda: fix write unthrottling The driver did not update its view of the available device buffer space until write() was called in task context. This meant that write_room() would return 0 even after the device had sent a write-unthrottle notification, something which could lead to blocked writers not being woken up (e.g. when using OPOST). Note that we must also request an unthrottle notification is case a write() request fills the device buffer exactly. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index 781b6723379f..39ed3ad32365 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -40,6 +40,8 @@ #define DRIVER_AUTHOR "Brian Warner " #define DRIVER_DESC "USB Keyspan PDA Converter driver" +#define KEYSPAN_TX_THRESHOLD 16 + struct keyspan_pda_private { int tx_room; int tx_throttled; @@ -110,7 +112,7 @@ static void keyspan_pda_request_unthrottle(struct work_struct *work) 7, /* request_unthrottle */ USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT, - 16, /* value: threshold */ + KEYSPAN_TX_THRESHOLD, 0, /* index */ NULL, 0, @@ -129,6 +131,8 @@ static void keyspan_pda_rx_interrupt(struct urb *urb) int retval; int status = urb->status; struct keyspan_pda_private *priv; + unsigned long flags; + priv = usb_get_serial_port_data(port); switch (status) { @@ -171,7 +175,10 @@ static void keyspan_pda_rx_interrupt(struct urb *urb) case 1: /* modemline change */ break; case 2: /* tx unthrottle interrupt */ + spin_lock_irqsave(&port->lock, flags); priv->tx_throttled = 0; + priv->tx_room = max(priv->tx_room, KEYSPAN_TX_THRESHOLD); + spin_unlock_irqrestore(&port->lock, flags); /* queue up a wakeup at scheduler time */ usb_serial_port_softint(port); break; @@ -505,7 +512,8 @@ static int keyspan_pda_write(struct tty_struct *tty, goto exit; } } - if (count > priv->tx_room) { + + if (count >= priv->tx_room) { /* we're about to completely fill the Tx buffer, so we'll be throttled afterwards. */ count = priv->tx_room; @@ -560,14 +568,17 @@ static void keyspan_pda_write_bulk_callback(struct urb *urb) static int keyspan_pda_write_room(struct tty_struct *tty) { struct usb_serial_port *port = tty->driver_data; - struct keyspan_pda_private *priv; - priv = usb_get_serial_port_data(port); - /* used by n_tty.c for processing of tabs and such. Giving it our - conservative guess is probably good enough, but needs testing by - running a console through the device. */ - return priv->tx_room; -} + struct keyspan_pda_private *priv = usb_get_serial_port_data(port); + unsigned long flags; + int room = 0; + + spin_lock_irqsave(&port->lock, flags); + if (test_bit(0, &port->write_urbs_free) && !priv->tx_throttled) + room = priv->tx_room; + spin_unlock_irqrestore(&port->lock, flags); + return room; +} static int keyspan_pda_chars_in_buffer(struct tty_struct *tty) { -- cgit v1.2.3-70-g09d2 From 79fe6826a5ebee2724d432a736ec04d8dca143ba Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:53 +0100 Subject: USB: serial: keyspan_pda: refactor write-room handling Add helper to retrieve the available device transfer-buffer space. Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 113 +++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 63 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index 39ed3ad32365..54a21a99c001 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -98,6 +98,42 @@ static const struct usb_device_id id_table_fake_xircom[] = { }; #endif +static int keyspan_pda_get_write_room(struct keyspan_pda_private *priv) +{ + struct usb_serial_port *port = priv->port; + struct usb_serial *serial = priv->serial; + u8 *room; + int rc; + + room = kmalloc(1, GFP_KERNEL); + if (!room) + return -ENOMEM; + + rc = usb_control_msg(serial->dev, + usb_rcvctrlpipe(serial->dev, 0), + 6, /* write_room */ + USB_TYPE_VENDOR | USB_RECIP_INTERFACE + | USB_DIR_IN, + 0, /* value: 0 means "remaining room" */ + 0, /* index */ + room, + 1, + 2000); + if (rc != 1) { + if (rc >= 0) + rc = -EIO; + dev_dbg(&port->dev, "roomquery failed: %d\n", rc); + goto out_free; + } + + dev_dbg(&port->dev, "roomquery says %d\n", *room); + rc = *room; +out_free: + kfree(room); + + return rc; +} + static void keyspan_pda_request_unthrottle(struct work_struct *work) { struct keyspan_pda_private *priv = @@ -436,7 +472,6 @@ static int keyspan_pda_tiocmset(struct tty_struct *tty, static int keyspan_pda_write(struct tty_struct *tty, struct usb_serial_port *port, const unsigned char *buf, int count) { - struct usb_serial *serial = port->serial; int request_unthrottle = 0; int rc = 0; struct keyspan_pda_private *priv; @@ -479,38 +514,11 @@ static int keyspan_pda_write(struct tty_struct *tty, device how much room it really has. This is done only on scheduler time, since usb_control_msg() sleeps. */ if (count > priv->tx_room && !in_interrupt()) { - u8 *room; - - room = kmalloc(1, GFP_KERNEL); - if (!room) { - rc = -ENOMEM; + rc = keyspan_pda_get_write_room(priv); + if (rc < 0) goto exit; - } - rc = usb_control_msg(serial->dev, - usb_rcvctrlpipe(serial->dev, 0), - 6, /* write_room */ - USB_TYPE_VENDOR | USB_RECIP_INTERFACE - | USB_DIR_IN, - 0, /* value: 0 means "remaining room" */ - 0, /* index */ - room, - 1, - 2000); - if (rc > 0) { - dev_dbg(&port->dev, "roomquery says %d\n", *room); - priv->tx_room = *room; - } - kfree(room); - if (rc < 0) { - dev_dbg(&port->dev, "roomquery failed\n"); - goto exit; - } - if (rc == 0) { - dev_dbg(&port->dev, "roomquery returned 0 bytes\n"); - rc = -EIO; /* device didn't return any data */ - goto exit; - } + priv->tx_room = rc; } if (count >= priv->tx_room) { @@ -614,48 +622,27 @@ static void keyspan_pda_dtr_rts(struct usb_serial_port *port, int on) static int keyspan_pda_open(struct tty_struct *tty, struct usb_serial_port *port) { - struct usb_serial *serial = port->serial; - u8 *room; - int rc = 0; - struct keyspan_pda_private *priv; + struct keyspan_pda_private *priv = usb_get_serial_port_data(port); + int rc; /* find out how much room is in the Tx ring */ - room = kmalloc(1, GFP_KERNEL); - if (!room) - return -ENOMEM; + rc = keyspan_pda_get_write_room(priv); + if (rc < 0) + return rc; - rc = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0), - 6, /* write_room */ - USB_TYPE_VENDOR | USB_RECIP_INTERFACE - | USB_DIR_IN, - 0, /* value */ - 0, /* index */ - room, - 1, - 2000); - if (rc < 0) { - dev_dbg(&port->dev, "%s - roomquery failed\n", __func__); - goto error; - } - if (rc == 0) { - dev_dbg(&port->dev, "%s - roomquery returned 0 bytes\n", __func__); - rc = -EIO; - goto error; - } - priv = usb_get_serial_port_data(port); - priv->tx_room = *room; - priv->tx_throttled = *room ? 0 : 1; + priv->tx_room = rc; + priv->tx_throttled = rc ? 0 : 1; /*Start reading from the device*/ rc = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); if (rc) { dev_dbg(&port->dev, "%s - usb_submit_urb(read int) failed\n", __func__); - goto error; + return rc; } -error: - kfree(room); - return rc; + + return 0; } + static void keyspan_pda_close(struct usb_serial_port *port) { struct keyspan_pda_private *priv = usb_get_serial_port_data(port); -- cgit v1.2.3-70-g09d2 From 7184933b52a6b3e64171819b77f0bab018696cb2 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:54 +0100 Subject: USB: serial: keyspan_pda: fix write implementation Fix stalled writes by checking the available buffer space after requesting an unthrottle notification in case the device buffer is already empty so that no notification is ever sent (e.g. when doing single character writes). This also means we can drop the room query from write() which was conditioned on in_interrupt() and prevented writing using this driver from atomic contexts (e.g. PPP). Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 109 ++++++++++++++++++--------------------- 1 file changed, 51 insertions(+), 58 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index 54a21a99c001..b3fb2ecefb31 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -44,7 +44,6 @@ struct keyspan_pda_private { int tx_room; - int tx_throttled; struct work_struct unthrottle_work; struct usb_serial *serial; struct usb_serial_port *port; @@ -138,9 +137,13 @@ static void keyspan_pda_request_unthrottle(struct work_struct *work) { struct keyspan_pda_private *priv = container_of(work, struct keyspan_pda_private, unthrottle_work); + struct usb_serial_port *port = priv->port; struct usb_serial *serial = priv->serial; + unsigned long flags; int result; + dev_dbg(&port->dev, "%s\n", __func__); + /* ask the device to tell us when the tx buffer becomes sufficiently empty */ result = usb_control_msg(serial->dev, @@ -156,8 +159,19 @@ static void keyspan_pda_request_unthrottle(struct work_struct *work) if (result < 0) dev_dbg(&serial->dev->dev, "%s - error %d from usb_control_msg\n", __func__, result); -} + /* + * Need to check available space after requesting notification in case + * buffer is already empty so that no notification is sent. + */ + result = keyspan_pda_get_write_room(priv); + if (result > KEYSPAN_TX_THRESHOLD) { + spin_lock_irqsave(&port->lock, flags); + priv->tx_room = max(priv->tx_room, result); + spin_unlock_irqrestore(&port->lock, flags); + usb_serial_port_softint(port); + } +} static void keyspan_pda_rx_interrupt(struct urb *urb) { @@ -212,7 +226,6 @@ static void keyspan_pda_rx_interrupt(struct urb *urb) break; case 2: /* tx unthrottle interrupt */ spin_lock_irqsave(&port->lock, flags); - priv->tx_throttled = 0; priv->tx_room = max(priv->tx_room, KEYSPAN_TX_THRESHOLD); spin_unlock_irqrestore(&port->lock, flags); /* queue up a wakeup at scheduler time */ @@ -472,35 +485,42 @@ static int keyspan_pda_tiocmset(struct tty_struct *tty, static int keyspan_pda_write(struct tty_struct *tty, struct usb_serial_port *port, const unsigned char *buf, int count) { - int request_unthrottle = 0; - int rc = 0; struct keyspan_pda_private *priv; unsigned long flags; + int room; + int rc; priv = usb_get_serial_port_data(port); - /* guess how much room is left in the device's ring buffer, and if we - want to send more than that, check first, updating our notion of - what is left. If our write will result in no room left, ask the - device to give us an interrupt when the room available rises above - a threshold, and hold off all writers (eventually, those using - select() or poll() too) until we receive that unthrottle interrupt. - Block if we can't write anything at all, otherwise write as much as - we can. */ + /* + * Guess how much room is left in the device's ring buffer. If our + * write will result in no room left, ask the device to give us an + * interrupt when the room available rises above a threshold but also + * query how much room is currently available (in case our guess was + * too conservative and the buffer is already empty when the + * unthrottle work is scheduled). + */ if (count == 0) { dev_dbg(&port->dev, "write request of 0 bytes\n"); return 0; } + if (count > port->bulk_out_size) + count = port->bulk_out_size; + /* we might block because of: the TX urb is in-flight (wait until it completes) the device is full (wait until it says there is room) */ spin_lock_irqsave(&port->lock, flags); - if (!test_bit(0, &port->write_urbs_free) || priv->tx_throttled) { + room = priv->tx_room; + if (!test_bit(0, &port->write_urbs_free) || room == 0) { spin_unlock_irqrestore(&port->lock, flags); return 0; } clear_bit(0, &port->write_urbs_free); + if (count > room) + count = room; + priv->tx_room -= count; spin_unlock_irqrestore(&port->lock, flags); /* At this point the URB is in our control, nobody else can submit it @@ -508,58 +528,30 @@ static int keyspan_pda_write(struct tty_struct *tty, finished). Also, the tx process is not throttled. So we are ready to write. */ - count = (count > port->bulk_out_size) ? port->bulk_out_size : count; + dev_dbg(&port->dev, "%s - count = %d, txroom = %d\n", __func__, count, room); - /* Check if we might overrun the Tx buffer. If so, ask the - device how much room it really has. This is done only on - scheduler time, since usb_control_msg() sleeps. */ - if (count > priv->tx_room && !in_interrupt()) { - rc = keyspan_pda_get_write_room(priv); - if (rc < 0) - goto exit; - - priv->tx_room = rc; - } + memcpy(port->write_urb->transfer_buffer, buf, count); + port->write_urb->transfer_buffer_length = count; - if (count >= priv->tx_room) { - /* we're about to completely fill the Tx buffer, so - we'll be throttled afterwards. */ - count = priv->tx_room; - request_unthrottle = 1; - } + rc = usb_submit_urb(port->write_urb, GFP_ATOMIC); + if (rc) { + dev_dbg(&port->dev, "usb_submit_urb(write bulk) failed\n"); - if (count) { - /* now transfer data */ - memcpy(port->write_urb->transfer_buffer, buf, count); - /* send the data out the bulk port */ - port->write_urb->transfer_buffer_length = count; + spin_lock_irqsave(&port->lock, flags); + priv->tx_room = max(priv->tx_room, room + count); + spin_unlock_irqrestore(&port->lock, flags); - priv->tx_room -= count; + set_bit(0, &port->write_urbs_free); - rc = usb_submit_urb(port->write_urb, GFP_ATOMIC); - if (rc) { - dev_dbg(&port->dev, "usb_submit_urb(write bulk) failed\n"); - goto exit; - } - } else { - /* There wasn't any room left, so we are throttled until - the buffer empties a bit */ - request_unthrottle = 1; + return rc; } - if (request_unthrottle) { - priv->tx_throttled = 1; /* block writers */ + if (count == room) schedule_work(&priv->unthrottle_work); - } - rc = count; -exit: - if (rc <= 0) - set_bit(0, &port->write_urbs_free); - return rc; + return count; } - static void keyspan_pda_write_bulk_callback(struct urb *urb) { struct usb_serial_port *port = urb->context; @@ -581,7 +573,7 @@ static int keyspan_pda_write_room(struct tty_struct *tty) int room = 0; spin_lock_irqsave(&port->lock, flags); - if (test_bit(0, &port->write_urbs_free) && !priv->tx_throttled) + if (test_bit(0, &port->write_urbs_free)) room = priv->tx_room; spin_unlock_irqrestore(&port->lock, flags); @@ -601,7 +593,7 @@ static int keyspan_pda_chars_in_buffer(struct tty_struct *tty) n_tty.c:normal_poll() ) that we're not writeable. */ spin_lock_irqsave(&port->lock, flags); - if (!test_bit(0, &port->write_urbs_free) || priv->tx_throttled) + if (!test_bit(0, &port->write_urbs_free) || priv->tx_room == 0) ret = 256; spin_unlock_irqrestore(&port->lock, flags); return ret; @@ -630,8 +622,9 @@ static int keyspan_pda_open(struct tty_struct *tty, if (rc < 0) return rc; + spin_lock_irq(&port->lock); priv->tx_room = rc; - priv->tx_throttled = rc ? 0 : 1; + spin_unlock_irq(&port->lock); /*Start reading from the device*/ rc = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); -- cgit v1.2.3-70-g09d2 From 6fded8bcbc2e34d2267442376bf4fe9dde196d65 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:55 +0100 Subject: USB: serial: keyspan_pda: increase transmitter threshold Increase the transmitter threshold so that writing isn't resumed until 128 bytes are available in the device buffer thereby allowing for larger and more efficient transfers. Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index b3fb2ecefb31..3816bbc928b2 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -40,7 +40,7 @@ #define DRIVER_AUTHOR "Brian Warner " #define DRIVER_DESC "USB Keyspan PDA Converter driver" -#define KEYSPAN_TX_THRESHOLD 16 +#define KEYSPAN_TX_THRESHOLD 128 struct keyspan_pda_private { int tx_room; -- cgit v1.2.3-70-g09d2 From 034e38e8f68767fb5438ae3e608ee82919674177 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:56 +0100 Subject: USB: serial: keyspan_pda: add write-fifo support Use the port write fifo and generic chars_and_buffer and write_room implementations when writing. This not only allows for more efficient transfers, but more importantly fixes the remaining issues related to the conservative write_room() implementation which could prevent the line discipline from making forward progress (e.g. waiting for n > 1 bytes of space to become available). Note that this also allows using the driver for the system console without dropping data when the write URB is busy (including when adding carriage return on line feed). Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 115 +++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 60 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index 3816bbc928b2..aa19dd181e42 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -5,6 +5,7 @@ * Copyright (C) 1999 - 2001 Greg Kroah-Hartman * Copyright (C) 1999, 2000 Brian Warner * Copyright (C) 2000 Al Borchers + * Copyright (C) 2020 Johan Hovold * * See Documentation/usb/usb-serial.rst for more information on using this * driver @@ -37,7 +38,7 @@ #undef XIRCOM #endif -#define DRIVER_AUTHOR "Brian Warner " +#define DRIVER_AUTHOR "Brian Warner , Johan Hovold " #define DRIVER_DESC "USB Keyspan PDA Converter driver" #define KEYSPAN_TX_THRESHOLD 128 @@ -49,6 +50,7 @@ struct keyspan_pda_private { struct usb_serial_port *port; }; +static int keyspan_pda_write_start(struct usb_serial_port *port); #define KEYSPAN_VENDOR_ID 0x06cd #define KEYSPAN_PDA_FAKE_ID 0x0103 @@ -228,6 +230,9 @@ static void keyspan_pda_rx_interrupt(struct urb *urb) spin_lock_irqsave(&port->lock, flags); priv->tx_room = max(priv->tx_room, KEYSPAN_TX_THRESHOLD); spin_unlock_irqrestore(&port->lock, flags); + + keyspan_pda_write_start(port); + /* queue up a wakeup at scheduler time */ usb_serial_port_softint(port); break; @@ -482,15 +487,15 @@ static int keyspan_pda_tiocmset(struct tty_struct *tty, return rc; } -static int keyspan_pda_write(struct tty_struct *tty, - struct usb_serial_port *port, const unsigned char *buf, int count) +static int keyspan_pda_write_start(struct usb_serial_port *port) { - struct keyspan_pda_private *priv; + struct keyspan_pda_private *priv = usb_get_serial_port_data(port); unsigned long flags; + struct urb *urb; + int count; int room; int rc; - priv = usb_get_serial_port_data(port); /* * Guess how much room is left in the device's ring buffer. If our * write will result in no room left, ask the device to give us an @@ -499,50 +504,48 @@ static int keyspan_pda_write(struct tty_struct *tty, * too conservative and the buffer is already empty when the * unthrottle work is scheduled). */ - if (count == 0) { - dev_dbg(&port->dev, "write request of 0 bytes\n"); - return 0; - } - - if (count > port->bulk_out_size) - count = port->bulk_out_size; /* we might block because of: the TX urb is in-flight (wait until it completes) the device is full (wait until it says there is room) */ spin_lock_irqsave(&port->lock, flags); + room = priv->tx_room; - if (!test_bit(0, &port->write_urbs_free) || room == 0) { + count = kfifo_len(&port->write_fifo); + + if (!test_bit(0, &port->write_urbs_free) || count == 0 || room == 0) { spin_unlock_irqrestore(&port->lock, flags); return 0; } - clear_bit(0, &port->write_urbs_free); + __clear_bit(0, &port->write_urbs_free); + if (count > room) count = room; + if (count > port->bulk_out_size) + count = port->bulk_out_size; + + urb = port->write_urb; + count = kfifo_out(&port->write_fifo, urb->transfer_buffer, count); + urb->transfer_buffer_length = count; + + port->tx_bytes += count; priv->tx_room -= count; - spin_unlock_irqrestore(&port->lock, flags); - /* At this point the URB is in our control, nobody else can submit it - again (the only sudden transition was the one from EINPROGRESS to - finished). Also, the tx process is not throttled. So we are - ready to write. */ + spin_unlock_irqrestore(&port->lock, flags); dev_dbg(&port->dev, "%s - count = %d, txroom = %d\n", __func__, count, room); - memcpy(port->write_urb->transfer_buffer, buf, count); - port->write_urb->transfer_buffer_length = count; - - rc = usb_submit_urb(port->write_urb, GFP_ATOMIC); + rc = usb_submit_urb(urb, GFP_ATOMIC); if (rc) { dev_dbg(&port->dev, "usb_submit_urb(write bulk) failed\n"); spin_lock_irqsave(&port->lock, flags); + port->tx_bytes -= count; priv->tx_room = max(priv->tx_room, room + count); + __set_bit(0, &port->write_urbs_free); spin_unlock_irqrestore(&port->lock, flags); - set_bit(0, &port->write_urbs_free); - return rc; } @@ -555,51 +558,38 @@ static int keyspan_pda_write(struct tty_struct *tty, static void keyspan_pda_write_bulk_callback(struct urb *urb) { struct usb_serial_port *port = urb->context; - struct keyspan_pda_private *priv; + unsigned long flags; - set_bit(0, &port->write_urbs_free); - priv = usb_get_serial_port_data(port); + spin_lock_irqsave(&port->lock, flags); + port->tx_bytes -= urb->transfer_buffer_length; + __set_bit(0, &port->write_urbs_free); + spin_unlock_irqrestore(&port->lock, flags); + + keyspan_pda_write_start(port); /* queue up a wakeup at scheduler time */ usb_serial_port_softint(port); } - -static int keyspan_pda_write_room(struct tty_struct *tty) +static int keyspan_pda_write(struct tty_struct *tty, struct usb_serial_port *port, + const unsigned char *buf, int count) { - struct usb_serial_port *port = tty->driver_data; - struct keyspan_pda_private *priv = usb_get_serial_port_data(port); - unsigned long flags; - int room = 0; - - spin_lock_irqsave(&port->lock, flags); - if (test_bit(0, &port->write_urbs_free)) - room = priv->tx_room; - spin_unlock_irqrestore(&port->lock, flags); + int rc; - return room; -} + dev_dbg(&port->dev, "%s - count = %d\n", __func__, count); -static int keyspan_pda_chars_in_buffer(struct tty_struct *tty) -{ - struct usb_serial_port *port = tty->driver_data; - struct keyspan_pda_private *priv; - unsigned long flags; - int ret = 0; + if (!count) + return 0; - priv = usb_get_serial_port_data(port); + count = kfifo_in_locked(&port->write_fifo, buf, count, &port->lock); - /* when throttled, return at least WAKEUP_CHARS to tell select() (via - n_tty.c:normal_poll() ) that we're not writeable. */ + rc = keyspan_pda_write_start(port); + if (rc) + return rc; - spin_lock_irqsave(&port->lock, flags); - if (!test_bit(0, &port->write_urbs_free) || priv->tx_room == 0) - ret = 256; - spin_unlock_irqrestore(&port->lock, flags); - return ret; + return count; } - static void keyspan_pda_dtr_rts(struct usb_serial_port *port, int on) { struct usb_serial *serial = port->serial; @@ -640,12 +630,19 @@ static void keyspan_pda_close(struct usb_serial_port *port) { struct keyspan_pda_private *priv = usb_get_serial_port_data(port); - usb_kill_urb(port->write_urb); + /* + * Stop the interrupt URB first as its completion handler may submit + * the write URB. + */ usb_kill_urb(port->interrupt_in_urb); + usb_kill_urb(port->write_urb); cancel_work_sync(&priv->unthrottle_work); -} + spin_lock_irq(&port->lock); + kfifo_reset(&port->write_fifo); + spin_unlock_irq(&port->lock); +} /* download the firmware to a "fake" device (pre-renumeration) */ static int keyspan_pda_fake_startup(struct usb_serial *serial) @@ -759,10 +756,8 @@ static struct usb_serial_driver keyspan_pda_device = { .open = keyspan_pda_open, .close = keyspan_pda_close, .write = keyspan_pda_write, - .write_room = keyspan_pda_write_room, .write_bulk_callback = keyspan_pda_write_bulk_callback, .read_int_callback = keyspan_pda_rx_interrupt, - .chars_in_buffer = keyspan_pda_chars_in_buffer, .throttle = keyspan_pda_rx_throttle, .unthrottle = keyspan_pda_rx_unthrottle, .set_termios = keyspan_pda_set_termios, -- cgit v1.2.3-70-g09d2 From 7604ce70b8f675582ae68064e125be4f5174dec0 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 27 Oct 2020 10:25:02 +0100 Subject: USB: serial: keyspan_pda: clean up xircom/entrega support Drop the separate Kconfig symbol for Xircom / Entrega and always include support in the keyspan_pda driver. Note that all configs that enabled CONFIG_USB_SERIAL_XIRCOM also enable CONFIG_USB_SERIAL_KEYSPAN_PDA. Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- arch/arm/configs/badge4_defconfig | 1 - arch/arm/configs/corgi_defconfig | 1 - arch/arm/configs/pxa_defconfig | 1 - arch/arm/configs/spitz_defconfig | 1 - arch/mips/configs/mtx1_defconfig | 1 - arch/mips/configs/rm200_defconfig | 1 - arch/powerpc/configs/g5_defconfig | 1 - arch/powerpc/configs/ppc6xx_defconfig | 1 - drivers/usb/serial/Kconfig | 19 +++------- drivers/usb/serial/Makefile | 1 - drivers/usb/serial/keyspan_pda.c | 68 +++++------------------------------ 11 files changed, 13 insertions(+), 83 deletions(-) diff --git a/arch/arm/configs/badge4_defconfig b/arch/arm/configs/badge4_defconfig index ef484c4cfd1a..d9119da65f48 100644 --- a/arch/arm/configs/badge4_defconfig +++ b/arch/arm/configs/badge4_defconfig @@ -89,7 +89,6 @@ CONFIG_USB_SERIAL_KEYSPAN=m CONFIG_USB_SERIAL_MCT_U232=m CONFIG_USB_SERIAL_PL2303=m CONFIG_USB_SERIAL_CYBERJACK=m -CONFIG_USB_SERIAL_XIRCOM=m CONFIG_USB_SERIAL_OMNINET=m CONFIG_EXT2_FS=m CONFIG_EXT3_FS=m diff --git a/arch/arm/configs/corgi_defconfig b/arch/arm/configs/corgi_defconfig index 4fec2ec379ad..911e880f06ed 100644 --- a/arch/arm/configs/corgi_defconfig +++ b/arch/arm/configs/corgi_defconfig @@ -191,7 +191,6 @@ CONFIG_USB_SERIAL_PL2303=m CONFIG_USB_SERIAL_SAFE=m CONFIG_USB_SERIAL_TI=m CONFIG_USB_SERIAL_CYBERJACK=m -CONFIG_USB_SERIAL_XIRCOM=m CONFIG_USB_SERIAL_OMNINET=m CONFIG_USB_EMI62=m CONFIG_USB_EMI26=m diff --git a/arch/arm/configs/pxa_defconfig b/arch/arm/configs/pxa_defconfig index d7b9eaf4783c..8654ece13004 100644 --- a/arch/arm/configs/pxa_defconfig +++ b/arch/arm/configs/pxa_defconfig @@ -574,7 +574,6 @@ CONFIG_USB_SERIAL_PL2303=m CONFIG_USB_SERIAL_SAFE=m CONFIG_USB_SERIAL_TI=m CONFIG_USB_SERIAL_CYBERJACK=m -CONFIG_USB_SERIAL_XIRCOM=m CONFIG_USB_SERIAL_OMNINET=m CONFIG_USB_EMI62=m CONFIG_USB_EMI26=m diff --git a/arch/arm/configs/spitz_defconfig b/arch/arm/configs/spitz_defconfig index a1cdbfa064c5..8b2c14424927 100644 --- a/arch/arm/configs/spitz_defconfig +++ b/arch/arm/configs/spitz_defconfig @@ -185,7 +185,6 @@ CONFIG_USB_SERIAL_PL2303=m CONFIG_USB_SERIAL_SAFE=m CONFIG_USB_SERIAL_TI=m CONFIG_USB_SERIAL_CYBERJACK=m -CONFIG_USB_SERIAL_XIRCOM=m CONFIG_USB_SERIAL_OMNINET=m CONFIG_USB_EMI62=m CONFIG_USB_EMI26=m diff --git a/arch/mips/configs/mtx1_defconfig b/arch/mips/configs/mtx1_defconfig index 914af125a7fa..9750bcc38f05 100644 --- a/arch/mips/configs/mtx1_defconfig +++ b/arch/mips/configs/mtx1_defconfig @@ -565,7 +565,6 @@ CONFIG_USB_SERIAL_SAFE=m CONFIG_USB_SERIAL_SIERRAWIRELESS=m CONFIG_USB_SERIAL_TI=m CONFIG_USB_SERIAL_CYBERJACK=m -CONFIG_USB_SERIAL_XIRCOM=m CONFIG_USB_SERIAL_OPTION=m CONFIG_USB_SERIAL_OMNINET=m CONFIG_USB_EMI62=m diff --git a/arch/mips/configs/rm200_defconfig b/arch/mips/configs/rm200_defconfig index 30d7c3db884e..3dc2da2bee0d 100644 --- a/arch/mips/configs/rm200_defconfig +++ b/arch/mips/configs/rm200_defconfig @@ -311,7 +311,6 @@ CONFIG_USB_SERIAL_PL2303=m CONFIG_USB_SERIAL_SAFE=m CONFIG_USB_SERIAL_SAFE_PADDED=y CONFIG_USB_SERIAL_CYBERJACK=m -CONFIG_USB_SERIAL_XIRCOM=m CONFIG_USB_SERIAL_OMNINET=m CONFIG_USB_LEGOTOWER=m CONFIG_USB_LCD=m diff --git a/arch/powerpc/configs/g5_defconfig b/arch/powerpc/configs/g5_defconfig index 1c674c4c1d86..1de0dbf6cbba 100644 --- a/arch/powerpc/configs/g5_defconfig +++ b/arch/powerpc/configs/g5_defconfig @@ -194,7 +194,6 @@ CONFIG_USB_SERIAL_SAFE=m CONFIG_USB_SERIAL_SAFE_PADDED=y CONFIG_USB_SERIAL_TI=m CONFIG_USB_SERIAL_CYBERJACK=m -CONFIG_USB_SERIAL_XIRCOM=m CONFIG_USB_SERIAL_OMNINET=m CONFIG_USB_APPLEDISPLAY=m CONFIG_EXT2_FS=y diff --git a/arch/powerpc/configs/ppc6xx_defconfig b/arch/powerpc/configs/ppc6xx_defconfig index 66e9a0fd64ff..ac92cbe1f581 100644 --- a/arch/powerpc/configs/ppc6xx_defconfig +++ b/arch/powerpc/configs/ppc6xx_defconfig @@ -911,7 +911,6 @@ CONFIG_USB_SERIAL_SAFE_PADDED=y CONFIG_USB_SERIAL_SIERRAWIRELESS=m CONFIG_USB_SERIAL_TI=m CONFIG_USB_SERIAL_CYBERJACK=m -CONFIG_USB_SERIAL_XIRCOM=m CONFIG_USB_SERIAL_OPTION=m CONFIG_USB_SERIAL_OMNINET=m CONFIG_USB_SERIAL_DEBUG=m diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig index 4007fa25a8ff..a21ff5ab6df9 100644 --- a/drivers/usb/serial/Kconfig +++ b/drivers/usb/serial/Kconfig @@ -298,12 +298,12 @@ config USB_SERIAL_IUU module will be called iuu_phoenix.o config USB_SERIAL_KEYSPAN_PDA - tristate "USB Keyspan PDA Single Port Serial Driver" + tristate "USB Keyspan PDA / Xircom Single Port Serial Driver" select USB_EZUSB_FX2 help - Say Y here if you want to use a Keyspan PDA single port USB to - serial converter device. This driver makes use of firmware - developed from scratch by Brian Warner. + Say Y here if you want to use a Keyspan PDA, Xircom or Entrega single + port USB to serial converter device. This driver makes use of + firmware developed from scratch by Brian Warner. To compile this driver as a module, choose M here: the module will be called keyspan_pda. @@ -538,17 +538,6 @@ config USB_SERIAL_CYBERJACK If unsure, say N. -config USB_SERIAL_XIRCOM - tristate "USB Xircom / Entrega Single Port Serial Driver" - select USB_EZUSB_FX2 - help - Say Y here if you want to use a Xircom or Entrega single port USB to - serial converter device. This driver makes use of firmware - developed from scratch by Brian Warner. - - To compile this driver as a module, choose M here: the - module will be called keyspan_pda. - config USB_SERIAL_WWAN tristate diff --git a/drivers/usb/serial/Makefile b/drivers/usb/serial/Makefile index 2d491e434f11..50c53aed787a 100644 --- a/drivers/usb/serial/Makefile +++ b/drivers/usb/serial/Makefile @@ -61,5 +61,4 @@ obj-$(CONFIG_USB_SERIAL_UPD78F0730) += upd78f0730.o obj-$(CONFIG_USB_SERIAL_VISOR) += visor.o obj-$(CONFIG_USB_SERIAL_WISHBONE) += wishbone-serial.o obj-$(CONFIG_USB_SERIAL_WHITEHEAT) += whiteheat.o -obj-$(CONFIG_USB_SERIAL_XIRCOM) += keyspan_pda.o obj-$(CONFIG_USB_SERIAL_XSENS_MT) += xsens_mt.o diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index aa19dd181e42..f102dbf83492 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -26,18 +26,6 @@ #include #include -/* make a simple define to handle if we are compiling keyspan_pda or xircom support */ -#if IS_ENABLED(CONFIG_USB_SERIAL_KEYSPAN_PDA) - #define KEYSPAN -#else - #undef KEYSPAN -#endif -#if IS_ENABLED(CONFIG_USB_SERIAL_XIRCOM) - #define XIRCOM -#else - #undef XIRCOM -#endif - #define DRIVER_AUTHOR "Brian Warner , Johan Hovold " #define DRIVER_DESC "USB Keyspan PDA Converter driver" @@ -64,18 +52,13 @@ static int keyspan_pda_write_start(struct usb_serial_port *port); #define ENTREGA_FAKE_ID 0x8093 static const struct usb_device_id id_table_combined[] = { -#ifdef KEYSPAN { USB_DEVICE(KEYSPAN_VENDOR_ID, KEYSPAN_PDA_FAKE_ID) }, -#endif -#ifdef XIRCOM { USB_DEVICE(XIRCOM_VENDOR_ID, XIRCOM_FAKE_ID) }, { USB_DEVICE(XIRCOM_VENDOR_ID, XIRCOM_FAKE_ID_2) }, { USB_DEVICE(ENTREGA_VENDOR_ID, ENTREGA_FAKE_ID) }, -#endif { USB_DEVICE(KEYSPAN_VENDOR_ID, KEYSPAN_PDA_ID) }, { } /* Terminating entry */ }; - MODULE_DEVICE_TABLE(usb, id_table_combined); static const struct usb_device_id id_table_std[] = { @@ -83,21 +66,13 @@ static const struct usb_device_id id_table_std[] = { { } /* Terminating entry */ }; -#ifdef KEYSPAN static const struct usb_device_id id_table_fake[] = { { USB_DEVICE(KEYSPAN_VENDOR_ID, KEYSPAN_PDA_FAKE_ID) }, - { } /* Terminating entry */ -}; -#endif - -#ifdef XIRCOM -static const struct usb_device_id id_table_fake_xircom[] = { { USB_DEVICE(XIRCOM_VENDOR_ID, XIRCOM_FAKE_ID) }, { USB_DEVICE(XIRCOM_VENDOR_ID, XIRCOM_FAKE_ID_2) }, { USB_DEVICE(ENTREGA_VENDOR_ID, ENTREGA_FAKE_ID) }, - { } + { } /* Terminating entry */ }; -#endif static int keyspan_pda_get_write_room(struct keyspan_pda_private *priv) { @@ -647,22 +622,21 @@ static void keyspan_pda_close(struct usb_serial_port *port) /* download the firmware to a "fake" device (pre-renumeration) */ static int keyspan_pda_fake_startup(struct usb_serial *serial) { + unsigned int vid = le16_to_cpu(serial->dev->descriptor.idVendor); const char *fw_name; /* download the firmware here ... */ ezusb_fx1_set_reset(serial->dev, 1); - if (0) { ; } -#ifdef KEYSPAN - else if (le16_to_cpu(serial->dev->descriptor.idVendor) == KEYSPAN_VENDOR_ID) + switch (vid) { + case KEYSPAN_VENDOR_ID: fw_name = "keyspan_pda/keyspan_pda.fw"; -#endif -#ifdef XIRCOM - else if ((le16_to_cpu(serial->dev->descriptor.idVendor) == XIRCOM_VENDOR_ID) || - (le16_to_cpu(serial->dev->descriptor.idVendor) == ENTREGA_VENDOR_ID)) + break; + case XIRCOM_VENDOR_ID: + case ENTREGA_VENDOR_ID: fw_name = "keyspan_pda/xircom_pgs.fw"; -#endif - else { + break; + default: dev_err(&serial->dev->dev, "%s: unknown vendor, aborting.\n", __func__); return -ENODEV; @@ -681,12 +655,8 @@ static int keyspan_pda_fake_startup(struct usb_serial *serial) return 1; } -#ifdef KEYSPAN MODULE_FIRMWARE("keyspan_pda/keyspan_pda.fw"); -#endif -#ifdef XIRCOM MODULE_FIRMWARE("keyspan_pda/xircom_pgs.fw"); -#endif static int keyspan_pda_port_probe(struct usb_serial_port *port) { @@ -716,7 +686,6 @@ static int keyspan_pda_port_remove(struct usb_serial_port *port) return 0; } -#ifdef KEYSPAN static struct usb_serial_driver keyspan_pda_fake_device = { .driver = { .owner = THIS_MODULE, @@ -727,20 +696,6 @@ static struct usb_serial_driver keyspan_pda_fake_device = { .num_ports = 1, .attach = keyspan_pda_fake_startup, }; -#endif - -#ifdef XIRCOM -static struct usb_serial_driver xircom_pgs_fake_device = { - .driver = { - .owner = THIS_MODULE, - .name = "xircom_no_firm", - }, - .description = "Xircom / Entrega PGS - (prerenumeration)", - .id_table = id_table_fake_xircom, - .num_ports = 1, - .attach = keyspan_pda_fake_startup, -}; -#endif static struct usb_serial_driver keyspan_pda_device = { .driver = { @@ -770,12 +725,7 @@ static struct usb_serial_driver keyspan_pda_device = { static struct usb_serial_driver * const serial_drivers[] = { &keyspan_pda_device, -#ifdef KEYSPAN &keyspan_pda_fake_device, -#endif -#ifdef XIRCOM - &xircom_pgs_fake_device, -#endif NULL }; -- cgit v1.2.3-70-g09d2 From 491d6927f0de587c1d322d8b29a1187b7e06a221 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:58 +0100 Subject: USB: serial: keyspan_pda: clean up comments and whitespace Clean up comment style, remove some stale or redundant comments and drop superfluous white space. Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 116 +++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 60 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index f102dbf83492..8df7c54d6986 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -11,7 +11,6 @@ * driver */ - #include #include #include @@ -121,8 +120,10 @@ static void keyspan_pda_request_unthrottle(struct work_struct *work) dev_dbg(&port->dev, "%s\n", __func__); - /* ask the device to tell us when the tx buffer becomes - sufficiently empty */ + /* + * Ask the device to tell us when the tx buffer becomes + * sufficiently empty. + */ result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0), 7, /* request_unthrottle */ @@ -208,7 +209,6 @@ static void keyspan_pda_rx_interrupt(struct urb *urb) keyspan_pda_write_start(port); - /* queue up a wakeup at scheduler time */ usb_serial_port_softint(port); break; default: @@ -227,31 +227,30 @@ exit: __func__, retval); } - static void keyspan_pda_rx_throttle(struct tty_struct *tty) { - /* stop receiving characters. We just turn off the URB request, and - let chars pile up in the device. If we're doing hardware - flowcontrol, the device will signal the other end when its buffer - fills up. If we're doing XON/XOFF, this would be a good time to - send an XOFF, although it might make sense to foist that off - upon the device too. */ struct usb_serial_port *port = tty->driver_data; + /* + * Stop receiving characters. We just turn off the URB request, and + * let chars pile up in the device. If we're doing hardware + * flowcontrol, the device will signal the other end when its buffer + * fills up. If we're doing XON/XOFF, this would be a good time to + * send an XOFF, although it might make sense to foist that off upon + * the device too. + */ usb_kill_urb(port->interrupt_in_urb); } - static void keyspan_pda_rx_unthrottle(struct tty_struct *tty) { struct usb_serial_port *port = tty->driver_data; - /* just restart the receive interrupt URB */ + /* just restart the receive interrupt URB */ if (usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL)) dev_dbg(&port->dev, "usb_submit_urb(read urb) failed\n"); } - static speed_t keyspan_pda_setbaud(struct usb_serial *serial, speed_t baud) { int rc; @@ -293,8 +292,6 @@ static speed_t keyspan_pda_setbaud(struct usb_serial *serial, speed_t baud) baud = 9600; } - /* rather than figure out how to sleep while waiting for this - to complete, I just use the "legacy" API. */ rc = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0), 0, /* set baud */ USB_TYPE_VENDOR @@ -307,10 +304,10 @@ static speed_t keyspan_pda_setbaud(struct usb_serial *serial, speed_t baud) 2000); /* timeout */ if (rc < 0) return 0; + return baud; } - static void keyspan_pda_break_ctl(struct tty_struct *tty, int break_state) { struct usb_serial_port *port = tty->driver_data; @@ -322,6 +319,7 @@ static void keyspan_pda_break_ctl(struct tty_struct *tty, int break_state) value = 1; /* start break */ else value = 0; /* clear break */ + result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0), 4, /* set break */ USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT, @@ -329,39 +327,35 @@ static void keyspan_pda_break_ctl(struct tty_struct *tty, int break_state) if (result < 0) dev_dbg(&port->dev, "%s - error %d from usb_control_msg\n", __func__, result); - /* there is something funky about this.. the TCSBRK that 'cu' performs - ought to translate into a break_ctl(-1),break_ctl(0) pair HZ/4 - seconds apart, but it feels like the break sent isn't as long as it - is on /dev/ttyS0 */ } - static void keyspan_pda_set_termios(struct tty_struct *tty, struct usb_serial_port *port, struct ktermios *old_termios) { struct usb_serial *serial = port->serial; speed_t speed; - /* cflag specifies lots of stuff: number of stop bits, parity, number - of data bits, baud. What can the device actually handle?: - CSTOPB (1 stop bit or 2) - PARENB (parity) - CSIZE (5bit .. 8bit) - There is minimal hw support for parity (a PSW bit seems to hold the - parity of whatever is in the accumulator). The UART either deals - with 10 bits (start, 8 data, stop) or 11 bits (start, 8 data, - 1 special, stop). So, with firmware changes, we could do: - 8N1: 10 bit - 8N2: 11 bit, extra bit always (mark?) - 8[EOMS]1: 11 bit, extra bit is parity - 7[EOMS]1: 10 bit, b0/b7 is parity - 7[EOMS]2: 11 bit, b0/b7 is parity, extra bit always (mark?) - - HW flow control is dictated by the tty->termios.c_cflags & CRTSCTS - bit. - - For now, just do baud. */ - + /* + * cflag specifies lots of stuff: number of stop bits, parity, number + * of data bits, baud. What can the device actually handle?: + * CSTOPB (1 stop bit or 2) + * PARENB (parity) + * CSIZE (5bit .. 8bit) + * There is minimal hw support for parity (a PSW bit seems to hold the + * parity of whatever is in the accumulator). The UART either deals + * with 10 bits (start, 8 data, stop) or 11 bits (start, 8 data, + * 1 special, stop). So, with firmware changes, we could do: + * 8N1: 10 bit + * 8N2: 11 bit, extra bit always (mark?) + * 8[EOMS]1: 11 bit, extra bit is parity + * 7[EOMS]1: 10 bit, b0/b7 is parity + * 7[EOMS]2: 11 bit, b0/b7 is parity, extra bit always (mark?) + * + * HW flow control is dictated by the tty->termios.c_cflags & CRTSCTS + * bit. + * + * For now, just do baud. + */ speed = tty_get_baud_rate(tty); speed = keyspan_pda_setbaud(serial, speed); @@ -370,17 +364,19 @@ static void keyspan_pda_set_termios(struct tty_struct *tty, /* It hasn't changed so.. */ speed = tty_termios_baud_rate(old_termios); } - /* Only speed can change so copy the old h/w parameters - then encode the new speed */ + /* + * Only speed can change so copy the old h/w parameters then encode + * the new speed. + */ tty_termios_copy_hw(&tty->termios, old_termios); tty_encode_baud_rate(tty, speed, speed); } - -/* modem control pins: DTR and RTS are outputs and can be controlled. - DCD, RI, DSR, CTS are inputs and can be read. All outputs can also be - read. The byte passed is: DTR(b7) DCD RI DSR CTS RTS(b2) unused unused */ - +/* + * Modem control pins: DTR and RTS are outputs and can be controlled. + * DCD, RI, DSR, CTS are inputs and can be read. All outputs can also be + * read. The byte passed is: DTR(b7) DCD RI DSR CTS RTS(b2) unused unused. + */ static int keyspan_pda_get_modem_info(struct usb_serial *serial, unsigned char *value) { @@ -404,7 +400,6 @@ static int keyspan_pda_get_modem_info(struct usb_serial *serial, return rc; } - static int keyspan_pda_set_modem_info(struct usb_serial *serial, unsigned char value) { @@ -480,10 +475,11 @@ static int keyspan_pda_write_start(struct usb_serial_port *port) * unthrottle work is scheduled). */ - /* we might block because of: - the TX urb is in-flight (wait until it completes) - the device is full (wait until it says there is room) - */ + /* + * We might block because of: + * the TX urb is in-flight (wait until it completes) + * the device is full (wait until it says there is room) + */ spin_lock_irqsave(&port->lock, flags); room = priv->tx_room; @@ -542,7 +538,6 @@ static void keyspan_pda_write_bulk_callback(struct urb *urb) keyspan_pda_write_start(port); - /* queue up a wakeup at scheduler time */ usb_serial_port_softint(port); } @@ -591,7 +586,6 @@ static int keyspan_pda_open(struct tty_struct *tty, priv->tx_room = rc; spin_unlock_irq(&port->lock); - /*Start reading from the device*/ rc = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL); if (rc) { dev_dbg(&port->dev, "%s - usb_submit_urb(read int) failed\n", __func__); @@ -648,10 +642,12 @@ static int keyspan_pda_fake_startup(struct usb_serial *serial) return -ENOENT; } - /* after downloading firmware Renumeration will occur in a - moment and the new device will bind to the real driver */ + /* + * After downloading firmware renumeration will occur in a moment and + * the new device will bind to the real driver. + */ - /* we want this device to fail to have a driver assigned to it. */ + /* We want this device to fail to have a driver assigned to it. */ return 1; } @@ -711,7 +707,7 @@ static struct usb_serial_driver keyspan_pda_device = { .open = keyspan_pda_open, .close = keyspan_pda_close, .write = keyspan_pda_write, - .write_bulk_callback = keyspan_pda_write_bulk_callback, + .write_bulk_callback = keyspan_pda_write_bulk_callback, .read_int_callback = keyspan_pda_rx_interrupt, .throttle = keyspan_pda_rx_throttle, .unthrottle = keyspan_pda_rx_unthrottle, -- cgit v1.2.3-70-g09d2 From fbbf41f64a8de352b67214c089366f97c7ac1678 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:45:59 +0100 Subject: USB: serial: keyspan_pda: use BIT() macro Use the BIT() macro instead of open coding. Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index 8df7c54d6986..f582e0a3ae56 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -422,13 +422,14 @@ static int keyspan_pda_tiocmget(struct tty_struct *tty) rc = keyspan_pda_get_modem_info(serial, &status); if (rc < 0) return rc; - value = - ((status & (1<<7)) ? TIOCM_DTR : 0) | - ((status & (1<<6)) ? TIOCM_CAR : 0) | - ((status & (1<<5)) ? TIOCM_RNG : 0) | - ((status & (1<<4)) ? TIOCM_DSR : 0) | - ((status & (1<<3)) ? TIOCM_CTS : 0) | - ((status & (1<<2)) ? TIOCM_RTS : 0); + + value = ((status & BIT(7)) ? TIOCM_DTR : 0) | + ((status & BIT(6)) ? TIOCM_CAR : 0) | + ((status & BIT(5)) ? TIOCM_RNG : 0) | + ((status & BIT(4)) ? TIOCM_DSR : 0) | + ((status & BIT(3)) ? TIOCM_CTS : 0) | + ((status & BIT(2)) ? TIOCM_RTS : 0); + return value; } @@ -445,14 +446,14 @@ static int keyspan_pda_tiocmset(struct tty_struct *tty, return rc; if (set & TIOCM_RTS) - status |= (1<<2); + status |= BIT(2); if (set & TIOCM_DTR) - status |= (1<<7); + status |= BIT(7); if (clear & TIOCM_RTS) - status &= ~(1<<2); + status &= ~BIT(2); if (clear & TIOCM_DTR) - status &= ~(1<<7); + status &= ~BIT(7); rc = keyspan_pda_set_modem_info(serial, status); return rc; } @@ -565,7 +566,7 @@ static void keyspan_pda_dtr_rts(struct usb_serial_port *port, int on) struct usb_serial *serial = port->serial; if (on) - keyspan_pda_set_modem_info(serial, (1 << 7) | (1 << 2)); + keyspan_pda_set_modem_info(serial, BIT(7) | BIT(2)); else keyspan_pda_set_modem_info(serial, 0); } -- cgit v1.2.3-70-g09d2 From 66c32e4833559d8683879b5fa6fe70332db53aec Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sun, 25 Oct 2020 18:46:00 +0100 Subject: USB: serial: keyspan_pda: drop redundant usb-serial pointer Drop the redundant struct usb_serial pointer from the driver port data. Acked-by: Sebastian Andrzej Siewior Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan_pda.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index f582e0a3ae56..e6f933e8d25f 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -76,7 +76,7 @@ static const struct usb_device_id id_table_fake[] = { static int keyspan_pda_get_write_room(struct keyspan_pda_private *priv) { struct usb_serial_port *port = priv->port; - struct usb_serial *serial = priv->serial; + struct usb_serial *serial = port->serial; u8 *room; int rc; @@ -114,7 +114,7 @@ static void keyspan_pda_request_unthrottle(struct work_struct *work) struct keyspan_pda_private *priv = container_of(work, struct keyspan_pda_private, unthrottle_work); struct usb_serial_port *port = priv->port; - struct usb_serial *serial = priv->serial; + struct usb_serial *serial = port->serial; unsigned long flags; int result; @@ -665,7 +665,6 @@ static int keyspan_pda_port_probe(struct usb_serial_port *port) return -ENOMEM; INIT_WORK(&priv->unthrottle_work, keyspan_pda_request_unthrottle); - priv->serial = port->serial; priv->port = port; usb_set_serial_port_data(port, priv); -- cgit v1.2.3-70-g09d2 From 5098e77962e7c8947f87bd8c5869c83e000a522a Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 Oct 2020 11:43:06 +0100 Subject: USB: serial: digi_acceleport: fix write-wakeup deadlocks The driver must not call tty_wakeup() while holding its private lock as line disciplines are allowed to call back into write() from write_wakeup(), leading to a deadlock. Also remove the unneeded work struct that was used to defer wakeup in order to work around a possible race in ancient times (see comment about n_tty write_chan() in commit 14b54e39b412 ("USB: serial: remove changelogs and old todo entries")). Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/digi_acceleport.c | 45 +++++++++++------------------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c index 016e7dec3196..968e08cdcb09 100644 --- a/drivers/usb/serial/digi_acceleport.c +++ b/drivers/usb/serial/digi_acceleport.c @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include @@ -198,14 +197,12 @@ struct digi_port { int dp_throttle_restart; wait_queue_head_t dp_flush_wait; wait_queue_head_t dp_close_wait; /* wait queue for close */ - struct work_struct dp_wakeup_work; struct usb_serial_port *dp_port; }; /* Local Function Declarations */ -static void digi_wakeup_write_lock(struct work_struct *work); static int digi_write_oob_command(struct usb_serial_port *port, unsigned char *buf, int count, int interruptible); static int digi_write_inb_command(struct usb_serial_port *port, @@ -356,26 +353,6 @@ __releases(lock) return timeout; } - -/* - * Digi Wakeup Write - * - * Wake up port, line discipline, and tty processes sleeping - * on writes. - */ - -static void digi_wakeup_write_lock(struct work_struct *work) -{ - struct digi_port *priv = - container_of(work, struct digi_port, dp_wakeup_work); - struct usb_serial_port *port = priv->dp_port; - unsigned long flags; - - spin_lock_irqsave(&priv->dp_port_lock, flags); - tty_port_tty_wakeup(&port->port); - spin_unlock_irqrestore(&priv->dp_port_lock, flags); -} - /* * Digi Write OOB Command * @@ -985,6 +962,7 @@ static void digi_write_bulk_callback(struct urb *urb) unsigned long flags; int ret = 0; int status = urb->status; + bool wakeup; /* port and serial sanity check */ if (port == NULL || (priv = usb_get_serial_port_data(port)) == NULL) { @@ -1011,6 +989,7 @@ static void digi_write_bulk_callback(struct urb *urb) } /* try to send any buffered data on this port */ + wakeup = true; spin_lock_irqsave(&priv->dp_port_lock, flags); priv->dp_write_urb_in_use = 0; if (priv->dp_out_buf_len > 0) { @@ -1026,19 +1005,18 @@ static void digi_write_bulk_callback(struct urb *urb) if (ret == 0) { priv->dp_write_urb_in_use = 1; priv->dp_out_buf_len = 0; + wakeup = false; } } - /* wake up processes sleeping on writes immediately */ - tty_port_tty_wakeup(&port->port); - /* also queue up a wakeup at scheduler time, in case we */ - /* lost the race in write_chan(). */ - schedule_work(&priv->dp_wakeup_work); - spin_unlock_irqrestore(&priv->dp_port_lock, flags); + if (ret && ret != -EPERM) dev_err_console(port, "%s: usb_submit_urb failed, ret=%d, port=%d\n", __func__, ret, priv->dp_port_num); + + if (wakeup) + tty_port_tty_wakeup(&port->port); } static int digi_write_room(struct tty_struct *tty) @@ -1238,7 +1216,6 @@ static int digi_port_init(struct usb_serial_port *port, unsigned port_num) init_waitqueue_head(&priv->dp_transmit_idle_wait); init_waitqueue_head(&priv->dp_flush_wait); init_waitqueue_head(&priv->dp_close_wait); - INIT_WORK(&priv->dp_wakeup_work, digi_wakeup_write_lock); priv->dp_port = port; init_waitqueue_head(&port->write_wait); @@ -1507,13 +1484,14 @@ static int digi_read_oob_callback(struct urb *urb) rts = C_CRTSCTS(tty); if (tty && opcode == DIGI_CMD_READ_INPUT_SIGNALS) { + bool wakeup = false; + spin_lock_irqsave(&priv->dp_port_lock, flags); /* convert from digi flags to termiox flags */ if (val & DIGI_READ_INPUT_SIGNALS_CTS) { priv->dp_modem_signals |= TIOCM_CTS; - /* port must be open to use tty struct */ if (rts) - tty_port_tty_wakeup(&port->port); + wakeup = true; } else { priv->dp_modem_signals &= ~TIOCM_CTS; /* port must be open to use tty struct */ @@ -1532,6 +1510,9 @@ static int digi_read_oob_callback(struct urb *urb) priv->dp_modem_signals &= ~TIOCM_CD; spin_unlock_irqrestore(&priv->dp_port_lock, flags); + + if (wakeup) + tty_port_tty_wakeup(&port->port); } else if (opcode == DIGI_CMD_TRANSMIT_IDLE) { spin_lock_irqsave(&priv->dp_port_lock, flags); priv->dp_transmit_idle = 1; -- cgit v1.2.3-70-g09d2 From 179dfb954790410f65605f1c479c029c2cd73c55 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 Oct 2020 11:44:21 +0100 Subject: USB: serial: remove write wait queue The digi_acceleport driver is the only driver still using the port write wake queue so move it to that driver's port data. Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/digi_acceleport.c | 12 ++++++------ include/linux/usb/serial.h | 2 -- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c index 968e08cdcb09..0ecd5316d85f 100644 --- a/drivers/usb/serial/digi_acceleport.c +++ b/drivers/usb/serial/digi_acceleport.c @@ -197,6 +197,7 @@ struct digi_port { int dp_throttle_restart; wait_queue_head_t dp_flush_wait; wait_queue_head_t dp_close_wait; /* wait queue for close */ + wait_queue_head_t write_wait; struct usb_serial_port *dp_port; }; @@ -381,7 +382,7 @@ static int digi_write_oob_command(struct usb_serial_port *port, while (count > 0) { while (oob_priv->dp_write_urb_in_use) { cond_wait_interruptible_timeout_irqrestore( - &oob_port->write_wait, DIGI_RETRY_TIMEOUT, + &oob_priv->write_wait, DIGI_RETRY_TIMEOUT, &oob_priv->dp_port_lock, flags); if (interruptible && signal_pending(current)) return -EINTR; @@ -444,7 +445,7 @@ static int digi_write_inb_command(struct usb_serial_port *port, while (priv->dp_write_urb_in_use && time_before(jiffies, timeout)) { cond_wait_interruptible_timeout_irqrestore( - &port->write_wait, DIGI_RETRY_TIMEOUT, + &priv->write_wait, DIGI_RETRY_TIMEOUT, &priv->dp_port_lock, flags); if (signal_pending(current)) return -EINTR; @@ -523,7 +524,7 @@ static int digi_set_modem_signals(struct usb_serial_port *port, while (oob_priv->dp_write_urb_in_use) { spin_unlock(&port_priv->dp_port_lock); cond_wait_interruptible_timeout_irqrestore( - &oob_port->write_wait, DIGI_RETRY_TIMEOUT, + &oob_priv->write_wait, DIGI_RETRY_TIMEOUT, &oob_priv->dp_port_lock, flags); if (interruptible && signal_pending(current)) return -EINTR; @@ -983,7 +984,7 @@ static void digi_write_bulk_callback(struct urb *urb) dev_dbg(&port->dev, "digi_write_bulk_callback: oob callback\n"); spin_lock_irqsave(&priv->dp_port_lock, flags); priv->dp_write_urb_in_use = 0; - wake_up_interruptible(&port->write_wait); + wake_up_interruptible(&priv->write_wait); spin_unlock_irqrestore(&priv->dp_port_lock, flags); return; } @@ -1216,10 +1217,9 @@ static int digi_port_init(struct usb_serial_port *port, unsigned port_num) init_waitqueue_head(&priv->dp_transmit_idle_wait); init_waitqueue_head(&priv->dp_flush_wait); init_waitqueue_head(&priv->dp_close_wait); + init_waitqueue_head(&priv->write_wait); priv->dp_port = port; - init_waitqueue_head(&port->write_wait); - usb_set_serial_port_data(port, priv); return 0; diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h index 8e67eff9039f..1c09b922f8b0 100644 --- a/include/linux/usb/serial.h +++ b/include/linux/usb/serial.h @@ -62,7 +62,6 @@ * @bulk_out_endpointAddress: endpoint address for the bulk out pipe for this * port. * @flags: usb serial port flags - * @write_wait: a wait_queue_head_t used by the port. * @work: work queue entry for the line discipline waking up. * @dev: pointer to the serial device * @@ -108,7 +107,6 @@ struct usb_serial_port { int tx_bytes; unsigned long flags; - wait_queue_head_t write_wait; struct work_struct work; unsigned long sysrq; /* sysrq timeout */ struct device dev; -- cgit v1.2.3-70-g09d2 From 975323ab8f116667676c30ca3502a6757bd89e8d Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 4 Nov 2020 17:47:27 +0100 Subject: USB: serial: mos7720: fix parallel-port state restore The parallel-port restore operations is called when a driver claims the port and is supposed to restore the provided state (e.g. saved when releasing the port). Fixes: b69578df7e98 ("USB: usbserial: mos7720: add support for parallel port on moschip 7715") Cc: stable # 2.6.35 Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/mos7720.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c index 5eed1078fac8..5a5d2a95070e 100644 --- a/drivers/usb/serial/mos7720.c +++ b/drivers/usb/serial/mos7720.c @@ -639,6 +639,8 @@ static void parport_mos7715_restore_state(struct parport *pp, spin_unlock(&release_lock); return; } + mos_parport->shadowDCR = s->u.pc.ctr; + mos_parport->shadowECR = s->u.pc.ecr; write_parport_reg_nonblock(mos_parport, MOS7720_DCR, mos_parport->shadowDCR); write_parport_reg_nonblock(mos_parport, MOS7720_ECR, -- cgit v1.2.3-70-g09d2 From 053af9e6e817fe58191b0d27bc67ba329d94ced2 Mon Sep 17 00:00:00 2001 From: Davidlohr Bueso Date: Thu, 19 Nov 2020 20:53:00 -0800 Subject: USB: serial: mos7720: defer state restore to a workqueue The parallel port restore operation currently defers writes to a tasklet, if it sees a locked disconnect mutex. The driver goes to a lot of trouble to ensure writes happen in a non-blocking context, but things can be greatly simplified if it's done in regular process context and this is not a system performance critical path. As such, instead of doing the state restore writes in softirq context, use a workqueue and just do regular synchronous writes. In addition to the cleanup, this also imposes less on the overall system as tasklets have been deprecated because of it's softirq implications, potentially blocking a higher priority task from running. Signed-off-by: Davidlohr Bueso Link: https://lore.kernel.org/r/20201120045300.28804-1-dave@stgolabs.net [johan: amend commit message ("softirq context")] Signed-off-by: Johan Hovold --- drivers/usb/serial/mos7720.c | 234 +++++++------------------------------------ 1 file changed, 35 insertions(+), 199 deletions(-) diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c index 5a5d2a95070e..41ee2984a0df 100644 --- a/drivers/usb/serial/mos7720.c +++ b/drivers/usb/serial/mos7720.c @@ -79,14 +79,6 @@ MODULE_DEVICE_TABLE(usb, id_table); #define DCR_INIT_VAL 0x0c /* SLCTIN, nINIT */ #define ECR_INIT_VAL 0x00 /* SPP mode */ -struct urbtracker { - struct mos7715_parport *mos_parport; - struct list_head urblist_entry; - struct kref ref_count; - struct urb *urb; - struct usb_ctrlrequest *setup; -}; - enum mos7715_pp_modes { SPP = 0<<5, PS2 = 1<<5, /* moschip calls this 'NIBBLE' mode */ @@ -96,12 +88,9 @@ enum mos7715_pp_modes { struct mos7715_parport { struct parport *pp; /* back to containing struct */ struct kref ref_count; /* to instance of this struct */ - struct list_head deferred_urbs; /* list deferred async urbs */ - struct list_head active_urbs; /* list async urbs in flight */ - spinlock_t listlock; /* protects list access */ bool msg_pending; /* usb sync call pending */ struct completion syncmsg_compl; /* usb sync call completed */ - struct tasklet_struct urb_tasklet; /* for sending deferred urbs */ + struct work_struct work; /* restore deferred writes */ struct usb_serial *serial; /* back to containing struct */ __u8 shadowECR; /* parallel port regs... */ __u8 shadowDCR; @@ -265,174 +254,8 @@ static void destroy_mos_parport(struct kref *kref) kfree(mos_parport); } -static void destroy_urbtracker(struct kref *kref) -{ - struct urbtracker *urbtrack = - container_of(kref, struct urbtracker, ref_count); - struct mos7715_parport *mos_parport = urbtrack->mos_parport; - - usb_free_urb(urbtrack->urb); - kfree(urbtrack->setup); - kfree(urbtrack); - kref_put(&mos_parport->ref_count, destroy_mos_parport); -} - /* - * This runs as a tasklet when sending an urb in a non-blocking parallel - * port callback had to be deferred because the disconnect mutex could not be - * obtained at the time. - */ -static void send_deferred_urbs(struct tasklet_struct *t) -{ - int ret_val; - unsigned long flags; - struct mos7715_parport *mos_parport = from_tasklet(mos_parport, t, - urb_tasklet); - struct urbtracker *urbtrack, *tmp; - struct list_head *cursor, *next; - struct device *dev; - - /* if release function ran, game over */ - if (unlikely(mos_parport->serial == NULL)) - return; - - dev = &mos_parport->serial->dev->dev; - - /* try again to get the mutex */ - if (!mutex_trylock(&mos_parport->serial->disc_mutex)) { - dev_dbg(dev, "%s: rescheduling tasklet\n", __func__); - tasklet_schedule(&mos_parport->urb_tasklet); - return; - } - - /* if device disconnected, game over */ - if (unlikely(mos_parport->serial->disconnected)) { - mutex_unlock(&mos_parport->serial->disc_mutex); - return; - } - - spin_lock_irqsave(&mos_parport->listlock, flags); - if (list_empty(&mos_parport->deferred_urbs)) { - spin_unlock_irqrestore(&mos_parport->listlock, flags); - mutex_unlock(&mos_parport->serial->disc_mutex); - dev_dbg(dev, "%s: deferred_urbs list empty\n", __func__); - return; - } - - /* move contents of deferred_urbs list to active_urbs list and submit */ - list_for_each_safe(cursor, next, &mos_parport->deferred_urbs) - list_move_tail(cursor, &mos_parport->active_urbs); - list_for_each_entry_safe(urbtrack, tmp, &mos_parport->active_urbs, - urblist_entry) { - ret_val = usb_submit_urb(urbtrack->urb, GFP_ATOMIC); - dev_dbg(dev, "%s: urb submitted\n", __func__); - if (ret_val) { - dev_err(dev, "usb_submit_urb() failed: %d\n", ret_val); - list_del(&urbtrack->urblist_entry); - kref_put(&urbtrack->ref_count, destroy_urbtracker); - } - } - spin_unlock_irqrestore(&mos_parport->listlock, flags); - mutex_unlock(&mos_parport->serial->disc_mutex); -} - -/* callback for parallel port control urbs submitted asynchronously */ -static void async_complete(struct urb *urb) -{ - struct urbtracker *urbtrack = urb->context; - int status = urb->status; - unsigned long flags; - - if (unlikely(status)) - dev_dbg(&urb->dev->dev, "%s - nonzero urb status received: %d\n", __func__, status); - - /* remove the urbtracker from the active_urbs list */ - spin_lock_irqsave(&urbtrack->mos_parport->listlock, flags); - list_del(&urbtrack->urblist_entry); - spin_unlock_irqrestore(&urbtrack->mos_parport->listlock, flags); - kref_put(&urbtrack->ref_count, destroy_urbtracker); -} - -static int write_parport_reg_nonblock(struct mos7715_parport *mos_parport, - enum mos_regs reg, __u8 data) -{ - struct urbtracker *urbtrack; - int ret_val; - unsigned long flags; - struct usb_serial *serial = mos_parport->serial; - struct usb_device *usbdev = serial->dev; - - /* create and initialize the control urb and containing urbtracker */ - urbtrack = kmalloc(sizeof(struct urbtracker), GFP_ATOMIC); - if (!urbtrack) - return -ENOMEM; - - urbtrack->urb = usb_alloc_urb(0, GFP_ATOMIC); - if (!urbtrack->urb) { - kfree(urbtrack); - return -ENOMEM; - } - urbtrack->setup = kmalloc(sizeof(*urbtrack->setup), GFP_ATOMIC); - if (!urbtrack->setup) { - usb_free_urb(urbtrack->urb); - kfree(urbtrack); - return -ENOMEM; - } - urbtrack->setup->bRequestType = (__u8)0x40; - urbtrack->setup->bRequest = (__u8)0x0e; - urbtrack->setup->wValue = cpu_to_le16(get_reg_value(reg, dummy)); - urbtrack->setup->wIndex = cpu_to_le16(get_reg_index(reg)); - urbtrack->setup->wLength = 0; - usb_fill_control_urb(urbtrack->urb, usbdev, - usb_sndctrlpipe(usbdev, 0), - (unsigned char *)urbtrack->setup, - NULL, 0, async_complete, urbtrack); - kref_get(&mos_parport->ref_count); - urbtrack->mos_parport = mos_parport; - kref_init(&urbtrack->ref_count); - INIT_LIST_HEAD(&urbtrack->urblist_entry); - - /* - * get the disconnect mutex, or add tracker to the deferred_urbs list - * and schedule a tasklet to try again later - */ - if (!mutex_trylock(&serial->disc_mutex)) { - spin_lock_irqsave(&mos_parport->listlock, flags); - list_add_tail(&urbtrack->urblist_entry, - &mos_parport->deferred_urbs); - spin_unlock_irqrestore(&mos_parport->listlock, flags); - tasklet_schedule(&mos_parport->urb_tasklet); - dev_dbg(&usbdev->dev, "tasklet scheduled\n"); - return 0; - } - - /* bail if device disconnected */ - if (serial->disconnected) { - kref_put(&urbtrack->ref_count, destroy_urbtracker); - mutex_unlock(&serial->disc_mutex); - return -ENODEV; - } - - /* add the tracker to the active_urbs list and submit */ - spin_lock_irqsave(&mos_parport->listlock, flags); - list_add_tail(&urbtrack->urblist_entry, &mos_parport->active_urbs); - spin_unlock_irqrestore(&mos_parport->listlock, flags); - ret_val = usb_submit_urb(urbtrack->urb, GFP_ATOMIC); - mutex_unlock(&serial->disc_mutex); - if (ret_val) { - dev_err(&usbdev->dev, - "%s: submit_urb() failed: %d\n", __func__, ret_val); - spin_lock_irqsave(&mos_parport->listlock, flags); - list_del(&urbtrack->urblist_entry); - spin_unlock_irqrestore(&mos_parport->listlock, flags); - kref_put(&urbtrack->ref_count, destroy_urbtracker); - return ret_val; - } - return 0; -} - -/* - * This is the the common top part of all parallel port callback operations that + * This is the common top part of all parallel port callback operations that * send synchronous messages to the device. This implements convoluted locking * that avoids two scenarios: (1) a port operation is called after usbserial * has called our release function, at which point struct mos7715_parport has @@ -458,6 +281,10 @@ static int parport_prologue(struct parport *pp) reinit_completion(&mos_parport->syncmsg_compl); spin_unlock(&release_lock); + /* ensure writes from restore are submitted before new requests */ + if (work_pending(&mos_parport->work)) + flush_work(&mos_parport->work); + mutex_lock(&mos_parport->serial->disc_mutex); if (mos_parport->serial->disconnected) { /* device disconnected */ @@ -482,6 +309,26 @@ static inline void parport_epilogue(struct parport *pp) complete(&mos_parport->syncmsg_compl); } +static void deferred_restore_writes(struct work_struct *work) +{ + struct mos7715_parport *mos_parport; + + mos_parport = container_of(work, struct mos7715_parport, work); + + mutex_lock(&mos_parport->serial->disc_mutex); + + /* if device disconnected, game over */ + if (mos_parport->serial->disconnected) + goto done; + + write_mos_reg(mos_parport->serial, dummy, MOS7720_DCR, + mos_parport->shadowDCR); + write_mos_reg(mos_parport->serial, dummy, MOS7720_ECR, + mos_parport->shadowECR); +done: + mutex_unlock(&mos_parport->serial->disc_mutex); +} + static void parport_mos7715_write_data(struct parport *pp, unsigned char d) { struct mos7715_parport *mos_parport = pp->private_data; @@ -641,10 +488,8 @@ static void parport_mos7715_restore_state(struct parport *pp, } mos_parport->shadowDCR = s->u.pc.ctr; mos_parport->shadowECR = s->u.pc.ecr; - write_parport_reg_nonblock(mos_parport, MOS7720_DCR, - mos_parport->shadowDCR); - write_parport_reg_nonblock(mos_parport, MOS7720_ECR, - mos_parport->shadowECR); + + schedule_work(&mos_parport->work); spin_unlock(&release_lock); } @@ -714,12 +559,9 @@ static int mos7715_parport_init(struct usb_serial *serial) mos_parport->msg_pending = false; kref_init(&mos_parport->ref_count); - spin_lock_init(&mos_parport->listlock); - INIT_LIST_HEAD(&mos_parport->active_urbs); - INIT_LIST_HEAD(&mos_parport->deferred_urbs); usb_set_serial_data(serial, mos_parport); /* hijack private pointer */ mos_parport->serial = serial; - tasklet_setup(&mos_parport->urb_tasklet, send_deferred_urbs); + INIT_WORK(&mos_parport->work, deferred_restore_writes); init_completion(&mos_parport->syncmsg_compl); /* cycle parallel port reset bit */ @@ -1869,8 +1711,6 @@ static void mos7720_release(struct usb_serial *serial) if (le16_to_cpu(serial->dev->descriptor.idProduct) == MOSCHIP_DEVICE_ID_7715) { - struct urbtracker *urbtrack; - unsigned long flags; struct mos7715_parport *mos_parport = usb_get_serial_data(serial); @@ -1883,21 +1723,17 @@ static void mos7720_release(struct usb_serial *serial) if (mos_parport->msg_pending) wait_for_completion_timeout(&mos_parport->syncmsg_compl, msecs_to_jiffies(MOS_WDR_TIMEOUT)); + /* + * If delayed work is currently scheduled, wait for it to + * complete. This also implies barriers that ensure the + * below serial clearing is not hoisted above the ->work. + */ + cancel_work_sync(&mos_parport->work); parport_remove_port(mos_parport->pp); usb_set_serial_data(serial, NULL); mos_parport->serial = NULL; - /* if tasklet currently scheduled, wait for it to complete */ - tasklet_kill(&mos_parport->urb_tasklet); - - /* unlink any urbs sent by the tasklet */ - spin_lock_irqsave(&mos_parport->listlock, flags); - list_for_each_entry(urbtrack, - &mos_parport->active_urbs, - urblist_entry) - usb_unlink_urb(urbtrack->urb); - spin_unlock_irqrestore(&mos_parport->listlock, flags); parport_del_port(mos_parport->pp); kref_put(&mos_parport->ref_count, destroy_mos_parport); -- cgit v1.2.3-70-g09d2 From 95168d624f3a8dee466525767200391a0fb006b9 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 16 Nov 2020 17:18:21 +0100 Subject: USB: serial: cp210x: return early on unchanged termios Return early from set_termios() in case no relevant terminal settings have changed. This avoids testing each parameter in turn and specifically allows the line-control handling to be cleaned up further. Signed-off-by: Johan Hovold --- drivers/usb/serial/cp210x.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index d0c05aa8a0d6..f1fd109d97d5 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -1352,6 +1352,15 @@ static void cp210x_disable_event_mode(struct usb_serial_port *port) port_priv->event_mode = false; } +static bool cp210x_termios_change(const struct ktermios *a, const struct ktermios *b) +{ + bool iflag_change; + + iflag_change = ((a->c_iflag ^ b->c_iflag) & INPCK); + + return tty_termios_hw_change(a, b) || iflag_change; +} + static void cp210x_set_termios(struct tty_struct *tty, struct usb_serial_port *port, struct ktermios *old_termios) { @@ -1359,6 +1368,9 @@ static void cp210x_set_termios(struct tty_struct *tty, unsigned int cflag, old_cflag; u16 bits; + if (!cp210x_termios_change(&tty->termios, old_termios)) + return; + cflag = tty->termios.c_cflag; old_cflag = old_termios->c_cflag; -- cgit v1.2.3-70-g09d2 From d42976296c3389b556913d249f7c5626b754ec26 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 16 Nov 2020 17:18:22 +0100 Subject: USB: serial: cp210x: clean up line-control handling Update the line-control settings in one request unconditionally instead of setting the word-length, parity and stop-bit settings separately. This avoids multiple requests when several settings are changed even if this scheme could potentially also be used to detect unsupported device settings. Since all device types but CP2101 appears to support all settings, let's handle that one specifically and also report back the unsupported settings properly through termios by clearing the corresponding bits. Also drop the related unnecessary debug printks. Signed-off-by: Johan Hovold --- drivers/usb/serial/cp210x.c | 101 ++++++++++++++++++-------------------------- 1 file changed, 41 insertions(+), 60 deletions(-) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index f1fd109d97d5..ad134e517849 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -1364,9 +1364,11 @@ static bool cp210x_termios_change(const struct ktermios *a, const struct ktermio static void cp210x_set_termios(struct tty_struct *tty, struct usb_serial_port *port, struct ktermios *old_termios) { + struct cp210x_serial_private *priv = usb_get_serial_data(port->serial); struct device *dev = &port->dev; unsigned int cflag, old_cflag; u16 bits; + int ret; if (!cp210x_termios_change(&tty->termios, old_termios)) return; @@ -1377,74 +1379,53 @@ static void cp210x_set_termios(struct tty_struct *tty, if (tty->termios.c_ospeed != old_termios->c_ospeed) cp210x_change_speed(tty, port, old_termios); - /* If the number of data bits is to be updated */ - if ((cflag & CSIZE) != (old_cflag & CSIZE)) { - cp210x_get_line_ctl(port, &bits); - bits &= ~BITS_DATA_MASK; - switch (cflag & CSIZE) { - case CS5: - bits |= BITS_DATA_5; - dev_dbg(dev, "%s - data bits = 5\n", __func__); - break; - case CS6: - bits |= BITS_DATA_6; - dev_dbg(dev, "%s - data bits = 6\n", __func__); - break; - case CS7: - bits |= BITS_DATA_7; - dev_dbg(dev, "%s - data bits = 7\n", __func__); - break; - case CS8: - default: - bits |= BITS_DATA_8; - dev_dbg(dev, "%s - data bits = 8\n", __func__); - break; - } - if (cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits)) - dev_dbg(dev, "Number of data bits requested not supported by device\n"); + /* CP2101 only supports CS8, 1 stop bit and non-stick parity. */ + if (priv->partnum == CP210X_PARTNUM_CP2101) { + tty->termios.c_cflag &= ~(CSIZE | CSTOPB | CMSPAR); + tty->termios.c_cflag |= CS8; } - if ((cflag & (PARENB|PARODD|CMSPAR)) != - (old_cflag & (PARENB|PARODD|CMSPAR))) { - cp210x_get_line_ctl(port, &bits); - bits &= ~BITS_PARITY_MASK; - if (cflag & PARENB) { - if (cflag & CMSPAR) { - if (cflag & PARODD) { - bits |= BITS_PARITY_MARK; - dev_dbg(dev, "%s - parity = MARK\n", __func__); - } else { - bits |= BITS_PARITY_SPACE; - dev_dbg(dev, "%s - parity = SPACE\n", __func__); - } - } else { - if (cflag & PARODD) { - bits |= BITS_PARITY_ODD; - dev_dbg(dev, "%s - parity = ODD\n", __func__); - } else { - bits |= BITS_PARITY_EVEN; - dev_dbg(dev, "%s - parity = EVEN\n", __func__); - } - } - } - if (cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits)) - dev_dbg(dev, "Parity mode not supported by device\n"); + bits = 0; + + switch (C_CSIZE(tty)) { + case CS5: + bits |= BITS_DATA_5; + break; + case CS6: + bits |= BITS_DATA_6; + break; + case CS7: + bits |= BITS_DATA_7; + break; + case CS8: + default: + bits |= BITS_DATA_8; + break; } - if ((cflag & CSTOPB) != (old_cflag & CSTOPB)) { - cp210x_get_line_ctl(port, &bits); - bits &= ~BITS_STOP_MASK; - if (cflag & CSTOPB) { - bits |= BITS_STOP_2; - dev_dbg(dev, "%s - stop bits = 2\n", __func__); + if (C_PARENB(tty)) { + if (C_CMSPAR(tty)) { + if (C_PARODD(tty)) + bits |= BITS_PARITY_MARK; + else + bits |= BITS_PARITY_SPACE; } else { - bits |= BITS_STOP_1; - dev_dbg(dev, "%s - stop bits = 1\n", __func__); + if (C_PARODD(tty)) + bits |= BITS_PARITY_ODD; + else + bits |= BITS_PARITY_EVEN; } - if (cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits)) - dev_dbg(dev, "Number of stop bits requested not supported by device\n"); } + if (C_CSTOPB(tty)) + bits |= BITS_STOP_2; + else + bits |= BITS_STOP_1; + + ret = cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits); + if (ret) + dev_err(&port->dev, "failed to set line control: %d\n", ret); + if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) { struct cp210x_flow_ctl flow_ctl; u32 ctl_hs; -- cgit v1.2.3-70-g09d2 From 46827bda2dd635aed8af0a232a8992d1481140d6 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 16 Nov 2020 17:18:23 +0100 Subject: USB: serial: cp210x: set terminal settings on open Unlike other drivers cp210x have been retrieving the current terminal settings from the device on open and reflecting those in termios. Due to how set_termios() used to be implemented, this saved a few control requests on open but has instead caused problems like broken flow control and has required adding workarounds for swapped line-control in cp2108 and line-speed initialisation on cp2104. This unusual implementation also complicates adding new features for no good reason. Rip out the corresponding code and the above mentioned workarounds and instead initialise the terminal settings unconditionally on open. Signed-off-by: Johan Hovold --- drivers/usb/serial/cp210x.c | 289 +------------------------------------------- 1 file changed, 6 insertions(+), 283 deletions(-) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index ad134e517849..bb9c3d7ccaee 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -31,9 +31,6 @@ */ static int cp210x_open(struct tty_struct *tty, struct usb_serial_port *); static void cp210x_close(struct usb_serial_port *); -static void cp210x_get_termios(struct tty_struct *, struct usb_serial_port *); -static void cp210x_get_termios_port(struct usb_serial_port *port, - tcflag_t *cflagp, unsigned int *baudp); static void cp210x_change_speed(struct tty_struct *, struct usb_serial_port *, struct ktermios *); static void cp210x_set_termios(struct tty_struct *, struct usb_serial_port *, @@ -267,7 +264,6 @@ enum cp210x_event_state { struct cp210x_port_private { u8 bInterfaceNumber; - bool has_swapped_line_ctl; bool event_mode; enum cp210x_event_state event_state; u8 lsr; @@ -593,46 +589,6 @@ static int cp210x_read_reg_block(struct usb_serial_port *port, u8 req, return result; } -/* - * Reads any 32-bit CP210X_ register identified by req. - */ -static int cp210x_read_u32_reg(struct usb_serial_port *port, u8 req, u32 *val) -{ - __le32 le32_val; - int err; - - err = cp210x_read_reg_block(port, req, &le32_val, sizeof(le32_val)); - if (err) { - /* - * FIXME Some callers don't bother to check for error, - * at least give them consistent junk until they are fixed - */ - *val = 0; - return err; - } - - *val = le32_to_cpu(le32_val); - - return 0; -} - -/* - * Reads any 16-bit CP210X_ register identified by req. - */ -static int cp210x_read_u16_reg(struct usb_serial_port *port, u8 req, u16 *val) -{ - __le16 le16_val; - int err; - - err = cp210x_read_reg_block(port, req, &le16_val, sizeof(le16_val)); - if (err) - return err; - - *val = le16_to_cpu(le16_val); - - return 0; -} - /* * Reads any 8-bit CP210X_ register identified by req. */ @@ -780,59 +736,6 @@ static int cp210x_write_vendor_block(struct usb_serial *serial, u8 type, } #endif -/* - * Detect CP2108 GET_LINE_CTL bug and activate workaround. - * Write a known good value 0x800, read it back. - * If it comes back swapped the bug is detected. - * Preserve the original register value. - */ -static int cp210x_detect_swapped_line_ctl(struct usb_serial_port *port) -{ - struct cp210x_port_private *port_priv = usb_get_serial_port_data(port); - u16 line_ctl_save; - u16 line_ctl_test; - int err; - - err = cp210x_read_u16_reg(port, CP210X_GET_LINE_CTL, &line_ctl_save); - if (err) - return err; - - err = cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, 0x800); - if (err) - return err; - - err = cp210x_read_u16_reg(port, CP210X_GET_LINE_CTL, &line_ctl_test); - if (err) - return err; - - if (line_ctl_test == 8) { - port_priv->has_swapped_line_ctl = true; - line_ctl_save = swab16(line_ctl_save); - } - - return cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, line_ctl_save); -} - -/* - * Must always be called instead of cp210x_read_u16_reg(CP210X_GET_LINE_CTL) - * to workaround cp2108 bug and get correct value. - */ -static int cp210x_get_line_ctl(struct usb_serial_port *port, u16 *ctl) -{ - struct cp210x_port_private *port_priv = usb_get_serial_port_data(port); - int err; - - err = cp210x_read_u16_reg(port, CP210X_GET_LINE_CTL, ctl); - if (err) - return err; - - /* Workaround swapped bytes in 16-bit value from CP210X_GET_LINE_CTL */ - if (port_priv->has_swapped_line_ctl) - *ctl = swab16(*ctl); - - return 0; -} - static int cp210x_open(struct tty_struct *tty, struct usb_serial_port *port) { struct cp210x_port_private *port_priv = usb_get_serial_port_data(port); @@ -844,16 +747,8 @@ static int cp210x_open(struct tty_struct *tty, struct usb_serial_port *port) return result; } - /* Configure the termios structure */ - cp210x_get_termios(tty, port); - - if (tty) { - /* The baud rate must be initialised on cp2104 */ - cp210x_change_speed(tty, port, NULL); - - if (I_INPCK(tty)) - cp210x_enable_event_mode(port); - } + if (tty) + cp210x_set_termios(tty, port, NULL); result = usb_serial_generic_open(tty, port); if (result) @@ -1032,167 +927,6 @@ static bool cp210x_tx_empty(struct usb_serial_port *port) return !count; } -/* - * cp210x_get_termios - * Reads the baud rate, data bits, parity, stop bits and flow control mode - * from the device, corrects any unsupported values, and configures the - * termios structure to reflect the state of the device - */ -static void cp210x_get_termios(struct tty_struct *tty, - struct usb_serial_port *port) -{ - unsigned int baud; - - if (tty) { - cp210x_get_termios_port(tty->driver_data, - &tty->termios.c_cflag, &baud); - tty_encode_baud_rate(tty, baud, baud); - } else { - tcflag_t cflag; - cflag = 0; - cp210x_get_termios_port(port, &cflag, &baud); - } -} - -/* - * cp210x_get_termios_port - * This is the heart of cp210x_get_termios which always uses a &usb_serial_port. - */ -static void cp210x_get_termios_port(struct usb_serial_port *port, - tcflag_t *cflagp, unsigned int *baudp) -{ - struct device *dev = &port->dev; - tcflag_t cflag; - struct cp210x_flow_ctl flow_ctl; - u32 baud; - u16 bits; - u32 ctl_hs; - u32 flow_repl; - - cp210x_read_u32_reg(port, CP210X_GET_BAUDRATE, &baud); - - dev_dbg(dev, "%s - baud rate = %d\n", __func__, baud); - *baudp = baud; - - cflag = *cflagp; - - cp210x_get_line_ctl(port, &bits); - cflag &= ~CSIZE; - switch (bits & BITS_DATA_MASK) { - case BITS_DATA_5: - dev_dbg(dev, "%s - data bits = 5\n", __func__); - cflag |= CS5; - break; - case BITS_DATA_6: - dev_dbg(dev, "%s - data bits = 6\n", __func__); - cflag |= CS6; - break; - case BITS_DATA_7: - dev_dbg(dev, "%s - data bits = 7\n", __func__); - cflag |= CS7; - break; - case BITS_DATA_8: - dev_dbg(dev, "%s - data bits = 8\n", __func__); - cflag |= CS8; - break; - case BITS_DATA_9: - dev_dbg(dev, "%s - data bits = 9 (not supported, using 8 data bits)\n", __func__); - cflag |= CS8; - bits &= ~BITS_DATA_MASK; - bits |= BITS_DATA_8; - cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits); - break; - default: - dev_dbg(dev, "%s - Unknown number of data bits, using 8\n", __func__); - cflag |= CS8; - bits &= ~BITS_DATA_MASK; - bits |= BITS_DATA_8; - cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits); - break; - } - - switch (bits & BITS_PARITY_MASK) { - case BITS_PARITY_NONE: - dev_dbg(dev, "%s - parity = NONE\n", __func__); - cflag &= ~PARENB; - break; - case BITS_PARITY_ODD: - dev_dbg(dev, "%s - parity = ODD\n", __func__); - cflag |= (PARENB|PARODD); - break; - case BITS_PARITY_EVEN: - dev_dbg(dev, "%s - parity = EVEN\n", __func__); - cflag &= ~PARODD; - cflag |= PARENB; - break; - case BITS_PARITY_MARK: - dev_dbg(dev, "%s - parity = MARK\n", __func__); - cflag |= (PARENB|PARODD|CMSPAR); - break; - case BITS_PARITY_SPACE: - dev_dbg(dev, "%s - parity = SPACE\n", __func__); - cflag &= ~PARODD; - cflag |= (PARENB|CMSPAR); - break; - default: - dev_dbg(dev, "%s - Unknown parity mode, disabling parity\n", __func__); - cflag &= ~PARENB; - bits &= ~BITS_PARITY_MASK; - cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits); - break; - } - - cflag &= ~CSTOPB; - switch (bits & BITS_STOP_MASK) { - case BITS_STOP_1: - dev_dbg(dev, "%s - stop bits = 1\n", __func__); - break; - case BITS_STOP_1_5: - dev_dbg(dev, "%s - stop bits = 1.5 (not supported, using 1 stop bit)\n", __func__); - bits &= ~BITS_STOP_MASK; - cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits); - break; - case BITS_STOP_2: - dev_dbg(dev, "%s - stop bits = 2\n", __func__); - cflag |= CSTOPB; - break; - default: - dev_dbg(dev, "%s - Unknown number of stop bits, using 1 stop bit\n", __func__); - bits &= ~BITS_STOP_MASK; - cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits); - break; - } - - cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl, - sizeof(flow_ctl)); - ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake); - if (ctl_hs & CP210X_SERIAL_CTS_HANDSHAKE) { - dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__); - /* - * When the port is closed, the CP210x hardware disables - * auto-RTS and RTS is deasserted but it leaves auto-CTS when - * in hardware flow control mode. When re-opening the port, if - * auto-CTS is enabled on the cp210x, then auto-RTS must be - * re-enabled in the driver. - */ - flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace); - flow_repl &= ~CP210X_SERIAL_RTS_MASK; - flow_repl |= CP210X_SERIAL_RTS_SHIFT(CP210X_SERIAL_RTS_FLOW_CTL); - flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl); - cp210x_write_reg_block(port, - CP210X_SET_FLOW, - &flow_ctl, - sizeof(flow_ctl)); - - cflag |= CRTSCTS; - } else { - dev_dbg(dev, "%s - flow control = NONE\n", __func__); - cflag &= ~CRTSCTS; - } - - *cflagp = cflag; -} - struct cp210x_rate { speed_t rate; speed_t high; @@ -1366,17 +1100,13 @@ static void cp210x_set_termios(struct tty_struct *tty, { struct cp210x_serial_private *priv = usb_get_serial_data(port->serial); struct device *dev = &port->dev; - unsigned int cflag, old_cflag; u16 bits; int ret; - if (!cp210x_termios_change(&tty->termios, old_termios)) + if (old_termios && !cp210x_termios_change(&tty->termios, old_termios)) return; - cflag = tty->termios.c_cflag; - old_cflag = old_termios->c_cflag; - - if (tty->termios.c_ospeed != old_termios->c_ospeed) + if (!old_termios || tty->termios.c_ospeed != old_termios->c_ospeed) cp210x_change_speed(tty, port, old_termios); /* CP2101 only supports CS8, 1 stop bit and non-stick parity. */ @@ -1426,7 +1156,7 @@ static void cp210x_set_termios(struct tty_struct *tty, if (ret) dev_err(&port->dev, "failed to set line control: %d\n", ret); - if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) { + if (!old_termios || C_CRTSCTS(tty) != (old_termios->c_cflag & CRTSCTS)) { struct cp210x_flow_ctl flow_ctl; u32 ctl_hs; u32 flow_repl; @@ -1443,7 +1173,7 @@ static void cp210x_set_termios(struct tty_struct *tty, ctl_hs &= ~CP210X_SERIAL_DSR_SENSITIVITY; ctl_hs &= ~CP210X_SERIAL_DTR_MASK; ctl_hs |= CP210X_SERIAL_DTR_SHIFT(CP210X_SERIAL_DTR_ACTIVE); - if (cflag & CRTSCTS) { + if (C_CRTSCTS(tty)) { ctl_hs |= CP210X_SERIAL_CTS_HANDSHAKE; flow_repl &= ~CP210X_SERIAL_RTS_MASK; @@ -1979,7 +1709,6 @@ static int cp210x_port_probe(struct usb_serial_port *port) { struct usb_serial *serial = port->serial; struct cp210x_port_private *port_priv; - int ret; port_priv = kzalloc(sizeof(*port_priv), GFP_KERNEL); if (!port_priv) @@ -1989,12 +1718,6 @@ static int cp210x_port_probe(struct usb_serial_port *port) usb_set_serial_port_data(port, port_priv); - ret = cp210x_detect_swapped_line_ctl(port); - if (ret) { - kfree(port_priv); - return ret; - } - return 0; } -- cgit v1.2.3-70-g09d2 From b339628ec08c51c3445f4b094e0011406fc36344 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 16 Nov 2020 17:18:24 +0100 Subject: USB: serial: cp210x: drop flow-control debugging Drop some unnecessary flow-control debugging. Signed-off-by: Johan Hovold --- drivers/usb/serial/cp210x.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index bb9c3d7ccaee..04d2b15ceded 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -1165,8 +1165,6 @@ static void cp210x_set_termios(struct tty_struct *tty, sizeof(flow_ctl)); ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake); flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace); - dev_dbg(dev, "%s - read ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n", - __func__, ctl_hs, flow_repl); ctl_hs &= ~CP210X_SERIAL_DSR_HANDSHAKE; ctl_hs &= ~CP210X_SERIAL_DCD_HANDSHAKE; @@ -1179,17 +1177,15 @@ static void cp210x_set_termios(struct tty_struct *tty, flow_repl &= ~CP210X_SERIAL_RTS_MASK; flow_repl |= CP210X_SERIAL_RTS_SHIFT( CP210X_SERIAL_RTS_FLOW_CTL); - dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__); } else { ctl_hs &= ~CP210X_SERIAL_CTS_HANDSHAKE; flow_repl &= ~CP210X_SERIAL_RTS_MASK; flow_repl |= CP210X_SERIAL_RTS_SHIFT( CP210X_SERIAL_RTS_ACTIVE); - dev_dbg(dev, "%s - flow control = NONE\n", __func__); } - dev_dbg(dev, "%s - write ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n", + dev_dbg(dev, "%s - ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n", __func__, ctl_hs, flow_repl); flow_ctl.ulControlHandshake = cpu_to_le32(ctl_hs); flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl); -- cgit v1.2.3-70-g09d2 From ed921771ffb65ce1f65d746fe8f88c0e0a829d76 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 16 Nov 2020 17:18:25 +0100 Subject: USB: serial: cp210x: refactor flow-control handling Add a helper function to be used to configure flow control. The flow-control code was the last caller that relied on the memset-on-failure behaviour of cp210x_read_reg_block(), which we can now drop in favour of bailing out on errors when retrieving the flow-control settings. This should also simplify adding support for software flow control. Signed-off-by: Johan Hovold --- drivers/usb/serial/cp210x.c | 97 ++++++++++++++++++++++----------------------- 1 file changed, 47 insertions(+), 50 deletions(-) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index 04d2b15ceded..c77fd09b91ce 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -555,14 +555,8 @@ static int cp210x_read_reg_block(struct usb_serial_port *port, u8 req, int result; dmabuf = kmalloc(bufsize, GFP_KERNEL); - if (!dmabuf) { - /* - * FIXME Some callers don't bother to check for error, - * at least give them consistent junk until they are fixed - */ - memset(buf, 0, bufsize); + if (!dmabuf) return -ENOMEM; - } result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0), req, REQTYPE_INTERFACE_TO_HOST, 0, @@ -576,12 +570,6 @@ static int cp210x_read_reg_block(struct usb_serial_port *port, u8 req, req, bufsize, result); if (result >= 0) result = -EIO; - - /* - * FIXME Some callers don't bother to check for error, - * at least give them consistent junk until they are fixed - */ - memset(buf, 0, bufsize); } kfree(dmabuf); @@ -1095,11 +1083,55 @@ static bool cp210x_termios_change(const struct ktermios *a, const struct ktermio return tty_termios_hw_change(a, b) || iflag_change; } +static void cp210x_set_flow_control(struct tty_struct *tty, + struct usb_serial_port *port, struct ktermios *old_termios) +{ + struct cp210x_flow_ctl flow_ctl; + u32 flow_repl; + u32 ctl_hs; + int ret; + + if (old_termios && C_CRTSCTS(tty) == (old_termios->c_cflag & CRTSCTS)) + return; + + ret = cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl, + sizeof(flow_ctl)); + if (ret) + return; + + ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake); + flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace); + + ctl_hs &= ~CP210X_SERIAL_DSR_HANDSHAKE; + ctl_hs &= ~CP210X_SERIAL_DCD_HANDSHAKE; + ctl_hs &= ~CP210X_SERIAL_DSR_SENSITIVITY; + ctl_hs &= ~CP210X_SERIAL_DTR_MASK; + ctl_hs |= CP210X_SERIAL_DTR_SHIFT(CP210X_SERIAL_DTR_ACTIVE); + + if (C_CRTSCTS(tty)) { + ctl_hs |= CP210X_SERIAL_CTS_HANDSHAKE; + flow_repl &= ~CP210X_SERIAL_RTS_MASK; + flow_repl |= CP210X_SERIAL_RTS_SHIFT(CP210X_SERIAL_RTS_FLOW_CTL); + } else { + ctl_hs &= ~CP210X_SERIAL_CTS_HANDSHAKE; + flow_repl &= ~CP210X_SERIAL_RTS_MASK; + flow_repl |= CP210X_SERIAL_RTS_SHIFT(CP210X_SERIAL_RTS_ACTIVE); + } + + dev_dbg(&port->dev, "%s - ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n", + __func__, ctl_hs, flow_repl); + + flow_ctl.ulControlHandshake = cpu_to_le32(ctl_hs); + flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl); + + cp210x_write_reg_block(port, CP210X_SET_FLOW, &flow_ctl, + sizeof(flow_ctl)); +} + static void cp210x_set_termios(struct tty_struct *tty, struct usb_serial_port *port, struct ktermios *old_termios) { struct cp210x_serial_private *priv = usb_get_serial_data(port->serial); - struct device *dev = &port->dev; u16 bits; int ret; @@ -1156,42 +1188,7 @@ static void cp210x_set_termios(struct tty_struct *tty, if (ret) dev_err(&port->dev, "failed to set line control: %d\n", ret); - if (!old_termios || C_CRTSCTS(tty) != (old_termios->c_cflag & CRTSCTS)) { - struct cp210x_flow_ctl flow_ctl; - u32 ctl_hs; - u32 flow_repl; - - cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl, - sizeof(flow_ctl)); - ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake); - flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace); - - ctl_hs &= ~CP210X_SERIAL_DSR_HANDSHAKE; - ctl_hs &= ~CP210X_SERIAL_DCD_HANDSHAKE; - ctl_hs &= ~CP210X_SERIAL_DSR_SENSITIVITY; - ctl_hs &= ~CP210X_SERIAL_DTR_MASK; - ctl_hs |= CP210X_SERIAL_DTR_SHIFT(CP210X_SERIAL_DTR_ACTIVE); - if (C_CRTSCTS(tty)) { - ctl_hs |= CP210X_SERIAL_CTS_HANDSHAKE; - - flow_repl &= ~CP210X_SERIAL_RTS_MASK; - flow_repl |= CP210X_SERIAL_RTS_SHIFT( - CP210X_SERIAL_RTS_FLOW_CTL); - } else { - ctl_hs &= ~CP210X_SERIAL_CTS_HANDSHAKE; - - flow_repl &= ~CP210X_SERIAL_RTS_MASK; - flow_repl |= CP210X_SERIAL_RTS_SHIFT( - CP210X_SERIAL_RTS_ACTIVE); - } - - dev_dbg(dev, "%s - ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n", - __func__, ctl_hs, flow_repl); - flow_ctl.ulControlHandshake = cpu_to_le32(ctl_hs); - flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl); - cp210x_write_reg_block(port, CP210X_SET_FLOW, &flow_ctl, - sizeof(flow_ctl)); - } + cp210x_set_flow_control(tty, port, old_termios); /* * Enable event-insertion mode only if input parity checking is -- cgit v1.2.3-70-g09d2 From daa919196be49815e342eb79fab81852102703c2 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 16 Nov 2020 17:18:26 +0100 Subject: USB: serial: cp210x: clean up dtr_rts() Clean up dtr_rts() by renaming the port parameter and adding missing whitespace. Signed-off-by: Johan Hovold --- drivers/usb/serial/cp210x.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index c77fd09b91ce..fbb10dfc56e3 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -46,7 +46,7 @@ static void cp210x_disconnect(struct usb_serial *); static void cp210x_release(struct usb_serial *); static int cp210x_port_probe(struct usb_serial_port *); static int cp210x_port_remove(struct usb_serial_port *); -static void cp210x_dtr_rts(struct usb_serial_port *p, int on); +static void cp210x_dtr_rts(struct usb_serial_port *port, int on); static void cp210x_process_read_urb(struct urb *urb); static void cp210x_enable_event_mode(struct usb_serial_port *port); static void cp210x_disable_event_mode(struct usb_serial_port *port); @@ -1234,12 +1234,12 @@ static int cp210x_tiocmset_port(struct usb_serial_port *port, return cp210x_write_u16_reg(port, CP210X_SET_MHS, control); } -static void cp210x_dtr_rts(struct usb_serial_port *p, int on) +static void cp210x_dtr_rts(struct usb_serial_port *port, int on) { if (on) - cp210x_tiocmset_port(p, TIOCM_DTR|TIOCM_RTS, 0); + cp210x_tiocmset_port(port, TIOCM_DTR | TIOCM_RTS, 0); else - cp210x_tiocmset_port(p, 0, TIOCM_DTR|TIOCM_RTS); + cp210x_tiocmset_port(port, 0, TIOCM_DTR | TIOCM_RTS); } static int cp210x_tiocmget(struct tty_struct *tty) -- cgit v1.2.3-70-g09d2 From a251963f76fa0226d0fdf0c4f989496f18d9ae7f Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 9 Dec 2020 11:42:21 +0100 Subject: USB: serial: option: add interface-number sanity check to flag handling Add an interface-number sanity check before testing the device flags to avoid relying on undefined behaviour when left shifting in case a device uses an interface number greater than or equal to BITS_PER_LONG (i.e. 64 or 32). Reported-by: syzbot+8881b478dad0a7971f79@syzkaller.appspotmail.com Fixes: c3a65808f04a ("USB: serial: option: reimplement interface masking") Cc: stable@vger.kernel.org Reviewed-by: Greg Kroah-Hartman Signed-off-by: Johan Hovold --- drivers/usb/serial/option.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c index 2a3bfd6f867e..c5908c4f2046 100644 --- a/drivers/usb/serial/option.c +++ b/drivers/usb/serial/option.c @@ -561,6 +561,9 @@ static void option_instat_callback(struct urb *urb); /* Device flags */ +/* Highest interface number which can be used with NCTRL() and RSVD() */ +#define FLAG_IFNUM_MAX 7 + /* Interface does not support modem-control requests */ #define NCTRL(ifnum) ((BIT(ifnum) & 0xff) << 8) @@ -2089,6 +2092,14 @@ static struct usb_serial_driver * const serial_drivers[] = { module_usb_serial_driver(serial_drivers, option_ids); +static bool iface_is_reserved(unsigned long device_flags, u8 ifnum) +{ + if (ifnum > FLAG_IFNUM_MAX) + return false; + + return device_flags & RSVD(ifnum); +} + static int option_probe(struct usb_serial *serial, const struct usb_device_id *id) { @@ -2105,7 +2116,7 @@ static int option_probe(struct usb_serial *serial, * the same class/subclass/protocol as the serial interfaces. Look at * the Windows driver .INF files for reserved interface numbers. */ - if (device_flags & RSVD(iface_desc->bInterfaceNumber)) + if (iface_is_reserved(device_flags, iface_desc->bInterfaceNumber)) return -ENODEV; /* @@ -2121,6 +2132,14 @@ static int option_probe(struct usb_serial *serial, return 0; } +static bool iface_no_modem_control(unsigned long device_flags, u8 ifnum) +{ + if (ifnum > FLAG_IFNUM_MAX) + return false; + + return device_flags & NCTRL(ifnum); +} + static int option_attach(struct usb_serial *serial) { struct usb_interface_descriptor *iface_desc; @@ -2136,7 +2155,7 @@ static int option_attach(struct usb_serial *serial) iface_desc = &serial->interface->cur_altsetting->desc; - if (!(device_flags & NCTRL(iface_desc->bInterfaceNumber))) + if (!iface_no_modem_control(device_flags, iface_desc->bInterfaceNumber)) data->use_send_setup = 1; if (device_flags & ZLP) -- cgit v1.2.3-70-g09d2 From 11fb08cffbebcbda76b3d882c19398c7571ab355 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Fri, 4 Dec 2020 16:47:37 +0000 Subject: USB: serial: ftdi_sio: report the valid GPIO lines to gpiolib Since it is pretty common for only some of the CBUS lines to be valid as GPIO lines, let's report such validity to the rest of the kernel. Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20201204164739.781812-3-maz@kernel.org Reviewed-by: Andy Shevchenko Reviewed-by: Linus Walleij Signed-off-by: Johan Hovold --- drivers/usb/serial/ftdi_sio.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index e0f4c3d9649c..13e575f16bcd 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -2002,6 +2002,19 @@ static int ftdi_gpio_direction_output(struct gpio_chip *gc, unsigned int gpio, return result; } +static int ftdi_gpio_init_valid_mask(struct gpio_chip *gc, + unsigned long *valid_mask, + unsigned int ngpios) +{ + struct usb_serial_port *port = gpiochip_get_data(gc); + struct ftdi_private *priv = usb_get_serial_port_data(port); + unsigned long map = priv->gpio_altfunc; + + bitmap_complement(valid_mask, &map, ngpios); + + return 0; +} + static int ftdi_read_eeprom(struct usb_serial *serial, void *dst, u16 addr, u16 nbytes) { @@ -2173,6 +2186,7 @@ static int ftdi_gpio_init(struct usb_serial_port *port) priv->gc.get_direction = ftdi_gpio_direction_get; priv->gc.direction_input = ftdi_gpio_direction_input; priv->gc.direction_output = ftdi_gpio_direction_output; + priv->gc.init_valid_mask = ftdi_gpio_init_valid_mask; priv->gc.get = ftdi_gpio_get; priv->gc.set = ftdi_gpio_set; priv->gc.get_multiple = ftdi_gpio_get_multiple; -- cgit v1.2.3-70-g09d2 From 5d47c887cceed5261e82e2cc7e996b877d5381f8 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Fri, 4 Dec 2020 16:47:39 +0000 Subject: USB: serial: ftdi_sio: drop GPIO line checking dead code Now that gpiolib can track the validity of GPIO pins, there is no need to check whether the line is valid in request(). Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20201204164739.781812-5-maz@kernel.org Reviewed-by: Andy Shevchenko [johan: amend commit message] Signed-off-by: Johan Hovold --- drivers/usb/serial/ftdi_sio.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index 13e575f16bcd..a41ce9e60b96 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -1841,9 +1841,6 @@ static int ftdi_gpio_request(struct gpio_chip *gc, unsigned int offset) struct ftdi_private *priv = usb_get_serial_port_data(port); int result; - if (priv->gpio_altfunc & BIT(offset)) - return -ENODEV; - mutex_lock(&priv->gpio_lock); if (!priv->gpio_used) { /* Set default pin states, as we cannot get them from device */ -- cgit v1.2.3-70-g09d2 From fddd408ad448efc49c67f8dfdc4e86b31c683a0c Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Fri, 4 Dec 2020 16:47:38 +0000 Subject: USB: serial: ftdi_sio: log the CBUS GPIO validity The validity of the ftdi CBUS GPIO is pretty hidden so far, and finding out *why* some GPIOs don't work is sometimes hard to identify. So let's help the user by displaying the map of the CBUS pins that are valid for a GPIO. Suggested-by: Linus Walleij Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20201204164739.781812-4-maz@kernel.org Reviewed-by: Andy Shevchenko [johan: demote to KERN_DEBUG, rephrase messages, drop ftx-prog warning] Signed-off-by: Johan Hovold --- drivers/usb/serial/ftdi_sio.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index a41ce9e60b96..94398f89e600 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -2009,6 +2009,12 @@ static int ftdi_gpio_init_valid_mask(struct gpio_chip *gc, bitmap_complement(valid_mask, &map, ngpios); + if (bitmap_empty(valid_mask, ngpios)) + dev_dbg(&port->dev, "no CBUS pin configured for GPIO\n"); + else + dev_dbg(&port->dev, "CBUS%*pbl configured for GPIO\n", ngpios, + valid_mask); + return 0; } -- cgit v1.2.3-70-g09d2