[PATCH] avr32-linux: at32ap700x: set DMA slave properties for MCI dw_dmac

[PATCH] avr32-linux: at32ap700x: set DMA slave properties for MCI dw_dmac

[Hein Tibosch]

@Andrew: could you please push the patch below? Hans-Christian
doesn't have a GIT tree on kernel.org

Thanks, Hein

The MCI makes use of the dw_dmac driver when DMA is being used.
Due to recent changes the driver was broken because:
- the SMS field in the CTLL register received the wrong value 0
- a patch in dw_dmac allowed for 64-bit transfers on the
memory side, giving an illegal value of 3 in the SRC/DST_TR_WIDTH
register.
This patch sets the SMS to 1 and limits the maximum transfer
width to 2 (32 bits)

Note: this can only be applied after my patch:
[PATCH 2/2] dw_dmac: max_mem_width limits value for
SRC/DST_TR_WID register

Signed-off-by: Hein Tibosch <hein_tibosch@yahoo.es>
---
 arch/avr32/mach-at32ap/at32ap700x.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
 diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
index 0445c4f..e7202af 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -1355,6 +1355,10 @@ at32_add_device_mci(unsigned int id, struct mci_platform_data *data)
 				| DWC_CFGH_DST_PER(1));
 	slave->sdata.cfg_lo &= ~(DWC_CFGL_HS_DST_POL
 				| DWC_CFGL_HS_SRC_POL);
+	/* Give CTLL SMS value 1 */
+	slave->sdata.src_master = 1;
+	/* SRC/DST_TR_WIDTH register only accepts 0,1,2 */
+	slave->sdata.max_mem_width = 2;
 
 	data->dma_slave = slave;
 
-- 
1.7.8.0

Re: [PATCH] avr32-linux: at32ap700x: set DMA slave properties for MCI dw_dmac

[Hans-Christian Egtvedt]

Around Mon 27 Aug 2012 04:56:55 +0800 or thereabout, Hein Tibosch wrote:

PS: I prefer avr32: prefix in the Subject for AVR32 kernel stuff, we already
know this is for the Linux kernel ;)

> @Andrew: could you please push the patch below? Hans-Christian
> doesn't have a GIT tree on kernel.org
> 
> Thanks, Hein
> 
> The MCI makes use of the dw_dmac driver when DMA is being used.
> Due to recent changes the driver was broken because:
> - the SMS field in the CTLL register received the wrong value 0
> - a patch in dw_dmac allowed for 64-bit transfers on the
> memory side, giving an illegal value of 3 in the SRC/DST_TR_WIDTH
> register.
> This patch sets the SMS to 1 and limits the maximum transfer
> width to 2 (32 bits)
> 
> Note: this can only be applied after my patch:
> [PATCH 2/2] dw_dmac: max_mem_width limits value for
> SRC/DST_TR_WID register

Thanks for going through this and fixing the MCI/DMA-combo on AVR32.

> Signed-off-by: Hein Tibosch <hein_tibosch@yahoo.es>
> ---
>  arch/avr32/mach-at32ap/at32ap700x.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>  diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
> index 0445c4f..e7202af 100644
> --- a/arch/avr32/mach-at32ap/at32ap700x.c
> +++ b/arch/avr32/mach-at32ap/at32ap700x.c
> @@ -1355,6 +1355,10 @@ at32_add_device_mci(unsigned int id, struct mci_platform_data *data)
>  				| DWC_CFGH_DST_PER(1));
>  	slave->sdata.cfg_lo &= ~(DWC_CFGL_HS_DST_POL
>  				| DWC_CFGL_HS_SRC_POL);
> +	/* Give CTLL SMS value 1 */

This comment gives me no information, I don't know what CTLL and SMS are, and
what does the magic value 1 do? Either skip the comment, or give a short
block here describing why src_master must be one, and max_mem_width must be 2.

> +	slave->sdata.src_master = 1;
> +	/* SRC/DST_TR_WIDTH register only accepts 0,1,2 */
> +	slave->sdata.max_mem_width = 2;

Something like:

/*
 * Setup DMA controller to let source be master, and transfer width to
 * 32-bit.
 */
slave->sdata.src_master = 1;
slave->sdata.max_mem_width = 2;

>  
>  	data->dma_slave = slave;
>  
-- 
mvh
Hans-Christian Egtvedt

Re: [PATCH] avr32-linux: at32ap700x: set DMA slave properties for MCI dw_dmac

[Andrew Morton]

On Mon, 27 Aug 2012 04:56:55 +0800
Hein Tibosch <hein_tibosch@yahoo.es> wrote:

> The MCI makes use of the dw_dmac driver when DMA is being used.
> Due to recent changes the driver was broken because:
> - the SMS field in the CTLL register received the wrong value 0
> - a patch in dw_dmac allowed for 64-bit transfers on the
> memory side, giving an illegal value of 3 in the SRC/DST_TR_WIDTH
> register.

Please try to avoid vague descriptions such as "recent changes" and "a
patch".  We like to see specifics: commit IDs and titles.

This way we can make more informed decisions about patch scheduling. 
We can ensure that the authors of the earlier commits get to review
these changes.  Maintainers of other trees can determine whether or not
their tree needs the particular fix.