Re: [Patch v8 2/2] dmaengine: add Qualcomm BAM dma driver
On Mon, Mar 03, 2014 at 09:38:03AM +, Shevchenko, Andriy wrote:
> > + if (IS_ERR(bdev->bamclk))
> > + return PTR_ERR(bdev->bamclk);
> > +
> > + ret = clk_prepare_enable(bdev->bamclk);
> > + if (ret) {
> > + dev_err(bdev->dev, "failed to prepare/enable clock\n");
> > + return ret;
> > + }
> > +
> > + ret = of_property_read_u32(pdev->dev.of_node, "qcom,ee", &bdev->ee);
> > + if (ret) {
> > + dev_err(bdev->dev, "Execution environment unspecified\n");
> > + return ret;
>
> goto err_disable_clk; ?
>
> >
> > + }
> > +
> > + ret = bam_init(bdev);
> > + if (ret)
> > + return ret;
>
> Ditto.
>
Good catch. I'll fix both of those.
--
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch v8 2/2] dmaengine: add Qualcomm BAM dma driver
On Sat, Mar 08, 2014 at 12:29:49AM +0200, Stanimir Vabanov wrote:
> > +#define BAM_IRQ_SRCS_EE(pipe) (0x0800 + ((pipe) * 0x80))
> > +#define BAM_IRQ_SRCS_MSK_EE(pipe) (0x0804 + ((pipe) * 0x80))
>
> s/pipe/ee ?
>
Ah good catch. I'll fix that.
> > +struct bam_chan {
> > + struct virt_dma_chan vc;
> > +
> > + struct bam_device *bdev;
> > +
> > + /* configuration from device tree */
> > + u32 id;
> > + u32 ee;
> > +
>
> do we need per channel ee? I'm asking because failed to find references
> to it.
>
You're right. This is dead variable. I had transitioned from channel to
device when I modified the bindings. Device is where it belongs. I'll fix
this.
> > +
> > +/**
> > + * bam_alloc_chan - Allocate channel resources for DMA channel.
> > + * @chan: specified channel
> > + *
> > + * This function allocates the FIFO descriptor memory
> > + */
> > +static int bam_alloc_chan(struct dma_chan *chan)
> > +{
> > + struct bam_chan *bchan = to_bam_chan(chan);
> > + struct bam_device *bdev = bchan->bdev;
> > +
>
> you could invert the logic and avoid extra indentation.
> if (bchan->fifo_virt)
> return 0;
>
True. I'll flip that.
--
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch v8 2/2] dmaengine: add Qualcomm BAM dma driver
Hi Andy,
Thanks for the patch.
> +#define BAM_IRQ_SRCS_EE(pipe)(0x0800 + ((pipe) * 0x80))
> +#define BAM_IRQ_SRCS_MSK_EE(pipe)(0x0804 + ((pipe) * 0x80))
s/pipe/ee ?
> +struct bam_chan {
> + struct virt_dma_chan vc;
> +
> + struct bam_device *bdev;
> +
> + /* configuration from device tree */
> + u32 id;
> + u32 ee;
> +
do we need per channel ee? I'm asking because failed to find references
to it.
> + struct bam_async_desc *curr_txd;/* current running dma */
> +
> + /* runtime configuration */
> + struct dma_slave_config slave;
> +
> + /* fifo storage */
> + struct bam_desc_hw *fifo_virt;
> + dma_addr_t fifo_phys;
> +
> + /* fifo markers */
> + unsigned short head;/* start of active descriptor entries */
> + unsigned short tail;/* end of active descriptor entries */
> +
> + unsigned int initialized; /* is the channel hw initialized? */
> + unsigned int paused;/* is the channel paused? */
> + unsigned int reconfigure; /* new slave config? */
> +
> + struct list_head node;
> +};
> +
> +
> +/**
> + * bam_alloc_chan - Allocate channel resources for DMA channel.
> + * @chan: specified channel
> + *
> + * This function allocates the FIFO descriptor memory
> + */
> +static int bam_alloc_chan(struct dma_chan *chan)
> +{
> + struct bam_chan *bchan = to_bam_chan(chan);
> + struct bam_device *bdev = bchan->bdev;
> +
you could invert the logic and avoid extra indentation.
if (bchan->fifo_virt)
return 0;
> + /* allocate FIFO descriptor space, but only if necessary */
> + if (!bchan->fifo_virt) {
> + bchan->fifo_virt = dma_alloc_writecombine(bdev->dev,
> + BAM_DESC_FIFO_SIZE, &bchan->fifo_phys,
> + GFP_KERNEL);
> +
> + if (!bchan->fifo_virt) {
> + dev_err(bdev->dev, "Failed to allocate desc fifo\n");
> + return -ENOMEM;
> + }
> + }
> +
> + return 0;
> +}
> +
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch v8 2/2] dmaengine: add Qualcomm BAM dma driver
On Mon, 2014-03-03 at 00:30 -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.
One comment below related to error path in probe().
>
> Signed-off-by: Andy Gross
> ---
> drivers/dma/Kconfig|9 +
> drivers/dma/Makefile |1 +
> drivers/dma/qcom_bam_dma.c | 1110
>
> 3 files changed, 1120 insertions(+)
> create mode 100644 drivers/dma/qcom_bam_dma.c
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 605b016..f87cef9 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -401,4 +401,13 @@ config DMATEST
> config DMA_ENGINE_RAID
> bool
>
> +config QCOM_BAM_DMA
> + tristate "QCOM BAM DMA support"
> + depends on ARCH_QCOM || (COMPILE_TEST && OF && ARM)
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + ---help---
> + Enable support for the QCOM BAM DMA controller. This controller
> + provides DMA capabilities for a variety of on-chip devices.
> +
> endif
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index a029d0f4..907b915 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -44,3 +44,4 @@ obj-$(CONFIG_DMA_JZ4740) += dma-jz4740.o
> obj-$(CONFIG_TI_CPPI41) += cppi41.o
> obj-$(CONFIG_K3_DMA) += k3dma.o
> obj-$(CONFIG_MOXART_DMA) += moxart-dma.o
> +obj-$(CONFIG_QCOM_BAM_DMA) += qcom_bam_dma.o
> diff --git a/drivers/dma/qcom_bam_dma.c b/drivers/dma/qcom_bam_dma.c
> new file mode 100644
> index 000..3210983
> --- /dev/null
> +++ b/drivers/dma/qcom_bam_dma.c
> @@ -0,0 +1,1110 @@
> +/*
> + * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +/*
> + * QCOM BAM DMA engine driver
> + *
> + * QCOM BAM DMA blocks are distributed amongst a number of the on-chip
> + * peripherals on the MSM 8x74. The configuration of the channels are
> dependent
> + * on the way they are hard wired to that specific peripheral. The
> peripheral
> + * device tree entries specify the configuration of each channel.
> + *
> + * The DMA controller requires the use of external memory for storage of the
> + * hardware descriptors for each channel. The descriptor FIFO is accessed
> as a
> + * circular buffer and operations are managed according to the offset within
> the
> + * FIFO. After pipe/channel reset, all of the pipe registers and internal
> state
> + * are back to defaults.
> + *
> + * During DMA operations, we write descriptors to the FIFO, being careful to
> + * handle wrapping and then write the last FIFO offset to that channel's
> + * P_EVNT_REG register to kick off the transaction. The P_SW_OFSTS register
> + * indicates the current FIFO offset that is being processed, so there is
> some
> + * indication of where the hardware is currently working.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "dmaengine.h"
> +#include "virt-dma.h"
> +
> +struct bam_desc_hw {
> + u32 addr; /* Buffer physical address */
> + u16 size; /* Buffer size in bytes */
> + u16 flags;
> +};
> +
> +#define DESC_FLAG_INT BIT(15)
> +#define DESC_FLAG_EOT BIT(14)
> +#define DESC_FLAG_EOB BIT(13)
> +
> +struct bam_async_desc {
> + struct virt_dma_desc vd;
> +
> + u32 num_desc;
> + u32 xfer_len;
> + struct bam_desc_hw *curr_desc;
> +
> + enum dma_transfer_direction dir;
> + size_t length;
> + struct bam_desc_hw desc[0];
> +};
> +
> +#define BAM_CTRL 0x
> +#define BAM_REVISION 0x0004
> +#define BAM_SW_REVISION 0x0080
> +#define BAM_NUM_PIPES0x003C
> +#define BAM_TIMER0x0040
> +#define BAM_TIMER_CTRL 0x0044
> +#define BAM_DESC_CNT_TRSHLD 0x0008
> +#define BAM_IRQ_SRCS 0x000C
> +#define BAM_IRQ_SRCS_MSK
Re: [Patch v8 2/2] dmaengine: add Qualcomm BAM dma driver
On Mon, 2014-03-03 at 00:30 -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).
[]
trivia:
> +static u32 process_channel_irqs(struct bam_device *bdev)
> +{
[]
> + for (i = 0; i < bdev->num_channels; i++) {
> + struct bam_chan *bchan = &bdev->channels[i];
> + if (srcs & BIT(i)) {
> + /* clear pipe irq */
> + pipe_stts = readl_relaxed(bdev->regs +
> + BAM_P_IRQ_STTS(i));
> +
> + writel_relaxed(pipe_stts, bdev->regs +
> + BAM_P_IRQ_CLR(i));
> +
> + spin_lock_irqsave(&bchan->vc.lock, flags);
> + async_desc = bchan->curr_txd;
> +
> + if (async_desc) {
> + async_desc->num_desc -= async_desc->xfer_len;
> + async_desc->curr_desc += async_desc->xfer_len;
> + bchan->curr_txd = NULL;
> +
> + /* manage FIFO */
> + bchan->head += async_desc->xfer_len;
> + bchan->head %= MAX_DESCRIPTORS;
> +
> + /*
> + * if complete, process cookie. Otherwise
> + * push back to front of desc_issued so that
> + * it gets restarted by the tasklet
> + */
> + if (!async_desc->num_desc)
> + vchan_cookie_complete(&async_desc->vd);
> + else
> + list_add(&async_desc->vd.node,
> + &bchan->vc.desc_issued);
> + }
> +
> + spin_unlock_irqrestore(&bchan->vc.lock, flags);
> + }
> + }
This could be written with fewer indents using continue
(and maybe faster using ffs too)
for (i = 0; i < bdev->num_channels; i++) {
struct bam_chan *bchan;
if (!(srcs & BIT(i)))
continue;
bchan = &bdev->channels[i];
/* clear pipe irq */
pipe_stts = readl_relaxed(bdev->regs + BAM_P_IRQ_STTS(i));
writel_relaxed(pipe_stts, bdev->regs + BAM_P_IRQ_CLR(i));
etc...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
