Skip to contentSkip to conversation

Add Qualcomm BAM dmaengine driver

PATCH Superseded Andy Gross 2 participants 9 messages Feb 21, 2014

Subsystems (3)

v6 of 9 Superseded Latest is v9 →

lore
Share & tools
lore

Andy Gross

author
#0 lore raw

v6 cover Add Qualcomm BAM dmaengine driver

This patch set introduces the dmaengine driver for the Qualcomm Bus Access Manager (BAM) DMA controller present on MSM 8x74 devices. A number of the on-chip devices have their own BAM DMA controller and use it to move data between system memory and peripherals or between two peripherals.

The initial version of this driver will only support slave DMA operations between system memory and peripherals.

Changes from v5:
- Separated copyright from BAM description
- Removed usage of slave dma direction
- Reworked slave config interface
- Removed max segment size restriction in slave_sg
- Added acked-by on DT patch
- Fixed nit comments

Changes from v4:
- Add devm_free_irq() to .remove to avoid race condition
- Free FIFO memory in .remove

Changes from v3:
- Remove unused bam_channel_dir.
- Remove incorrect write to BAM_IRQ_SRCS_EE (read only).
- Remove dma direction from DT binding and revise driver to use direction from prep_slave_sg.
- Remove unnecessary channel reset from channel_init. This could affect channels controlled from other execution environments.
- Change terminate_all to also take care of the current active descriptor.
- Rework .remove function to correctly mask interrupts and clean up resources and tasklets.

Changes from v2:
- Corrected Kconfig dependencies
- Moved execution environment ID to controller DT binding. The EE is a global setting across all of the channels on the controller.
- Combined header into source file.
- Corrected copyright date.
- Moved channel hardware initialization to occur when channel is used for the first time.
- Converted dma_alloc_coherent to dma_alloc_writecombine
- Removed unecessary reset of channel from the dma terminate_all
- Corrected usage of EE in irq handler and channel configuration functions.
- Changed resource functions inside probe to use correct APIs.
- Removed dma filter function and modified dma_xlate to use dma_get_slave_channel API
- Fixed various nit comments

Changes from v1:
- Converted driver to use virt-dma
- Reworked probe function per review comments
- tx_status function now computes and returns residuals
- Removed proprietary slave config. Removed associated include file.
- Renamed files to reflect vendor name instead of specific device
- Converted to use (readl|writel)_relaxed w/ appropriate barriers
- Removed unions in favor of standard types.

Andy Gross (2):
dmaengine: qcom_bam_dma: Add device tree binding
dmaengine: add Qualcomm BAM dma driver

 .../devicetree/bindings/dma/qcom_bam_dma.txt       |   48 +
 drivers/dma/Kconfig                                |    9 +
 drivers/dma/Makefile                               |    1 +
 drivers/dma/qcom_bam_dma.c                         | 1107 ++++++++++++++++++++
 4 files changed, 1165 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
 create mode 100644 drivers/dma/qcom_bam_dma.c
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
author
#1 lore raw

PATCH v6 1/2 dmaengine: qcom_bam_dma: Add device tree binding

Add device tree binding support for the QCOM BAM DMA driver.

---
 .../devicetree/bindings/dma/qcom_bam_dma.txt       |   48 ++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +48 -0
new file mode 100644
index 0000000..86344f1
@@ -0,0 +1,48 @@
+QCOM BAM DMA controller
+
+Required properties:
+- compatible:	Must be "qcom,bam-v1.4.0" for MSM8974 V1
+		Must be "qcom,bam-v1.4.1" for MSM8974 V2
+- reg: Address range for DMA registers
+- interrupts: single interrupt for this controller
+- #dma-cells: must be <1>
+- clocks: required clock
+- clock-names: name of clock
+- qcom,ee : indicates the active Execution Environment identifier (0-7)
+
+Example:
+
+	uart-bam: dma at f9984000 = {
+		compatible = "qcom,bam-v1.4.1";
+		reg = <0xf9984000 0x15000>;
+		interrupts = <0 94 0>;
+		clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
+		clock-names = "bam_clk";
+		#dma-cells = <1>;
+		qcom,ee = <0>;
+	};
+
+Client:
+Required properties:
+- dmas: List of dma channel requests
+- dma-names: Names of aforementioned requested channels
+
+Clients must use the format described in the dma.txt file, using a two cell
+specifier for each channel.
+
+The three cells in order are:
+  1. A phandle pointing to the DMA controller
+  2. The channel number
+
+Example:
+	serial at f991e000 {
+		compatible = "qcom,msm-uart";
+		reg = <0xf991e000 0x1000>
+			<0xf9944000 0x19000>;
+		interrupts = <0 108 0>;
+		clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
+		clock-names = "core", "iface";
+
+		dmas = <&uart-bam 0>, <&uart-bam 1>;
+		dma-names = "rx", "tx";
+	};
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#2 lore raw

Re: [Patch v6 1/2] dmaengine: qcom_bam_dma: Add device tree binding

Quoted 15 lines · Andy Gross

Add device tree binding support for the QCOM BAM DMA driver.

---
 .../devicetree/bindings/dma/qcom_bam_dma.txt       |   48 ++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +0 -0
new file mode 100644
index 0000000..86344f1

Documentation/devicetree/bindings/dma/qcom_bam_dma.txt line 5

Quoted 5 lines
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +5 -0
@@ -0,0 +1,48 @@
+QCOM BAM DMA controller
+
+Required properties:
+- compatible:	Must be "qcom,bam-v1.4.0" for MSM8974 V1
+		Must be "qcom,bam-v1.4.1" for MSM8974 V2

This looks a bit odd. How about:

- compatible: must contain:
* "qcom,bam-v1.4.0" for MSM8974 V1
* "qcom,bam-v1.4.1" for MSM8974 V2

Documentation/devicetree/bindings/dma/qcom_bam_dma.txt line 7

Quoted 2 lines
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +2 -0
+- reg: Address range for DMA registers
+- interrupts: single interrupt for this controller

This device only has a single interrupt? Or there's only one we care about at the moment?

Documentation/devicetree/bindings/dma/qcom_bam_dma.txt line 10

Quoted 3 lines
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +3 -0
+- #dma-cells: must be <1>
+- clocks: required clock
+- clock-names: name of clock

Either describe the _exact_ name this binding expects for any clocks input, or get rid of clock-names. I would prefer the former.

Quoted 8 lines
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +8 -0
+- qcom,ee : indicates the active Execution Environment identifier (0-7)
+
+Example:
+
+	uart-bam: dma at f9984000 = {
+		compatible = "qcom,bam-v1.4.1";
+		reg = <0xf9984000 0x15000>;
+		interrupts = <0 94 0>;

Documentation/devicetree/bindings/dma/qcom_bam_dma.txt line 28

Quoted 10 lines
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +10 -0
+		clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
+		clock-names = "bam_clk";
+		#dma-cells = <1>;
+		qcom,ee = <0>;
+	};
+
+Client:
+Required properties:
+- dmas: List of dma channel requests
+- dma-names: Names of aforementioned requested channels

Do we really need to describe the client binding? Do we not have a generic DMA binding doc we can refer to?

Documentation/devicetree/bindings/dma/qcom_bam_dma.txt line 35

Quoted 7 lines
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +7 -0
+
+Clients must use the format described in the dma.txt file, using a two cell
+specifier for each channel.
+
+The three cells in order are:
+  1. A phandle pointing to the DMA controller
+  2. The channel number

s/three/two/

I think this can go if we refer to a generic document for the client binding and state in the #dma-cells description that the single dma cell is the channel number.

Thanks,
Mark.

author
#3 lore raw

Re: [Patch v6 1/2] dmaengine: qcom_bam_dma: Add device tree binding

Quoted 3 lines · Mark Rutland
Quoted 2 lines · Andy Gross

Add device tree binding support for the QCOM BAM DMA driver.

[snip]

Quoted 10 lines

Documentation/devicetree/bindings/dma/qcom_bam_dma.txt line 5

Quoted 3 lines
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +3 -0
+Required properties:
+- compatible:	Must be "qcom,bam-v1.4.0" for MSM8974 V1
+		Must be "qcom,bam-v1.4.1" for MSM8974 V2

