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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 <[email protected]>
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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
>> ---
>> 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