Hi, 2011/8/10 Sebastian Andrzej Siewior <bige...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
