Re: Re: [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC

["Péter Ujfalusi" <peter.ujfalusi@ti.com>]

On Tuesday 22 November 2011 16:01:05 Mark Brown wrote:
> On Tue, Nov 22, 2011 at 04:01:57PM +0200, Peter Ujfalusi wrote:
> > +	switch (params_rate(params)) {
> > +	case 96000:
> > +	case 192000:
> > +		break;
> 
> Why doesn't the driver need to tell the hardware what sample rate to run
> at?

Because the OMAP4 DMIC can only support on 96KHz...
Will be fixed.

> 
> > +	dmic_clk = clk_get(dmic->dev, "dmic_fck");
> > +	if (IS_ERR(dmic_clk)) {
> > +		dev_err(dmic->dev, "cant get dmic_fck\n");
> > +		return -ENODEV;
> > +	}
> 
> Why aren't we getting and holding a reference to the clock over the
> entire lifetime of the driver?

We only need the reference for the dmic_fclk for reparenting which will happen 
only once in most cases (or not at all, if pad_clks_ck is going to be used). I 
don't think we should hold the reference for the dmic_fclk.
The clock handling is done via pm_runtime_get/put_sync().

> > +	/* disable clock while reparenting */
> > +	pm_runtime_put_sync(dmic->dev);
> > +	ret = clk_set_parent(dmic_clk, parent_clk);
> > +	pm_runtime_get_sync(dmic->dev);
> 
> Since we're only allowing reclocking while idle shouldn't the clock
> already be disabled?  Seems like it ought to be good for power if
> nothing else...

We enable the clocks at dai_startup for the DMIC (and disable them on 
dai_shutdown). We can not reparent while the clocks are enabled.
This is the reason for this part.
 
> > +static int omap_dmic_set_clkdiv(struct snd_soc_dai *dai,
> > +				int div_id, int div)
> > +{
> 
> DMIC clocking is usually fairly simple so it seems surprising that the
> driver isn't able to figure this out for itself.

The clock towards the external digital mics are based on the DMIC_FCLK rate.
In case of DMIC_FCLK = 19.2MHz, 24.576MHz we can select between two dividers 
which will result different clocks for the external microphones.
I would rather leave this decision to the machine driver which knows the 
external components, and can pick the best divider for them.

--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html