On Tue, Oct 13, 2009 at 03:03:11PM +0300, Peter Ujfalusi wrote:
> b) The nSample mode implementation uses one interrupt line from DAC33 to
Just a thought, but could a timer based approach work here?
> + case SND_SOC_BIAS_PREPARE:
> + break;
> + case SND_SOC_BIAS_STANDBY:
> + dac33_soft_power(codec, 0);
> + break;
> + case SND_SOC_BIAS_OFF:
> + break;
> + }
It'd be nice to do the full power down in BIAS_OFF for future use.
> +static irqreturn_t dac33_interrupt_handler(int irq, void *dev)
> +{
> + struct snd_soc_codec *codec = dev;
> + struct tlv320dac33_priv *dac33 = codec->private_data;
> +
> + if (dac33->state == DAC33_PLAYBACK)
> + queue_work(dac33->dac33_wq, &dac33->work);
> +
> + return IRQ_HANDLED;
> +}
Should that return IRQ_HANDLED if it's not expecting an interrupt? I'd
also expect to see something that either acknowledges the interrupt to
deassert it or keeps it masked until the work has had a chance to do
something.
> + pdata = (struct tlv320dac33_platform_data *)client->dev.platform_data;
Unneeded cast.
> + dac33->dac33_wq = create_rt_workqueue("tlv320dac33");
> + if (dac33->dac33_wq == NULL) {
> + ret = -ENOMEM;
> + goto error_wq;
> + }
Might be nice to skip the workqueue stuff if we don't have an IRQ.
Conditional registration of the nSample controls based on having the IRQ
may also be nice.
> + ret = snd_soc_register_codec(codec);
> + if (ret != 0) {
> + dev_err(codec->dev, "Failed to register codec: %d\n", ret);
> + goto error_codec;
> + }
> + ret = snd_soc_register_dai(&dac33_dai);
> + if (ret != 0) {
> + dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
> + snd_soc_unregister_codec(codec);
> + goto error_codec;
> + }
I'd do these last so that we don't have ASoC level init trying to use
the device only to have it yanked from underneath it by a failure in the
following code.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
