[PATCH 1/2] USB: max3421: Improve retransmit handling for NAK responses

[PATCH 1/2] USB: max3421: Improve retransmit handling for NAK responses

[Matt Lee]

This reverts a previously 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

Re: [PATCH 1/2] USB: max3421: Improve retransmit handling for NAK responses

[Greg KH]

On Mon, Feb 10, 2025 at 07:57:20AM -0600, Matt Lee wrote:
> This reverts a previously submitted patch where the slow retransmit was removed.

What is that git commit id you are reverting?  Please list it and mark
the Fixes: and cc: stable tag as needed.

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

Odd line-wrapping :(

> 
> 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);

Did you run scripts/checkpatch.pl on this change?

And why is 2 ok?  How did that number get picked?

thanks,

greg k-h

Re: [PATCH 1/2] USB: max3421: Improve retransmit handling for NAK responses

[Greg KH]

On Mon, Feb 10, 2025 at 07:48:28AM -0600, Matt Lee wrote:
> This reverts a previously submitted patch where the slow retransmit was
> removed.
> 
> 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(+)

I got 2 copies of this, is that intentional?

And patch 2/2 should be threaded with this one, did you use git
send-email to send these out?

thanks,

greg k-h