| Message ID | 1243331191-11445-2-git-send-email-vapier@gentoo.org |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 26, 2009 at 05:46:31AM -0400, Mike Frysinger wrote: > diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c > index 57699c2..dcb552f 100644 > --- a/drivers/mtd/maps/uclinux.c > +++ b/drivers/mtd/maps/uclinux.c > @@ -55,8 +55,13 @@ static int __init uclinux_mtd_init(void) > { > struct mtd_info *mtd; > struct map_info *mapp; > +#ifdef CONFIG_BLACKFIN > + extern unsigned long memory_mtd_start; > + unsigned long addr = (unsigned long) memory_mtd_start; > +#else > extern char _ebss; > unsigned long addr = (unsigned long) &_ebss; > +#endif > > mapp = &uclinux_ram_map; > mapp->phys = addr; NAK. I know there's no accounting for taste, but it would be nice to at least see some minimal amount of effort going in to fixing these things sanely rather than just lazily shoving ifdefs in wherever possible. In this case you should just kill all of that crap off, and have the platforms that use this set uclinux_ram_map up themselves, it's already a global. Of course you can use _ebss as a default value for uclinux_ram_map.phys and just override it in your platform.
On Tue, May 26, 2009 at 07:31, Paul Mundt wrote: > On Tue, May 26, 2009 at 05:46:31AM -0400, Mike Frysinger wrote: >> diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c >> index 57699c2..dcb552f 100644 >> --- a/drivers/mtd/maps/uclinux.c >> +++ b/drivers/mtd/maps/uclinux.c >> @@ -55,8 +55,13 @@ static int __init uclinux_mtd_init(void) >> { >> struct mtd_info *mtd; >> struct map_info *mapp; >> +#ifdef CONFIG_BLACKFIN >> + extern unsigned long memory_mtd_start; >> + unsigned long addr = (unsigned long) memory_mtd_start; >> +#else >> extern char _ebss; >> unsigned long addr = (unsigned long) &_ebss; >> +#endif >> >> mapp = &uclinux_ram_map; >> mapp->phys = addr; > > NAK. > > I know there's no accounting for taste, but it would be nice to at least > see some minimal amount of effort going in to fixing these things > sanely rather than just lazily shoving ifdefs in wherever possible. > > In this case you should just kill all of that crap off, and have the > platforms that use this set uclinux_ram_map up themselves, it's already > a global. Of course you can use _ebss as a default value for > uclinux_ram_map.phys and just override it in your platform. i would agree if it were a board-specific issue, but it's an arch issue, so pushing it to the boards level is wrong. i can however replace the addr with a global weak and add a symbol into the Blackfin arch code to override it. the structs being global in this file is simply wrong from what i can tell ... everything in there should be static. i'll send a patch for that later on. -mike
On Tue, May 26, 2009 at 12:42:48PM -0400, Mike Frysinger wrote: > On Tue, May 26, 2009 at 07:31, Paul Mundt wrote: > > On Tue, May 26, 2009 at 05:46:31AM -0400, Mike Frysinger wrote: > >> diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c > >> index 57699c2..dcb552f 100644 > >> --- a/drivers/mtd/maps/uclinux.c > >> +++ b/drivers/mtd/maps/uclinux.c > >> @@ -55,8 +55,13 @@ static int __init uclinux_mtd_init(void) > >> ??{ > >> ?? ?? ?? struct mtd_info *mtd; > >> ?? ?? ?? struct map_info *mapp; > >> +#ifdef CONFIG_BLACKFIN > >> + ?? ?? extern unsigned long memory_mtd_start; > >> + ?? ?? unsigned long addr = (unsigned long) memory_mtd_start; > >> +#else > >> ?? ?? ?? extern char _ebss; > >> ?? ?? ?? unsigned long addr = (unsigned long) &_ebss; > >> +#endif > >> > >> ?? ?? ?? mapp = &uclinux_ram_map; > >> ?? ?? ?? mapp->phys = addr; > > > > NAK. > > > > I know there's no accounting for taste, but it would be nice to at least > > see some minimal amount of effort going in to fixing these things > > sanely rather than just lazily shoving ifdefs in wherever possible. > > > > In this case you should just kill all of that crap off, and have the > > platforms that use this set uclinux_ram_map up themselves, it's already > > a global. Of course you can use _ebss as a default value for > > uclinux_ram_map.phys and just override it in your platform. > > i would agree if it were a board-specific issue, but it's an arch > issue, so pushing it to the boards level is wrong. i can however > replace the addr with a global weak and add a symbol into the Blackfin > arch code to override it. > I obviously meant architectures setting up the address, not the board code, as this has nothing at all to do with boards. There are already plenty of cases in setup_arch() for filling in uclinux mtd data, one more isn't going to make a difference. I don't see anything wrong with keeping uclinux_ram_map as a global however, particularly since platforms that need to special case the mapping can easily do this under the existing ifdef. Adding weak symbols for something like this just seems silly.
On Tue, May 26, 2009 at 12:47, Paul Mundt wrote: > On Tue, May 26, 2009 at 12:42:48PM -0400, Mike Frysinger wrote: >> On Tue, May 26, 2009 at 07:31, Paul Mundt wrote: >> > On Tue, May 26, 2009 at 05:46:31AM -0400, Mike Frysinger wrote: >> >> diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c >> >> index 57699c2..dcb552f 100644 >> >> --- a/drivers/mtd/maps/uclinux.c >> >> +++ b/drivers/mtd/maps/uclinux.c >> >> @@ -55,8 +55,13 @@ static int __init uclinux_mtd_init(void) >> >> ??{ >> >> ?? ?? ?? struct mtd_info *mtd; >> >> ?? ?? ?? struct map_info *mapp; >> >> +#ifdef CONFIG_BLACKFIN >> >> + ?? ?? extern unsigned long memory_mtd_start; >> >> + ?? ?? unsigned long addr = (unsigned long) memory_mtd_start; >> >> +#else >> >> ?? ?? ?? extern char _ebss; >> >> ?? ?? ?? unsigned long addr = (unsigned long) &_ebss; >> >> +#endif >> >> >> >> ?? ?? ?? mapp = &uclinux_ram_map; >> >> ?? ?? ?? mapp->phys = addr; >> > >> > NAK. >> > >> > I know there's no accounting for taste, but it would be nice to at least >> > see some minimal amount of effort going in to fixing these things >> > sanely rather than just lazily shoving ifdefs in wherever possible. >> > >> > In this case you should just kill all of that crap off, and have the >> > platforms that use this set uclinux_ram_map up themselves, it's already >> > a global. Of course you can use _ebss as a default value for >> > uclinux_ram_map.phys and just override it in your platform. >> >> i would agree if it were a board-specific issue, but it's an arch >> issue, so pushing it to the boards level is wrong. i can however >> replace the addr with a global weak and add a symbol into the Blackfin >> arch code to override it. >> > I obviously meant architectures setting up the address, not the board > code, as this has nothing at all to do with boards. There are already > plenty of cases in setup_arch() for filling in uclinux mtd data, one more > isn't going to make a difference. > > I don't see anything wrong with keeping uclinux_ram_map as a global > however, particularly since platforms that need to special case the > mapping can easily do this under the existing ifdef. Adding weak symbols > for something like this just seems silly. the point of the weak symbol was so that the map could provide a sane default that works for most everyone out there without having to copy & paste the same code to every arch, and to make new arch porters worry about what needs to be done to use this very trivial map -mike
diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c index 57699c2..dcb552f 100644 --- a/drivers/mtd/maps/uclinux.c +++ b/drivers/mtd/maps/uclinux.c @@ -55,8 +55,13 @@ static int __init uclinux_mtd_init(void) { struct mtd_info *mtd; struct map_info *mapp; +#ifdef CONFIG_BLACKFIN + extern unsigned long memory_mtd_start; + unsigned long addr = (unsigned long) memory_mtd_start; +#else extern char _ebss; unsigned long addr = (unsigned long) &_ebss; +#endif mapp = &uclinux_ram_map; mapp->phys = addr;
Due to a processor anomaly (05000263 to be exact), most Blackfin parts cannot keep the embedded filesystem image directly after the kernel in RAM. Instead, the filesystem needs to be relocated to the end of memory. As such, we need to tweak the initial filesystem address for Blackfin systems. Signed-off-by: Mike Frysinger <vapier@gentoo.org> CC: Greg Ungerer <gerg@uclinux.org> CC: uclinux-dev@uclinux.org CC: linux-mtd@lists.infradead.org --- drivers/mtd/maps/uclinux.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)