On Fri, Dec 17, 2010 at 10:16:16PM -0800, Stephen Warren wrote: > Mark Brown wrote: > > 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 > 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. If nothing else it should be possible to have a function the machine drivers can call to do all this stuff rather than having to replicate the code. > 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. Ah, right - the two interfaces have to be synchronous. That does limit things rather. I take it this restriction applies even if the Tegra is not clock master on the I2S interface? What you should also do here is when one of the interfaces is active set constraints on the other interface in its startup() so userspace knows it can't select a sample rate that's not supported by the currently active clock rate. > 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. Like I said above factoring out the actual mechanics of the clock programming would still be good - if it could say "configure for 8kHz based rates" rather than having to fiddle with the clock tree directly that'd still be a win. Probably a default call which will flip between the two clock bases for use on systems with only a single I2S link in use would be handy too. > 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? That'd probably be too restrictive - it'd mean that if you only have one of the ports in use (like on Harmoney) you couldn't switch between the two different clock ratios. > 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? That's a requirement for MCLK rather than BCLK. BCLK just needs to be high enough to clock through the data needed to transfer the samples and smaller than whatever the absolute maximum specified in the datasheet is. The 256fs MCLK requirement is standard for audio parts, some DSP based parts actually need higher ratios such as 512fs. -- 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