USB: serial: ftdi_sio: configurable read-URB defer, per-port low_latency, latency_timer reliability

PATCH In review Chinna Mopurigari Naveen Kumar Reddy 3 participants 25 messages Jun 22 – Jul 22

Subsystems (2)

USB USB serial

lore

Share & tools

lore

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-22 07:39 UTC #0 lore raw

cover USB: serial: ftdi_sio: configurable read-URB defer, per-port low_latency, latency_timer reliability

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

Signature · 1 line

2.43.0

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-22 07:39 UTC #1 lore raw

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

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 <[email protected]>

---
 drivers/usb/serial/ftdi_sio.c | 274 +++++++++++++++++++++++++++++++++-
 1 file changed, 273 insertions(+), 1 deletion(-)

drivers/usb/serial/ftdi_sio.c +273 -1

index af14548fa03d..8ae887107de7 100644
@@ -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 <[email protected]>, Bill Ryder <[email protected]>, Kuba Ober <[email protected]>, Andreas Mohr, Johan Hovold <[email protected]>"
 #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");

Signature · 1 line

2.43.0

Greg Kroah-Hartman

maintainer

2026-06-22 08:51 UTC #2 lore raw

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

drivers/usb/serial/ftdi_sio.c static struct usb_serial_driver * const serial_drivers[] = { · line 3145

Quoted 3 lines · Chinna Mopurigari Naveen Kumar Reddy

drivers/usb/serial/ftdi_sio.c +3 -0

+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

Greg Kroah-Hartman

maintainer

2026-06-22 08:52 UTC #3 lore raw

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

In reply to Chinna Mopurigari Naveen Kumar Reddy

drivers/usb/serial/ftdi_sio.c static void ftdi_gpio_remove(struct usb_serial_port *port) { } · line 2236

Quoted 3 lines · Chinna Mopurigari Naveen Kumar Reddy

drivers/usb/serial/ftdi_sio.c +3 -0

+		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

Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)

author

2026-06-22 09:05 UTC #4 lore raw

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

Hi Greg,

Quoted 2 lines · Greg Kroah-Hartman

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 <[email protected]>
Sent: Monday, 22 June 2026 4:52 pm
To: Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG) <[email protected]>
Cc: Johan Hovold <[email protected]>; [email protected]; [email protected]; Arun Pappan (FTDI-SG) <[email protected]>
Subject: Re: [PATCH 1/6] USB: serial: ftdi_sio: add configurable inter-batch defer for read URBs

drivers/usb/serial/ftdi_sio.c static void ftdi_gpio_remove(struct usb_serial_port *port) { } · line 2236

Quoted 3 lines · Chinna Mopurigari Naveen Kumar Reddy

drivers/usb/serial/ftdi_sio.c +3 -0

+		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

Greg Kroah-Hartman

maintainer

2026-06-22 09:27 UTC #5 lore raw

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

Quoted 13 lines · Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)

Hi Greg,
Quoted 2 lines · Greg Kroah-Hartman
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.

Quoted 6 lines

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

Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)

author

2026-06-22 09:43 UTC #6 lore raw

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

Hi Greg,

Quoted 2 lines · Greg Kroah-Hartman

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 <[email protected]>
Sent: Monday, 22 June 2026 5:28 pm
To: Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG) <[email protected]>
Cc: Johan Hovold <[email protected]>; [email protected]; [email protected]; Arun Pappan (FTDI-SG) <[email protected]>
Subject: Re: [PATCH 1/6] USB: serial: ftdi_sio: add configurable inter-batch defer for read URBs

Quoted 13 lines · Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)

Hi Greg,
Quoted 2 lines · Greg Kroah-Hartman
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.

Quoted 7 lines

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.

thanks,

greg k-h

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-22 07:39 UTC #7 lore raw

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

In reply to 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 <[email protected]>

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

drivers/usb/serial/ftdi_sio.c +41 -7

index 8ae887107de7..ac0951ba74a0 100644
@@ -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;

Signature · 1 line

2.43.0

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-22 07:39 UTC #8 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/usb/serial/ftdi_sio.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/usb/serial/ftdi_sio.c +17 -0

index ac0951ba74a0..c0cda19f074a 100644
@@ -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)

Signature · 1 line

2.43.0

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-22 07:39 UTC #9 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/usb/serial/ftdi_sio.c | 66 +++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

drivers/usb/serial/ftdi_sio.c +66 -0

index c0cda19f074a..8faa073f1383 100644
@@ -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;

Signature · 1 line

2.43.0

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-22 07:39 UTC #10 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/usb/serial/ftdi_sio.c | 57 ++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 5 deletions(-)

drivers/usb/serial/ftdi_sio.c +52 -5

index 8faa073f1383..c631c6d0a1a5 100644
@@ -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);

Signature · 1 line

2.43.0

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-22 07:39 UTC #11 lore raw

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

In reply to 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 <[email protected]>

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

