public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/3] common: add ifdefs around bouncebuf.c body
Date: Wed, 07 Nov 2012 10:00:12 -0700	[thread overview]
Message-ID: <509A939C.3000906@wwwdotorg.org> (raw)
In-Reply-To: <201211071421.07941.marex@denx.de>

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.

  reply	other threads:[~2012-11-07 17:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-05 23:04 [U-Boot] [PATCH 1/3] common: add ifdefs around bouncebuf.c body Stephen Warren
2012-11-05 23:04 ` [U-Boot] [PATCH 2/3] common: rework bouncebuf implementation Stephen Warren
2012-11-05 23:54   ` Simon Glass
2012-11-06 18:44     ` Stephen Warren
2012-11-06 19:30     ` Stephen Warren
2012-11-05 23:04 ` [U-Boot] [PATCH 3/3] mmc: tegra: use bounce buffer APIs Stephen Warren
2012-11-06  0:00   ` Simon Glass
2012-11-06 18:50     ` Stephen Warren
2012-11-06 19:03       ` Simon Glass
2012-11-05 23:47 ` [U-Boot] [PATCH 1/3] common: add ifdefs around bouncebuf.c body Simon Glass
2012-11-06 18:04   ` Stephen Warren
2012-11-06  0:54 ` Marek Vasut
2012-11-06 18:07   ` Stephen Warren
2012-11-06 22:43     ` Marek Vasut
2012-11-06 22:49       ` Stephen Warren
2012-11-06 22:57         ` Marek Vasut
2012-11-06 23:13           ` Stephen Warren
2012-11-07 13:21             ` Marek Vasut
2012-11-07 17:00               ` Stephen Warren [this message]
2012-11-08  1:20                 ` Marek Vasut

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=509A939C.3000906@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox