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

[Péter Ujfalusi]

On Wednesday 23 November 2011 14:30:50 Mark Brown wrote:
> Meh, I guess.  It's hard to love code-wise.

So you would prefer me to enable the OMAP DMIC's clocks at pcm_trigger:start 
time, and disable them on pcm_trigger:stop?
I have seen cases when the driver did not received the pcm_trigger:stop prior 
to pcm_close call, so in that case a safety check in dai_shutdown is needed to 
be sure the dmic clocks are really disabled.
I would still prefer to keep the dmic clocks enabled for the duration the 
stream is open. On the other hand if I had it in pcm_trigger, I don't need to 
play with the clocks when reparenting...

> > > If that's what you're doing then it seems like the machine drivers
> > > should be use set_sysclk() (or perhaps even the clk API) to set up
> > > the
> > > rate they're looking for from the visible clock rather than fiddling
> > > about with magic divider values.  That way they can say exactly what
> > > they want directly in terms of the result they're looking for.
> > 
> > In OMAP4 DMIC the divider can not be chosen freely.
> > The clock provided to the external microphones will depend on the
> > selected DMIC_FCLK, and the divider.
> > If I ask the machine driver to ask for specific speed for the external
> > mic, the writer of the machine driver anyways have to look up the table
> > from the TRM for the possible frequencies. So instead of providing
> > magic divider it has to provide magic speed.
> 
> Sure, but on the other hand it means that someone reading the machine
> driver can tell what's going on without going back to the magic table
> either.  Having the rates in the code makes the code more legible and
> means that people can at least refer to the DMIC driver for a list of
> supported rates rather than having to find the TRM.
> 
> I'd also guess that it's much more likely that people will remember
> clock rates they can set than divider tables but perhaps that's just me.

Sure, I will change the driver accordingly.

--
Péter