linux-kernel - Re: [Patch v5 1/2] dmaengine: add Qualcomm BAM dma driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140211180344.GS10628@intel.com>
Date:	Tue, 11 Feb 2014 23:33:44 +0530
From:	Vinod Koul <vinod.koul@...el.com>
To:	Josh Cartwright <joshc@...eaurora.org>
Cc:	Andy Gross <agross@...eaurora.org>,
	Dan Williams <dan.j.williams@...el.com>,
	dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-arm-msm@...r.kernel.org
Subject: Re: [Patch v5 1/2] dmaengine: add Qualcomm BAM dma driver

On Tue, Feb 11, 2014 at 11:49:10AM -0600, Josh Cartwright wrote:
> On Tue, Feb 11, 2014 at 11:00:48PM +0530, Vinod Koul wrote:
> > On Tue, Feb 04, 2014 at 02:42:35PM -0600, Andy Gross wrote:
> > > Add the DMA engine driver for the QCOM Bus Access Manager (BAM) DMA controller
> > > found in the MSM 8x74 platforms.
> > > 
> > > Each BAM DMA device is associated with a specific on-chip peripheral.  Each
> > > channel provides a uni-directional data transfer engine that is capable of
> > > transferring data between the peripheral and system memory (System mode), or
> > > between two peripherals (BAM2BAM).
> > > 
> > > The initial release of this driver only supports slave transfers between
> > > peripherals and system memory.
> > > 
> > > Signed-off-by: Andy Gross <agross@...eaurora.org>
> >
> > > +++ b/drivers/dma/qcom_bam_dma.c
> [..]
> > > +static void bam_reset_channel(struct bam_chan *bchan)
> > > +{
> > > +	struct bam_device *bdev = bchan->bdev;
> > > +
> > > +	/* reset channel */
> > > +	writel_relaxed(1, bdev->regs + BAM_P_RST(bchan->id));
> > > +	writel_relaxed(0, bdev->regs + BAM_P_RST(bchan->id));
> > > +
> > > +	/* don't allow reorder of the channel reset */
> > > +	wmb();
> > Documentation/memory-barriers.txt describes wmb() as a CPU barier but based on
> > above you want it to be a compiler barrier then you should do 1st write,
> > barrier(), second write.
> 
> It could also be that the intent was to prevent these writes from being
> ordered before setting the initialized flag below, either way the
> comment could be made clearer.
yes for that case, but i am suspecting the comment is correct as it would make
sense to ensure reset is in sequence...

-- 
~Vinod
> 
> > > +
> > > +	/* make sure hw is initialized when channel is used the first time  */
> > > +	bchan->initialized = 0;
> > > +}
> 
-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/