[U-Boot] [RFC] fastboot gadget support

Remy Bohmer linux at bohmer.net
Wed Aug 24 19:34:25 UTC 2011


Hi,

2011/8/10 Sebastian Andrzej Siewior <bigeasy at linutronix.de>:
>
> It was tested before it has been re-written. It is possible that it is
> broken. It is posted for reference only not for merging.

Looking at this code, I can already say that the way it is written is
not suitable for mainline acceptance at all. If you at least would run
the Linux checkpatch over it, it would result in an almost endless
list of style warnings and errors ;-)
But, since it RFC only, let's look into the design parts and not be
too picky about the style (yet)

I may be missing something, but it appears to me that this particular
patch for the fastboot framework makes several assumptions about:
* if MMC is available (max 2 controllers, namely 0 or 1). Let' s say
my hardware doe not support it, and I do not want to enable it due to
codesize reasons...
* if NAND is available and it is either YAFFS or raw, what if I have UBI?
* Or let's drop the MMC and NAND, and assume we only have a harddrive
or USB storage on our board ;-)
* Hardcoded NAND block sizes which are evil.
* storage_medium is hardcoded set to NAND, never to EMMC, so what is
the EMMC code doing here?

Furthermore:
* Board code is mixed up with generic code.
* drivers/usb/dwc3/fastboot_oem.c, drivers/usb/dwc3/misc.c,
drivers/usb/dwc3/sparse.c contain code that has _nothing_ to do with
USB.
* generic files (for example like include/linux/usb/ch9.h) are adapted
with changes not even used by the code.
* Mix up of different licenses: U-boot is still GPLv2, while this
patch contains Apache based licenses (Not sure if it conflicts with
GPL, but it seems strange)
* it makes a lot of assumptions how the hardware looks like.
* It is not properly separated across different subsystems. There need
to be a proper separation between drivers, library code, U-boot core
code and board files. Everything that is board specific should go in
board files. NAND-availability or partitioning is board specific, MMC
as well, the only assumption you can make about hardware that should
always be available is RAM, you can only not assume how much there is,
and which address area is free. NAND and MMC usage should be
completely configurable per board.
* There need to be proper Documentation in the doc directory.
* It would be great to have at least a demo tool in /tools or a
commonly used OSS package that provides the tools, such that the
mechanism can be tested as well.

> This is the faastboot gadget code based on
> git://git.omapzoom.org/repo/u-boot.git as of commit 601ff71c8 including
> a few changes. Some of them are:
> - generic mmc framework

I have not found a ' generic'  mmc framework, only a 'fastboot'
dedicated mmc framework.

So, I have no objections to the protocol or the mechanism itself, as
long as it is properly implemented.

Kind regards,

Remy



More information about the linaro-dev mailing list