[U-Boot] [PATCH V2 2/2] common: add new boot media kconfig entry

[Masahiro Yamada <yamada.masahiro@socionext.com>]

Hi.

2016-06-28 15:39 GMT+09:00 Peng Fan <van.freenix@gmail.com>:
> Hi Masahiro,
> On Tue, Jun 28, 2016 at 02:24:00PM +0900, Masahiro Yamada wrote:
>>Hi.
>>
>>
>>2016-06-28 14:02 GMT+09:00 Peng Fan <van.freenix@gmail.com>:
>>> Hi Tom,
>>>
>>> On Fri, Jun 24, 2016 at 06:57:44PM -0400, Tom Rini wrote:
>>>>On Sun, Jun 19, 2016 at 06:20:52PM +0800, Peng Fan wrote:
>>>>> Hi Masahiro,
>>>>>
>>>>> +Simon
>>>>> On Fri, Jun 17, 2016 at 07:08:23PM +0900, Masahiro Yamada wrote:
>>>>> >2016-06-17 18:39 GMT+09:00 Peng Fan <van.freenix@gmail.com>:
>>>>> >> Add CONFIG_{SD|NAND|ONENAND|SPI|QSPI|SATA}_BOOT kconfig entries.
>>>>> >>
>>>>> >> SoCs supports loading U-Boot from different medias to DRAM, such as
>>>>> >> i.MX6/7 supports loading U-Boot to DRAM from sd/emmc/nand/qspi/spi/sata
>>>>> >> and etc. For i.MX, imximage will generate different IVT headers according
>>>>> >> to boot medias.
>>>>> >>
>>>>> >> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>> >> Cc: Simon Glass <sjg@chromium.org>
>>>>> >> Cc: Heiko Schocher <hs@denx.de>
>>>>> >> Cc: Joe Hershberger <joe.hershberger@ni.com>
>>>>> >> Cc: Bin Meng <bmeng.cn@gmail.com>
>>>>> >> Cc: Christophe Ricard <christophe-h.ricard@st.com>
>>>>> >> Cc: Nikita Kiryanov <nikita@compulab.co.il>
>>>>> >> Cc: Francois Retief <fgretief@spaceteq.co.za>
>>>>> >> Cc: Tom Rini <trini@konsulko.com>
>>>>> >> ---
>>>>> >>
>>>>> >> V2:
>>>>> >>  Move NOR_BOOT to the patch 1/2.
>>>>> >>  The idea of this patch is for adding different boot media support for
>>>>> >>  i.MXes. And I'll post out following patches if this patch is accepted.
>>>>> >>  I ran moveconfig.py, but I did not include the results into a patch.
>>>>> >>  This patch does not break the boards which defined NAND_BOOT/SD_BOOT and
>>>>> >>  etc, and I prefer to let board owners to move to defconfig later.
>>>>> >>
>>>>> >>  common/Kconfig | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> >>  1 file changed, 48 insertions(+)
>>>>> >>
>>>>> >> diff --git a/common/Kconfig b/common/Kconfig
>>>>> >> index 04d092c..f0f6ee1 100644
>>>>> >> --- a/common/Kconfig
>>>>> >> +++ b/common/Kconfig
>>>>> >> @@ -108,6 +108,54 @@ config NOR_BOOT
>>>>> >>           as the ROM only partially sets up pinmux.  We also default to using
>>>>> >>           NOR for environment.
>>>>> >>
>>>>> >> +config NAND_BOOT
>>>>> >> +       bool "Support for booting from NAND flash"
>>>>> >> +       default n
>>>>> >> +       help
>>>>> >> +         Enabling this will make a U-Boot binary that is capable of being
>>>>> >> +         booted via NAND flash. This is not a must, some SoCs need this,
>>>>> >> +         somes not.
>>>>> >> +
>>>>> >> +config ONENAND_BOOT
>>>>> >> +       bool "Support for booting from ONENAND"
>>>>> >> +       default n
>>>>> >> +       help
>>>>> >> +         Enabling this will make a U-Boot binary that is capable of being
>>>>> >> +         booted via ONENAND. This is not a must, some SoCs need this,
>>>>> >> +         somes not.
>>>>> >> +
>>>>> >> +config QSPI_BOOT
>>>>> >> +       bool "Support for booting from QSPI flash"
>>>>> >> +       default n
>>>>> >> +       help
>>>>> >> +         Enabling this will make a U-Boot binary that is capable of being
>>>>> >> +         booted via QSPI flash. This is not a must, some SoCs need this,
>>>>> >> +         somes not.
>>>>> >> +
>>>>> >> +config SATA_BOOT
>>>>> >> +       bool "Support for booting from SATA"
>>>>> >> +       default n
>>>>> >> +       help
>>>>> >> +         Enabling this will make a U-Boot binary that is capable of being
>>>>> >> +         booted via SATA. This is not a must, some SoCs need this,
>>>>> >> +         somes not.
>>>>> >> +
>>>>> >> +config SD_BOOT
>>>>> >> +       bool "Support for booting from SD/EMMC"
>>>>> >> +       default n
>>>>> >> +       help
>>>>> >> +         Enabling this will make a U-Boot binary that is capable of being
>>>>> >> +         booted via SD/EMMC. This is not a must, some SoCs need this,
>>>>> >> +         somes not.
>>>>> >> +
>>>>> >> +config SPI_BOOT
>>>>> >> +       bool "Support for booting from SPI flash"
>>>>> >> +       default n
>>>>> >> +       help
>>>>> >> +         Enabling this will make a U-Boot binary that is capable of being
>>>>> >> +         booted via SPI flash. This is not a must, some SoCs need this,
>>>>> >> +         somes not.
>>>>> >> +
>>>>> >>  endmenu
>>>>> >
>>>>> >
>>>>> >Do you intend to replace
>>>>> >CONFIG_SPL_NOR_SUPPORT
>>>>> >CONFIG_SPL_NAND_SUPPORT
>>>>> >CONFIG_SPL_USB_SUPPORT
>>>>> >CONFIG_SPL_MMC_SUPPORT
>>>>> >etc. with these options?
>>>>> >
>>>>>
>>>>> I missed these.
>>>>>
>>>>> >
>>>>> >Currently, common/spl/spl.c uses CONFIG_SPL_*_SUPPORT
>>>>> >to enable/disable capable boot devices.
>>>>>
>>>>> I think we could use a common option to replace the ones used in SPL.
>>>>
>>>>I'm not sure that CONFIG_xxx_BOOT and CONFIG_SPL_xxx_SUPPORT are the
>>>>same thing.  For example, CONFIG_NOR_BOOT on am335x means "we are
>>>>building to support booting from NOR, so include all of that stuff
>>>>that's normally not included".  While CONFIG_SPL_NAND_SUPPORT means
>>>>include NAND support in SPL, which is not mutually exclusive with MMC,
>>>>etc.
>>>
>>> ok. Then, better keep them. For i.MX6, CONFIG_XX_BOOT have the same meaning
>>> with am335x as you said.
>>>
>>> Do you have other comments? If not touching SPL, I would like to see this
>>> patch set applied.
>>>
>>> Thanks,
>>> Peng.
>>>>
>>
>>
>>I am not familiar with TI SoCs,
>
> Me too.

