On Fri, Apr 11, 2014 at 10:42 AM, Dan Williams [off-list ref] wrote:

A few more comments:

On Fri, Apr 11, 2014 at 12:41 AM,  [off-list ref] wrote:

quoted

From: Xuelin Shi <redacted>

The RaidEngine is a new FSL hardware used for Raid5/6 acceration.

This patch enables the RaidEngine functionality and provides
hardware offloading capability for memcpy, xor and pq computation.
It works with async_tx.

Signed-off-by: Harninder Rai <redacted>
Signed-off-by: Naveen Burmi <redacted>
Signed-off-by: Xuelin Shi <redacted>
---
 changes for v3:
  - fix memory allocation flag GFP_xxx usage.
  - add re_jr_issue_pending call in cleanup.
  - remove unnecessary dma_run_dependencies(...).
  - use dma_cookie_complete(...) instead of direct updating cookie.

 drivers/dma/Kconfig    |  11 +
 drivers/dma/Makefile   |   1 +
 drivers/dma/fsl_raid.c | 878 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/dma/fsl_raid.h | 308 +++++++++++++++++
 4 files changed, 1198 insertions(+)
 create mode 100644 drivers/dma/fsl_raid.c
 create mode 100644 drivers/dma/fsl_raid.h

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 605b016..829f41c 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig

@@ -100,6 +100,17 @@ config FSL_DMA
          EloPlus is on mpc85xx and mpc86xx and Pxxx parts, and the Elo3 is on
          some Txxx and Bxxx parts.

+config FSL_RAID
+        tristate "Freescale RAID engine Support"
+        depends on FSL_SOC && !FSL_DMA

This won't work in the case where FSL_DMA=m.

Instead, I think you want to use "depends on
!ASYNC_TX_ENABLE_CHANNEL_SWITCH" which is the actual dependency.

quoted

+        select DMA_ENGINE
+        select DMA_ENGINE_RAID
+        ---help---
+          Enable support for Freescale RAID Engine. RAID Engine is
+          available on some QorIQ SoCs (like P5020). It has
+          the capability to offload memcpy, xor and pq computation
+         for raid5/6.
+

[..]

quoted

diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
new file mode 100644
index 0000000..4b389b1
--- /dev/null
+++ b/drivers/dma/fsl_raid.c

@@ -0,0 +1,878 @@

[..]

quoted

+void fill_cfd_frame(struct cmpnd_frame *cf, u8 index,
+               size_t length, dma_addr_t addr, bool final)
+{
+       u32 efrl = length & CF_LENGTH_MASK;
+       efrl |= final << CF_FINAL_SHIFT;
+       cf[index].efrl32 = efrl;
+       cf[index].addr_high = (addr >> 32) & HWDESC_ADDR_HIGH_MASK;

Use "upper_32_bits()" here otherwise:

  drivers/dma/fsl_raid.c: In function 'fill_cfd_frame':
  drivers/dma/fsl_raid.c:258:2: warning: right shift count >= width of type

quoted

+       cf[index].addr_low = (u32)addr;

Use lower_32_bits() here to be symmetrical.

quoted

+}
+
+static struct fsl_re_dma_async_tx_desc *re_jr_init_desc(struct re_jr *jr,
+       struct fsl_re_dma_async_tx_desc *desc, void *cf, dma_addr_t paddr)
+{
+       desc->jr = jr;
+       desc->async_tx.tx_submit = re_jr_tx_submit;
+       dma_async_tx_descriptor_init(&desc->async_tx, &jr->chan);
+       INIT_LIST_HEAD(&desc->node);
+
+       desc->hwdesc.fmt32 = FRAME_FORMAT << HWDESC_FMT_SHIFT;
+       desc->hwdesc.lbea32 = (paddr >> 32) & HWDESC_ADDR_HIGH_MASK;

Same comment/warning as above...

quoted

+       desc->hwdesc.addr_low = (u32)paddr;

ditto.

Oh, and be mindful of checkpatch output:

ERROR: trailing whitespace
#35: FILE: drivers/dma/Kconfig:106:
+        select DMA_ENGINE $

ERROR: trailing whitespace
#42: FILE: drivers/dma/Kconfig:113:
+ $

total: 2 errors, 0 warnings, 1207 lines checked