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

From: Josh Cartwright
Date: Tue Feb 11 2014 - 12:52:48 EST


Ugh.

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@xxxxxxxxxxxxxx>
> >
> > > +++ 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

^ after

> comment could be made clearer.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/