linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] dmaengine: mediatek-cqdma: add dt-bindings and remove redundant queue
@ 2020-05-28  9:57 EastL
  2020-05-28  9:57 ` [PATCH v4 1/4] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings EastL
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: EastL @ 2020-05-28  9:57 UTC (permalink / raw)
  To: Sean Wang
  Cc: vkoul, robh+dt, mark.rutland, matthias.bgg, dmaengine,
	linux-kernel, linux-arm-kernel, linux-mediatek, devicetree,
	wsd_upstream

This patch set adds document the devicetree bindings for MediaTek Command-Queue DMA controller,
and remove redundant queue structure, add dma-channel-mask for DMA capability and fix compatible.

Changes since v3:
- fix dt_binding_check errors

Changes since v2:
- add devicetree bindings for MediaTek Command-Queue DMA controller

Changes since v1:
- remove redundant queue structure
- fix wrong description and tags in the earlier patch
- add dma-channel-mask for DMA capability
- fix compatible for common

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/4] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings
  2020-05-28  9:57 [PATCH v4] dmaengine: mediatek-cqdma: add dt-bindings and remove redundant queue EastL
@ 2020-05-28  9:57 ` EastL
  2020-05-29 19:24   ` Rob Herring
  2020-05-28  9:57 ` [PATCH v4 3/4] dmaengine: mediatek-cqdma: fix compatible EastL
  2020-05-28  9:57 ` [PATCH v4 4/4] dmaengine: mediatek-cqdma: add dma mask for capability EastL
  2 siblings, 1 reply; 10+ messages in thread
From: EastL @ 2020-05-28  9:57 UTC (permalink / raw)
  To: Sean Wang
  Cc: vkoul, robh+dt, mark.rutland, matthias.bgg, dmaengine,
	linux-kernel, linux-arm-kernel, linux-mediatek, devicetree,
	wsd_upstream, EastL

Document the devicetree bindings for MediaTek Command-Queue DMA controller
which could be found on MT6779 SoC or other similar Mediatek SoCs.

Signed-off-by: EastL <EastL.Lee@mediatek.com>
---
 .../devicetree/bindings/dma/mtk-cqdma.yaml         | 100 +++++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/mtk-cqdma.yaml

diff --git a/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml b/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
new file mode 100644
index 0000000..045aa0c
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/mtk-cqdma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Command-Queue DMA controller Device Tree Binding
+
+maintainers:
+  - EastL <EastL.Lee@mediatek.com>
+
+description:
+  MediaTek Command-Queue DMA controller (CQDMA) on Mediatek SoC
+  is dedicated to memory-to-memory transfer through queue based
+  descriptor management.
+
+properties:
+  "#dma-cells":
+    minimum: 1
+    # Should be enough
+    maximum: 255
+    description:
+      Used to provide DMA controller specific information.
+
+  compatible:
+    const: mediatek,cqdma
+
+  reg:
+    minItems: 1
+    maxItems: 255
+
+  interrupts:
+    minItems: 1
+    maxItems: 255
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: cqdma
+
+  dma-channel-mask:
+    description:
+      Bitmask of available DMA channels in ascending order that are
+      not reserved by firmware and are available to the
+      kernel. i.e. first channel corresponds to LSB.
+      The first item in the array is for channels 0-31, the second is for
+      channels 32-63, etc.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      minItems: 1
+      # Should be enough
+      maxItems: 255
+
+  dma-channels:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+      Number of DMA channels supported by the controller.
+
+  dma-requests:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+      Number of DMA request signals supported by the controller.
+
+required:
+  - "#dma-cells"
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - dma-channel-mask
+  - dma-channels
+  - dma-requests
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/mt6779-clk.h>
+    cqdma: dma-controller@10212000 {
+        compatible = "mediatek,cqdma";
+        reg = <0 0x10212000 0 0x80>,
+            <0 0x10212080 0 0x80>,
+            <0 0x10212100 0 0x80>;
+        interrupts = <GIC_SPI 139 IRQ_TYPE_LEVEL_LOW>,
+            <GIC_SPI 140 IRQ_TYPE_LEVEL_LOW>,
+            <GIC_SPI 141 IRQ_TYPE_LEVEL_LOW>;
+        clocks = <&infracfg_ao CLK_INFRA_CQ_DMA>;
+        clock-names = "cqdma";
+        dma-channel-mask = <63>;
+        dma-channels = <3>;
+        dma-requests = <32>;
+        #dma-cells = <1>;
+    };
+
+...
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/4] dmaengine: mediatek-cqdma: fix compatible
  2020-05-28  9:57 [PATCH v4] dmaengine: mediatek-cqdma: add dt-bindings and remove redundant queue EastL
  2020-05-28  9:57 ` [PATCH v4 1/4] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings EastL
