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?

> +     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?

> +     /* 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...

> +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.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to