| Message ID | 1320932982-19967-2-git-send-email-loic.minier@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 11/10/2011 02:49 PM, Loïc Minier wrote: > Signed-off-by: Loïc Minier <loic.minier@linaro.org> > --- > Makefile | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/Makefile b/Makefile > index 294c762..0ccc3bf 100644 > --- a/Makefile > +++ b/Makefile > @@ -365,6 +365,10 @@ ALL-$(CONFIG_MMC_U_BOOT) += $(obj)mmc_spl/u-boot-mmc-spl.bin > ALL-$(CONFIG_SPL) += $(obj)spl/u-boot-spl.bin > ALL-$(CONFIG_OF_SEPARATE) += $(obj)u-boot.dtb $(obj)u-boot-dtb.bin > > +ifneq ($(CONFIG_IMX_CONFIG),) > +ALL-y += $(obj)u-boot.imx > +endif > + > all: $(ALL-y) $(SUBDIR_EXAMPLES) > > $(obj)u-boot.dtb: $(obj)u-boot Acked-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
On Thursday 10 November 2011 08:49:41 Loïc Minier wrote: > --- a/Makefile > +++ b/Makefile > > +ifneq ($(CONFIG_IMX_CONFIG),) > +ALL-y += $(obj)u-boot.imx > +endif why won't this work: ALL-$(CONFIG_IMX_CONFIG) += $(obj)u-boot.mix that was the point of naming it "ALL-y" in the first place ... -mike
On Thu, Nov 10, 2011, Mike Frysinger wrote: > > +ifneq ($(CONFIG_IMX_CONFIG),) > > +ALL-y += $(obj)u-boot.imx > > +endif > why won't this work: > ALL-$(CONFIG_IMX_CONFIG) += $(obj)u-boot.mix > that was the point of naming it "ALL-y" in the first place ... That's what I tried at first, but CONFIG_IMX_CONFIG is actually the name of the config file to pass to mkimage; same for u-boot.kwb. CONFIG_IMX_CONFIG is set in boards.cfg.
On Thursday 10 November 2011 18:45:30 Loïc Minier wrote: > On Thu, Nov 10, 2011, Mike Frysinger wrote: > > > +ifneq ($(CONFIG_IMX_CONFIG),) > > > +ALL-y += $(obj)u-boot.imx > > > +endif > > > > why won't this work: > > ALL-$(CONFIG_IMX_CONFIG) += $(obj)u-boot.mix > > that was the point of naming it "ALL-y" in the first place ... > > That's what I tried at first, but CONFIG_IMX_CONFIG is actually the > name of the config file to pass to mkimage; same for u-boot.kwb. > > CONFIG_IMX_CONFIG is set in boards.cfg. ugly undocumented CONFIG's are awesome this really should be in a subdir rather than the top level. we want to keep arch/soc-specific cruft out of the top level Makefile if possible. isn't there a sub-config.mk somewhere you could add the line unconditionally ? -mike
On 11.11.2011 00:45, Loïc Minier wrote: > On Thu, Nov 10, 2011, Mike Frysinger wrote: >>> +ifneq ($(CONFIG_IMX_CONFIG),) >>> +ALL-y += $(obj)u-boot.imx >>> +endif >> why won't this work: >> ALL-$(CONFIG_IMX_CONFIG) += $(obj)u-boot.mix >> that was the point of naming it "ALL-y" in the first place ... > > That's what I tried at first, but CONFIG_IMX_CONFIG is actually the > name of the config file to pass to mkimage; same for u-boot.kwb. > > CONFIG_IMX_CONFIG is set in boards.cfg. > why not ALL-$(CONFIG_UBOOT_IMG) += $(obj)u-boot.img ALL-$(CONFIG_UBOOT_IMX) += $(obj)u-boot.imx ALL-$(CONFIG_UBOOT_KWB) += $(obj)u-boot.kwb then you can define CONFIG_UBOOT_KWB or CONFIG_UBOOT_IMX in your board config if needed Best regards, Daniel
On Friday 11 November 2011 07:21 AM, Mike Frysinger wrote: > On Thursday 10 November 2011 18:45:30 Loïc Minier wrote: >> On Thu, Nov 10, 2011, Mike Frysinger wrote: >>>> +ifneq ($(CONFIG_IMX_CONFIG),) >>>> +ALL-y += $(obj)u-boot.imx >>>> +endif >>> >>> why won't this work: >>> ALL-$(CONFIG_IMX_CONFIG) += $(obj)u-boot.mix >>> that was the point of naming it "ALL-y" in the first place ... >> >> That's what I tried at first, but CONFIG_IMX_CONFIG is actually the >> name of the config file to pass to mkimage; same for u-boot.kwb. >> >> CONFIG_IMX_CONFIG is set in boards.cfg. > > ugly undocumented CONFIG's are awesome > > this really should be in a subdir rather than the top level. we want to keep > arch/soc-specific cruft out of the top level Makefile if possible. isn't there > a sub-config.mk somewhere you could add the line unconditionally ? This is what I have done for u-boot.img for OMAP4. arch/arm/cpu/armv7/omap4/config.mk has this: ifdef CONFIG_SPL_BUILD ALL-y += $(OBJTREE)/MLO else ALL-y += $(obj)u-boot.img endif However, this may have to be duplicated in many such config.mk files. br, Aneesh
On Fri, Nov 11, 2011, Daniel Schwierzeck wrote: > ALL-$(CONFIG_UBOOT_IMG) += $(obj)u-boot.img > ALL-$(CONFIG_UBOOT_IMX) += $(obj)u-boot.imx > ALL-$(CONFIG_UBOOT_KWB) += $(obj)u-boot.kwb > > then you can define CONFIG_UBOOT_KWB or CONFIG_UBOOT_IMX in your > board config if needed That'd be a fine solution for me if others agree it's the right approach; ideally, we would even set CONFIG_UBOOT_IMX when CONFIG_IMX_CONFIG is set (not sure where though).
On Fri, Nov 11, 2011, Aneesh V wrote: > arch/arm/cpu/armv7/omap4/config.mk has this: > > ifdef CONFIG_SPL_BUILD > ALL-y += $(OBJTREE)/MLO > else > ALL-y += $(obj)u-boot.img > endif > > However, this may have to be duplicated in many such config.mk files. Ah I had seen this snippet and wondered why you hadn't put it in spl/Makefile, but now I realize that only the MLO part could go there, and that you don't want to built u-boot.img when building a spl. I don't really mind whether we set these things in some specific .mk files or in Makefile + spl/Makefile; I care that it's consistent and that related things are kept together as to update them together. I guess we can follow the OMAP approach for other boards.
On Thu, Nov 10, 2011, Mike Frysinger wrote: > > CONFIG_IMX_CONFIG is set in boards.cfg. > > ugly undocumented CONFIG's are awesome > > this really should be in a subdir rather than the top level. we want > to keep arch/soc-specific cruft out of the top level Makefile if > possible. isn't there a sub-config.mk somewhere you could add the > line unconditionally ? Can't see one, but Aneesh proposes the OMAP approach of arch/arm/cpu/**/config.mk files. The CONFIG_IMX_CONFIG actually changes per-board, so the boards.cfg approach to set it seems ok; however we could try defining the u-boot.imx rule in a common i.MX makefile snippet somewhere. Perhaps arch/arm/cpu/armv7/config.mk or arch/arm/cpu/armv7/mx5/Makefile, or a new file; I don't have a strong opinion.
On Friday 11 November 2011 08:35:14 Loïc Minier wrote: > On Thu, Nov 10, 2011, Mike Frysinger wrote: > > > CONFIG_IMX_CONFIG is set in boards.cfg. > > > > ugly undocumented CONFIG's are awesome > > > > this really should be in a subdir rather than the top level. we want > > to keep arch/soc-specific cruft out of the top level Makefile if > > possible. isn't there a sub-config.mk somewhere you could add the > > line unconditionally ? > > Can't see one, but Aneesh proposes the OMAP approach of > arch/arm/cpu/**/config.mk files. > > The CONFIG_IMX_CONFIG actually changes per-board, so the boards.cfg > approach to set it seems ok; however we could try defining the > u-boot.imx rule in a common i.MX makefile snippet somewhere. > > Perhaps arch/arm/cpu/armv7/config.mk or > arch/arm/cpu/armv7/mx5/Makefile, or a new file; I don't have a strong > opinion. if there's a config option to key off of, and it affects multiple arm arches/socs, then arch/arm/config.mk would probably be the best place to add the lines. maybe Albert has an opinion on the arch/arm/ layout. -mike
On 11/11/2011 02:35 PM, Loïc Minier wrote: > On Thu, Nov 10, 2011, Mike Frysinger wrote: >>> CONFIG_IMX_CONFIG is set in boards.cfg. >> >> ugly undocumented CONFIG's are awesome >> >> this really should be in a subdir rather than the top level. we want >> to keep arch/soc-specific cruft out of the top level Makefile if >> possible. isn't there a sub-config.mk somewhere you could add the >> line unconditionally ? > > Can't see one, but Aneesh proposes the OMAP approach of > arch/arm/cpu/**/config.mk files. > > The CONFIG_IMX_CONFIG actually changes per-board, so the boards.cfg > approach to set it seems ok; however we could try defining the > u-boot.imx rule in a common i.MX makefile snippet somewhere. > > Perhaps arch/arm/cpu/armv7/config.mk or > arch/arm/cpu/armv7/mx5/Makefile, or a new file; I don't have a strong > opinion. Then maybe in arch/arm/cpu/armv7/config.mk - the coming MX6 SOC will use CONFIG_IMX_CONFIG, too. Stefano
Dear =?iso-8859-1?Q?Lo=EFc?= Minier, In message <20111111133514.GC2895@bee.dooz.org> you wrote: > > The CONFIG_IMX_CONFIG actually changes per-board, so the boards.cfg > approach to set it seems ok; however we could try defining the No. The ability to pass additional parameters in boards.cfg was primarily needed to be able to remove existing board support from the top level Makefile, which often used lots of such stuff. New boards, or changes to existing boards, should only make minimal use of this, or even better help to reduce that. Best regards, Wolfgang Denk
On 11/11/2011 14:26, Loïc Minier wrote: > On Fri, Nov 11, 2011, Daniel Schwierzeck wrote: >> ALL-$(CONFIG_UBOOT_IMG) += $(obj)u-boot.img >> ALL-$(CONFIG_UBOOT_IMX) += $(obj)u-boot.imx >> ALL-$(CONFIG_UBOOT_KWB) += $(obj)u-boot.kwb >> >> then you can define CONFIG_UBOOT_KWB or CONFIG_UBOOT_IMX in your >> board config if needed > > That'd be a fine solution for me if others agree it's the right > approach; ideally, we would even set CONFIG_UBOOT_IMX when > CONFIG_IMX_CONFIG is set (not sure where though). Sorry to bring a dead thread to new life... The issue is not yet solved and I am asking if we can do something for the incoming release 2012.04. I would prefer we do not add a new switch CONFIG_UBOOT_IMX into the configuration file, because it is possible to have the same u-boot image running on different storages, for example on NAND or NOR, and they have different offsets. This means different imximage (header), while u-boot itself (binary) remains the same. So this information should be not interpreted by the compiler. Rereading the thread a major point is that we must not change the main Makefile. What about if the changes suggested in the patch are put into arch/arm/cpu/armv7/config.mk ? Best regards, Stefano Babic
On Mon, Apr 2, 2012 at 13:03, Stefano Babic wrote: > On 11/11/2011 14:26, Loďc Minier wrote: >> On Fri, Nov 11, 2011, Daniel Schwierzeck wrote: >>> ALL-$(CONFIG_UBOOT_IMG) += $(obj)u-boot.img >>> ALL-$(CONFIG_UBOOT_IMX) += $(obj)u-boot.imx >>> ALL-$(CONFIG_UBOOT_KWB) += $(obj)u-boot.kwb >>> >>> then you can define CONFIG_UBOOT_KWB or CONFIG_UBOOT_IMX in your >>> board config if needed >> >> That'd be a fine solution for me if others agree it's the right >> approach; ideally, we would even set CONFIG_UBOOT_IMX when >> CONFIG_IMX_CONFIG is set (not sure where though). > > Sorry to bring a dead thread to new life... > > The issue is not yet solved and I am asking if we can do something for > the incoming release 2012.04. I would prefer we do not add a new switch > CONFIG_UBOOT_IMX into the configuration file, because it is possible to > have the same u-boot image running on different storages, for example on > NAND or NOR, and they have different offsets. This means different > imximage (header), while u-boot itself (binary) remains the same. So > this information should be not interpreted by the compiler. i'm not sure i follow. it's been a while since this thread came through, but it seemed like you could handle this (defining the various CONFIG_xxx stuff) in arch/soc asm/config.h files ? > Rereading the thread a major point is that we must not change the main > Makefile. What about if the changes suggested in the patch are put into > arch/arm/cpu/armv7/config.mk ? the suggested ALL-$(CONFIG_xxx) syntax should be usable in the arch/soc config.mk files -mike
On 02/04/2012 20:25, Mike Frysinger wrote: > > the suggested ALL-$(CONFIG_xxx) syntax should be usable in the > arch/soc config.mk files Right. I push a new version ;-) Stefano
diff --git a/Makefile b/Makefile index 294c762..0ccc3bf 100644 --- a/Makefile +++ b/Makefile @@ -365,6 +365,10 @@ ALL-$(CONFIG_MMC_U_BOOT) += $(obj)mmc_spl/u-boot-mmc-spl.bin ALL-$(CONFIG_SPL) += $(obj)spl/u-boot-spl.bin ALL-$(CONFIG_OF_SEPARATE) += $(obj)u-boot.dtb $(obj)u-boot-dtb.bin +ifneq ($(CONFIG_IMX_CONFIG),) +ALL-y += $(obj)u-boot.imx +endif + all: $(ALL-y) $(SUBDIR_EXAMPLES) $(obj)u-boot.dtb: $(obj)u-boot
Signed-off-by: Loïc Minier <loic.minier@linaro.org> --- Makefile | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)