| Message ID | 20190722124457.1093886-3-arnd@arndb.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/3,v2] dmaengine: dw-edma: fix unnecessary stack usage | expand |
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>
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,
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(-)