drivers/usb/serial/ftdi_sio.c +43 -7

index c631c6d0a1a5..7d7644ecab06 100644
@@ -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);

Signature · 1 line

2.43.0

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-22 09:44 UTC #12 lore raw

v2 cover USB: serial: ftdi_sio: latency_timer reliability fixes

In reply to 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

Signature · 1 line

2.43.0

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-22 09:44 UTC #13 lore raw

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

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 <[email protected]>

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

drivers/usb/serial/ftdi_sio.c +41 -7

index af14548fa03d..7aaa7fc1be71 100644
@@ -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;

Signature · 1 line

2.43.0

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-22 09:44 UTC #14 lore raw

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

In reply to Chinna Mopurigari Naveen Kumar Reddy

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 <[email protected]>

---
 drivers/usb/serial/ftdi_sio.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/usb/serial/ftdi_sio.c +17 -0

index 7aaa7fc1be71..e7f13eca7ae6 100644
@@ -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)

Signature · 1 line

2.43.0

Greg Kroah-Hartman

maintainer

2026-06-22 10:19 UTC #15 lore raw

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

Quoted 38 lines · 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 <[email protected]>
---
 drivers/usb/serial/ftdi_sio.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
drivers/usb/serial/ftdi_sio.c +4 -0
index 7aaa7fc1be71..e7f13eca7ae6 100644
@@ -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

drivers/usb/serial/ftdi_sio.c static ssize_t latency_timer_store(struct device *dev, · line 1699

Quoted 10 lines

drivers/usb/serial/ftdi_sio.c +10 -0

+	 * 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

Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)

author

2026-06-23 00:20 UTC #16 lore raw

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

Hi Greg,

Quoted 2 lines · Greg Kroah-Hartman

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

Will do, thanks. I'll change it to dev_dbg() in v3.

I'll hold the respin for a day or so to collect any further review comments (including on patch 1/2) and address them together.

Thanks,
Naveen

-----Original Message-----
From: Greg Kroah-Hartman <[email protected]>
Sent: Monday, 22 June 2026 6:19 pm
To: Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG) <[email protected]>
Cc: Johan Hovold <[email protected]>; [email protected]; [email protected]; Arun Pappan (FTDI-SG) <[email protected]>
Subject: Re: [PATCH v2 2/2] USB: serial: ftdi_sio: make explicit latency_timer sysfs write authoritative

Quoted 39 lines · 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 <[email protected]>
---
 drivers/usb/serial/ftdi_sio.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
(quoted diff) +0 -0
b/drivers/usb/serial/ftdi_sio.c index 7aaa7fc1be71..e7f13eca7ae6 100644
drivers/usb/serial/ftdi_sio.c +4 -0
@@ -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

drivers/usb/serial/ftdi_sio.c static ssize_t latency_timer_store(struct device *dev, · line 1699

Quoted 10 lines

drivers/usb/serial/ftdi_sio.c +10 -0

+	 * 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

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-23 08:03 UTC #17 lore raw

v3 cover USB: serial: ftdi_sio: latency_timer reliability fixes

In reply to Chinna Mopurigari Naveen Kumar Reddy

Two independent correctness fixes for ftdi_sio:

Changes since v2:
- patch 2/2: lower the override notice from dev_info() to dev_dbg() so the driver stays quiet in normal operation (Greg Kroah-Hartman).

Changes since v1:
- Dropped the urb_defer_timer_ns and low_latency_defer_ns module-parameter patches, which addressed DMA-channel starvation on a host controller that does not enforce DMA-channel fairness. As Greg Kroah-Hartman noted, that belongs in the host controller driver, not as a per-driver workaround in ftdi_sio.
- Dropped the per-port low_latency sysfs attribute and its serialisation fix, which depended on the hrtimer infrastructure introduced by the dropped patch and have no standalone purpose without it.

Earlier postings:
[v1] https://lore.kernel.org/linux-usb/[email protected]/
[v2] https://lore.kernel.org/linux-usb/[email protected]/

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

base-commit: ba3e43a9e601636f5edb54e259a74f96ca3b8fd8

Signature · 1 line

2.43.0

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-23 08:03 UTC #18 lore raw

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

Signed-off-by Chinna Mopurigari Naveen Kumar Reddy <[email protected]>

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

drivers/usb/serial/ftdi_sio.c +41 -7

index af14548fa03d..7aaa7fc1be71 100644
@@ -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;

Signature · 1 line

2.43.0

Johan Hovold

maintainer

2026-07-21 07:33 UTC #19 lore raw

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

Quoted 6 lines · 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.

As you described here in v1 of this patch you observed this with an out-of-tree host controller driver where this is just one symptom.

So this falls in the same category of random function driver workarounds for what are really host controller driver issues.

Just configure your latency timers before overloading the controller, or have your userspace retry on errors.

Johan

Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)

author

2026-07-21 07:45 UTC #20 lore raw

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

