[PATCH 0/2] Fix CPPI Warnings during tear down after ISOCH transfers

[PATCH 0/2] Fix CPPI Warnings during tear down after ISOCH transfers

[George Cherian]

Warinings are seen after  ISOCH transfers, during channel tear down.
This is mainly beacause we handle ISOCH differently as compared to 
other transfers. 

Patch 1: make sure we do channel tear down only if channel is busy.
	 If not the tear down will never succeed.

Patch 2: ISOCH completions are done differently, so this might lead to 
	reprogram of dma channel on which already a teardown is done.
	

George Cherian (2):
  dma: cppi41: start tear down only if channel is busy
  usb: musb: musb_cppi41: Dont reprogram DMA if tear down is initiated

 drivers/dma/cppi41.c           | 7 +++++--
 drivers/usb/musb/musb_cppi41.c | 3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)

-- 
1.8.1

[PATCH 1/2] dma: cppi41: start tear down only if channel is busy

[George Cherian]

Start the channel tear down only if the channel is busy, else just
bail out. In some cases its seen that by the time the tear down is
initiated the cppi completes the DMA, especially in ISOCH transfers.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/dma/cppi41.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index c18aebf..d028f36 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -620,12 +620,15 @@ static int cppi41_stop_chan(struct dma_chan *chan)
 	u32 desc_phys;
 	int ret;
 
+	desc_phys = lower_32_bits(c->desc_phys);
+	desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
+	if (!cdd->chan_busy[desc_num])
+		return 0;
+
 	ret = cppi41_tear_down_chan(c);
 	if (ret)
 		return ret;
 
-	desc_phys = lower_32_bits(c->desc_phys);
-	desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
 	WARN_ON(!cdd->chan_busy[desc_num]);
 	cdd->chan_busy[desc_num] = NULL;
 
-- 
1.8.1

Re: [PATCH 1/2] dma: cppi41: start tear down only if channel is busy

[Shevchenko, Andriy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1843 bytes --]

On Thu, 2014-02-27 at 10:44 +0530, George Cherian wrote:
> Start the channel tear down only if the channel is busy, else just
> bail out. In some cases its seen that by the time the tear down is
> initiated the cppi completes the DMA, especially in ISOCH transfers.
> 
> Signed-off-by: George Cherian <george.cherian@ti.com>
> ---
>  drivers/dma/cppi41.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> index c18aebf..d028f36 100644
> --- a/drivers/dma/cppi41.c
> +++ b/drivers/dma/cppi41.c
> @@ -620,12 +620,15 @@ static int cppi41_stop_chan(struct dma_chan *chan)
>  	u32 desc_phys;
>  	int ret;
>  
> +	desc_phys = lower_32_bits(c->desc_phys);
> +	desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
> +	if (!cdd->chan_busy[desc_num])
> +		return 0;
> +
>  	ret = cppi41_tear_down_chan(c);
>  	if (ret)
>  		return ret;
>  
> -	desc_phys = lower_32_bits(c->desc_phys);
> -	desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
>  	WARN_ON(!cdd->chan_busy[desc_num]);

Do you still need this WARN_ON?

>  	cdd->chan_busy[desc_num] = NULL;
>  


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 1/2] dma: cppi41: start tear down only if channel is busy

[George Cherian]

On 2/27/2014 2:17 PM, Shevchenko, Andriy wrote:
> On Thu, 2014-02-27 at 10:44 +0530, George Cherian wrote:
>> Start the channel tear down only if the channel is busy, else just
>> bail out. In some cases its seen that by the time the tear down is
>> initiated the cppi completes the DMA, especially in ISOCH transfers.
>>
>> Signed-off-by: George Cherian <george.cherian@ti.com>
>> ---
>>   drivers/dma/cppi41.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
>> index c18aebf..d028f36 100644
>> --- a/drivers/dma/cppi41.c
>> +++ b/drivers/dma/cppi41.c
>> @@ -620,12 +620,15 @@ static int cppi41_stop_chan(struct dma_chan *chan)
>>   	u32 desc_phys;
>>   	int ret;
>>   
>> +	desc_phys = lower_32_bits(c->desc_phys);
>> +	desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
>> +	if (!cdd->chan_busy[desc_num])
>> +		return 0;
>> +
>>   	ret = cppi41_tear_down_chan(c);
>>   	if (ret)
>>   		return ret;
>>   
>> -	desc_phys = lower_32_bits(c->desc_phys);
>> -	desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
>>   	WARN_ON(!cdd->chan_busy[desc_num]);
> Do you still need this WARN_ON?

Yes in some cases wherein the channel is busy and tear down didn't 
happen successfully.
>
>>   	cdd->chan_busy[desc_num] = NULL;
>>   
>


-- 
-George

[PATCH 2/2] usb: musb: musb_cppi41: Dont reprogram DMA if tear down is initiated

[George Cherian]

Reprogramming the DMA after tear down is initiated leads to warning.
This is mainly seen with ISOCH since we do a delayed completion for
ISOCH transfers. In ISOCH transfers dma_completion should not reprogram
if the channel tear down is initiated.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/musb/musb_cppi41.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index f3ec7d2..e201b1e 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -132,7 +132,8 @@ static void cppi41_trans_done(struct cppi41_dma_channel *cppi41_channel)
 	struct musb_hw_ep *hw_ep = cppi41_channel->hw_ep;
 	struct musb *musb = hw_ep->musb;
 
-	if (!cppi41_channel->prog_len) {
+	if (!cppi41_channel->prog_len ||
+	    (cppi41_channel->channel.status == MUSB_DMA_STATUS_FREE)) {
 
 		/* done, complete */
 		cppi41_channel->channel.actual_len =
-- 
1.8.1

Re: [PATCH 0/2] Fix CPPI Warnings during tear down after ISOCH transfers

[Vinod Koul]

On Thu, Feb 27, 2014 at 10:44:39AM +0530, George Cherian wrote:
> Warinings are seen after  ISOCH transfers, during channel tear down.
> This is mainly beacause we handle ISOCH differently as compared to 
> other transfers. 
> 
> Patch 1: make sure we do channel tear down only if channel is busy.
> 	 If not the tear down will never succeed.
> 
> Patch 2: ISOCH completions are done differently, so this might lead to 
> 	reprogram of dma channel on which already a teardown is done.
Applied, Thanks

-- 
~Vinod