On 7 Aug 2010, at 21:28, Mike Frysinger vapier@gentoo.org wrote:
+config SND_BF5XX_SOC_ADAU1361
- tristate "SoC ADAU1361 Audio support"
- depends on SND_BF5XX_I2S
- select SND_BF5XX_SOC_I2S
- select SND_SOC_ADAU1361
- select I2C
- help
Say Y if you want to add support for ADAU1361 SoC audio.
Identical comments to the last machine driver you posted here.
It'd be really helpful if you could do a bit more pre-review on the stuff you're posting, especially where you've got a bunch of drivers that follow similar patterns and are going to generate exactly the same feedback. Posting a large batch of patches at once isn't a problem but if you're sending additional patches later on it'd be helpful to take into account the feedback that has already been given on prior serieses.
- switch (params_rate(params)) {
- case 11025:
- case 22050:
- case 44100:
- case 88200:
pll_out = ADAU1361_PLL_FREQ_441;break;- case 8000:
- case 12000:
- case 16000:
- case 24000:
- case 32000:
- case 48000:
- case 64000:
- case 96000:
pll_out = ADAU1361_PLL_FREQ_48;break;- default:
pll_out = ADAU1361_PLL_FREQ_441;break;- }
It seems silly for the machine drivers to all have to manually do this lookup - it appears to be entirely and inflexibly related to the sample rate in use. May as well just specify the input clock to the device.
- /* set codec DAI configuration */
- ret = codec_dai->ops->set_fmt(codec_dai,
SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |SND_SOC_DAIFMT_CBM_CFM);- if (ret < 0)
return ret;
There are accessor functions for this stuff. I've stopped reviewing the code at this point.