On Tue, Sep 22, 2009 at 01:28:58PM -0600, miguel.aguilar@xxxxxxxxxxxx wrote:
Looks mostly good - the main issue here is the device registration
stuff, everything else is fairly minor.
> + select SND_SOC_CQ0093VC if ARCH_DAVINCI_DM365
This probably should have no dependency (see below about device
registration).
> +#define AUDIO_NAME "cq0093"
> +#define CQ93VC_VERSION "0.1"
I'd just drop these.
> +static int cq93vc_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *dai)
> +{
> + return 0;
> +}
Just omit this if it's empty.
> +static const struct snd_kcontrol_new cq93vc_snd_controls[] = {
> + SOC_SINGLE("PGA Capture Volume", VC_REG05, 0, 0x03, 0),
> + SOC_SINGLE("Mono DAC Playback Volume", VC_REG09, 0, 0x3f, 0),
Any chance of adding TLV information for these? Not essential but it's
nice for userspace applications.
> +static int cq93vc_add_controls(struct snd_soc_codec *codec)
> +{
Please use snd_soc_add_controls instead.
> +static int cq93vc_set_bias_level(struct snd_soc_codec *codec,
> + enum snd_soc_bias_level level)
> +{
> + switch (level) {
> + case SND_SOC_BIAS_ON:
> + /* all power is driven by DAPM system */
> + cq93vc_write(codec, VC_REG12, POWER_ALL_ON);
The code is OK but the driver isn't using DAPM.
> +static int cq93vc_probe(struct platform_device *pdev)
> +{
You should make the driver a regular platform device - that way you
won't need to do the trick with the global variable to get the resources
and you won't need to register the DAIs on module_init. See wm8350 for
an example of doing this with a platform device.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
