[PATCH 0/6] USB: serial: ftdi_sio: configurable read-URB defer, per-port low_latency, latency_timer reliability

[PATCH 0/6] USB: serial: ftdi_sio: configurable read-URB defer, per-port low_latency, latency_timer reliability

[Chinna Mopurigari Naveen Kumar Reddy]

This series adds an opt-in mechanism to bound ftdi_sio's read-URB
resubmission rate, a per-port escape hatch for it, and two small
reliability fixes to the existing latency_timer sysfs attribute.  None
of it changes the driver's default behaviour.

Background
==========

ftdi_sio resubmits read URBs as fast as the previous URB completes.
On host controllers that do not arbitrate DMA-channel allocation
between devices, a single FT4232 at high baud (4 channels x ~916 kbps)
can keep most or all of the controller's DMA channels occupied;
multiple FT4232 chips on the same bus exhaust them entirely.  The
control endpoint and unrelated devices on the same bus then make no
progress -- device enumeration fails, hub events stall.  The
BCM2835/BCM2837 DWC_OTG controller used on the Raspberry Pi Compute
Module 3 is the platform where this was characterised.

This series introduces a small inter-batch defer on read-URB
resubmission that bounds per-device DMA-channel occupancy.  It is
disabled by default; behaviour on every unaffected host is unchanged.

Series layout
=============

Patch 1 adds the inter-batch defer and the urb_defer_timer_ns module
parameter that controls it (default 0 = disabled).  It also adds a
small helper, ftdi_atomic_submit_read_urbs(), used wherever read URBs
are resubmitted from completion or hrtimer (atomic) context:
usb_serial_generic_submit_read_urbs() looks atomic-usable because it
takes a gfp_t, but its error-recovery path calls usb_kill_urb(), which
sleeps -- so a failed inner submit under bus pressure would sleep in
atomic context.  The helper submits each URB inline with
usb_submit_urb(GFP_ATOMIC), which does not sleep.

Patch 2 wraps usb_control_msg() with a retry on the documented
transient errno values (-ETIMEDOUT, -EPIPE, -EPROTO) seen under heavy
bus pressure, and converts write_latency_timer() to use it.

Patch 3 makes an explicit sysfs write to .../latency_timer
authoritative against the legacy ASYNC_LOW_LATENCY tty-flag clamp set
via TIOCSSERIAL by tools such as setserial(8).

Patch 4 adds a per-port low_latency sysfs attribute that bypasses the
inter-batch defer on a single port when the defer is globally enabled.

Patch 5 serialises the low_latency toggle against the read-bulk
callback so the attribute can be flipped while a port is actively
reading or writing.

Patch 6 adds a low_latency_defer_ns module parameter: instead of fully
bypassing the defer, a low_latency port can be paced with a small
per-port interval (e.g. 2 ms), which lets several low_latency ports
run concurrently without re-starving control transfers.

Testing
=======

Functionally validated on the affected hardware (Raspberry Pi Compute
Module 3, BCM2837 DWC_OTG, two FT4232 chips, eight RS485 channels at
916 kbps each) on the 6.12 stable kernel: with the defer enabled,
worst-case control-transfer latency under concurrent load dropped from
~50 ms (stock) to ~1 ms; per-port low_latency restores native read
latency on a chosen port; the paced path (low_latency_defer_ns=2ms)
lets four low_latency ports run without starving control transfers or
the USB-Ethernet path.

Runtime-tested on mainline at the series base (arm64, Raspberry Pi 4,
VL805 xHCI host, CONFIG_DEBUG_ATOMIC_SLEEP=y): module load/unload,
default-path loopback regression check, the deferred-resubmit path
(urb_defer_timer_ns=30ms) with live data, low_latency toggling under
load (200 cycles, no stall), explicit latency_timer writes, the paced
low_latency path (low_latency_defer_ns=2ms), and 50x open/close churn
-- all pass with no atomic-context splats.  Note this host does not
reproduce the DMA-starvation problem (that requires the DWC_OTG
controller); the mainline run is functional validation of the new
code paths.

scripts/checkpatch.pl --strict is clean on every patch.

Chinna Mopurigari Naveen Kumar Reddy (6):
  USB: serial: ftdi_sio: add configurable inter-batch defer for read
    URBs
  USB: serial: ftdi_sio: retry transient errors on chip-side control
    transfers
  USB: serial: ftdi_sio: make explicit latency_timer sysfs write
    authoritative
  USB: serial: ftdi_sio: add per-port low_latency sysfs attribute
  USB: serial: ftdi_sio: serialise low_latency toggle against
    read_bulk_callback
  USB: serial: ftdi_sio: pace low_latency ports with
    low_latency_defer_ns

 drivers/usb/serial/ftdi_sio.c | 488 +++++++++++++++++++++++++++++++++-
 1 file changed, 480 insertions(+), 8 deletions(-)


base-commit: ba3e43a9e601636f5edb54e259a74f96ca3b8fd8
-- 
2.43.0

[PATCH 1/6] USB: serial: ftdi_sio: add configurable inter-batch defer for read URBs

[Chinna Mopurigari Naveen Kumar Reddy]

Some USB host controllers do not enforce DMA-channel fairness
between devices.  On such hosts, ftdi_sio's read-URB resubmission
rate from a single high-throughput device can occupy every
available DMA channel and block progress on the control endpoint
and unrelated devices on the same bus, with visible consequences
such as failed device enumeration and stalled hub events.  The
BCM2835 DWC_OTG controller used on the Raspberry Pi Compute Module
3 is one example: at ~916 kbps per channel across two FT4232 chips
(eight RS485 channels), the symptom is reproducible.

Most hosts are not affected and should see no behavioural change.
Field testing on a CM3 with two FT4232 chips shows the symptom
only when more than five channels are concurrently open at this
baud; smaller configurations on the same host work fine with no
defer at all and would only pay a latency cost for it.  A default
defer would therefore be inappropriate.

Add a module parameter urb_defer_timer_ns whose default is 0,
which leaves the new code path inactive: ftdi_read_bulk_callback()
delegates straight to usb_serial_generic_read_bulk_callback() and
the driver is bit-for-bit equivalent to the version without this
patch.  Setting urb_defer_timer_ns to a non-zero value (clamped to
FTDI_URB_DEFER_MAX_NS = 100 ms) enables a per-port inter-batch
defer: after each read URB is processed, the port is throttled and
an hrtimer is armed; on expiry, usb_serial_generic_unthrottle()
clears USB_SERIAL_THROTTLED and the generic submit path resubmits
the next batch.  This bounds per-device DMA-channel occupancy on
the host controller without changing how each URB is processed.

The parameter is settable at module load time
(modprobe ftdi_sio urb_defer_timer_ns=30000000) or at runtime via
/sys/module/ftdi_sio/parameters/urb_defer_timer_ns; runtime
changes apply to ports probed after the change.

A new ftdi_close op cancels any pending hrtimer and drops the tty
reference captured at open.  ftdi_port_remove() also cancels the
timer so it cannot fire on freed memory after a hot unplug.

Read URBs are resubmitted from the hrtimer callback (and, when no tty
is bound, directly from the completion handler) -- both atomic
contexts.  usb_serial_generic_submit_read_urbs() cannot be used there:
its error-recovery path calls usb_kill_urb(), which sleeps, so a
failed inner submit under bus pressure would sleep in atomic context.
A small helper, ftdi_atomic_submit_read_urbs(), submits each free read
URB inline with usb_submit_urb(GFP_ATOMIC) -- which does not sleep --
and returns the URB to the free list on failure for the next
completion to re-drive.

100 ms is the upper bound at which most real-world RS-232/RS-485
protocols still function; values higher than this are rejected as
indicative of misconfiguration.

Subsequent patches in this series add a per-port low_latency sysfs
attribute that selectively bypasses the defer on a single port
when it is globally enabled, and the serialisation needed to
toggle that attribute safely under load.

