[PATCH v2] ARM: pl330: Fix a race condition

[javi.merino@arm.com (Javi Merino)]

On 07/12/11 10:01, Javi Merino wrote:
> On 07/12/11 07:52, Kukjin Kim wrote:
>> Jassi Brar wrote:
>>>
>>> On 29 November 2011 15:23, Javi Merino <javi.merino@arm.com> wrote:
>>>>>>>>> On Samsung's Exynos4 platform, while testing audio playback with
>>>>>> i2s
>>>>>>>>> interface, the above change causes the playback to freeze. The
>>>>>>>>> _thrd_active(thrd) call always returns '1' and hence _start(thrd)
>>>>>> is
>>>>>>>>> not getting called.
>>>>>>>>
>>>>>>>> If _thrd_active(thrd) returns '1', that means there is an active
>>>>>>>> transfer still running or, if it has finished, you haven't called
>>>>>>>> pl330_update() to acknowledge that.  pl330_update() calls _start()
>>>>>> as
>>>>>>>> soon as it can.
>>>>>>>>
>>>>>>>> drivers/dma/pl330.c registers the irq handler in pl330_probe(), so
>>>>>>>> when
>>>>>>>> the transaction finishes, pl330_update() should clear it and call
>>>>>>>> _start().  If there is any outstanding transaction, it should start
>>>>>>>> straight away. If there isn't, it would mark the channel as free,
>>>>>> so
>>>>>>>> _thrd_active() should return '0'.  If _thrd_active() is still '1',
>>>>>> then
>>>>>>>> something has gone wrong in the way.
>>>>>>>>
>>>>>>>> Does this shed some light?
>>>>>>>>
>>>>>>>
>>>>>>> Your patch makes the memcpy operation on dmatest.c and net DMA be
>>>>>> frozen too
>>>>>>> as well as Samsung audio playback.
>>>>>>
>>>>>> Is the IRQ correctly registered in drivers/dma/pl330.c:pl330_probe()?
>>>>>> Do you get interrupts when the transfer finish?
>>>>> Sure. IRQ works well.
>>>>
>>>> Ok, so can you check if pl330_update() is correctly marking the request
>>>> as free?  Do you know if there is another request in the queue when that
>>>> happens?
>>>>
>>>> Thomas, you said in a previous email that _thrd_active() always returned
>>>> '1'.  Was that after a request in req[0] finished?
>>>>  - If so, can you check that MARK_FREE was actually called for that
>>>>    request in pl330_update()?
>>>>  - If it was after a request in req[1] finished and there was a
>>>>    request already waiting in req[0], can you debug why _start()
>>>>    didn't activate it.
>>>>
>>> Javi, could you please check if you too get the memcpy failure with
>>> dmatest ?
>>
>> (Please adding me in this thread)
>>
>> Hi Javi,
>> How was going on the test? If any update, please let us know.
> 
> I'm currently very busy at work, I'll run it as soon as I can.

Ok, I think I've just reproduced it in my end with the kernel's dmatest
module.  After the first transaction it looks like the dma test wasn't
able to issue any more transactions.  I'll have a look at it tomorrow
and try to answer my own questions ;)

Cheers,
Javi