[U-Boot] [PATCH 1/3] common: add ifdefs around bouncebuf.c body

[Marek Vasut <marex@denx.de>]

Dear Stephen Warren,

> On 11/07/2012 06:21 AM, Marek Vasut wrote:
> > Dear Stephen Warren,
> > 
> >> On 11/06/2012 03:57 PM, Marek Vasut wrote:
> >>> Dear Stephen Warren,
> >>> 
> >>>> On 11/06/2012 03:43 PM, Marek Vasut wrote:
> >>>>> Dear Stephen Warren,
> >>>>> 
> >>>>>> On 11/05/2012 05:54 PM, Marek Vasut wrote:
> >>>>>>> Dear Stephen Warren,
> >>>>>>> 
> >>>>>>>> From: Stephen Warren <swarren@nvidia.com>
> >>>>>>>> 
> >>>>>>>> If a U-Boot config file enables CONFIG_BOUNCE_BUFFER only for the
> >>>>>>>> main U-Boot build and not for the SPL, then config.mk will contain
> >>>>>>>> CONFIG_BOUNCE_BUFFER=y, so common/Makefile will build bouncebuf.c
> >>>>>>>> for both the SPL and main U-Boot, but config.h won't set
> >>>>>>>> CONFIG_BOUNCE_BUFFER for the SPL, so bouncebuf.h will provide
> >>>>>>>> static inline functions, which will conflict with the compiled
> >>>>>>>> bouncebuf.c. Solve this by guarding the body of bouncebuf.c with
> >>>>>>>> the ifdef to avoid conflicts.
> >>>>>>> 
> >>>>>>> Uh, don't you want the bounce buffer not compiled in for SPL? Then
> >>>>>>> maybe add CONFIG_SPL_BOUNCE_BUFFER to force BB to be compiled into
> >>>>>>> SPL or something ...
> >>>>>> 
> >>>>>> Not compiling bouncebuf.c for SPL would solve this too. I have no
> >>>>>> idea what build system contortions would be required to do this
> >>>>>> though. Do you think the build system should be fixed first rather
> >>>>>> than taking this series/patch?
> >>>>>> 
> >>>>>> I guess we shouldn't need a separate CONFIG_SPL_BOUNCE_BUFFER option
> >>>>>> though; we should rather simply set CONFIG_SPL_BOUNCE_BUFFER
> >>>>>> appropriately for SPL and non-SPL, and have everything key off that
> >>>>>> one variable, right?
> >>>>> 
> >>>>> How will you be able to configure it separately for spl and non-spl ?
> >>>> 
> >>>> For example,
> >>>> 
> >>>> include/configs/trimslice.h contains:
> >>>> 
> >>>> /* SD/MMC */
> >>>> #define CONFIG_MMC
> >>>> #define CONFIG_GENERIC_MMC
> >>>> #define CONFIG_TEGRA_MMC
> >>>> #define CONFIG_CMD_MMC
> >>>> 
> >>>> However, we don't use MMC in our SPL, but don't want to pollute every
> >>>> Tegra board file with ifdefs for SPL, so
> >>>> include/configs/tegra-common-post.h (which is included at the end of
> >>>> trimslice.h) contains:
> >>>> 
> >>>> #ifdef CONFIG_SPL_BUILD
> >>>> ...
> >>>> /* remove MMC support */
> >>>> #ifdef CONFIG_MMC
> >>>> #undef CONFIG_MMC
> >>>> #endif
> >>>> #ifdef CONFIG_GENERIC_MMC
> >>>> #undef CONFIG_GENERIC_MMC
> >>>> #endif
> >>>> #ifdef CONFIG_TEGRA_MMC
> >>>> #undef CONFIG_TEGRA_MMC
> >>>> #endif
> >>>> #ifdef CONFIG_CMD_MMC
> >>>> #undef CONFIG_CMD_MMC
> >>>> #endif
> >>>> ...
> >>>> #endif
> >>>> 
> >>>> And in the V1 patch I proposed to tegra-common-post.h, I added the
> >>>> following at the end:
> >>>> 
> >>>> #ifdef CONFIG_TEGRA_MMC
> >>>> #define CONFIG_BOUNCE_BUFFER
> >>>> #endif
> >>> 
> >>> Yet, this doesn't solve the problem with SPL ... since for SPL, you'd
> >>> have to do ifdef CONFIG_SPL, no?
> >> 
> >> Sorry, what problem with the SPL is this not solving?
> > 
> > I think I was tired when replying (sorry, the conference is really heavy
> > on me). I though you wanted to disable BB only for SPL, but now I see BB
> > being enabled depends on tegra mmc.
> > 
> > btw. shouldn't --gc-sections remove BB code if it's not used at all?
> 
> Yes, I assume so. In v2 of the patch series I have simply enabled
> CONFIG_BOUNCE_BUFFER unconditionally on Tegra, which removes the need
> for any SPL-specific changes.
> 
> Before enabling it:
> 
> Configuring for trimslice board...
>    text	   data	    bss	    dec	    hex	filename
>  245625	   9304	 274968	 529897	  815e9	./u-boot
>   14451	    208	     72	  14731	   398b	./spl/u-boot-spl
> 
> After enabling it:
> 
> Configuring for trimslice board...
>    text	   data	    bss	    dec	    hex	filename
>  245742	   9304	 274964	 530010	  8165a	./u-boot
>   14451	    208	     72	  14731	   398b	./spl/u-boot-spl
> 
> SPL didn't change since, so this seems to be working fine.

Good!