| Message ID | 1401992872-31985-2-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Simon, On Thu, 5 Jun 2014 12:27:49 -0600 Simon Glass <sjg@chromium.org> wrote: > This was lost sometime in the Kbuild conversion. Add it back. Not lost. It was moved to examples/Makefile. Prior to Kbuild conversion, config.mk was like this: ------------------>8---------------------- BCURDIR = $(subst $(SRCTREE)/,,$(CURDIR:$(obj)%=%)) ifeq ($(findstring examples/,$(BCURDIR)),) ifeq ($(CONFIG_SPL_BUILD),) ifdef FTRACE CFLAGS += -finstrument-functions -DFTRACE endif endif endif --------------------8<------------------------- "-finstrument-functions -DFTRACE" was enabled only under examples/ directory. (Do you remember why?) That's why I moved it to examples/Makefile to keep the equivalent behavior. > diff --git a/config.mk b/config.mk > index 05864aa..0c45c09 100644 > --- a/config.mk > +++ b/config.mk > @@ -46,6 +46,10 @@ ifdef BOARD > sinclude $(srctree)/board/$(BOARDDIR)/config.mk # include board specific rules > endif > > +ifdef FTRACE > +PLATFORM_CPPFLAGS += -finstrument-functions -DFTRACE > +endif > + > ######################################################################### > > RELFLAGS := $(PLATFORM_RELFLAGS) OK. If you want to enable this flag over the whole source tree, please remove it from examples/Makefile. BTW, In the description of commit 5c2aeac5ae, you mentioned tracing feature is not supported by SPL. But you are enabling FTRACE also on SPL in this patch. Are you sure there is no bad impact on SPL? Best Regards Masahiro Yamada
Hi Masahiro, On 10 June 2014 00:47, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote: > Hi Simon, > > > On Thu, 5 Jun 2014 12:27:49 -0600 > Simon Glass <sjg@chromium.org> wrote: > >> This was lost sometime in the Kbuild conversion. Add it back. > > Not lost. > It was moved to examples/Makefile. > > > Prior to Kbuild conversion, config.mk was like this: > > ------------------>8---------------------- > BCURDIR = $(subst $(SRCTREE)/,,$(CURDIR:$(obj)%=%)) > > ifeq ($(findstring examples/,$(BCURDIR)),) > ifeq ($(CONFIG_SPL_BUILD),) > ifdef FTRACE > CFLAGS += -finstrument-functions -DFTRACE > endif > endif > endif > --------------------8<------------------------- > > > "-finstrument-functions -DFTRACE" was enabled > only under examples/ directory. > (Do you remember why?) > > That's why I moved it to examples/Makefile > to keep the equivalent behavior. I don't think it is the same. In my code I was trying to make sure there was NO tracing in example directory, and SPL. I think I should go the same way, so will update my patch. > > > > >> diff --git a/config.mk b/config.mk >> index 05864aa..0c45c09 100644 >> --- a/config.mk >> +++ b/config.mk >> @@ -46,6 +46,10 @@ ifdef BOARD >> sinclude $(srctree)/board/$(BOARDDIR)/config.mk # include board specific rules >> endif >> >> +ifdef FTRACE >> +PLATFORM_CPPFLAGS += -finstrument-functions -DFTRACE >> +endif >> + >> ######################################################################### >> >> RELFLAGS := $(PLATFORM_RELFLAGS) > > > OK. > If you want to enable this flag over the whole source tree, > please remove it from examples/Makefile. OK > > > BTW, > In the description of commit 5c2aeac5ae, > you mentioned tracing feature is not supported by SPL. > > But you are enabling FTRACE also on SPL in this patch. > Are you sure there is no bad impact on SPL? Yes I should remove this otherwise it will at best bloat the code for SPL. I think it is probably best just to revert that part of the Makefile. Thanks for looking at this, saves me another patch... Regards, Simon
Hi Masahiro, On 11 June 2014 23:42, Simon Glass <sjg@chromium.org> wrote: > Hi Masahiro, > > Yes I should remove this otherwise it will at best bloat the code for > SPL. I think it is probably best just to revert that part of the > Makefile. Although actually I'm not sure how to have different flags for everything except SPL and examples/ - any clues? Regards, Simon
Hi Simon On Wed, 11 Jun 2014 23:42:25 -0400 Simon Glass <sjg@chromium.org> wrote: > Hi Masahiro, > > On 10 June 2014 00:47, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote: > > Hi Simon, > > > > > > On Thu, 5 Jun 2014 12:27:49 -0600 > > Simon Glass <sjg@chromium.org> wrote: > > > >> This was lost sometime in the Kbuild conversion. Add it back. > > > > Not lost. > > It was moved to examples/Makefile. > > > > > > Prior to Kbuild conversion, config.mk was like this: > > > > ------------------>8---------------------- > > BCURDIR = $(subst $(SRCTREE)/,,$(CURDIR:$(obj)%=%)) > > > > ifeq ($(findstring examples/,$(BCURDIR)),) > > ifeq ($(CONFIG_SPL_BUILD),) > > ifdef FTRACE > > CFLAGS += -finstrument-functions -DFTRACE > > endif > > endif > > endif > > --------------------8<------------------------- > > > > > > "-finstrument-functions -DFTRACE" was enabled > > only under examples/ directory. > > (Do you remember why?) > > > > That's why I moved it to examples/Makefile > > to keep the equivalent behavior. > > I don't think it is the same. In my code I was trying to make sure > there was NO tracing in example directory, and SPL. I think I should > go the same way, so will update my patch. > > > Oops. I totally screwed up. So sorry. I'm crazy. "-finstrument-functions -DFTRACE" must be __disabled__ under examples/ and spl/. I broke FTRACE feature. My apologies. Best Regards Masahiro Yamada
Hi Simon, On Wed, 11 Jun 2014 23:50:48 -0400 Simon Glass <sjg@chromium.org> wrote: > Hi Masahiro, > > On 11 June 2014 23:42, Simon Glass <sjg@chromium.org> wrote: > > Hi Masahiro, > > > > Yes I should remove this otherwise it will at best bloat the code for > > SPL. I think it is probably best just to revert that part of the > > Makefile. > > Although actually I'm not sure how to have different flags for > everything except SPL and examples/ - any clues? Me neither. But one possible solution is: Add the CONFIG_SPL_BUILD conditional to config.mk to disable it for SPL. ifeq ($(CONFIG_SPL_BUILD),) ifdef FTRACE CFLAGS += -finstrument-functions -DFTRACE endif endif exmpales/ seems having troubles with FTRACE. So, disable it when FTRACE=1 is defined. examples/Makefile is like this: ifndef FTRACE ifndef CONFIG_SANDBOX subdir-y += standalone subdir-$(CONFIG_API) += api endif endif BTW, can FTRACE build for all boards? I checked out v2014.01 (pre-kbuild) - make sandbox_config; make FTRACE=1 - make snow_config; make CROSS_COMPILE=arm-linux-gnueabi- FRACE=1 succeeded. But, - make omap3_beagle_config; make CROSS_COMPILE=arm-linux-gnueabi- FRACE=1 faied with lots of undefined reference errors to __cyg_profile_func_enter/_exit. I don't know why. Best Regards Masahiro Yamada
Hi Masahiro, On 12 June 2014 20:56, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote: > > Hi Simon, > > > On Wed, 11 Jun 2014 23:50:48 -0400 > Simon Glass <sjg@chromium.org> wrote: > > > Hi Masahiro, > > > > On 11 June 2014 23:42, Simon Glass <sjg@chromium.org> wrote: > > > Hi Masahiro, > > > > > > Yes I should remove this otherwise it will at best bloat the code for > > > SPL. I think it is probably best just to revert that part of the > > > Makefile. > > > > Although actually I'm not sure how to have different flags for > > everything except SPL and examples/ - any clues? > > > Me neither. > > But one possible solution is: [snip] That seems to work for me, thank you. Please don't worry about breaking it. If I was worried about the tests I would have come up with a way of automatically running them all by now. U-Boot needs a 'make test' but the test coverage is still pretty poor, so it is not high on my priority list. > > BTW, can FTRACE build for all boards? > > I checked out v2014.01 (pre-kbuild) > > - make sandbox_config; make FTRACE=1 > - make snow_config; make CROSS_COMPILE=arm-linux-gnueabi- FRACE=1 > > succeeded. But, > > - make omap3_beagle_config; make CROSS_COMPILE=arm-linux-gnueabi- FRACE=1 > > faied with lots of undefined reference errors to __cyg_profile_func_enter/_exit. > > > I don't know why. You need to enable it - see exynos5-dt.h which has: /* Allow tracing to be enabled */ #define CONFIG_TRACE #define CONFIG_CMD_TRACE #define CONFIG_TRACE_BUFFER_SIZE (16 << 20) #define CONFIG_TRACE_EARLY_SIZE (8 << 20) #define CONFIG_TRACE_EARLY #define CONFIG_TRACE_EARLY_ADDR 0x50000000 It would be nice to automate these settings, but for now you must add these settings for the board you are using. Actually I think it would be easy except for the 'early trace' feature, which needs to have a pre-relocation memory area to use. Regards, Simon
diff --git a/config.mk b/config.mk index 05864aa..0c45c09 100644 --- a/config.mk +++ b/config.mk @@ -46,6 +46,10 @@ ifdef BOARD sinclude $(srctree)/board/$(BOARDDIR)/config.mk # include board specific rules endif +ifdef FTRACE +PLATFORM_CPPFLAGS += -finstrument-functions -DFTRACE +endif + ######################################################################### RELFLAGS := $(PLATFORM_RELFLAGS)
This was lost sometime in the Kbuild conversion. Add it back. Check that the trace test now passes: $ ./test/trace/test-trace.sh Simple trace test / sanity check using sandbox /tmp/filemHKPGw Build sandbox O=sandbox FTRACE=1 GEN /home/sjg/c/src/third_party/u-boot/files/sandbox/Makefile Configuring for sandbox board... Check results Test passed Signed-off-by: Simon Glass <sjg@chromium.org> --- config.mk | 4 ++++ 1 file changed, 4 insertions(+)