Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
On Jan 21, 2014, at 5:01 PM, Andy Gross agr...@codeaurora.org wrote:
On Tue, Jan 21, 2014 at 09:03:43AM +0100, Arnd Bergmann wrote:
On Monday 20 January 2014 16:52:45 Andy Gross wrote:
+#ifdef CONFIG_OF
+static const struct of_device_id bam_of_match[] = {
+ { .compatible = qcom,bam-v1.4.0, },
+ { .compatible = qcom,bam-v1.4.1, },
+ {}
+};
+MODULE_DEVICE_TABLE(of, bam_of_match);
+#endif
Also, you can remove the #ifdef here and the of_match_ptr() below.
If this is removed, then I'll have to add the OF dependency in the Kconfig,
correct?
I believe it will still compile without the CONFIG_OF dependency, but
having the dependency still makes sense as it's impossible to use the
driver without CONFIG_OF.
The best dependency line is probably
depends on (ARCH_MSM OF) || COMPILE_TEST
If you expect the same driver to be used on non-MSM platforms from
qualcomm, e.g. some networking or server equipment, you can also just
drop the ARCH_MSM dependency.
Thanks for the clarification. I think I'll probably do:
ARCH_MSM_DT || (COMPILE_TEST ARM)
Didn’t you need it to be:
ARCH_MSM_DT || (COMPILE_TEST OF ARM)
- k
--
Employee of Qualcomm Innovation Center, Inc.
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-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
On Thu, Jan 23, 2014 at 02:17:19PM -0600, Kumar Gala wrote: [] Thanks for the clarification. I think I'll probably do: ARCH_MSM_DT || (COMPILE_TEST ARM) Didn?t you need it to be: ARCH_MSM_DT || (COMPILE_TEST OF ARM) - k Yes. That's the version I am going with. I might be able to get away without the OF if it compiles fine without it, provided it stubs out the OF stuff if the config option is not present. -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
On Monday 20 January 2014 16:52:45 Andy Gross wrote:
+#ifdef CONFIG_OF
+static const struct of_device_id bam_of_match[] = {
+ { .compatible = qcom,bam-v1.4.0, },
+ { .compatible = qcom,bam-v1.4.1, },
+ {}
+};
+MODULE_DEVICE_TABLE(of, bam_of_match);
+#endif
Also, you can remove the #ifdef here and the of_match_ptr() below.
If this is removed, then I'll have to add the OF dependency in the Kconfig,
correct?
I believe it will still compile without the CONFIG_OF dependency, but
having the dependency still makes sense as it's impossible to use the
driver without CONFIG_OF.
The best dependency line is probably
depends on (ARCH_MSM OF) || COMPILE_TEST
If you expect the same driver to be used on non-MSM platforms from
qualcomm, e.g. some networking or server equipment, you can also just
drop the ARCH_MSM dependency.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
On Tue, Jan 21, 2014 at 09:03:43AM +0100, Arnd Bergmann wrote:
On Monday 20 January 2014 16:52:45 Andy Gross wrote:
+#ifdef CONFIG_OF
+static const struct of_device_id bam_of_match[] = {
+ { .compatible = qcom,bam-v1.4.0, },
+ { .compatible = qcom,bam-v1.4.1, },
+ {}
+};
+MODULE_DEVICE_TABLE(of, bam_of_match);
+#endif
Also, you can remove the #ifdef here and the of_match_ptr() below.
If this is removed, then I'll have to add the OF dependency in the Kconfig,
correct?
I believe it will still compile without the CONFIG_OF dependency, but
having the dependency still makes sense as it's impossible to use the
driver without CONFIG_OF.
The best dependency line is probably
depends on (ARCH_MSM OF) || COMPILE_TEST
If you expect the same driver to be used on non-MSM platforms from
qualcomm, e.g. some networking or server equipment, you can also just
drop the ARCH_MSM dependency.
Thanks for the clarification. I think I'll probably do:
ARCH_MSM_DT || (COMPILE_TEST ARM)
The latter due to my use of writel_relaxed.
--
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-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
On Tuesday 21 January 2014 17:01:03 Andy Gross wrote: Thanks for the clarification. I think I'll probably do: ARCH_MSM_DT || (COMPILE_TEST ARM) The latter due to my use of writel_relaxed. Yes, looks good. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
On Fri, Jan 17, 2014 at 11:49:27PM +0100, Arnd Bergmann wrote:
On Friday 10 January 2014, Andy Gross wrote:
+static bool bam_dma_filter(struct dma_chan *chan, void *data)
+{
+ struct bam_filter_args *args = data;
+ struct bam_chan *bchan = to_bam_chan(chan);
+
+ if (args-dev == chan-device
+ args-id == bchan-id) {
+
+ /* we found the channel, so lets set the EE and dir */
+ bchan-ee = args-ee;
+ bchan-slave.direction = args-dir ?
+ DMA_DEV_TO_MEM : DMA_MEM_TO_DEV;
+ return true;
+ }
+
+ return false;
+}
A filter function should no longer be needed.
Ok, will remove.
+static struct dma_chan *bam_dma_xlate(struct of_phandle_args *dma_spec,
+ struct of_dma *of)
+{
+ struct bam_filter_args args;
+ dma_cap_mask_t cap;
+
+ if (dma_spec-args_count != 3)
+ return NULL;
+
+ args.dev = of-of_dma_data;
+ args.id = dma_spec-args[0];
+ args.ee = dma_spec-args[1];
+ args.dir = dma_spec-args[2];
+
+ dma_cap_zero(cap);
+ dma_cap_set(DMA_SLAVE, cap);
+
+ return dma_request_channel(cap, bam_dma_filter, args);
+}
Instead, call dma_get_slave_channel() with the right channel that you already
know
here.
Agreed.
+ if (pdev-dev.of_node) {
+ ret = of_dma_controller_register(pdev-dev.of_node,
+ bam_dma_xlate, bdev-common);
+
+ if (ret) {
+ dev_err(bdev-dev, failed to register of_dma\n);
+ goto err_unregister_dma;
+ }
+ }
No need to check for pdev-dev.of_node when that is the only mode of probing.
Good point. I'll remove extraneous check.
+
+#ifdef CONFIG_OF
+static const struct of_device_id bam_of_match[] = {
+ { .compatible = qcom,bam-v1.4.0, },
+ { .compatible = qcom,bam-v1.4.1, },
+ {}
+};
+MODULE_DEVICE_TABLE(of, bam_of_match);
+#endif
Also, you can remove the #ifdef here and the of_match_ptr() below.
If this is removed, then I'll have to add the OF dependency in the Kconfig,
correct?
+
+static struct platform_driver bam_dma_driver = {
+ .probe = bam_dma_probe,
+ .remove = bam_dma_remove,
+ .driver = {
+ .name = bam-dma-engine,
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(bam_of_match),
+ },
+};
+
+static int __init bam_dma_init(void)
+{
+ return platform_driver_register(bam_dma_driver);
+}
+
+static void __exit bam_dma_exit(void)
+{
+ return platform_driver_unregister(bam_dma_driver);
+}
+
module_platform_driver()
Will fix.
diff --git a/drivers/dma/qcom_bam_dma.h b/drivers/dma/qcom_bam_dma.h
new file mode 100644
index 000..2cb3b5f
--- /dev/null
+++ b/drivers/dma/qcom_bam_dma.h
@@ -0,0 +1,268 @@
+#ifndef __QCOM_BAM_DMA_H__
+#define __QCOM_BAM_DMA_H__
+
+#include linux/dmaengine.h
+#include virt-dma.h
+
+enum bam_channel_dir {
+ BAM_PIPE_CONSUMER = 0, /* channel reads from data-fifo or memory */
+ BAM_PIPE_PRODUCER, /* channel writes to data-fifo or memory */
+};
Since the header does not serve as an interface, just move all the contents
into the driver directly.
OK. SBoyd made the same comment. I'll go ahead and collapse both down to 1
file.
+struct bam_desc_hw {
+ u32 addr; /* Buffer physical address */
+ u16 size; /* Buffer size in bytes */
+ u16 flags;
+} __packed;
Remove __packed here, it only makes the access less efficient but does not
change
the layout, which is already packed.
Ok. Will fix.
--
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-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
On Tue, Jan 14, 2014 at 11:43:48AM -0800, Stephen Boyd wrote:
(Mostly nitpicks)
On 01/10, 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 agr...@codeaurora.org
---
drivers/dma/Kconfig| 9 +
drivers/dma/Makefile | 1 +
drivers/dma/qcom_bam_dma.c | 843
+
drivers/dma/qcom_bam_dma.h | 268 ++
4 files changed, 1121 insertions(+)
create mode 100644 drivers/dma/qcom_bam_dma.c
create mode 100644 drivers/dma/qcom_bam_dma.h
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index c823daa..e58e6d2 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -384,4 +384,13 @@ config DMATEST
config DMA_ENGINE_RAID
bool
+config QCOM_BAM_DMA
+ tristate QCOM BAM DMA support
+ depends on ARCH_MSM || COMPILE_TEST
I don't think writel_relaxed() is available on every arch, so
it's possible this will break some random arch that doesn't have
that function.
I'll look into this to see. If that's the case, I can remove the COMPILE_TEST
if there is no alternative.
+ 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/qcom_bam_dma.c b/drivers/dma/qcom_bam_dma.c
new file mode 100644
index 000..7a84b02
--- /dev/null
+++ b/drivers/dma/qcom_bam_dma.c
[...]
+static int bam_alloc_chan(struct dma_chan *chan)
[...]
+
+ /* Go ahead and initialize the pipe/channel hardware here
+ - Reset the channel to clear internal state of the FIFO
+ - Program in the FIFO address
+ - Configure the irq based on the EE passed in from the DT entry
+ - Set mode, direction and enable channel
+
+ We do this here because the channel can only be enabled once and
+ can only be disabled via a reset. If done here, we don't have to
+ manage additional state to figure out when to do this
+ */
Multi-line comments are of the form:
/*
* comment
*/
Right. I converted some comments and didn't do the correct multi-line
+
+ bam_reset_channel(bdev, bchan-id);
+
+ /* write out 8 byte aligned address. We have enough space for this
+ because we allocated 1 more descriptor (8 bytes) than we can use */
+ writel_relaxed(ALIGN(bchan-fifo_phys, sizeof(struct bam_desc_hw)),
+ bdev-regs + BAM_P_DESC_FIFO_ADDR(bchan-id));
+ writel_relaxed(BAM_DESC_FIFO_SIZE, bdev-regs +
+ BAM_P_FIFO_SIZES(bchan-id));
[...]
+
+/**
+ * bam_dma_terminate_all - terminate all transactions
+ * @chan: dma channel
+ *
+ * Idles channel and dequeues and frees all transactions
+ * No callbacks are done
+ *
+*/
Weird '*' starting the line here and on the next function.
Will fix.
+static void bam_dma_terminate_all(struct dma_chan *chan)
+{
+ struct bam_chan *bchan = to_bam_chan(chan);
+ struct bam_device *bdev = bchan-bdev;
+
+ bam_reset_channel(bdev, bchan-id);
+
+ vchan_free_chan_resources(bchan-vc);
+}
+
+/**
+ * bam_control - DMA device control
+ * @chan: dma channel
+ * @cmd: control cmd
+ * @arg: cmd argument
+ *
+ * Perform DMA control command
+ *
+*/
+static int bam_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
+ unsigned long arg)
+{
+ struct bam_chan *bchan = to_bam_chan(chan);
+ struct bam_device *bdev = bchan-bdev;
+ int ret = 0;
+ unsigned long flag;
+
[...]
+/**
+ * bam_dma_irq - irq handler for bam controller
+ * @irq: IRQ of interrupt
+ * @data: callback data
+ *
+ * IRQ handler for the bam controller
+ */
+static irqreturn_t bam_dma_irq(int irq, void *data)
+{
+ struct bam_device *bdev = (struct bam_device *)data;
Unnecessary cast from void.
Fixed.
+static int bam_dma_probe(struct platform_device *pdev)
+{
[...]
+
+ irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (!irq_res) {
+ dev_err(bdev-dev, irq resource is missing\n);
+ return -EINVAL;
+ }
Please use platform_get_irq() instead.
Fixed.
diff --git a/drivers/dma/qcom_bam_dma.h b/drivers/dma/qcom_bam_dma.h
new file mode 100644
index 000..2cb3b5f
--- /dev/null
+++ b/drivers/dma/qcom_bam_dma.h
[...]
+#define
Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
On Mon, Jan 13, 2014 at 10:31:01AM +, Shevchenko, Andriy wrote:
On Fri, 2014-01-10 at 13:07 -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 agr...@codeaurora.org
---
drivers/dma/Kconfig| 9 +
drivers/dma/Makefile | 1 +
drivers/dma/qcom_bam_dma.c | 843
+
drivers/dma/qcom_bam_dma.h | 268 ++
4 files changed, 1121 insertions(+)
create mode 100644 drivers/dma/qcom_bam_dma.c
create mode 100644 drivers/dma/qcom_bam_dma.h
[...]
+ * bam_tx_status - returns status of transaction
+ * @chan: dma channel
+ * @cookie: transaction cookie
+ * @txstate: DMA transaction state
+ *
+ * Return status of dma transaction
+ */
+static enum dma_status bam_tx_status(struct dma_chan *chan, dma_cookie_t
cookie,
+ struct dma_tx_state *txstate)
+{
+ struct bam_chan *bchan = to_bam_chan(chan);
+ struct virt_dma_desc *vd;
+ int ret;
+ size_t residue = 0;
+ unsigned int i;
+ unsigned long flags;
+
+ ret = dma_cookie_status(chan, cookie, txstate);
+
Redundant empty line.
Will remove.
+ if (ret == DMA_COMPLETE)
+ return ret;
+
+ if (!txstate)
+ return bchan-paused ? DMA_PAUSED : ret;
+
+ spin_lock_irqsave(bchan-vc.lock, flags);
+ vd = vchan_find_desc(bchan-vc, cookie);
+ if (vd)
+ residue = container_of(vd, struct bam_async_desc, vd)-length;
+ else if (bchan-curr_txd bchan-curr_txd-vd.tx.cookie == cookie)
+ for (i = 0; i bchan-curr_txd-num_desc; i++)
+ residue += bchan-curr_txd-curr_desc[i].size;
+
+ dma_set_residue(txstate, residue);
I'm pretty sure you could do this outside of spin lock.
Yes, I'll move it.
+
+ spin_unlock_irqrestore(bchan-vc.lock, flags);
+
+ if (ret == DMA_IN_PROGRESS bchan-paused)
+ ret = DMA_PAUSED;
+
+ return ret;
+}
+
+/**
+ * bam_start_dma - start next transaction
+ * @bchan - bam dma channel
+ *
+ * Note: must hold bam dma channel vc.lock
+ */
+static void bam_start_dma(struct bam_chan *bchan)
+{
+ struct virt_dma_desc *vd = vchan_next_desc(bchan-vc);
+ struct bam_device *bdev = bchan-bdev;
+ struct bam_async_desc *async_desc;
+ struct bam_desc_hw *desc;
+ struct bam_desc_hw *fifo = PTR_ALIGN(bchan-fifo_virt,
+ sizeof(struct bam_desc_hw));
+
+ if (!vd)
+ return;
+
+ list_del(vd-node);
+
+ async_desc = container_of(vd, struct bam_async_desc, vd);
+ bchan-curr_txd = async_desc;
+
+ desc = bchan-curr_txd-curr_desc;
+
+ if (async_desc-num_desc MAX_DESCRIPTORS)
+ async_desc-xfer_len = MAX_DESCRIPTORS;
+ else
+ async_desc-xfer_len = async_desc-num_desc;
+
+ /* set INT on last descriptor */
+ desc[async_desc-xfer_len - 1].flags |= DESC_FLAG_INT;
+
+ if (bchan-tail + async_desc-xfer_len MAX_DESCRIPTORS) {
+ u32 partial = MAX_DESCRIPTORS - bchan-tail;
+
+ memcpy(fifo[bchan-tail], desc,
+ partial * sizeof(struct bam_desc_hw));
+ memcpy(fifo, desc[partial], (async_desc-xfer_len - partial) *
+ sizeof(struct bam_desc_hw));
I'm just curious if you could avoid memcpys at all somehow.
Unfortunately not. The descriptors have to be copied into the FIFO memory that
is being used by the dma controller for this channel. Due to the way the FIFO
works, I have to copy into the FIFO during either the issue_pending or when I
start the next transaction. Either way, it means copying from the txd to the
FIFO.
+ } else
Keep style
OK.
} else {
...
}
Have you run checkpatch.pl?
Yes. And I fixed any discrepancies before sending this.
+ memcpy(fifo[bchan-tail], desc,
+ async_desc-xfer_len * sizeof(struct bam_desc_hw));
+
+ bchan-tail += async_desc-xfer_len;
+ bchan-tail %= MAX_DESCRIPTORS;
+
+ /* ensure descriptor writes and dma start not reordered */
+ wmb();
+ writel_relaxed(bchan-tail * sizeof(struct bam_desc_hw),
+ bdev-regs + BAM_P_EVNT_REG(bchan-id));
+}
+
+/**
+ * dma_tasklet - DMA IRQ tasklet
+ * @data: tasklet argument (bam controller structure)
+ *
+ * Sets up next DMA operation and then processes all completed transactions
+ */
+static void
Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
On Friday 10 January 2014, Andy Gross wrote:
+static bool bam_dma_filter(struct dma_chan *chan, void *data)
+{
+ struct bam_filter_args *args = data;
+ struct bam_chan *bchan = to_bam_chan(chan);
+
+ if (args-dev == chan-device
+ args-id == bchan-id) {
+
+ /* we found the channel, so lets set the EE and dir */
+ bchan-ee = args-ee;
+ bchan-slave.direction = args-dir ?
+ DMA_DEV_TO_MEM : DMA_MEM_TO_DEV;
+ return true;
+ }
+
+ return false;
+}
A filter function should no longer be needed.
+static struct dma_chan *bam_dma_xlate(struct of_phandle_args *dma_spec,
+ struct of_dma *of)
+{
+ struct bam_filter_args args;
+ dma_cap_mask_t cap;
+
+ if (dma_spec-args_count != 3)
+ return NULL;
+
+ args.dev = of-of_dma_data;
+ args.id = dma_spec-args[0];
+ args.ee = dma_spec-args[1];
+ args.dir = dma_spec-args[2];
+
+ dma_cap_zero(cap);
+ dma_cap_set(DMA_SLAVE, cap);
+
+ return dma_request_channel(cap, bam_dma_filter, args);
+}
Instead, call dma_get_slave_channel() with the right channel that you already
know
here.
+ if (pdev-dev.of_node) {
+ ret = of_dma_controller_register(pdev-dev.of_node,
+ bam_dma_xlate, bdev-common);
+
+ if (ret) {
+ dev_err(bdev-dev, failed to register of_dma\n);
+ goto err_unregister_dma;
+ }
+ }
No need to check for pdev-dev.of_node when that is the only mode of probing.
+
+#ifdef CONFIG_OF
+static const struct of_device_id bam_of_match[] = {
+ { .compatible = qcom,bam-v1.4.0, },
+ { .compatible = qcom,bam-v1.4.1, },
+ {}
+};
+MODULE_DEVICE_TABLE(of, bam_of_match);
+#endif
Also, you can remove the #ifdef here and the of_match_ptr() below.
+
+static struct platform_driver bam_dma_driver = {
+ .probe = bam_dma_probe,
+ .remove = bam_dma_remove,
+ .driver = {
+ .name = bam-dma-engine,
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(bam_of_match),
+ },
+};
+
+static int __init bam_dma_init(void)
+{
+ return platform_driver_register(bam_dma_driver);
+}
+
+static void __exit bam_dma_exit(void)
+{
+ return platform_driver_unregister(bam_dma_driver);
+}
+
module_platform_driver()
diff --git a/drivers/dma/qcom_bam_dma.h b/drivers/dma/qcom_bam_dma.h
new file mode 100644
index 000..2cb3b5f
--- /dev/null
+++ b/drivers/dma/qcom_bam_dma.h
@@ -0,0 +1,268 @@
+#ifndef __QCOM_BAM_DMA_H__
+#define __QCOM_BAM_DMA_H__
+
+#include linux/dmaengine.h
+#include virt-dma.h
+
+enum bam_channel_dir {
+ BAM_PIPE_CONSUMER = 0, /* channel reads from data-fifo or memory */
+ BAM_PIPE_PRODUCER, /* channel writes to data-fifo or memory */
+};
Since the header does not serve as an interface, just move all the contents
into the driver directly.
+struct bam_desc_hw {
+ u32 addr; /* Buffer physical address */
+ u16 size; /* Buffer size in bytes */
+ u16 flags;
+} __packed;
Remove __packed here, it only makes the access less efficient but does not
change
the layout, which is already packed.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
(Mostly nitpicks)
On 01/10, 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 agr...@codeaurora.org
---
drivers/dma/Kconfig| 9 +
drivers/dma/Makefile | 1 +
drivers/dma/qcom_bam_dma.c | 843
+
drivers/dma/qcom_bam_dma.h | 268 ++
4 files changed, 1121 insertions(+)
create mode 100644 drivers/dma/qcom_bam_dma.c
create mode 100644 drivers/dma/qcom_bam_dma.h
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index c823daa..e58e6d2 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -384,4 +384,13 @@ config DMATEST
config DMA_ENGINE_RAID
bool
+config QCOM_BAM_DMA
+ tristate QCOM BAM DMA support
+ depends on ARCH_MSM || COMPILE_TEST
I don't think writel_relaxed() is available on every arch, so
it's possible this will break some random arch that doesn't have
that function.
+ 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/qcom_bam_dma.c b/drivers/dma/qcom_bam_dma.c
new file mode 100644
index 000..7a84b02
--- /dev/null
+++ b/drivers/dma/qcom_bam_dma.c
[...]
+static int bam_alloc_chan(struct dma_chan *chan)
[...]
+
+ /* Go ahead and initialize the pipe/channel hardware here
+- Reset the channel to clear internal state of the FIFO
+- Program in the FIFO address
+- Configure the irq based on the EE passed in from the DT entry
+- Set mode, direction and enable channel
+
+We do this here because the channel can only be enabled once and
+can only be disabled via a reset. If done here, we don't have to
+manage additional state to figure out when to do this
+ */
Multi-line comments are of the form:
/*
* comment
*/
+
+ bam_reset_channel(bdev, bchan-id);
+
+ /* write out 8 byte aligned address. We have enough space for this
+because we allocated 1 more descriptor (8 bytes) than we can use */
+ writel_relaxed(ALIGN(bchan-fifo_phys, sizeof(struct bam_desc_hw)),
+ bdev-regs + BAM_P_DESC_FIFO_ADDR(bchan-id));
+ writel_relaxed(BAM_DESC_FIFO_SIZE, bdev-regs +
+ BAM_P_FIFO_SIZES(bchan-id));
[...]
+
+/**
+ * bam_dma_terminate_all - terminate all transactions
+ * @chan: dma channel
+ *
+ * Idles channel and dequeues and frees all transactions
+ * No callbacks are done
+ *
+*/
Weird '*' starting the line here and on the next function.
+static void bam_dma_terminate_all(struct dma_chan *chan)
+{
+ struct bam_chan *bchan = to_bam_chan(chan);
+ struct bam_device *bdev = bchan-bdev;
+
+ bam_reset_channel(bdev, bchan-id);
+
+ vchan_free_chan_resources(bchan-vc);
+}
+
+/**
+ * bam_control - DMA device control
+ * @chan: dma channel
+ * @cmd: control cmd
+ * @arg: cmd argument
+ *
+ * Perform DMA control command
+ *
+*/
+static int bam_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
+ unsigned long arg)
+{
+ struct bam_chan *bchan = to_bam_chan(chan);
+ struct bam_device *bdev = bchan-bdev;
+ int ret = 0;
+ unsigned long flag;
+
[...]
+/**
+ * bam_dma_irq - irq handler for bam controller
+ * @irq: IRQ of interrupt
+ * @data: callback data
+ *
+ * IRQ handler for the bam controller
+ */
+static irqreturn_t bam_dma_irq(int irq, void *data)
+{
+ struct bam_device *bdev = (struct bam_device *)data;
Unnecessary cast from void.
+static int bam_dma_probe(struct platform_device *pdev)
+{
[...]
+
+ irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (!irq_res) {
+ dev_err(bdev-dev, irq resource is missing\n);
+ return -EINVAL;
+ }
Please use platform_get_irq() instead.
diff --git a/drivers/dma/qcom_bam_dma.h b/drivers/dma/qcom_bam_dma.h
new file mode 100644
index 000..2cb3b5f
--- /dev/null
+++ b/drivers/dma/qcom_bam_dma.h
[...]
+#define BAM_CNFG_BITS_DEFAULT(BAM_PIPE_CNFG |\
+ BAM_NO_EXT_P_RST | \
+ BAM_IBC_DISABLE | \
+ BAM_SB_CLK_REQ |\
+ BAM_PSM_CSW_REQ | \
+ BAM_PSM_P_RES | \
+
Re: [PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
On Fri, 2014-01-10 at 13:07 -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 agr...@codeaurora.org
---
drivers/dma/Kconfig| 9 +
drivers/dma/Makefile | 1 +
drivers/dma/qcom_bam_dma.c | 843
+
drivers/dma/qcom_bam_dma.h | 268 ++
4 files changed, 1121 insertions(+)
create mode 100644 drivers/dma/qcom_bam_dma.c
create mode 100644 drivers/dma/qcom_bam_dma.h
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index c823daa..e58e6d2 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -384,4 +384,13 @@ config DMATEST
config DMA_ENGINE_RAID
bool
+config QCOM_BAM_DMA
+ tristate QCOM BAM DMA support
+ depends on ARCH_MSM || COMPILE_TEST
+ 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 0ce2da9..7ef950a 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -42,3 +42,4 @@ obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
obj-$(CONFIG_DMA_JZ4740) += dma-jz4740.o
obj-$(CONFIG_TI_CPPI41) += cppi41.o
obj-$(CONFIG_K3_DMA) += k3dma.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..7a84b02
--- /dev/null
+++ b/drivers/dma/qcom_bam_dma.c
@@ -0,0 +1,843 @@
+/*
+ * QCOM BAM DMA engine driver
+ *
+ * Copyright (c) 2013, 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 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 linux/kernel.h
+#include linux/io.h
+#include linux/init.h
+#include linux/slab.h
+#include linux/module.h
+#include linux/interrupt.h
+#include linux/dma-mapping.h
+#include linux/scatterlist.h
+#include linux/device.h
+#include linux/platform_device.h
+#include linux/of.h
+#include linux/of_address.h
+#include linux/of_irq.h
+#include linux/of_dma.h
+#include linux/clk.h
+
+#include dmaengine.h
+#include qcom_bam_dma.h
+
+
+/**
+ * bam_reset_channel - Reset individual BAM DMA channel
+ * @bdev: bam device
+ * @channel: channel id
+ */
+static void bam_reset_channel(struct bam_device *bdev, u32 channel)
+{
+ /* reset channel */
+ writel_relaxed(1, bdev-regs + BAM_P_RST(channel));
+ writel_relaxed(0, bdev-regs + BAM_P_RST(channel));
+}
+
+/**
+ * bam_alloc_chan - Allocate channel resources for DMA channel.
+ * @chan: specified channel
+ *
+ * This function allocates the FIFO descriptor memory and resets the channel
+ */
+static int bam_alloc_chan(struct dma_chan *chan)
+{
+ struct bam_chan *bchan = to_bam_chan(chan);
+ struct bam_device *bdev = bchan-bdev;
+ u32 val;
+
+ /* allocate FIFO descriptor space, but only if necessary */
+ if (!bchan-fifo_virt) {
+ bchan-fifo_virt =
[PATCH v2 1/2] dmaengine: add Qualcomm BAM dma driver
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 agr...@codeaurora.org
---
drivers/dma/Kconfig| 9 +
drivers/dma/Makefile | 1 +
drivers/dma/qcom_bam_dma.c | 843 +
drivers/dma/qcom_bam_dma.h | 268 ++
4 files changed, 1121 insertions(+)
create mode 100644 drivers/dma/qcom_bam_dma.c
create mode 100644 drivers/dma/qcom_bam_dma.h
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index c823daa..e58e6d2 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -384,4 +384,13 @@ config DMATEST
config DMA_ENGINE_RAID
bool
+config QCOM_BAM_DMA
+ tristate QCOM BAM DMA support
+ depends on ARCH_MSM || COMPILE_TEST
+ 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 0ce2da9..7ef950a 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -42,3 +42,4 @@ obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
obj-$(CONFIG_DMA_JZ4740) += dma-jz4740.o
obj-$(CONFIG_TI_CPPI41) += cppi41.o
obj-$(CONFIG_K3_DMA) += k3dma.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..7a84b02
--- /dev/null
+++ b/drivers/dma/qcom_bam_dma.c
@@ -0,0 +1,843 @@
+/*
+ * QCOM BAM DMA engine driver
+ *
+ * Copyright (c) 2013, 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 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 linux/kernel.h
+#include linux/io.h
+#include linux/init.h
+#include linux/slab.h
+#include linux/module.h
+#include linux/interrupt.h
+#include linux/dma-mapping.h
+#include linux/scatterlist.h
+#include linux/device.h
+#include linux/platform_device.h
+#include linux/of.h
+#include linux/of_address.h
+#include linux/of_irq.h
+#include linux/of_dma.h
+#include linux/clk.h
+
+#include dmaengine.h
+#include qcom_bam_dma.h
+
+
+/**
+ * bam_reset_channel - Reset individual BAM DMA channel
+ * @bdev: bam device
+ * @channel: channel id
+ */
+static void bam_reset_channel(struct bam_device *bdev, u32 channel)
+{
+ /* reset channel */
+ writel_relaxed(1, bdev-regs + BAM_P_RST(channel));
+ writel_relaxed(0, bdev-regs + BAM_P_RST(channel));
+}
+
+/**
+ * bam_alloc_chan - Allocate channel resources for DMA channel.
+ * @chan: specified channel
+ *
+ * This function allocates the FIFO descriptor memory and resets the channel
+ */
+static int bam_alloc_chan(struct dma_chan *chan)
+{
+ struct bam_chan *bchan = to_bam_chan(chan);
+ struct bam_device *bdev = bchan-bdev;
+ u32 val;
+
+ /* allocate FIFO descriptor space, but only if necessary */
+ if (!bchan-fifo_virt) {
+ bchan-fifo_virt = dma_alloc_coherent(bdev-dev,
+ BAM_DESC_FIFO_SIZE, bchan-fifo_phys,
+ GFP_KERNEL);
+
+