Hi Johan,

Understood. I will drop this patch. Thanks for the review.

Regards,
Navee

-----Original Message-----
From: Johan Hovold <[email protected]>
Sent: Tuesday, 21 July 2026 3:34 pm
To: Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG) <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>; [email protected]; [email protected]; Arun Pappan (FTDI-SG) <[email protected]>
Subject: Re: [PATCH v3 1/2] USB: serial: ftdi_sio: retry transient errors on chip-side control transfers

New Sender Alert! You don't often receive emails from this sender.

Quoted 6 lines · 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.

As you described here in v1 of this patch you observed this with an out-of-tree host controller driver where this is just one symptom.

So this falls in the same category of random function driver workarounds for what are really host controller driver issues.

Just configure your latency timers before overloading the controller, or have your userspace retry on errors.

Johan

Chinna Mopurigari Naveen Kumar Reddy

author

2026-06-23 08:03 UTC #21 lore raw

PATCH v3 2/2 USB: serial: ftdi_sio: make explicit latency_timer sysfs write authoritative

In reply to Chinna Mopurigari Naveen Kumar Reddy

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 <[email protected]>

---
 drivers/usb/serial/ftdi_sio.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/usb/serial/ftdi_sio.c +17 -0

index 7aaa7fc1be71..d2547ad76423 100644
@@ -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_dbg(&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)

Signature · 1 line

2.43.0

Johan Hovold

maintainer

2026-07-21 08:03 UTC #22 lore raw

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

Quoted 17 lines · 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. The override is logged at debug level (dev_dbg) for diagnostics only.

Quoted 3 lines

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.

No, I don't think we should be changing this now.

The driver has behaved this way since 2009 and commit 557aaa7ffab6 ("ft232: support the ASYNC_LOW_LATENCY flag") which explicitly let the standard setserial interface override the "obscure chip specific" interface.

Also note that the value written to sysfs does take effect once the low-latency flag is cleared so we can't really return an error like -EBUSY either.

I don't know how libftdi uses the flag, but it generally would not even be set unless you explicitly set it using setserial.

Perhaps you can send a fix to libftdi so that it uses the sysfs interface directly and leaves the flag alone if that's the library that's causing the flag to unknowingly be left set.

Johan

Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)

author

2026-07-22 05:29 UTC #23 lore raw

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

Hi Johan,

Thanks for the clarification. I'll drop this and leave the existing precedence unchanged - I checked, and nothing on our side was actually setting ASYNC_LOW_LATENCY.

Regards,
Naveen

-----Original Message-----
From: Johan Hovold <[email protected]>
Sent: Tuesday, 21 July 2026 4:04 pm
To: Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG) <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>; [email protected]; [email protected]; Arun Pappan (FTDI-SG) <[email protected]>
Subject: Re: [PATCH v3 2/2] USB: serial: ftdi_sio: make explicit latency_timer sysfs write authoritative

Quoted 17 lines · 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. The override is logged at debug level (dev_dbg) for diagnostics only.

Quoted 3 lines

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.

No, I don't think we should be changing this now.

The driver has behaved this way since 2009 and commit 557aaa7ffab6
("ft232: support the ASYNC_LOW_LATENCY flag") which explicitly let the standard setserial interface override the "obscure chip specific" interface.

Also note that the value written to sysfs does take effect once the low-latency flag is cleared so we can't really return an error like -EBUSY either.

I don't know how libftdi uses the flag, but it generally would not even be set unless you explicitly set it using setserial.

Perhaps you can send a fix to libftdi so that it uses the sysfs interface directly and leaves the flag alone if that's the library that's causing the flag to unknowingly be left set.

Johan

Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)

author

2026-07-17 06:39 UTC #24 lore raw

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

In reply to Chinna Mopurigari Naveen Kumar Reddy

Hi Johan and Greg,

A gentle ping on this v3 series - any feedback would be appreciated. Happy to respin if needed.

Thanks and Regards
Naveen

-----Original Message-----
From: Chinna Mopurigari Naveen Kumar Reddy (FTDI-SG)
Sent: Tuesday, 23 June 2026 4:04 pm
To: Johan Hovold <[email protected]>; Greg Kroah-Hartman <[email protected]>
Cc: [email protected]; [email protected]; Arun Pappan (FTDI-SG) <[email protected]>
Subject: [PATCH v3 0/2] USB: serial: ftdi_sio: latency_timer reliability fixes

Two independent correctness fixes for ftdi_sio:

Changes since v2:
- patch 2/2: lower the override notice from dev_info() to dev_dbg() so the driver stays quiet in normal operation (Greg Kroah-Hartman).

Earlier postings:
[v1] https://lore.kernel.org/linux-usb/[email protected]/
[v2] https://lore.kernel.org/linux-usb/[email protected]/

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

base-commit: ba3e43a9e601636f5edb54e259a74f96ca3b8fd8

Signature · 1 line

2.43.0