Patchwork [v2] usb: serial: defer URB submission for ftdi_sio

login
register
mail settings
Submitter Ramachandran Srinivasan (BRT-SG)
Date April 9, 2019, 10:47 a.m.
Message ID <1554806826-11543-1-git-send-email-srinivasan.r@brtchip.com>
Download mbox | patch
Permalink /patch/768489/
State New
Headers show

Comments

Ramachandran Srinivasan (BRT-SG) - April 9, 2019, 10:47 a.m.
From: Srinivasan R <srinivasan.r@brtchip.com>

Deferring the URB resubmission, this can help time share the available
DMA chanels of DWC_OTG host controller in RaspberryPi.This change can
fix the problem, failed to enumerate the USB device when all the 8
instance of ftdi_sio serial driver is open by application. many bugs
had been rasied for RaspberryPi around this problem can be solved

Signed-off-by: Srinivasan Ramachandran <srinivasan.r@brtchip.com>
---
 drivers/usb/serial/ftdi_sio.c | 153 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 152 insertions(+), 1 deletion(-)

--
2.7.4


Please note BRT have updated their Privacy Policy.

Please click on the following link to access and review.

BRT Privacy Policy<http://brtchip.com/privacy-policy/>
Johan Hovold - April 15, 2019, 8 a.m.
On Tue, Apr 09, 2019 at 10:47:42AM +0000, Ramachandran Srinivasan (BRT-SG) wrote:
> From: Srinivasan R <srinivasan.r@brtchip.com>
> 
> Deferring the URB resubmission, this can help time share the available
> DMA chanels of DWC_OTG host controller in RaspberryPi.This change can
> fix the problem, failed to enumerate the USB device when all the 8
> instance of ftdi_sio serial driver is open by application. many bugs
> had been rasied for RaspberryPi around this problem can be solved

Thanks for resending with a commit message. Your patch is still
white-space damaged (check your mail setup, and make sure you can send a
patch to yourself and apply it with git am), but with the above
description I think I can give some feedback already.

First of all, if there's a problem here, it would need to be fixed in
the host-controller driver of the rpi as we don't want to add
workarounds for host-controller issues to every USB driver.

I suggest you write up an even more detailed bug report describing the
problem you're seeing and send it to the list.

Meanwhile, you can try increasing the latency timer of the connected
ftdi devices to reduce the amount of traffic they generate when they're
otherwise idle (16 ms default, can be increased up to 255 ms I think).
That may possibly allow you to push the limits of the rpi.

Johan

Patch

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 1d8077e..63edd38 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -40,12 +40,16 @@ 
 #include <linux/usb.h>
 #include <linux/serial.h>
 #include <linux/usb/serial.h>
+#include <linux/hrtimer.h>
+#include <linux/ktime.h>
 #include "ftdi_sio.h"
 #include "ftdi_sio_ids.h"

 #define DRIVER_AUTHOR "Greg Kroah-Hartman <greg@kroah.com>, Bill Ryder <bryder@sgi.com>, Kuba Ober <kuba@mareimbrium.org>, Andreas Mohr, Johan Hovold <jhovold@gmail.com>"
 #define DRIVER_DESC "USB FTDI Serial Converters Driver"

+#define HUNDREDMS    (100 * 1000000)
+#define TIMEOUT  (30 * 1000000)  // mill seconds, default value

 struct ftdi_private {
 enum ftdi_chip_type chip_type;
@@ -72,6 +76,11 @@  struct ftdi_private {
 unsigned int latency;/* latency setting in use */
 unsigned short max_packet_size;
 struct mutex cfg_lock; /* Avoid mess by parallel calls of config ioctl() and change_speed() */
+spinlock_tlock;
+ktime_t ktime;
+struct hrtimer etx_hr_timer;
+struct tty_struct *tty;
+struct device   *dev;
 };

 /* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -1046,10 +1055,15 @@  static int  ftdi_sio_probe(struct usb_serial *serial,
 static int  ftdi_sio_port_probe(struct usb_serial_port *port);
 static int  ftdi_sio_port_remove(struct usb_serial_port *port);
 static int  ftdi_open(struct tty_struct *tty, struct usb_serial_port *port);
+static void ftdi_close(struct usb_serial_port *port);
+static void ftdi_cancel_timer(struct usb_serial_port *port);
 static void ftdi_dtr_rts(struct usb_serial_port *port, int on);
 static void ftdi_process_read_urb(struct urb *urb);
 static int ftdi_prepare_write_buffer(struct usb_serial_port *port,
 void *dest, size_t size);
+static void ftdi_serial_read_bulk_callback(struct urb *urb);
+static void ftdi_serial_throttle(struct tty_struct *tty);
+static void ftdi_serial_unthrottle(struct tty_struct *tty);
 static void ftdi_set_termios(struct tty_struct *tty,
 struct usb_serial_port *port, struct ktermios *old);
 static int  ftdi_tiocmget(struct tty_struct *tty);
@@ -1083,10 +1097,12 @@  static struct usb_serial_driver ftdi_sio_device = {
 .port_probe =ftdi_sio_port_probe,
 .port_remove =ftdi_sio_port_remove,
 .open =ftdi_open,
+.close =ftdi_close,
 .dtr_rts =ftdi_dtr_rts,
 .throttle =usb_serial_generic_throttle,
 .unthrottle =usb_serial_generic_unthrottle,
 .process_read_urb =ftdi_process_read_urb,
+.read_bulk_callback = ftdi_serial_read_bulk_callback,
 .prepare_write_buffer =ftdi_prepare_write_buffer,
 .tiocmget =ftdi_tiocmget,
 .tiocmset =ftdi_tiocmset,
@@ -1102,6 +1118,13 @@  static struct usb_serial_driver * const serial_drivers[] = {
 &ftdi_sio_device, NULL
 };

+/*
+ * Module parameter to control URB defer timer for FTDI-based .
+ * USB serial converter, If this value is not set in /etc/modprobe.d/
+ * its value will be set to 30ms, maximum value of the delay can be 100ms
+ */
+static unsigned long urb_defer_timer = TIMEOUT;
+

 #define WDR_TIMEOUT 5000 /* default urb timeout */
 #define WDR_SHORT_TIMEOUT 1000/* shorter urb timeout */
@@ -1112,6 +1135,33 @@  static struct usb_serial_driver * const serial_drivers[] = {
  * ***************************************************************************
  */

+static enum hrtimer_restart timer_callbackserial(struct hrtimer *timer)
+{
+struct ftdi_private *priv = container_of(timer, struct ftdi_private,
+etx_hr_timer);
+int stopped = 0;
+unsigned long flags;
+
+spin_lock_irqsave(&priv->lock, flags);
+if (!priv->tty)
+stopped = 1;
+spin_unlock_irqrestore(&priv->lock, flags);
+if (!stopped)
+ftdi_serial_unthrottle(priv->tty);
+return HRTIMER_NORESTART;
+}
+
+static void ftdi_cancel_timer(struct usb_serial_port *port)
+{
+unsigned long flags;
+struct ftdi_private *priv = usb_get_serial_port_data(port);
+
+hrtimer_cancel(&priv->etx_hr_timer);
+spin_lock_irqsave(&port->lock, flags);
+priv->tty = NULL;
+spin_unlock_irqrestore(&port->lock, flags);
+}
+
 static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)
 {
 unsigned short int divisor;
@@ -1815,9 +1865,87 @@  static int ftdi_sio_port_probe(struct usb_serial_port *port)
 priv->latency = 16;
 write_latency_timer(port);
 create_sysfs_attrs(port);
+priv->ktime = ktime_set(0, ((urb_defer_timer > 0) &&
+(urb_defer_timer <= HUNDREDMS)) ? urb_defer_timer : TIMEOUT);
+hrtimer_init(&priv->etx_hr_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+priv->etx_hr_timer.function = &timer_callbackserial;
+priv->dev = &port->dev;
+spin_lock_init(&port->lock);
 return 0;
 }

+static void ftdi_serial_read_bulk_callback(struct urb *urb)
+{
+struct usb_serial_port *port = urb->context;
+unsigned long flags;
+int status = urb->status;
+int i;
+
+for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i) {
+if (urb == port->read_urbs[i])
+break;
+}
+set_bit(i, &port->read_urbs_free);
+
+dev_dbg(&port->dev, "%s - urb %d, len %d\n", __func__, i,
+urb->actual_length);
+switch (status) {
+case 0:
+break;
+case -ENOENT:
+case -ECONNRESET:
+case -ESHUTDOWN:
+dev_dbg(&port->dev, "%s - urb stopped: %d\n",
+__func__, status);
+return;
+case -EPIPE:
+dev_err(&port->dev, "%s - urb stopped: %d\n",
+__func__, status);
+return;
+default:
+dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
+__func__, status);
+return;
+}
+
+spin_lock_irqsave(&port->lock, flags);
+port->throttled = port->throttle_req;
+if (!port->throttled) {
+spin_unlock_irqrestore(&port->lock, flags);
+/*Stop submitting back the URB, later in the Timer callback
+ * submit
+ */
+ftdi_serial_throttle(tty_port_tty_get(&port->port));
+} else {
+spin_unlock_irqrestore(&port->lock, flags);
+}
+ftdi_process_read_urb(urb);
+}
+static void ftdi_serial_throttle(struct tty_struct *tty)
+{
+struct usb_serial_port *port = tty->driver_data;
+unsigned long flags;
+
+spin_lock_irqsave(&port->lock, flags);
+port->throttle_req = 1;
+spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void ftdi_serial_unthrottle(struct tty_struct *tty)
+{
+struct usb_serial_port *port = tty->driver_data;
+unsigned long flags;
+int was_throttled;
+
+spin_lock_irqsave(&port->lock, flags);
+was_throttled = port->throttled;
+port->throttled = port->throttle_req = 0;
+spin_unlock_irqrestore(&port->lock, flags);
+
+if (was_throttled)
+usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
+}
+
 /* Setup for the USB-UIRT device, which requires hardwired
  * baudrate (38400 gets mapped to 312500) */
 /* Called from usbserial:serial_probe */
@@ -1956,11 +2084,18 @@  static int ftdi_open(struct tty_struct *tty, struct usb_serial_port *port)
    This is same behaviour as serial.c/rs_open() - Kuba */

 /* ftdi_set_termios  will send usb control messages */
-if (tty)
+if (tty) {
 ftdi_set_termios(tty, port, NULL);
+priv->tty = tty;
+}

 return usb_serial_generic_open(tty, port);
 }