@ 2020-05-28  9:57 ` EastL
  2020-05-28 13:39   ` Matthias Brugger
  2020-05-28  9:57 ` [PATCH v4 4/4] dmaengine: mediatek-cqdma: add dma mask for capability EastL
  2 siblings, 1 reply; 10+ messages in thread
From: EastL @ 2020-05-28  9:57 UTC (permalink / raw)
  To: Sean Wang
  Cc: vkoul, robh+dt, mark.rutland, matthias.bgg, dmaengine,
	linux-kernel, linux-arm-kernel, linux-mediatek, devicetree,
	wsd_upstream, EastL

This patch fixes mediatek-cqdma compatible to common.

Signed-off-by: EastL <EastL.Lee@mediatek.com>
---
 drivers/dma/mediatek/mtk-cqdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
index 905bbcb..bca7118 100644
--- a/drivers/dma/mediatek/mtk-cqdma.c
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -544,7 +544,7 @@ static void mtk_cqdma_hw_deinit(struct mtk_cqdma_device *cqdma)
 }
 
 static const struct of_device_id mtk_cqdma_match[] = {
-	{ .compatible = "mediatek,mt6765-cqdma" },
+	{ .compatible = "mediatek,cqdma" },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, mtk_cqdma_match);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 4/4] dmaengine: mediatek-cqdma: add dma mask for capability
  2020-05-28  9:57 [PATCH v4] dmaengine: mediatek-cqdma: add dt-bindings and remove redundant queue EastL
  2020-05-28  9:57 ` [PATCH v4 1/4] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings EastL
  2020-05-28  9:57 ` [PATCH v4 3/4] dmaengine: mediatek-cqdma: fix compatible EastL
@ 2020-05-28  9:57 ` EastL
  2020-05-28 14:10   ` Matthias Brugger
  2 siblings, 1 reply; 10+ messages in thread
From: EastL @ 2020-05-28  9:57 UTC (permalink / raw)
  To: Sean Wang
  Cc: vkoul, robh+dt, mark.rutland, matthias.bgg, dmaengine,
	linux-kernel, linux-arm-kernel, linux-mediatek, devicetree,
	wsd_upstream, EastL

This patch add dma mask for capability.

Change-Id: I31f4622f9541d769702029532e5f5f185815dda2
Signed-off-by: EastL <EastL.Lee@mediatek.com>
---
 drivers/dma/mediatek/mtk-cqdma.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
index bca7118..1805a76 100644
--- a/drivers/dma/mediatek/mtk-cqdma.c
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -117,6 +117,7 @@ struct mtk_cqdma_vchan {
  * @clk:                    The clock that device internal is using
  * @dma_requests:           The number of VCs the device supports to
  * @dma_channels:           The number of PCs the device supports to
+ * @dma_mask:               A mask for DMA capability
  * @vc:                     The pointer to all available VCs
  * @pc:                     The pointer to all the underlying PCs
  */
@@ -126,6 +127,7 @@ struct mtk_cqdma_device {
 
 	u32 dma_requests;
 	u32 dma_channels;
+	u32 dma_mask;
 	struct mtk_cqdma_vchan *vc;
 	struct mtk_cqdma_pchan **pc;
 };
@@ -549,6 +551,7 @@ static void mtk_cqdma_hw_deinit(struct mtk_cqdma_device *cqdma)
 };
 MODULE_DEVICE_TABLE(of, mtk_cqdma_match);
 
+static u64 cqdma_dmamask;
 static int mtk_cqdma_probe(struct platform_device *pdev)
 {
 	struct mtk_cqdma_device *cqdma;
@@ -607,6 +610,16 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
 		cqdma->dma_channels = MTK_CQDMA_NR_PCHANS;
 	}
 
+	if (pdev->dev.of_node && of_property_read_u32(pdev->dev.of_node,
+						      "dma-channel-mask",
+						      &cqdma->dma_mask)) {
+		dev_info(&pdev->dev,
+			 "Using 0 as missing dma-channel-mask property\n");
+	} else {
+		cqdma_dmamask = DMA_BIT_MASK(cqdma->dma_mask);
+		pdev->dev.dma_mask = &cqdma_dmamask;
+	}
+
 	cqdma->pc = devm_kcalloc(&pdev->dev, cqdma->dma_channels,
 				 sizeof(*cqdma->pc), GFP_KERNEL);
 	if (!cqdma->pc)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 3/4] dmaengine: mediatek-cqdma: fix compatible
  2020-05-28  9:57 ` [PATCH v4 3/4] dmaengine: mediatek-cqdma: fix compatible EastL
