[PATCH 0/8 v2] k3dma patches to add support for hi3660/HiKey960

[PATCH 0/8 v2] k3dma patches to add support for hi3660/HiKey960

[John Stultz]

This patch series is based on recent work by Tanglei Han, and
adds support for hi3660 SoCs as found on the HiKey960 board,
along with a few patches I've been carrying.

Review and feedback would be greatly appreciated!

thanks
-john

Cc: Tanglei Han <hantanglei@huawei.com>
Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Ryan Grachek <ryan@edited.us>
CC: devicetree@vger.kernel.org
Cc: dmaengine@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org

John Stultz (3):
  Documentation: bindings: k3dma: Add binding for dma-avail-chan
  arm64: dts: hi3660: Add dma to uart nodes
  arm64: dts: hi3660: Fixup unofficial dma-min-chan to dma-avail-chan

Li Yu (2):
  dma: k3dma: Delete axi_config
  dma: k3dma: Add support to dma_avail_chan

Youlin Wang (3):
  Documentation: bindings: k3dma: Extend the k3dma driver binding to
    support hisi-asp
  dma: k3dma: Upgrade k3dma driver to support hisi_asp_dma hardware
  arm64: dts: hi3660: Add hisi asp dma device

 Documentation/devicetree/bindings/dma/k3dma.txt |  7 ++-
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi       | 22 ++++++++-
 drivers/dma/k3dma.c                             | 60 +++++++++++++++++++++----
 3 files changed, 78 insertions(+), 11 deletions(-)

-- 
2.7.4

[PATCH 1/8 v2] Documentation: bindings: k3dma: Extend the k3dma driver binding to support hisi-asp

[John Stultz]

From: Youlin Wang <wwx575822@notesmail.huawei.com>

Extend the k3dma driver binding to support hisi-asp hardware
variants.

Cc: Vinod Koul <vkoul@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
Cc: Tanglei Han <hantanglei@huawei.com>
Cc: Ryan Grachek <ryan@edited.us>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: dmaengine@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
Signed-off-by: Tanglei Han <hantanglei@huawei.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2: Simplify patch, removing extranious examples
---
 Documentation/devicetree/bindings/dma/k3dma.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
index 4945aea..10a2f15 100644
--- a/Documentation/devicetree/bindings/dma/k3dma.txt
+++ b/Documentation/devicetree/bindings/dma/k3dma.txt
@@ -3,7 +3,9 @@
 See dma.txt first
 
 Required properties:
-- compatible: Should be "hisilicon,k3-dma-1.0"
+- compatible: Must be one of
+-              "hisilicon,k3-dma-1.0"
+-              "hisilicon,hisi-pcm-asp-dma-1.0"
 - reg: Should contain DMA registers location and length.
 - interrupts: Should contain one interrupt shared by all channel
 - #dma-cells: see dma.txt, should be 1, para number
-- 
2.7.4

[PATCH 2/8 v2] Documentation: bindings: k3dma: Add binding for dma-avail-chan

[John Stultz]

Some dma channels can be reserved for secure mode or other
hardware on the SoC, so provide a binding for a bitmask
listing the available channels for the kernel to use.

Cc: Vinod Koul <vkoul@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Tanglei Han <hantanglei@huawei.com>
Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
Cc: Ryan Grachek <ryan@edited.us>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: dmaengine@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 Documentation/devicetree/bindings/dma/k3dma.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
index 10a2f15..1c466c1 100644
--- a/Documentation/devicetree/bindings/dma/k3dma.txt
+++ b/Documentation/devicetree/bindings/dma/k3dma.txt
@@ -14,6 +14,9 @@ Required properties:
 		have specific request line
 - clocks: clock required
 
+Optional properties:
+- dma-avail-chan: Bitmask of available physical channels
+
 Example:
 
 Controller:
-- 
2.7.4

[PATCH 3/8 v2] dma: k3dma: Upgrade k3dma driver to support hisi_asp_dma hardware

[John Stultz]

From: Youlin Wang <wwx575822@notesmail.huawei.com>

There is an new "hisi-pcm-asp-dma-1.0" device added in
"arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
So we have to add a matching id in the driver file:
 .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"

And also hisi-pcm-asp dma device needs no setting to the clock.
So we skip this by adding and using soc data flags.

After above this driver will support both k3 and hisi_asp dma hardware.

Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
Cc: Ryan Grachek <ryan@edited.us>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: dmaengine@vger.kernel.org
Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
Signed-off-by: Tanglei Han <hantanglei@huawei.com>
[jstultz: Reworked to use of_match_data]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2:
* Reworked to use of_match_data
---
 drivers/dma/k3dma.c | 37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index fdec2b6..df61406 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -116,6 +116,13 @@ struct k3_dma_dev {
 	unsigned int		irq;
 };
 
+
+#define K3_FLAG_NOCLK  (1<<0)
+struct k3dma_soc_data {
+	unsigned long flags;
+};
+
+
 #define to_k3_dma(dmadev) container_of(dmadev, struct k3_dma_dev, slave)
 
 static int k3_dma_config_write(struct dma_chan *chan,
@@ -790,8 +797,21 @@ static int k3_dma_transfer_resume(struct dma_chan *chan)
 	return 0;
 }
 
+static const struct k3dma_soc_data k3_v1_dma_data = {
+	.flags = 0,
+};
+
+static const struct k3dma_soc_data asp_v1_dma_data = {
+	.flags = K3_FLAG_NOCLK,
+};
+
 static const struct of_device_id k3_pdma_dt_ids[] = {
-	{ .compatible = "hisilicon,k3-dma-1.0", },
+	{ .compatible = "hisilicon,k3-dma-1.0",
+	  .data = &k3_v1_dma_data
+	},
+	{ .compatible = "hisilicon,hisi-pcm-asp-dma-1.0",
+	  .data = &asp_v1_dma_data
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, k3_pdma_dt_ids);
@@ -810,6 +830,7 @@ static struct dma_chan *k3_of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 
 static int k3_dma_probe(struct platform_device *op)
 {
+	const struct k3dma_soc_data *soc_data;
 	struct k3_dma_dev *d;
 	const struct of_device_id *of_id;
 	struct resource *iores;
@@ -823,6 +844,10 @@ static int k3_dma_probe(struct platform_device *op)
 	if (!d)
 		return -ENOMEM;
 
+	soc_data = device_get_match_data(&op->dev);
+	if (!soc_data)
+		return -EINVAL;
+
 	d->base = devm_ioremap_resource(&op->dev, iores);
 	if (IS_ERR(d->base))
 		return PTR_ERR(d->base);
@@ -835,10 +860,12 @@ static int k3_dma_probe(struct platform_device *op)
 				"dma-requests", &d->dma_requests);
 	}
 
