At Fri, 15 Sep 2006 14:19:39 +0100,
Liam Girdwood wrote:
>
> +/* supported DAI types */
> +#define cpu_dai_name(x) \
> + (((x) == SND_SOC_DAI_AC97) ? "AC97" : \
> + ((x) == SND_SOC_DAI_I2S) ? "I2S" : \
> + ((x) == SND_SOC_DAI_PCM) ? "PCM" : NULL)
I'd write in a static inline function with switch().
> +static DEFINE_MUTEX(pcm_mutex);
> +static DEFINE_MUTEX(io_mutex);
> +static struct workqueue_struct *soc_workq = NULL;
The zero-initialization can be omitted.
> +/* get rate format from rate */
> +static inline int soc_get_rate_format(int rate)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(rates); i++) {
> + if(rates[i] == rate)
Cosmetic: A space is preferred between if and parenthesis.
Similar lines can be found all over places.
> + dbgc("mult --> div support mult %d\n", SND_SOC_FSB_REAL(1<<i));
Cosmetic: Try to keep lines within 80-char as much as possible.
> +/*
> + * Called by ALSA when a PCM substream is opened, the runtime->hw record is
> + * then initialized and any private data can be allocated. This also calls
> + * startup for the cpu DAI, platform, machine and codec DAI.
> + */
> +static int soc_pcm_open(struct snd_pcm_substream *substream)
> +{
(snip)
> + snd_pcm_limit_hw_rates(runtime);
> + if (!runtime->hw.rates) {
> + printk(KERN_ERR "asoc: %s <-> %s No matching rates\n",
> + rtd->codec_dai->name, rtd->cpu_dai->name);
> + mutex_unlock(&pcm_mutex);
> + return -ENODEV;
No shutdown or close ops?
> + }
> + if(!runtime->hw.formats) {
> + printk(KERN_ERR "asoc: %s <-> %s No matching formats\n",
> + rtd->codec_dai->name, rtd->cpu_dai->name);
> + mutex_unlock(&pcm_mutex);
> + return -ENODEV;
> + }
> + if(!runtime->hw.channels_min || !runtime->hw.channels_max) {
> + printk(KERN_ERR "asoc: %s <-> %s No matching channels\n",
> + rtd->codec_dai->name, rtd->cpu_dai->name);
> + mutex_unlock(&pcm_mutex);
> + return -ENODEV;
Ditto.
> + }
> +
> + dbg("asoc: %s <-> %s info:\n", rtd->codec_dai->name, rtd->cpu_dai->name);
> + dbg("asoc: rate mask 0x%x \nasoc: min ch %d max ch %d\nasoc: min rate %d max rate %d\n",
> + runtime->hw.rates, runtime->hw.channels_min,
> + runtime->hw.channels_max, runtime->hw.rate_min, runtime->hw.rate_max);
> +
> +
> + if(substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + rtd->cpu_dai->playback.active = rtd->codec_dai->playback.active = 1;
> + else
> + rtd->cpu_dai->capture.active = rtd->codec_dai->capture.active = 1;
> + rtd->cpu_dai->active = rtd->codec_dai->active = 1;
> + rtd->cpu_dai->runtime = runtime;
> + socdev->codec->active++;
> + mutex_unlock(&pcm_mutex);
> + return ret;
It must be zero, right?
> +static int soc_new_pcm(struct snd_soc_device *socdev,
> + struct snd_soc_dai_link *dai_link, int num)
> +{
(snip)
> + if((ret = snd_pcm_new(codec->card, new_name, codec->pcm_devs++,
> + playback, capture, &pcm)) < 0) {
> + printk(KERN_ERR "asoc: can't create pcm for codec %s\n", codec->name);
Memory leak?
> + return ret;
> + }
> +
> + pcm->private_data = rtd;
> + soc_pcm_ops.mmap = socdev->platform->pcm_ops->mmap;
> + soc_pcm_ops.pointer = socdev->platform->pcm_ops->pointer;
> + soc_pcm_ops.ioctl = socdev->platform->pcm_ops->ioctl;
> + soc_pcm_ops.copy = socdev->platform->pcm_ops->copy;
> + soc_pcm_ops.silence = socdev->platform->pcm_ops->silence;
> + soc_pcm_ops.ack = socdev->platform->pcm_ops->ack;
> + soc_pcm_ops.page = socdev->platform->pcm_ops->page;
> +
> + if (playback)
> + snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &soc_pcm_ops);
> +
> + if (capture)
> + snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &soc_pcm_ops);
> +
> + if((ret = socdev->platform->pcm_new(codec->card, codec_dai, pcm)) < 0) {
> + printk(KERN_ERR "asoc: platform pcm constructor failed\n");
> + return ret;
Ditto.
> +/* codec register dump */
> +static ssize_t codec_reg_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct snd_soc_device *devdata = dev_get_drvdata(dev);
> + struct snd_soc_codec *codec = devdata->codec;
> + int i, step = 1, count = 0;
> +
> + if(!codec->reg_cache_size)
> + return 0;
> +
> + if(codec->reg_cache_step)
> + step = codec->reg_cache_step;
> +
> + count += sprintf(buf, "%s registers\n", codec->name);
> + for(i = 0; i < codec->reg_cache_size; i += step) {
> + count += sprintf(buf + count, "%2x: %4x\n", i, codec->read(codec, i));
> + }
No need for braces.
> + * snd_soc_new_pcms - create new sound card and pcms
> + * @socdev: the SoC audio device
> + *
> + * Create a new sound card based upon the codec and interface pcms.
> + *
> + * Returns 0 for success, else error.
> + */
> +int snd_soc_new_pcms(struct snd_soc_device *socdev)
> +{
> + struct snd_soc_codec *codec = socdev->codec;
> + struct snd_soc_machine *machine = socdev->machine;
> + int ret = 0, i;
> +
> + mutex_lock(&codec->mutex);
> +
> + /* register a sound card */
> + codec->card = snd_card_new(SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1, codec->owner, 0);
I feel it's better to pass index and id explicilty from the caller.
Otherwise you have no way to specify them.
Takashi
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/alsa-devel
