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

[Greg KH <gregkh@linuxfoundation.org>]

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