-	d->clk = devm_clk_get(&op->dev, NULL);
-	if (IS_ERR(d->clk)) {
-		dev_err(&op->dev, "no dma clk\n");
-		return PTR_ERR(d->clk);
+	if (!(soc_data->flags & K3_FLAG_NOCLK)) {
+		d->clk = devm_clk_get(&op->dev, NULL);
+		if (IS_ERR(d->clk)) {
+			dev_err(&op->dev, "no dma clk\n");
+			return PTR_ERR(d->clk);
+		}
 	}
 
 	irq = platform_get_irq(op, 0);
-- 
2.7.4

[PATCH 4/8 v2] dma: k3dma: Delete axi_config

[John Stultz]

From: Li Yu <liyu65@hisilicon.com>

Axi_config controls whether DMA resources can be accessed in non-secure
mode, such as linux kernel. The register should be set by the bootloader
stage and depends on the device.

Thus, this patch removes axi_config from k3dma driver.

Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Tanglei Han <hantanglei@huawei.com>
Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
Cc: Ryan Grachek <ryan@edited.us>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: dmaengine@vger.kernel.org
Signed-off-by: Li Yu <liyu65@hisilicon.com>
Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
[jstultz: Minor tweaks to commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma/k3dma.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index df61406..b2060bf 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -52,8 +52,6 @@
 #define CX_SRC			0x814
 #define CX_DST			0x818
 #define CX_CFG			0x81c
-#define AXI_CFG			0x820
-#define AXI_CFG_DEFAULT		0x201201
 
 #define CX_LLI_CHAIN_EN		0x2
 #define CX_CFG_EN		0x1
@@ -168,7 +166,6 @@ static void k3_dma_set_desc(struct k3_dma_phy *phy, struct k3_desc_hw *hw)
 	writel_relaxed(hw->count, phy->base + CX_CNT0);
 	writel_relaxed(hw->saddr, phy->base + CX_SRC);
 	writel_relaxed(hw->daddr, phy->base + CX_DST);
-	writel_relaxed(AXI_CFG_DEFAULT, phy->base + AXI_CFG);
 	writel_relaxed(hw->config, phy->base + CX_CFG);
 }
 
-- 
2.7.4

[PATCH 5/8 v2] dma: k3dma: Add support to dma_avail_chan

[John Stultz]

From: Li Yu <liyu65@hisilicon.com>

dma_avail_chan as a property for k3dma, it defines available dma
channels which a non-secure mode driver can use.

One sample usage of this is in Hi3660 SoC. DMA channel 0 is
reserved to lpm3, which is a coprocessor for power management. So
as a result, any request in kernel (which runs on main processor
and in non-secure mode) should start from at least channel 1.

Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Tanglei Han <hantanglei@huawei.com>
Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
Cc: Ryan Grachek <ryan@edited.us>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: dmaengine@vger.kernel.org
Signed-off-by: Li Yu <liyu65@hisilicon.com>
[jstultz: Reworked to use a channel mask]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/dma/k3dma.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
index b2060bf..431094b 100644
--- a/drivers/dma/k3dma.c
+++ b/drivers/dma/k3dma.c
@@ -111,6 +111,7 @@ struct k3_dma_dev {
 	struct dma_pool		*pool;
 	u32			dma_channels;
 	u32			dma_requests;
+	u32			dma_avail_chan;
 	unsigned int		irq;
 };
 
@@ -318,6 +319,9 @@ static void k3_dma_tasklet(unsigned long arg)
 	/* check new channel request in d->chan_pending */
 	spin_lock_irq(&d->lock);
 	for (pch = 0; pch < d->dma_channels; pch++) {
+		if (!(d->dma_avail_chan & (1<<pch)))
+			continue;
+
 		p = &d->phy[pch];
 
 		if (p->vchan == NULL && !list_empty(&d->chan_pending)) {
@@ -335,6 +339,9 @@ static void k3_dma_tasklet(unsigned long arg)
 	spin_unlock_irq(&d->lock);
 
 	for (pch = 0; pch < d->dma_channels; pch++) {
+		if (!(d->dma_avail_chan & (1<<pch)))
+			continue;
+
 		if (pch_alloc & (1 << pch)) {
 			p = &d->phy[pch];
 			c = p->vchan;
@@ -855,6 +862,13 @@ static int k3_dma_probe(struct platform_device *op)
 				"dma-channels", &d->dma_channels);
 		of_property_read_u32((&op->dev)->of_node,
 				"dma-requests", &d->dma_requests);
+		ret = of_property_read_u32((&op->dev)->of_node,
+				"dma-avail-chan", &d->dma_avail_chan);
+		if (ret) {
+			dev_warn(&op->dev,
+				 "dma-avail-chan doesn't exist, considering all as available.\n");
+			d->dma_avail_chan = (u32)~0UL;
+		}
 	}
 
 	if (!(soc_data->flags & K3_FLAG_NOCLK)) {
@@ -886,8 +900,12 @@ static int k3_dma_probe(struct platform_device *op)
 		return -ENOMEM;
 
 	for (i = 0; i < d->dma_channels; i++) {
-		struct k3_dma_phy *p = &d->phy[i];
+		struct k3_dma_phy *p;
+
+		if (!(d->dma_avail_chan & (1<<i)))
+			continue;
 
+		p = &d->phy[i];
 		p->idx = i;
 		p->base = d->base + i * 0x40;
 	}
-- 
2.7.4

[PATCH 6/8 v2] arm64: dts: hi3660: Add dma to uart nodes

[John Stultz]

Try to add DMA support to the uart nodes following
the assignments made in the dts from the victoria vendor kernel
here:
https://consumer.huawei.com/en/opensource/detail/?siteCode=worldwide&keywords=p10&fileType=openSourceSoftware&pageSize=10&curPage=1

Cc: Tanglei Han <hantanglei@huawei.com>
Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
Cc: Ryan Grachek <ryan@edited.us>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index 20ae40d..aaa2b04 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -466,6 +466,8 @@
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x0 0xfdf02000 0x0 0x1000>;
 			interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
+			dma-names = "rx", "tx";
+			dmas =  <&dma0 0 &dma0 1>;
 			clocks = <&crg_ctrl HI3660_CLK_MUX_UART0>,
 				 <&crg_ctrl HI3660_PCLK>;
 			clock-names = "uartclk", "apb_pclk";
@@ -478,6 +480,8 @@
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x0 0xfdf00000 0x0 0x1000>;
 			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
+			dma-names = "rx", "tx";
+			dmas =  <&dma0 2 &dma0 3>;
 			clocks = <&crg_ctrl HI3660_CLK_GATE_UART1>,
 				 <&crg_ctrl HI3660_CLK_GATE_UART1>;
 			clock-names = "uartclk", "apb_pclk";
@@ -490,6 +494,8 @@
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x0 0xfdf03000 0x0 0x1000>;
 			interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
+			dma-names = "rx", "tx";
+			dmas =  <&dma0 4 &dma0 5>;
 			clocks = <&crg_ctrl HI3660_CLK_GATE_UART2>,
 				 <&crg_ctrl HI3660_PCLK>;
 			clock-names = "uartclk", "apb_pclk";
@@ -514,6 +520,8 @@
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x0 0xfdf01000 0x0 0x1000>;
 			interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
+			dma-names = "rx", "tx";
+			dmas =  <&dma0 6 &dma0 7>;
 			clocks = <&crg_ctrl HI3660_CLK_GATE_UART4>,
 				 <&crg_ctrl HI3660_CLK_GATE_UART4>;
 			clock-names = "uartclk", "apb_pclk";
@@ -526,6 +534,8 @@
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x0 0xfdf05000 0x0 0x1000>;
 			interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
+			dma-names = "rx", "tx";
+			dmas =  <&dma0 8 &dma0 9>;
 			clocks = <&crg_ctrl HI3660_CLK_GATE_UART5>,
 				 <&crg_ctrl HI3660_CLK_GATE_UART5>;
 			clock-names = "uartclk", "apb_pclk";
-- 
2.7.4

[PATCH 7/8 v2] arm64: dts: hi3660: Add hisi asp dma device

[John Stultz]

From: Youlin Wang <wwx575822@notesmail.huawei.com>

Add asp-dma device to hi3660 dts

Cc: Tanglei Han <hantanglei@huawei.com>
Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
Cc: Ryan Grachek <ryan@edited.us>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
Signed-off-by: Tanglei Han <hantanglei@huawei.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2: Removed undocumented bindings
---
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index aaa2b04..df4e96d 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -569,6 +569,16 @@
 			dma-type = "hi3660_dma";
 		};
 
+		asp_dmac: dma-controller@e804b000 {
+			compatible = "hisilicon,hisi-pcm-asp-dma-1.0";
+			reg = <0x0 0xe804b000 0x0 0x1000>;
+			#dma-cells = <1>;
+			dma-channels = <16>;
+			dma-requests = <32>;
+			interrupts = <GIC_SPI 216 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "asp_dma_irq";
+		};
+
 		rtc0: rtc@fff04000 {
 			compatible = "arm,pl031", "arm,primecell";
 			reg = <0x0 0Xfff04000 0x0 0x1000>;
-- 
2.7.4

[PATCH 8/8 v2] arm64: dts: hi3660: Fixup unofficial dma-min-chan to dma-avail-chan

[John Stultz]

A undocumented and unimplemented binding got into the hi3660 dtsi,
and this switches that binding to the now documented one.

Cc: Tanglei Han <hantanglei@huawei.com>
Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
Cc: Ryan Grachek <ryan@edited.us>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index df4e96d..654da63 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -562,7 +562,7 @@
 			#dma-cells = <1>;
 			dma-channels = <16>;
 			dma-requests = <32>;
-			dma-min-chan = <1>;
+			dma-avail-chan = <0xfffe>;
 			interrupts = <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&crg_ctrl HI3660_CLK_GATE_DMAC>;
 			dma-no-cci;
-- 
2.7.4

Re: [PATCH 0/8 v2] k3dma patches to add support for hi3660/HiKey960

[Manivannan Sadhasivam]

Hi John,

On Fri, Jan 04, 2019 at 12:56:20PM -0800, John Stultz wrote:
> This patch series is based on recent work by Tanglei Han, and
> adds support for hi3660 SoCs as found on the HiKey960 board,

Not sure about the description/subject here! This patchset adds support
for ASP DMAC found in HI3660 SoCs, Peripheral DMAC is already supported.

Thanks,
Mani

> along with a few patches I've been carrying.
> 
> Review and feedback would be greatly appreciated!
> 
> thanks
> -john
> 
> Cc: Tanglei Han <hantanglei@huawei.com>
> Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: Ryan Grachek <ryan@edited.us>
> CC: devicetree@vger.kernel.org
> Cc: dmaengine@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> 
> John Stultz (3):
>   Documentation: bindings: k3dma: Add binding for dma-avail-chan
>   arm64: dts: hi3660: Add dma to uart nodes
>   arm64: dts: hi3660: Fixup unofficial dma-min-chan to dma-avail-chan
> 
> Li Yu (2):
>   dma: k3dma: Delete axi_config
>   dma: k3dma: Add support to dma_avail_chan
> 
> Youlin Wang (3):
>   Documentation: bindings: k3dma: Extend the k3dma driver binding to
>     support hisi-asp
>   dma: k3dma: Upgrade k3dma driver to support hisi_asp_dma hardware
>   arm64: dts: hi3660: Add hisi asp dma device
> 
>  Documentation/devicetree/bindings/dma/k3dma.txt |  7 ++-
>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi       | 22 ++++++++-
>  drivers/dma/k3dma.c                             | 60 +++++++++++++++++++++----
>  3 files changed, 78 insertions(+), 11 deletions(-)
> 
> -- 
> 2.7.4
> 

Re: [PATCH 3/8 v2] dma: k3dma: Upgrade k3dma driver to support hisi_asp_dma hardware

[Manivannan Sadhasivam]

Hi John,

On Fri, Jan 04, 2019 at 12:56:23PM -0800, John Stultz wrote:
> From: Youlin Wang <wwx575822@notesmail.huawei.com>
> 
> There is an new "hisi-pcm-asp-dma-1.0" device added in
> "arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
> So we have to add a matching id in the driver file:
>  .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"
> 
> And also hisi-pcm-asp dma device needs no setting to the clock.
> So we skip this by adding and using soc data flags.
> 
> After above this driver will support both k3 and hisi_asp dma hardware.
> 

Small description about the hardware (ASP DMAC) would be really helpful.

Thanks,
Mani

> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> Cc: Ryan Grachek <ryan@edited.us>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: dmaengine@vger.kernel.org
> Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
> Signed-off-by: Tanglei Han <hantanglei@huawei.com>
> [jstultz: Reworked to use of_match_data]
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v2:
> * Reworked to use of_match_data
> ---
>  drivers/dma/k3dma.c | 37 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
> index fdec2b6..df61406 100644
> --- a/drivers/dma/k3dma.c
> +++ b/drivers/dma/k3dma.c
> @@ -116,6 +116,13 @@ struct k3_dma_dev {
>  	unsigned int		irq;
>  };
>  
> +
> +#define K3_FLAG_NOCLK  (1<<0)
> +struct k3dma_soc_data {
> +	unsigned long flags;
> +};
> +
> +
>  #define to_k3_dma(dmadev) container_of(dmadev, struct k3_dma_dev, slave)
>  
>  static int k3_dma_config_write(struct dma_chan *chan,
> @@ -790,8 +797,21 @@ static int k3_dma_transfer_resume(struct dma_chan *chan)
>  	return 0;
>  }
>  
> +static const struct k3dma_soc_data k3_v1_dma_data = {
> +	.flags = 0,
> +};
> +
> +static const struct k3dma_soc_data asp_v1_dma_data = {
> +	.flags = K3_FLAG_NOCLK,
> +};
> +
>  static const struct of_device_id k3_pdma_dt_ids[] = {
> -	{ .compatible = "hisilicon,k3-dma-1.0", },
> +	{ .compatible = "hisilicon,k3-dma-1.0",
> +	  .data = &k3_v1_dma_data
> +	},
> +	{ .compatible = "hisilicon,hisi-pcm-asp-dma-1.0",
> +	  .data = &asp_v1_dma_data
> +	},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, k3_pdma_dt_ids);
> @@ -810,6 +830,7 @@ static struct dma_chan *k3_of_dma_simple_xlate(struct of_phandle_args *dma_spec,
>  
>  static int k3_dma_probe(struct platform_device *op)
>  {
> +	const struct k3dma_soc_data *soc_data;
>  	struct k3_dma_dev *d;
>  	const struct of_device_id *of_id;
>  	struct resource *iores;
> @@ -823,6 +844,10 @@ static int k3_dma_probe(struct platform_device *op)
>  	if (!d)
>  		return -ENOMEM;
>  
> +	soc_data = device_get_match_data(&op->dev);
> +	if (!soc_data)
> +		return -EINVAL;
> +
>  	d->base = devm_ioremap_resource(&op->dev, iores);
>  	if (IS_ERR(d->base))
>  		return PTR_ERR(d->base);
> @@ -835,10 +860,12 @@ static int k3_dma_probe(struct platform_device *op)
>  				"dma-requests", &d->dma_requests);
>  	}
>  
> -	d->clk = devm_clk_get(&op->dev, NULL);
> -	if (IS_ERR(d->clk)) {
> -		dev_err(&op->dev, "no dma clk\n");
> -		return PTR_ERR(d->clk);
> +	if (!(soc_data->flags & K3_FLAG_NOCLK)) {
> +		d->clk = devm_clk_get(&op->dev, NULL);
> +		if (IS_ERR(d->clk)) {
> +			dev_err(&op->dev, "no dma clk\n");
> +			return PTR_ERR(d->clk);
> +		}
>  	}
>  
>  	irq = platform_get_irq(op, 0);
> -- 
> 2.7.4
> 

Re: [PATCH 4/8 v2] dma: k3dma: Delete axi_config

[Manivannan Sadhasivam]

On Fri, Jan 04, 2019 at 12:56:24PM -0800, John Stultz wrote:
> From: Li Yu <liyu65@hisilicon.com>
> 
> Axi_config controls whether DMA resources can be accessed in non-secure
> mode, such as linux kernel. The register should be set by the bootloader
> stage and depends on the device.
> 
> Thus, this patch removes axi_config from k3dma driver.
> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Tanglei Han <hantanglei@huawei.com>
> Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> Cc: Ryan Grachek <ryan@edited.us>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: dmaengine@vger.kernel.org
> Signed-off-by: Li Yu <liyu65@hisilicon.com>
> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
> [jstultz: Minor tweaks to commit message]
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/dma/k3dma.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
> index df61406..b2060bf 100644
> --- a/drivers/dma/k3dma.c
> +++ b/drivers/dma/k3dma.c
> @@ -52,8 +52,6 @@
>  #define CX_SRC			0x814
>  #define CX_DST			0x818
>  #define CX_CFG			0x81c
> -#define AXI_CFG			0x820
> -#define AXI_CFG_DEFAULT		0x201201
>  
>  #define CX_LLI_CHAIN_EN		0x2
>  #define CX_CFG_EN		0x1
> @@ -168,7 +166,6 @@ static void k3_dma_set_desc(struct k3_dma_phy *phy, struct k3_desc_hw *hw)
>  	writel_relaxed(hw->count, phy->base + CX_CNT0);
>  	writel_relaxed(hw->saddr, phy->base + CX_SRC);
>  	writel_relaxed(hw->daddr, phy->base + CX_DST);
> -	writel_relaxed(AXI_CFG_DEFAULT, phy->base + AXI_CFG);
>  	writel_relaxed(hw->config, phy->base + CX_CFG);
>  }
>  
> -- 
> 2.7.4
> 

Re: [PATCH 6/8 v2] arm64: dts: hi3660: Add dma to uart nodes

[Manivannan Sadhasivam]

Hi John,

On Fri, Jan 04, 2019 at 12:56:26PM -0800, John Stultz wrote:
> Try to add DMA support to the uart nodes following
> the assignments made in the dts from the victoria vendor kernel
> here:
> https://consumer.huawei.com/en/opensource/detail/?siteCode=worldwide&keywords=p10&fileType=openSourceSoftware&pageSize=10&curPage=1
> 
> Cc: Tanglei Han <hantanglei@huawei.com>
> Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> Cc: Ryan Grachek <ryan@edited.us>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index 20ae40d..aaa2b04 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -466,6 +466,8 @@
>  			compatible = "arm,pl011", "arm,primecell";
>  			reg = <0x0 0xfdf02000 0x0 0x1000>;
>  			interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
> +			dma-names = "rx", "tx";
> +			dmas =  <&dma0 0 &dma0 1>;

Usage of DMA channel 0 contradicts with the description provided in
patch, "dma: k3dma: Add support to dma_avail_chan".

Thanks,
Mani

>  			clocks = <&crg_ctrl HI3660_CLK_MUX_UART0>,
>  				 <&crg_ctrl HI3660_PCLK>;
>  			clock-names = "uartclk", "apb_pclk";
> @@ -478,6 +480,8 @@
>  			compatible = "arm,pl011", "arm,primecell";
>  			reg = <0x0 0xfdf00000 0x0 0x1000>;
>  			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
> +			dma-names = "rx", "tx";
> +			dmas =  <&dma0 2 &dma0 3>;
>  			clocks = <&crg_ctrl HI3660_CLK_GATE_UART1>,
>  				 <&crg_ctrl HI3660_CLK_GATE_UART1>;
>  			clock-names = "uartclk", "apb_pclk";
> @@ -490,6 +494,8 @@
>  			compatible = "arm,pl011", "arm,primecell";
>  			reg = <0x0 0xfdf03000 0x0 0x1000>;
>  			interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
> +			dma-names = "rx", "tx";
> +			dmas =  <&dma0 4 &dma0 5>;
>  			clocks = <&crg_ctrl HI3660_CLK_GATE_UART2>,
>  				 <&crg_ctrl HI3660_PCLK>;
>  			clock-names = "uartclk", "apb_pclk";
> @@ -514,6 +520,8 @@
>  			compatible = "arm,pl011", "arm,primecell";
>  			reg = <0x0 0xfdf01000 0x0 0x1000>;
>  			interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
> +			dma-names = "rx", "tx";
> +			dmas =  <&dma0 6 &dma0 7>;
>  			clocks = <&crg_ctrl HI3660_CLK_GATE_UART4>,
>  				 <&crg_ctrl HI3660_CLK_GATE_UART4>;
>  			clock-names = "uartclk", "apb_pclk";
> @@ -526,6 +534,8 @@
>  			compatible = "arm,pl011", "arm,primecell";
>  			reg = <0x0 0xfdf05000 0x0 0x1000>;
>  			interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
> +			dma-names = "rx", "tx";
> +			dmas =  <&dma0 8 &dma0 9>;
>  			clocks = <&crg_ctrl HI3660_CLK_GATE_UART5>,
>  				 <&crg_ctrl HI3660_CLK_GATE_UART5>;
>  			clock-names = "uartclk", "apb_pclk";
> -- 
> 2.7.4
> 

Re: [PATCH 7/8 v2] arm64: dts: hi3660: Add hisi asp dma device

[Manivannan Sadhasivam]

On Fri, Jan 04, 2019 at 12:56:27PM -0800, John Stultz wrote:
> From: Youlin Wang <wwx575822@notesmail.huawei.com>
> 
> Add asp-dma device to hi3660 dts
> 
> Cc: Tanglei Han <hantanglei@huawei.com>
> Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> Cc: Ryan Grachek <ryan@edited.us>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
> Signed-off-by: Tanglei Han <hantanglei@huawei.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
> v2: Removed undocumented bindings
> ---
>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index aaa2b04..df4e96d 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -569,6 +569,16 @@
>  			dma-type = "hi3660_dma";
>  		};
>  
> +		asp_dmac: dma-controller@e804b000 {
> +			compatible = "hisilicon,hisi-pcm-asp-dma-1.0";
> +			reg = <0x0 0xe804b000 0x0 0x1000>;
> +			#dma-cells = <1>;
> +			dma-channels = <16>;
> +			dma-requests = <32>;
> +			interrupts = <GIC_SPI 216 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "asp_dma_irq";
> +		};
> +
>  		rtc0: rtc@fff04000 {
>  			compatible = "arm,pl031", "arm,primecell";
>  			reg = <0x0 0Xfff04000 0x0 0x1000>;
> -- 
> 2.7.4
> 

Re: [PATCH 2/8 v2] Documentation: bindings: k3dma: Add binding for dma-avail-chan

[Manivannan Sadhasivam]

Hi John,

On Fri, Jan 04, 2019 at 12:56:22PM -0800, John Stultz wrote:
> Some dma channels can be reserved for secure mode or other
> hardware on the SoC, so provide a binding for a bitmask
> listing the available channels for the kernel to use.
> 
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Tanglei Han <hantanglei@huawei.com>
> Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> Cc: Ryan Grachek <ryan@edited.us>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: dmaengine@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  Documentation/devicetree/bindings/dma/k3dma.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
> index 10a2f15..1c466c1 100644
> --- a/Documentation/devicetree/bindings/dma/k3dma.txt
> +++ b/Documentation/devicetree/bindings/dma/k3dma.txt
> @@ -14,6 +14,9 @@ Required properties:
>  		have specific request line
>  - clocks: clock required
>  
> +Optional properties:
> +- dma-avail-chan: Bitmask of available physical channels
> +

This property looks too generic. Since this is specific to HiSi SoCs,
this could be "hisi-dma-avail-chan"?

Thanks,
Mani

>  Example:
>  
>  Controller:
> -- 
> 2.7.4
> 

Re: [PATCH 0/8 v2] k3dma patches to add support for hi3660/HiKey960

[John Stultz]

On Fri, Jan 4, 2019 at 7:37 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> Hi John,
>
> On Fri, Jan 04, 2019 at 12:56:20PM -0800, John Stultz wrote:
> > This patch series is based on recent work by Tanglei Han, and
> > adds support for hi3660 SoCs as found on the HiKey960 board,
>
> Not sure about the description/subject here! This patchset adds support
> for ASP DMAC found in HI3660 SoCs, Peripheral DMAC is already supported.
>

So yes, it does enable ASP DMAC support, but the full set also enables
the Peripheral DMAC as well in a few ways:
* Avoiding setting the AXI register, which is configured differently on hi3660.
* By adding support for the dma-avail-chan (so we don't try to use
reserved channels)

thanks
-john

Re: [PATCH 6/8 v2] arm64: dts: hi3660: Add dma to uart nodes

[John Stultz]

On Fri, Jan 4, 2019 at 7:49 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> Hi John,
>
> On Fri, Jan 04, 2019 at 12:56:26PM -0800, John Stultz wrote:
> > Try to add DMA support to the uart nodes following
> > the assignments made in the dts from the victoria vendor kernel
> > here:
> > https://consumer.huawei.com/en/opensource/detail/?siteCode=worldwide&keywords=p10&fileType=openSourceSoftware&pageSize=10&curPage=1
> >
> > Cc: Tanglei Han <hantanglei@huawei.com>
> > Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> > Cc: Ryan Grachek <ryan@edited.us>
> > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Cc: Wei Xu <xuwei5@hisilicon.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > ---
> >  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > index 20ae40d..aaa2b04 100644
> > --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > @@ -466,6 +466,8 @@
> >                       compatible = "arm,pl011", "arm,primecell";
> >                       reg = <0x0 0xfdf02000 0x0 0x1000>;
> >                       interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
> > +                     dma-names = "rx", "tx";
> > +                     dmas =  <&dma0 0 &dma0 1>;
>
> Usage of DMA channel 0 contradicts with the description provided in
> patch, "dma: k3dma: Add support to dma_avail_chan".

Hrm. Good point.  I'll double check w/ Dr Su on this, I'm not sure if
that inconsistency is due to the the vendor kernel (where these came
from) having different reserved channels or just something overlooked
if the uart0 is not actually being used (as we find on hikey960 as
well).

thanks
-john

Re: [PATCH 2/8 v2] Documentation: bindings: k3dma: Add binding for dma-avail-chan

[John Stultz]

On Fri, Jan 4, 2019 at 8:00 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> Hi John,
>
> On Fri, Jan 04, 2019 at 12:56:22PM -0800, John Stultz wrote:
> > Some dma channels can be reserved for secure mode or other
> > hardware on the SoC, so provide a binding for a bitmask
> > listing the available channels for the kernel to use.
> >
> > Cc: Vinod Koul <vkoul@kernel.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Tanglei Han <hantanglei@huawei.com>
> > Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> > Cc: Ryan Grachek <ryan@edited.us>
> > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Cc: dmaengine@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/dma/k3dma.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
> > index 10a2f15..1c466c1 100644
> > --- a/Documentation/devicetree/bindings/dma/k3dma.txt
> > +++ b/Documentation/devicetree/bindings/dma/k3dma.txt
> > @@ -14,6 +14,9 @@ Required properties:
> >               have specific request line
> >  - clocks: clock required
> >
> > +Optional properties:
> > +- dma-avail-chan: Bitmask of available physical channels
> > +
>
> This property looks too generic. Since this is specific to HiSi SoCs,
> this could be "hisi-dma-avail-chan"?

I'm fine to change it, but I'm not sure I fully understand the
rational. Can you help me understand?
Are device node-binding names supposed to have global scope? I assumed
the node property names are basically scoped to the entry?
Further, having some dma channels be reserved doesn't seem to be too
unique a concept, so I'm not sure what we gain long term by prefixing
it?

thanks
-john

Re: [PATCH 2/8 v2] Documentation: bindings: k3dma: Add binding for dma-avail-chan

[Manivannan Sadhasivam]

On Fri, Jan 04, 2019 at 08:39:34PM -0800, John Stultz wrote:
> On Fri, Jan 4, 2019 at 8:00 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > Hi John,
> >
> > On Fri, Jan 04, 2019 at 12:56:22PM -0800, John Stultz wrote:
> > > Some dma channels can be reserved for secure mode or other
> > > hardware on the SoC, so provide a binding for a bitmask
> > > listing the available channels for the kernel to use.
> > >
> > > Cc: Vinod Koul <vkoul@kernel.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Tanglei Han <hantanglei@huawei.com>
> > > Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> > > Cc: Ryan Grachek <ryan@edited.us>
> > > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Cc: dmaengine@vger.kernel.org
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > > ---
> > >  Documentation/devicetree/bindings/dma/k3dma.txt | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > index 10a2f15..1c466c1 100644
> > > --- a/Documentation/devicetree/bindings/dma/k3dma.txt
> > > +++ b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > @@ -14,6 +14,9 @@ Required properties:
> > >               have specific request line
> > >  - clocks: clock required
> > >
> > > +Optional properties:
> > > +- dma-avail-chan: Bitmask of available physical channels
> > > +
> >
> > This property looks too generic. Since this is specific to HiSi SoCs,
> > this could be "hisi-dma-avail-chan"?
> 
> I'm fine to change it, but I'm not sure I fully understand the
> rational. Can you help me understand?
> Are device node-binding names supposed to have global scope? I assumed
> the node property names are basically scoped to the entry?

IIUC properties documented in subsystem binding (dma.txt in this case)
will have global scope. Those which are not documented in this binding
are specific to vendor IPs and should be prefixed with the vendor
prefix (hisi in this case).

> Further, having some dma channels be reserved doesn't seem to be too
> unique a concept, so I'm not sure what we gain long term by prefixing
> it?
> 

Right, but this brings up the point of having this functionality in
generic DMA engine so that the DMA controller drivers need not handle.
So either we should move this available channel check to DMA Engine
and document the property in dma.txt so that other IPs can also use it
or keep the functionality in K3 driver and use HiSi prefix for the
property.

But I'd like to hear Vinod/Rob's opinion on this!

Thanks,
Mani

> thanks
> -john

Re: [PATCH 2/8 v2] Documentation: bindings: k3dma: Add binding for dma-avail-chan

[John Stultz]

On Fri, Jan 4, 2019 at 8:53 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Fri, Jan 04, 2019 at 08:39:34PM -0800, John Stultz wrote:
> > On Fri, Jan 4, 2019 at 8:00 PM Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > Hi John,
> > >
> > > On Fri, Jan 04, 2019 at 12:56:22PM -0800, John Stultz wrote:
> > > > Some dma channels can be reserved for secure mode or other
> > > > hardware on the SoC, so provide a binding for a bitmask
> > > > listing the available channels for the kernel to use.
> > > >
> > > > Cc: Vinod Koul <vkoul@kernel.org>
> > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > Cc: Tanglei Han <hantanglei@huawei.com>
> > > > Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> > > > Cc: Ryan Grachek <ryan@edited.us>
> > > > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > Cc: dmaengine@vger.kernel.org
> > > > Cc: devicetree@vger.kernel.org
> > > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > > > ---
> > > >  Documentation/devicetree/bindings/dma/k3dma.txt | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > index 10a2f15..1c466c1 100644
> > > > --- a/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > +++ b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > @@ -14,6 +14,9 @@ Required properties:
> > > >               have specific request line
> > > >  - clocks: clock required
> > > >
> > > > +Optional properties:
> > > > +- dma-avail-chan: Bitmask of available physical channels
> > > > +
> > >
> > > This property looks too generic. Since this is specific to HiSi SoCs,
> > > this could be "hisi-dma-avail-chan"?
> >
> > I'm fine to change it, but I'm not sure I fully understand the
> > rational. Can you help me understand?
> > Are device node-binding names supposed to have global scope? I assumed
> > the node property names are basically scoped to the entry?
>
> IIUC properties documented in subsystem binding (dma.txt in this case)
> will have global scope. Those which are not documented in this binding
> are specific to vendor IPs and should be prefixed with the vendor
> prefix (hisi in this case).

Thanks I appreciate the explanation here. I hadn't really understood
this point, and really haven't developed much "taste" in what makes a
good or bad binding.

> > Further, having some dma channels be reserved doesn't seem to be too
> > unique a concept, so I'm not sure what we gain long term by prefixing
> > it?
> >
>
> Right, but this brings up the point of having this functionality in
> generic DMA engine so that the DMA controller drivers need not handle.
> So either we should move this available channel check to DMA Engine
> and document the property in dma.txt so that other IPs can also use it
> or keep the functionality in K3 driver and use HiSi prefix for the
> property.
>
> But I'd like to hear Vinod/Rob's opinion on this!

Sure. Though for now I'll prefix it as the logic is handled at the
driver level.

thanks
-john

Re: [PATCH 3/8 v2] dma: k3dma: Upgrade k3dma driver to support hisi_asp_dma hardware

[John Stultz]

On Fri, Jan 4, 2019 at 7:42 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
> On Fri, Jan 04, 2019 at 12:56:23PM -0800, John Stultz wrote:
> > From: Youlin Wang <wwx575822@notesmail.huawei.com>
> >
> > There is an new "hisi-pcm-asp-dma-1.0" device added in
> > "arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
> > So we have to add a matching id in the driver file:
> >  .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"
> >
> > And also hisi-pcm-asp dma device needs no setting to the clock.
> > So we skip this by adding and using soc data flags.
> >
> > After above this driver will support both k3 and hisi_asp dma hardware.
> >
>
> Small description about the hardware (ASP DMAC) would be really helpful.

I've taken a shot at this (along with integrating your other feedback
- thanks again for the review!), though as I don't have direct
documentation, my knowledge is a bit second hand.

See here:
https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=754a79facf1af0b59a7f8fd63050da12ebf5521e

Let me know if you have suggestions for more specific changes!
thanks
-john

Re: [PATCH 6/8 v2] arm64: dts: hi3660: Add dma to uart nodes

[John Stultz]

On Fri, Jan 4, 2019 at 8:34 PM John Stultz <john.stultz@linaro.org> wrote:
>
> On Fri, Jan 4, 2019 at 7:49 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > Hi John,
> >
> > On Fri, Jan 04, 2019 at 12:56:26PM -0800, John Stultz wrote:
> > > Try to add DMA support to the uart nodes following
> > > the assignments made in the dts from the victoria vendor kernel
> > > here:
> > > https://consumer.huawei.com/en/opensource/detail/?siteCode=worldwide&keywords=p10&fileType=openSourceSoftware&pageSize=10&curPage=1
> > >
> > > Cc: Tanglei Han <hantanglei@huawei.com>
> > > Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> > > Cc: Ryan Grachek <ryan@edited.us>
> > > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Cc: Wei Xu <xuwei5@hisilicon.com>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: linux-arm-kernel@lists.infradead.org
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > > ---
> > >  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > > index 20ae40d..aaa2b04 100644
> > > --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > > +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > > @@ -466,6 +466,8 @@
> > >                       compatible = "arm,pl011", "arm,primecell";
> > >                       reg = <0x0 0xfdf02000 0x0 0x1000>;
> > >                       interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
> > > +                     dma-names = "rx", "tx";
> > > +                     dmas =  <&dma0 0 &dma0 1>;
> >
> > Usage of DMA channel 0 contradicts with the description provided in
> > patch, "dma: k3dma: Add support to dma_avail_chan".
>
> Hrm. Good point.  I'll double check w/ Dr Su on this, I'm not sure if
> that inconsistency is due to the the vendor kernel (where these came
> from) having different reserved channels or just something overlooked
> if the uart0 is not actually being used (as we find on hikey960 as
> well).

Hrm. So it seems like uart0 is mapped to the dma0 chan0, but the
device specific files in the vendor tree overwite the dma values:
                serial0: uart@fdf02000 {
                        pinctrl-names = "default", "idle";
                        pinctrl-0 = <&gpio053_pmx_func
&gpio054_pmx_func &gpio053_cfg_func &gpio054_cfg_func>;
                        pinctrl-1 = <&gpio053_pmx_idle
&gpio054_pmx_idle &gpio053_cfg_idle &gpio054_cfg_idle>;
                        dma-names = "", "";
                        dmas = <>;
                        clock-rate = <0 19200000>;
                        status = "ok";
                };

But I went ahead and pulled the dma values on uart0.

thaks
-john

Re: [PATCH 3/8 v2] dma: k3dma: Upgrade k3dma driver to support hisi_asp_dma hardware

[Manivannan Sadhasivam]

On Fri, Jan 04, 2019 at 09:22:46PM -0800, John Stultz wrote:
> On Fri, Jan 4, 2019 at 7:42 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> > On Fri, Jan 04, 2019 at 12:56:23PM -0800, John Stultz wrote:
> > > From: Youlin Wang <wwx575822@notesmail.huawei.com>
> > >
> > > There is an new "hisi-pcm-asp-dma-1.0" device added in
> > > "arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
> > > So we have to add a matching id in the driver file:
> > >  .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"
> > >
> > > And also hisi-pcm-asp dma device needs no setting to the clock.
> > > So we skip this by adding and using soc data flags.
> > >
> > > After above this driver will support both k3 and hisi_asp dma hardware.
> > >
> >
> > Small description about the hardware (ASP DMAC) would be really helpful.
> 
> I've taken a shot at this (along with integrating your other feedback
> - thanks again for the review!), though as I don't have direct
> documentation, my knowledge is a bit second hand.
> 
> See here:
> https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=754a79facf1af0b59a7f8fd63050da12ebf5521e
> 
> Let me know if you have suggestions for more specific changes!

Description looks good to me. But looks like you are not protecting the
other clk APIs like clk_prepare_enable and clk_disable_unprepare. Don't
they fail when the relevant clk is not found?

Thanks,
Mani

> thanks
> -john

Re: [PATCH 3/8 v2] dma: k3dma: Upgrade k3dma driver to support hisi_asp_dma hardware

[John Stultz]

On Fri, Jan 4, 2019 at 9:39 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Fri, Jan 04, 2019 at 09:22:46PM -0800, John Stultz wrote:
> > On Fri, Jan 4, 2019 at 7:42 PM Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > > On Fri, Jan 04, 2019 at 12:56:23PM -0800, John Stultz wrote:
> > > > From: Youlin Wang <wwx575822@notesmail.huawei.com>
> > > >
> > > > There is an new "hisi-pcm-asp-dma-1.0" device added in
> > > > "arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
> > > > So we have to add a matching id in the driver file:
> > > >  .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"
> > > >
> > > > And also hisi-pcm-asp dma device needs no setting to the clock.
> > > > So we skip this by adding and using soc data flags.
> > > >
> > > > After above this driver will support both k3 and hisi_asp dma hardware.
> > > >
> > >
> > > Small description about the hardware (ASP DMAC) would be really helpful.
> >
> > I've taken a shot at this (along with integrating your other feedback
> > - thanks again for the review!), though as I don't have direct
> > documentation, my knowledge is a bit second hand.
> >
> > See here:
> > https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=754a79facf1af0b59a7f8fd63050da12ebf5521e
> >
> > Let me know if you have suggestions for more specific changes!
>
> Description looks good to me. But looks like you are not protecting the
> other clk APIs like clk_prepare_enable and clk_disable_unprepare. Don't
> they fail when the relevant clk is not found?

No, those calls just return 0 if null clock is passed

int clk_prepare(struct clk *clk)
{
        if (!clk)
                return 0;
...

thanks
-john

Re: [PATCH 3/8 v2] dma: k3dma: Upgrade k3dma driver to support hisi_asp_dma hardware

[Manivannan Sadhasivam]

On Fri, Jan 04, 2019 at 09:44:17PM -0800, John Stultz wrote:
> On Fri, Jan 4, 2019 at 9:39 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Fri, Jan 04, 2019 at 09:22:46PM -0800, John Stultz wrote:
> > > On Fri, Jan 4, 2019 at 7:42 PM Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > > On Fri, Jan 04, 2019 at 12:56:23PM -0800, John Stultz wrote:
> > > > > From: Youlin Wang <wwx575822@notesmail.huawei.com>
> > > > >
> > > > > There is an new "hisi-pcm-asp-dma-1.0" device added in
> > > > > "arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
> > > > > So we have to add a matching id in the driver file:
> > > > >  .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"
> > > > >
> > > > > And also hisi-pcm-asp dma device needs no setting to the clock.
> > > > > So we skip this by adding and using soc data flags.
> > > > >
> > > > > After above this driver will support both k3 and hisi_asp dma hardware.
> > > > >
> > > >
> > > > Small description about the hardware (ASP DMAC) would be really helpful.
> > >
> > > I've taken a shot at this (along with integrating your other feedback
> > > - thanks again for the review!), though as I don't have direct
> > > documentation, my knowledge is a bit second hand.
> > >
> > > See here:
> > > https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=754a79facf1af0b59a7f8fd63050da12ebf5521e
> > >
> > > Let me know if you have suggestions for more specific changes!
> >
> > Description looks good to me. But looks like you are not protecting the
> > other clk APIs like clk_prepare_enable and clk_disable_unprepare. Don't
> > they fail when the relevant clk is not found?
> 
> No, those calls just return 0 if null clock is passed
> 
> int clk_prepare(struct clk *clk)
> {
>         if (!clk)
>                 return 0;
> ...

Ah, okay. Didn't look into the definition.

So with the change in description,

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> 
> thanks
> -john

Re: [PATCH 2/8 v2] Documentation: bindings: k3dma: Add binding for dma-avail-chan

[Vinod Koul]

On 05-01-19, 10:23, Manivannan Sadhasivam wrote:
> On Fri, Jan 04, 2019 at 08:39:34PM -0800, John Stultz wrote:
> > On Fri, Jan 4, 2019 at 8:00 PM Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > Hi John,
> > >
> > > On Fri, Jan 04, 2019 at 12:56:22PM -0800, John Stultz wrote:
> > > > Some dma channels can be reserved for secure mode or other
> > > > hardware on the SoC, so provide a binding for a bitmask
> > > > listing the available channels for the kernel to use.
> > > >
> > > > Cc: Vinod Koul <vkoul@kernel.org>
> > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > Cc: Tanglei Han <hantanglei@huawei.com>
> > > > Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> > > > Cc: Ryan Grachek <ryan@edited.us>
> > > > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > Cc: dmaengine@vger.kernel.org
> > > > Cc: devicetree@vger.kernel.org
> > > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > > > ---
> > > >  Documentation/devicetree/bindings/dma/k3dma.txt | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > index 10a2f15..1c466c1 100644
> > > > --- a/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > +++ b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > @@ -14,6 +14,9 @@ Required properties:
> > > >               have specific request line
> > > >  - clocks: clock required
> > > >
> > > > +Optional properties:
> > > > +- dma-avail-chan: Bitmask of available physical channels
> > > > +
> > >
> > > This property looks too generic. Since this is specific to HiSi SoCs,
> > > this could be "hisi-dma-avail-chan"?
> > 
> > I'm fine to change it, but I'm not sure I fully understand the
> > rational. Can you help me understand?
> > Are device node-binding names supposed to have global scope? I assumed
> > the node property names are basically scoped to the entry?
> 
> IIUC properties documented in subsystem binding (dma.txt in this case)
> will have global scope. Those which are not documented in this binding
> are specific to vendor IPs and should be prefixed with the vendor
> prefix (hisi in this case).
> 
> > Further, having some dma channels be reserved doesn't seem to be too
> > unique a concept, so I'm not sure what we gain long term by prefixing
> > it?
> > 
> 
> Right, but this brings up the point of having this functionality in
> generic DMA engine so that the DMA controller drivers need not handle.
> So either we should move this available channel check to DMA Engine
> and document the property in dma.txt so that other IPs can also use it
> or keep the functionality in K3 driver and use HiSi prefix for the
> property.
> 
> But I'd like to hear Vinod/Rob's opinion on this!

So there are two parts, first is if this new property of using 'some'
channels of controller is generic enough, the answer is unfortunately
yes, so we should move this to dma.txt as a generic property

But I don't agree the dmaengine core should handle it, we may add
helpers, but controllers registers N channels and they would do so, core
should not do filtering

-- 
~Vinod

Re: [PATCH 2/8 v2] Documentation: bindings: k3dma: Add binding for dma-avail-chan

[Manivannan Sadhasivam]

Hi Vinod,

On Sat, Jan 05, 2019 at 07:16:10PM +0530, Vinod Koul wrote:
> On 05-01-19, 10:23, Manivannan Sadhasivam wrote:
> > On Fri, Jan 04, 2019 at 08:39:34PM -0800, John Stultz wrote:
> > > On Fri, Jan 4, 2019 at 8:00 PM Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > >
> > > > Hi John,
> > > >
> > > > On Fri, Jan 04, 2019 at 12:56:22PM -0800, John Stultz wrote:
> > > > > Some dma channels can be reserved for secure mode or other
> > > > > hardware on the SoC, so provide a binding for a bitmask
> > > > > listing the available channels for the kernel to use.
> > > > >
> > > > > Cc: Vinod Koul <vkoul@kernel.org>
> > > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > > Cc: Tanglei Han <hantanglei@huawei.com>
> > > > > Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> > > > > Cc: Ryan Grachek <ryan@edited.us>
> > > > > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > Cc: dmaengine@vger.kernel.org
> > > > > Cc: devicetree@vger.kernel.org
> > > > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/dma/k3dma.txt | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > > index 10a2f15..1c466c1 100644
> > > > > --- a/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > > +++ b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > > @@ -14,6 +14,9 @@ Required properties:
> > > > >               have specific request line
> > > > >  - clocks: clock required
> > > > >
> > > > > +Optional properties:
> > > > > +- dma-avail-chan: Bitmask of available physical channels
> > > > > +
> > > >
> > > > This property looks too generic. Since this is specific to HiSi SoCs,
> > > > this could be "hisi-dma-avail-chan"?
> > > 
> > > I'm fine to change it, but I'm not sure I fully understand the
> > > rational. Can you help me understand?
> > > Are device node-binding names supposed to have global scope? I assumed
> > > the node property names are basically scoped to the entry?
> > 
> > IIUC properties documented in subsystem binding (dma.txt in this case)
> > will have global scope. Those which are not documented in this binding
> > are specific to vendor IPs and should be prefixed with the vendor
> > prefix (hisi in this case).
> > 
> > > Further, having some dma channels be reserved doesn't seem to be too
> > > unique a concept, so I'm not sure what we gain long term by prefixing
> > > it?
> > > 
> > 
> > Right, but this brings up the point of having this functionality in
> > generic DMA engine so that the DMA controller drivers need not handle.
> > So either we should move this available channel check to DMA Engine
> > and document the property in dma.txt so that other IPs can also use it
> > or keep the functionality in K3 driver and use HiSi prefix for the
> > property.
> > 
> > But I'd like to hear Vinod/Rob's opinion on this!
> 
> So there are two parts, first is if this new property of using 'some'
> channels of controller is generic enough, the answer is unfortunately
> yes, so we should move this to dma.txt as a generic property
> 
> But I don't agree the dmaengine core should handle it, we may add
> helpers, but controllers registers N channels and they would do so, core
> should not do filtering
> 

Okay. But won't it create ambiguity? What if a new driver developer
assmes that he can use this property to filter the channels for his own
DMA controller? Since we are _explicitly_ stating that these channels
should be filtered, why the dmaengine core can't handle it?

If the property is generic, then it makes sense to keep the
functionality also generic.

Thanks,
Mani

> -- 
> ~Vinod

Re: [PATCH 2/8 v2] Documentation: bindings: k3dma: Add binding for dma-avail-chan

[Vinod Koul]

On 05-01-19, 19:38, Manivannan Sadhasivam wrote:
> Hi Vinod,
> 
> On Sat, Jan 05, 2019 at 07:16:10PM +0530, Vinod Koul wrote:
> > On 05-01-19, 10:23, Manivannan Sadhasivam wrote:
> > > On Fri, Jan 04, 2019 at 08:39:34PM -0800, John Stultz wrote:
> > > > On Fri, Jan 4, 2019 at 8:00 PM Manivannan Sadhasivam
> > > > <manivannan.sadhasivam@linaro.org> wrote:
> > > > >
> > > > > Hi John,
> > > > >
> > > > > On Fri, Jan 04, 2019 at 12:56:22PM -0800, John Stultz wrote:
> > > > > > Some dma channels can be reserved for secure mode or other
> > > > > > hardware on the SoC, so provide a binding for a bitmask
> > > > > > listing the available channels for the kernel to use.
> > > > > >
> > > > > > Cc: Vinod Koul <vkoul@kernel.org>
> > > > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > > > Cc: Tanglei Han <hantanglei@huawei.com>
> > > > > > Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> > > > > > Cc: Ryan Grachek <ryan@edited.us>
> > > > > > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > > > Cc: dmaengine@vger.kernel.org
> > > > > > Cc: devicetree@vger.kernel.org
> > > > > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/dma/k3dma.txt | 3 +++
> > > > > >  1 file changed, 3 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/dma/k3dma.txt b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > > > index 10a2f15..1c466c1 100644
> > > > > > --- a/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > > > +++ b/Documentation/devicetree/bindings/dma/k3dma.txt
> > > > > > @@ -14,6 +14,9 @@ Required properties:
> > > > > >               have specific request line
> > > > > >  - clocks: clock required
> > > > > >
> > > > > > +Optional properties:
> > > > > > +- dma-avail-chan: Bitmask of available physical channels
> > > > > > +
> > > > >
> > > > > This property looks too generic. Since this is specific to HiSi SoCs,
> > > > > this could be "hisi-dma-avail-chan"?
> > > > 
> > > > I'm fine to change it, but I'm not sure I fully understand the
> > > > rational. Can you help me understand?
> > > > Are device node-binding names supposed to have global scope? I assumed
> > > > the node property names are basically scoped to the entry?
> > > 
> > > IIUC properties documented in subsystem binding (dma.txt in this case)
> > > will have global scope. Those which are not documented in this binding
> > > are specific to vendor IPs and should be prefixed with the vendor
> > > prefix (hisi in this case).
> > > 
> > > > Further, having some dma channels be reserved doesn't seem to be too
> > > > unique a concept, so I'm not sure what we gain long term by prefixing
> > > > it?
> > > > 
> > > 
> > > Right, but this brings up the point of having this functionality in
> > > generic DMA engine so that the DMA controller drivers need not handle.
> > > So either we should move this available channel check to DMA Engine
> > > and document the property in dma.txt so that other IPs can also use it
> > > or keep the functionality in K3 driver and use HiSi prefix for the
> > > property.
> > > 
> > > But I'd like to hear Vinod/Rob's opinion on this!
> > 
> > So there are two parts, first is if this new property of using 'some'
> > channels of controller is generic enough, the answer is unfortunately
> > yes, so we should move this to dma.txt as a generic property
> > 
> > But I don't agree the dmaengine core should handle it, we may add
> > helpers, but controllers registers N channels and they would do so, core
> > should not do filtering
> > 
> 
> Okay. But won't it create ambiguity? What if a new driver developer
> assmes that he can use this property to filter the channels for his own
> DMA controller? Since we are _explicitly_ stating that these channels
> should be filtered, why the dmaengine core can't handle it?
> 
> If the property is generic, then it makes sense to keep the
> functionality also generic.

Core doesnt have a view of channels to be filtered, it looks at N
channels getting registered and works on those.

Till now folks do not create channels for 'filtered' ones and register
the ones kernel can use..

-- 
~Vinod