Message ID | 1352156642-7975-1-git-send-email-swarren@wwwdotorg.org |
---|---|
State | Superseded |
Headers | show |
Hi Stephen, On Mon, Nov 5, 2012 at 3:04 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > 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. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> Acked-by: Simon Glass <sjg@chromium.org> This seems like a problem that might come up in other areas. I wonder if SPL should have its own autoconf.mk? > --- > This series is based on u-boot/master. I've CC'd the MMC and Tegra > maintainers since they'll presumably need to ack the changes in order for > these patches to all be applied in one place. Marek seems to be the main > MXS MMC maintainer as far as I can tell. > > common/bouncebuf.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/common/bouncebuf.c b/common/bouncebuf.c > index 4f827f8..ffd3c90 100644 > --- a/common/bouncebuf.c > +++ b/common/bouncebuf.c > @@ -27,6 +27,7 @@ > #include <errno.h> > #include <bouncebuf.h> > > +#ifdef CONFIG_BOUNCE_BUFFER > static int addr_aligned(void *data, size_t len) > { > const ulong align_mask = ARCH_DMA_MINALIGN - 1; > @@ -90,3 +91,4 @@ int bounce_buffer_stop(void **data, size_t len, void **backup, uint8_t flags) > > return 0; > } > +#endif > -- > 1.7.0.4 > Regards, Simon
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 ... > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > This series is based on u-boot/master. I've CC'd the MMC and Tegra > maintainers since they'll presumably need to ack the changes in order for > these patches to all be applied in one place. Marek seems to be the main > MXS MMC maintainer as far as I can tell. What did I get myself into ... ;-) > common/bouncebuf.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/common/bouncebuf.c b/common/bouncebuf.c > index 4f827f8..ffd3c90 100644 > --- a/common/bouncebuf.c > +++ b/common/bouncebuf.c > @@ -27,6 +27,7 @@ > #include <errno.h> > #include <bouncebuf.h> > > +#ifdef CONFIG_BOUNCE_BUFFER > static int addr_aligned(void *data, size_t len) > { > const ulong align_mask = ARCH_DMA_MINALIGN - 1; > @@ -90,3 +91,4 @@ int bounce_buffer_stop(void **data, size_t len, void > **backup, uint8_t flags) > > return 0; > } > +#endif Best regards, Marek Vasut
On 11/05/2012 04:47 PM, Simon Glass wrote: > Hi Stephen, > > On Mon, Nov 5, 2012 at 3:04 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> 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. >> >> Signed-off-by: Stephen Warren <swarren@nvidia.com> > > Acked-by: Simon Glass <sjg@chromium.org> > > This seems like a problem that might come up in other areas. I wonder > if SPL should have its own autoconf.mk? That might be a good idea. Is the config.h separate for SPL-vs-non-SPL? Perhaps it doesn't need to be because it's simply always evaluated at each individual compile time, whereas perhaps autoconf.mk is generated once rather than evaluated? As you can tell, I have not looked into this or most aspects of U-Boot's build system, so I have no idea how feasible fixing the build system for this would be.
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?
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 ? Best regards, Marek Vasut
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
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? Best regards, Marek Vasut
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?
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? Best regards,
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.
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!
diff --git a/common/bouncebuf.c b/common/bouncebuf.c index 4f827f8..ffd3c90 100644 --- a/common/bouncebuf.c +++ b/common/bouncebuf.c @@ -27,6 +27,7 @@ #include <errno.h> #include <bouncebuf.h> +#ifdef CONFIG_BOUNCE_BUFFER static int addr_aligned(void *data, size_t len) { const ulong align_mask = ARCH_DMA_MINALIGN - 1; @@ -90,3 +91,4 @@ int bounce_buffer_stop(void **data, size_t len, void **backup, uint8_t flags) return 0; } +#endif