Skip to contentSkip to conversation

Two musb fixes for v4.10-rc cycle

PATCH Superseded Tony Lindgren 1 participant 4 messages Jan 19, 2017 2 messages not in the archive

Subsystems (2)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

cover Two musb fixes for v4.10-rc cycle

Hi all,

Here are two fixes for v4.10-rc cycle to deal with error -75 and -115 issues when plugging in USB mass storage device to a hub.

Note that I will also post two cppi41 dma related patches, but those can be applied separately.

Regards,

Tony

Tony Lindgren (2):
usb: musb: Fix host mode error -71 regression
usb: musb: Tiny dma in transfers won't complete with cpp41

 drivers/usb/musb/musb_core.c | 15 ++-------------
 drivers/usb/musb/musb_core.h |  1 -
 drivers/usb/musb/musb_host.c |  9 ++++++++-
 3 files changed, 10 insertions(+), 15 deletions(-)
Signature · 5 lines
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20170119004918.9986-1-tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org

#2 lore raw

PATCH 1/2 usb: musb: Fix host mode error -71 regression

Commit 467d5c980709 ("usb: musb: Implement session bit based runtime PM for musb-core") started implementing musb generic runtime PM support by introducing devctl register session bit based state control.

This caused a regression where if a USB mass storage device is connected to a USB hub, we can get:

usb 1-1: reset high-speed USB device number 2 using musb-hdrc usb 1-1: device descriptor read/64, error -71
usb 1-1.1: new high-speed USB device number 4 using musb-hdrc

This is because before the USB storage device is connected, musb is in OTG_STATE_A_SUSPEND. And we currently only set need_finish_resume in musb_stage0_irq() and the related code calling finish_resume_work in musb_resume() and musb_runtime_resume() never gets called.

To fix the issue, we can call schedule_delayed_work() directly in musb_stage0_irq() to have finish_resume_work run.

And we should no longer never get interrupts when when suspended. We have changed musb to no longer need pm_runtime_irqsafe(). The need_finish_resume flag was added in commit 9298b4aad37e ("usb:
musb: fix device hotplug behind hub") and no longer applies as far as I can tell. So let's just remove the earlier code that no longer is needed.

Fixes 467d5c980709 ("usb: musb: Implement session bit based

runtime PM for musb-core")

---
 drivers/usb/musb/musb_core.c | 15 ++-------------
 drivers/usb/musb/musb_core.h |  1 -
 2 files changed, 2 insertions(+), 14 deletions(-)
drivers/usb/musb/musb_core.c +2 -12
@@ -594,11 +594,11 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 						| MUSB_PORT_STAT_RESUME;
 				musb->rh_timer = jiffies
 					+ msecs_to_jiffies(USB_RESUME_TIMEOUT);
-				musb->need_finish_resume = 1;
-
 				musb->xceiv->otg->state = OTG_STATE_A_HOST;
 				musb->is_active = 1;
 				musb_host_resume_root_hub(musb);
+				schedule_delayed_work(&musb->finish_resume_work,
+					msecs_to_jiffies(USB_RESUME_TIMEOUT));
 				break;
 			case OTG_STATE_B_WAIT_ACON:
 				musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
@@ -2710,11 +2710,6 @@ static int musb_resume(struct device *dev)
 	mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV;
 	if ((devctl & mask) != (musb->context.devctl & mask))
 		musb->port1_status = 0;
-	if (musb->need_finish_resume) {
-		musb->need_finish_resume = 0;
-		schedule_delayed_work(&musb->finish_resume_work,
-				      msecs_to_jiffies(USB_RESUME_TIMEOUT));
-	}
 
 	/*
 	 * The USB HUB code expects the device to be in RPM_ACTIVE once it came
@@ -2766,12 +2761,6 @@ static int musb_runtime_resume(struct device *dev)
 
 	musb_restore_context(musb);
 
-	if (musb->need_finish_resume) {
-		musb->need_finish_resume = 0;
-		schedule_delayed_work(&musb->finish_resume_work,
-				msecs_to_jiffies(USB_RESUME_TIMEOUT));
-	}
#3 lore raw

PATCH 2/2 usb: musb: Tiny dma in transfers won't complete with cpp41

In reply to a message not in the archive

At least with the cppi41 dma, size 1 in dma transfers will just wait until the device is disconnected. And it also seems that enumerating a USB stick with a hub can take a USB reset with smallish size in transfers.

This causes timeouts in cppi41 dma runtime PM.

Fix the issue by adding a minimum size of 16 which is based on my obeservations on BeagleBone to make enumerating more reliable.

Fixes fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
---
 drivers/usb/musb/musb_host.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/usb/musb/musb_host.c +8 -1
@@ -743,7 +743,14 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
 
 	musb_ep_select(mbase, epnum);
 
-	if (is_out && !len) {
+	/*
+	 * Skip dma for zero sized out and small in transfers.
+	 * At least cppi41 in dma will just hang with size of 1 until the
+	 * device is connected. For sizes 8 and less it seems to take a
+	 * long time to complete. Let's use minimum size of 16 to avoid
+	 * tiny in DMA transfers.
+	 */
+	if ((is_out && !len) || (!is_out && (len < 16))) {
 		use_dma = 0;
 		csr = musb_readw(epio, MUSB_TXCSR);
 		csr &= ~MUSB_TXCSR_DMAENAB;
Signature · 5 lines
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20170119004918.9986-3-tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org

#5 lore raw

Re: [PATCH 2/2] usb: musb: Tiny dma in transfers won't complete with cpp41

Quoted 25 lines · Tony Lindgren

At least with the cppi41 dma, size 1 in dma transfers will just wait until the device is disconnected. And it also seems that enumerating a USB stick with a hub can take a USB reset with smallish size in transfers.

This causes timeouts in cppi41 dma runtime PM.

Fix the issue by adding a minimum size of 16 which is based on my obeservations on BeagleBone to make enumerating more reliable.

Fixes fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
---
 drivers/usb/musb/musb_host.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/usb/musb/musb_host.c +1 -1
@@ -743,7 +743,14 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
 
 	musb_ep_select(mbase, epnum);
 
-	if (is_out && !len) {
+	/*

drivers/usb/musb/musb_host.c static void musb_ep_program(struct musb *musb, u8 epnum, · line 756

Quoted 10 lines
drivers/usb/musb/musb_host.c +7 -0
+	 * Skip dma for zero sized out and small in transfers.
+	 * At least cppi41 in dma will just hang with size of 1 until the
+	 * device is connected. For sizes 8 and less it seems to take a
+	 * long time to complete. Let's use minimum size of 16 to avoid
+	 * tiny in DMA transfers.
+	 */
+	if ((is_out && !len) || (!is_out && (len < 16))) {
 		use_dma = 0;
 		csr = musb_readw(epio, MUSB_TXCSR);
 		csr &= ~MUSB_TXCSR_DMAENAB;

Uh I think this needs a separate check to not mess with the TXCSR for in transfers.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html