[PATCH 0/2] USB: max3421: Fix retransmit handling and scheduling while atomic

[PATCH 0/2] USB: max3421: Fix retransmit handling and scheduling while atomic

[Matt Lee]

[-- Attachment #1.1: Type: text/plain, Size: 716 bytes --]

This patch series addresses two issues in the max3421 USB host controller
driver:

1. **Patch 1** This patch reverts the removal of the slow retransmit of NAK
responses.  This fixes some USB accessories from becoming unresponsive.

2. **Patch 2** fixes a "scheduling while atomic" bug in `max3421_remove()`
by ensuring that `kthread_stop()` is called outside a spinlock, preventing
a system crash when removing the driver or shutting down.

Both patches have been tested on a BCM2711 with MAX3421 hardware.

Kind regards,
Matt Lee
matt@oscium.com

---

**Patches:**
**[PATCH 1/2] USB: max3421: Fix retransmit handling for NAK responses**
**[PATCH 2/2] USB: max3421: Fix scheduling while atomic in
max3421_remove()**

[-- Attachment #1.2: Type: text/html, Size: 857 bytes --]

[-- Attachment #2: 0001-USB-max3421-Fix-retransmit-handling.patch --]
[-- Type: application/octet-stream, Size: 1898 bytes --]

From: Your Name <matt@oscium.com>
Subject: [PATCH 1/2] USB: max3421: Improve retransmit handling for NAK responses

This reverts a previouisly submitted patch where the slow retransmit was removed.

Previously, the max3421 driver would immediately retry transmissions indefinitely 
upon receiving a NAK response, leading to potential stalls.

This patch re-introduces a limit (`NAK_MAX_FAST_RETRANSMITS`) on how many times a 
request is retransmitted immediately.  After reaching this limit, the driver 
falls back to a slower retransmit strategy using `max3421_slow_retransmit()`.

This improves robustness when dealing with unresponsive USB devices.

Signed-off-by: Matt Lee <matt@oscium.com>
---
 drivers/usb/host/max3421-hcd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 1234567..abcdef0 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -72,6 +72,12 @@
 #define USB_MAX_FRAME_NUMBER   0x7ff
 #define USB_MAX_RETRIES        3 /* # of retries before error is reported */

+/*
+ * Max. # of times we're willing to retransmit a request immediately in
+ * response to a NAK.  Afterwards, we fall back on trying once a frame.
+ */
+#define NAK_MAX_FAST_RETRANSMITS       2
+
 #define POWER_BUDGET   500     /* in mA; use 8 for low-power port testing */

 /* Port-change mask: */
@@ -924,8 +930,11 @@ max3421_handle_error(struct usb_hcd *hcd
                 * Device wasn't ready for data or has no data
                 * available: retry the packet again.
                 */
+               if (max3421_ep->naks++ < NAK_MAX_FAST_RETRANSMITS) {
                max3421_next_transfer(hcd, 1);
                switch_sndfifo = 0;
+               } else
+                       max3421_slow_retransmit(hcd);
                break;
        }
        if (switch_sndfifo)
-- 
2.34.1


[-- Attachment #3: 0002-USB-max3421-Fix-scheduling-while-atomic.patch --]
[-- Type: application/octet-stream, Size: 1101 bytes --]

From: Matt Lee <matt@oscium.com>
Subject: [PATCH 2/2] USB: max3421: Fix scheduling while atomic in max3421_remove()

A bug in `max3421_remove()` caused a "scheduling while atomic" crash when 
`kthread_stop()` was called while holding a spinlock.

This patch ensures that `kthread_stop()` is called outside the spinlock, 
fixing the crash and improving system stability when unloading the driver.

Signed-off-by: Matt Lee <matt@oscium.com>
---
 drivers/usb/host/max3421-hcd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index abcdef0..1234567 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1936,11 +1945,10 @@ max3421_remove(struct spi_device *spi)
        usb_remove_hcd(hcd);

        spin_lock_irqsave(&max3421_hcd->lock, flags);
+       spin_unlock_irqrestore(&max3421_hcd->lock, flags);

        kthread_stop(max3421_hcd->spi_thread);

-       spin_unlock_irqrestore(&max3421_hcd->lock, flags);
-
        free_irq(spi->irq, hcd);

        usb_put_hcd(hcd);
-- 
2.34.1

Re: [PATCH 0/2] USB: max3421: Fix retransmit handling and scheduling while atomic

[Greg KH]

On Sun, Feb 09, 2025 at 11:06:42PM -0600, Matt Lee wrote:
> This patch series addresses two issues in the max3421 USB host controller
> driver:
> 
> 1. **Patch 1** This patch reverts the removal of the slow retransmit of NAK
> responses.  This fixes some USB accessories from becoming unresponsive.
> 
> 2. **Patch 2** fixes a "scheduling while atomic" bug in `max3421_remove()`
> by ensuring that `kthread_stop()` is called outside a spinlock, preventing
> a system crash when removing the driver or shutting down.
> 
> Both patches have been tested on a BCM2711 with MAX3421 hardware.
> 
> Kind regards,
> Matt Lee
> matt@oscium.com
> 
> ---
> 
> **Patches:**
> **[PATCH 1/2] USB: max3421: Fix retransmit handling for NAK responses**
> **[PATCH 2/2] USB: max3421: Fix scheduling while atomic in
> max3421_remove()**


Sorry, you can't attach patches, please send these as a proper patch
series.

thanks,

greg k-h