+static void ftdi_close(struct usb_serial_port *port)
+{
+usb_serial_generic_close(port);
+ftdi_cancel_timer(port);
+}

 static void ftdi_dtr_rts(struct usb_serial_port *port, int on)
 {
@@ -2127,6 +2262,8 @@  static void ftdi_process_read_urb(struct urb *urb)
 int i;
 int len;
 int count = 0;
+unsigned long flags;
+bool isstoped = false;

 for (i = 0; i < urb->actual_length; i += priv->max_packet_size) {
 len = min_t(int, urb->actual_length - i, priv->max_packet_size);
@@ -2135,6 +2272,18 @@  static void ftdi_process_read_urb(struct urb *urb)

 if (count)
 tty_flip_buffer_push(&port->port);
+
+spin_lock_irqsave(&port->lock, flags);
+if (!priv->tty)
+isstoped = true;
+/*Start only one Timer for all the URB Buffers per port*/
+if ((port->throttled) && (isstoped == false)) {
+spin_unlock_irqrestore(&port->lock, flags);
+hrtimer_start(&priv->etx_hr_timer, priv->ktime,
+HRTIMER_MODE_REL);
+} else {
+spin_unlock_irqrestore(&port->lock, flags);
+}
 }

 static void ftdi_break_ctl(struct tty_struct *tty, int break_state)
@@ -2475,3 +2624,5 @@  MODULE_LICENSE("GPL");

 module_param(ndi_latency_timer, int, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(ndi_latency_timer, "NDI device latency timer override");
+module_param(urb_defer_timer, ulong, 0644);
+MODULE_PARM_DESC(urb_defer_timer, "urb_defer delay in ms, default value 30ms");