This looks a bit odd. How about:

- compatible: must contain:
* "qcom,bam-v1.4.0" for MSM8974 V1
* "qcom,bam-v1.4.1" for MSM8974 V2

Agreed. I'll fix that.

Quoted 6 lines
Quoted 2 lines
+- reg: Address range for DMA registers
+- interrupts: single interrupt for this controller

This device only has a single interrupt? Or there's only one we care about at the moment?

Yes only a single interrupt. I can remove the s.

Quoted 6 lines
Quoted 3 lines
+- #dma-cells: must be <1>
+- clocks: required clock
+- clock-names: name of clock

Either describe the _exact_ name this binding expects for any clocks input, or get rid of clock-names. I would prefer the former.

So then I'd use the full name in the driver file. That's fine.

[snip]

Quoted 8 lines

Documentation/devicetree/bindings/dma/qcom_bam_dma.txt line 28

Quoted 4 lines
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +4 -0
+Client:
+Required properties:
+- dmas: List of dma channel requests
+- dma-names: Names of aforementioned requested channels

Do we really need to describe the client binding? Do we not have a generic DMA binding doc we can refer to?

Originally there was more information in the entry. It was pared down over time and I didn't go back to referencing the generic doc. I'll do this and that'll fix this and your next comment.

[snip]

Signature · 3 lines
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
#4 lore raw

Re: [Patch v6 1/2] dmaengine: qcom_bam_dma: Add device tree binding

Quoted 18 lines · Andy Gross
Quoted 3 lines · Mark Rutland
Quoted 2 lines · Andy Gross

Add device tree binding support for the QCOM BAM DMA driver.

[snip]

Quoted 10 lines

Documentation/devicetree/bindings/dma/qcom_bam_dma.txt line 5

Quoted 3 lines
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +3 -0
+Required properties:
+- compatible:	Must be "qcom,bam-v1.4.0" for MSM8974 V1
+		Must be "qcom,bam-v1.4.1" for MSM8974 V2

This looks a bit odd. How about:

- compatible: must contain:
* "qcom,bam-v1.4.0" for MSM8974 V1
* "qcom,bam-v1.4.1" for MSM8974 V2

Agreed. I'll fix that.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
+- reg: Address range for DMA registers
+- interrupts: single interrupt for this controller

This device only has a single interrupt? Or there's only one we care about at the moment?

Yes only a single interrupt. I can remove the s.

Please don't, the interrupts proeprty is standard and shouldn't change.

I was only asking to ensure that all interrupts were described in the binding, which they are. :)

Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
+- #dma-cells: must be <1>
+- clocks: required clock
+- clock-names: name of clock

Either describe the _exact_ name this binding expects for any clocks input, or get rid of clock-names. I would prefer the former.

So then I'd use the full name in the driver file. That's fine.

Yup.

Quoted 11 lines

[snip]

Quoted 8 lines

Documentation/devicetree/bindings/dma/qcom_bam_dma.txt line 28

Quoted 4 lines
Documentation/devicetree/bindings/dma/qcom_bam_dma.txt new file +4 -0
+Client:
+Required properties:
+- dmas: List of dma channel requests
+- dma-names: Names of aforementioned requested channels

Do we really need to describe the client binding? Do we not have a generic DMA binding doc we can refer to?

Quoted 3 lines

Originally there was more information in the entry. It was pared down over time and I didn't go back to referencing the generic doc. I'll do this and that'll fix this and your next comment.

Sounds good!

Cheers,
Mark.

author
#5 lore raw

Re: [Patch v6 1/2] dmaengine: qcom_bam_dma: Add device tree binding

On Fri, Feb 21, 2014 at 05:36:47PM +0000, Mark Rutland wrote:
[snip]

Quoted 8 lines
Quoted 2 lines

Yes only a single interrupt. I can remove the s.

Please don't, the interrupts proeprty is standard and shouldn't change.

I was only asking to ensure that all interrupts were described in the binding, which they are. :)

Responding to emails before coffee is bad.....