@ 2020-05-28 13:39   ` Matthias Brugger
  2020-06-12  8:12     ` EastL
  0 siblings, 1 reply; 10+ messages in thread
From: Matthias Brugger @ 2020-05-28 13:39 UTC (permalink / raw)
  To: EastL, Sean Wang
  Cc: vkoul, robh+dt, mark.rutland, dmaengine, linux-kernel,
	linux-arm-kernel, linux-mediatek, devicetree, wsd_upstream



On 28/05/2020 11:57, EastL wrote:
> This patch fixes mediatek-cqdma compatible to common.
> 
> Signed-off-by: EastL <EastL.Lee@mediatek.com>
> ---
>  drivers/dma/mediatek/mtk-cqdma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
> index 905bbcb..bca7118 100644
> --- a/drivers/dma/mediatek/mtk-cqdma.c
> +++ b/drivers/dma/mediatek/mtk-cqdma.c
> @@ -544,7 +544,7 @@ static void mtk_cqdma_hw_deinit(struct mtk_cqdma_device *cqdma)
>  }
>  
>  static const struct of_device_id mtk_cqdma_match[] = {
> -	{ .compatible = "mediatek,mt6765-cqdma" },
> +	{ .compatible = "mediatek,cqdma" },

We can't just delete and old compatible. If other cqdma IP blocks are the same
as mt6795, we should instead add entries in the binding description with
fallback compatible. For example for mt6779 the DTS would look like this:
compatible = "mediatek,mt6779-cqdma", "mediatek,mt6765-cqdma";

This way we the kernel will take care to bind the device against the driver with
mt7665-cqdma, but leaves us the posibillity to add any changes to the driver in
the future if we find some bugs/features for mt6779 that are not present in mt6765.

Regards,
Matthias

>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, mtk_cqdma_match);
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 4/4] dmaengine: mediatek-cqdma: add dma mask for capability
  2020-05-28  9:57 ` [PATCH v4 4/4] dmaengine: mediatek-cqdma: add dma mask for capability EastL
@ 2020-05-28 14:10   ` Matthias Brugger
  2020-06-12  8:13     ` EastL
  0 siblings, 1 reply; 10+ messages in thread
From: Matthias Brugger @ 2020-05-28 14:10 UTC (permalink / raw)
  To: EastL, Sean Wang
  Cc: vkoul, robh+dt, mark.rutland, dmaengine, linux-kernel,
	linux-arm-kernel, linux-mediatek, devicetree, wsd_upstream



On 28/05/2020 11:57, EastL wrote:
> This patch add dma mask for capability.
> 
> Change-Id: I31f4622f9541d769702029532e5f5f185815dda2

No Change-Id in the commit message please.

> Signed-off-by: EastL <EastL.Lee@mediatek.com>
> ---
>  drivers/dma/mediatek/mtk-cqdma.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
> index bca7118..1805a76 100644
> --- a/drivers/dma/mediatek/mtk-cqdma.c
> +++ b/drivers/dma/mediatek/mtk-cqdma.c
> @@ -117,6 +117,7 @@ struct mtk_cqdma_vchan {
>   * @clk:                    The clock that device internal is using
>   * @dma_requests:           The number of VCs the device supports to
>   * @dma_channels:           The number of PCs the device supports to
> + * @dma_mask:               A mask for DMA capability
>   * @vc:                     The pointer to all available VCs
>   * @pc:                     The pointer to all the underlying PCs
>   */
> @@ -126,6 +127,7 @@ struct mtk_cqdma_device {
>  
>  	u32 dma_requests;
>  	u32 dma_channels;
> +	u32 dma_mask;
>  	struct mtk_cqdma_vchan *vc;
>  	struct mtk_cqdma_pchan **pc;
>  };
> @@ -549,6 +551,7 @@ static void mtk_cqdma_hw_deinit(struct mtk_cqdma_device *cqdma)
>  };
>  MODULE_DEVICE_TABLE(of, mtk_cqdma_match);
>  
> +static u64 cqdma_dmamask;
>  static int mtk_cqdma_probe(struct platform_device *pdev)
>  {
>  	struct mtk_cqdma_device *cqdma;
> @@ -607,6 +610,16 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
>  		cqdma->dma_channels = MTK_CQDMA_NR_PCHANS;
>  	}
>  
> +	if (pdev->dev.of_node && of_property_read_u32(pdev->dev.of_node,
> +						      "dma-channel-mask",
> +						      &cqdma->dma_mask)) {

I'd prefer:

if (pdev->dev.of_node)
    ret = of_property_read_u32(pdev->dev.of_node,
                               "dma-channel-mask",
                               &cqdma->dma_mask))
if (ret) {
    dev_warn(&pdev->dev,
             "Using 0 as missing dma-channel-mask
              property\n");
    cqdma->dma_mask = 0;
}

> +		dev_info(&pdev->dev,
> +			 "Using 0 as missing dma-channel-mask property\n");

dev_warn should be OK.

> +	} else {
> +		cqdma_dmamask = DMA_BIT_MASK(cqdma->dma_mask);
> +		pdev->dev.dma_mask = &cqdma_dmamask;

if (dma_set_mask(&pdev->dev,
    DMA_BIT_MASK(cqdma->dma_mask)) {
         /* error out */
}

> +	}
> +
>  	cqdma->pc = devm_kcalloc(&pdev->dev, cqdma->dma_channels,
>  				 sizeof(*cqdma->pc), GFP_KERNEL);
>  	if (!cqdma->pc)
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/4] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings
  2020-05-28  9:57 ` [PATCH v4 1/4] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings EastL