Signed-off-by: Chinna Mopurigari Naveen Kumar Reddy <naveen.reddy@ftdichip.com>
---
 drivers/usb/serial/ftdi_sio.c | 274 +++++++++++++++++++++++++++++++++-
 1 file changed, 273 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index af14548fa03d..8ae887107de7 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -41,12 +41,25 @@
 #include <linux/serial.h>
 #include <linux/gpio/driver.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"
 
+/*
+ * Configurable inter-batch defer on read-URB resubmission. Useful on
+ * host controllers that do not enforce DMA-channel fairness between
+ * devices (the BCM2835 DWC_OTG on the Raspberry Pi Compute Module 3
+ * is one example), where an unbounded read-URB rate from a single
+ * high-throughput device can occupy every DMA channel and prevent
+ * progress on the control endpoint and unrelated devices on the same
+ * bus. Bounded to 100 ms; disabled by default.
+ */
+#define FTDI_URB_DEFER_MAX_NS	(100 * 1000000)
+
 enum ftdi_chip_type {
 	SIO,
 	FT232A,
@@ -89,6 +102,13 @@ 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() */
+
+	/* Inter-batch defer state (see FTDI_URB_DEFER_MAX_NS). */
+	spinlock_t		urb_defer_lock;	/* guards tty and timer_active */
+	ktime_t			urb_defer_ktime;
+	struct hrtimer		urb_defer_timer;
+	struct tty_struct	*urb_defer_tty;
+	bool			urb_defer_timer_active;
 #ifdef CONFIG_GPIOLIB
 	struct gpio_chip gc;
 	struct mutex gpio_lock;	/* protects GPIO state */
@@ -2167,6 +2187,95 @@ static void ftdi_gpio_remove(struct usb_serial_port *port) { }
  * ***************************************************************************
  */
 
+/*
+ * Module parameter: inter-batch defer between read-URB resubmissions,
+ * in nanoseconds.
+ *
+ * Default 0 -- defer disabled, behaviour identical to prior versions
+ * of this driver. Set to a non-zero value (typically 30000000 = 30 ms;
+ * clamped to FTDI_URB_DEFER_MAX_NS = 100 ms) to bound the read-URB
+ * resubmission rate on hosts whose USB controller does not enforce
+ * DMA-channel fairness. Settable at module load time
+ * (modprobe ftdi_sio urb_defer_timer_ns=30000000) or at runtime via
+ * /sys/module/ftdi_sio/parameters/urb_defer_timer_ns; the new value
+ * takes effect on ports probed after the change.
+ */
+static unsigned long urb_defer_timer_ns;
+
+/*
+ * hrtimer callback that fires after the configured defer interval and
+ * un-throttles the port so the generic submit-read-urbs path can run
+ * again. The timer is armed with HRTIMER_MODE_REL, so this runs in
+ * atomic (hard-irq) context and must not sleep.
+ */
+/*
+ * Resubmit the port's free read URBs from atomic context.
+ *
+ * usb_serial_generic_submit_read_urbs() accepts a gfp_t and looks
+ * usable from atomic context, but its error-recovery path calls
+ * usb_kill_urb(), which sleeps waiting for the URB to be reaped.  If
+ * an individual usb_submit_urb() fails -- which can happen under bus
+ * pressure -- calling that helper from a completion handler or hrtimer
+ * callback would sleep in atomic context.  Submit each free URB inline
+ * instead; usb_submit_urb() does not sleep under GFP_ATOMIC.  On
+ * failure the URB is returned to the free list and the next completion
+ * re-drives submission.
+ */
+static void ftdi_atomic_submit_read_urbs(struct usb_serial_port *port)
+{
+	int i;
+	int res;
+
+	for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i) {
+		if (!test_and_clear_bit(i, &port->read_urbs_free))
+			continue;
+
+		res = usb_submit_urb(port->read_urbs[i], GFP_ATOMIC);
+		if (res) {
+			dev_dbg(&port->dev, "%s - submit failed: %d\n",
+				__func__, res);
+			set_bit(i, &port->read_urbs_free);
+		}
+	}
+}
+
+static enum hrtimer_restart ftdi_urb_defer_timer_callback(struct hrtimer *timer)
+{
+	struct ftdi_private *priv = container_of(timer, struct ftdi_private,
+						 urb_defer_timer);
+	struct tty_struct *tty;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->urb_defer_lock, flags);
+	priv->urb_defer_timer_active = false;
+	tty = priv->urb_defer_tty;
+	if (tty)
+		tty_kref_get(tty);
+	spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
+
+	if (tty) {
+		struct usb_serial_port *port = tty->driver_data;
+
+		/*
+		 * This is the body of usb_serial_generic_unthrottle(), but
+		 * that helper resubmits with GFP_KERNEL and may sleep; we are
+		 * in the hrtimer's atomic context, so clear the throttle bit
+		 * and resubmit with GFP_ATOMIC instead.
+		 */
+		clear_bit(USB_SERIAL_THROTTLED, &port->flags);
+		/*
+		 * Order the throttle-bit clear before the resubmit, pairing
+		 * with the barrier that guards the read_urbs_free update in
+		 * ftdi_read_bulk_callback().
+		 */
+		smp_mb__after_atomic();
+		ftdi_atomic_submit_read_urbs(port);
+		tty_kref_put(tty);
+	}
+
+	return HRTIMER_NORESTART;
+}
+
 static int ftdi_probe(struct usb_serial *serial, const struct usb_device_id *id)
 {
 	const struct ftdi_quirk *quirk = (struct ftdi_quirk *)id->driver_info;
@@ -2186,6 +2295,7 @@ static int ftdi_port_probe(struct usb_serial_port *port)
 {
 	const struct ftdi_quirk *quirk = usb_get_serial_data(port->serial);
 	struct ftdi_private *priv;
+	unsigned long defer_ns;
 	int result;
 
 	priv = kzalloc_obj(struct ftdi_private);
@@ -2193,6 +2303,20 @@ static int ftdi_port_probe(struct usb_serial_port *port)
 		return -ENOMEM;
 
 	mutex_init(&priv->cfg_lock);
+	spin_lock_init(&priv->urb_defer_lock);
+
+	/*
+	 * urb_defer_timer_ns == 0 leaves the inter-batch defer disabled
+	 * for this port; ftdi_read_bulk_callback() short-circuits to the
+	 * generic callback below. Otherwise clamp out-of-range values to
+	 * the documented maximum.
+	 */
+	defer_ns = urb_defer_timer_ns;
+	if (defer_ns > FTDI_URB_DEFER_MAX_NS)
+		defer_ns = FTDI_URB_DEFER_MAX_NS;
+	priv->urb_defer_ktime = ns_to_ktime(defer_ns);
+	hrtimer_setup(&priv->urb_defer_timer, &ftdi_urb_defer_timer_callback,
+		      CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 
 	if (quirk && quirk->port_probe)
 		quirk->port_probe(priv);
@@ -2307,6 +2431,8 @@ static void ftdi_port_remove(struct usb_serial_port *port)
 {
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
 
+	hrtimer_cancel(&priv->urb_defer_timer);
+
 	ftdi_gpio_remove(port);
 
 	kfree(priv);
@@ -2316,6 +2442,7 @@ static int ftdi_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
 	struct usb_device *dev = port->serial->dev;
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
+	unsigned long flags;
 
 	/* No error checking for this (will get errors later anyway) */
 	/* See ftdi_sio.h for description of what is reset */
@@ -2329,12 +2456,46 @@ 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);
+		spin_lock_irqsave(&priv->urb_defer_lock, flags);
+		priv->urb_defer_tty = tty_kref_get(tty);
+		priv->urb_defer_timer_active = false;
+		spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
+	}
 
 	return usb_serial_generic_open(tty, port);
 }
 
+static void ftdi_close(struct usb_serial_port *port)
+{
+	struct ftdi_private *priv = usb_get_serial_port_data(port);
+	struct tty_struct *tty;
+	unsigned long flags;
+
+	/*
+	 * Stop the defer machinery before tearing down the read URBs.
+	 * Clearing urb_defer_tty first means the timer callback (and a
+	 * racing read completion) sees no tty and will not resubmit;
+	 * cancelling the hrtimer then guarantees it cannot fire after
+	 * usb_serial_generic_close() has killed the read URBs. Doing this
+	 * after the close, as before, left a window where the timer could
+	 * resubmit URBs behind a closing port.
+	 */
+	spin_lock_irqsave(&priv->urb_defer_lock, flags);
+	tty = priv->urb_defer_tty;
+	priv->urb_defer_tty = NULL;
+	priv->urb_defer_timer_active = false;
+	spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
+
+	hrtimer_cancel(&priv->urb_defer_timer);
+
+	usb_serial_generic_close(port);
+
+	if (tty)
+		tty_kref_put(tty);
+}
+
 static void ftdi_dtr_rts(struct usb_serial_port *port, int on)
 {
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
@@ -2519,6 +2680,111 @@ static void ftdi_process_read_urb(struct urb *urb)
 		tty_flip_buffer_push(&port->port);
 }
 
+/*
+ * Read bulk completion handler with URB-defer throttling.
+ *
+ * Behaves like usb_serial_generic_read_bulk_callback() except that,
+ * after processing the URB and marking it free, it throttles the
+ * port and arms urb_defer_timer so the next batch of read URBs is
+ * submitted at most once per urb_defer_timer_ns. This caps the
+ * controller-side DMA pressure created by this device.
+ */
+static void ftdi_read_bulk_callback(struct urb *urb)
+{
+	struct usb_serial_port *port = urb->context;
+	struct ftdi_private *priv = usb_get_serial_port_data(port);
+	unsigned char *data = urb->transfer_buffer;
+	struct tty_struct *tty;
+	bool stopped = false;
+	bool start_timer = false;
+	int status = urb->status;
+	unsigned long flags;
+	int i;
+
+	/*
+	 * Inter-batch defer disabled for this port (urb_defer_timer_ns
+	 * was 0 at probe). Use the generic callback directly so this
+	 * port behaves bit-for-bit like upstream ftdi_sio without this
+	 * patch series applied.
+	 */
+	if (ktime_to_ns(priv->urb_defer_ktime) == 0) {
+		usb_serial_generic_read_bulk_callback(urb);
+		return;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i) {
+		if (urb == port->read_urbs[i])
+			break;
+	}
+
+	dev_dbg(&port->dev, "%s - urb %d, len %d\n", __func__, i,
+		urb->actual_length);
+	switch (status) {
+	case 0:
+		usb_serial_debug_data(&port->dev, __func__,
+				      urb->actual_length, data);
+		port->serial->type->process_read_urb(urb);
+		break;
+	case -ENOENT:
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+		dev_dbg(&port->dev, "%s - urb stopped: %d\n",
+			__func__, status);
+		stopped = true;
+		break;
+	case -EPIPE:
+		dev_err(&port->dev, "%s - urb stopped: %d\n",
+			__func__, status);
+		stopped = true;
+		break;
+	default:
+		dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
+			__func__, status);
+		break;
+	}
+
+	/* Ensure URB processing is observed before we mark the URB free. */
+	smp_mb__before_atomic();
+	set_bit(i, &port->read_urbs_free);
+	/* Ensure the free bit is observed before we read THROTTLED below. */
+	smp_mb__after_atomic();
+
+	if (stopped)
+		return;
+
+	if (test_bit(USB_SERIAL_THROTTLED, &port->flags))
+		return;
+
+	spin_lock_irqsave(&priv->urb_defer_lock, flags);
+	tty = priv->urb_defer_tty;
+	if (tty) {
+		tty_kref_get(tty);
+		if (!priv->urb_defer_timer_active) {
+			priv->urb_defer_timer_active = true;
+			start_timer = true;
+		}
+	}
+	spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
+
+	if (tty) {
+		usb_serial_generic_throttle(tty);
+		if (start_timer)
+			hrtimer_start(&priv->urb_defer_timer,
+				      priv->urb_defer_ktime,
+				      HRTIMER_MODE_REL);
+		tty_kref_put(tty);
+	} else {
+		/*
+		 * No tty is bound to this port (e.g. it was opened as a
+		 * console). The inter-batch defer needs a tty to throttle,
+		 * so resubmit the read URBs immediately rather than stalling
+		 * the read stream.  We are in completion (atomic) context, so
+		 * use the atomic-safe helper.
+		 */
+		ftdi_atomic_submit_read_urbs(port);
+	}
+}
+
 static int ftdi_break_ctl(struct tty_struct *tty, int break_state)
 {
 	struct usb_serial_port *port = tty->driver_data;
@@ -2850,10 +3116,12 @@ static struct usb_serial_driver ftdi_device = {
 	.port_probe =		ftdi_port_probe,
 	.port_remove =		ftdi_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_read_bulk_callback,
 	.prepare_write_buffer =	ftdi_prepare_write_buffer,
 	.tiocmget =		ftdi_tiocmget,
 	.tiocmset =		ftdi_tiocmset,
@@ -2872,6 +3140,10 @@ static struct usb_serial_driver * const serial_drivers[] = {
 };
 module_usb_serial_driver(serial_drivers, id_table_combined);
 
+module_param(urb_defer_timer_ns, ulong, 0644);
+MODULE_PARM_DESC(urb_defer_timer_ns,
+		 "Inter-batch defer between read-URB resubmissions, in nanoseconds. 0 (default) disables the defer. Max 100000000 = 100 ms.");
+
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
-- 
2.43.0

[PATCH 2/6] USB: serial: ftdi_sio: retry transient errors on chip-side control transfers

[Chinna Mopurigari Naveen Kumar Reddy]

usb_control_msg() can return -ETIMEDOUT, -EPIPE or -EPROTO on a
functioning device when the host controller is momentarily unable to
complete the transfer -- typically under heavy USB bus load.  A common
real-world scenario is multiple high-baud (~916 kbps) FTDI channels
sharing a host controller with limited DMA-channel fairness, e.g. the
BCM2835 DWC_OTG controller used on the Raspberry Pi Compute Module 3.

When this happens during a one-shot userspace operation such as a
sysfs write to /sys/bus/usb-serial/devices/ttyUSBx/latency_timer, the
write returns -EIO to userspace and the chip's per-channel latency
timer register is not updated.  A short retry usually succeeds.

Introduce a small helper, ftdi_send_request(), that wraps
usb_control_msg() with up to FTDI_CONTROL_RETRIES attempts on the
documented transient errno values, separated by
FTDI_CONTROL_RETRY_DELAY_MS.  Non-transient errors are returned
immediately as before.  Each retry is logged with dev_warn() so the
underlying bus condition remains visible in dmesg.

Convert write_latency_timer() to use the helper.  Other chip-side
control transfer sites in this driver may benefit from the same
treatment as transient failures are reported there; this patch keeps
the scope to the one site for which userspace-visible failures have
been observed.

Signed-off-by: Chinna Mopurigari Naveen Kumar Reddy <naveen.reddy@ftdichip.com>
---
 drivers/usb/serial/ftdi_sio.c | 48 ++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 8ae887107de7..ac0951ba74a0 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -30,6 +30,7 @@
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/slab.h>
+#include <linux/delay.h>
 #include <linux/tty.h>
 #include <linux/tty_driver.h>
 #include <linux/tty_flip.h>
@@ -1383,10 +1384,46 @@ static int change_speed(struct tty_struct *tty, struct usb_serial_port *port)
 	return rv;
 }
 
+/*
+ * Send a chip-side control request, retrying transient bus errors.
+ *
+ * On a healthy device, usb_control_msg() can still return -ETIMEDOUT,
+ * -EPIPE or -EPROTO when the host controller is under heavy load --
+ * for example multiple high-baud FTDI channels sharing a host
+ * controller with limited DMA-channel fairness. Failing a single
+ * one-shot configuration (e.g. a sysfs latency_timer write) to the
+ * caller as -EIO in that situation is unhelpful: the next attempt
+ * usually succeeds.  Retry a small number of times before giving up.
+ */
+#define FTDI_CONTROL_RETRIES		3
+#define FTDI_CONTROL_RETRY_DELAY_MS	2
+
+static int ftdi_send_request(struct usb_serial_port *port, u8 request,
+			     u8 requesttype, u16 value, u16 index)
+{
+	struct usb_device *udev = port->serial->dev;
+	int attempts;
+	int rv = -EIO;
+
+	for (attempts = 0; attempts < FTDI_CONTROL_RETRIES; ++attempts) {
+		rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				     request, requesttype, value, index,
+				     NULL, 0, WDR_TIMEOUT);
+		if (rv >= 0)
+			return rv;
+		if (rv != -ETIMEDOUT && rv != -EPIPE && rv != -EPROTO)
+			return rv;
+		dev_warn(&port->dev,
+			 "control msg req 0x%02x attempt %d returned %d, retrying\n",
+			 request, attempts + 1, rv);
+		msleep(FTDI_CONTROL_RETRY_DELAY_MS);
+	}
+	return rv;
+}
+
 static int write_latency_timer(struct usb_serial_port *port)
 {
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
-	struct usb_device *udev = port->serial->dev;
 	int rv;
 	int l = priv->latency;
 
@@ -1398,12 +1435,9 @@ static int write_latency_timer(struct usb_serial_port *port)
 
 	dev_dbg(&port->dev, "%s: setting latency timer = %i\n", __func__, l);
 
-	rv = usb_control_msg(udev,
-			     usb_sndctrlpipe(udev, 0),
-			     FTDI_SIO_SET_LATENCY_TIMER_REQUEST,
-			     FTDI_SIO_SET_LATENCY_TIMER_REQUEST_TYPE,
-			     l, priv->channel,
-			     NULL, 0, WDR_TIMEOUT);
+	rv = ftdi_send_request(port, FTDI_SIO_SET_LATENCY_TIMER_REQUEST,
+			       FTDI_SIO_SET_LATENCY_TIMER_REQUEST_TYPE,
+			       l, priv->channel);
 	if (rv < 0)
 		dev_err(&port->dev, "Unable to write latency timer: %i\n", rv);
 	return rv;
-- 
2.43.0

[PATCH 3/6] USB: serial: ftdi_sio: make explicit latency_timer sysfs write authoritative

[Chinna Mopurigari Naveen Kumar Reddy]

write_latency_timer() clamps the value programmed into the FT chip's
per-channel latency_timer register to 1 whenever ASYNC_LOW_LATENCY is
set in priv->flags.  ASYNC_LOW_LATENCY is set by userspace via
TIOCSSERIAL, used by setserial(8), libftdi and certain tcsetattr
paths.  The interaction with the existing sysfs latency_timer
attribute is surprising: once any of those tools has set the flag, a
later write of "16" (or any other value) to
/sys/bus/usb-serial/devices/ttyUSBx/latency_timer is silently
clamped to 1 and never reaches the chip.

The store path is the most explicit way userspace can ask for a
particular latency_timer value; treat it as authoritative.  On an
explicit sysfs write, clear ASYNC_LOW_LATENCY before calling
write_latency_timer() so the requested value is what the chip
register actually receives.  Emit a dev_info() so the override is
visible.

Reads continue to honour ASYNC_LOW_LATENCY (returning "1") so any
userspace that previously inspected the attribute to confirm
low-latency mode keeps working until it does its own explicit write.

Signed-off-by: Chinna Mopurigari Naveen Kumar Reddy <naveen.reddy@ftdichip.com>
---
 drivers/usb/serial/ftdi_sio.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index ac0951ba74a0..c0cda19f074a 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1703,6 +1703,23 @@ static ssize_t latency_timer_store(struct device *dev,
 	if (kstrtou8(valbuf, 10, &v))
 		return -EINVAL;
 
+	/*
+	 * An explicit sysfs write wins over the legacy ASYNC_LOW_LATENCY
+	 * tty-flag override.  Without this, if any userspace tool
+	 * (setserial(8), libftdi, certain tcsetattr paths) had set
+	 * ASYNC_LOW_LATENCY via TIOCSSERIAL, write_latency_timer() would
+	 * silently clamp the chip register to 1 regardless of what was
+	 * written to sysfs.  Clearing the flag here makes sysfs the
+	 * authoritative source so the next chip-side write uses exactly
+	 * the value the caller asked for.
+	 */
+	if (priv->flags & ASYNC_LOW_LATENCY) {
+		dev_info(&port->dev,
+			 "explicit latency_timer=%u clears ASYNC_LOW_LATENCY flag\n",
+			 v);
+		priv->flags &= ~ASYNC_LOW_LATENCY;
+	}
+
 	priv->latency = v;
 	rv = write_latency_timer(port);
 	if (rv < 0)
-- 
2.43.0

[PATCH 4/6] USB: serial: ftdi_sio: add per-port low_latency sysfs attribute

[Chinna Mopurigari Naveen Kumar Reddy]

When the inter-batch read-URB defer added by the previous patch is
enabled globally (urb_defer_timer_ns != 0), every port served by
this driver pays a small read-side latency.  In multi-channel
products it is common for one or more channels to be
latency-critical while others tolerate the defer; today there is
no way to express that on a per-port basis.

Add a new writable boolean sysfs attribute, low_latency, on every
ftdi_sio port:

  /sys/bus/usb-serial/devices/ttyUSBx/low_latency
    0 (default): inter-batch defer applies to this port when the
                 defer is globally enabled.
    1          : defer is bypassed on this port; the generic
                 read-bulk callback handles URB completions
                 directly with no throttle / hrtimer interaction.

The implementation adds a bool low_latency field to struct
ftdi_private (zero-initialised by kzalloc), settable via the new
attribute.  ftdi_read_bulk_callback() short-circuits to
usb_serial_generic_read_bulk_callback() at the top of the function
when the flag is set, skipping the throttle/hrtimer path entirely.

Writing 1 also cancels any in-flight defer hrtimer and clears the
timer_active bookkeeping so the next URB completion takes the
direct path immediately.

A udev rule typically sets the flag at device-add for the
latency-critical port; other ports are unaffected.

Signed-off-by: Chinna Mopurigari Naveen Kumar Reddy <naveen.reddy@ftdichip.com>
---
 drivers/usb/serial/ftdi_sio.c | 66 +++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index c0cda19f074a..8faa073f1383 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -110,6 +110,15 @@ struct ftdi_private {
 	struct hrtimer		urb_defer_timer;
 	struct tty_struct	*urb_defer_tty;
 	bool			urb_defer_timer_active;
+	/*
+	 * Per-port opt-out of the inter-batch defer.  When true, set via
+	 * /sys/bus/usb-serial/devices/ttyUSBx/low_latency, the read-bulk
+	 * callback short-circuits to the generic upstream callback and
+	 * skips the hrtimer-based throttle.  Defaults to false: the
+	 * defer applies to every port unless this port is explicitly
+	 * opted out (and the defer is globally enabled to begin with).
+	 */
+	bool			low_latency;
 #ifdef CONFIG_GPIOLIB
 	struct gpio_chip gc;
 	struct mutex gpio_lock;	/* protects GPIO state */
@@ -1728,6 +1737,51 @@ static ssize_t latency_timer_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(latency_timer);
 
+/*
+ * /sys/bus/usb-serial/devices/ttyUSBx/low_latency
+ *   0 (default): inter-batch defer applies to this port (only
+ *                meaningful when the defer is globally enabled
+ *                via urb_defer_timer_ns).
+ *   1          : defer bypassed on this port; the upstream
+ *                read-bulk callback handles URB completions with
+ *                no throttle / hrtimer interaction.
+ * Typically written by a udev rule on the latency-critical port at
+ * device-add. Writing 1 also cancels any in-flight defer timer so
+ * the next URB submits immediately.
+ */
+static ssize_t low_latency_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct usb_serial_port *port = to_usb_serial_port(dev);
+	struct ftdi_private *priv = usb_get_serial_port_data(port);
+
+	return sprintf(buf, "%u\n", priv->low_latency ? 1 : 0);
+}
+
+static ssize_t low_latency_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *valbuf, size_t count)
+{
+	struct usb_serial_port *port = to_usb_serial_port(dev);
+	struct ftdi_private *priv = usb_get_serial_port_data(port);
+	unsigned long flags;
+	u8 v;
+
+	if (kstrtou8(valbuf, 10, &v))
+		return -EINVAL;
+
+	priv->low_latency = !!v;
+
+	if (priv->low_latency) {
+		hrtimer_cancel(&priv->urb_defer_timer);
+		spin_lock_irqsave(&priv->urb_defer_lock, flags);
+		priv->urb_defer_timer_active = false;
+		spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
+	}
+	return count;
+}
+static DEVICE_ATTR_RW(low_latency);
+
 /* Write an event character directly to the FTDI register.  The ASCII
    value is in the low 8 bits, with the enable bit in the 9th bit. */
 static ssize_t event_char_store(struct device *dev,
@@ -1762,6 +1816,7 @@ static DEVICE_ATTR_WO(event_char);
 static struct attribute *ftdi_attrs[] = {
 	&dev_attr_event_char.attr,
 	&dev_attr_latency_timer.attr,
+	&dev_attr_low_latency.attr,
 	NULL
 };
 
@@ -2763,6 +2818,17 @@ static void ftdi_read_bulk_callback(struct urb *urb)
 		return;
 	}
 
+	/*
+	 * Latency-critical ports opt out of the inter-batch defer via
+	 * /sys/.../low_latency. Delegate straight to the generic
+	 * callback so the data stream sees no added latency on this
+	 * port even when the defer is globally enabled.
+	 */
+	if (priv->low_latency) {
+		usb_serial_generic_read_bulk_callback(urb);
+		return;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i) {
 		if (urb == port->read_urbs[i])
 			break;
-- 
2.43.0

[PATCH 5/6] USB: serial: ftdi_sio: serialise low_latency toggle against read_bulk_callback

[Chinna Mopurigari Naveen Kumar Reddy]

Toggling the per-port low_latency sysfs attribute while the port is
actively reading or writing was found to occasionally leave the port
wedged.  The application would block on read() until restarted.

The race is between low_latency_store() and ftdi_read_bulk_callback():

  1. ftdi_read_bulk_callback() passes the top-of-function fast-path
     low_latency check (sees false) and proceeds towards the
     throttle/start-timer block.
  2. low_latency_store() runs on another CPU and sets the flag.
     Previously it only cancelled the hrtimer and cleared
     timer_active -- it did not clear USB_SERIAL_THROTTLED.
  3. ftdi_read_bulk_callback() continues past the lock, calls
     usb_serial_generic_throttle() which sets USB_SERIAL_THROTTLED,
     and arms the (now-cancelled-and-restarted) hrtimer.
  4. Every subsequent URB completion now takes the low_latency fast
     path at the top of the callback, delegates to the upstream
     generic callback which marks the URB free, sees
     USB_SERIAL_THROTTLED set, and refuses to resubmit.  The data
     stream stalls.

Close the race:

  a) low_latency_store() now publishes the new value under
     urb_defer_lock so the callback's locked re-check observes it
     with proper ordering, then cancels the hrtimer synchronously,
     then calls usb_serial_generic_unthrottle() to clear
     USB_SERIAL_THROTTLED and resubmit any free URBs.  This is done
     in both directions of the toggle -- when nothing is parked it
     is a cheap no-op submit.

  b) ftdi_read_bulk_callback() re-checks low_latency under the same
     urb_defer_lock immediately before the throttle/start-timer
     block.  On bail it calls usb_serial_generic_submit_read_urbs()
     to resubmit the URB itself, closing the brief window where the
     URB is marked free but neither the store nor anyone else has
     resubmitted.

