| Message ID | 1357835498-23904-1-git-send-email-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New, archived |
| Headers | show |
On 01/11/2013 02:31 AM, 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 safe 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. > > Just in case I missed something a warning is added if the driver has to > fall back to the RAM mapping. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Changes since v2, id:1350027693-19528-1-git-send-email-u.kleine-koenig@pengutronix.de: > > - drop a few "ram"s from printks instead of making them "ram/rom" > - fix a typo in the commit log and add rational for the introduced warning. > > Who is responsible for taking (or not) these two patches? David? Artem? > > Best regards > Uwe > > --- > drivers/mtd/maps/Kconfig | 2 +- > drivers/mtd/maps/uclinux.c | 26 +++++++++++++++++++++----- > 2 files changed, 22 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig > index 2e47c2e..0dc86fc 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 || COLDFIRE) > + depends on (MTD_RAM=y || MTD_ROM=y) && (!MMU || COLDFIRE) > 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 299bf88..b3a9c54 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]: probe address=0x%x size=0x%x\n", > (int) mapp->phys, (int) mapp->size); > > /* > @@ -82,7 +89,16 @@ 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"); I still don't like this, as per: http://lists.infradead.org/pipermail/linux-mtd/2012-October/044612.html Regards Greg > + } > + > if (!mtd) { > printk("uclinux[mtd]: failed to find a mapping?\n"); > return(-ENXIO); > @@ -118,6 +134,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 MTD for uClinux"); > > /****************************************************************************/ >
Hello Greg, On Fri, Jan 11, 2013 at 11:37:29PM +1000, Greg Ungerer wrote: > On 01/11/2013 02:31 AM, 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 safe 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. > > > >Just in case I missed something a warning is added if the driver has to > >fall back to the RAM mapping. > > > >Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > >--- > >Changes since v2, id:1350027693-19528-1-git-send-email-u.kleine-koenig@pengutronix.de: > > > > - drop a few "ram"s from printks instead of making them "ram/rom" > > - fix a typo in the commit log and add rational for the introduced warning. > > > >Who is responsible for taking (or not) these two patches? David? Artem? > > > >Best regards > >Uwe > > > >--- > > drivers/mtd/maps/Kconfig | 2 +- > > drivers/mtd/maps/uclinux.c | 26 +++++++++++++++++++++----- > > 2 files changed, 22 insertions(+), 6 deletions(-) > > > >diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig > >index 2e47c2e..0dc86fc 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 || COLDFIRE) > >+ depends on (MTD_RAM=y || MTD_ROM=y) && (!MMU || COLDFIRE) > > 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 299bf88..b3a9c54 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]: probe address=0x%x size=0x%x\n", > > (int) mapp->phys, (int) mapp->size); > > > > /* > >@@ -82,7 +89,16 @@ 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"); > > I still don't like this, as per: > > http://lists.infradead.org/pipermail/linux-mtd/2012-October/044612.html I remember your concerns, but as you didn't write back on my last try to explain why I consider the warning useful, I resent. You wrote: 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, so the warning will not trigger for you. And this is correct, because the only thing I want to catch is map_rom not being able to provide everything needed when map_ram does. I don't know what you would prefer. Just drop the warning? Force the usage of map_rom by depending on (or selecting) MTD_ROM? Best regards Uwe
On 01/12/2013 12:02 AM, Uwe Kleine-König wrote: > Hello Greg, > > On Fri, Jan 11, 2013 at 11:37:29PM +1000, Greg Ungerer wrote: >> On 01/11/2013 02:31 AM, 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 safe 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. >>> >>> Just in case I missed something a warning is added if the driver has to >>> fall back to the RAM mapping. >>> >>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> >>> --- >>> Changes since v2, id:1350027693-19528-1-git-send-email-u.kleine-koenig@pengutronix.de: >>> >>> - drop a few "ram"s from printks instead of making them "ram/rom" >>> - fix a typo in the commit log and add rational for the introduced warning. >>> >>> Who is responsible for taking (or not) these two patches? David? Artem? >>> >>> Best regards >>> Uwe >>> >>> --- >>> drivers/mtd/maps/Kconfig | 2 +- >>> drivers/mtd/maps/uclinux.c | 26 +++++++++++++++++++++----- >>> 2 files changed, 22 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig >>> index 2e47c2e..0dc86fc 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 || COLDFIRE) >>> + depends on (MTD_RAM=y || MTD_ROM=y) && (!MMU || COLDFIRE) >>> 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 299bf88..b3a9c54 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]: probe address=0x%x size=0x%x\n", >>> (int) mapp->phys, (int) mapp->size); >>> >>> /* >>> @@ -82,7 +89,16 @@ 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"); >> >> I still don't like this, as per: >> >> http://lists.infradead.org/pipermail/linux-mtd/2012-October/044612.html > I remember your concerns, but as you didn't write back on my last try to > explain why I consider the warning useful, I resent. > > You wrote: > > 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, so the warning will not trigger for you. And this is correct, > because the only thing I want to catch is map_rom not being able to > provide everything needed when map_ram does. But why? I still don't see how this is a useful runtime message. > I don't know what you would prefer. Just drop the warning? Force the > usage of map_rom by depending on (or selecting) MTD_ROM? I would suggest dropping the message. And yes making the use map_rom depend on CONFIG_MAP_ROM being enabled would seem to make sense to me. Regards Greg
diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig index 2e47c2e..0dc86fc 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 || COLDFIRE) + depends on (MTD_RAM=y || MTD_ROM=y) && (!MMU || COLDFIRE) 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 299bf88..b3a9c54 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]: probe address=0x%x size=0x%x\n", (int) mapp->phys, (int) mapp->size); /* @@ -82,7 +89,16 @@ 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"); return(-ENXIO); @@ -118,6 +134,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 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 safe 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. Just in case I missed something a warning is added if the driver has to fall back to the RAM mapping. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Changes since v2, id:1350027693-19528-1-git-send-email-u.kleine-koenig@pengutronix.de: - drop a few "ram"s from printks instead of making them "ram/rom" - fix a typo in the commit log and add rational for the introduced warning. Who is responsible for taking (or not) these two patches? David? Artem? Best regards Uwe --- drivers/mtd/maps/Kconfig | 2 +- drivers/mtd/maps/uclinux.c | 26 +++++++++++++++++++++----- 2 files changed, 22 insertions(+), 6 deletions(-)