From: stanimir.varbanov@linaro.org (Stanimir Varbanov)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/4] dmaengine: qcom_bam_dma: clear BAM interrupt only if it is rised
Date: Wed, 2 Dec 2015 14:56:57 +0200 [thread overview]
Message-ID: <565EEA99.3070207@linaro.org> (raw)
In-Reply-To: <6461444.oollnBsbrb@wuerfel>
On 12/01/2015 12:29 PM, Arnd Bergmann wrote:
> On Tuesday 01 December 2015 11:14:57 Stanimir Varbanov wrote:
>> + if (srcs & BAM_IRQ) {
>> clr_mask = readl_relaxed(bam_addr(bdev, 0, BAM_IRQ_STTS));
>>
>> - /* don't allow reorder of the various accesses to the BAM registers */
>> - mb();
>> + /*
>> + * don't allow reorder of the various accesses to the BAM
>> + * registers
>> + */
>> + mb();
>>
>> - writel_relaxed(clr_mask, bam_addr(bdev, 0, BAM_IRQ_CLR));
>> + writel_relaxed(clr_mask, bam_addr(bdev, 0, BAM_IRQ_CLR));
>> + }
>>
>
> I think the comment here should be moved: change the writel_relaxed()
> to writel(), which already includes the appropriate barriers, and
If we agree with such a change it should be subject to another patch.
> add a comment at the readl_relaxed() to explain why it doesn't need
> a barrier.
Infact I'm not sure that readl_relaxed(BAM_IRQ_STTS) does not need
barrier. If I read the code above correctly the mb() should guarantee
that all load and store operations before it are happened before the
write to BAM_IRQ_CLR register, and on the other hand if we replace
writel_relaxed with writel, the writel has wmb() which guarantees only
store operations. Did I miss something?
--
regards,
Stan
next prev parent reply other threads:[~2015-12-02 12:56 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-01 9:14 [PATCH 0/4] bam dma fixes and one dt extension Stanimir Varbanov
2015-12-01 9:14 ` [PATCH 1/4] dmaengine: qcom_bam_dma: fix dma free memory on remove Stanimir Varbanov
2015-12-01 9:14 ` [PATCH 2/4] dmaengine: qcom_bam_dma: clear BAM interrupt only if it is rised Stanimir Varbanov
2015-12-01 10:29 ` Arnd Bergmann
2015-12-02 12:56 ` Stanimir Varbanov [this message]
2015-12-02 13:05 ` Arnd Bergmann
2015-12-02 16:47 ` Stanimir Varbanov
2015-12-01 17:28 ` Andy Gross
2015-12-01 9:14 ` [PATCH 3/4] dmaengine: qcom_bam_dma: use correct pipe FIFO size Stanimir Varbanov
2015-12-01 10:28 ` Arnd Bergmann
2015-12-01 17:25 ` Andy Gross
2015-12-01 20:22 ` Arnd Bergmann
2015-12-01 17:23 ` Andy Gross
2015-12-02 16:44 ` Stanimir Varbanov
2015-12-02 17:22 ` Andy Gross
2015-12-10 13:18 ` Stanimir Varbanov
2016-01-29 4:38 ` Andy Gross
2015-12-01 9:14 ` [PATCH 4/4] dmaengine: qcom_bam_dma: add controlled remotely dt property Stanimir Varbanov
2015-12-01 17:30 ` Andy Gross
2016-04-05 21:33 ` [PATCH 0/4] bam dma fixes and one dt extension Andy Gross
2016-04-05 23:06 ` Stanimir Varbanov
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=565EEA99.3070207@linaro.org \
--to=stanimir.varbanov@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).