From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Fri, 21 Aug 2015 10:16:49 +0200 Subject: [U-Boot] [U-Boot, v3, 15/15] of: clean up OF_CONTROL ifdef conditionals In-Reply-To: References: <1439332317-727-16-git-send-email-yamada.masahiro@socionext.com> <201508192315.27705.marex@denx.de> Message-ID: <201508211016.49824.marex@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Friday, August 21, 2015 at 09:55:25 AM, Masahiro Yamada wrote: > Hi Marek, Hi Masahiro, > 2015-08-20 6:15 GMT+09:00 Marek Vasut : > > On Wednesday, August 19, 2015 at 02:02:18 PM, Tom Rini wrote: > >> On Wed, Aug 19, 2015 at 06:39:29AM +0200, Marek Vasut wrote: > >> > On Tuesday, August 18, 2015 at 07:53:23 PM, Tom Rini wrote: > >> > > On Wed, Aug 12, 2015 at 07:31:55AM +0900, Masahiro Yamada wrote: > >> > > > We have flipped CONFIG_SPL_DISABLE_OF_CONTROL. We have cleansing > >> > > > devices, $(SPL_) and CONFIG_IS_ENABLED(), so we are ready to clear > >> > > > > >> > > > away the ugly logic in include/fdtdec.h: > >> > > > #ifdef CONFIG_OF_CONTROL > >> > > > # if defined(CONFIG_SPL_BUILD) && !defined(SPL_OF_CONTROL) > >> > > > # define OF_CONTROL 0 > >> > > > # else > >> > > > # define OF_CONTROL 1 > >> > > > # endif > >> > > > #else > >> > > > # define OF_CONTROL 0 > >> > > > #endif > >> > > > > >> > > > Now CONFIG_IS_ENABLED(OF_CONTROL) is the substitute. It refers to > >> > > > CONFIG_OF_CONTROL for U-boot proper and CONFIG_SPL_OF_CONTROL for > >> > > > SPL. > >> > > > > >> > > > Also, we no longer have to cancel CONFIG_OF_CONTROL in > >> > > > include/config_uncmd_spl.h and scripts/Makefile.spl. > >> > > > > >> > > > Signed-off-by: Masahiro Yamada > >> > > > Reviewed-by: Tom Rini > >> > > > Reviewed-by: Simon Glass > >> > > > Acked-by: Linus Walleij > >> > > > >> > > (With a slight change due to samsun PR, but applied cleanly after > >> > > rename) > >> > > Applied to u-boot/master, thanks! > >> > > >> > And this just broke just about every SoCFPGA platform (try ie. > >> > socfpga_cyclone5) ;-) I poked into this a bit and noticed that adding > >> > #include into include/configs/socfpga_common.h fixes > >> > the issue, but I'd like to check with you if this is the correct way > >> > of fixing it. > >> > >> Broke it how? > >> $ ./tools/buildman/buildman socfpga_cyclone5 > >> boards.cfg is up to date. Nothing to do. > >> Building current source for 1 boards (1 thread, 6 jobs per thread) > >> > >> 1 0 0 /1 socfpga_cyclone5 > > > > Looks like I wasn't really accurate and mainline actually managed to > > avoid this breakage. I only triggered it with a custom patch, which > > enables env in SPI NOR (see the spit below). > > > > I suspect the problem is caused by tools/envcrc.c, which does include the > > board config file (which uses this CONFIG_IS_ENABLED()), but does not > > include a header file which defines it. > > > > The question therefore is, shall we not used CONFIG_IS_ENABLED() in board > > config files or shall we include linux/kconfig.h in tools/envcrc.c ? > > Our final goal is: > We should not use any CONFIG_* in host tools. > Actually, some host tools in U-boot depend on the board > configuration, which is weird. > If we could rip off CONFIG_* from them, neither CONFIG_IS_ENABLED() > nor linux/kconfig.h > is needed for host tools. I believe this is the right thing to do. > Any volunteer? > > > What I can suggest now is: > Do not use CONFIG_IS_ENABLED() in board config headers. Yup, this is what I'll do :) > include/config/microblaze-generic.h does this: > > #if !defined(CONFIG_OF_CONTROL) || \ > (defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_OF_CONTROL)) > /* ddr sdram - main memory */ > # define CONFIG_SYS_SDRAM_BASE XILINX_RAM_START > # define CONFIG_SYS_SDRAM_SIZE XILINX_RAM_SIZE > #endif > > > This is indeed ugly, but works... I don't think I even need this :) Best regards, Marek Vasut