| Message ID | 1350027693-19528-1-git-send-email-u.kleine-koenig@pengutronix.de |
|---|---|
| State | RFC |
| Headers | show |
Hi Uwe, On 12/10/12 17:41, Uwe Kleine-König wrote: > This allows to put the filesystem at a defined address in ROM allowing > to save more precious RAM. > > I think it's save to default to ROM because the intention of using the ^^^^ safe > uclinux map is to use a romfs and so mtd-ram doesn't give you anything > that mtd-rom doesn't. > > Signed-off-by: Uwe Kleine-K÷nig <u.kleine-koenig@pengutronix.de> Unfortunately email to me goes through an exchange server and openchange, and it manages to often mangle anything more than 7bit ascii. Not too much I can do about it, sorry. > Changed since (implicit) v1: > > - don't make uclinux_ram_map static as pointed out by Mike and Greg. > > drivers/mtd/maps/Kconfig | 2 +- > drivers/mtd/maps/uclinux.c | 28 ++++++++++++++++++++++------ > 2 files changed, 23 insertions(+), 7 deletions(-) > > diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig > index 5ba2458..d945950 100644 > --- a/drivers/mtd/maps/Kconfig > +++ b/drivers/mtd/maps/Kconfig > @@ -443,7 +443,7 @@ config MTD_GPIO_ADDR > > config MTD_UCLINUX > bool "Generic uClinux RAM/ROM filesystem support" > - depends on MTD_RAM=y && !MMU > + depends on (MTD_RAM=y || MTD_ROM=y) && !MMU > help > Map driver to support image based filesystems for uClinux. > > diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c > index c3bb304..f5e8e9a 100644 > --- a/drivers/mtd/maps/uclinux.c > +++ b/drivers/mtd/maps/uclinux.c > @@ -24,11 +24,12 @@ > /****************************************************************************/ > > struct map_info uclinux_ram_map = { > - .name = "RAM", > - .phys = (unsigned long)__bss_stop, > .size = 0, > }; > > +static unsigned long physaddr = -1; > +module_param(physaddr, ulong, S_IRUGO); > + > static struct mtd_info *uclinux_ram_mtdinfo; > > /****************************************************************************/ > @@ -60,11 +61,17 @@ static int __init uclinux_mtd_init(void) > struct map_info *mapp; > > mapp = &uclinux_ram_map; > + > + if (physaddr == -1) > + mapp->phys = (resource_size_t)__bss_stop; > + else > + mapp->phys = physaddr; > + > if (!mapp->size) > mapp->size = PAGE_ALIGN(ntohl(*((unsigned long *)(mapp->phys + 8)))); > mapp->bankwidth = 4; > > - printk("uclinux[mtd]: RAM probe address=0x%x size=0x%x\n", > + printk("uclinux[mtd]: RAM/ROM probe address=0x%x size=0x%x\n", > (int) mapp->phys, (int) mapp->size); Is there any value in saying "RAM/ROM"? Why don't we just drop those chars altogether. > > mapp->virt = ioremap_nocache(mapp->phys, mapp->size); > @@ -76,9 +83,18 @@ static int __init uclinux_mtd_init(void) > > simple_map_init(mapp); > > - mtd = do_map_probe("map_ram", mapp); > + mapp->name = "ROM"; > + mtd = do_map_probe("map_rom", mapp); > + if (!mtd) { > + /* fall back to ram probing for compatibility reasons */ > + mapp->name = "RAM"; > + mtd = do_map_probe("map_ram", mapp); > + if (mtd && IS_ENABLED(CONFIG_MTD_ROM)) > + pr_err("Failed to map rom, but ram succeeded. Please report this issue!\n"); Do we really want this message? My predominate usage of this code is in RAM mappings. Network loading kernel+filesystem images on bare boards. Anyone who wants to know and is looking in the kernel boot messages will see something like: Creating 1 MTD partitions on "RAM": 0x000000000000-0x0000000d8000 : "ROMfs" So they will know what type of mapping it was loaded from. > + } > + > if (!mtd) { > - printk("uclinux[mtd]: failed to find a mapping?\n"); > + printk("uclinux[mtd]: failed to find a rom/ram mapping?\n"); Again, is there any point in changing this message? Adding "rom/ram" doesn't add any value here. Regards Greg > iounmap(mapp->virt); > return(-ENXIO); > } > @@ -115,6 +131,6 @@ module_exit(uclinux_mtd_cleanup); > > MODULE_LICENSE("GPL"); > MODULE_AUTHOR("Greg Ungerer <gerg@snapgear.com>"); > -MODULE_DESCRIPTION("Generic RAM based MTD for uClinux"); > +MODULE_DESCRIPTION("Generic RAM/ROM based MTD for uClinux"); > > /****************************************************************************/ >
Hello, On Mon, Oct 15, 2012 at 02:14:55PM +1000, Greg Ungerer wrote: > Hi Uwe, > > On 12/10/12 17:41, Uwe Kleine-König wrote: > >This allows to put the filesystem at a defined address in ROM allowing > >to save more precious RAM. > > > >I think it's save to default to ROM because the intention of using the > ^^^^ > safe > > >uclinux map is to use a romfs and so mtd-ram doesn't give you anything > >that mtd-rom doesn't. > > > >Signed-off-by: Uwe Kleine-K÷nig <u.kleine-koenig@pengutronix.de> > > Unfortunately email to me goes through an exchange server and openchange, > and it manages to often mangle anything more than 7bit ascii. Not too > much I can do about it, sorry. hehe, so openchange is really MS compatible now? :-) > >Changed since (implicit) v1: > > > > - don't make uclinux_ram_map static as pointed out by Mike and Greg. > > > > drivers/mtd/maps/Kconfig | 2 +- > > drivers/mtd/maps/uclinux.c | 28 ++++++++++++++++++++++------ > > 2 files changed, 23 insertions(+), 7 deletions(-) > > > >diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig > >index 5ba2458..d945950 100644 > >--- a/drivers/mtd/maps/Kconfig > >+++ b/drivers/mtd/maps/Kconfig > >@@ -443,7 +443,7 @@ config MTD_GPIO_ADDR > > > > config MTD_UCLINUX > > bool "Generic uClinux RAM/ROM filesystem support" > >- depends on MTD_RAM=y && !MMU > >+ depends on (MTD_RAM=y || MTD_ROM=y) && !MMU > > help > > Map driver to support image based filesystems for uClinux. > > > >diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c > >index c3bb304..f5e8e9a 100644 > >--- a/drivers/mtd/maps/uclinux.c > >+++ b/drivers/mtd/maps/uclinux.c > >@@ -24,11 +24,12 @@ > > /****************************************************************************/ > > > > struct map_info uclinux_ram_map = { > >- .name = "RAM", > >- .phys = (unsigned long)__bss_stop, > > .size = 0, > > }; > > > >+static unsigned long physaddr = -1; > >+module_param(physaddr, ulong, S_IRUGO); > >+ > > static struct mtd_info *uclinux_ram_mtdinfo; > > > > /****************************************************************************/ > >@@ -60,11 +61,17 @@ static int __init uclinux_mtd_init(void) > > struct map_info *mapp; > > > > mapp = &uclinux_ram_map; > >+ > >+ if (physaddr == -1) > >+ mapp->phys = (resource_size_t)__bss_stop; > >+ else > >+ mapp->phys = physaddr; > >+ > > if (!mapp->size) > > mapp->size = PAGE_ALIGN(ntohl(*((unsigned long *)(mapp->phys + 8)))); > > mapp->bankwidth = 4; > > > >- printk("uclinux[mtd]: RAM probe address=0x%x size=0x%x\n", > >+ printk("uclinux[mtd]: RAM/ROM probe address=0x%x size=0x%x\n", > > (int) mapp->phys, (int) mapp->size); > > Is there any value in saying "RAM/ROM"? > Why don't we just drop those chars altogether. ok. > > mapp->virt = ioremap_nocache(mapp->phys, mapp->size); > >@@ -76,9 +83,18 @@ static int __init uclinux_mtd_init(void) > > > > simple_map_init(mapp); > > > >- mtd = do_map_probe("map_ram", mapp); > >+ mapp->name = "ROM"; > >+ mtd = do_map_probe("map_rom", mapp); > >+ if (!mtd) { > >+ /* fall back to ram probing for compatibility reasons */ > >+ mapp->name = "RAM"; > >+ mtd = do_map_probe("map_ram", mapp); > >+ if (mtd && IS_ENABLED(CONFIG_MTD_ROM)) > >+ pr_err("Failed to map rom, but ram succeeded. Please report this issue!\n"); > > Do we really want this message? > My predominate usage of this code is in RAM mappings. Network > loading kernel+filesystem images on bare boards. Anyone who wants > to know and is looking in the kernel boot messages will see > something like: > > Creating 1 MTD partitions on "RAM": > 0x000000000000-0x0000000d8000 : "ROMfs" > > So they will know what type of mapping it was loaded from. I want it because if nobody reports it it might well be possible to drop map_ram support. Best regards Uwe
Hi Uwe, On 15/10/12 16:58, Uwe Kleine-König wrote: > On Mon, Oct 15, 2012 at 02:14:55PM +1000, Greg Ungerer wrote: >>> mapp->virt = ioremap_nocache(mapp->phys, mapp->size); >>> @@ -76,9 +83,18 @@ static int __init uclinux_mtd_init(void) >>> >>> simple_map_init(mapp); >>> >>> - mtd = do_map_probe("map_ram", mapp); >>> + mapp->name = "ROM"; >>> + mtd = do_map_probe("map_rom", mapp); >>> + if (!mtd) { >>> + /* fall back to ram probing for compatibility reasons */ >>> + mapp->name = "RAM"; >>> + mtd = do_map_probe("map_ram", mapp); >>> + if (mtd && IS_ENABLED(CONFIG_MTD_ROM)) >>> + pr_err("Failed to map rom, but ram succeeded. Please report this issue!\n"); >> >> Do we really want this message? >> My predominate usage of this code is in RAM mappings. Network >> loading kernel+filesystem images on bare boards. Anyone who wants >> to know and is looking in the kernel boot messages will see >> something like: >> >> Creating 1 MTD partitions on "RAM": >> 0x000000000000-0x0000000d8000 : "ROMfs" >> >> So they will know what type of mapping it was loaded from. > I want it because if nobody reports it it might well be possible to drop > map_ram support. I am not sure I follow. The message is only printed if both ROM and RAM mappings are enabled. Many of the configs I use only have RAM mappings enabled. Regards Greg ------------------------------------------------------------------------ Greg Ungerer -- Principal Engineer EMAIL: gerg@snapgear.com SnapGear Group, McAfee PHONE: +61 7 3435 2888 8 Gardner Close FAX: +61 7 3217 5323 Milton, QLD, 4064, Australia WEB: http://www.SnapGear.com
Hello Greg, On Tue, Oct 16, 2012 at 11:13:05AM +1000, Greg Ungerer wrote: > On 15/10/12 16:58, Uwe Kleine-König wrote: > >On Mon, Oct 15, 2012 at 02:14:55PM +1000, Greg Ungerer wrote: > >>> mapp->virt = ioremap_nocache(mapp->phys, mapp->size); > >>>@@ -76,9 +83,18 @@ static int __init uclinux_mtd_init(void) > >>> > >>> simple_map_init(mapp); > >>> > >>>- mtd = do_map_probe("map_ram", mapp); > >>>+ mapp->name = "ROM"; > >>>+ mtd = do_map_probe("map_rom", mapp); > >>>+ if (!mtd) { > >>>+ /* fall back to ram probing for compatibility reasons */ > >>>+ mapp->name = "RAM"; > >>>+ mtd = do_map_probe("map_ram", mapp); > >>>+ if (mtd && IS_ENABLED(CONFIG_MTD_ROM)) > >>>+ pr_err("Failed to map rom, but ram succeeded. Please report this issue!\n"); > >> > >>Do we really want this message? > >>My predominate usage of this code is in RAM mappings. Network > >>loading kernel+filesystem images on bare boards. Anyone who wants > >>to know and is looking in the kernel boot messages will see > >>something like: > >> > >> Creating 1 MTD partitions on "RAM": > >> 0x000000000000-0x0000000d8000 : "ROMfs" > >> > >>So they will know what type of mapping it was loaded from. > >I want it because if nobody reports it it might well be possible to drop > >map_ram support. > > I am not sure I follow. The message is only printed if both ROM and > RAM mappings are enabled. Many of the configs I use only have RAM > mappings enabled. Yeah, in this case it doesn't help. I also thought about adding a warning for !IS_ENABLED(CONFIG_MTD_ROM). But this seemed too harsh, so I dropped the idea. Still, the theory is that MTD_ROM should be enough for the uclinux map. If it's not it's at least not silently fixed up and we might get a report about what was wrong with my assumption. Best regards Uwe
diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig index 5ba2458..d945950 100644 --- a/drivers/mtd/maps/Kconfig +++ b/drivers/mtd/maps/Kconfig @@ -443,7 +443,7 @@ config MTD_GPIO_ADDR config MTD_UCLINUX bool "Generic uClinux RAM/ROM filesystem support" - depends on MTD_RAM=y && !MMU + depends on (MTD_RAM=y || MTD_ROM=y) && !MMU help Map driver to support image based filesystems for uClinux. diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c index c3bb304..f5e8e9a 100644 --- a/drivers/mtd/maps/uclinux.c +++ b/drivers/mtd/maps/uclinux.c @@ -24,11 +24,12 @@ /****************************************************************************/ struct map_info uclinux_ram_map = { - .name = "RAM", - .phys = (unsigned long)__bss_stop, .size = 0, }; +static unsigned long physaddr = -1; +module_param(physaddr, ulong, S_IRUGO); + static struct mtd_info *uclinux_ram_mtdinfo; /****************************************************************************/ @@ -60,11 +61,17 @@ static int __init uclinux_mtd_init(void) struct map_info *mapp; mapp = &uclinux_ram_map; + + if (physaddr == -1) + mapp->phys = (resource_size_t)__bss_stop; + else + mapp->phys = physaddr; + if (!mapp->size) mapp->size = PAGE_ALIGN(ntohl(*((unsigned long *)(mapp->phys + 8)))); mapp->bankwidth = 4; - printk("uclinux[mtd]: RAM probe address=0x%x size=0x%x\n", + printk("uclinux[mtd]: RAM/ROM probe address=0x%x size=0x%x\n", (int) mapp->phys, (int) mapp->size); mapp->virt = ioremap_nocache(mapp->phys, mapp->size); @@ -76,9 +83,18 @@ static int __init uclinux_mtd_init(void) simple_map_init(mapp); - mtd = do_map_probe("map_ram", mapp); + mapp->name = "ROM"; + mtd = do_map_probe("map_rom", mapp); + if (!mtd) { + /* fall back to ram probing for compatibility reasons */ + mapp->name = "RAM"; + mtd = do_map_probe("map_ram", mapp); + if (mtd && IS_ENABLED(CONFIG_MTD_ROM)) + pr_err("Failed to map rom, but ram succeeded. Please report this issue!\n"); + } + if (!mtd) { - printk("uclinux[mtd]: failed to find a mapping?\n"); + printk("uclinux[mtd]: failed to find a rom/ram mapping?\n"); iounmap(mapp->virt); return(-ENXIO); } @@ -115,6 +131,6 @@ module_exit(uclinux_mtd_cleanup); MODULE_LICENSE("GPL"); MODULE_AUTHOR("Greg Ungerer <gerg@snapgear.com>"); -MODULE_DESCRIPTION("Generic RAM based MTD for uClinux"); +MODULE_DESCRIPTION("Generic RAM/ROM based MTD for uClinux"); /****************************************************************************/
This allows to put the filesystem at a defined address in ROM allowing to save more precious RAM. I think it's save to default to ROM because the intention of using the uclinux map is to use a romfs and so mtd-ram doesn't give you anything that mtd-rom doesn't. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> -- Changed since (implicit) v1: - don't make uclinux_ram_map static as pointed out by Mike and Greg. drivers/mtd/maps/Kconfig | 2 +- drivers/mtd/maps/uclinux.c | 28 ++++++++++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-)