[v2,2/2] dmaengine: cppi41: Ignore EINPROGRESS for PM runtime in interrupt handler

Message ID 20170109170337.6957-3-abailon@baylibre.com
State New
Headers show

Commit Message

Alexandre Bailon Jan. 9, 2017, 5:03 p.m.
We can occasionally get -EINPROGRESS for pm_runtime_get.
This is happening when an interrupt is fired before PM runtime had time
to update the PM state to RESUMED.
In that case, don't print any error.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

---
 drivers/dma/cppi41.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

-- 
2.10.2

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

Comments

Grygorii Strashko Jan. 9, 2017, 7:11 p.m. | #1
On 01/09/2017 12:39 PM, Tony Lindgren wrote:
> * Alexandre Bailon <abailon@baylibre.com> [170109 09:04]:

>> We can occasionally get -EINPROGRESS for pm_runtime_get.

>> This is happening when an interrupt is fired before PM runtime had time

>> to update the PM state to RESUMED.

>> In that case, don't print any error.

> 

> Hmm if the cppi41 interrupt fires, the device has resumed :)

> 

> I think we should have a cppi41.c specific flag that we can set

> at the end of cppi41_resume() instead of list_empty() or

> pm_runtime_active().

> 

> Regars,

> 

> Tony

> 

> 

>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

>> ---

>>  drivers/dma/cppi41.c | 5 ++---

>>  1 file changed, 2 insertions(+), 3 deletions(-)

>>

>> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c

>> index 025fee4..2306020 100644

>> --- a/drivers/dma/cppi41.c

>> +++ b/drivers/dma/cppi41.c

>> @@ -320,7 +320,7 @@ static irqreturn_t cppi41_irq(int irq, void *data)

>>  			int error;

>>  

>>  			error = pm_runtime_get(cdd->ddev.dev);

>> -			if (error < 0)

>> +			if ((error != -EINPROGRESS) && error < 0)

>>  				dev_err(cdd->ddev.dev, "%s pm runtime get: %i\n",

>>  					__func__, error);


Another question of curiosity :) -EINPROGRESS and pm_runtime_get() in general doesn't
guarantee that device is powered on and accessible immediately after this call, but
few lines below there are code which try to access registers 
	desc = cppi41_pop_desc(cdd, q_num);
Is it ok?

>>  

>> @@ -492,8 +492,7 @@ static void cppi41_dma_issue_pending(struct dma_chan *chan)

>>  		 */

>>  		if (error == -EINPROGRESS) {

>>  			spin_lock_irqsave(&cdd->lock, flags);

>> -			if (list_empty(&cdd->pending))

>> -				active = true;

>> +			active = !!list_empty(&cdd->pending);

>>  			spin_unlock_irqrestore(&cdd->lock, flags);

>>  		}

>>  	}

>> -- 

>> 2.10.2


> 


-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch hide | download patch | download mbox

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 025fee4..2306020 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -320,7 +320,7 @@  static irqreturn_t cppi41_irq(int irq, void *data)
 			int error;
 
 			error = pm_runtime_get(cdd->ddev.dev);
-			if (error < 0)
+			if ((error != -EINPROGRESS) && error < 0)
 				dev_err(cdd->ddev.dev, "%s pm runtime get: %i\n",
 					__func__, error);
 
@@ -492,8 +492,7 @@  static void cppi41_dma_issue_pending(struct dma_chan *chan)
 		 */
 		if (error == -EINPROGRESS) {
 			spin_lock_irqsave(&cdd->lock, flags);
-			if (list_empty(&cdd->pending))
-				active = true;
+			active = !!list_empty(&cdd->pending);
 			spin_unlock_irqrestore(&cdd->lock, flags);
 		}
 	}