| Message ID | 1385296571-23599-1-git-send-email-ezequiel.garcia@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Nov 24, 2013 at 09:36:11AM -0300, Ezequiel Garcia wrote: > In order to remove the following ugly message: > > BUG: mapping for 0x00000000 at 0xff000000 out of vmalloc space > > the iotable mappings should be re-located inside the vmalloc > region. Such move was introduced at commit: > > commit 0536bdf33faff4d940ac094c77998cfac368cfff > Author: Nicolas Pitre <nicolas.pitre@linaro.org> > Date: Thu Aug 25 00:35:59 2011 -0400 > > ARM: move iotable mappings within the vmalloc region > > While at it, let's add some nicer defines to make the code > more readable. > > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> NAK. I'm guessing that you haven't properly tested this. Grep for UNCACHED_PHYS_0, and what you'll find is that your change will break standby and suspend on all PXA platforms.
On Sun, Nov 24, 2013 at 12:41:25PM +0000, Russell King - ARM Linux wrote: > On Sun, Nov 24, 2013 at 09:36:11AM -0300, Ezequiel Garcia wrote: > > In order to remove the following ugly message: > > > > BUG: mapping for 0x00000000 at 0xff000000 out of vmalloc space > > > > the iotable mappings should be re-located inside the vmalloc > > region. Such move was introduced at commit: > > > > commit 0536bdf33faff4d940ac094c77998cfac368cfff > > Author: Nicolas Pitre <nicolas.pitre@linaro.org> > > Date: Thu Aug 25 00:35:59 2011 -0400 > > > > ARM: move iotable mappings within the vmalloc region > > > > While at it, let's add some nicer defines to make the code > > more readable. > > > > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > NAK. I'm guessing that you haven't properly tested this. Grep for > UNCACHED_PHYS_0, and what you'll find is that your change will break > standby and suspend on all PXA platforms. Right, I fail to grep about this properly, sorry about that. Anyway, after grepping for 'UNCACHED' and doing some testings I found a few interesting things. First, UNCACHED_PHYS_0 seems to be used by PXA27xx and PXA25xx. The board I have here is PXA3xx, so there's no way I can test anything. Question: Should we add an #ifdef around the mapping, or is that just pure churn? Second, suspend/resume seems broken on my CM-X300 board, or at least the wake-up. It enters suspend/standby but then never seem to wake. Maybe I'm not pushing the right button, but according to the reference guide the SW2 (connected to EXTWAKE#) should do the trick. Ideas?
On Sun, 24 Nov 2013, Ezequiel Garcia wrote: > In order to remove the following ugly message: > > BUG: mapping for 0x00000000 at 0xff000000 out of vmalloc space > > the iotable mappings should be re-located inside the vmalloc > region. Such move was introduced at commit: > > commit 0536bdf33faff4d940ac094c77998cfac368cfff > Author: Nicolas Pitre <nicolas.pitre@linaro.org> > Date: Thu Aug 25 00:35:59 2011 -0400 > > ARM: move iotable mappings within the vmalloc region > > While at it, let's add some nicer defines to make the code > more readable. > > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > arch/arm/mach-pxa/generic.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/mach-pxa/generic.c b/arch/arm/mach-pxa/generic.c > index 4225417..7c01095 100644 > --- a/arch/arm/mach-pxa/generic.c > +++ b/arch/arm/mach-pxa/generic.c > @@ -24,6 +24,7 @@ > #include <mach/hardware.h> > #include <asm/mach/map.h> > #include <asm/mach-types.h> > +#include <asm/pgtable.h> > > #include <mach/reset.h> > #include <mach/smemc.h> > @@ -77,6 +78,11 @@ EXPORT_SYMBOL(get_clk_frequency_khz); > * Note: virtual 0xfffe0000-0xffffffff is reserved for the vector table > * and cache flush area. > */ > + > +#define UNCACHED_PHY 0x00000000 > +#define UNCACHED_SIZE SZ_1M > +#define UNCACHED_VIRT (VMALLOC_END - UNCACHED_SIZE) > + > static struct map_desc common_io_desc[] __initdata = { > { /* Devs */ > .virtual = 0xf2000000, > @@ -84,9 +90,9 @@ static struct map_desc common_io_desc[] __initdata = { > .length = 0x02000000, > .type = MT_DEVICE > }, { /* UNCACHED_PHYS_0 */ > - .virtual = 0xff000000, > - .pfn = __phys_to_pfn(0x00000000), > - .length = 0x00100000, > + .virtual = UNCACHED_VIRT, > + .pfn = __phys_to_pfn(UNCACHED_PHY), > + .length = UNCACHED_SIZE, > .type = MT_DEVICE > } > }; You might wish to change the virtual address for this mapping, but nowhere in your patch you are changing users of that virtual mapping. Nicolas
On Sun, Nov 24, 2013 at 06:26:45PM -0500, Nicolas Pitre wrote: > On Sun, 24 Nov 2013, Ezequiel Garcia wrote: > > > In order to remove the following ugly message: > > > > BUG: mapping for 0x00000000 at 0xff000000 out of vmalloc space > > > > the iotable mappings should be re-located inside the vmalloc > > region. Such move was introduced at commit: > > > > commit 0536bdf33faff4d940ac094c77998cfac368cfff > > Author: Nicolas Pitre <nicolas.pitre@linaro.org> > > Date: Thu Aug 25 00:35:59 2011 -0400 > > > > ARM: move iotable mappings within the vmalloc region > > > > While at it, let's add some nicer defines to make the code > > more readable. > > > > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > --- > > arch/arm/mach-pxa/generic.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm/mach-pxa/generic.c b/arch/arm/mach-pxa/generic.c > > index 4225417..7c01095 100644 > > --- a/arch/arm/mach-pxa/generic.c > > +++ b/arch/arm/mach-pxa/generic.c > > @@ -24,6 +24,7 @@ > > #include <mach/hardware.h> > > #include <asm/mach/map.h> > > #include <asm/mach-types.h> > > +#include <asm/pgtable.h> > > > > #include <mach/reset.h> > > #include <mach/smemc.h> > > @@ -77,6 +78,11 @@ EXPORT_SYMBOL(get_clk_frequency_khz); > > * Note: virtual 0xfffe0000-0xffffffff is reserved for the vector table > > * and cache flush area. > > */ > > + > > +#define UNCACHED_PHY 0x00000000 > > +#define UNCACHED_SIZE SZ_1M > > +#define UNCACHED_VIRT (VMALLOC_END - UNCACHED_SIZE) > > + > > static struct map_desc common_io_desc[] __initdata = { > > { /* Devs */ > > .virtual = 0xf2000000, > > @@ -84,9 +90,9 @@ static struct map_desc common_io_desc[] __initdata = { > > .length = 0x02000000, > > .type = MT_DEVICE > > }, { /* UNCACHED_PHYS_0 */ > > - .virtual = 0xff000000, > > - .pfn = __phys_to_pfn(0x00000000), > > - .length = 0x00100000, > > + .virtual = UNCACHED_VIRT, > > + .pfn = __phys_to_pfn(UNCACHED_PHY), > > + .length = UNCACHED_SIZE, > > .type = MT_DEVICE > > } > > }; > > You might wish to change the virtual address for this mapping, but > nowhere in your patch you are changing users of that virtual mapping. > Yeah, this patch is pure crap. I'll re-work it and submit a new (hopefully not stupid) version. Thanks for taking a look!
diff --git a/arch/arm/mach-pxa/generic.c b/arch/arm/mach-pxa/generic.c index 4225417..7c01095 100644 --- a/arch/arm/mach-pxa/generic.c +++ b/arch/arm/mach-pxa/generic.c @@ -24,6 +24,7 @@ #include <mach/hardware.h> #include <asm/mach/map.h> #include <asm/mach-types.h> +#include <asm/pgtable.h> #include <mach/reset.h> #include <mach/smemc.h> @@ -77,6 +78,11 @@ EXPORT_SYMBOL(get_clk_frequency_khz); * Note: virtual 0xfffe0000-0xffffffff is reserved for the vector table * and cache flush area. */ + +#define UNCACHED_PHY 0x00000000 +#define UNCACHED_SIZE SZ_1M +#define UNCACHED_VIRT (VMALLOC_END - UNCACHED_SIZE) + static struct map_desc common_io_desc[] __initdata = { { /* Devs */ .virtual = 0xf2000000, @@ -84,9 +90,9 @@ static struct map_desc common_io_desc[] __initdata = { .length = 0x02000000, .type = MT_DEVICE }, { /* UNCACHED_PHYS_0 */ - .virtual = 0xff000000, - .pfn = __phys_to_pfn(0x00000000), - .length = 0x00100000, + .virtual = UNCACHED_VIRT, + .pfn = __phys_to_pfn(UNCACHED_PHY), + .length = UNCACHED_SIZE, .type = MT_DEVICE } };
In order to remove the following ugly message: BUG: mapping for 0x00000000 at 0xff000000 out of vmalloc space the iotable mappings should be re-located inside the vmalloc region. Such move was introduced at commit: commit 0536bdf33faff4d940ac094c77998cfac368cfff Author: Nicolas Pitre <nicolas.pitre@linaro.org> Date: Thu Aug 25 00:35:59 2011 -0400 ARM: move iotable mappings within the vmalloc region While at it, let's add some nicer defines to make the code more readable. Cc: Nicolas Pitre <nicolas.pitre@linaro.org> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- arch/arm/mach-pxa/generic.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)