* [PATCH 5/7] [media] mipi-csis: make sparse happy [not found] <cover.1443737682.git.mchehab@osg.samsung.com> @ 2015-10-01 22:17 ` Mauro Carvalho Chehab 2015-10-02 22:25 ` Arnd Bergmann 2015-10-01 22:17 ` [PATCH 6/7] [media] c8sectpfe: fix namespace on memcpy/memset Mauro Carvalho Chehab 1 sibling, 1 reply; 7+ messages in thread From: Mauro Carvalho Chehab @ 2015-10-01 22:17 UTC (permalink / raw) To: linux-arm-kernel Fix the namespace issue that causes this warning: drivers/media/platform/exynos4-is/mipi-csis.c:709:17: warning: incorrect type in argument 2 (different address spaces) drivers/media/platform/exynos4-is/mipi-csis.c:709:17: expected void const *<noident> drivers/media/platform/exynos4-is/mipi-csis.c:709:17: got void [noderef] <asn:2>* Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c index d74e1bec3d86..4b85105dc159 100644 --- a/drivers/media/platform/exynos4-is/mipi-csis.c +++ b/drivers/media/platform/exynos4-is/mipi-csis.c @@ -706,7 +706,8 @@ static irqreturn_t s5pcsis_irq_handler(int irq, void *dev_id) else offset = S5PCSIS_PKTDATA_ODD; - memcpy(pktbuf->data, state->regs + offset, pktbuf->len); + memcpy(pktbuf->data, (u8 __force *)state->regs + offset, + pktbuf->len); pktbuf->data = NULL; rmb(); } -- 2.4.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/7] [media] mipi-csis: make sparse happy 2015-10-01 22:17 ` [PATCH 5/7] [media] mipi-csis: make sparse happy Mauro Carvalho Chehab @ 2015-10-02 22:25 ` Arnd Bergmann 2015-10-05 10:24 ` Sylwester Nawrocki 0 siblings, 1 reply; 7+ messages in thread From: Arnd Bergmann @ 2015-10-02 22:25 UTC (permalink / raw) To: linux-arm-kernel On Thursday 01 October 2015 19:17:27 Mauro Carvalho Chehab wrote: > diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c > index d74e1bec3d86..4b85105dc159 100644 > --- a/drivers/media/platform/exynos4-is/mipi-csis.c > +++ b/drivers/media/platform/exynos4-is/mipi-csis.c > @@ -706,7 +706,8 @@ static irqreturn_t s5pcsis_irq_handler(int irq, void *dev_id) > else > offset = S5PCSIS_PKTDATA_ODD; > > - memcpy(pktbuf->data, state->regs + offset, pktbuf->len); > + memcpy(pktbuf->data, (u8 __force *)state->regs + offset, > + pktbuf->len); > pktbuf->data = NULL; > I think this is what memcpy_toio() is meant for. Arnd ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 5/7] [media] mipi-csis: make sparse happy 2015-10-02 22:25 ` Arnd Bergmann @ 2015-10-05 10:24 ` Sylwester Nawrocki 2015-10-05 11:07 ` Arnd Bergmann 0 siblings, 1 reply; 7+ messages in thread From: Sylwester Nawrocki @ 2015-10-05 10:24 UTC (permalink / raw) To: linux-arm-kernel On 03/10/15 00:25, Arnd Bergmann wrote: > On Thursday 01 October 2015 19:17:27 Mauro Carvalho Chehab wrote: >> > diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c >> > index d74e1bec3d86..4b85105dc159 100644 >> > --- a/drivers/media/platform/exynos4-is/mipi-csis.c >> > +++ b/drivers/media/platform/exynos4-is/mipi-csis.c >> > @@ -706,7 +706,8 @@ static irqreturn_t s5pcsis_irq_handler(int irq, void *dev_id) >> > else >> > offset = S5PCSIS_PKTDATA_ODD; >> > >> > - memcpy(pktbuf->data, state->regs + offset, pktbuf->len); >> > + memcpy(pktbuf->data, (u8 __force *)state->regs + offset, >> > + pktbuf->len); >> > pktbuf->data = NULL; >> > > > I think this is what memcpy_toio() is meant for. Exactly memcpy_fromio(). But it's implementation is inefficient on ARCH=arm, memcpy_fromio() will be translated to a loop of readb(), only if an arm sub-architecture provides a processor instruction to access memory by byte. Each readb() also involves a memory barrier. That's all what we wanted to avoid. AFAIR using memcpy_fromio() was causing increase of the copy operation several times comparing to memcpy(). On arm64 it looks better, but this driver is currently used only on arm32. I would prefer to add (void __force *) instead: memcpy(pktbuf->data, (void __force *)state->regs + offset, pktbuf->len); Alternatively, the memset could just be replaced by a loop of u32 reads - __raw_readl(); -- Thanks, Sylwester ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 5/7] [media] mipi-csis: make sparse happy 2015-10-05 10:24 ` Sylwester Nawrocki @ 2015-10-05 11:07 ` Arnd Bergmann 2015-10-05 12:23 ` Sylwester Nawrocki 0 siblings, 1 reply; 7+ messages in thread From: Arnd Bergmann @ 2015-10-05 11:07 UTC (permalink / raw) To: linux-arm-kernel On Monday 05 October 2015 12:24:40 Sylwester Nawrocki wrote: > On 03/10/15 00:25, Arnd Bergmann wrote: > > On Thursday 01 October 2015 19:17:27 Mauro Carvalho Chehab wrote: > >> > diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c > >> > index d74e1bec3d86..4b85105dc159 100644 > >> > --- a/drivers/media/platform/exynos4-is/mipi-csis.c > >> > +++ b/drivers/media/platform/exynos4-is/mipi-csis.c > >> > @@ -706,7 +706,8 @@ static irqreturn_t s5pcsis_irq_handler(int irq, void *dev_id) > >> > else > >> > offset = S5PCSIS_PKTDATA_ODD; > >> > > >> > - memcpy(pktbuf->data, state->regs + offset, pktbuf->len); > >> > + memcpy(pktbuf->data, (u8 __force *)state->regs + offset, > >> > + pktbuf->len); > >> > pktbuf->data = NULL; > >> > > > > > I think this is what memcpy_toio() is meant for. > > Exactly memcpy_fromio(). But it's implementation is inefficient on > ARCH=arm, memcpy_fromio() will be translated to a loop of readb(), > only if an arm sub-architecture provides a processor instruction > to access memory by byte. Each readb() also involves a memory barrier. > That's all what we wanted to avoid. AFAIR using memcpy_fromio() was > causing increase of the copy operation several times comparing to > memcpy(). On arm64 it looks better, but this driver is currently > used only on arm32. > > I would prefer to add (void __force *) instead: > > memcpy(pktbuf->data, (void __force *)state->regs + offset, pktbuf->len); > > Alternatively, the memset could just be replaced by a loop of > u32 reads - __raw_readl(); You are right for old kernels, but this was fixed in 7ddfe625cb ("ARM: optimize memset_io()/memcpy_fromio()/memcpy_toio()") at least for little-endian kernels and should be fine now on ARM just like everywhere else. Arnd ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 5/7] [media] mipi-csis: make sparse happy 2015-10-05 11:07 ` Arnd Bergmann @ 2015-10-05 12:23 ` Sylwester Nawrocki 0 siblings, 0 replies; 7+ messages in thread From: Sylwester Nawrocki @ 2015-10-05 12:23 UTC (permalink / raw) To: linux-arm-kernel On 05/10/15 13:07, Arnd Bergmann wrote: > On Monday 05 October 2015 12:24:40 Sylwester Nawrocki wrote: >> > On 03/10/15 00:25, Arnd Bergmann wrote: >>> > > On Thursday 01 October 2015 19:17:27 Mauro Carvalho Chehab wrote: >>>>> > >> > diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c >>>>> > >> > index d74e1bec3d86..4b85105dc159 100644 >>>>> > >> > --- a/drivers/media/platform/exynos4-is/mipi-csis.c >>>>> > >> > +++ b/drivers/media/platform/exynos4-is/mipi-csis.c >>>>> > >> > @@ -706,7 +706,8 @@ static irqreturn_t s5pcsis_irq_handler(int irq, void *dev_id) >>>>> > >> > else >>>>> > >> > offset = S5PCSIS_PKTDATA_ODD; >>>>> > >> > >>>>> > >> > - memcpy(pktbuf->data, state->regs + offset, pktbuf->len); >>>>> > >> > + memcpy(pktbuf->data, (u8 __force *)state->regs + offset, >>>>> > >> > + pktbuf->len); >>>>> > >> > pktbuf->data = NULL; >>>>> > >> > >>> > > >>> > > I think this is what memcpy_toio() is meant for. >> > >> > Exactly memcpy_fromio(). But it's implementation is inefficient on >> > ARCH=arm, memcpy_fromio() will be translated to a loop of readb(), >> > only if an arm sub-architecture provides a processor instruction >> > to access memory by byte. Each readb() also involves a memory barrier. >> > That's all what we wanted to avoid. AFAIR using memcpy_fromio() was >> > causing increase of the copy operation several times comparing to >> > memcpy(). On arm64 it looks better, but this driver is currently >> > used only on arm32. >> > >> > I would prefer to add (void __force *) instead: >> > >> > memcpy(pktbuf->data, (void __force *)state->regs + offset, pktbuf->len); >> > >> > Alternatively, the memset could just be replaced by a loop of >> > u32 reads - __raw_readl(); > > You are right for old kernels, but this was fixed in 7ddfe625cb ("ARM: > optimize memset_io()/memcpy_fromio()/memcpy_toio()") at least for > little-endian kernels and should be fine now on ARM just like > everywhere else. Indeed, I had just previously checked it in 4.0 kernel and missed those recent further optimizations. It should be fine to replace memcpy() with memcpy_fromio() then. -- Thanks, Sylwester ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 6/7] [media] c8sectpfe: fix namespace on memcpy/memset [not found] <cover.1443737682.git.mchehab@osg.samsung.com> 2015-10-01 22:17 ` [PATCH 5/7] [media] mipi-csis: make sparse happy Mauro Carvalho Chehab @ 2015-10-01 22:17 ` Mauro Carvalho Chehab 2015-10-02 22:27 ` Arnd Bergmann 1 sibling, 1 reply; 7+ messages in thread From: Mauro Carvalho Chehab @ 2015-10-01 22:17 UTC (permalink / raw) To: linux-arm-kernel Solves those sparse warnings: drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c:1087:9: warning: incorrect type in argument 1 (different address spaces) drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c:1087:9: expected void *<noident> drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c:1087:9: got void [noderef] <asn:2>*<noident> drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c:1090:9: warning: incorrect type in argument 1 (different address spaces) drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c:1090:9: expected void *<noident> drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c:1090:9: got void [noderef] <asn:2>* Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c index 486aef50d99b..e358b9163a68 100644 --- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c +++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c @@ -1084,10 +1084,10 @@ static void load_dmem_segment(struct c8sectpfei *fei, Elf32_Phdr *phdr, seg_num, phdr->p_paddr, phdr->p_filesz, dst, phdr->p_memsz); - memcpy((void __iomem *)dst, (void *)fw->data + phdr->p_offset, + memcpy((void __force *)dst, (void *)fw->data + phdr->p_offset, phdr->p_filesz); - memset((void __iomem *)dst + phdr->p_filesz, 0, + memset((void __force *)dst + phdr->p_filesz, 0, phdr->p_memsz - phdr->p_filesz); } -- 2.4.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 6/7] [media] c8sectpfe: fix namespace on memcpy/memset 2015-10-01 22:17 ` [PATCH 6/7] [media] c8sectpfe: fix namespace on memcpy/memset Mauro Carvalho Chehab @ 2015-10-02 22:27 ` Arnd Bergmann 0 siblings, 0 replies; 7+ messages in thread From: Arnd Bergmann @ 2015-10-02 22:27 UTC (permalink / raw) To: linux-arm-kernel On Thursday 01 October 2015 19:17:28 Mauro Carvalho Chehab wrote: > @@ -1084,10 +1084,10 @@ static void load_dmem_segment(struct c8sectpfei *fei, Elf32_Phdr *phdr, > seg_num, phdr->p_paddr, phdr->p_filesz, > dst, phdr->p_memsz); > > - memcpy((void __iomem *)dst, (void *)fw->data + phdr->p_offset, > + memcpy((void __force *)dst, (void *)fw->data + phdr->p_offset, > phdr->p_filesz); > > - memset((void __iomem *)dst + phdr->p_filesz, 0, > + memset((void __force *)dst + phdr->p_filesz, 0, > phdr->p_memsz - phdr->p_filesz); > } > Same here: this should really use memcpy_toio() for the first one, though it seems we don't have a corresponding memset_io(). Arnd ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-10-05 12:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1443737682.git.mchehab@osg.samsung.com>
2015-10-01 22:17 ` [PATCH 5/7] [media] mipi-csis: make sparse happy Mauro Carvalho Chehab
2015-10-02 22:25 ` Arnd Bergmann
2015-10-05 10:24 ` Sylwester Nawrocki
2015-10-05 11:07 ` Arnd Bergmann
2015-10-05 12:23 ` Sylwester Nawrocki
2015-10-01 22:17 ` [PATCH 6/7] [media] c8sectpfe: fix namespace on memcpy/memset Mauro Carvalho Chehab
2015-10-02 22:27 ` Arnd Bergmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).