On Wed, Nov 26, 2008 at 06:47:58PM +0800, Stanley.Miao wrote: > Add a shared omap SoC driver to avoid reduplicate code among omap soc drivers. It's really good to see work on this but I've got some concerns with the approach being taken here. > + > +config SND_OMAP_SOC_OVERO > + tristate "SoC Audio support for Gumstix Overo" > + depends on SND_OMAP_SOC && MACH_OVERO This won't apply - this and several of the other machines are already present. > +#ifdef CONFIG_SND_OMAP_SOC_N810 > +#include "../codecs/tlv320aic3x.h" > +#define CODEC_SYS_CLOCK 12000000 > +#define CODEC_NAME "TLV320AIC33" > +#define STREAM_NAME "AIC33" > +#define CODEC_DEV (&soc_codec_dev_aic3x) > +#define CODEC_DAI (&aic3x_dai) > +#define CODEC_SETUP_DATA (&&n810_aic33_setup) > +#define DAI_LINK_INIT (n810_aic33_init) > +#define SOC_OPS_STARTUP (n810_startup) > +#define SOC_OPS_SHUTDOWN (n810_shutdown) Please change this to use a platform data based approach rather than these ifdefs: it's not a good approach for readability and it prevents building multiple board drivers in one kernel which isn't good for distributions. The s3c24xx_uda134x.c provides an example of how this can be done. It may be more sensible to have a separate driver per codec. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
