RE: [PATCH v3] dmaengine: driver support for FSL RaidEngine device.

[Xuelin Shi <xuelin.shi@xxxxxxxxxxxxx>]

Hi Dan,

fsl dma device and fsl raid device are two differenct devices that both provide async_memcpy
capability, so I use !FSL_DMA to disable the fsl dma device. 

That's to say, either select fsldma device, either fsl raid device.

Thanks,
Xuelin Shi

-----Original Message-----
From: dan.j.williams@xxxxxxxxx [mailto:dan.j.williams@xxxxxxxxx] On Behalf Of Dan Williams
Sent: 2014年4月12日 1:43
To: Shi Xuelin-B29237
Cc: Koul, Vinod; andriy.shevchenko@xxxxxxxxx; dmaengine@xxxxxxxxxxxxxxx; linuxppc-dev; Rai Harninder-B01044; Burmi Naveen-B16502
Subject: Re: [PATCH v3] dmaengine: driver support for FSL RaidEngine device.

A few more comments:

On Fri, Apr 11, 2014 at 12:41 AM,  <xuelin.shi@xxxxxxxxxxxxx> wrote:
> From: Xuelin Shi <xuelin.shi@xxxxxxxxxxxxx>
>
> 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 <harninder.rai@xxxxxxxxxxxxx>
> Signed-off-by: Naveen Burmi <naveenburmi@xxxxxxxxxxxxx>
> Signed-off-by: Xuelin Shi <xuelin.shi@xxxxxxxxxxxxx>
> ---
>  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.

> +        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.
> +
[..]
> 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 @@
[..]
> +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

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

Use lower_32_bits() here to be symmetrical.

> +}
> +
> +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...

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

ditto.


?韬{.n?????%??檩??w?{.n???Ё?撖)?骅w*jg????????G??⒏⒎?:+v????????????＂??????