@ 2020-05-29 19:24   ` Rob Herring
  2020-06-11  8:00     ` EastL
  0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2020-05-29 19:24 UTC (permalink / raw)
  To: EastL
  Cc: Sean Wang, vkoul, mark.rutland, matthias.bgg, dmaengine,
	linux-kernel, linux-arm-kernel, linux-mediatek, devicetree,
	wsd_upstream

On Thu, May 28, 2020 at 05:57:09PM +0800, EastL wrote:
> Document the devicetree bindings for MediaTek Command-Queue DMA controller
> which could be found on MT6779 SoC or other similar Mediatek SoCs.
> 
> Signed-off-by: EastL <EastL.Lee@mediatek.com>

Need a full name.

> ---
>  .../devicetree/bindings/dma/mtk-cqdma.yaml         | 100 +++++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
> 
> diff --git a/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml b/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
> new file mode 100644
> index 0000000..045aa0c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/mtk-cqdma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Command-Queue DMA controller Device Tree Binding
> +
> +maintainers:
> +  - EastL <EastL.Lee@mediatek.com>
> +
> +description:
> +  MediaTek Command-Queue DMA controller (CQDMA) on Mediatek SoC
> +  is dedicated to memory-to-memory transfer through queue based
> +  descriptor management.
> +

Need a $ref to dma-controller.yaml

> +properties:
> +  "#dma-cells":
> +    minimum: 1
> +    # Should be enough
> +    maximum: 255
> +    description:
> +      Used to provide DMA controller specific information.
> +
> +  compatible:
> +    const: mediatek,cqdma

Needs SoC specific compatible string(s).

> +
> +  reg:
> +    minItems: 1
> +    maxItems: 255

You can have 255 register regions?

You need to define what each region is if more than 1.

> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 255

255 interrupts?

> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: cqdma
> +
> +  dma-channel-mask:
> +    description:
> +      Bitmask of available DMA channels in ascending order that are
> +      not reserved by firmware and are available to the
> +      kernel. i.e. first channel corresponds to LSB.
> +      The first item in the array is for channels 0-31, the second is for
> +      channels 32-63, etc.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +    items:
> +      minItems: 1
> +      # Should be enough
> +      maxItems: 255

