* [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 0 siblings, 1 reply; 5+ messages in thread From: Mauro Carvalho Chehab @ 2015-10-01 22:17 UTC (permalink / raw) To: Linux Media Mailing List Cc: Mauro Carvalho Chehab, Kyungmin Park, Sylwester Nawrocki, Kukjin Kim, Krzysztof Kozlowski, linux-arm-kernel, linux-samsung-soc 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] 5+ messages in thread
* Re: [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; 5+ messages in thread From: Arnd Bergmann @ 2015-10-02 22:25 UTC (permalink / raw) To: linux-arm-kernel Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Krzysztof Kozlowski, linux-samsung-soc, Kyungmin Park, Kukjin Kim, Sylwester Nawrocki 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] 5+ messages in thread
* Re: [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; 5+ messages in thread From: Sylwester Nawrocki @ 2015-10-05 10:24 UTC (permalink / raw) To: Arnd Bergmann, Mauro Carvalho Chehab Cc: linux-arm-kernel, Linux Media Mailing List, Krzysztof Kozlowski, linux-samsung-soc, Kyungmin Park, Kukjin Kim 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] 5+ messages in thread
* Re: [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; 5+ messages in thread From: Arnd Bergmann @ 2015-10-05 11:07 UTC (permalink / raw) To: Sylwester Nawrocki Cc: Mauro Carvalho Chehab, linux-arm-kernel, Linux Media Mailing List, Krzysztof Kozlowski, linux-samsung-soc, Kyungmin Park, Kukjin Kim 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] 5+ messages in thread
* Re: [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; 5+ messages in thread From: Sylwester Nawrocki @ 2015-10-05 12:23 UTC (permalink / raw) To: Arnd Bergmann Cc: Mauro Carvalho Chehab, linux-arm-kernel, Linux Media Mailing List, Krzysztof Kozlowski, linux-samsung-soc, Kyungmin Park, Kukjin Kim 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] 5+ messages in thread
end of thread, other threads:[~2015-10-05 12:24 UTC | newest]
Thread overview: 5+ 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox