Mark Brown wrote:
>
> On Fri, Dec 17, 2010 at 02:41:29PM -0700, Stephen Warren wrote:
> > + * harmony.c - Harmony machine ASoC driver
>
> Lots of direct fiddling with clocks here. It feels like a lot of this
> should be abstracted out of the machine drivers - the WM8903 is a fairly
> straightforward CODEC from the system integration point of view, 99% of
> single DAI CODECs are going to require identical configuration, and much
> of the clocking configuration looks like it's actually directed at the
> I2S controller and should be handled there.
Yes, the I2S driver does seem the correct place for this. However, it
might be tough to make that work due to how the HW clocking is set up.
The root of the audio clocks is a single PLL, pll_a, with one output,
pll_a_out0. This one output feeds the audio module clock and both
I2S controllers. While the feed to at least each I2S controller has a
Separate divider per controller, the PLL can't run fast enough to be a
multiple of both the two clock rates the driver currently uses[1]. So,
once we've pick a 44.1KHz-class clock rate for one I2S controller, that
clock also applies to the other I2S controller, albeit perhaps with a
different divider.
Hence, I put the clock programming into the machine driver, which is
the single place that knows whether the limitation is relevant (it
isn't if only one I2S controller is in use like on Harmony), and so
that the machine driver can control policy when there are conflicts.
Perhaps the I2S driver can still manage all this though, and simply
apply PCM constraints to any subsequent I2S controller's streams when
one stream/controller is configured for the first time?
[1] Well, I'm basing that on tegra2_clocks.c's max_rate value, which I've
already proved wrong since the pll table already contains a configuration
for a higher clock than max_rate was set to; I should go check this with
the HW designers.
> > + switch (srate) {
> > + case 11025:
> > + case 22050:
> > + case 44100:
> > + case 88200:
> > + bitclock = 11289600;
> > + break;
> > + case 8000:
> > + case 16000:
> > + case 32000:
> > + case 48000:
> > + case 64000:
> > + case 96000:
> > + bitclock = 12288000;
> > + break;
>
> These seem remarkably high bitclocks for most of the sample rates -
> even for 88.2k and 96k these are 128fs. Some documentation as to what's
> going on here would be useful.
That's true. I just took these clock values from the original driver by
Scott/Vijay, but I should investigate some more. It's possible those clocks
were meant for the pll_a output, and should be further divided for the I2S
module input, to exactly the minimums you mentioned elsewhere.
I'm unsure what the requirements are for the "audio" clock either; e.g.
whether it should be identical to the I2S bit clock, simply larger than
either I2S bit clock, etc. Similar sharing issues apply as with the pll_a
itself discussed above.
One note though: I thought I remembered the WM8903 datasheet recommending
using a high rate for the bitclk for best performance of the codec e.g.
256fs. Perhaps that was for MCLK and not BCLK?
--
nvpublic
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