This already has a definition in dma-common.yaml. Don't copy-n-paste 
it. Just add any constraints you have. Like what is the max number of 
channels?

> +
> +  dma-channels:
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    description:
> +      Number of DMA channels supported by the controller.
> +
> +  dma-requests:
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    description:
> +      Number of DMA request signals supported by the controller.

Same comment on these 2.

> +
> +required:
> +  - "#dma-cells"
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - dma-channel-mask
> +  - dma-channels
> +  - dma-requests
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt6779-clk.h>
> +    cqdma: dma-controller@10212000 {
> +        compatible = "mediatek,cqdma";
> +        reg = <0 0x10212000 0 0x80>,
> +            <0 0x10212080 0 0x80>,
> +            <0 0x10212100 0 0x80>;

Examples default to 1 cell each for address and size.

> +        interrupts = <GIC_SPI 139 IRQ_TYPE_LEVEL_LOW>,
> +            <GIC_SPI 140 IRQ_TYPE_LEVEL_LOW>,
> +            <GIC_SPI 141 IRQ_TYPE_LEVEL_LOW>;
> +        clocks = <&infracfg_ao CLK_INFRA_CQ_DMA>;
> +        clock-names = "cqdma";
> +        dma-channel-mask = <63>;
> +        dma-channels = <3>;
> +        dma-requests = <32>;
> +        #dma-cells = <1>;
> +    };
> +
> +...
> -- 
> 1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/4] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings
  2020-05-29 19:24   ` Rob Herring
@ 2020-06-11  8:00     ` EastL
  0 siblings, 0 replies; 10+ messages in thread
From: EastL @ 2020-06-11  8:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sean Wang, vkoul, mark.rutland, matthias.bgg, dmaengine,
	linux-kernel, linux-arm-kernel, linux-mediatek, devicetree,
	wsd_upstream

On Fri, 2020-05-29 at 13:24 -0600, Rob Herring wrote:
> On Thu, May 28, 2020 at 05:57:09PM +0800, EastL wrote:
> > Document the devicetree bindings for MediaTek Command-Queue DMA controller
> > which could be found on MT6779 SoC or other similar Mediatek SoCs.
> > 
> > Signed-off-by: EastL <EastL.Lee@mediatek.com>
> 
> Need a full name

OK
> .
> 
> > ---
> >  .../devicetree/bindings/dma/mtk-cqdma.yaml         | 100 +++++++++++++++++++++
> >  1 file changed, 100 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml b/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
> > new file mode 100644
> > index 0000000..045aa0c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/mtk-cqdma.yaml
> > @@ -0,0 +1,100 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> Dual license new bindings:
> 
> (GPL-2.0-only OR BSD-2-Clause)

OK
> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/dma/mtk-cqdma.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek Command-Queue DMA controller Device Tree Binding
> > +
> > +maintainers:
> > +  - EastL <EastL.Lee@mediatek.com>
> > +
> > +description:
> > +  MediaTek Command-Queue DMA controller (CQDMA) on Mediatek SoC
> > +  is dedicated to memory-to-memory transfer through queue based
> > +  descriptor management.
> > +
> 
> Need a $ref to dma-controller.yaml

OK
> 
> > +properties:
> > +  "#dma-cells":
> > +    minimum: 1
> > +    # Should be enough
> > +    maximum: 255
> > +    description:
> > +      Used to provide DMA controller specific information.
> > +
> > +  compatible:
> > +    const: mediatek,cqdma
> 
> Needs SoC specific compatible string(s).
OK
> 
> > +
> > +  reg:
> > +    minItems: 1
> > +    maxItems: 255
> 
> You can have 255 register regions?
No, I'll fix maxItems to 5
> 
> You need to define what each region is if more than 1.
> 
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 255
> 
> 255 interrupts?

the same, 5 interripts.
> 
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: cqdma
> > +
> > +  dma-channel-mask:
> > +    description:
> > +      Bitmask of available DMA channels in ascending order that are
> > +      not reserved by firmware and are available to the
> > +      kernel. i.e. first channel corresponds to LSB.
> > +      The first item in the array is for channels 0-31, the second is for
> > +      channels 32-63, etc.
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    items:
> > +      minItems: 1
> > +      # Should be enough
> > +      maxItems: 255
> 
> This already has a definition in dma-common.yaml. Don't copy-n-paste 
> it. Just add any constraints you have. Like what is the max number of 
> channels?

OK, the max channel number is 5, I'll fix it on next version.
> 
> > +
> > +  dma-channels:
> > +    $ref: /schemas/types.yaml#definitions/uint32
> > +    description:
> > +      Number of DMA channels supported by the controller.
> > +
> > +  dma-requests:
> > +    $ref: /schemas/types.yaml#definitions/uint32
> > +    description:
> > +      Number of DMA request signals supported by the controller.
> 
> Same comment on these 2.

OK
> 
> > +
> > +required:
> > +  - "#dma-cells"
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - dma-channel-mask
> > +  - dma-channels
> > +  - dma-requests
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/mt6779-clk.h>
> > +    cqdma: dma-controller@10212000 {
> > +        compatible = "mediatek,cqdma";
> > +        reg = <0 0x10212000 0 0x80>,
> > +            <0 0x10212080 0 0x80>,
> > +            <0 0x10212100 0 0x80>;
> 
> Examples default to 1 cell each for address and size.
OK
> 
> > +        interrupts = <GIC_SPI 139 IRQ_TYPE_LEVEL_LOW>,
> > +            <GIC_SPI 140 IRQ_TYPE_LEVEL_LOW>,
> > +            <GIC_SPI 141 IRQ_TYPE_LEVEL_LOW>;
> > +        clocks = <&infracfg_ao CLK_INFRA_CQ_DMA>;
> > +        clock-names = "cqdma";
> > +        dma-channel-mask = <63>;
> > +        dma-channels = <3>;
> > +        dma-requests = <32>;
> > +        #dma-cells = <1>;
> > +    };
> > +
> > +...
> > -- 
> > 1.9.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 3/4] dmaengine: mediatek-cqdma: fix compatible
  2020-05-28 13:39   ` Matthias Brugger
@ 2020-06-12  8:12     ` EastL
  0 siblings, 0 replies; 10+ messages in thread
From: EastL @ 2020-06-12  8:12 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Sean Wang, vkoul, robh+dt, mark.rutland, dmaengine, linux-kernel,
	linux-arm-kernel, linux-mediatek, devicetree, wsd_upstream

