All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/8] hwconfig and some its users (was: Re: [PATCH 2/6] Add FSL "Can use" framework)
@ 2009-04-29 21:48 Anton Vorontsov
  2009-04-29 21:50 ` [U-Boot] [PATCH 1/8] Add simple hwconfig infrastructure Anton Vorontsov
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Anton Vorontsov @ 2009-04-29 21:48 UTC (permalink / raw)
  To: u-boot

On Thu, Feb 19, 2009 at 08:56:50PM +0100, Wolfgang Denk wrote:
> Dear Anton Vorontsov,
> 
> In message <20090219154545.GB26618@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: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2009-06-02 22:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-29 21:48 [U-Boot] [PATCH 0/8] hwconfig and some its users (was: Re: [PATCH 2/6] Add FSL "Can use" framework) Anton Vorontsov
2009-04-29 21:50 ` [U-Boot] [PATCH 1/8] Add simple hwconfig infrastructure Anton Vorontsov
2009-04-29 21:59   ` Anton Vorontsov
2009-04-30 20:00   ` Kim Phillips
2009-04-30 20:22     ` Anton Vorontsov
2009-04-30 22:31   ` Wolfgang Denk
2009-04-30 23:12     ` Anton Vorontsov
2009-05-01  7:33       ` Wolfgang Denk
2009-06-02 20:51   ` Andy Fleming
2009-06-02 22:11     ` Anton Vorontsov
2009-04-29 21:50 ` [U-Boot] [PATCH 2/8] fsl_esdhc: Add device tree fixups Anton Vorontsov
2009-04-29 21:50 ` [U-Boot] [PATCH 3/8] mpc83xx: MPC837XERDB: Add support for FSL eSDHC Anton Vorontsov
2009-04-29 21:50 ` [U-Boot] [PATCH 4/8] mpc83xx: MPC837XEMDS: Fixup eSDHC nodes in device tree Anton Vorontsov
2009-04-29 21:50 ` [U-Boot] [PATCH 5/8] fdt_support, usb: Move fdt_fixup_dr_usb routine to drivers/usb/otg Anton Vorontsov
2009-04-29 21:50 ` [U-Boot] [PATCH 6/8] fsl_dr_usb: Fixup disabled USB controllers nodes in device tree Anton Vorontsov
2009-04-29 21:50 ` [U-Boot] [PATCH 7/8] mpc83xx: MPC8315ERDB: Use hwconfig for board type selection Anton Vorontsov
2009-04-29 21:50 ` [U-Boot] [PATCH 8/8] mpc83xx: MPC837xEMDS: Use hwconfig instead of pci_external_arbiter variable Anton Vorontsov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.