* [PATCH 1/2] staging/vc04_services: initialize cache line size properly
@ 2017-03-01 22:50 Arnd Bergmann
2017-03-01 22:50 ` [PATCH 2/2] staging/vc04_services: add CONFIG_OF dependency Arnd Bergmann
2017-03-02 1:59 ` [PATCH 1/2] staging/vc04_services: initialize cache line size properly Michael Zoran
0 siblings, 2 replies; 4+ messages in thread
From: Arnd Bergmann @ 2017-03-01 22:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Arnd Bergmann, Stephen Warren, Lee Jones, Eric Anholt,
Michael Zoran, Stefan Wahren, Daniel Stone, popcornmix,
linux-rpi-kernel, linux-arm-kernel, devel, linux-kernel
While debugging another problem I noticed that g_cache_line_size gets set
to sizeof(CACHE_LINE_SIZE), which is sizeof(int) or 4, while presumably
CACHE_LINE_SIZE (e.g. 32) was meant.
This initializes it the way it was meant.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 3aeffcb9c87e..b0e9eb6ff73f 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -78,7 +78,7 @@ struct vchiq_pagelist_info {
};
static void __iomem *g_regs;
-static unsigned int g_cache_line_size = sizeof(CACHE_LINE_SIZE);
+static unsigned int g_cache_line_size = CACHE_LINE_SIZE;
static unsigned int g_fragments_size;
static char *g_fragments_base;
static char *g_free_fragments;
--
2.9.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] staging/vc04_services: add CONFIG_OF dependency
2017-03-01 22:50 [PATCH 1/2] staging/vc04_services: initialize cache line size properly Arnd Bergmann
@ 2017-03-01 22:50 ` Arnd Bergmann
2017-03-02 1:59 ` [PATCH 1/2] staging/vc04_services: initialize cache line size properly Michael Zoran
1 sibling, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2017-03-01 22:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Arnd Bergmann, Stephen Warren, Lee Jones, Eric Anholt,
Daniel Stone, Noralf Trønnes, Colin Ian King, popcornmix,
Stefan Wahren, linux-rpi-kernel, linux-arm-kernel, devel,
linux-kernel
After several hours of debugging this obviously bogus but elaborate
gcc-7.0.1 warning,
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c: In function 'vchiq_complete_bulk':
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:603:4: error: argument 2 null where non-null expected [-Werror=nonnull]
memcpy((char *)page_address(pages[0]) +
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pagelist->offset,
~~~~~~~~~~~~~~~~~
fragments,
~~~~~~~~~~
head_bytes);
~~~~~~~~~~~
In file included from include/linux/string.h:18:0,
from include/linux/bitmap.h:8,
from include/linux/cpumask.h:11,
from include/linux/interrupt.h:9,
from drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:37:
arch/arm/include/asm/string.h:16:15: note: in a call to function 'memcpy' declared here
extern void * memcpy(void *, const void *, __kernel_size_t) __nocapture(2);
^~~~~~
I have concluded that gcc was technically right in the first place:
vchiq_complete_bulk is an externally visible function that calls
free_pagelist(), which in turn derives a pointer from the global
g_fragments_base variable.
g_fragments_base is initialized in vchiq_platform_init(), but
we only get there if of_property_read_u32() successfully reads the
cache line size. When CONFIG_OF is disabled, this always fails, and
g_fragments_base is guaranteed to be NULL when vchiq_complete_bulk()
gets called.
This adds a CONFIG_OF Kconfig dependency, which is also technically correct
but nonobvious, and thus seems like a good fit for the warning.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/vc04_services/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
index e61e4ca064a8..74094fff4367 100644
--- a/drivers/staging/vc04_services/Kconfig
+++ b/drivers/staging/vc04_services/Kconfig
@@ -1,6 +1,7 @@
config BCM2835_VCHIQ
tristate "Videocore VCHIQ"
depends on HAS_DMA
+ depends on OF
depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
default y
help
--
2.9.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] staging/vc04_services: initialize cache line size properly
2017-03-01 22:50 [PATCH 1/2] staging/vc04_services: initialize cache line size properly Arnd Bergmann
2017-03-01 22:50 ` [PATCH 2/2] staging/vc04_services: add CONFIG_OF dependency Arnd Bergmann
@ 2017-03-02 1:59 ` Michael Zoran
2017-03-06 12:53 ` Greg Kroah-Hartman
1 sibling, 1 reply; 4+ messages in thread
From: Michael Zoran @ 2017-03-02 1:59 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman
Cc: Stephen Warren, Lee Jones, Eric Anholt, Stefan Wahren,
Daniel Stone, popcornmix, linux-rpi-kernel, linux-arm-kernel,
devel, linux-kernel
Hi Arnd,
I submitted a change which is in Linux-next now that makes the whole
CACHE_LINE_SIZE macro meaningless. It now always reads the size from
the DT and errors out with -ENODEV if the property is missing.
I was going to submit a change to delete the macro completely, just
never got to it.
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit
/drivers/staging/vc04_services/interface?id=6cf1bf636a067eb308cb3a8322b
9d6b1844a075d
On Wed, 2017-03-01 at 23:50 +0100, Arnd Bergmann wrote:
> While debugging another problem I noticed that g_cache_line_size gets
> set
> to sizeof(CACHE_LINE_SIZE), which is sizeof(int) or 4, while
> presumably
> CACHE_LINE_SIZE (e.g. 32) was meant.
>
> This initializes it the way it was meant.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c |
> 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index 3aeffcb9c87e..b0e9eb6ff73f 100644
> ---
> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -78,7 +78,7 @@ struct vchiq_pagelist_info {
> };
>
> static void __iomem *g_regs;
> -static unsigned int g_cache_line_size = sizeof(CACHE_LINE_SIZE);
> +static unsigned int g_cache_line_size = CACHE_LINE_SIZE;
> static unsigned int g_fragments_size;
> static char *g_fragments_base;
> static char *g_free_fragments;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] staging/vc04_services: initialize cache line size properly
2017-03-02 1:59 ` [PATCH 1/2] staging/vc04_services: initialize cache line size properly Michael Zoran
@ 2017-03-06 12:53 ` Greg Kroah-Hartman
0 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2017-03-06 12:53 UTC (permalink / raw)
To: Michael Zoran
Cc: Arnd Bergmann, Stefan Wahren, devel, Daniel Stone,
Stephen Warren, Lee Jones, linux-kernel, Eric Anholt,
linux-rpi-kernel, popcornmix, linux-arm-kernel
On Wed, Mar 01, 2017 at 05:59:38PM -0800, Michael Zoran wrote:
> Hi Arnd,
>
> I submitted a change which is in Linux-next now that makes the whole
> CACHE_LINE_SIZE macro meaningless. It now always reads the size from
> the DT and errors out with -ENODEV if the property is missing.
>
> I was going to submit a change to delete the macro completely, just
> never got to it.
>
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit
> /drivers/staging/vc04_services/interface?id=6cf1bf636a067eb308cb3a8322b
> 9d6b1844a075d
This should be in 4.11-rc1.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-06 12:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-01 22:50 [PATCH 1/2] staging/vc04_services: initialize cache line size properly Arnd Bergmann
2017-03-01 22:50 ` [PATCH 2/2] staging/vc04_services: add CONFIG_OF dependency Arnd Bergmann
2017-03-02 1:59 ` [PATCH 1/2] staging/vc04_services: initialize cache line size properly Michael Zoran
2017-03-06 12:53 ` Greg Kroah-Hartman
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).