On Thu, 2020-05-28 at 15:39 +0200, Matthias Brugger wrote:
> 
> On 28/05/2020 11:57, EastL wrote:
> > This patch fixes mediatek-cqdma compatible to common.
> > 
> > Signed-off-by: EastL <EastL.Lee@mediatek.com>
> > ---
> >  drivers/dma/mediatek/mtk-cqdma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
> > index 905bbcb..bca7118 100644
> > --- a/drivers/dma/mediatek/mtk-cqdma.c
> > +++ b/drivers/dma/mediatek/mtk-cqdma.c
> > @@ -544,7 +544,7 @@ static void mtk_cqdma_hw_deinit(struct mtk_cqdma_device *cqdma)
> >  }
> >  
> >  static const struct of_device_id mtk_cqdma_match[] = {
> > -	{ .compatible = "mediatek,mt6765-cqdma" },
> > +	{ .compatible = "mediatek,cqdma" },
> 
> We can't just delete and old compatible. If other cqdma IP blocks are the same
> as mt6795, we should instead add entries in the binding description with
> fallback compatible. For example for mt6779 the DTS would look like this:
> compatible = "mediatek,mt6779-cqdma", "mediatek,mt6765-cqdma";
> 
> This way we the kernel will take care to bind the device against the driver with
> mt7665-cqdma, but leaves us the posibillity to add any changes to the driver in
> the future if we find some bugs/features for mt6779 that are not present in mt6765.
> 
> Regards,
> Matthias
> 
> >  	{ /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, mtk_cqdma_match);
> > 

OK, Can I add a common compatible? Like this

static const struct of_device_id mtk_cqdma_match[] = {
{ .compatible = "mediatek,mt6765-cqdma" },
{ .compatible = "mediatek,common-cqdma" },


Regards,
EastL

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 4/4] dmaengine: mediatek-cqdma: add dma mask for capability
  2020-05-28 14:10   ` Matthias Brugger
@ 2020-06-12  8:13     ` EastL
  0 siblings, 0 replies; 10+ messages in thread
From: EastL @ 2020-06-12  8:13 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Sean Wang, vkoul, robh+dt, mark.rutland, dmaengine, linux-kernel,
	linux-arm-kernel, linux-mediatek, devicetree, wsd_upstream

On Thu, 2020-05-28 at 16:10 +0200, Matthias Brugger wrote:
> 
> On 28/05/2020 11:57, EastL wrote:
> > This patch add dma mask for capability.
> > 
> > Change-Id: I31f4622f9541d769702029532e5f5f185815dda2
> 
> No Change-Id in the commit message please.
> 
> > Signed-off-by: EastL <EastL.Lee@mediatek.com>
> > ---
> >  drivers/dma/mediatek/mtk-cqdma.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
> > index bca7118..1805a76 100644
> > --- a/drivers/dma/mediatek/mtk-cqdma.c
> > +++ b/drivers/dma/mediatek/mtk-cqdma.c
> > @@ -117,6 +117,7 @@ struct mtk_cqdma_vchan {
> >   * @clk:                    The clock that device internal is using
> >   * @dma_requests:           The number of VCs the device supports to
> >   * @dma_channels:           The number of PCs the device supports to
> > + * @dma_mask:               A mask for DMA capability
> >   * @vc:                     The pointer to all available VCs
> >   * @pc:                     The pointer to all the underlying PCs
> >   */
> > @@ -126,6 +127,7 @@ struct mtk_cqdma_device {
> >  
> >  	u32 dma_requests;
> >  	u32 dma_channels;
> > +	u32 dma_mask;
> >  	struct mtk_cqdma_vchan *vc;
> >  	struct mtk_cqdma_pchan **pc;
> >  };
> > @@ -549,6 +551,7 @@ static void mtk_cqdma_hw_deinit(struct mtk_cqdma_device *cqdma)
> >  };
> >  MODULE_DEVICE_TABLE(of, mtk_cqdma_match);
> >  
> > +static u64 cqdma_dmamask;
> >  static int mtk_cqdma_probe(struct platform_device *pdev)
> >  {
> >  	struct mtk_cqdma_device *cqdma;
> > @@ -607,6 +610,16 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
> >  		cqdma->dma_channels = MTK_CQDMA_NR_PCHANS;
> >  	}
> >  
> > +	if (pdev->dev.of_node && of_property_read_u32(pdev->dev.of_node,
> > +						      "dma-channel-mask",
> > +						      &cqdma->dma_mask)) {
> 
> I'd prefer:
> 
> if (pdev->dev.of_node)
>     ret = of_property_read_u32(pdev->dev.of_node,
>                                "dma-channel-mask",
>                                &cqdma->dma_mask))
> if (ret) {
>     dev_warn(&pdev->dev,
>              "Using 0 as missing dma-channel-mask
>               property\n");
>     cqdma->dma_mask = 0;
> }
> 
> > +		dev_info(&pdev->dev,
> > +			 "Using 0 as missing dma-channel-mask property\n");
> 
> dev_warn should be OK.
> 
> > +	} else {
> > +		cqdma_dmamask = DMA_BIT_MASK(cqdma->dma_mask);
> > +		pdev->dev.dma_mask = &cqdma_dmamask;
> 
> if (dma_set_mask(&pdev->dev,
>     DMA_BIT_MASK(cqdma->dma_mask)) {
>          /* error out */
> }
> 
OK, I'll fix it on next version.

> > +	}
> > +
> >  	cqdma->pc = devm_kcalloc(&pdev->dev, cqdma->dma_channels,
> >  				 sizeof(*cqdma->pc), GFP_KERNEL);
> >  	if (!cqdma->pc)
> > 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-06-12  8:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-28  9:57 [PATCH v4] dmaengine: mediatek-cqdma: add dt-bindings and remove redundant queue EastL
2020-05-28  9:57 ` [PATCH v4 1/4] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings EastL
2020-05-29 19:24   ` Rob Herring
2020-06-11  8:00     ` EastL
2020-05-28  9:57 ` [PATCH v4 3/4] dmaengine: mediatek-cqdma: fix compatible EastL
2020-05-28 13:39   ` Matthias Brugger
2020-06-12  8:12     ` EastL
2020-05-28  9:57 ` [PATCH v4 4/4] dmaengine: mediatek-cqdma: add dma mask for capability EastL
2020-05-28 14:10   ` Matthias Brugger
2020-06-12  8:13     ` EastL

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).