[4.19] please include b65ba0c362be665192381cc59e3ac3ef6f0dd1e1

[4.19] please include b65ba0c362be665192381cc59e3ac3ef6f0dd1e1

[Fabian Godehardt]

Hi all,

please include b65ba0c362be665192381cc59e3ac3ef6f0dd1e1 also on the
stable-trees up to v5.10 (i think v5.13 was the first fixed tree).

Serial gadget on AM335X is also affected, breaks with NULL pointer
references and needs this patch. Here is the patch for the v4.19
tree, cherry picked and manually applied from original commit
b65ba0c362be665192381cc59e3ac3ef6f0dd1e1:

From 483d904168b08cf1497c73516c432bde9ae94055 Mon Sep 17 00:00:00 2001
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Date: Fri, 28 May 2021 16:04:46 +0200
Subject: [PATCH] usb: musb: fix MUSB_QUIRK_B_DISCONNECT_99 handling

In commit 92af4fc6ec33 ("usb: musb: Fix suspend with devices
connected for a64"), the logic to support the
MUSB_QUIRK_B_DISCONNECT_99 quirk was modified to only conditionally
schedule the musb->irq_work delayed work.

This commit badly breaks ECM Gadget on AM335X. Indeed, with this
commit, one can observe massive packet loss:

$ ping 192.168.0.100
...
15 packets transmitted, 3 received, 80% packet loss, time 14316ms

Reverting this commit brings back a properly functioning ECM
Gadget. An analysis of the commit seems to indicate that a mistake was
made: the previous code was not falling through into the
MUSB_QUIRK_B_INVALID_VBUS_91, but now it is, unless the condition is
taken.

Changing the logic to be as it was before the problematic commit *and*
only conditionally scheduling musb->irq_work resolves the regression:

$ ping 192.168.0.100
...
64 packets transmitted, 64 received, 0% packet loss, time 64475ms

Fixes: 92af4fc6ec33 ("usb: musb: Fix suspend with devices connected for a64")
Cc: stable@vger.kernel.org
Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Tested-by: Drew Fustini <drew@beagleboard.org>
Acked-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Link: https://lore.kernel.org/r/20210528140446.278076-1-thomas.petazzoni@bootlin.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/musb/musb_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 2a874058dff1..4d2de9ce03f9 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1873,9 +1873,8 @@ static void musb_pm_runtime_check_session(struct musb *musb)
 			schedule_delayed_work(&musb->irq_work,
 					      msecs_to_jiffies(1000));
 			musb->quirk_retries--;
-			break;
 		}
-		/* fall through */
+		break;
 	case MUSB_QUIRK_B_INVALID_VBUS_91:
 		if (musb->quirk_retries && !musb->flush_irq_work) {
 			musb_dbg(musb,
-- 
2.30.2

Re: [4.19] please include b65ba0c362be665192381cc59e3ac3ef6f0dd1e1

[Greg KH]

On Tue, Dec 19, 2023 at 08:56:41AM +0100, Fabian Godehardt wrote:
> Hi all,
> 
> please include b65ba0c362be665192381cc59e3ac3ef6f0dd1e1 also on the
> stable-trees up to v5.10 (i think v5.13 was the first fixed tree).
> 
> Serial gadget on AM335X is also affected, breaks with NULL pointer
> references and needs this patch. Here is the patch for the v4.19
> tree, cherry picked and manually applied from original commit
> b65ba0c362be665192381cc59e3ac3ef6f0dd1e1:
> 
> >From 483d904168b08cf1497c73516c432bde9ae94055 Mon Sep 17 00:00:00 2001
> From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Date: Fri, 28 May 2021 16:04:46 +0200
> Subject: [PATCH] usb: musb: fix MUSB_QUIRK_B_DISCONNECT_99 handling
> 
> In commit 92af4fc6ec33 ("usb: musb: Fix suspend with devices
> connected for a64"), the logic to support the
> MUSB_QUIRK_B_DISCONNECT_99 quirk was modified to only conditionally
> schedule the musb->irq_work delayed work.
> 
> This commit badly breaks ECM Gadget on AM335X. Indeed, with this
> commit, one can observe massive packet loss:
> 
> $ ping 192.168.0.100
> ...
> 15 packets transmitted, 3 received, 80% packet loss, time 14316ms
> 
> Reverting this commit brings back a properly functioning ECM
> Gadget. An analysis of the commit seems to indicate that a mistake was
> made: the previous code was not falling through into the
> MUSB_QUIRK_B_INVALID_VBUS_91, but now it is, unless the condition is
> taken.
> 
> Changing the logic to be as it was before the problematic commit *and*
> only conditionally scheduling musb->irq_work resolves the regression:
> 
> $ ping 192.168.0.100
> ...
> 64 packets transmitted, 64 received, 0% packet loss, time 64475ms
> 
> Fixes: 92af4fc6ec33 ("usb: musb: Fix suspend with devices connected for a64")
> Cc: stable@vger.kernel.org
> Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Tested-by: Drew Fustini <drew@beagleboard.org>
> Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Link: https://lore.kernel.org/r/20210528140446.278076-1-thomas.petazzoni@bootlin.com
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---

As you did the backport, you too need to sign off on this. Can you
resend this with that properly added?

thanks,

greg k-h

Re: [4.19] please include b65ba0c362be665192381cc59e3ac3ef6f0dd1e1

[Fabian Godehardt]

Hi,

> As you did the backport, you too need to sign off on this. Can you
> resend this with that properly added?

Sure:

From c88f647a482b7258b428c9ae567a9fce6fa95e96 Mon Sep 17 00:00:00 2001
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Date: Fri, 28 May 2021 16:04:46 +0200
Subject: [PATCH] usb: musb: fix MUSB_QUIRK_B_DISCONNECT_99 handling

In commit 92af4fc6ec33 ("usb: musb: Fix suspend with devices
connected for a64"), the logic to support the
MUSB_QUIRK_B_DISCONNECT_99 quirk was modified to only conditionally
schedule the musb->irq_work delayed work.

This commit badly breaks ECM Gadget on AM335X. Indeed, with this
commit, one can observe massive packet loss:

$ ping 192.168.0.100
...
15 packets transmitted, 3 received, 80% packet loss, time 14316ms

Reverting this commit brings back a properly functioning ECM
Gadget. An analysis of the commit seems to indicate that a mistake was
made: the previous code was not falling through into the
MUSB_QUIRK_B_INVALID_VBUS_91, but now it is, unless the condition is
taken.

Changing the logic to be as it was before the problematic commit *and*
only conditionally scheduling musb->irq_work resolves the regression:

$ ping 192.168.0.100
...
64 packets transmitted, 64 received, 0% packet loss, time 64475ms

Fixes: 92af4fc6ec33 ("usb: musb: Fix suspend with devices connected for a64")
Cc: stable@vger.kernel.org
Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Tested-by: Drew Fustini <drew@beagleboard.org>
Acked-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Link: https://lore.kernel.org/r/20210528140446.278076-1-thomas.petazzoni@bootlin.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Fabian Godehardt <fg@emlix.com>
---
 drivers/usb/musb/musb_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 2a874058dff1..4d2de9ce03f9 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1873,9 +1873,8 @@ static void musb_pm_runtime_check_session(struct musb *musb)
 			schedule_delayed_work(&musb->irq_work,
 					      msecs_to_jiffies(1000));
 			musb->quirk_retries--;
-			break;
 		}
-		/* fall through */
+		break;
 	case MUSB_QUIRK_B_INVALID_VBUS_91:
 		if (musb->quirk_retries && !musb->flush_irq_work) {
 			musb_dbg(musb,
-- 
2.30.2