From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sughosh Ganu Date: Mon, 27 Feb 2012 17:32:53 +0530 Subject: [U-Boot] [PATCH v4 07/13] davinci: Use correct #ifdef around gdata/bdata In-Reply-To: References: <1329787975-6695-1-git-send-email-sjg@chromium.org> <1329787975-6695-8-git-send-email-sjg@chromium.org> <20120223172555.GA15287@Hardy> <20120227101613.GA2183@Hardy> <20120227105628.GA2991@Hardy> Message-ID: <20120227120253.GA3066@Hardy> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de hi Christian, On Mon Feb 27, 2012 at 12:37:16PM +0100, Christian Riesch wrote: > Hi Sughosh, > > On Mon, Feb 27, 2012 at 11:56 AM, Sughosh Ganu wrote: > > hi Christian, > > > > On Mon Feb 27, 2012 at 11:39:42AM +0100, Christian Riesch wrote: > >> Hi, > >> > >> On Mon, Feb 27, 2012 at 11:16 AM, Sughosh Ganu wrote: > > > > > > > >> >> >> ?arch/arm/cpu/arm926ejs/davinci/spl.c | ? ?2 ++ > >> >> >> ?1 files changed, 2 insertions(+), 0 deletions(-) > >> >> >> > >> >> >> diff --git a/arch/arm/cpu/arm926ejs/davinci/spl.c b/arch/arm/cpu/arm926ejs/davinci/spl.c > >> >> >> index b1eff26..2861907 100644 > >> >> >> --- a/arch/arm/cpu/arm926ejs/davinci/spl.c > >> >> >> +++ b/arch/arm/cpu/arm926ejs/davinci/spl.c > >> >> >> @@ -32,10 +32,12 @@ > >> >> >> > >> >> >> ?#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT > >> >> >> > >> >> >> +#ifdef CONFIG_SPL_SPI_LOAD > >> >> >> ?DECLARE_GLOBAL_DATA_PTR; > >> >> >> ?/* Define global data structure pointer to it*/ > >> >> >> ?static gd_t gdata __attribute__ ((section(".data"))); > >> >> >> ?static bd_t bdata __attribute__ ((section(".data"))); > >> >> >> +#endif > > > > > > > >> >> > ?Can you specify which boards you get this warning for. With your > >> >> > ?patch to add libcommon to hawkboard's spl image, this is now also > >> >> > ?needed for hawkboard which uses CONFIG_SPL_NAND_LOAD. > >> > >> Simon's patch is for the hawkboard, since due to another patch in his > >> patchset LIBCOMMON is enabled in hawkboard's SPL. Now we have a board > >> that boots from NAND with SPL and has LIBCOMMON enabled (Simon, I did > >> not check the rest of your patchset, why do you need LIBCOMMON on the > >> hawkboard at all?) > > > > ?LIBCOMMON is now needed as the generic relocation based functions > > ?are part of the libcommon.o, which are being enabled in the same > > ?patchset for all arm boards. So if i understand correct, all arm > > ?board based spl's now need libcommon and libgeneric. > > > > ?The only thing i see is that libcommon and libgeneric are not > > ?defined for cam_enc_4xx board which uses spl, and this patchset does > > ?not add it either. Not sure whether it got missed. > > When I asked Heiko Schocher a few month ago why he defined putc and > puts in arch/arm/cpu/arm926ejs/davinci/spl.c he replied that he could > not use LIBCOMMON due to size limitations for the SPL. So I guess that > this board will not be able to use the generic relocation functions, > unless the SPL is smaller than 16kB, right? Simon's patchset will > break this board then, right? That is exactly what i reported in one of the threads in response to addition of libcommon and libgeneric to the hawkboard's spl. In fact, this might cause problems on quite a few boards with spl size restrictions. I am not sure, whether the generic relocation feature should be turned on by default on all boards or should be a config option -- at least for the spl builds. Another option would be to move it to a place where it is not needed to compile in the entire libcommon/libgeneric support that is not needed for the generic relocation code. I think that would help us keep the generic relocation without the size bloat that we see right now. http://lists.denx.de/pipermail/u-boot/2012-February/118567.html > >> void board_init_r(gd_t *id, ulong dummy) > >> { > >> #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT > >> ? ? ? ? mem_malloc_init(CONFIG_SYS_TEXT_BASE - CONFIG_SYS_MALLOC_LEN, > >> ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_MALLOC_LEN); > > > > ?Can you please explain why we need the mem_malloc_init. I did not > > ?include this, and spl boots up just fine on my board. > > > > malloc is required for the SPI code only, so you could also put it > within #ifdef CONFIG_SPL_SPI_LOAD Ok, i will move the common changes to a static function, and call it from both the nand and spi load cases. Or, should i wait till a consensus is drawn on whether to enable this feature for spl images also. In case this is not needed for spl, then we don't need this change. -sughosh