Signature · 3 lines
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
author
#6 lore raw

PATCH v6 2/2 dmaengine: add Qualcomm BAM dma driver

In reply to Andy Gross

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.

---
 drivers/dma/Kconfig        |    9 +
 drivers/dma/Makefile       |    1 +
 drivers/dma/qcom_bam_dma.c | 1107 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1117 insertions(+)
 create mode 100644 drivers/dma/qcom_bam_dma.c
drivers/dma/Kconfig +9 -0
index 605b016..c13e1f8 100644
@@ -401,4 +401,13 @@ config DMATEST
 config DMA_ENGINE_RAID
 	bool
 
+config QCOM_BAM_DMA
+	tristate "QCOM BAM DMA support"
+	depends on ARCH_MSM_DT || (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
drivers/dma/Makefile +1 -0
index a029d0f4..907b915 100644
@@ -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
drivers/dma/qcom_bam_dma.c new file +1107 -0

1,107 lines not on this page · raw message

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#7 lore raw

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

Quoted 18 lines · Andy Gross

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.

---
 drivers/dma/Kconfig        |    9 +
 drivers/dma/Makefile       |    1 +
 drivers/dma/qcom_bam_dma.c | 1107 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1117 insertions(+)
 create mode 100644 drivers/dma/qcom_bam_dma.c

[...]

Quoted 1 line

+ bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");

The binding document should describe the "bam_clk" string in the clock-names description.

[...]

drivers/dma/qcom_bam_dma.c line 984

Quoted 3 lines
drivers/dma/qcom_bam_dma.c new file +3 -0
+       ret = of_property_read_u32(pdev->dev.of_node, "qcom,ee", &bdev->ee);
+       if (ret) {
+               dev_err(bdev->dev, "EE unspecified\n");

It would be nice to expand EE to what it stands for, at least in the error but possibly the binding too.

drivers/dma/qcom_bam_dma.c line 1088

Quoted 3 lines
drivers/dma/qcom_bam_dma.c new file +3 -0
+static const struct of_device_id bam_of_match[] = {
+       { .compatible = "qcom,bam-v1.4.0", },
+       { .compatible = "qcom,bam-v1.4.1", },

What are the differences between the two?

Is v1.4.1 an extension of v1.4.0?

Could you have the following in DTs:

  compatible = "qcom,bam-v1.4.1", "qcom,bam-v1.4.0";

Then you'd only need the v1.4.0 string in the driver for now.

Thanks,
Mark.

author
#8 lore raw

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

Quoted 1 line · Mark Rutland

On Fri, Feb 21, 2014 at 06:43:05AM +0000, Andy Gross wrote:

[snip]

Quoted 5 lines
Quoted 1 line

+ bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");

The binding document should describe the "bam_clk" string in the clock-names description.

OK.

Quoted 6 lines

drivers/dma/qcom_bam_dma.c line 984

Quoted 3 lines
drivers/dma/qcom_bam_dma.c new file +3 -0
+       ret = of_property_read_u32(pdev->dev.of_node, "qcom,ee", &bdev->ee);
+       if (ret) {
+               dev_err(bdev->dev, "EE unspecified\n");

It would be nice to expand EE to what it stands for, at least in the error but possibly the binding too.

I can expand it to execution environment. This is a trustzone thing and is required from a security perspective.

Quoted 7 lines

drivers/dma/qcom_bam_dma.c line 1088

Quoted 3 lines
drivers/dma/qcom_bam_dma.c new file +3 -0
+static const struct of_device_id bam_of_match[] = {
+       { .compatible = "qcom,bam-v1.4.0", },
+       { .compatible = "qcom,bam-v1.4.1", },

What are the differences between the two?

Is v1.4.1 an extension of v1.4.0?

On second look, the 1.4.1 has hardware fixes. So the software API is the same. I thought there were some configuration changes, but that isn't the case.

Quoted 5 lines

Could you have the following in DTs:

  compatible = "qcom,bam-v1.4.1", "qcom,bam-v1.4.0";

Then you'd only need the v1.4.0 string in the driver for now.

Good point. That's easy to change.

Signature · 3 lines
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