On Wed, Feb 26, 2014 at 08:05:41PM +0400, Alexander Shiyan wrote: > +static int clps711x_dai_set_clkdiv(struct snd_soc_dai *dai, int div_id, int div) > +{ > + struct clps711x_dai *s = dev_get_drvdata(dai->dev); > + > + s->div2 = div; > + > + return 0; > +} What is this divider and why does the machine driver need to manually configure it? > +static int clps711x_pcm_trigger(struct snd_pcm_substream *substream, int cmd) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct clps711x_dai *dai = snd_soc_platform_get_drvdata(rtd->platform); > + > + switch (cmd) { > + case SNDRV_PCM_TRIGGER_START: > + atomic_set(&dai->running, 1); > + hrtimer_start(&dai->hrt, ns_to_ktime(dai->reload), > + HRTIMER_MODE_REL); > + break; I'm wondering if the FIQ code can be shared with the i.MX FIQ code? Not something that should block merging but it seems like it might be helpful for other platforms that need to use FIQ support. > +static int clps711x_pcm_silence(struct snd_pcm_substream *substream, > + int channel, snd_pcm_uframes_t pos, > + snd_pcm_uframes_t count) > +{ > + return 0; > +} Omit empty functions. > +#if defined(CONFIG_SND_CLPS711X_SOC_MODULE) > +irqreturn_t no_action(int irq, void *dev_id) > +{ > + return IRQ_NONE; > +} > +#endif Eh? Waht's this about? > +static void clps711x_pcm_free(struct snd_pcm *pcm) > +{ > + struct snd_soc_pcm_runtime *rtd = pcm->private_data; > + struct clps711x_dai *dai = snd_soc_platform_get_drvdata(rtd->platform); > + > + /* Manually free IRQ */ > + devm_free_irq(rtd->platform->dev, dai->irq, NULL); Why is this using devm_?