On Thu, Feb 19, 2009 at 08:56:50PM +0100, Wolfgang Denk wrote: > Dear Anton Vorontsov, > > In message <20090219154545.gb26...@oksana.dev.rtsoft.ru> you wrote: > > So far it's used for specifying whether we want to use FSL DR USB or > > FSL eSDHC devices on MPC837X processors. > > > > There are two more candidates for future use: > > 1. USB ULPI PHY vs. TSEC1 on MPC8315E-RDB boards; > > 2. Marvell vs. Vitesse PHYs on MPC8313E-RDB boards. > > If you know that might need to be extended, than better plan for it > right from the beginning.
OK. Done. > > diff --git a/board/freescale/common/fsl_can_use.c > > b/board/freescale/common/fsl_can_use.c > > new file mode 100644 > > index 0000000..17cc20f > > --- /dev/null > > +++ b/board/freescale/common/fsl_can_use.c > > That's a very strange name for a function, IMHO. I would expect that > it has something to do with using a CAN controller... > > > +int __fsl_can_use_dr_usb(void) > > If you plan to make this extendable, please use a more generic name. > For example: fsl_hwconfig() [note that leading __ is reserved]. > > > + const char *usb_or_esdhc = getenv("usb_or_esdhc"); > > Please make it extendable, use a more generic name for one (and only > one) environment variable. It makes littel sense to pollyte the > envrionment with N additional variables. For example, call it > "hwconfig". Allow that this variable holds a list of config settings, > separated for example by comma or colon or ... > > > + if (!usb_or_esdhc || !strcmp(usb_or_esdhc, "usb")) > > + return 1; > > + > > + if (!strcmp(usb_or_esdhc, "esdhc")) > > + return 0; > > This doesn't scale as well. Use a table of known strings and (static > inline) function pointers - this is much easier to extend when new > options need to get added. > > > Once we've reached this point, we might even lean back and think which > part of this is FSL specific. None, tight? So make this a generic > feature and look around which other code can be replaced by it. > > We can probably define both the content of option name / handler > function pointer table and the respective handler functions in a board > specific file - eventually even the board config file. > > That would make for a flexible solution. Here it is, patches on the way. It's very simple. Just an environment variable, but it should be enough to do what we need currently. I should write some documentation though. Thanksб -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot