From: sbranden@broadcom•com (Scott Branden)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] dmaengine: pl330: fix the race condition in pl330 driver.
Date: Mon, 30 Mar 2015 20:40:43 -0700 [thread overview]
Message-ID: <551A173B.4010704@broadcom.com> (raw)
In-Reply-To: <20150330172546.GI7192@intel.com>
Hi Vinod, Jassi,
Some details on the problem encountered.
On 15-03-30 10:25 AM, Vinod Koul wrote:
> On Mon, Mar 30, 2015 at 10:17:17PM +0530, Jassi Brar wrote:
>> On Fri, Mar 27, 2015 at 5:25 AM, Scott Branden <sbranden@broadcom•com> wrote:
>>> From: ismail <ismail@broadcom•com>
>>>
>>> Update the thread running index before issuing the
>>> GO command to the DMAC.
>>>
>>> Tested-by: Mohamed Ismail Abdul Packir Mohamed <ismail@broadcom•com>
>>> Reviewed-by: Ray Jui <rjui@broadcom•com>
>>> Reviewed-by: Arun Parameswaran <aparames@broadcom•com>
>>> Reviewed-by: Scott Branden <sbranden@broadcom•com>
>>> Signed-off-by: Scott Branden <sbranden@broadcom•com>
>>> Signed-off-by: Mohamed Ismail Abdul Packir Mohamed <ismail@broadcom•com>
>>> ---
>>> drivers/dma/pl330.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>>> index 0e1f567..631642d 100644
>>> --- a/drivers/dma/pl330.c
>>> +++ b/drivers/dma/pl330.c
>>> @@ -1072,11 +1072,11 @@ static bool _trigger(struct pl330_thread *thrd)
>>> /* Set to generate interrupts for SEV */
>>> writel(readl(regs + INTEN) | (1 << thrd->ev), regs + INTEN);
>>>
>>> + thrd->req_running = idx;
>>> +
>>> /* Only manager can execute GO */
>>> _execute_DBGINSN(thrd, insn, true);
>>>
>>> - thrd->req_running = idx;
>>> -
>> It would help to know what the behavior looks like before and after
>> the patch. If anything we should look at locking rather the
>> reordering.
> Yes that ia fair request, looking at changelog it is hard to understand the
> issue seen?
>
We encountered this problem as we modified the driver to make SMC calls
to a TZ handler. This slowed down the driver to the point where DMA
transactions easily failed. I believe the same could be accomplished by
adding a delay between the GOCMD and update of the req_running and
running the built in dmatest.
The DMA transaction is broken if the interrupt occurs before the
thrd->req_running is updated.
The pl330 issues a GOCMD (in _trigger function) to start a new transfer.
The issue of GOCMD generates an interrupt and the IRQ handler will call
the pl330_update function to process the interrupt.
The pl330_update function will verify the thread running index and break
the transaction, if the thread running index is not set.
Regards,
Scott
next prev parent reply other threads:[~2015-03-31 3:40 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-26 23:55 [PATCH] dmaengine: pl330: fix the race condition in pl330 driver Scott Branden
2015-03-29 21:55 ` Scott Branden
2015-03-30 16:47 ` Jassi Brar
2015-03-30 17:25 ` Vinod Koul
2015-03-31 3:40 ` Scott Branden [this message]
2015-03-31 5:20 ` Jassi Brar
2015-03-31 22:04 ` Scott Branden
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=551A173B.4010704@broadcom.com \
--to=sbranden@broadcom$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox