[3/3,v2] dmaengine: dw-edma: fix endianess confusion
diff mbox series

Message ID 20190722124457.1093886-3-arnd@arndb.de
State In Next
Commit 6f4722b1d1ebf274deb0459a36fad57a7d7fab31
Headers show
Series
  • [1/3,v2] dmaengine: dw-edma: fix unnecessary stack usage
Related show

Commit Message

Arnd Bergmann July 22, 2019, 12:44 p.m. UTC
When building with 'make C=1', sparse reports an endianess bug:

drivers/dma/dw-edma/dw-edma-v0-debugfs.c:60:30: warning: cast removes address space of expression
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24: warning: incorrect type in argument 1 (different address spaces)
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    expected void const volatile [noderef] <asn:2>*addr
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    got void *[assigned] ptr
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24: warning: incorrect type in argument 1 (different address spaces)
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    expected void const volatile [noderef] <asn:2>*addr
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    got void *[assigned] ptr
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24: warning: incorrect type in argument 1 (different address spaces)
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    expected void const volatile [noderef] <asn:2>*addr
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    got void *[assigned] ptr

The current code is clearly wrong, as it passes an endian-swapped word
into a register function where it gets swapped again. Just pass the variables
directly into lower_32_bits()/upper_32_bits().

Fixes: 7e4b8a4fbe2c ("dmaengine: Add Synopsys eDMA IP version 0 support")
Link: https://lore.kernel.org/lkml/20190617131820.2470686-1-arnd@arndb.de/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: remove unneeded local variables
---
 drivers/dma/dw-edma/dw-edma-v0-core.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

Comments

Gustavo Pimentel July 22, 2019, 1:34 p.m. UTC | #1
On Mon, Jul 22, 2019 at 13:44:45, Arnd Bergmann <arnd@arndb.de> wrote:

> When building with 'make C=1', sparse reports an endianess bug:
> 
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:60:30: warning: cast removes address space of expression
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24: warning: incorrect type in argument 1 (different address spaces)
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    expected void const volatile [noderef] <asn:2>*addr
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    got void *[assigned] ptr
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24: warning: incorrect type in argument 1 (different address spaces)
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    expected void const volatile [noderef] <asn:2>*addr
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    got void *[assigned] ptr
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24: warning: incorrect type in argument 1 (different address spaces)
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    expected void const volatile [noderef] <asn:2>*addr
> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:86:24:    got void *[assigned] ptr
> 
> The current code is clearly wrong, as it passes an endian-swapped word
> into a register function where it gets swapped again. Just pass the variables
> directly into lower_32_bits()/upper_32_bits().
> 
> Fixes: 7e4b8a4fbe2c ("dmaengine: Add Synopsys eDMA IP version 0 support")
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_lkml_20190617131820.2470686-2D1-2Darnd-40arndb.de_&d=DwIDAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=Tzn8ciklqeA_SXj1k0nN9QiPfGqx8kgsSRaM6-IkOwk&s=nOFhosE33DqpmKyI-X7EI576wW7M2Voile7t7KQpJEQ&e= 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: remove unneeded local variables
> ---
>  drivers/dma/dw-edma/dw-edma-v0-core.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 97e3fd41c8a8..692de47b1670 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -195,7 +195,6 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
>  	struct dw_edma_v0_lli __iomem *lli;
>  	struct dw_edma_v0_llp __iomem *llp;
>  	u32 control = 0, i = 0;
> -	u64 sar, dar, addr;
>  	int j;
>  
>  	lli = chunk->ll_region.vaddr;
> @@ -214,13 +213,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
>  		/* Transfer size */
>  		SET_LL(&lli[i].transfer_size, child->sz);
>  		/* SAR - low, high */
> -		sar = cpu_to_le64(child->sar);
> -		SET_LL(&lli[i].sar_low, lower_32_bits(sar));
> -		SET_LL(&lli[i].sar_high, upper_32_bits(sar));
> +		SET_LL(&lli[i].sar_low, lower_32_bits(child->sar));
> +		SET_LL(&lli[i].sar_high, upper_32_bits(child->sar));
>  		/* DAR - low, high */
> -		dar = cpu_to_le64(child->dar);
> -		SET_LL(&lli[i].dar_low, lower_32_bits(dar));
> -		SET_LL(&lli[i].dar_high, upper_32_bits(dar));
> +		SET_LL(&lli[i].dar_low, lower_32_bits(child->dar));
> +		SET_LL(&lli[i].dar_high, upper_32_bits(child->dar));
>  		i++;
>  	}
>  
> @@ -232,9 +229,8 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
>  	/* Channel control */
>  	SET_LL(&llp->control, control);
>  	/* Linked list  - low, high */
> -	addr = cpu_to_le64(chunk->ll_region.paddr);
> -	SET_LL(&llp->llp_low, lower_32_bits(addr));
> -	SET_LL(&llp->llp_high, upper_32_bits(addr));
> +	SET_LL(&llp->llp_low, lower_32_bits(chunk->ll_region.paddr));
> +	SET_LL(&llp->llp_high, upper_32_bits(chunk->ll_region.paddr));
>  }
>  
>  void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> @@ -242,7 +238,6 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  	struct dw_edma_chan *chan = chunk->chan;
>  	struct dw_edma *dw = chan->chip->dw;
>  	u32 tmp;
> -	u64 llp;
>  
>  	dw_edma_v0_core_write_chunk(chunk);
>  
> @@ -262,9 +257,10 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  		SET_CH(dw, chan->dir, chan->id, ch_control1,
>  		       (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
>  		/* Linked list - low, high */
> -		llp = cpu_to_le64(chunk->ll_region.paddr);
> -		SET_CH(dw, chan->dir, chan->id, llp_low, lower_32_bits(llp));
> -		SET_CH(dw, chan->dir, chan->id, llp_high, upper_32_bits(llp));
> +		SET_CH(dw, chan->dir, chan->id, llp_low,
> +		       lower_32_bits(chunk->ll_region.paddr));
> +		SET_CH(dw, chan->dir, chan->id, llp_high,
> +		       upper_32_bits(chunk->ll_region.paddr));
>  	}
>  	/* Doorbell */
>  	SET_RW(dw, chan->dir, doorbell,
> -- 
> 2.20.0


Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>

Patch
diff mbox series

diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 97e3fd41c8a8..692de47b1670 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -195,7 +195,6 @@  static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
 	struct dw_edma_v0_lli __iomem *lli;
 	struct dw_edma_v0_llp __iomem *llp;
 	u32 control = 0, i = 0;
-	u64 sar, dar, addr;
 	int j;
 
 	lli = chunk->ll_region.vaddr;
@@ -214,13 +213,11 @@  static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
 		/* Transfer size */
 		SET_LL(&lli[i].transfer_size, child->sz);
 		/* SAR - low, high */
-		sar = cpu_to_le64(child->sar);
-		SET_LL(&lli[i].sar_low, lower_32_bits(sar));
-		SET_LL(&lli[i].sar_high, upper_32_bits(sar));
+		SET_LL(&lli[i].sar_low, lower_32_bits(child->sar));
+		SET_LL(&lli[i].sar_high, upper_32_bits(child->sar));
 		/* DAR - low, high */
-		dar = cpu_to_le64(child->dar);
-		SET_LL(&lli[i].dar_low, lower_32_bits(dar));
-		SET_LL(&lli[i].dar_high, upper_32_bits(dar));
+		SET_LL(&lli[i].dar_low, lower_32_bits(child->dar));
+		SET_LL(&lli[i].dar_high, upper_32_bits(child->dar));
 		i++;
 	}
 
@@ -232,9 +229,8 @@  static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
 	/* Channel control */
 	SET_LL(&llp->control, control);
 	/* Linked list  - low, high */
-	addr = cpu_to_le64(chunk->ll_region.paddr);
-	SET_LL(&llp->llp_low, lower_32_bits(addr));
-	SET_LL(&llp->llp_high, upper_32_bits(addr));
+	SET_LL(&llp->llp_low, lower_32_bits(chunk->ll_region.paddr));
+	SET_LL(&llp->llp_high, upper_32_bits(chunk->ll_region.paddr));
 }
 
 void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
@@ -242,7 +238,6 @@  void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 	struct dw_edma_chan *chan = chunk->chan;
 	struct dw_edma *dw = chan->chip->dw;
 	u32 tmp;
-	u64 llp;
 
 	dw_edma_v0_core_write_chunk(chunk);
 
@@ -262,9 +257,10 @@  void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 		SET_CH(dw, chan->dir, chan->id, ch_control1,
 		       (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
 		/* Linked list - low, high */
-		llp = cpu_to_le64(chunk->ll_region.paddr);
-		SET_CH(dw, chan->dir, chan->id, llp_low, lower_32_bits(llp));
-		SET_CH(dw, chan->dir, chan->id, llp_high, upper_32_bits(llp));
+		SET_CH(dw, chan->dir, chan->id, llp_low,
+		       lower_32_bits(chunk->ll_region.paddr));
+		SET_CH(dw, chan->dir, chan->id, llp_high,
+		       upper_32_bits(chunk->ll_region.paddr));
 	}
 	/* Doorbell */
 	SET_RW(dw, chan->dir, doorbell,