Re: [PATCH v4 2/2] Add Intel LGM soc DMA support.

2020-07-13 Thread Andy Shevchenko
On Mon, Jul 13, 2020 at 04:41:31PM +0800, Reddy, MallikarjunaX wrote:
> On 7/9/2020 7:09 PM, Andy Shevchenko wrote:
> > On Thu, Jul 09, 2020 at 02:01:06PM +0800, Amireddy Mallikarjuna reddy wrote:
> > > Add DMA controller driver for Lightning Mountain(LGM) family of SoCs.
> > > 
> > > The main function of the DMA controller is the transfer of data from/to 
> > > any
> > > DPlus compliant peripheral to/from the memory. A memory to memory copy
> > > capability can also be configured.
> > > 
> > > This ldma driver is used for configure the device and channnels for data
> > > and control paths.
> > > +#include "../virt-dma.h"
> > I didn't find any evidence this driver utilizes virt-dma API in full.
> > For example, there is a virt_dma_desc structure and descriptor management 
> > around it.
> > Why don't you use it?
> Lgm dma soc has its own hardware descriptor.
> and each dma channel is associated with a peripheral, it is one to one
> mapping between channel and associated peripheral.

And this neither objects nor answers to my question.

Hint: above mentioned structure is an abstraction of hardware descriptors for
easier management in Linux kernel.

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v4 2/2] Add Intel LGM soc DMA support.

2020-07-13 Thread Reddy, MallikarjunaX

Thanks for the review Andy. My comments inline.

On 7/9/2020 7:09 PM, Andy Shevchenko wrote:

On Thu, Jul 09, 2020 at 02:01:06PM +0800, Amireddy Mallikarjuna reddy wrote:

Add DMA controller driver for Lightning Mountain(LGM) family of SoCs.

The main function of the DMA controller is the transfer of data from/to any
DPlus compliant peripheral to/from the memory. A memory to memory copy
capability can also be configured.

This ldma driver is used for configure the device and channnels for data
and control paths.
+#include "../virt-dma.h"

I didn't find any evidence this driver utilizes virt-dma API in full.
For example, there is a virt_dma_desc structure and descriptor management 
around it.
Why don't you use it?

Lgm dma soc has its own hardware descriptor.
and each dma channel is associated with a peripheral, it is one to one 
mapping between channel and associated peripheral.




Re: [PATCH v4 2/2] Add Intel LGM soc DMA support.

2020-07-09 Thread Andy Shevchenko
On Thu, Jul 09, 2020 at 02:01:06PM +0800, Amireddy Mallikarjuna reddy wrote:
> Add DMA controller driver for Lightning Mountain(LGM) family of SoCs.
> 
> The main function of the DMA controller is the transfer of data from/to any
> DPlus compliant peripheral to/from the memory. A memory to memory copy
> capability can also be configured.
> 
> This ldma driver is used for configure the device and channnels for data
> and control paths.

> +#include "../virt-dma.h"

I didn't find any evidence this driver utilizes virt-dma API in full.
For example, there is a virt_dma_desc structure and descriptor management 
around it.
Why don't you use it?

-- 
With Best Regards,
Andy Shevchenko




[PATCH v4 2/2] Add Intel LGM soc DMA support.

2020-07-09 Thread Amireddy Mallikarjuna reddy
Add DMA controller driver for Lightning Mountain(LGM) family of SoCs.

The main function of the DMA controller is the transfer of data from/to any
DPlus compliant peripheral to/from the memory. A memory to memory copy
capability can also be configured.

This ldma driver is used for configure the device and channnels for data
and control paths.

Signed-off-by: Amireddy Mallikarjuna reddy 
---
v1:
- Initial version.

v2:
- Fix device tree bot issues, correspondign driver changes done.
- Fix kerntel test robot warnings.
  
  >> drivers/dma/lgm/lgm-dma.c:729:5: warning: no previous prototype for 
