RE: [PATCH 1/2] dma: Add Freescale qDMA engine driver support
Hi Vinod,
Thanks for your review, please see my comments inline.
Best Regards,
Yuan Yao
> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Monday, October 05, 2015 10:37 PM
> To: Yuan Yao-B46683
> Cc: shawn@linaro.org; dan.j.willi...@intel.com;
> dmaeng...@vger.kernel.org; linux-ker...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 1/2] dma: Add Freescale qDMA engine driver support
>
> On Fri, Sep 11, 2015 at 01:53:52PM +0800, Yuan Yao wrote:
>
> > +Examples:
> > +
> > + qdma: qdma@839 {
> > + compatible = "fsl,ls-qdma";
> > + reg = <0x0 0x838 0x0 0x2>;
> > + interrupts = ,
> > + ;
> > + interrupt-names = "qdma-tx", "qdma-err";
> > + big-endian;
> > + channels = <1>;
> > + };
>
> Binding should be a separate patch
[Yuan Yao]
Ok, Thanks.
>
> > +FREESCALE qDMA DRIVER
> > +M: Yuan Yao
> > +L: linux-arm-ker...@lists.infradead.org
>
> not dmaengine ML ?
[Yuan Yao] Ok, Thanks.
>
>
> > +config FSL_QDMA
> > + tristate "Freescale qDMA engine support"
> > + select DMA_ENGINE
> > + select DMA_VIRTUAL_CHANNELS
>
> No depends on arch, can it work on x86?
[Yuan Yao] Ok, Thanks.
>
> > +static int fsl_qdma_alloc_chan_resources(struct dma_chan *chan) {
> > + struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan);
> > +
> > + fsl_chan->desc = NULL;
> > + return 0;
> > +}
>
> why do you need this it seems to do nothing
[Yuan Yao] I will remove it.
>
> > +static struct fsl_qdma_desc *fsl_qdma_alloc_desc(struct fsl_qdma_chan
> > +*fsl_chan) {
> > + struct fsl_qdma_desc *fsl_desc;
> > +
> > + fsl_desc = kzalloc(sizeof(*fsl_desc), GFP_NOWAIT);
> > +
>
> empty line here is not required
>
> > + if (!fsl_desc)
> > + return NULL;
> > +
> > + fsl_desc->qchan = fsl_chan;
> > +
> > + return fsl_desc;
>
> why not return fsl_desc->qchan ;
>
[Yuan Yao]
I still need some data in fsl_desc. So I have to return fsl_desc here.
>
> > + dma_cap_set(DMA_PRIVATE, fsl_qdma->dma_dev.cap_mask);
> > + dma_cap_set(DMA_SLAVE, fsl_qdma->dma_dev.cap_mask);
> > + dma_cap_set(DMA_MEMCPY, fsl_qdma->dma_dev.cap_mask);
> > +
> > + fsl_qdma->dma_dev.dev = &pdev->dev;
> > + fsl_qdma->dma_dev.device_alloc_chan_resources
> > + = fsl_qdma_alloc_chan_resources;
> > + fsl_qdma->dma_dev.device_free_chan_resources
> > + = fsl_qdma_free_chan_resources;
> > + fsl_qdma->dma_dev.device_tx_status = fsl_qdma_tx_status;
> > + fsl_qdma->dma_dev.device_prep_dma_memcpy =
> fsl_qdma_prep_memcpy;
> > + fsl_qdma->dma_dev.device_issue_pending =
> fsl_qdma_issue_pending;
>
> You claim DMA_SLAVE but no prep_ for that?
>
[Yuan Yao] It's a mistake. I will remove it.\
> > +
> > +static int __init fsl_qdma_init(void) {
> > + return platform_driver_register(&fsl_qdma_driver);
> > +}
> > +subsys_initcall(fsl_qdma_init);
> why subsys_init?
>
[Yuan Yao] For a preventive, some driver base on DMA, QDMA have to initialize
earlier than them.
Even now there is no kernel driver base on QDMA, But I still think the
subsys_init is better.
> --
> ~Vinod
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] dma: Add Freescale qDMA engine driver support
On Fri, Sep 11, 2015 at 01:53:52PM +0800, Yuan Yao wrote:
> +Examples:
> +
> + qdma: qdma@839 {
> + compatible = "fsl,ls-qdma";
> + reg = <0x0 0x838 0x0 0x2>;
> + interrupts = ,
> + ;
> + interrupt-names = "qdma-tx", "qdma-err";
> + big-endian;
> + channels = <1>;
> + };
Binding should be a separate patch
> +FREESCALE qDMA DRIVER
> +M: Yuan Yao
> +L: linux-arm-ker...@lists.infradead.org
not dmaengine ML ?
> +config FSL_QDMA
> + tristate "Freescale qDMA engine support"
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
No depends on arch, can it work on x86?
> +static int fsl_qdma_alloc_chan_resources(struct dma_chan *chan)
> +{
> + struct fsl_qdma_chan *fsl_chan = to_fsl_qdma_chan(chan);
> +
> + fsl_chan->desc = NULL;
> + return 0;
> +}
why do you need this it seems to do nothing
> +static struct fsl_qdma_desc *fsl_qdma_alloc_desc(struct fsl_qdma_chan
> *fsl_chan)
> +{
> + struct fsl_qdma_desc *fsl_desc;
> +
> + fsl_desc = kzalloc(sizeof(*fsl_desc), GFP_NOWAIT);
> +
empty line here is not required
> + if (!fsl_desc)
> + return NULL;
> +
> + fsl_desc->qchan = fsl_chan;
> +
> + return fsl_desc;
why not return fsl_desc->qchan ;
> + dma_cap_set(DMA_PRIVATE, fsl_qdma->dma_dev.cap_mask);
> + dma_cap_set(DMA_SLAVE, fsl_qdma->dma_dev.cap_mask);
> + dma_cap_set(DMA_MEMCPY, fsl_qdma->dma_dev.cap_mask);
> +
> + fsl_qdma->dma_dev.dev = &pdev->dev;
> + fsl_qdma->dma_dev.device_alloc_chan_resources
> + = fsl_qdma_alloc_chan_resources;
> + fsl_qdma->dma_dev.device_free_chan_resources
> + = fsl_qdma_free_chan_resources;
> + fsl_qdma->dma_dev.device_tx_status = fsl_qdma_tx_status;
> + fsl_qdma->dma_dev.device_prep_dma_memcpy = fsl_qdma_prep_memcpy;
> + fsl_qdma->dma_dev.device_issue_pending = fsl_qdma_issue_pending;
You claim DMA_SLAVE but no prep_ for that?
> +
> +static int __init fsl_qdma_init(void)
> +{
> + return platform_driver_register(&fsl_qdma_driver);
> +}
> +subsys_initcall(fsl_qdma_init);
why subsys_init?
--
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] dma: Add Freescale qDMA engine driver support
On Fri, Sep 11, 2015 at 12:53 AM, Yuan Yao wrote:
> Add Freescale Queue Direct Memory Access(qDMA) controller support.
> This module can be found on LS-1 and LS-2 SoCs.
>
> This add the legacy mode support for qDMA.
>
> Signed-off-by: Yuan Yao
> ---
> Documentation/devicetree/bindings/dma/fsl-qdma.txt | 43 ++
> MAINTAINERS| 7 +
> drivers/dma/Kconfig| 10 +
> drivers/dma/Makefile | 1 +
> drivers/dma/fsl-qdma.c | 521
> +
> 5 files changed, 582 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/fsl-qdma.txt
> create mode 100644 drivers/dma/fsl-qdma.c
>
> diff --git a/Documentation/devicetree/bindings/dma/fsl-qdma.txt
> b/Documentation/devicetree/bindings/dma/fsl-qdma.txt
> new file mode 100644
> index 000..cdae71c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/fsl-qdma.txt
> @@ -0,0 +1,43 @@
> +* Freescale queue Direct Memory Access Controller(qDMA) Controller
> +
> + The qDMA controller transfers blocks of data between one source and one or
> more
> +destinations. The blocks of data transferred can be represented in memory as
> contiguous
> +or non-contiguous using scatter/gather table(s). Channel virtualization is
> supported
> +through enqueuing of DMA jobs to, or dequeuing DMA jobs from, different work
> +queues.
> + Legacy mode is primarily included for software requiring the earlier
> +QorIQ DMA programming model. This mode provides a simple programming
> +model not utilizing the datapath architecture. In legacy mode, DMA
> +operations are directly configured through a set of architectural
> +registers per channel.
Is this binding only covering the legacy mode? The binding should
describe the whole IP block no matter if we have driver for all the
features.
> +
> +* qDMA Controller
> +Required properties:
> +- compatible :
> + - "fsl,ls-qdma" for qDMA used similar to that on LS SoC
Compatible need to be specific like "fsl,ls1021a-qdma". See
http://www.devicetree.org/Device_Tree_Usage.
> +- reg : Specifies base physical address(s) and size of the qDMA registers.
> + The region is qDMA control register's address and size.
> +- interrupts : A list of interrupt-specifiers, one for each entry in
> + interrupt-names.
> +- interrupt-names : Should contain:
> + "qdma-tx" - the interrupt
> + "qdma-err" - the error interrupt
> +- channels : Number of channels supported by the controller
> +
> +Optional properties:
> +- big-endian: If present registers and hardware scatter/gather descriptors
> + of the qDMA are implemented in big endian mode, otherwise in little
Endian
> + mode.
> +
> +
> +Examples:
> +
> + qdma: qdma@839 {
> + compatible = "fsl,ls-qdma";
> + reg = <0x0 0x838 0x0 0x2>;
> + interrupts = ,
> + ;
> + interrupt-names = "qdma-tx", "qdma-err";
> + big-endian;
> + channels = <1>;
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5772ccf..a4d1b52 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4357,6 +4357,13 @@ L: linuxppc-...@lists.ozlabs.org
> S: Maintained
> F: drivers/dma/fsldma.*
>
> +FREESCALE qDMA DRIVER
> +M: Yuan Yao
> +L: linux-arm-ker...@lists.infradead.org
Interestingly you listed arm mailing list instead of dmaengine mailing list.
> +S: Maintained
> +F: Documentation/devicetree/bindings/dma/fsl-qdma.txt
> +F: drivers/dma/fsl-qdma.c
> +
> FREESCALE I2C CPM DRIVER
> M: Jochen Friedrich
> L: linuxppc-...@lists.ozlabs.org
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index b458475..e29e985 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -193,6 +193,16 @@ config FSL_EDMA
> multiplexing capability for DMA request sources(slot).
> This module can be found on Freescale Vybrid and LS-1 SoCs.
>
> +config FSL_QDMA
> + tristate "Freescale qDMA engine support"
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + help
> + Support the Freescale qDMA engine with command queue and legacy
> mode.
> + Channel virtualization is supported through enqueuing of DMA jobs
> to,
> + or dequeuing DMA jobs from, different work queues.
> + This module can be found on Freescale LS SoCs.
Better to spell out Layerscape
> +
> config FSL_RAID
> tristate "Freescale RAID engine Support"
> depends on FSL_SOC && !ASYNC_TX_ENABLE_CHANNEL_SWITCH
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 7711a71..8de7526 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_DW_DMAC_CORE) += dw/
> obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
> obj-$(CONFIG_FSL_DMA) += fsldma.o
> obj-$(CONFIG_FSL
[PATCH 1/2] dma: Add Freescale qDMA engine driver support
Add Freescale Queue Direct Memory Access(qDMA) controller support.
This module can be found on LS-1 and LS-2 SoCs.
This add the legacy mode support for qDMA.
Signed-off-by: Yuan Yao
---
Documentation/devicetree/bindings/dma/fsl-qdma.txt | 43 ++
MAINTAINERS| 7 +
drivers/dma/Kconfig| 10 +
drivers/dma/Makefile | 1 +
drivers/dma/fsl-qdma.c | 521 +
5 files changed, 582 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/fsl-qdma.txt
create mode 100644 drivers/dma/fsl-qdma.c
diff --git a/Documentation/devicetree/bindings/dma/fsl-qdma.txt
b/Documentation/devicetree/bindings/dma/fsl-qdma.txt
new file mode 100644
index 000..cdae71c
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/fsl-qdma.txt
@@ -0,0 +1,43 @@
+* Freescale queue Direct Memory Access Controller(qDMA) Controller
+
+ The qDMA controller transfers blocks of data between one source and one or
more
+destinations. The blocks of data transferred can be represented in memory as
contiguous
+or non-contiguous using scatter/gather table(s). Channel virtualization is
supported
+through enqueuing of DMA jobs to, or dequeuing DMA jobs from, different work
+queues.
+ Legacy mode is primarily included for software requiring the earlier
+QorIQ DMA programming model. This mode provides a simple programming
+model not utilizing the datapath architecture. In legacy mode, DMA
+operations are directly configured through a set of architectural
+registers per channel.
+
+* qDMA Controller
+Required properties:
+- compatible :
+ - "fsl,ls-qdma" for qDMA used similar to that on LS SoC
+- reg : Specifies base physical address(s) and size of the qDMA registers.
+ The region is qDMA control register's address and size.
+- interrupts : A list of interrupt-specifiers, one for each entry in
+ interrupt-names.
+- interrupt-names : Should contain:
+ "qdma-tx" - the interrupt
+ "qdma-err" - the error interrupt
+- channels : Number of channels supported by the controller
+
+Optional properties:
+- big-endian: If present registers and hardware scatter/gather descriptors
+ of the qDMA are implemented in big endian mode, otherwise in little
+ mode.
+
+
+Examples:
+
+ qdma: qdma@839 {
+ compatible = "fsl,ls-qdma";
+ reg = <0x0 0x838 0x0 0x2>;
+ interrupts = ,
+ ;
+ interrupt-names = "qdma-tx", "qdma-err";
+ big-endian;
+ channels = <1>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 5772ccf..a4d1b52 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4357,6 +4357,13 @@ L: linuxppc-...@lists.ozlabs.org
S: Maintained
F: drivers/dma/fsldma.*
+FREESCALE qDMA DRIVER
+M: Yuan Yao
+L: linux-arm-ker...@lists.infradead.org
+S: Maintained
+F: Documentation/devicetree/bindings/dma/fsl-qdma.txt
+F: drivers/dma/fsl-qdma.c
+
FREESCALE I2C CPM DRIVER
M: Jochen Friedrich
L: linuxppc-...@lists.ozlabs.org
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index b458475..e29e985 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -193,6 +193,16 @@ config FSL_EDMA
multiplexing capability for DMA request sources(slot).
This module can be found on Freescale Vybrid and LS-1 SoCs.
+config FSL_QDMA
+ tristate "Freescale qDMA engine support"
+ select DMA_ENGINE
+ select DMA_VIRTUAL_CHANNELS
+ help
+ Support the Freescale qDMA engine with command queue and legacy mode.
+ Channel virtualization is supported through enqueuing of DMA jobs to,
+ or dequeuing DMA jobs from, different work queues.
+ This module can be found on Freescale LS SoCs.
+
config FSL_RAID
tristate "Freescale RAID engine Support"
depends on FSL_SOC && !ASYNC_TX_ENABLE_CHANNEL_SWITCH
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 7711a71..8de7526 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_DW_DMAC_CORE) += dw/
obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
obj-$(CONFIG_FSL_DMA) += fsldma.o
obj-$(CONFIG_FSL_EDMA) += fsl-edma.o
+obj-$(CONFIG_FSL_QDMA) += fsl-qdma.o
obj-$(CONFIG_FSL_RAID) += fsl_raid.o
obj-$(CONFIG_HSU_DMA) += hsu/
obj-$(CONFIG_IMG_MDC_DMA) += img-mdc-dma.o
diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c
new file mode 100644
index 000..846cdba
--- /dev/null
+++ b/drivers/dma/fsl-qdma.c
@@ -0,0 +1,521 @@
+/*
+ * drivers/dma/fsl-qdma.c
+ *
+ * Copyright 2014-2015 Freescale Semiconductor, Inc.
+ *
+ * Driver for the Freescale qDMA engine with legacy mode.
+ * This module can be found on Freescale LS SoCs.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General
