| Message ID | 1395257399-359545-11-git-send-email-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
Hello Arnd, On Wed, Mar 19, 2014 at 08:29:07PM +0100, Arnd Bergmann wrote: > The efm32 platform does not provide a zreladdr-y line its Makefile.boot, > so we always have to use CONFIG_AUTO_ZRELADDR in order to successfully > build and link a zImage. I wonder why you need to AUTO_ZRELADDR (which is there to guess zreladdr) while efm32 doesn't have an MMU and so there is nothing to guess. So I think this patch fixes a build problem, but it's not a good change. Best regards Uwe > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Cc: Uwe Kleine-König <kernel@pengutronix.de> > --- > arch/arm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 62ede9b..5776c12 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -422,6 +422,7 @@ config ARCH_EFM32 > bool "Energy Micro efm32" > depends on !MMU > select ARCH_REQUIRE_GPIOLIB > + select AUTO_ZRELADDR > select ARM_NVIC > select CLKSRC_OF > select COMMON_CLK > -- > 1.8.3.2 > >
On Thursday 20 March 2014 21:48:47 Uwe Kleine-König wrote: > > On Wed, Mar 19, 2014 at 08:29:07PM +0100, Arnd Bergmann wrote: > > The efm32 platform does not provide a zreladdr-y line its Makefile.boot, > > so we always have to use CONFIG_AUTO_ZRELADDR in order to successfully > > build and link a zImage. > I wonder why you need to AUTO_ZRELADDR (which is there to guess > zreladdr) while efm32 doesn't have an MMU and so there is nothing to > guess. So I think this patch fixes a build problem, but it's not a good > change. It is required in order to build a compressed zImage file. You mentioned before that your system does not have enough RAM to support this, but the compile-time option exists, and there is no dependency on MMU support for it, nor should there be. Arnd
Hello Arnd, On Thu, Mar 20, 2014 at 11:16:20PM +0100, Arnd Bergmann wrote: > On Thursday 20 March 2014 21:48:47 Uwe Kleine-König wrote: > > > > On Wed, Mar 19, 2014 at 08:29:07PM +0100, Arnd Bergmann wrote: > > > The efm32 platform does not provide a zreladdr-y line its Makefile.boot, > > > so we always have to use CONFIG_AUTO_ZRELADDR in order to successfully > > > build and link a zImage. > > I wonder why you need to AUTO_ZRELADDR (which is there to guess > > zreladdr) while efm32 doesn't have an MMU and so there is nothing to > > guess. So I think this patch fixes a build problem, but it's not a good > > change. > > It is required in order to build a compressed zImage file. You mentioned > before that your system does not have enough RAM to support this, but > the compile-time option exists, and there is no dependency on MMU support > for it, nor should there be. My objection isn't about having only little RAM. AUTO_ZRELADDR is about guessing the physical address corresponding to PAGE_OFFSET. But without an MMU there is nothing to guess. So I wonder if the better change would be to do: #ifdef CONFIG_MMU #ifdef CONFIG_AUTO_ZRELADDR ... guess zreladdress based on instruction pointer #else ... use zreladdr from Makefile.boot #endif #else ... use zreladdr = PAGE_OFFSET #endif Best regards Uwe
On Friday 21 March 2014 15:32:24 Uwe Kleine-König wrote: > On Thu, Mar 20, 2014 at 11:16:20PM +0100, Arnd Bergmann wrote: > > On Thursday 20 March 2014 21:48:47 Uwe Kleine-König wrote: > > > > > > On Wed, Mar 19, 2014 at 08:29:07PM +0100, Arnd Bergmann wrote: > > > > The efm32 platform does not provide a zreladdr-y line its Makefile.boot, > > > > so we always have to use CONFIG_AUTO_ZRELADDR in order to successfully > > > > build and link a zImage. > > > I wonder why you need to AUTO_ZRELADDR (which is there to guess > > > zreladdr) while efm32 doesn't have an MMU and so there is nothing to > > > guess. So I think this patch fixes a build problem, but it's not a good > > > change. > > > > It is required in order to build a compressed zImage file. You mentioned > > before that your system does not have enough RAM to support this, but > > the compile-time option exists, and there is no dependency on MMU support > > for it, nor should there be. > My objection isn't about having only little RAM. AUTO_ZRELADDR is about > guessing the physical address corresponding to PAGE_OFFSET. But without > an MMU there is nothing to guess. So I wonder if the better change would > be to do: > > #ifdef CONFIG_MMU > #ifdef CONFIG_AUTO_ZRELADDR > ... guess zreladdress based on instruction pointer > #else > ... use zreladdr from Makefile.boot > #endif > #else > ... use zreladdr = PAGE_OFFSET > #endif > I don't see a reason to change the existing logic, it works for both MMU and NOMMU kernels, and you are talking about *three* instructions here. Also, once we integrate efm32 into ARCH_MULTIPLATFORM, it AUTO_ZRELADDR will be set anyway, unless we add another useless conditional. Arnd
Hello Arnd, On Fri, Mar 21, 2014 at 04:10:37PM +0100, Arnd Bergmann wrote: > On Friday 21 March 2014 15:32:24 Uwe Kleine-König wrote: > > On Thu, Mar 20, 2014 at 11:16:20PM +0100, Arnd Bergmann wrote: > > > On Thursday 20 March 2014 21:48:47 Uwe Kleine-König wrote: > > > > > > > > On Wed, Mar 19, 2014 at 08:29:07PM +0100, Arnd Bergmann wrote: > > > > > The efm32 platform does not provide a zreladdr-y line its Makefile.boot, > > > > > so we always have to use CONFIG_AUTO_ZRELADDR in order to successfully > > > > > build and link a zImage. > > > > I wonder why you need to AUTO_ZRELADDR (which is there to guess > > > > zreladdr) while efm32 doesn't have an MMU and so there is nothing to > > > > guess. So I think this patch fixes a build problem, but it's not a good > > > > change. > > > > > > It is required in order to build a compressed zImage file. You mentioned > > > before that your system does not have enough RAM to support this, but > > > the compile-time option exists, and there is no dependency on MMU support > > > for it, nor should there be. > > My objection isn't about having only little RAM. AUTO_ZRELADDR is about > > guessing the physical address corresponding to PAGE_OFFSET. But without > > an MMU there is nothing to guess. So I wonder if the better change would > > be to do: > > > > #ifdef CONFIG_MMU > > #ifdef CONFIG_AUTO_ZRELADDR > > ... guess zreladdress based on instruction pointer > > #else > > ... use zreladdr from Makefile.boot > > #endif > > #else > > ... use zreladdr = PAGE_OFFSET > > #endif > > > > I don't see a reason to change the existing logic, it works for > both MMU and NOMMU kernels, and you are talking about *three* instructions > here. it doesn't matter how many instructions are involved. The relevant difference is that with my approach you fix the problem for all no-MMU platforms, with yours you only fix efm32. (OK, I think there are not too many no-MMU platforms, but still.) An even easier implementation would be to add something like: ifeq($(zreladdr-y)$(CONFIG_MMU),) zreladdr-y := CONFIG_PAGE_OFFSET + CONFIG_TEXT_OFFSET endif to arch/arm/boot/Makefile. But I don't care much, if you still want to make EFM32 select AUTO_ZRELADDR go ahead. Best regards Uwe
On Fri, Mar 21, 2014 at 1:54 PM, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > Hello Arnd, > > On Fri, Mar 21, 2014 at 04:10:37PM +0100, Arnd Bergmann wrote: >> On Friday 21 March 2014 15:32:24 Uwe Kleine-König wrote: >> > On Thu, Mar 20, 2014 at 11:16:20PM +0100, Arnd Bergmann wrote: >> > > On Thursday 20 March 2014 21:48:47 Uwe Kleine-König wrote: >> > > > >> > > > On Wed, Mar 19, 2014 at 08:29:07PM +0100, Arnd Bergmann wrote: >> > > > > The efm32 platform does not provide a zreladdr-y line its Makefile.boot, >> > > > > so we always have to use CONFIG_AUTO_ZRELADDR in order to successfully >> > > > > build and link a zImage. >> > > > I wonder why you need to AUTO_ZRELADDR (which is there to guess >> > > > zreladdr) while efm32 doesn't have an MMU and so there is nothing to >> > > > guess. So I think this patch fixes a build problem, but it's not a good >> > > > change. >> > > >> > > It is required in order to build a compressed zImage file. You mentioned >> > > before that your system does not have enough RAM to support this, but >> > > the compile-time option exists, and there is no dependency on MMU support >> > > for it, nor should there be. >> > My objection isn't about having only little RAM. AUTO_ZRELADDR is about >> > guessing the physical address corresponding to PAGE_OFFSET. But without >> > an MMU there is nothing to guess. So I wonder if the better change would >> > be to do: >> > >> > #ifdef CONFIG_MMU >> > #ifdef CONFIG_AUTO_ZRELADDR >> > ... guess zreladdress based on instruction pointer >> > #else >> > ... use zreladdr from Makefile.boot >> > #endif >> > #else >> > ... use zreladdr = PAGE_OFFSET >> > #endif >> > >> >> I don't see a reason to change the existing logic, it works for >> both MMU and NOMMU kernels, and you are talking about *three* instructions >> here. > it doesn't matter how many instructions are involved. The relevant > difference is that with my approach you fix the problem for all no-MMU > platforms, with yours you only fix efm32. (OK, I think there are not too > many no-MMU platforms, but still.) An even easier implementation would > be to add something like: > > ifeq($(zreladdr-y)$(CONFIG_MMU),) > zreladdr-y := CONFIG_PAGE_OFFSET + CONFIG_TEXT_OFFSET > endif > > to arch/arm/boot/Makefile. > > But I don't care much, if you still want to make EFM32 select > AUTO_ZRELADDR go ahead. How about a kconfig fix: bool "Auto calculation of the decompressed kernel image address" if MMU default y if !MMU Rob
On Friday 21 March 2014, Rob Herring wrote: > On Fri, Mar 21, 2014 at 1:54 PM, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > On Fri, Mar 21, 2014 at 04:10:37PM +0100, Arnd Bergmann wrote: > >> I don't see a reason to change the existing logic, it works for > >> both MMU and NOMMU kernels, and you are talking about three instructions > >> here. > > it doesn't matter how many instructions are involved. The relevant > > difference is that with my approach you fix the problem for all no-MMU > > platforms, with yours you only fix efm32. (OK, I think there are not too > > many no-MMU platforms, but still.) An even easier implementation would > > be to add something like: > > > > ifeq($(zreladdr-y)$(CONFIG_MMU),) > > zreladdr-y := CONFIG_PAGE_OFFSET + CONFIG_TEXT_OFFSET > > endif > > > > to arch/arm/boot/Makefile. This makes sense, we can add it as soon as we have a use for it. At the moment, the only problem we have is a randconfig build error, and the obvious change I proposed should work just fine. > > But I don't care much, if you still want to make EFM32 select > > AUTO_ZRELADDR go ahead. > > How about a kconfig fix: > > bool "Auto calculation of the decompressed kernel image address" if MMU > default y if !MMU This however would break builds that today for some reason cannot use AUTO_ZRELADDR on !MMU kernels, i.e. has a funny bootloader that uses ZBOOT_ROM or other tricks. In principle any platform should work with MMU disabled (although I'm sure there are tons of bugs), and not every one uses AUTO_ZRELADDR today. Uwe's suggestion above takes advantage of the fact that !MMU-kernels know what physical address they run on. Arnd
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 62ede9b..5776c12 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -422,6 +422,7 @@ config ARCH_EFM32 bool "Energy Micro efm32" depends on !MMU select ARCH_REQUIRE_GPIOLIB + select AUTO_ZRELADDR select ARM_NVIC select CLKSRC_OF select COMMON_CLK
The efm32 platform does not provide a zreladdr-y line its Makefile.boot, so we always have to use CONFIG_AUTO_ZRELADDR in order to successfully build and link a zImage. Signed-off-by: Arnd Bergmann <arnd@arndb.de> Cc: Uwe Kleine-König <kernel@pengutronix.de> --- arch/arm/Kconfig | 1 + 1 file changed, 1 insertion(+)