function 'intel_dma_chan_desc_cfg' [-Wmissing-prototypes]
  int intel_dma_chan_desc_cfg(struct dma_chan *chan, dma_addr_t desc_base,
  ^
  drivers/dma/lgm/lgm-dma.c:729:1: note: declare 'static' if the function is 
not intended to be used outside of this translation unit
  int intel_dma_chan_desc_cfg(struct dma_chan *chan, dma_addr_t desc_base,
  ^
  static
  1 warning generated.

  vim +/intel_dma_chan_desc_cfg +729 drivers/dma/lgm/lgm-dma.c

728
  > 729 int intel_dma_chan_desc_cfg(struct dma_chan *chan, dma_addr_t desc_base,
730 int desc_num)
731 {
732 return ldma_chan_desc_cfg(to_ldma_chan(chan), desc_base, 
desc_num);
733 }
734 EXPORT_SYMBOL_GPL(intel_dma_chan_desc_cfg);
735

   Reported-by: kernel test robot 
   ---

v3:
- Fix smatch warning.
  
  smatch warnings:
  drivers/dma/lgm/lgm-dma.c:1306 ldma_cfg_init() error: uninitialized symbol 
'ret'.

  Reported-by: kernel test robot 
  Reported-by: Dan Carpenter 
  

v4:
- Address Thomas Langer comments in dtbinding and corresponding driver side 
changes.
---
 drivers/dma/Kconfig |2 +
 drivers/dma/Makefile|1 +
 drivers/dma/lgm/Kconfig |9 +
 drivers/dma/lgm/Makefile|2 +
 drivers/dma/lgm/lgm-dma.c   | 1941 +++
 include/linux/dma/lgm_dma.h |   27 +
 6 files changed, 1982 insertions(+)
 create mode 100644 drivers/dma/lgm/Kconfig
 create mode 100644 drivers/dma/lgm/Makefile
 create mode 100644 drivers/dma/lgm/lgm-dma.c
 create mode 100644 include/linux/dma/lgm_dma.h

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index de41d7928bff..caeaf12fd524 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -737,6 +737,8 @@ source "drivers/dma/ti/Kconfig"
 
 source "drivers/dma/fsl-dpaa2-qdma/Kconfig"
 
+source "drivers/dma/lgm/Kconfig"
+
 # clients
 comment "DMA Clients"
depends on DMA_ENGINE
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index e60f81331d4c..0b899b076f4e 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_XGENE_DMA) += xgene-dma.o
 obj-$(CONFIG_ZX_DMA) += zx_dma.o
 obj-$(CONFIG_ST_FDMA) += st_fdma.o
 obj-$(CONFIG_FSL_DPAA2_QDMA) += fsl-dpaa2-qdma/
+obj-$(CONFIG_INTEL_LDMA) += lgm/
 
 obj-y += mediatek/
 obj-y += qcom/
diff --git a/drivers/dma/lgm/Kconfig b/drivers/dma/lgm/Kconfig
new file mode 100644
index ..bdb5b0d91afb
--- /dev/null
+++ b/drivers/dma/lgm/Kconfig
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config INTEL_LDMA
+   bool "Lightning Mountain centralized low speed DMA and high speed DMA 
controllers"
+   select DMA_ENGINE
+   select DMA_VIRTUAL_CHANNELS
+   help
+ Enable support for intel Lightning Mountain SOC DMA controllers.
+ These controllers provide DMA capabilities for a variety of on-chip
+ devices such as SSC, HSNAND and GSWIP.
diff --git a/drivers/dma/lgm/Makefile b/drivers/dma/lgm/Makefile
new file mode 100644
index ..f318a8eff464
--- /dev/null
+++ b/drivers/dma/lgm/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_INTEL_LDMA)   += lgm-dma.o
diff --git a/drivers/dma/lgm/lgm-dma.c b/drivers/dma/lgm/lgm-dma.c
new file mode 100644
index ..91c0a28fe8fb
--- /dev/null
+++ b/drivers/dma/lgm/lgm-dma.c
@@ -0,0 +1,1941 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Lightning Mountain centralized low speed and high speed DMA controller 
driver
+ *
+ * Copyright (c) 2016 ~ 2020 Intel Corporation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "../dmaengine.h"
+#include "../virt-dma.h"
+
+#define DRIVER_NAME"lgm-ldma"
+
+#define DMA_ID 0x0008
+#define DMA_ID_REV GENMASK(7, 0)
+#define DMA_ID_PNR GENMASK(19, 16)
+#define DMA_ID_CHNRGENMASK(26, 20)
+#define DMA_ID_DW_128B BIT(27)
+#define DMA_ID_AW_36B  BIT(28)
+#define