Re: [PATCH] dmaengine: sh: Add support DMA-Engine driver for DMA of

[Dan Williams <dan.j.williams@intel.com>]

On Wed, Mar 11, 2009 at 11:44 PM, Nobuhiro Iwamatsu
<iwamatsu.nobuhiro@renesas.com> wrote:
> This supports DMA-Engine driver for DMA of SuperH.
> This supported all DMA channels, and it was tested in SH7722/SH7780.
> This can not use with SH DMA API and can control this in Kconfig.
>
> Signed-off-by: Nobuhiro Iwamatsu <iwamatsu.nobuhiro@renesas.com>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Haavard Skinnemoen <hskinnemoen@atmel.com>
> Cc: Maciej Sosnowski <maciej.sosnowski@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> ---
>  arch/sh/drivers/dma/Kconfig  |   12 +-
>  arch/sh/drivers/dma/Makefile |    3 +-
>  arch/sh/include/asm/dma-sh.h |   11 +
>  drivers/dma/Kconfig          |    9 +
>  drivers/dma/Makefile         |    2 +
>  drivers/dma/shdma.c          |  743 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/dma/shdma.h          |   65 ++++
>  7 files changed, 840 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/dma/shdma.c
>  create mode 100644 drivers/dma/shdma.h

Hi,

I have not finished a full review but one problem jumps out, the use
of spin_lock_irqsave to protect against channel/descriptor
manipulations.  The highest level of protection that net_dma and
async_tx assume is spin_lock_bh.  It seems like the pieces of
sh_dmae_interrupt() that touch the descriptor can be moved to the
tasklet, then the locks can be downgraded.

Your other patch, to set the alignment in dmatest, makes me wonder if
this engine can handle unaligned accesses?  If it can not then set the
DMA_PRIVATE capability bit at device registration time to prevent
net_dma and other "public" clients from using these channels.  Public
clients assume that there are no alignment constraints.

Thanks,
Dan