Signed-off-by: Chinna Mopurigari Naveen Kumar Reddy <naveen.reddy@ftdichip.com>
---
 drivers/usb/serial/ftdi_sio.c | 57 ++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 8faa073f1383..c631c6d0a1a5 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1764,20 +1764,54 @@ static ssize_t low_latency_store(struct device *dev,
 {
 	struct usb_serial_port *port = to_usb_serial_port(dev);
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
+	struct tty_struct *tty;
 	unsigned long flags;
 	u8 v;
 
 	if (kstrtou8(valbuf, 10, &v))
 		return -EINVAL;
 
+	/*
+	 * Toggling low_latency while the port is actively reading or
+	 * writing is racy unless we serialise carefully.  Between the
+	 * moment ftdi_read_bulk_callback() passes its top-of-function
+	 * low_latency check and the moment it sets USB_SERIAL_THROTTLED
+	 * + starts the hrtimer, this store may set low_latency = true.
+	 * If we then merely cancelled the timer (without clearing
+	 * USB_SERIAL_THROTTLED), every subsequent URB completion would
+	 * take the generic callback path, mark the URB free, observe
+	 * the throttled bit, refuse to resubmit, and silently stall
+	 * the data stream -- only an application restart would recover.
+	 *
+	 * Three things, in order, close the race:
+	 *   1. Publish low_latency under urb_defer_lock so the
+	 *      callback's re-check (in the throttle block below)
+	 *      observes the new value with proper ordering.
+	 *   2. Cancel the defer hrtimer synchronously so no late
+	 *      timer callback can race the unthrottle.
+	 *   3. Force-unthrottle the port and resubmit any free URBs.
+	 *      Done in both directions of the toggle -- when nothing is
+	 *      parked this is a cheap no-op submit; it eliminates the
+	 *      wedge case either way.
+	 */
+	spin_lock_irqsave(&priv->urb_defer_lock, flags);
 	priv->low_latency = !!v;
+	tty = priv->urb_defer_tty;
+	if (tty)
+		tty_kref_get(tty);
+	spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
 
-	if (priv->low_latency) {
-		hrtimer_cancel(&priv->urb_defer_timer);
-		spin_lock_irqsave(&priv->urb_defer_lock, flags);
-		priv->urb_defer_timer_active = false;
-		spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
+	hrtimer_cancel(&priv->urb_defer_timer);
+
+	spin_lock_irqsave(&priv->urb_defer_lock, flags);
+	priv->urb_defer_timer_active = false;
+	spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
+
+	if (tty) {
+		usb_serial_generic_unthrottle(tty);
+		tty_kref_put(tty);
 	}
+
 	return count;
 }
 static DEVICE_ATTR_RW(low_latency);
@@ -2873,6 +2907,19 @@ static void ftdi_read_bulk_callback(struct urb *urb)
 		return;
 
 	spin_lock_irqsave(&priv->urb_defer_lock, flags);
+	/*
+	 * Re-check low_latency under the lock.  Pairs with the locked
+	 * publish in low_latency_store().  If the toggle landed
+	 * between the top-of-function fast-path check and here, do
+	 * not throttle or arm the timer; resubmit the URB ourselves
+	 * so the data stream does not stall in the narrow window
+	 * before the store's unthrottle runs.
+	 */
+	if (priv->low_latency) {
+		spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
+		ftdi_atomic_submit_read_urbs(port);
+		return;
+	}
 	tty = priv->urb_defer_tty;
 	if (tty) {
 		tty_kref_get(tty);
-- 
2.43.0

[PATCH 6/6] USB: serial: ftdi_sio: pace low_latency ports with low_latency_defer_ns

[Chinna Mopurigari Naveen Kumar Reddy]

The low_latency attribute makes a port bypass the inter-batch read
defer entirely: read URBs are resubmitted immediately, giving the
lowest read latency but keeping a host-controller DMA channel occupied
almost continuously.  On controllers that do not enforce DMA-channel
fairness (such as the BCM2835 DWC_OTG on the Raspberry Pi Compute
Module 3) only about two such ports can run at high baud before
control transfers and other devices on the same bus -- a USB-attached
Ethernet adapter, for instance -- are starved again.

Add a module parameter, low_latency_defer_ns, that applies a small
inter-batch defer to low_latency ports instead of bypassing the defer
entirely.  Default 0 preserves the existing full-bypass behaviour.  A
small non-zero value (e.g. 2000000 = 2 ms) paces each low_latency port
with that interval, roughly halving how long it occupies a DMA
channel, so more low_latency ports can coexist without starving
control transfers, at the cost of a bounded read latency (~the value,
well under the urb_defer_timer_ns default).

The value is read on each completion, so runtime changes via
/sys/module/ftdi_sio/parameters/low_latency_defer_ns take effect
immediately on every low_latency port; it is clamped to
FTDI_URB_DEFER_MAX_NS.  As a rough guide, on the CM3/DWC_OTG with
eight 916 kbps channels about 1 ms per low_latency port beyond two
keeps control-transfer latency and unrelated USB throughput bounded;
the optimum is workload-dependent and best confirmed by measurement.

Signed-off-by: Chinna Mopurigari Naveen Kumar Reddy <naveen.reddy@ftdichip.com>
---
 drivers/usb/serial/ftdi_sio.c | 50 ++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index c631c6d0a1a5..7d7644ecab06 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -2342,6 +2342,26 @@ static void ftdi_gpio_remove(struct usb_serial_port *port) { }
  */
 static unsigned long urb_defer_timer_ns;
 
+/*
+ * Optional small inter-batch defer applied to low_latency ports, in
+ * nanoseconds.
+ *
+ * Default 0 keeps low_latency ports fully bypassed (immediate read-URB
+ * resubmit, lowest latency), which pins a host-controller DMA channel
+ * almost continuously.  On a controller that does not enforce
+ * DMA-channel fairness only about two such ports can run at high baud
+ * before control transfers and other devices on the bus are starved.
+ *
+ * A small non-zero value (e.g. 2000000 = 2 ms) instead paces each
+ * low_latency port with that interval, roughly halving its
+ * DMA-channel occupancy so more low_latency ports can coexist, at the
+ * cost of a bounded read latency (~the value, still well under the
+ * urb_defer_timer_ns default).  Read live, so runtime changes apply
+ * immediately to every low_latency port; clamped to
+ * FTDI_URB_DEFER_MAX_NS.
+ */
+static unsigned long low_latency_defer_ns;
+
 /*
  * hrtimer callback that fires after the configured defer interval and
  * un-throttles the port so the generic submit-read-urbs path can run
@@ -2854,11 +2874,13 @@ static void ftdi_read_bulk_callback(struct urb *urb)
 
 	/*
 	 * Latency-critical ports opt out of the inter-batch defer via
-	 * /sys/.../low_latency. Delegate straight to the generic
-	 * callback so the data stream sees no added latency on this
-	 * port even when the defer is globally enabled.
+	 * /sys/.../low_latency.  With low_latency_defer_ns == 0 (default),
+	 * delegate straight to the generic callback so the data stream
+	 * sees no added latency on this port.  A non-zero
+	 * low_latency_defer_ns instead paces this port with that (small)
+	 * interval to bound its DMA-channel occupancy.
 	 */
-	if (priv->low_latency) {
+	if (priv->low_latency && !low_latency_defer_ns) {
 		usb_serial_generic_read_bulk_callback(urb);
 		return;
 	}
@@ -2915,7 +2937,7 @@ static void ftdi_read_bulk_callback(struct urb *urb)
 	 * so the data stream does not stall in the narrow window
 	 * before the store's unthrottle runs.
 	 */
-	if (priv->low_latency) {
+	if (priv->low_latency && !low_latency_defer_ns) {
 		spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
 		ftdi_atomic_submit_read_urbs(port);
 		return;
@@ -2931,10 +2953,21 @@ static void ftdi_read_bulk_callback(struct urb *urb)
 	spin_unlock_irqrestore(&priv->urb_defer_lock, flags);
 
 	if (tty) {
+		ktime_t defer_kt = priv->urb_defer_ktime;
+
+		/*
+		 * A low_latency port that is being paced (low_latency_defer_ns
+		 * != 0) uses that small per-port interval instead of the
+		 * global inter-batch defer.
+		 */
+		if (priv->low_latency)
+			defer_kt = ns_to_ktime(min_t(unsigned long,
+						     low_latency_defer_ns,
+						     FTDI_URB_DEFER_MAX_NS));
+
 		usb_serial_generic_throttle(tty);
 		if (start_timer)
-			hrtimer_start(&priv->urb_defer_timer,
-				      priv->urb_defer_ktime,
+			hrtimer_start(&priv->urb_defer_timer, defer_kt,
 				      HRTIMER_MODE_REL);
 		tty_kref_put(tty);
 	} else {
@@ -3307,6 +3340,9 @@ module_usb_serial_driver(serial_drivers, id_table_combined);
 module_param(urb_defer_timer_ns, ulong, 0644);
 MODULE_PARM_DESC(urb_defer_timer_ns,
 		 "Inter-batch defer between read-URB resubmissions, in nanoseconds. 0 (default) disables the defer. Max 100000000 = 100 ms.");
+module_param(low_latency_defer_ns, ulong, 0644);
+MODULE_PARM_DESC(low_latency_defer_ns,
+		 "Inter-batch defer applied to low_latency ports, in nanoseconds. 0 (default) bypasses the defer entirely on low_latency ports; a small value (e.g. 2000000 = 2 ms) paces them so more can run without starving control transfers. Max 100000000 = 100 ms.");
 
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
-- 
2.43.0

Re: [PATCH 1/6] USB: serial: ftdi_sio: add configurable inter-batch defer for read URBs

[Greg Kroah-Hartman]

On Mon, Jun 22, 2026 at 03:38:34PM +0800, Chinna Mopurigari Naveen Kumar Reddy wrote:
> +module_param(urb_defer_timer_ns, ulong, 0644);
> +MODULE_PARM_DESC(urb_defer_timer_ns,
> +		 "Inter-batch defer between read-URB resubmissions, in nanoseconds. 0 (default) disables the defer. Max 100000000 = 100 ms.");

This is not the 1990's, module parameters should not be used anymore as
they are not scalable or configurable or very well handled at all.

Please use a better abi if you really need this, BUT, I feel this is
totally the wrong place to do this.  You are assuming a single driver is
the source of the problem, when the real problem is the host controller
driver for this USB host.  Are you going to go and make this type of
change to every individual USB driver instead to paper over the
broken-ness of that USB host controller?

Why not just fix this at the root of the problem, that way you don't
have to modify all individual drivers at all?

thanks,

greg k-h

Re: [PATCH 1/6] USB: serial: ftdi_sio: add configurable inter-batch defer for read URBs

[Greg Kroah-Hartman]

On Mon, Jun 22, 2026 at 03:38:34PM +0800, Chinna Mopurigari Naveen Kumar Reddy wrote:
> +		if (res) {
> +			dev_dbg(&port->dev, "%s - submit failed: %d\n",
> +				__func__, res);

Minor nit, dev_dbg() already has the __func__ information in it, never
duplicate it again as that's just waste.

thanks,

greg k-h

RE: [PATCH 1/6] USB: serial: ftdi_sio: add configurable inter-batch defer for read URBs

[Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)]

Hi Greg,

On Mon, Jun 22, 2026 at 04:52:00PM +0000, Greg Kroah-Hartman wrote:
> This is not the 1990's, module parameters should not be used anymore...
> Why not just fix this at the root of the problem?

Thank you for the review, Greg.

You are right on both points.

On the module parameter: I will switch to sysfs if the per-driver approach
is accepted, or drop it entirely.

On the location: your core point is well-taken. The DMA channel starvation
is a BCM2835 DWC_OTG host controller deficiency, and the correct fix
belongs in the host controller driver, not in each USB device driver.
I will investigate fixing it there instead. If that is not feasible (the
BCM2835 DWC_OTG is a vendor tree that Broadcom/RPi maintain outside
mainline), I will report back before proposing any per-driver workaround.

I will drop patch 1 from the series for now and resubmit patches 2-6
independently, as those address ftdi_sio-specific issues (latency_timer
reliability, low_latency race, per-port sysfs attribute) that stand on
their own merits.

On the dev_dbg() nit: understood, I will remove __func__ in v2.

Thanks,
Naveen

-----Original Message-----
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Sent: Monday, 22 June 2026 4:52 pm
To: Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG) <naveen.reddy@ftdichip.com>
Cc: Johan Hovold <johan@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Arun Pappan (FTDI-SG) <arun.pappan@ftdichip.com>
Subject: Re: [PATCH 1/6] USB: serial: ftdi_sio: add configurable inter-batch defer for read URBs

On Mon, Jun 22, 2026 at 03:38:34PM +0800, Chinna Mopurigari Naveen Kumar Reddy wrote:
> +		if (res) {
> +			dev_dbg(&port->dev, "%s - submit failed: %d\n",
> +				__func__, res);

Minor nit, dev_dbg() already has the __func__ information in it, never duplicate it again as that's just waste.

thanks,

greg k-h

Re: [PATCH 1/6] USB: serial: ftdi_sio: add configurable inter-batch defer for read URBs

[Greg Kroah-Hartman]

On Mon, Jun 22, 2026 at 09:05:53AM +0000, Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG) wrote:
> Hi Greg,
> 
> On Mon, Jun 22, 2026 at 04:52:00PM +0000, Greg Kroah-Hartman wrote:
> > This is not the 1990's, module parameters should not be used anymore...
> > Why not just fix this at the root of the problem?
> 
> Thank you for the review, Greg.
> 
> You are right on both points.
> 
> On the module parameter: I will switch to sysfs if the per-driver approach
> is accepted, or drop it entirely.
> 
> On the location: your core point is well-taken. The DMA channel starvation
> is a BCM2835 DWC_OTG host controller deficiency, and the correct fix
> belongs in the host controller driver, not in each USB device driver.
> I will investigate fixing it there instead. If that is not feasible (the
> BCM2835 DWC_OTG is a vendor tree that Broadcom/RPi maintain outside
> mainline), I will report back before proposing any per-driver workaround.

As this is an out-of-tree issue, there's nothing we can do in our
in-tree drivers for that.  Please work with that team to either get
their code merged properly upstream with us, where we can all work on
the issue together, or just work with them out-of-tree.

thanks,

greg k-h

RE: [PATCH 1/6] USB: serial: ftdi_sio: add configurable inter-batch defer for read URBs

[Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)]

Hi Greg,

On Mon, Jun 22, 2026 at 05:28:00PM +0000, Greg Kroah-Hartman wrote:
> As this is an out-of-tree issue, there's nothing we can do in our
> in-tree drivers for that.

Understood, thanks Greg. I will drop the urb_defer_timer_ns patch and
the low_latency patches that depended on it, and take the host
controller starvation issue to the Raspberry Pi / BCM2835 USB folks
separately.

I will shortly post a v2 with just the two standalone ftdi_sio fixes
(transient control-transfer retry, and making an explicit latency_timer
sysfs write authoritative), which are independent of the host
controller issue.

Thanks,
Naveen

-----Original Message-----
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Sent: Monday, 22 June 2026 5:28 pm
To: Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG) <naveen.reddy@ftdichip.com>
Cc: Johan Hovold <johan@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Arun Pappan (FTDI-SG) <arun.pappan@ftdichip.com>
Subject: Re: [PATCH 1/6] USB: serial: ftdi_sio: add configurable inter-batch defer for read URBs

On Mon, Jun 22, 2026 at 09:05:53AM +0000, Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG) wrote:
> Hi Greg,
> 
> On Mon, Jun 22, 2026 at 04:52:00PM +0000, Greg Kroah-Hartman wrote:
> > This is not the 1990's, module parameters should not be used anymore...
> > Why not just fix this at the root of the problem?
> 
> Thank you for the review, Greg.
> 
> You are right on both points.
> 
> On the module parameter: I will switch to sysfs if the per-driver 
> approach is accepted, or drop it entirely.
> 
> On the location: your core point is well-taken. The DMA channel 
> starvation is a BCM2835 DWC_OTG host controller deficiency, and the 
> correct fix belongs in the host controller driver, not in each USB device driver.
> I will investigate fixing it there instead. If that is not feasible 
> (the
> BCM2835 DWC_OTG is a vendor tree that Broadcom/RPi maintain outside 
> mainline), I will report back before proposing any per-driver workaround.

As this is an out-of-tree issue, there's nothing we can do in our in-tree drivers for that.  Please work with that team to either get their code merged properly upstream with us, where we can all work on the issue together, or just work with them out-of-tree.

thanks,

greg k-h

[PATCH v2 0/2] USB: serial: ftdi_sio: latency_timer reliability fixes

[Chinna Mopurigari Naveen Kumar Reddy]

v2: Drop patches 1, 4, 5, 6 from v1 per Greg Kroah-Hartman's review.

Patches 1 and 6 used module parameters (urb_defer_timer_ns and
low_latency_defer_ns) to address DMA-channel starvation on a host
controller that does not enforce DMA-channel fairness.  Greg correctly
pointed out that the host controller driver is the right place to fix
that, and that ftdi_sio should not carry per-driver workarounds for an
out-of-tree host controller deficiency.  Those patches are dropped.

Patches 4 and 5 (per-port low_latency sysfs attribute and its
serialisation fix) depended on the hrtimer infrastructure introduced
by the dropped patch 1 and have no standalone purpose without it.
They are dropped too.

This v2 contains only the two patches that are independent correctness
fixes for ftdi_sio itself:

  1. Retry transient errors (-ETIMEDOUT / -EPIPE / -EPROTO) on
     chip-side control transfers so that a single transient USB hiccup
     under bus load does not fail an otherwise-healthy one-shot
     configuration (e.g. a latency_timer sysfs write) back to
     userspace as -EIO.

  2. Make an explicit sysfs write to latency_timer authoritative so
     that a userspace write is not silently clamped to 1 by the
     ASYNC_LOW_LATENCY tty flag left set by an earlier TIOCSSERIAL
     (setserial(8), libftdi, certain tcsetattr paths).

Chinna Mopurigari Naveen Kumar Reddy (2):
  USB: serial: ftdi_sio: retry transient errors on chip-side control
    transfers
  USB: serial: ftdi_sio: make explicit latency_timer sysfs write
    authoritative

 drivers/usb/serial/ftdi_sio.c | 65 +++++++++++++++++++++++++++++++----
 1 file changed, 58 insertions(+), 7 deletions(-)


base-commit: ba3e43a9e601636f5edb54e259a74f96ca3b8fd8
-- 
2.43.0

[PATCH v2 1/2] USB: serial: ftdi_sio: retry transient errors on chip-side control transfers

[Chinna Mopurigari Naveen Kumar Reddy]

usb_control_msg() can return -ETIMEDOUT, -EPIPE or -EPROTO on a
functioning device when the host controller is momentarily unable to
complete the transfer.  These are transient conditions that surface
under heavy USB bus load -- for example when several high-baud FTDI
channels share a busy host controller -- and a short retry generally
succeeds.

When such an error happens during a one-shot userspace operation such
as a sysfs write to /sys/bus/usb-serial/devices/ttyUSBx/latency_timer,
the write fails back to userspace with -EIO and the chip's per-channel
latency timer register is left unchanged, even though the device is
healthy and the next attempt would have worked.

Introduce a small helper, ftdi_send_request(), that wraps
usb_control_msg() with up to FTDI_CONTROL_RETRIES attempts on these
documented transient errno values, separated by
FTDI_CONTROL_RETRY_DELAY_MS.  Non-transient errors are returned
immediately, as before.  Each retry is logged with dev_warn() so the
underlying bus condition stays visible in dmesg.

Convert write_latency_timer() to use the helper.  Only this site is
converted, as it is the one where a transient failure has a directly
userspace-visible effect; other chip-side control transfers are left
unchanged.

Signed-off-by: Chinna Mopurigari Naveen Kumar Reddy <naveen.reddy@ftdichip.com>
---
 drivers/usb/serial/ftdi_sio.c | 48 ++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index af14548fa03d..7aaa7fc1be71 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -30,6 +30,7 @@
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/slab.h>
+#include <linux/delay.h>
 #include <linux/tty.h>
 #include <linux/tty_driver.h>
 #include <linux/tty_flip.h>
@@ -1363,10 +1364,46 @@ static int change_speed(struct tty_struct *tty, struct usb_serial_port *port)
 	return rv;
 }
 
+/*
+ * Send a chip-side control request, retrying transient bus errors.
+ *
+ * On a healthy device, usb_control_msg() can still return -ETIMEDOUT,
+ * -EPIPE or -EPROTO when the host controller is under heavy load --
+ * for example multiple high-baud FTDI channels sharing a host
+ * controller with limited DMA-channel fairness. Failing a single
+ * one-shot configuration (e.g. a sysfs latency_timer write) to the
+ * caller as -EIO in that situation is unhelpful: the next attempt
+ * usually succeeds.  Retry a small number of times before giving up.
+ */
+#define FTDI_CONTROL_RETRIES		3
+#define FTDI_CONTROL_RETRY_DELAY_MS	2
+
+static int ftdi_send_request(struct usb_serial_port *port, u8 request,
+			     u8 requesttype, u16 value, u16 index)
+{
+	struct usb_device *udev = port->serial->dev;
+	int attempts;
+	int rv = -EIO;
+
+	for (attempts = 0; attempts < FTDI_CONTROL_RETRIES; ++attempts) {
+		rv = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				     request, requesttype, value, index,
+				     NULL, 0, WDR_TIMEOUT);
+		if (rv >= 0)
+			return rv;
+		if (rv != -ETIMEDOUT && rv != -EPIPE && rv != -EPROTO)
+			return rv;
+		dev_warn(&port->dev,
+			 "control msg req 0x%02x attempt %d returned %d, retrying\n",
+			 request, attempts + 1, rv);
+		msleep(FTDI_CONTROL_RETRY_DELAY_MS);
+	}
+	return rv;
+}
+
 static int write_latency_timer(struct usb_serial_port *port)
 {
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
-	struct usb_device *udev = port->serial->dev;
 	int rv;
 	int l = priv->latency;
 
@@ -1378,12 +1415,9 @@ static int write_latency_timer(struct usb_serial_port *port)
 
 	dev_dbg(&port->dev, "%s: setting latency timer = %i\n", __func__, l);
 
-	rv = usb_control_msg(udev,
-			     usb_sndctrlpipe(udev, 0),
-			     FTDI_SIO_SET_LATENCY_TIMER_REQUEST,
-			     FTDI_SIO_SET_LATENCY_TIMER_REQUEST_TYPE,
-			     l, priv->channel,
-			     NULL, 0, WDR_TIMEOUT);
+	rv = ftdi_send_request(port, FTDI_SIO_SET_LATENCY_TIMER_REQUEST,
+			       FTDI_SIO_SET_LATENCY_TIMER_REQUEST_TYPE,
+			       l, priv->channel);
 	if (rv < 0)
 		dev_err(&port->dev, "Unable to write latency timer: %i\n", rv);
 	return rv;
-- 
2.43.0

[PATCH v2 2/2] USB: serial: ftdi_sio: make explicit latency_timer sysfs write authoritative

[Chinna Mopurigari Naveen Kumar Reddy]

write_latency_timer() clamps the value programmed into the FT chip's
per-channel latency_timer register to 1 whenever ASYNC_LOW_LATENCY is
set in priv->flags.  ASYNC_LOW_LATENCY is set by userspace via
TIOCSSERIAL, used by setserial(8), libftdi and certain tcsetattr
paths.  The interaction with the existing sysfs latency_timer
attribute is surprising: once any of those tools has set the flag, a
later write of "16" (or any other value) to
/sys/bus/usb-serial/devices/ttyUSBx/latency_timer is silently
clamped to 1 and never reaches the chip.

The store path is the most explicit way userspace can ask for a
particular latency_timer value; treat it as authoritative.  On an
explicit sysfs write, clear ASYNC_LOW_LATENCY before calling
write_latency_timer() so the requested value is what the chip
register actually receives.  Emit a dev_info() so the override is
visible.

Reads continue to honour ASYNC_LOW_LATENCY (returning "1") so any
userspace that previously inspected the attribute to confirm
low-latency mode keeps working until it does its own explicit write.

Signed-off-by: Chinna Mopurigari Naveen Kumar Reddy <naveen.reddy@ftdichip.com>
---
 drivers/usb/serial/ftdi_sio.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7aaa7fc1be71..e7f13eca7ae6 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1683,6 +1683,23 @@ static ssize_t latency_timer_store(struct device *dev,
 	if (kstrtou8(valbuf, 10, &v))
 		return -EINVAL;
 
+	/*
+	 * An explicit sysfs write wins over the legacy ASYNC_LOW_LATENCY
+	 * tty-flag override.  Without this, if any userspace tool
+	 * (setserial(8), libftdi, certain tcsetattr paths) had set
+	 * ASYNC_LOW_LATENCY via TIOCSSERIAL, write_latency_timer() would
+	 * silently clamp the chip register to 1 regardless of what was
+	 * written to sysfs.  Clearing the flag here makes sysfs the
+	 * authoritative source so the next chip-side write uses exactly
+	 * the value the caller asked for.
+	 */
+	if (priv->flags & ASYNC_LOW_LATENCY) {
+		dev_info(&port->dev,
+			 "explicit latency_timer=%u clears ASYNC_LOW_LATENCY flag\n",
+			 v);
+		priv->flags &= ~ASYNC_LOW_LATENCY;
+	}
+
 	priv->latency = v;
 	rv = write_latency_timer(port);
 	if (rv < 0)
-- 
2.43.0

Re: [PATCH v2 2/2] USB: serial: ftdi_sio: make explicit latency_timer sysfs write authoritative

[Greg Kroah-Hartman]

On Mon, Jun 22, 2026 at 05:43:40PM +0800, Chinna Mopurigari Naveen Kumar Reddy wrote:
> write_latency_timer() clamps the value programmed into the FT chip's
> per-channel latency_timer register to 1 whenever ASYNC_LOW_LATENCY is
> set in priv->flags.  ASYNC_LOW_LATENCY is set by userspace via
> TIOCSSERIAL, used by setserial(8), libftdi and certain tcsetattr
> paths.  The interaction with the existing sysfs latency_timer
> attribute is surprising: once any of those tools has set the flag, a
> later write of "16" (or any other value) to
> /sys/bus/usb-serial/devices/ttyUSBx/latency_timer is silently
> clamped to 1 and never reaches the chip.
> 
> The store path is the most explicit way userspace can ask for a
> particular latency_timer value; treat it as authoritative.  On an
> explicit sysfs write, clear ASYNC_LOW_LATENCY before calling
> write_latency_timer() so the requested value is what the chip
> register actually receives.  Emit a dev_info() so the override is
> visible.
> 
> Reads continue to honour ASYNC_LOW_LATENCY (returning "1") so any
> userspace that previously inspected the attribute to confirm
> low-latency mode keeps working until it does its own explicit write.
> 
> Signed-off-by: Chinna Mopurigari Naveen Kumar Reddy <naveen.reddy@ftdichip.com>
> ---
>  drivers/usb/serial/ftdi_sio.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 7aaa7fc1be71..e7f13eca7ae6 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -1683,6 +1683,23 @@ static ssize_t latency_timer_store(struct device *dev,
>  	if (kstrtou8(valbuf, 10, &v))
>  		return -EINVAL;
>  
> +	/*
> +	 * An explicit sysfs write wins over the legacy ASYNC_LOW_LATENCY
> +	 * tty-flag override.  Without this, if any userspace tool
> +	 * (setserial(8), libftdi, certain tcsetattr paths) had set
> +	 * ASYNC_LOW_LATENCY via TIOCSSERIAL, write_latency_timer() would
> +	 * silently clamp the chip register to 1 regardless of what was
> +	 * written to sysfs.  Clearing the flag here makes sysfs the
> +	 * authoritative source so the next chip-side write uses exactly
> +	 * the value the caller asked for.
> +	 */
> +	if (priv->flags & ASYNC_LOW_LATENCY) {
> +		dev_info(&port->dev,
> +			 "explicit latency_timer=%u clears ASYNC_LOW_LATENCY flag\n",
> +			 v);

When drivers work properly, they are quiet.  Make this a debugging
message at the most please.

thanks,

greg k-h