diff mbox

[v2,1/2] mtd: fix wrong usage of ioremap_nocache() in uclinux.c map driver

Message ID 1342676566-25651-1-git-send-email-gerg@snapgear.com
State Accepted
Commit 08a3c4bc23e2b71191ed95d4fd3177c23660e34f
Headers show

Commit Message

Greg Ungerer July 19, 2012, 5:42 a.m. UTC 
From: Greg Ungerer <gerg@uclinux.org>

The uclinux.c mapping driver uses ioremap_nocache() to map its physical
mapping address to a system virtual address. Problem is that the region
it is mapping is not device memory. It is ordinary system RAM. On most
non-MMU systems this doesn't matter, and the mapping is always a 1:1
translation of the address. On paged memory systems on some architectures
the page table mappings are not compatible between normal RAM and device
memory.

If we want to use the uclinux.c mapping driver on real MMU enabled systems
we should be using the kernel virtual address that the mapping is at. For
architectures that support the traditional initrd they use phys_to_virt or
__va to convert the physical start initrd address to a kernel usable virtual
address. The uclinux filesystem mapping is even more restrictive than the
typical initrd, it always follows the kernels own bss section (so always in
directly mapped memory). Therefore we can use the usual phys_to_virt to
translate the physical start address to a virtual address.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
v2: added comments about use of phys_to_virt

 drivers/mtd/maps/uclinux.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

Comments

Artem Bityutskiy Aug. 17, 2012, 1:16 p.m. UTC | #1 
On Thu, 2012-07-19 at 15:42 +1000, gerg@snapgear.com wrote:
> From: Greg Ungerer <gerg@uclinux.org>
> 
> The uclinux.c mapping driver uses ioremap_nocache() to map its physical
> mapping address to a system virtual address. Problem is that the region
> it is mapping is not device memory. It is ordinary system RAM. On most
> non-MMU systems this doesn't matter, and the mapping is always a 1:1
> translation of the address. On paged memory systems on some architectures
> the page table mappings are not compatible between normal RAM and device
> memory.

I cannot compile m68k because of the following errors:

arch/m68k/platform/coldfire/clk.c:54:13: error: redefinition of 'clk_get'
include/linux/clk.h:281:27: note: previous definition of 'clk_get' was here
arch/m68k/platform/coldfire/clk.c:68:5: error: redefinition of 'clk_enable'
include/linux/clk.h:295:19: note: previous definition of 'clk_enable' was here
arch/m68k/platform/coldfire/clk.c:80:6: error: redefinition of 'clk_disable'
include/linux/clk.h:300:20: note: previous definition of 'clk_disable' was here
arch/m68k/platform/coldfire/clk.c:90:6: error: redefinition of 'clk_put'
include/linux/clk.h:291:20: note: previous definition of 'clk_put' was here
arch/m68k/platform/coldfire/clk.c:97:15: error: redefinition of 'clk_get_rate'
include/linux/clk.h:302:29: note: previous definition of 'clk_get_rate' was here
arch/m68k/platform/coldfire/clk.c:150:13: error: redefinition of 'devm_clk_get'
include/linux/clk.h:286:27: note: previous definition of 'devm_clk_get' was here
make[2]: *** [arch/m68k/platform/coldfire/clk.o] Error 1

so I cannot compile-test this patch. Do you have a work-around patch for
that compilation issue?
Greg Ungerer Aug. 18, 2012, 11:11 p.m. UTC | #2 
Hi Artem,

