On Tue, Dec 06, 2016 at 05:57:57PM +0200, Igor Grinberg wrote:
> Hi Tom,
>
> On 12/06/16 16:32, Tom Rini wrote:
> > On Tue, Dec 06, 2016 at 02:48:27PM +0100, Fabien Parent wrote:
> >
> >> Moving to CONFIG_SPL_*_LOAD options to Kconfig offers several advantage:
> >> * simpler config headers
> >> * on some boards we can easily switch to another boot media without
> >> needing
> >> to modify the config headers.
> >>
> >> This series fixes an issue in davinci where a wrong option was used in
> >> place
> >> of a CONFIG_SPL_*_LOAD option, then move the options to Kconfig options,
> >> and
> >> finally start using these Kconfig options for the OMAPL138-LCDK board.
> >>
> >> Fabien Parent (3):
> >> davinci: spl: use correct macro to select boot device
> >> SPL: create Kconfig options for CONFIG_SPL_*_LOAD
> >> davinci: omapl138_lcdk: use new CONFIG_SPL_*_LOAD Kconfig options
> >>
> >> arch/arm/mach-davinci/spl.c | 2 +-
> >> common/spl/Kconfig | 25 +++++++++++++++++++++++++
> >> configs/omapl138_lcdk_defconfig | 1 +
> >> include/configs/omapl138_lcdk.h | 2 --
> >> scripts/config_whitelist.txt | 3 ---
> >> 5 files changed, 27 insertions(+), 6 deletions(-)
> >
> > So, I think this shows that some of the SPL framework needs to be
> > revisited for davinci. First, lets make it clear what
> > CONFIG_SPL_{SPI,NAND,MMC}_LOAD is doing.
> >
> > CONFIG_SPL_MMC_LOAD is used to flag that on davinci we're loading U-Boot
> > from MMC. It's not set / used today but I assume it was working when I
> > introduced all of this.
> >
> > CONFIG_SPL_NAND_LOAD is used for two things. First, it is used to flag
> > that on davinci we're loading U-Boot from NAND. Second, it is used to
> > enable the non-SPL_FRAMEWORK NAND driver
> > (drivers/mtd/nand/nand_spl_load.c). This driver is not used on davinci.
> >
> > CONFIG_SPL_SPI_LOAD is used for two things. First, it is used to flag
> > that on davinci we're loading U-Boot from SPI flash. Second, it used
> > globally to enable common/spl/spl_spi.c.
> >
> > NAND boot is done here via CONFIG_SPL_NAND_SIMPLE which is the regular
> > SPL framework based NAND driver (drivers/mtd/nand/nand_spl_simple.c).
> > This also means that the patch to update CONFIG_SYS_NAND_U_BOOT_SIZE was
> > not needed since we don't use that driver.
> >
> > Now, I think that in retrospect
> > arch/arm/mach-davinci/spl.c::spl_boot_device could be re-worked to key
> > off of CONFIG_SPL_NAND_SIMPLE / CONFIG_SPL_SPI_SUPPORT / SPL_MMC_SUPPORT.
> >
> > And a good but possibly complex series would be to consolidate the usage
> > of SPL_SPI_SUPPORT, SPL_SPI_FLASH_SUPPORT and SPL_SPI_LOAD just in to
> > SPL_SPI_SUPPORT. I'll probably try and do this myself as there's a ton
> > of build testing and size checking to make sure nothing odd breaks here
> > to do.
>
> Just a thought...
> Are you sure you want to combine all three (spi, spi flash, and spi load)
> under one define?
> Won't be there any case for parsing some spi device (say eeprom) in the SPL,
> but no spi flash to load U-Boot from?
> It might be sensible to keep the "spi flash" and the "spi load" together, but
> I think it might be more beneficial to keep the spi bus support apart.Ah, good point. I don't know if we have that case today, but that would be one of the gotchas I'd be size-checking for. And another place where the 'imply' keyword Masahiro pointed out earlier would be helpful for too. Thanks! -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
