On Fri, Apr 18, 2008 at 10:37:36PM +0300, Jarkko Nikula wrote:
I'm might be completely wrong here since I've used this codec only in configuration where the interface rate or FSref equals to audio sampling rate , but as far as I've interpreted the spec correctly we can have e.g. the case where MCLK is 22.5792 MHz, FSref 44.1 kHz (i.e. PLL can be disabled) but audio sampling rate is 11.025 kHz (== params_rate(params)).
Of course. Fsref is a setting for the filter to tell it which internal base-frequency it is operating on. But the setup of the sampling frequency has different meaning. Also, the clocking diagram (page 27 in the aic33 manual) doesn't even show that entity, or did I miss anything after staring at this for some days? ;)
But I don't want to speculate or do my own interpretation here. I'll ask this from TI next week from my jarkko.nikula@nokia.com and can put you as a cc as well.
Ok, thanks :)
Your latest patch has:
- if (bypass_pll)
return 0;
Yes, since all other setup can be skipped in case the PLL is bypassed. Did you miss the question here? ;)
Again, I'm might be wrong but I NDAC and NADC refers to register AIC3X_SAMPLE_RATE_SEL_REG which driver already takes into account properly.
Only in case the PLL is enabled. I agree that there could be some more setup around all the possible settings in this clock generation, but I'd my approach as a good first start.
And don't take my comments as a nitpicking. Definitely patch like this is worth to get in but we have to make sure that no possible bugs are introduced. -EINVAL is much better alternative for some new configurations.
No, I don't take it as nitpicking :) I agree that a driver for such a complex chip has to be modified with care to have any regression. But I don't see such as far, so I'd take this patch as a step towards a better solution. Unless someone comes up claiming that it's breaking anything.
Thanks and best rgeards, Daniel