* [PATCH 1/3] USB: serial: metro-usb: replace unnecessary atomic allocation
From: Johan Hovold @ 2026-06-23 15:21 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623152148.316149-1-johan@kernel.org>
The unthrottle callback is allowed to sleep so pass the correct GFP flag
to usb_submit_urb() to avoid unnecessary atomic allocations.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/metro-usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/serial/metro-usb.c b/drivers/usb/serial/metro-usb.c
index 35473544f1c8..f42ad5dec35e 100644
--- a/drivers/usb/serial/metro-usb.c
+++ b/drivers/usb/serial/metro-usb.c
@@ -329,7 +329,7 @@ static void metrousb_unthrottle(struct tty_struct *tty)
spin_unlock_irqrestore(&metro_priv->lock, flags);
/* Submit the urb to read from the port. */
- result = usb_submit_urb(port->interrupt_in_urb, GFP_ATOMIC);
+ result = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
if (result)
dev_err(&port->dev,
"failed submitting interrupt in urb error code=%d\n",
--
2.53.0
^ permalink raw reply related
* [PATCH 2/3] USB: serial: metro-usb: fix unthrottle race
From: Johan Hovold @ 2026-06-23 15:21 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623152148.316149-1-johan@kernel.org>
If the completion handler races with unthrottle() both functions may try
to resubmit the same interrupt-in urb, but at most one will succeed.
Fix the unthrottle logic using a throttle-requested flag so that only
one attempt to resubmit the urb is made to avoid logging an error.
Fixes: 43d186fe992d ("USB: serial: add metro-usb driver to the tree")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/metro-usb.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/serial/metro-usb.c b/drivers/usb/serial/metro-usb.c
index f42ad5dec35e..8458713277f4 100644
--- a/drivers/usb/serial/metro-usb.c
+++ b/drivers/usb/serial/metro-usb.c
@@ -36,6 +36,7 @@
struct metrousb_private {
spinlock_t lock;
int throttled;
+ int throttle_req;
unsigned long control_state;
};
@@ -143,7 +144,10 @@ static void metrousb_read_int_callback(struct urb *urb)
/* Set any port variables. */
spin_lock_irqsave(&metro_priv->lock, flags);
- throttled = metro_priv->throttled;
+ if (metro_priv->throttle_req) {
+ metro_priv->throttled = 1;
+ throttled = 1;
+ }
spin_unlock_irqrestore(&metro_priv->lock, flags);
if (throttled)
@@ -175,6 +179,7 @@ static int metrousb_open(struct tty_struct *tty, struct usb_serial_port *port)
spin_lock_irqsave(&metro_priv->lock, flags);
metro_priv->control_state = 0;
metro_priv->throttled = 0;
+ metro_priv->throttle_req = 0;
spin_unlock_irqrestore(&metro_priv->lock, flags);
/* Clear the urb pipe. */
@@ -269,7 +274,7 @@ static void metrousb_throttle(struct tty_struct *tty)
/* Set the private information for the port to stop reading data. */
spin_lock_irqsave(&metro_priv->lock, flags);
- metro_priv->throttled = 1;
+ metro_priv->throttle_req = 1;
spin_unlock_irqrestore(&metro_priv->lock, flags);
}
@@ -321,19 +326,23 @@ static void metrousb_unthrottle(struct tty_struct *tty)
struct usb_serial_port *port = tty->driver_data;
struct metrousb_private *metro_priv = usb_get_serial_port_data(port);
unsigned long flags;
+ int throttled;
int result = 0;
/* Set the private information for the port to resume reading data. */
spin_lock_irqsave(&metro_priv->lock, flags);
+ throttled = metro_priv->throttled;
metro_priv->throttled = 0;
+ metro_priv->throttle_req = 0;
spin_unlock_irqrestore(&metro_priv->lock, flags);
- /* Submit the urb to read from the port. */
- result = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
- if (result)
- dev_err(&port->dev,
- "failed submitting interrupt in urb error code=%d\n",
- result);
+ if (throttled) {
+ result = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
+ if (result) {
+ dev_err(&port->dev, "failed to submit interrupt in urb: %d\n",
+ result);
+ }
+ }
}
static struct usb_serial_driver metrousb_device = {
--
2.53.0
^ permalink raw reply related
* [PATCH 3/3] USB: serial: metro-usb: drop redundant initialisations
From: Johan Hovold @ 2026-06-23 15:21 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623152148.316149-1-johan@kernel.org>
Three functions are initialising their return value variables at
declaration only to later assign them unconditionally.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/metro-usb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/serial/metro-usb.c b/drivers/usb/serial/metro-usb.c
index 8458713277f4..22c5f071f116 100644
--- a/drivers/usb/serial/metro-usb.c
+++ b/drivers/usb/serial/metro-usb.c
@@ -109,7 +109,7 @@ static void metrousb_read_int_callback(struct urb *urb)
unsigned char *data = urb->transfer_buffer;
unsigned long flags;
int throttled = 0;
- int result = 0;
+ int result;
dev_dbg(&port->dev, "%s\n", __func__);
@@ -173,7 +173,7 @@ static int metrousb_open(struct tty_struct *tty, struct usb_serial_port *port)
struct usb_serial *serial = port->serial;
struct metrousb_private *metro_priv = usb_get_serial_port_data(port);
unsigned long flags;
- int result = 0;
+ int result;
/* Set the private data information for the port. */
spin_lock_irqsave(&metro_priv->lock, flags);
@@ -327,7 +327,7 @@ static void metrousb_unthrottle(struct tty_struct *tty)
struct metrousb_private *metro_priv = usb_get_serial_port_data(port);
unsigned long flags;
int throttled;
- int result = 0;
+ int result;
/* Set the private information for the port to resume reading data. */
spin_lock_irqsave(&metro_priv->lock, flags);
--
2.53.0
^ permalink raw reply related
* [PATCH 0/3] USB: serial: metro-usb: fix unthrottle race
From: Johan Hovold @ 2026-06-23 15:21 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
This series fixes a mostly benign unthrottle race. Included is also
preparatory patch replacing an unnecessary atomic allocation and a
related variable declaration cleanup.
Johan
Johan Hovold (3):
USB: serial: metro-usb: replace unnecessary atomic allocation
USB: serial: metro-usb: fix unthrottle race
USB: serial: metro-usb: drop redundant initialisations
drivers/usb/serial/metro-usb.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH] USB: serial: digi_acceleport: fix write buffer corruption
From: Johan Hovold @ 2026-06-23 15:12 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold, stable
The digi_write_inb_command() is supposed to wait for the write urb to
become available or return an error, but instead it updates the transfer
buffer and tries to resubmit the urb on timeout.
To make things worse, for commands like break control where no timeout
is used, the driver would corrupt the urb immediately due to a broken
jiffies comparison (on 32-bit machines this takes five minutes of uptime
to trigger due to INITIAL_JIFFIES).
Fix this by adding the missing return on timeout and waiting
indefinitely when no timeout has been specified as intended.
This issue was (sort of) flagged by Sashiko when reviewing an unrelated
change to the driver.
Link: https://sashiko.dev/#/patchset/20260610132232.356139-1-johan%40kernel.org?part=11
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 88c61030982a..fa5c3539f806 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -429,20 +429,22 @@ static int digi_write_inb_command(struct usb_serial_port *port,
int len;
struct digi_port *priv = usb_get_serial_port_data(port);
unsigned char *data = port->write_urb->transfer_buffer;
+ unsigned long expire;
unsigned long flags;
dev_dbg(&port->dev, "digi_write_inb_command: TOP: port=%d, count=%d\n",
priv->dp_port_num, count);
if (timeout)
- timeout += jiffies;
- else
- timeout = ULONG_MAX;
+ expire = jiffies + timeout;
spin_lock_irqsave(&priv->dp_port_lock, flags);
while (count > 0 && ret == 0) {
- while (priv->dp_write_urb_in_use &&
- time_before(jiffies, timeout)) {
+ while (priv->dp_write_urb_in_use) {
+ if (timeout && time_after(jiffies, expire)) {
+ ret = -ETIMEDOUT;
+ break;
+ }
cond_wait_interruptible_timeout_irqrestore(
&priv->write_wait, DIGI_RETRY_TIMEOUT,
&priv->dp_port_lock, flags);
@@ -451,6 +453,9 @@ static int digi_write_inb_command(struct usb_serial_port *port,
spin_lock_irqsave(&priv->dp_port_lock, flags);
}
+ if (ret)
+ break;
+
/* len must be a multiple of 4 and small enough to */
/* guarantee the write will send buffered data first, */
/* so commands are in order with data and not split */
--
2.53.0
^ permalink raw reply related
* [PATCH] USB: serial: digi_acceleport: fix hard lockup on disconnect
From: Johan Hovold @ 2026-06-23 15:11 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold, stable
If submitting the OOB write urb fails persistently (e.g if the device is
being disconnected) the driver would loop indefinitely with interrupts
disabled.
Check for urb submission errors when sending OOB commands to avoid
hanging if, for example, open(), set_termios() or close() races with a
physical disconnect.
This is issue was flagged by Sashiko when reviewing an unrelated change
to the driver.
Link: https://sashiko.dev/#/patchset/20260610132232.356139-1-johan%40kernel.org?part=1
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 6899aebfd6ae..88c61030982a 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -392,12 +392,14 @@ static int digi_write_oob_command(struct usb_serial_port *port,
len &= ~3;
memcpy(oob_port->write_urb->transfer_buffer, buf, len);
oob_port->write_urb->transfer_buffer_length = len;
+
ret = usb_submit_urb(oob_port->write_urb, GFP_ATOMIC);
- if (ret == 0) {
- oob_priv->dp_write_urb_in_use = 1;
- count -= len;
- buf += len;
- }
+ if (ret)
+ break;
+
+ oob_priv->dp_write_urb_in_use = 1;
+ count -= len;
+ buf += len;
}
spin_unlock_irqrestore(&oob_priv->dp_port_lock, flags);
if (ret)
--
2.53.0
^ permalink raw reply related
* [PATCH v2 02/12] USB: serial: digi_acceleport: fix port registration order
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold, stable
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
The driver submits the read urbs for all ports when the first port is
opened, which could happen before the other ports have been probed and
their private data set up.
If such an urb completes before the port has been probed, the completion
handler will not resubmit it, thus preventing any further reads.
Fix the ordering issue by not submitting the port read urbs until the
port is opened.
This also avoids wasting resources (e.g. power) when ports are not in
use.
Note that the port write urbs are already stopped on close (unless
unbinding, but they are also stopped by core on disconnect).
Fixes: fb44ff854e14 ("USB: digi_acceleport: fix port-data memory leak")
Cc: stable@vger.kernel.org # 3.7
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 48 +++++++++++-----------------
1 file changed, 19 insertions(+), 29 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 097769525ca4..046cd9d57600 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -1069,7 +1069,6 @@ static int digi_open(struct tty_struct *tty, struct usb_serial_port *port)
unsigned char buf[32];
struct digi_port *priv = usb_get_serial_port_data(port);
struct ktermios not_termios;
- int throttled;
/* be sure the device is started up */
if (digi_startup_device(port->serial) != 0)
@@ -1099,17 +1098,14 @@ static int digi_open(struct tty_struct *tty, struct usb_serial_port *port)
}
spin_lock_irq(&priv->dp_port_lock);
- throttled = priv->dp_throttle_restart;
priv->dp_throttled = 0;
priv->dp_throttle_restart = 0;
spin_unlock_irq(&priv->dp_port_lock);
- if (throttled) {
- ret = usb_submit_urb(port->read_urb, GFP_KERNEL);
- if (ret) {
- dev_err(&port->dev, "failed to submit read urb: %d\n", ret);
- return ret;
- }
+ ret = usb_submit_urb(port->read_urb, GFP_KERNEL);
+ if (ret) {
+ dev_err(&port->dev, "failed to submit read urb: %d\n", ret);
+ return ret;
}
return 0;
@@ -1123,6 +1119,8 @@ static void digi_close(struct usb_serial_port *port)
unsigned char buf[32];
struct digi_port *priv = usb_get_serial_port_data(port);
+ usb_kill_urb(port->read_urb);
+
mutex_lock(&port->serial->disc_mutex);
/* if disconnected, just clear flags */
if (port->serial->disconnected)
@@ -1185,15 +1183,15 @@ static void digi_close(struct usb_serial_port *port)
/*
* Digi Startup Device
*
- * Starts reads on all ports. Must be called AFTER startup, with
+ * Starts read on the OOB port. Must be called AFTER startup, with
* urbs initialized. Returns 0 if successful, non-zero error otherwise.
*/
static int digi_startup_device(struct usb_serial *serial)
{
- int i, ret = 0;
struct digi_serial *serial_priv = usb_get_serial_data(serial);
- struct usb_serial_port *port;
+ struct usb_serial_port *oob_port = serial_priv->ds_oob_port;
+ int ret;
/* be sure this happens exactly once */
spin_lock(&serial_priv->ds_serial_lock);
@@ -1204,19 +1202,13 @@ static int digi_startup_device(struct usb_serial *serial)
serial_priv->ds_device_started = 1;
spin_unlock(&serial_priv->ds_serial_lock);
- /* start reading from each bulk in endpoint for the device */
- /* set USB_DISABLE_SPD flag for write bulk urbs */
- for (i = 0; i < serial->type->num_ports + 1; i++) {
- port = serial->port[i];
- ret = usb_submit_urb(port->read_urb, GFP_KERNEL);
- if (ret != 0) {
- dev_err(&port->dev,
- "%s: usb_submit_urb failed, ret=%d, port=%d\n",
- __func__, ret, i);
- break;
- }
+ ret = usb_submit_urb(oob_port->read_urb, GFP_KERNEL);
+ if (ret) {
+ dev_err(&serial->interface->dev, "failed to submit OOB read urb: %d\n", ret);
+ return ret;
}
- return ret;
+
+ return 0;
}
static int digi_port_init(struct usb_serial_port *port, unsigned port_num)
@@ -1287,13 +1279,11 @@ static int digi_startup(struct usb_serial *serial)
static void digi_disconnect(struct usb_serial *serial)
{
- int i;
+ struct digi_serial *serial_priv = usb_get_serial_data(serial);
+ struct usb_serial_port *oob_port = serial_priv->ds_oob_port;
- /* stop reads and writes on all ports */
- for (i = 0; i < serial->type->num_ports + 1; i++) {
- usb_kill_urb(serial->port[i]->read_urb);
- usb_kill_urb(serial->port[i]->write_urb);
- }
+ usb_kill_urb(oob_port->read_urb);
+ usb_kill_urb(oob_port->write_urb);
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 12/12] USB: serial: digi_acceleport: clean up inb command submission
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
Clean up the inb command handling a bit by removing an unnecessary line
break and moving the assignment operator before breaking another long
expression.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 6a557fc1ad85..6790892ca2a4 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -453,11 +453,10 @@ static int digi_write_inb_command(struct usb_serial_port *port,
if (priv->dp_out_buf_len > 0) {
data[0] = DIGI_CMD_SEND_DATA;
data[1] = priv->dp_out_buf_len;
- memcpy(data + 2, priv->dp_out_buf,
- priv->dp_out_buf_len);
+ memcpy(data + 2, priv->dp_out_buf, priv->dp_out_buf_len);
memcpy(data + 2 + priv->dp_out_buf_len, buf, len);
- port->write_urb->transfer_buffer_length
- = priv->dp_out_buf_len + 2 + len;
+ port->write_urb->transfer_buffer_length =
+ priv->dp_out_buf_len + 2 + len;
} else {
memcpy(data, buf, len);
port->write_urb->transfer_buffer_length = len;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 06/12] USB: serial: digi_acceleport: clean up declarations and whitespace
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
Clean up the driver by moving some declarations to approximate reverse
xmas style and removing some stray newlines (and adding a few for
readability).
While at it, also replace two spaces before tabs in the driver structs.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 88 +++++++++-------------------
1 file changed, 29 insertions(+), 59 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index ae4807455dc0..0dd898a96c33 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -271,7 +271,7 @@ static struct usb_serial_driver digi_acceleport_2_device = {
.dtr_rts = digi_dtr_rts,
.write = digi_write,
.write_room = digi_write_room,
- .write_bulk_callback = digi_write_bulk_callback,
+ .write_bulk_callback = digi_write_bulk_callback,
.read_bulk_callback = digi_read_bulk_callback,
.chars_in_buffer = digi_chars_in_buffer,
.throttle = digi_rx_throttle,
@@ -300,7 +300,7 @@ static struct usb_serial_driver digi_acceleport_4_device = {
.close = digi_close,
.write = digi_write,
.write_room = digi_write_room,
- .write_bulk_callback = digi_write_bulk_callback,
+ .write_bulk_callback = digi_write_bulk_callback,
.read_bulk_callback = digi_read_bulk_callback,
.chars_in_buffer = digi_chars_in_buffer,
.throttle = digi_rx_throttle,
@@ -334,7 +334,6 @@ static struct usb_serial_driver * const serial_drivers[] = {
* interruptible_sleep_on_timeout is deprecated and has been replaced
* with the equivalent code.
*/
-
static long cond_wait_interruptible_timeout_irqrestore(
wait_queue_head_t *q, long timeout,
spinlock_t *lock, unsigned long flags)
@@ -367,15 +366,14 @@ static struct usb_serial_port *digi_get_oob_port(struct usb_serial *serial)
* the interruptible flag is true, or a negative error
* returned by usb_submit_urb.
*/
-
static int digi_write_oob_command(struct usb_serial_port *port,
unsigned char *buf, int count, int interruptible)
{
- int ret = 0;
- int len;
struct usb_serial_port *oob_port = digi_get_oob_port(port->serial);
struct digi_port *oob_priv = usb_get_serial_port_data(oob_port);
unsigned long flags;
+ int ret = 0;
+ int len;
dev_dbg(&port->dev,
"digi_write_oob_command: TOP: port=%d, count=%d\n",
@@ -410,10 +408,8 @@ static int digi_write_oob_command(struct usb_serial_port *port,
dev_err(&port->dev, "%s: usb_submit_urb failed, ret=%d\n",
__func__, ret);
return ret;
-
}
-
/*
* Digi Write In Band Command
*
@@ -425,15 +421,14 @@ static int digi_write_oob_command(struct usb_serial_port *port,
* timeout ticks. Returns 0 if successful, or a negative
* error returned by digi_write.
*/
-
static int digi_write_inb_command(struct usb_serial_port *port,
unsigned char *buf, int count, unsigned long timeout)
{
- int ret = 0;
- int len;
struct digi_port *priv = usb_get_serial_port_data(port);
unsigned char *data = port->write_urb->transfer_buffer;
unsigned long flags;
+ int ret = 0;
+ int len;
dev_dbg(&port->dev, "digi_write_inb_command: TOP: port=%d, count=%d\n",
priv->dp_port_num, count);
@@ -483,7 +478,6 @@ static int digi_write_inb_command(struct usb_serial_port *port,
count -= len;
buf += len;
}
-
}
spin_unlock_irqrestore(&priv->dp_port_lock, flags);
@@ -494,7 +488,6 @@ static int digi_write_inb_command(struct usb_serial_port *port,
return ret;
}
-
/*
* Digi Set Modem Signals
*
@@ -504,17 +497,15 @@ static int digi_write_inb_command(struct usb_serial_port *port,
* -EINTR if interrupted while sleeping, or a non-zero error
* returned by usb_submit_urb.
*/
-
static int digi_set_modem_signals(struct usb_serial_port *port,
unsigned int modem_signals, int interruptible)
{
-
- int ret;
struct digi_port *port_priv = usb_get_serial_port_data(port);
struct usb_serial_port *oob_port = digi_get_oob_port(port->serial);
struct digi_port *oob_priv = usb_get_serial_port_data(oob_port);
unsigned char *data = oob_port->write_urb->transfer_buffer;
unsigned long flags;
+ int ret;
dev_dbg(&port->dev,
"digi_set_modem_signals: TOP: port=%d, modem_signals=0x%x\n",
@@ -572,14 +563,13 @@ static int digi_set_modem_signals(struct usb_serial_port *port,
* is only called from close, and only one process can be in close on a
* port at a time, so its ok.
*/
-
static int digi_transmit_idle(struct usb_serial_port *port,
unsigned long timeout)
{
- int ret;
- unsigned char buf[2];
struct digi_port *priv = usb_get_serial_port_data(port);
+ unsigned char buf[2];
unsigned long flags;
+ int ret;
spin_lock_irqsave(&priv->dp_port_lock, flags);
priv->dp_transmit_idle = 0;
@@ -606,16 +596,15 @@ static int digi_transmit_idle(struct usb_serial_port *port,
}
priv->dp_transmit_idle = 0;
spin_unlock_irqrestore(&priv->dp_port_lock, flags);
- return 0;
+ return 0;
}
-
static void digi_rx_throttle(struct tty_struct *tty)
{
- unsigned long flags;
struct usb_serial_port *port = tty->driver_data;
struct digi_port *priv = usb_get_serial_port_data(port);
+ unsigned long flags;
/* stop receiving characters by not resubmitting the read urb */
spin_lock_irqsave(&priv->dp_port_lock, flags);
@@ -624,13 +613,12 @@ static void digi_rx_throttle(struct tty_struct *tty)
spin_unlock_irqrestore(&priv->dp_port_lock, flags);
}
-
static void digi_rx_unthrottle(struct tty_struct *tty)
{
- int ret = 0;
- unsigned long flags;
struct usb_serial_port *port = tty->driver_data;
struct digi_port *priv = usb_get_serial_port_data(port);
+ unsigned long flags;
+ int ret = 0;
spin_lock_irqsave(&priv->dp_port_lock, flags);
@@ -650,7 +638,6 @@ static void digi_rx_unthrottle(struct tty_struct *tty)
__func__, ret, priv->dp_port_num);
}
-
static void digi_set_termios(struct tty_struct *tty,
struct usb_serial_port *port,
const struct ktermios *old_termios)
@@ -761,7 +748,6 @@ static void digi_set_termios(struct tty_struct *tty,
/* set stop bits */
if ((cflag & CSTOPB) != (old_cflag & CSTOPB)) {
-
if ((cflag & CSTOPB))
arg = DIGI_STOP_BITS_2;
else
@@ -771,7 +757,6 @@ static void digi_set_termios(struct tty_struct *tty,
buf[i++] = priv->dp_port_num;
buf[i++] = arg;
buf[i++] = 0;
-
}
/* set input flow control */
@@ -840,7 +825,6 @@ static void digi_set_termios(struct tty_struct *tty,
tty_encode_baud_rate(tty, baud, baud);
}
-
static int digi_break_ctl(struct tty_struct *tty, int break_state)
{
struct usb_serial_port *port = tty->driver_data;
@@ -854,43 +838,41 @@ static int digi_break_ctl(struct tty_struct *tty, int break_state)
return digi_write_inb_command(port, buf, 4, 0);
}
-
static int digi_tiocmget(struct tty_struct *tty)
{
struct usb_serial_port *port = tty->driver_data;
struct digi_port *priv = usb_get_serial_port_data(port);
- unsigned int val;
unsigned long flags;
+ unsigned int val;
spin_lock_irqsave(&priv->dp_port_lock, flags);
val = priv->dp_modem_signals;
spin_unlock_irqrestore(&priv->dp_port_lock, flags);
+
return val;
}
-
static int digi_tiocmset(struct tty_struct *tty,
unsigned int set, unsigned int clear)
{
struct usb_serial_port *port = tty->driver_data;
struct digi_port *priv = usb_get_serial_port_data(port);
- unsigned int val;
unsigned long flags;
+ unsigned int val;
spin_lock_irqsave(&priv->dp_port_lock, flags);
val = (priv->dp_modem_signals & ~clear) | set;
spin_unlock_irqrestore(&priv->dp_port_lock, flags);
+
return digi_set_modem_signals(port, val, 1);
}
-
static int digi_write(struct tty_struct *tty, struct usb_serial_port *port,
const unsigned char *buf, int count)
{
-
- int ret, data_len, new_len;
struct digi_port *priv = usb_get_serial_port_data(port);
unsigned char *data = port->write_urb->transfer_buffer;
+ int ret, data_len, new_len;
unsigned long flags;
dev_dbg(&port->dev, "digi_write: TOP: port=%d, count=%d\n",
@@ -953,21 +935,20 @@ static int digi_write(struct tty_struct *tty, struct usb_serial_port *port,
"%s: usb_submit_urb failed, ret=%d, port=%d\n",
__func__, ret, priv->dp_port_num);
dev_dbg(&port->dev, "digi_write: returning %d\n", ret);
- return ret;
+ return ret;
}
static void digi_write_bulk_callback(struct urb *urb)
{
-
struct usb_serial_port *port = urb->context;
struct usb_serial *serial;
struct digi_port *priv;
struct digi_serial *serial_priv;
- unsigned long flags;
- int ret = 0;
int status = urb->status;
+ unsigned long flags;
bool wakeup;
+ int ret = 0;
/* port and serial sanity check */
if (port == NULL || (priv = usb_get_serial_port_data(port)) == NULL) {
@@ -1040,8 +1021,8 @@ static unsigned int digi_write_room(struct tty_struct *tty)
spin_unlock_irqrestore(&priv->dp_port_lock, flags);
dev_dbg(&port->dev, "digi_write_room: port=%d, room=%u\n", priv->dp_port_num, room);
- return room;
+ return room;
}
static unsigned int digi_chars_in_buffer(struct tty_struct *tty)
@@ -1071,10 +1052,10 @@ static void digi_dtr_rts(struct usb_serial_port *port, int on)
static int digi_open(struct tty_struct *tty, struct usb_serial_port *port)
{
- int ret;
- unsigned char buf[32];
struct digi_port *priv = usb_get_serial_port_data(port);
struct ktermios not_termios;
+ unsigned char buf[32];
+ int ret;
/* be sure the device is started up */
if (digi_startup_device(port->serial) != 0)
@@ -1117,13 +1098,12 @@ static int digi_open(struct tty_struct *tty, struct usb_serial_port *port)
return 0;
}
-
static void digi_close(struct usb_serial_port *port)
{
+ struct digi_port *priv = usb_get_serial_port_data(port);
+ unsigned char buf[32];
DEFINE_WAIT(wait);
int ret;
- unsigned char buf[32];
- struct digi_port *priv = usb_get_serial_port_data(port);
usb_kill_urb(port->read_urb);
@@ -1180,14 +1160,12 @@ static void digi_close(struct usb_serial_port *port)
usb_kill_urb(port->write_urb);
}
-
/*
* Digi Startup Device
*
* Starts read on the OOB port. Must be called AFTER startup, with
* urbs initialized. Returns 0 if successful, non-zero error otherwise.
*/
-
static int digi_startup_device(struct usb_serial *serial)
{
struct digi_serial *serial_priv = usb_get_serial_data(serial);
@@ -1276,7 +1254,6 @@ static int digi_startup(struct usb_serial *serial)
return 0;
}
-
static void digi_disconnect(struct usb_serial *serial)
{
struct digi_serial *serial_priv = usb_get_serial_data(serial);
@@ -1286,7 +1263,6 @@ static void digi_disconnect(struct usb_serial *serial)
usb_kill_urb(oob_port->write_urb);
}
-
static void digi_release(struct usb_serial *serial)
{
struct digi_serial *serial_priv;
@@ -1318,8 +1294,8 @@ static void digi_read_bulk_callback(struct urb *urb)
struct usb_serial_port *port = urb->context;
struct digi_port *priv;
struct digi_serial *serial_priv;
- int ret;
int status = urb->status;
+ int ret;
/* port sanity check, do not resubmit if port is not valid */
if (port == NULL)
@@ -1361,7 +1337,6 @@ static void digi_read_bulk_callback(struct urb *urb)
"%s: failed resubmitting urb, ret=%d, port=%d\n",
__func__, ret, priv->dp_port_num);
}
-
}
/*
@@ -1373,7 +1348,6 @@ static void digi_read_bulk_callback(struct urb *urb)
* It returns 0 if successful, 1 if successful but the port is
* throttled, and -1 if the sanity checks failed.
*/
-
static int digi_read_inb_callback(struct urb *urb)
{
struct usb_serial_port *port = urb->context;
@@ -1451,10 +1425,8 @@ static int digi_read_inb_callback(struct urb *urb)
dev_dbg(&port->dev, "%s: unknown opcode: %d\n", __func__, opcode);
return throttled ? 1 : 0;
-
}
-
/*
* Digi Read OOB Callback
*
@@ -1463,10 +1435,8 @@ static int digi_read_inb_callback(struct urb *urb)
* the port->serial is valid. It returns 0 if successful, and
* -1 if the sanity checks failed.
*/
-
static int digi_read_oob_callback(struct urb *urb)
{
-
struct usb_serial_port *port = urb->context;
struct usb_serial *serial = port->serial;
struct tty_struct *tty;
@@ -1474,8 +1444,8 @@ static int digi_read_oob_callback(struct urb *urb)
unsigned char *buf = urb->transfer_buffer;
int opcode, line, status, val;
unsigned long flags;
- int i;
unsigned int rts;
+ int i;
if (urb->actual_length < 4)
return -1;
@@ -1545,8 +1515,8 @@ static int digi_read_oob_callback(struct urb *urb)
}
tty_kref_put(tty);
}
- return 0;
+ return 0;
}
module_usb_serial_driver(serial_drivers, id_table_combined);
--
2.53.0
^ permalink raw reply related
* [PATCH v2 11/12] USB: serial: digi_acceleport: clean up write completion
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
Clean up the write completion handler by adding a temporary variable for
the transfer buffer and using the pre-existing urb pointer while
dropping some redundant casts.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index f7630fdcdceb..6a557fc1ad85 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -936,6 +936,7 @@ static void digi_write_bulk_callback(struct urb *urb)
struct usb_serial_port *port = urb->context;
struct digi_serial *serial_priv = usb_get_serial_data(port->serial);
struct digi_port *priv = usb_get_serial_port_data(port);
+ unsigned char *data = urb->transfer_buffer;
unsigned long flags;
bool wakeup;
int ret = 0;
@@ -955,15 +956,13 @@ static void digi_write_bulk_callback(struct urb *urb)
spin_lock_irqsave(&priv->dp_port_lock, flags);
priv->dp_write_urb_in_use = 0;
if (priv->dp_out_buf_len > 0) {
- *((unsigned char *)(port->write_urb->transfer_buffer))
- = (unsigned char)DIGI_CMD_SEND_DATA;
- *((unsigned char *)(port->write_urb->transfer_buffer) + 1)
- = (unsigned char)priv->dp_out_buf_len;
- port->write_urb->transfer_buffer_length =
- priv->dp_out_buf_len + 2;
- memcpy(port->write_urb->transfer_buffer + 2, priv->dp_out_buf,
- priv->dp_out_buf_len);
- ret = usb_submit_urb(port->write_urb, GFP_ATOMIC);
+ data[0] = DIGI_CMD_SEND_DATA;
+ data[1] = priv->dp_out_buf_len;
+ memcpy(data + 2, priv->dp_out_buf, priv->dp_out_buf_len);
+
+ urb->transfer_buffer_length = priv->dp_out_buf_len + 2;
+
+ ret = usb_submit_urb(urb, GFP_ATOMIC);
if (ret == 0) {
priv->dp_write_urb_in_use = 1;
priv->dp_out_buf_len = 0;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 09/12] USB: serial: digi_acceleport: drop unused in-buf define
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
Drop the in-buf size define which has not been used since the port
buffers were removed by commit 5fea2a4dabdf ("USB: digi_acceleport
further buffer clean up").
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index e4ab41d9f3a5..8c21c669c1a2 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -32,10 +32,6 @@
/* so we can be sure to send the full buffer in one urb */
#define DIGI_OUT_BUF_SIZE 8
-/* port input buffer length -- must be >= transfer buffer length - 3 */
-/* so we can be sure to hold at least one full buffer from one urb */
-#define DIGI_IN_BUF_SIZE 64
-
/* retry timeout while sleeping */
#define DIGI_RETRY_TIMEOUT (HZ/10)
--
2.53.0
^ permalink raw reply related
* [PATCH v2 03/12] USB: serial: digi_acceleport: drop unused wait queue
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
Drop the close wait queue which has not been used since commit
335f8514f200 ("tty: Bring the usb tty port structure into more use").
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 046cd9d57600..83c4bacc2df8 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -194,7 +194,6 @@ struct digi_port {
int dp_throttled;
int dp_throttle_restart;
wait_queue_head_t dp_flush_wait;
- wait_queue_head_t dp_close_wait; /* wait queue for close */
wait_queue_head_t write_wait;
struct usb_serial_port *dp_port;
};
@@ -1174,7 +1173,6 @@ static void digi_close(struct usb_serial_port *port)
exit:
spin_lock_irq(&priv->dp_port_lock);
priv->dp_write_urb_in_use = 0;
- wake_up_interruptible(&priv->dp_close_wait);
spin_unlock_irq(&priv->dp_port_lock);
mutex_unlock(&port->serial->disc_mutex);
}
@@ -1223,7 +1221,6 @@ static int digi_port_init(struct usb_serial_port *port, unsigned port_num)
priv->dp_port_num = port_num;
init_waitqueue_head(&priv->dp_transmit_idle_wait);
init_waitqueue_head(&priv->dp_flush_wait);
- init_waitqueue_head(&priv->dp_close_wait);
init_waitqueue_head(&priv->write_wait);
priv->dp_port = port;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 05/12] USB: serial: digi_acceleport: add oob port helper
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
Add a helper function for retrieving the OOB port to replace two
convoluted expressions.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 7a52600c09b1..ae4807455dc0 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -350,6 +350,13 @@ __releases(lock)
return timeout;
}
+static struct usb_serial_port *digi_get_oob_port(struct usb_serial *serial)
+{
+ struct digi_serial *serial_priv = usb_get_serial_data(serial);
+
+ return serial_priv->ds_oob_port;
+}
+
/*
* Digi Write OOB Command
*
@@ -366,7 +373,7 @@ static int digi_write_oob_command(struct usb_serial_port *port,
{
int ret = 0;
int len;
- struct usb_serial_port *oob_port = (struct usb_serial_port *)((struct digi_serial *)(usb_get_serial_data(port->serial)))->ds_oob_port;
+ struct usb_serial_port *oob_port = digi_get_oob_port(port->serial);
struct digi_port *oob_priv = usb_get_serial_port_data(oob_port);
unsigned long flags;
@@ -504,7 +511,7 @@ static int digi_set_modem_signals(struct usb_serial_port *port,
int ret;
struct digi_port *port_priv = usb_get_serial_port_data(port);
- struct usb_serial_port *oob_port = (struct usb_serial_port *) ((struct digi_serial *)(usb_get_serial_data(port->serial)))->ds_oob_port;
+ struct usb_serial_port *oob_port = digi_get_oob_port(port->serial);
struct digi_port *oob_priv = usb_get_serial_port_data(oob_port);
unsigned char *data = oob_port->write_urb->transfer_buffer;
unsigned long flags;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 00/12] USB: serial: digi_acceleport: registration fix and cleanups
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
This series fixes a port registration ordering issue and makes the
driver stop all I/O when the device is not in use, while cleaning up the
driver somewhat.
Johan
Changes in v2:
- fix broken rx after throttle (new)
- rebase on fix now in mainline
Johan Hovold (12):
USB: serial: digi_acceleport: fix broken rx after throttle
USB: serial: digi_acceleport: fix port registration order
USB: serial: digi_acceleport: drop unused wait queue
USB: serial: digi_acceleport: always stop write urb on close
USB: serial: digi_acceleport: add oob port helper
USB: serial: digi_acceleport: clean up declarations and whitespace
USB: serial: digi_acceleport: drop redundant driver data sanity checks
USB: serial: digi_acceleport: stop OOB I/O when not in use
USB: serial: digi_acceleport: drop unused in-buf define
USB: serial: digi_acceleport: clean up xfer buf length expression
USB: serial: digi_acceleport: clean up write completion
USB: serial: digi_acceleport: clean up inb command submission
drivers/usb/serial/digi_acceleport.c | 304 +++++++++++----------------
1 file changed, 122 insertions(+), 182 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH v2 08/12] USB: serial: digi_acceleport: stop OOB I/O when not in use
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
The driver submits the OOB read urb on first open of a port and does not
stop it until the device is disconnected.
Add an open counter and submit the urb on first open and stop it on last
close to avoid wasting resources (e.g. power) when the device is not in
use.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 100 ++++++++++++++-------------
1 file changed, 51 insertions(+), 49 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 64b3f42af981..e4ab41d9f3a5 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -176,10 +176,10 @@
/* Structures */
struct digi_serial {
- spinlock_t ds_serial_lock;
+ struct mutex open_mutex;
struct usb_serial_port *ds_oob_port; /* out-of-band port */
int ds_oob_port_num; /* index of out-of-band port */
- int ds_device_started;
+ int open_count;
};
struct digi_port {
@@ -226,9 +226,7 @@ static unsigned int digi_chars_in_buffer(struct tty_struct *tty);
static int digi_open(struct tty_struct *tty, struct usb_serial_port *port);
static void digi_close(struct usb_serial_port *port);
static void digi_dtr_rts(struct usb_serial_port *port, int on);
-static int digi_startup_device(struct usb_serial *serial);
static int digi_startup(struct usb_serial *serial);
-static void digi_disconnect(struct usb_serial *serial);
static void digi_release(struct usb_serial *serial);
static int digi_port_probe(struct usb_serial_port *port);
static void digi_port_remove(struct usb_serial_port *port);
@@ -281,7 +279,6 @@ static struct usb_serial_driver digi_acceleport_2_device = {
.tiocmget = digi_tiocmget,
.tiocmset = digi_tiocmset,
.attach = digi_startup,
- .disconnect = digi_disconnect,
.release = digi_release,
.port_probe = digi_port_probe,
.port_remove = digi_port_remove,
@@ -310,7 +307,6 @@ static struct usb_serial_driver digi_acceleport_4_device = {
.tiocmget = digi_tiocmget,
.tiocmset = digi_tiocmset,
.attach = digi_startup,
- .disconnect = digi_disconnect,
.release = digi_release,
.port_probe = digi_port_probe,
.port_remove = digi_port_remove,
@@ -1034,6 +1030,43 @@ static void digi_dtr_rts(struct usb_serial_port *port, int on)
digi_set_modem_signals(port, on * (TIOCM_DTR | TIOCM_RTS), 1);
}
+static int digi_open_oob_port(struct usb_serial *serial)
+{
+ struct digi_serial *serial_priv = usb_get_serial_data(serial);
+ struct usb_serial_port *oob_port = serial_priv->ds_oob_port;
+ int ret = 0;
+
+ mutex_lock(&serial_priv->open_mutex);
+
+ if (serial_priv->open_count++ == 0) {
+ ret = usb_submit_urb(oob_port->read_urb, GFP_KERNEL);
+ if (ret) {
+ dev_err(&serial->interface->dev, "failed to submit OOB read urb: %d\n",
+ ret);
+ serial_priv->open_count--;
+ }
+ }
+
+ mutex_unlock(&serial_priv->open_mutex);
+
+ return ret;
+}
+
+static void digi_close_oob_port(struct usb_serial *serial)
+{
+ struct digi_serial *serial_priv = usb_get_serial_data(serial);
+ struct usb_serial_port *oob_port = serial_priv->ds_oob_port;
+
+ mutex_lock(&serial_priv->open_mutex);
+
+ if (serial_priv->open_count-- == 1) {
+ usb_kill_urb(oob_port->read_urb);
+ usb_kill_urb(oob_port->write_urb);
+ }
+
+ mutex_unlock(&serial_priv->open_mutex);
+}
+
static int digi_open(struct tty_struct *tty, struct usb_serial_port *port)
{
struct digi_port *priv = usb_get_serial_port_data(port);
@@ -1041,9 +1074,9 @@ static int digi_open(struct tty_struct *tty, struct usb_serial_port *port)
unsigned char buf[32];
int ret;
- /* be sure the device is started up */
- if (digi_startup_device(port->serial) != 0)
- return -ENXIO;
+ ret = digi_open_oob_port(port->serial);
+ if (ret)
+ return ret;
/* read modem signals automatically whenever they change */
buf[0] = DIGI_CMD_READ_INPUT_SIGNALS;
@@ -1076,10 +1109,15 @@ static int digi_open(struct tty_struct *tty, struct usb_serial_port *port)
ret = usb_submit_urb(port->read_urb, GFP_KERNEL);
if (ret) {
dev_err(&port->dev, "failed to submit read urb: %d\n", ret);
- return ret;
+ goto err_close_oob;
}
return 0;
+
+err_close_oob:
+ digi_close_oob_port(port->serial);
+
+ return ret;
}
static void digi_close(struct usb_serial_port *port)
@@ -1142,36 +1180,8 @@ static void digi_close(struct usb_serial_port *port)
/* shutdown any outstanding bulk writes */
usb_kill_urb(port->write_urb);
-}
-
-/*
- * Digi Startup Device
- *
- * Starts read on the OOB port. Must be called AFTER startup, with
- * urbs initialized. Returns 0 if successful, non-zero error otherwise.
- */
-static int digi_startup_device(struct usb_serial *serial)
-{
- struct digi_serial *serial_priv = usb_get_serial_data(serial);
- struct usb_serial_port *oob_port = serial_priv->ds_oob_port;
- int ret;
-
- /* be sure this happens exactly once */
- spin_lock(&serial_priv->ds_serial_lock);
- if (serial_priv->ds_device_started) {
- spin_unlock(&serial_priv->ds_serial_lock);
- return 0;
- }
- serial_priv->ds_device_started = 1;
- spin_unlock(&serial_priv->ds_serial_lock);
- ret = usb_submit_urb(oob_port->read_urb, GFP_KERNEL);
- if (ret) {
- dev_err(&serial->interface->dev, "failed to submit OOB read urb: %d\n", ret);
- return ret;
- }
-
- return 0;
+ digi_close_oob_port(port->serial);
}
static int digi_port_init(struct usb_serial_port *port, unsigned port_num)
@@ -1222,7 +1232,8 @@ static int digi_startup(struct usb_serial *serial)
if (!serial_priv)
return -ENOMEM;
- spin_lock_init(&serial_priv->ds_serial_lock);
+ mutex_init(&serial_priv->open_mutex);
+
serial_priv->ds_oob_port_num = oob_port_num;
serial_priv->ds_oob_port = serial->port[oob_port_num];
@@ -1238,15 +1249,6 @@ static int digi_startup(struct usb_serial *serial)
return 0;
}
-static void digi_disconnect(struct usb_serial *serial)
-{
- struct digi_serial *serial_priv = usb_get_serial_data(serial);
- struct usb_serial_port *oob_port = serial_priv->ds_oob_port;
-
- usb_kill_urb(oob_port->read_urb);
- usb_kill_urb(oob_port->write_urb);
-}
-
static void digi_release(struct usb_serial *serial)
{
struct digi_serial *serial_priv;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 01/12] USB: serial: digi_acceleport: fix broken rx after throttle
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold, stable
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
If the port is closed while throttled, the read urb is never resubmitted
and the port will not receive any further data until the device is
reconnected (or the driver is rebound).
Clear the throttle flags and submit the urb if needed when opening the
port.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 6899aebfd6ae..097769525ca4 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -1069,6 +1069,7 @@ static int digi_open(struct tty_struct *tty, struct usb_serial_port *port)
unsigned char buf[32];
struct digi_port *priv = usb_get_serial_port_data(port);
struct ktermios not_termios;
+ int throttled;
/* be sure the device is started up */
if (digi_startup_device(port->serial) != 0)
@@ -1096,6 +1097,21 @@ static int digi_open(struct tty_struct *tty, struct usb_serial_port *port)
not_termios.c_iflag = ~tty->termios.c_iflag;
digi_set_termios(tty, port, ¬_termios);
}
+
+ spin_lock_irq(&priv->dp_port_lock);
+ throttled = priv->dp_throttle_restart;
+ priv->dp_throttled = 0;
+ priv->dp_throttle_restart = 0;
+ spin_unlock_irq(&priv->dp_port_lock);
+
+ if (throttled) {
+ ret = usb_submit_urb(port->read_urb, GFP_KERNEL);
+ if (ret) {
+ dev_err(&port->dev, "failed to submit read urb: %d\n", ret);
+ return ret;
+ }
+ }
+
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 07/12] USB: serial: digi_acceleport: drop redundant driver data sanity checks
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
The urb context pointer does not change while an urb is in flight so
there is never a need to check for NULL on completion.
The port driver data is not freed until the port is unbound at which
point all I/O for that port has been stopped (and I/O is no longer
started for a port that has not yet been probed).
The device driver data is not freed until after the driver has been
unbound and at which point all I/O has also ceased.
Drop the redundant, overly defensive (and still incomplete) sanity
checks from the completion callbacks.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 40 +++-------------------------
1 file changed, 4 insertions(+), 36 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 0dd898a96c33..64b3f42af981 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -942,28 +942,12 @@ static int digi_write(struct tty_struct *tty, struct usb_serial_port *port,
static void digi_write_bulk_callback(struct urb *urb)
{
struct usb_serial_port *port = urb->context;
- struct usb_serial *serial;
- struct digi_port *priv;
- struct digi_serial *serial_priv;
- int status = urb->status;
+ struct digi_serial *serial_priv = usb_get_serial_data(port->serial);
+ struct digi_port *priv = usb_get_serial_port_data(port);
unsigned long flags;
bool wakeup;
int ret = 0;
- /* port and serial sanity check */
- if (port == NULL || (priv = usb_get_serial_port_data(port)) == NULL) {
- pr_err("%s: port or port->private is NULL, status=%d\n",
- __func__, status);
- return;
- }
- serial = port->serial;
- if (serial == NULL || (serial_priv = usb_get_serial_data(serial)) == NULL) {
- dev_err(&port->dev,
- "%s: serial or serial->private is NULL, status=%d\n",
- __func__, status);
- return;
- }
-
/* handle oob callback */
if (priv->dp_port_num == serial_priv->ds_oob_port_num) {
dev_dbg(&port->dev, "digi_write_bulk_callback: oob callback\n");
@@ -1292,27 +1276,11 @@ static void digi_port_remove(struct usb_serial_port *port)
static void digi_read_bulk_callback(struct urb *urb)
{
struct usb_serial_port *port = urb->context;
- struct digi_port *priv;
- struct digi_serial *serial_priv;
+ struct digi_serial *serial_priv = usb_get_serial_data(port->serial);
+ struct digi_port *priv = usb_get_serial_port_data(port);
int status = urb->status;
int ret;
- /* port sanity check, do not resubmit if port is not valid */
- if (port == NULL)
- return;
- priv = usb_get_serial_port_data(port);
- if (priv == NULL) {
- dev_err(&port->dev, "%s: port->private is NULL, status=%d\n",
- __func__, status);
- return;
- }
- if (port->serial == NULL ||
- (serial_priv = usb_get_serial_data(port->serial)) == NULL) {
- dev_err(&port->dev, "%s: serial is bad or serial->private "
- "is NULL, status=%d\n", __func__, status);
- return;
- }
-
/* do not resubmit urb if it has any status error */
if (status) {
dev_err(&port->dev,
--
2.53.0
^ permalink raw reply related
* [PATCH v2 04/12] USB: serial: digi_acceleport: always stop write urb on close
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
Explicitly stop the write urb on close() also if the device is being
unbound instead of relying on core to do it after returning.
Note that the dp_write_urb_in_use flag is cleared by the completion
handler.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 83c4bacc2df8..7a52600c09b1 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -1121,7 +1121,6 @@ static void digi_close(struct usb_serial_port *port)
usb_kill_urb(port->read_urb);
mutex_lock(&port->serial->disc_mutex);
- /* if disconnected, just clear flags */
if (port->serial->disconnected)
goto exit;
@@ -1167,14 +1166,11 @@ static void digi_close(struct usb_serial_port *port)
TASK_INTERRUPTIBLE);
schedule_timeout(DIGI_CLOSE_TIMEOUT);
finish_wait(&priv->dp_flush_wait, &wait);
+exit:
+ mutex_unlock(&port->serial->disc_mutex);
/* shutdown any outstanding bulk writes */
usb_kill_urb(port->write_urb);
-exit:
- spin_lock_irq(&priv->dp_port_lock);
- priv->dp_write_urb_in_use = 0;
- spin_unlock_irq(&priv->dp_port_lock);
- mutex_unlock(&port->serial->disc_mutex);
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 10/12] USB: serial: digi_acceleport: clean up xfer buf length expression
From: Johan Hovold @ 2026-06-23 15:08 UTC (permalink / raw)
To: linux-usb; +Cc: Greg Kroah-Hartman, linux-kernel, Johan Hovold
In-Reply-To: <20260623150826.314727-1-johan@kernel.org>
Add the missing space around operators in transfer-buffer length
expressions to make the code more readable.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/serial/digi_acceleport.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c b/drivers/usb/serial/digi_acceleport.c
index 8c21c669c1a2..f7630fdcdceb 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -445,7 +445,7 @@ static int digi_write_inb_command(struct usb_serial_port *port,
/* len must be a multiple of 4 and small enough to */
/* guarantee the write will send buffered data first, */
/* so commands are in order with data and not split */
- len = min(count, port->bulk_out_size-2-priv->dp_out_buf_len);
+ len = min(count, port->bulk_out_size - 2 - priv->dp_out_buf_len);
if (len > 4)
len &= ~3;
@@ -871,7 +871,7 @@ static int digi_write(struct tty_struct *tty, struct usb_serial_port *port,
priv->dp_port_num, count);
/* copy user data (which can sleep) before getting spin lock */
- count = min(count, port->bulk_out_size-2);
+ count = min(count, port->bulk_out_size - 2);
count = min(64, count);
/* be sure only one write proceeds at a time */
@@ -893,7 +893,7 @@ static int digi_write(struct tty_struct *tty, struct usb_serial_port *port,
/* allow space for any buffered data and for new data, up to */
/* transfer buffer size - 2 (for command and length bytes) */
- new_len = min(count, port->bulk_out_size-2-priv->dp_out_buf_len);
+ new_len = min(count, port->bulk_out_size - 2 - priv->dp_out_buf_len);
data_len = new_len + priv->dp_out_buf_len;
if (data_len == 0) {
@@ -901,7 +901,7 @@ static int digi_write(struct tty_struct *tty, struct usb_serial_port *port,
return 0;
}
- port->write_urb->transfer_buffer_length = data_len+2;
+ port->write_urb->transfer_buffer_length = data_len + 2;
*data++ = DIGI_CMD_SEND_DATA;
*data++ = data_len;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v1 0/3] thunderbold: A few cleanups
From: Uwe Kleine-König (The Capable Hub) @ 2026-06-23 14:35 UTC (permalink / raw)
To: Mika Westerberg
Cc: Mika Westerberg, Yehezkel Bernat, Andreas Noever, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, linux-kernel, linux-usb
In-Reply-To: <20260623121746.GD3066@black.igk.intel.com>
[-- Attachment #1: Type: text/plain, Size: 753 bytes --]
Hello Mika,
On Tue, Jun 23, 2026 at 02:17:46PM +0200, Mika Westerberg wrote:
> On Thu, Jun 18, 2026 at 12:14:49PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > Uwe Kleine-König (The Capable Hub) (3):
> > thunderbold: Stop passing matched device ID to .probe()
> > thunderbold: Assert that a service driver has a probe callback
> > thunderbold: Drop comma after device id array terminator
>
> Fixed the typo "thunderbold" -> "thunderbolt" and applied all to
Oh.
> thunderbolt.git/next. I also took the networking patch, let me know if
> that's not okay (I'm the maintainer of that driver too and it looked fine).
Sounds fine to me. So assuming you're not offending the network guys,
that should be ok.
Thanks!
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [RFC PATCH] usb: xhci: report USB2 resume-exit timeout as an error
From: Pengpeng Hou @ 2026-06-23 14:05 UTC (permalink / raw)
To: Mathias Nyman, Greg Kroah-Hartman; +Cc: Pengpeng Hou, linux-usb, linux-kernel
xhci_handle_usb2_port_link_resume() waits for the USB2 resume exit
completion after requesting U0. If that wait times out, it only logs a
warning and then continues through the normal resume-done path, ending
the port resume and marking the suspend change as completed.
Return -ETIMEDOUT on resume-exit timeout so the port status path reports
an error instead of a successful resume. The patch still ends the
usbcore port-resume accounting before returning. This is intended as an
RFC patch because xHCI resume state handling is policy-sensitive.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
drivers/usb/host/xhci-hub.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index b0264bd85..b7b7d8b31 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -998,21 +998,23 @@ static int xhci_handle_usb2_port_link_resume(struct xhci_port *port,
- if (time_left) {
- if (!port->slot_id) {
- xhci_dbg(xhci, "slot_id is zero\n");
- return -ENODEV;
- }
- xhci_ring_device(xhci, port->slot_id);
- } else {
+ if (!time_left) {
int port_status = xhci_portsc_readl(port);
xhci_warn(xhci, "Port resume timed out, port %d-%d: 0x%x\n",
hcd->self.busnum, wIndex + 1, port_status);
/*
* keep rexit_active set if U0 transition failed so we
* know to report PORT_STAT_SUSPEND status back to
* usbcore. It will be cleared later once the port is
* out of RESUME/U3 state
*/
+ usb_hcd_end_port_resume(&hcd->self, wIndex);
+ return -ETIMEDOUT;
}
+ if (!port->slot_id) {
+ xhci_dbg(xhci, "slot_id is zero\n");
+ return -ENODEV;
+ }
+ xhci_ring_device(xhci, port->slot_id);
+
usb_hcd_end_port_resume(&hcd->self, wIndex);
bus_state->port_c_suspend |= 1 << wIndex;
bus_state->suspended_ports &= ~(1 << wIndex);
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* [PATCH V2] thunderbolt: fix bandwidth group reservation indexing
From: raoxu @ 2026-06-23 13:37 UTC (permalink / raw)
To: andreas.noever; +Cc: westeri, YehezkelShB, linux-usb, linux-kernel, raoxu
From: Xu Rao <raoxu@uniontech.com>
Group ID 0 is reserved, while valid bandwidth groups use IDs 1 through
7. MAX_GROUPS is used both for tb_cm::groups, which stores allocatable
bandwidth groups, and for group_reserved[], which is indexed directly
by Group ID.
tb_init_bandwidth_groups() assigns i + 1 to each entry in
tb_cm::groups. Keeping seven entries therefore creates exactly the valid
Group IDs 1 through 7.
However, group_reserved[MAX_GROUPS] currently also has seven entries,
providing indices 0 through 6. When a tunnel belongs to Group ID 7,
tb_consumed_dp_bandwidth() reads and may write one element past the end
of the array. The reservation for that group is consequently not
included in the consumed bandwidth total either.
Define MAX_GROUPS as 7 + 1 so arrays indexed directly by Group ID cover
the complete 0 through 7 range, including the reserved ID 0. Size
tb_cm::groups as MAX_GROUPS - 1 so only seven bandwidth group objects
are initialized and tb_init_bandwidth_groups() continues to assign IDs
1 through 7. tb_consumed_dp_bandwidth() can then retain the direct
Group ID indexing, with Group ID 7 selecting the final valid array
element instead of accessing beyond it.
Fixes: 52a4490e89d7 ("thunderbolt: Reserve released DisplayPort bandwidth for a group for 10 seconds")
Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
Changes in v2:
- Drop the zero-based Group ID conversion used in v1 and keep
group->index as the direct group_reserved[] index.
- Include the reserved Group ID 0 in MAX_GROUPS so direct-indexed arrays
cover the complete Group ID range 0 through 7.
- Size tb_cm::groups as MAX_GROUPS - 1 so
tb_init_bandwidth_groups() continues to create only the seven valid
groups with IDs 1 through 7.
drivers/thunderbolt/tb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 76323255439a..51f909db9383 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -41,7 +41,7 @@
*/
#define TB_ASYM_THRESHOLD 45000
-#define MAX_GROUPS 7 /* max Group_ID is 7 */
+#define MAX_GROUPS (7 + 1) /* Group ID 0 is reserved */
static unsigned int asym_threshold = TB_ASYM_THRESHOLD;
module_param_named(asym_threshold, asym_threshold, uint, 0444);
@@ -66,7 +66,7 @@ struct tb_cm {
struct list_head dp_resources;
bool hotplug_active;
struct delayed_work remove_work;
- struct tb_bandwidth_group groups[MAX_GROUPS];
+ struct tb_bandwidth_group groups[MAX_GROUPS - 1];
};
static inline struct tb *tcm_to_tb(struct tb_cm *tcm)
--
2.50.1
^ permalink raw reply related
* Re: [PATCH] usbcore: Add quirk for 255-bytes initial config read
From: Alan Stern @ 2026-06-23 13:17 UTC (permalink / raw)
To: Nikhil Solanke; +Cc: linux-usb, gregkh, linux-kernel, michal.pecio, stable
In-Reply-To: <CAFgddhLVAp7nMX4YUHoaG+Q_Hm6w1uq9df2kH+4RWiJJGYDdhw@mail.gmail.com>
On Tue, Jun 23, 2026 at 01:01:12PM +0530, Nikhil Solanke wrote:
> I have a v2 patch ready with all the requested changes along with the
> documentation in Documentation/admin-guide/kernel-parameters.txt. Is
> there any other place where I have to write documentation?
No.
> Also, I would like to know the "low-level" type reason as to why do we
> have 2 separate buffers. a desc and then a bigbuffer? Why don't we
> just realloc the desc buffer? Does this have something to do with
> reallocs in general?
No particular reason. I guess it just seemed more straightforward to
allocate a second, larger buffer than to expand the first, smaller
buffer.
> Also (a bit more tangent), can the usb device potentially fingerprint
> the host os? if we are asking for 9 bytes first and windows ask for
> 255, is it possible that some usb devices will fingerprint the OS
> based on this, and behave differently? are there any other such places
> where fingerprinting is possible? In those cases, is it theoretically
> possible that this patch might fix some weird devices that "seem to
> work" on windows but not on linux?
Of course it's possible. Devices can do whatever they want, as long as
they obey the requirements of the USB spec.
> I might just add this one line to
> documentation that it might theoretically fix other usb devices as
> well instead of it just being a quirk to fix a game controller.
Isn't that true of any quirk? As a theoretical possibility, I mean.
Alan Stern
^ permalink raw reply
* Re: [RFC PATCH] thunderbolt: icm: report switch runtime resume timeout
From: Mika Westerberg @ 2026-06-23 12:36 UTC (permalink / raw)
To: Pengpeng Hou
Cc: Andreas Noever, Mika Westerberg, Yehezkel Bernat, linux-usb,
linux-kernel
In-Reply-To: <20260623061811.62475-1-pengpeng@iscas.ac.cn>
Hi,
On Tue, Jun 23, 2026 at 02:18:11PM +0800, Pengpeng Hou wrote:
> icm_runtime_resume_switch() waits for firmware to signal switch runtime-
> resume completion. If the wait expires, it only prints a debug message
> and still returns success to the runtime PM path.
IIRC that's intentional. Otherwise it would have logged it as warn or so.
This can happen if the device is removed while we were runtime suspended.
Do you see any problem with this?
^ permalink raw reply
* Re: [PATCH] USB: misc: iowarrior: fix use-after-free in release
From: Sam Sun @ 2026-06-23 12:28 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, Oliver Neukum, linux-usb, linux-kernel
In-Reply-To: <ajlZby4OXzCKECmJ@hovoldconsulting.com>
On Mon, Jun 22, 2026 at 11:49 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Fri, Jun 19, 2026 at 11:03:37PM +0800, Yue Sun wrote:
> > From: Sam Sun <samsun1006219@gmail.com>
> >
> > iowarrior_release() clears dev->opened while holding dev->mutex and then
> > unlocks the mutex before freeing the device when it has already been
> > disconnected. If iowarrior_disconnect() is waiting for the same mutex, it
> > can acquire it while mutex_unlock() in release is still in progress,
> > observe dev->opened == 0, and free the same object.
>
> The above description is not correct as it seems to suggests that there
> is a double-free here, which there is not.
>
> Either release() or disconnect() will free the driver data, but the
> other one may still be executing mutex_unlock() when that happens.
>
> > This violates the
> > mutex_unlock() lifetime rule because the mutex storage can be freed before
> > mutex_unlock() has returned.
>
> Indeed. Thanks for catching this.
>
> > Fix this by adding a kref to struct iowarrior and holding one reference
> > for the USB interface, one for each opened file, and one for each
> > in-flight asynchronous write URB. Release, disconnect and write completion
> > now only drop their references after they are done using the object, and
> > the object is freed only after all users have released it.
>
> You only need a reference for each open file (and one while the driver
> is bound). All I/O must cease on disconnect so there is no need to keep
> an additional reference for each outstanding write.
>
> Note that there is another bug here for which the fix has not yet been
> applied though:
>
> https://lore.kernel.org/all/20260523170523.1074563-1-johan@kernel.org/
>
> > Reported-by: Sam Sun <samsun1006219@gmail.com>
>
> You shouldn't add a reported-by tag for issues you fix yourself. A Link
> to the syzbot report is good to have though.
>
> > Closes: https://lore.kernel.org/r/20260618080204.38322-1-samsun1006219@gmail.com
>
> I only saw your report last week and missed that you had posted this fix
> until today when I was about to submit a series fixing this driver and a
> few others that got this wrong:
>
> https://lore.kernel.org/all/20260622152612.116422-1-johan@kernel.org/
>
> With the commit message fixed and the write references dropped they are
> mostly equivalent.
>
Thanks for taking a look and for the detailed explanation!
I agree that the per-write URB references are unnecessary once the
outstanding write URBs are killed unconditionally on disconnect. Your
series is a better and smaller fix.
I'll drop my version in favour of your series.
Best regards,
Yue
> > Signed-off-by: Sam Sun <samsun1006219@gmail.com>
> > ---
>
> > @@ -95,6 +97,10 @@ struct iowarrior {
> > struct usb_anchor submitted;
> > };
> >
> > +#define to_iowarrior_dev(d) container_of(d, struct iowarrior, kref)
>
> I know we have a few of these in the tree already, but it's not really
> needed for a single user and the "d" is a bit misleading as these macros
> are typically used to cast from a struct device (not a kref).
>
> > +
> > +static void iowarrior_delete(struct kref *kref);
> > +
> > /*--------------*/
> > /* globals */
> > /*--------------*/
> > @@ -235,13 +241,16 @@ static void iowarrior_write_callback(struct urb *urb)
> > /* tell a waiting writer the interrupt-out-pipe is available again */
> > atomic_dec(&dev->write_busy);
> > wake_up_interruptible(&dev->write_wait);
> > + kref_put(&dev->kref, iowarrior_delete);
> > }
>
> > @@ -454,12 +463,14 @@ static ssize_t iowarrior_write(struct file *file,
> > goto error;
> > }
> > usb_anchor_urb(int_out_urb, &dev->submitted);
> > + kref_get(&dev->kref);
> > retval = usb_submit_urb(int_out_urb, GFP_KERNEL);
> > if (retval) {
> > dev_dbg(&dev->interface->dev,
> > "submit error %d for urb nr.%d\n",
> > retval, atomic_read(&dev->write_busy));
> > usb_unanchor_urb(int_out_urb);
> > + kref_put(&dev->kref, iowarrior_delete);
> > goto error;
> > }
> > /* submit was ok */
>
> All outstanding writes should be stopped on disconnect (and soon will be
> when the fix I mentioned above is merged), so this is not needed.
>
> > @@ -637,6 +648,7 @@ static int iowarrior_open(struct inode *inode, struct file *file)
> > }
> > /* increment our usage count for the driver */
> > ++dev->opened;
> > + kref_get(&dev->kref);
> > /* save our object in the file's private structure */
> > file->private_data = dev;
> > retval = 0;
> > @@ -657,13 +669,13 @@ static int iowarrior_release(struct inode *inode, struct file *file)
> > dev = file->private_data;
> > if (!dev)
> > return -ENODEV;
> > + file->private_data = NULL;
>
> This looks like an unrelated change.
>
> > /* lock our device */
> > mutex_lock(&dev->mutex);
> >
> > if (dev->opened <= 0) {
> > retval = -ENODEV; /* close called more than once */
> > - mutex_unlock(&dev->mutex);
> > } else {
> > dev->opened = 0; /* we're closing now */
> > retval = 0;
>
> > @@ -918,8 +929,8 @@ static void iowarrior_disconnect(struct usb_interface *interface)
> > } else {
> > /* no process is using the device, cleanup now */
> > mutex_unlock(&dev->mutex);
> > - iowarrior_delete(dev);
> > }
>
> The unlock should now be moved after the conditional.
>
> > + kref_put(&dev->kref, iowarrior_delete);
> > }
> >
> > /* usb specific object needed to register this driver with the usb subsystem */
>
> Johan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox