Skip to contentSkip to conversation

Two cpp41 pm runtime found when testing with usb

PATCH In review Tony Lindgren 2 participants 7 messages Nov 9, 2016 – Nov 15, 2016 2 messages not in the archive dmaengine

lore
Share & tools
lore
#0 lore raw

cover Two cpp41 pm runtime found when testing with usb

Hi,

I found two pm runtime issues when testing with usb on beaglebone.

In the am335x case cppi41 and two instances of musb controller share the same interconnect wrapper module, so any pm issues with musb or cppi41 will keep the interconnect wrapper module busy.

Regards,

Tony

Tony Lindgren (2):
dma: cppi41: Fix list not empty warning on module removal
dma: cppi41: Fix unpaired pm runtime when only a USB hub is connected

 drivers/dma/cppi41.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
Signature · 5 lines
2.10.2
--
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): 20161109164759.24913-1-tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org

#2 lore raw

PATCH 1/2 dma: cppi41: Fix list not empty warning on module removal

If musb controller is configured with USB peripherals and we have enumerated with a USB host, we can get warnings on removal of the modules:

WARNING: CPU: 0 PID: 1269 at drivers/dma/cppi41.c:391

cppi41_dma_free_chan_resources

Fix the issue by adding the missing pm_runtime_get to cppi41_dma_free_chan_resources to make sure the pending work list is cleared on removal.

Fixes fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
---
 drivers/dma/cppi41.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/dma/cppi41.c +5 -0
@@ -1072,7 +1072,12 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 static int cppi41_dma_remove(struct platform_device *pdev)
 {
 	struct cppi41_dd *cdd = platform_get_drvdata(pdev);
+	int error;
 
+	error = pm_runtime_get_sync(&pdev->dev);
+	if (error < 0)
+		dev_err(&pdev->dev, "%s could not pm_runtime_get: %i\n",
+			__func__, error);
 	of_dma_controller_free(pdev->dev.of_node);
 	dma_async_device_unregister(&cdd->ddev);
 
Signature · 5 lines
2.10.2
--
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
#3 lore raw

PATCH 2/2 dma: cppi41: Fix unpaired pm runtime when only a USB hub is connected

In reply to a message not in the archive

On am335x with musb host we can end up with unpaired pm runtime calls if a hub with no devices is connected and disconnected.

This is because of the conditional pm runtime calls which are always a bad idea. Let's fix the issue by making them unconditional and paired in each function.

Fixes fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
---
 drivers/dma/cppi41.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/dma/cppi41.c +8 -2
@@ -318,6 +318,11 @@ static irqreturn_t cppi41_irq(int irq, void *data)
 		while (val) {
 			u32 desc, len;
 
+			status = pm_runtime_get(cdd->ddev.dev);
+			if (status < 0)
+				dev_err(cdd->ddev.dev, "%s pm runtime get: %i\n",
+					__func__, status);
+
 			q_num = __fls(val);
 			val &= ~(1 << q_num);
 			q_num += 32 * i;
@@ -338,7 +343,6 @@ static irqreturn_t cppi41_irq(int irq, void *data)
 			dma_cookie_complete(&c->txd);
 			dmaengine_desc_get_callback_invoke(&c->txd, NULL);
 
-			/* Paired with cppi41_dma_issue_pending */
 			pm_runtime_mark_last_busy(cdd->ddev.dev);
 			pm_runtime_put_autosuspend(cdd->ddev.dev);
 		}
@@ -460,7 +464,6 @@ static void cppi41_dma_issue_pending(struct dma_chan *chan)
 	struct cppi41_dd *cdd = c->cdd;
 	int error;
 
-	/* PM runtime paired with dmaengine_desc_get_callback_invoke */
 	error = pm_runtime_get(cdd->ddev.dev);
 	if ((error != -EINPROGRESS) && error < 0) {
 		dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
@@ -473,6 +476,9 @@ static void cppi41_dma_issue_pending(struct dma_chan *chan)
 		push_desc_queue(c);
 	else
 		pending_desc(c);
+
+	pm_runtime_mark_last_busy(cdd->ddev.dev);
+	pm_runtime_put_autosuspend(cdd->ddev.dev);
 }
 
 static u32 get_host_pd0(u32 length)
Signature · 5 lines
2.10.2
--
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
#4 lore raw
In reply to a message not in the archive
Quoted 7 lines · Tony Lindgren

Hi,

I found two pm runtime issues when testing with usb on beaglebone.

In the am335x case cppi41 and two instances of musb controller share the same interconnect wrapper module, so any pm issues with musb or cppi41 will keep the interconnect wrapper module busy.

Applied both. And as I have told you previously please use the correct subsystem tag. I have fixed them again!

Signature · 5 lines
~Vinod
--
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
#5 lore raw
Quoted 11 lines · Vinod Koul
Quoted 7 lines · Tony Lindgren

Hi,

I found two pm runtime issues when testing with usb on beaglebone.

In the am335x case cppi41 and two instances of musb controller share the same interconnect wrapper module, so any pm issues with musb or cppi41 will keep the interconnect wrapper module busy.

Applied both. And as I have told you previously please use the correct subsystem tag. I have fixed them again!

Sorry about that. What do you prefer for future reference? We are using both "dma: cppi41" and "dmaengine: cppi41" currently..

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw
Quoted 10 lines · Tony Lindgren
Quoted 9 lines · Vinod Koul
Quoted 7 lines · Tony Lindgren

Hi,

I found two pm runtime issues when testing with usb on beaglebone.

In the am335x case cppi41 and two instances of musb controller share the same interconnect wrapper module, so any pm issues with musb or cppi41 will keep the interconnect wrapper module busy.

Quoted 5 lines
Quoted 2 lines

Applied both. And as I have told you previously please use the correct subsystem tag. I have fixed them again!

Sorry about that. What do you prefer for future reference? We are using both "dma: cppi41" and "dmaengine: cppi41" currently..

"dmaengine: cppi41: xxx" would be better

Signature · 5 lines
~Vinod
--
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
#8 lore raw
Quoted 11 lines · Vinod Koul
Quoted 10 lines · Tony Lindgren
Quoted 9 lines · Vinod Koul
Quoted 7 lines · Tony Lindgren

Hi,

I found two pm runtime issues when testing with usb on beaglebone.

In the am335x case cppi41 and two instances of musb controller share the same interconnect wrapper module, so any pm issues with musb or cppi41 will keep the interconnect wrapper module busy.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

Applied both. And as I have told you previously please use the correct subsystem tag. I have fixed them again!

Sorry about that. What do you prefer for future reference? We are using both "dma: cppi41" and "dmaengine: cppi41" currently..

"dmaengine: cppi41: xxx" would be better

OK will use that for future patches then.

Thanks,

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