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