At Tue, 31 Jul 2007 18:17:33 +0200,I wrote:> > At Tue, 31 Jul 2007 11:12:44 -0500,> Timur Tabi wrote:> > > > Takashi Iwai wrote:> > > At Tue, 31 Jul 2007 10:46:52 -0500,> > > Timur Tabi wrote:> > >> This patch adds ALSA SoC support for the Cirrus Logic CS4270 codec. The> > >> following features are suppored:> > >>> > >> 1) Stand-alone and software mode> > >> 2) Software mode via I2C only> > >> 3) Master mode, not Slave> > >> 4) No power management> > >>> > >> Signed-off-by: Timur Tabi <timur@xxxxxxxxxxxxx>> > > > > > Thanks, it can be now applied fine.> > > > > > The only thing I noticed is below (oh I should have mentioned it> > > before...):> > > > > >> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig> > >> index e5fb437..7824880 100644> > >> --- a/sound/soc/codecs/Kconfig> > >> +++ b/sound/soc/codecs/Kconfig> > >> @@ -17,3 +17,23 @@ config SND_SOC_WM8753> > >> config SND_SOC_WM9712> > >> tristate> > >> depends on SND_SOC> > >> +> > >> +# Cirrus Logic CS4270 Codec> > >> +config SND_SOC_CS4270> > >> + tristate> > >> + depends on SND_SOC> > >> +> > >> +# Cirrus Logic CS4270 Codec Hardware Mute Support> > >> +# Select if you have external muting circuitry attached to your CS4270.> > >> +config SND_SOC_CS4270_HWMUTE> > >> + bool> > >> + depends on SND_SOC_CS4270> > >> +> > >> +# Cirrus Logic CS4270 Codec VD = 3.3V Errata> > >> +# Select if you are affected by the errata where the part will not function> > >> +# if MCLK divide-by-1.5 is selected and VD is set to 3.3V. The driver will> > >> +# not select any sample rates that require MCLK to be divided by 1.5.> > >> +config SND_SOC_CS4270_VD33_ERRATA> > >> + bool> > >> + depends on SND_SOC_CS4270> > >> +> > > > > > I'd suggest to convert these comments to help texts.> > > Also, usually the items to be chosen via menuconfig have> > > tristate "XXXX"> > > or> > > bool "XXX"> > > > > > Could you fix them, then I'll finally merge it to upstream?> > > > I am following the model of the other Kconfig options. If I add the "XXXX" and the help > > text, then they become selectable from "make menuconfig", but they would be the only ones > > to show up like that.> > > > I think the reason why they are like this is because they're supposed to be selected from > > the machine driver that uses them, like this:> > Doh, I see, these are codecs only for reverse selections.> Sorry for confusion. Then I'll merge it as it is. OK, now found another problem. The driver seems not built withoutCONFIG_I2C. In file included from sound/soc/codecs/cs4270.c:1:sound/soc/codecs/cs4270.c:694: error: ʽcs4270_set_dai_sysclkʼ undeclared here (not in a function)sound/soc/codecs/cs4270.c:695: error: ʽcs4270_set_dai_fmtʼ undeclared here (not in a function) Shouldn't the driver be dependent on I2C? Also, #ifdef CONFIG_I2C isn't enough. It should be #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE) since I2C can be a module. I already merged V3 to HG tree, so please post the additional patchagainst it for fixing these issues. thanks, Takashi_______________________________________________Alsa-devel mailing listAlsa-devel@xxxxxxxxxxxxxxxxxxxx://mailman.alsa-project.org/mailman/listinfo/alsa-devel
