[U-Boot] [PATCH v2 4/4] spi: Convert CONFIG_DM_SPI* to CONFIG_$(SPL_TPL_)DM_SPI*

Tue Sep 3 16:52:11 UTC 2019

On Tue, Sep 03, 2019 at 05:56:36PM +0200, Lukasz Majewski wrote:
> Hi Tom,
> 
> > On Fri, Aug 23, 2019 at 11:02:27PM +0200, Lukasz Majewski wrote:
> > > Hi Tom,
> > >   
> > > > On Tue, Aug 13, 2019 at 03:47:31PM +0200, Lukasz Majewski wrote:  
> > > > > This change allows more fine tuning of driver model based SPI
> > > > > support in SPL and TPL. It is now possible to explicitly
> > > > > enable/disable the DM_SPI support in SPL and TPL via Kconfig
> > > > > option.
> > > > > 
> > > > > Before this change it was necessary to use:
> > > > >     /* SPI Flash Configs */
> > > > >     #if defined(CONFIG_SPL_BUILD)
> > > > >     #undef CONFIG_DM_SPI
> > > > >     #undef CONFIG_DM_SPI_FLASH
> > > > >     #undef CONFIG_SPI_FLASH_MTD
> > > > >     #endif
> > > > > 
> > > > > in the ./include/configs/<board>.h, which is error prone and
> > > > > shall be avoided when we strive to switch to Kconfig.
> > > > > 
> > > > > The goal of this patch:
> > > > > 
> > > > > Provide distinction for DM_SPI support in both U-Boot proper and
> > > > > SPL (TPL). Valid use case is when U-Boot proper wants to use
> > > > > DM_SPI, but SPL must still support non DM driver.
> > > > > 
> > > > > Another use case is the conversion of non DM/DTS SPI driver to
> > > > > support DM/DTS. When such driver needs to work in both SPL and
> > > > > U-Boot proper, the distinction is needed in Kconfig (also if SPL
> > > > > version of the driver supports OF_PLATDATA).
> > > > > 
> > > > > In the end of the day one would have to support following use
> > > > > cases (in single driver file - e.g. mxs_spi.c):
> > > > > 
> > > > > - U-Boot proper driver supporting DT/DTS
> > > > > - U-Boot proper driver without DT/DTS support (deprecated)
> > > > > - SPL driver without DT/DTS support
> > > > > - SPL (and TPL) driver with DT/DTS (when the SoC has enough
> > > > > resources to run full blown DT/DTS)
> > > > > - SPL driver with DT/DTS and SPL_OF_PLATDATA (when one have
> > > > > constrained environment with no fitImage and OF_LIBFDT support).
> > > > > 
> > > > > Some boards do require SPI support (with DM) in SPL (TPL) and
> > > > > some only have DM_SPI{_FLASH} defined to allow compiling SPL.
> > > > > 
> > > > > This patch converts #ifdef CONFIG_DM_SPI* to #if
> > > > > CONFIG_IS_ENABLED(DM_SPI) and provides corresponding defines in
> > > > > Kconfig.
> > > > > 
> > > > > Signed-off-by: Lukasz Majewski <lukma at denx.de>
> > > > > Tested-by: Adam Ford <aford173 at gmail.com> #da850-evm    
> > > > 
> > > > Sorry I didn't bisect down to which part of the series is doing
> > > > this, but I see problems with:
> > > > ls1046ardb_sdcard ls1046ardb_qspi_spl ls1043ardb_nand
> > > > ls1046aqds_tfa ls1046aq ds_nand ls1046ardb_qspi
> > > > ls1046aqds_sdcard_ifc ls1046aqds_SECURE_BOOT
> > > > ls1046aqds_sdcard_qspi ls1 046aqds_qspi ls1043ardb_sdcard
> > > > ls1043aqds_sdcard_ifc ls1046ardb_tfa ls1046ardb_tfa_SECURE_BOOT
> > > > ls1043aqds_nand ls1046aqds_lpuart ls1046ardb_emmc
> > > > ls1046aqds_tfa_SECURE_BOOT ls1046afrwy_tfa ls 1046aqds
> > > > ls1043ardb_nand_SECURE_BOOT ls1046ardb_qspi_SECURE_BOOT
> > > > ls1043aqds_sdcard_qspi
> > > > 
> > > > Some of which are dependency problems about SPL/SPL_DM.  I also
> > > > see: aarch64: (for 225/225 boards) all -294.2 bss -0.0 data -11.7
> > > > rodata -72.5 spl/u-boot-spl:all -0.3 spl/u-boot-spl:text -0.3 text
> > > > -210.0 [snip] ls1043ardb_nand: all -9435 data -376 rodata -2331
> > > > text -6728 ls1043ardb_sdcard: all -9435 data -376 rodata -2331
> > > > text -6728 ls1043aqds_sdcard_ifc: all -9435 data -376 rodata -2331
> > > > text -6728 ls1043aqds_nand: all -9435 data -376 rodata -2331 text
> > > > -6728 ls1043ardb_nand_SECURE_BOOT: all -9435 data -376 rodata
> > > > -2331 text -6728 ls1043ardb_sdcard_SECURE_BOOT: all -9435 data
> > > > -376 rodata -2331 text -6728 ls1043aqds_sdcard_qspi: all -9436
> > > > bss -8 data -376 rodata -2324 text -6728
> > > > 
> > > > which I think means a few conversions weren't right.  
> > > 
> > > It looks like some parts of the SPL are not build ....  
> > 
> > Yes, there's some dependency problem Kconfig is spotting related to
> > SPL/SPL_DM.
> > 
> > > Is the delta of size available from travis-CI or from any other
> > > script (maybe there is some buildman switch)?  
> > 
> > Yes, it's part of buildman.  I do:
> > $ export SOURCE_DATE_EPOCH=`date +%s`
> > $ ./tools/buildman/buildman -o  /tmp/test --step 0 -b origin/master..
> > --force-build -CveE $ ./tools/buildman/buildman -o  /tmp/test --step
> > 0 -b origin/master.. -Ssdel
> > 
> > to get size changes between point A and point Z in a branch, and omit
> > --step 0 when I need to see which patch in between them caused the
> > size change.
> > 
> 
> I cannot reproduce your results on current master branch:
> SHA1:d22c8be964a870f59d2fdab6c67cefa0c4799364
> 
> for this series applied.
> 
> echo $SOURCE_DATE_EPOCH
> 1567523778
> 
> ./tools/buildman/buildman.py -b HEAD --count=3 ls1043
> --output-dir=../BUILD/ --force-build -CveE
> 
> ./tools/buildman/buildman.py -b HEAD --count=3 ls1043
> --output-dir=../BUILD/ -Ssdel
> 
> 
> Just gives me some warnings (without the size difference):
> 
>    aarch64:  w+   ls1043aqds_nand ls1043ardb_nand
>    ls1043aqds_sdcard_qspi ls1043aqds_sdcard_ifc
>    ls1043ardb_nand_SECURE_BOOT ls1043ardb_sdcard
>    ls1043ardb_sdcard_SECURE_BOOT
>    +(ls1043aqds_nand,ls1043ardb_nand,ls1043ardb_nand_SECURE_BOOT,ls1043aqds_sdcard_ifc,ls1043aqds_sdcard_qspi,ls1043ardb_sdcard,ls1043ardb_sdcard_SECURE_BOOT)
>    ===================== WARNING ======================
>    +(ls1043aqds_nand,ls1043ardb_nand,ls1043ardb_nand_SECURE_BOOT,ls1043aqds_sdcard_ifc,ls1043aqds_sdcard_qspi,ls1043ardb_sdcard,ls1043ardb_sdcard_SECURE_BOOT)
>    This board does not use CONFIG_DM_SPI_FLASH. Please update
>    +(ls1043aqds_nand,ls1043ardb_nand,ls1043ardb_nand_SECURE_BOOT,ls1043aqds_sdcard_ifc,ls1043aqds_sdcard_qspi,ls1043ardb_sdcard,ls1043ardb_sdcard_SECURE_BOOT)
>    the board to use CONFIG_SPI_FLASH before the v2019.07 release.
>    +(ls1043aqds_nand,ls1043ardb_nand,ls1043ardb_nand_SECURE_BOOT,ls1043aqds_sdcard_ifc,ls1043aqds_sdcard_qspi,ls1043ardb_sdcard,ls1043ardb_sdcard_SECURE_BOOT)
>    Failure to update by the deadline may result in board removal.
>    +(ls1043aqds_nand,ls1043ardb_nand,ls1043ardb_nand_SECURE_BOOT,ls1043aqds_sdcard_ifc,ls1043aqds_sdcard_qspi,ls1043ardb_sdcard,ls1043ardb_sdcard_SECURE_BOOT)
>    See doc/driver-model/MIGRATION.txt for more info.
>    +(ls1043aqds_nand,ls1043ardb_nand,ls1043ardb_nand_SECURE_BOOT,ls1043aqds_sdcard_ifc,ls1043aqds_sdcard_qspi,ls1043ardb_sdcard,ls1043ardb_sdcard_SECURE_BOOT)
>    ====================================================
>    +(ls1043ardb_sdcard_SECURE_BOOT) This board does not use
>    CONFIG_DM_MMC. Please update +(ls1043ardb_sdcard_SECURE_BOOT) the
>    board to use CONFIG_DM_MMC before the v2019.04 release.
>    +(ls1043ardb_sdcard_SECURE_BOOT) This board does not use
>    CONFIG_DM_USB. Please update +(ls1043ardb_sdcard_SECURE_BOOT) the
>    board to use CONFIG_DM_USB before the v2019.07 release.
> 
> 
> Am I doing something wrong? Or am I missing any extra switches for
> buildman?
> 
> 
> And I don't know why the ' -b HEAD..HEAD~~~ ' option returns:
> 
>  Range 'HEAD..HEAD~~~' has no commits    , as I'm able to use HEAD with
>  -b switch and count=X ?

You should see output like (grabbing from a currently open log):
01: i2c: mxc: add CONFIG_CLK support
   aarch64:  w+   ls1043aqds_nand ls1043ardb_nand
...
17: Merge tag 'for-v2019.10-v2' of https://gitlab.denx.de/u-boot/custodians/u-boot-i2c
... 

when it's run right and comparing points A and Z.  As far as I can tell,
it's really important for the upstream tracking part (as far as git
branches goes) to be right, for this to work.  So if you for example
create a "SPI-conversion" branch off of master, and in turn master
tracks origin/master, if you 'git merge SPI-conversion' into master then
buildman --step 0 -b master does what I was talking about.  Telling git
"HEAD..HEAD~~~" is backwards?  Play around with 'git log --oneline
RANGE' to see what you need to say to get what you want.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190903/9d8819fb/attachment.sig>