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