[U-Boot] [RFC PATCH] mmc: omap_hsmmc: convert to use dm block devies

Tue Feb 7 17:11:21 UTC 2017

On Mon, Feb 06, 2017 at 11:31:14AM -0600, Andrew F. Davis wrote:
> On 02/03/2017 11:23 AM, Tom Rini wrote:
> > On Fri, Feb 03, 2017 at 09:57:03AM +0530, Vignesh R wrote:
> >>
> >>
> >> On Friday 03 February 2017 03:48 AM, Strashko, Grygorii wrote:
> >>> Convert OMAP hsmmc driver to use driver-model block devices.
> >>>
> >>> Signed-off-by: Grygorii Strashko <grygorii.strashko at ti.com>
> >>> ---
> >>> Hi All,
> >>>
> >>> First of all, sorry if my questions/problems are looks dumb, I'm very new with u-boot.
> >>>
> >>> This is my attampt to enable CONFIG_BLK on OMAP platforms which is blocked now
> >>> by omap_hsmmc driver. omap_hsmmc required to be updated to use driver-model block
> >>> devices at minimum (and max to use dm_mmc_ops). Also, as per my understanding,
> >>> CONFIG_BLK is blocker for other tasks like enabling driver model for OMAP sata devices.
> >>>
> >>> With this patch I can boot from mmc on am335x-evm, but there are
> >>> two problems I need help with: 
> >>> 1) My changes in Makefiles looks really ugly, but without them SPL build will
> >>>   fail because undef'ing in include/configs/am335x_evm.h does not take effect
> >>>   in Makefile (thanks Vignesh for the information [1]) and I, honestly, do not
> >>>   know how to fix it in better way, so I'd be appreciated for any help.
> >>>   Comparing to Vignesh's case, files which need to be excluded from build
> >>>   are generic and I worry that there can be dependecy from CONFIG_SPL_DM.
> >>>
> >>
> >>>
> >>> diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig
> >>> index 6f624e9..d36e7d4 100644
> >>> --- a/configs/am335x_evm_defconfig
> >>> +++ b/configs/am335x_evm_defconfig
> >>> @@ -30,7 +30,6 @@ CONFIG_CMD_GPIO=y
> >>>  CONFIG_CMD_EXT4_WRITE=y
> >>>  CONFIG_OF_CONTROL=y
> >>>  CONFIG_OF_LIST="am335x-evm am335x-bone am335x-boneblack am335x-evmsk am335x-bonegreen am335x-icev2"
> >>> -# CONFIG_BLK is not set
> >>>  CONFIG_DFU_MMC=y
> >>>  CONFIG_DFU_NAND=y
> >>>  CONFIG_DFU_RAM=y
> >>> diff --git a/drivers/block/Makefile b/drivers/block/Makefile
> >>> index 41217c1..44baee3 100644
> >>> --- a/drivers/block/Makefile
> >>> +++ b/drivers/block/Makefile
> >>> @@ -5,12 +5,16 @@
> >>>  # SPDX-License-Identifier:	GPL-2.0+
> >>>  #
> >>>  
> >>> -obj-$(CONFIG_BLK) += blk-uclass.o
> >>> +obj-$(CONFIG_$(SPL_)BLK) += blk-uclass.o
> >>>  
> >>>  ifndef CONFIG_BLK
> >>>  obj-y += blk_legacy.o
> >>>  endif
> >>>  
> >>> +ifdef SPL_
> >>> +obj-y += blk_legacy.o
> >>> +endif
> >>
> >>
> >> I am facing a same problem with DM_ETH as well.
> >> How about something like:
> >>
> >> ifeq ($(CONFIG_$(SPL_)DM)$(CONFIG_BLK),yy)
> >> obj-y += blk_uclass.o
> >> else
> >> obj-y += blk_legacy.o
> >> endif
> >>
> >>
> >> Is this an acceptable solution?
> > 
> > No, I think this is really highlighting that we need to figure out a
> > solution to using DM inside of SPL on these classes of devices.  I think
> > the last time I started to think out loud about how to solve this my
> > suggestion was to make a "dummy" board dts file that is correct enough
> > for SPL needs and that we can bring up full U-Boot on.  Off the top of
> > my head, the differences between BBB/BBW/GP-EVM/EVM-SK/BBG are not
> > things we would see inside of SPL.  In a similar manner we should be
> > able to get am43xx going, and dra7xx/am57xx going.
> > 
> > 
> 
> We don't really need DT support in SPL for using DM, we could statically
> instantiate device drivers using a board file type system,
> platform_add_device(device_data) etc..
> 
> My biggest objection is in SPL bloat. What do we get by adding DM to
> SPL? Code re-use is nice, but SPL had hard needs in speed and size that
> will continue to be eroded by these changes.
> 
> Perhaps it is time to re-consider adding separate defconfigs/Makefiles
> for SPL?

The problem we need to solve is to also clean up the code and keep it
clean.  The fine line we need to walk is the ability to fit into both
reasonably small and still reasonably supported SoCs and the need to
continue to be able to re-use code and unify code.  And part of the
answer here may be to say that newer compilers will be the required
minimum in some platforms too.  For example with am335x_hs:
gcc-6.1.1 (Debian 6.1.1-9):
$ size am335x_hs_evm/u-boot-spl
   text    data     bss     dec     hex filename
  37810    2452  198920  239182   3a64e am335x_hs_evm/u-boot-spl
gcc-4.9.0:
$ size am335x_hs_evm/u-boot-spl
   text    data     bss     dec     hex filename
  42082    2452  198980  243514   3b73a am335x_hs_evm/u-boot-spl

Since we're just talking text my assumption is that the fix for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54303 being available vs
not (it's also not fixed in gcc-5) is what is making up that large
difference.  And going from 698 bytes headroom to 4970 bytes headroom
will make things a little easier to deal with.

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