On 08/17/2012 11:16 PM, Artem Bityutskiy wrote:
> On Thu, 2012-07-19 at 15:42 +1000, gerg@snapgear.com wrote:
>> From: Greg Ungerer <gerg@uclinux.org>
>>
>> The uclinux.c mapping driver uses ioremap_nocache() to map its physical
>> mapping address to a system virtual address. Problem is that the region
>> it is mapping is not device memory. It is ordinary system RAM. On most
>> non-MMU systems this doesn't matter, and the mapping is always a 1:1
>> translation of the address. On paged memory systems on some architectures
>> the page table mappings are not compatible between normal RAM and device
>> memory.
>
> I cannot compile m68k because of the following errors:
>
> arch/m68k/platform/coldfire/clk.c:54:13: error: redefinition of 'clk_get'
> include/linux/clk.h:281:27: note: previous definition of 'clk_get' was here
> arch/m68k/platform/coldfire/clk.c:68:5: error: redefinition of 'clk_enable'
> include/linux/clk.h:295:19: note: previous definition of 'clk_enable' was here
> arch/m68k/platform/coldfire/clk.c:80:6: error: redefinition of 'clk_disable'
> include/linux/clk.h:300:20: note: previous definition of 'clk_disable' was here
> arch/m68k/platform/coldfire/clk.c:90:6: error: redefinition of 'clk_put'
> include/linux/clk.h:291:20: note: previous definition of 'clk_put' was here
> arch/m68k/platform/coldfire/clk.c:97:15: error: redefinition of 'clk_get_rate'
> include/linux/clk.h:302:29: note: previous definition of 'clk_get_rate' was here
> arch/m68k/platform/coldfire/clk.c:150:13: error: redefinition of 'devm_clk_get'
> include/linux/clk.h:286:27: note: previous definition of 'devm_clk_get' was here
> make[2]: *** [arch/m68k/platform/coldfire/clk.o] Error 1
>
> so I cannot compile-test this patch. Do you have a work-around patch for
> that compilation issue?

Yes, apply this patch:

   http://marc.info/?l=linux-m68k&m=134397575425772&w=2

It will be going to Linus this week.

Regards
Greg
Artem Bityutskiy Aug. 20, 2012, 9:17 a.m. UTC | #3 
On Thu, 2012-07-19 at 15:42 +1000, gerg@snapgear.com wrote:
> From: Greg Ungerer <gerg@uclinux.org>
> 
> The uclinux.c mapping driver uses ioremap_nocache() to map its physical
> mapping address to a system virtual address. Problem is that the region
> it is mapping is not device memory. It is ordinary system RAM. On most
> non-MMU systems this doesn't matter, and the mapping is always a 1:1
> translation of the address. On paged memory systems on some architectures
> the page table mappings are not compatible between normal RAM and device
> memory.

Took both to l2-mtd.git tree, thanks!
diff mbox

Patch

diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c
index cfff454..43400b6 100644
--- a/drivers/mtd/maps/uclinux.c
+++ b/drivers/mtd/maps/uclinux.c
@@ -68,10 +68,16 @@  static int __init uclinux_mtd_init(void)
 	printk("uclinux[mtd]: RAM probe address=0x%x size=0x%x\n",
 	       	(int) mapp->phys, (int) mapp->size);
 
-	mapp->virt = ioremap_nocache(mapp->phys, mapp->size);
+	/*
+	 * The filesystem is guaranteed to be in direct mapped memory. It is
+	 * directly following the kernels own bss region. Following the same
+	 * mechanism used by architectures setting up traditional initrds we
+	 * use phys_to_virt to get the virtual address of its start.
+	 */
+	mapp->virt = phys_to_virt(mapp->phys);
 
 	if (mapp->virt == 0) {
-		printk("uclinux[mtd]: ioremap_nocache() failed\n");
+		printk("uclinux[mtd]: no virtual mapping?\n");
 		return(-EIO);
 	}
 
@@ -80,7 +86,6 @@  static int __init uclinux_mtd_init(void)
 	mtd = do_map_probe("map_ram", mapp);
 	if (!mtd) {
 		printk("uclinux[mtd]: failed to find a mapping?\n");
-		iounmap(mapp->virt);
 		return(-ENXIO);
 	}
 
@@ -103,10 +108,8 @@  static void __exit uclinux_mtd_cleanup(void)
 		map_destroy(uclinux_ram_mtdinfo);
 		uclinux_ram_mtdinfo = NULL;
 	}
-	if (uclinux_ram_map.virt) {
-		iounmap((void *) uclinux_ram_map.virt);
+	if (uclinux_ram_map.virt)
 		uclinux_ram_map.virt = 0;
-	}
 }
 
 /****************************************************************************/