* Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> [090112 18:16]: > On Mon, Jan 12, 2009 at 06:10:53PM +0200, Tony Lindgren wrote: > > * Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> [090112 18:01]: > > > On Mon, Jan 12, 2009 at 05:35:28PM +0200, Tony Lindgren wrote: > > > > Well at least you could remove some parens. > > > > How about (dma_trigger / 32) << 19 instead? > > > > > > Oh, and further to my previous reply there is also the general principle > > > of writing what you mean. So, if you mean to clear the least significant > > > 5 bits, write it as a mask with ~0x1f, not as a divide. > > > > > > And no, you don't need ~(0x1f) - the parens there are pure noise. ~0x1f > > > does just as well and isn't in any way confusing to the compiler. > > > To put it another way, parens around a single value are completely > > > meaningless. > > > > Here's this one with the extra parens removed. > > If you're concerned about useless parens, then... > > > diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c > > index 692d2b4..660a4eb 100644 > > --- a/arch/arm/plat-omap/dma.c > > +++ b/arch/arm/plat-omap/dma.c > > @@ -279,10 +279,7 @@ void omap_set_dma_transfer_params(int lch, int data_type, int elem_count, > > > > val = dma_read(CCR(lch)); > > val &= ~(3 << 19); > > - if (dma_trigger > 63) > > - val |= 1 << 20; > > - if (dma_trigger > 31) > > - val |= 1 << 19; > > + val |= ((dma_trigger & ~0x1f) << 14); > > > > val &= ~(0x1f); > > val |= (dma_trigger & 0x1f); > > should change to: > > val = dma_read(CCR(lch)); > val &= ~(3 << 19); > val |= (dma_trigger & ~0x1f) << 14; > > val &= ~0x1f; > val |= dma_trigger & 0x1f; How about this? Of course the bits should be defined for DMA_SYNCHRO_CONTROL_UPPER and DMA_SYNCHRO_CONTROL, but that may not count as a fix.. Tony
>From 80637cc6235ffcb8cd5ec4dd9a91cbd2b885d71b Mon Sep 17 00:00:00 2001 From: Anand Gadiyar <gadiyar@xxxxxx> Date: Mon, 12 Jan 2009 16:01:03 +0200 Subject: [PATCH] ARM: OMAP: Fix DMA CCR programming for request line > 63, v3 Bug in existing code causes synchro control to be set +32 if request line greater than 63 is used. Also clean up the function a bit by removing extra parens and clearing the bits at before write. Reported by Wenbiao Wang. Signed-off-by: Anand Gadiyar <gadiyar@xxxxxx> Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx> diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c index 692d2b4..e77373c 100644 --- a/arch/arm/plat-omap/dma.c +++ b/arch/arm/plat-omap/dma.c @@ -278,14 +278,11 @@ void omap_set_dma_transfer_params(int lch, int data_type, int elem_count, u32 val; val = dma_read(CCR(lch)); - val &= ~(3 << 19); - if (dma_trigger > 63) - val |= 1 << 20; - if (dma_trigger > 31) - val |= 1 << 19; - - val &= ~(0x1f); - val |= (dma_trigger & 0x1f); + + /* DMA_SYNCHRO_CONTROL_UPPER depends on the channel number */ + val &= ~((3 << 19) | 0x1f); + val |= (dma_trigger & ~0x1f) << 14; + val |= dma_trigger & 0x1f; if (sync_mode & OMAP_DMA_SYNC_FRAME) val |= 1 << 5;