I missed to read git-log, I meant:
I am not familiar with i.MX SoCs.
(and I am not familiar with TI SoC, either)


>>so please help me to understand this.
>>
>>
>>+config NOR_BOOT
>>+       bool "Boot from NOR"
>>+       default n
>>+       help
>>+         U-Boot image is stored in NOR flash.
>>
>>
>>In the statement "U-Boot image is stored in NOR flash",
>>what "U-Boot" stands for?
>>SPL? or U-Boot proper?
>>
>>
>>If it points to SPL, it makes sense.
>
> In my case,
> SPL + U-Boot + OS.   means SPL
> Only U-Boot  + OS.   means U-Boot.


So, this board disables SPL
for an in-place executable device, like NOR.


I did likewise for my boards before,
and I found this was a PITA.

For a boot mode with SPL,
lowlevel_init code must be compiled into SPL.

For a boot mode without SPL,
lowlevel_init code must go into U-Boot proper.


Then, I ended up sprinkling ifdef CONFIG_SPL and
ifdef CONFIG_SPL_BUILD here are there.

This also loaded me per boot-mode testing
because boot sequence is too different for with/without SPL.


In my personal opinion, SPL should be enabled all the time.
Even for NOR boot, we can split the boot image
into SPL and U-Boot proper by using common/spl/spl_nor.c

Now, my SoC "select SPL"
and I can use an identical boot image
regardless of boot mode.

This may not be always possible,
but I am wondering if we could do someting
to mitigate the pain from per boot-mode defconfig.





>>
>>Theoretically, we can store SPL and U-Boot proper in different devices.
>>
>>CONFIG_FOO_BOOT means that SPL is stored in a FOO device.
>>(In other words, the hard-wired Boot ROM loads the SPL from the device FOO.
>>
>>
>>CONFIG_SPL_FOO_SUPPORT means that SPL supports loading U-Boot proper
>>from FOO a device.
>>In other words, the U-Boot proper can be stored in FOO device.
>
> From doc/README.SPL
> To support generic U-Boot libraries and drivers in the SPL binary one can
> optionally define CONFIG_SPL_XXX_SUPPORT. Currently following options
> are supported:
>
> ....
> CONFIG_SPL_MMC_SUPPORT (drivers/mmc/libmmc.o)
> ....
> CONFIG_SPL_NAND_SUPPORT (drivers/mtd/nand/libnand.o)
>
>>
>>
>>Correct?
>>
>>
>>One more question,
>>what will happen if both CONFIG_NOR_BOOT and CONFIG_NAND_BOOT are enabled?
>
> In my case, we do not allow CONFIG_NOR_BOOT and CONFIG_NAND_BOOT both defined.
> If both defined, U-Boot(I do not use spl) may not boot up in my case.


Perhaps, we should use a "choice" menu
to choose CONFIG_*_BOOT exclusively,
but I am not sure if it is the case for other vendors.
It looks like CONFIG_*_BOOT is used by TI as well as Freescale.


At least for my SoCs, I do not need these options at all
because one single boot image can boot from any device.


This patch is adding
  # CONFIG_NOR_BOOT is not set
  # CONFIG_NAND_BOOT is not set
  # CONFIG_QSPI_BOOT is not set
  ...

to my .config, which does not make sense.


Perhaps, can we define these symbols in an SoC Kconfig,
or surround them with "if <SOC>"?



-- 
Best Regards
Masahiro Yamada