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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
