On Wed, 2012-02-01 at 11:50 +0000, Mark Brown wrote:
On Wed, Feb 01, 2012 at 04:55:29PM +0530, Ashish Chavan wrote:
+static const u32 da7210_pll_div[][COL_CNT] = {
- /* for MASTER mode, fs = 44.1Khz */
- { 12000000, 0xE8, 0x6C, 0x2, }, /* MCLK=12Mhz */
Even better than defines to index into the array would be an array of structs with named members...
OK, will do that.
- if (da7210->mclk_rate) {
/* PLL mode, disable PLL bypass */
snd_soc_update_bits(codec, DA7210_PLL_DIV3, DA7210_PLL_BYP, 0);
- } else {
/* PLL bypass mode, enable PLL bypass */
snd_soc_update_bits(codec, DA7210_PLL_DIV3, DA7210_PLL_BYP,
DA7210_PLL_BYP);
- }
This is really weird - if anything it looks the wrong way round. mclk_rate is what's set by set_sysclk() so if the user has configured MCLK we will never use it even if it's at a suitable rate but if the user hasn't configured one we rely on it directly. If anything I'd expect this code to enable the PLL only if the MCLK is not a suitable rate.
Actually it worked fine for me because of the way platform driver is written. It basically uses static configurations for PLL and PLL bypass modes. It calls set_sysclk() only in case of PLL. So the codec driver more or less uses mckl_rate as a flag to see if PLL is enabled. I know it's not clean and may be I can update it to comply with rest of the stuff.
The normal pattern is that set_sysclk() specifies the clock the bulk of the device will be using, when used with the PLL that'd be the PLL output not the PLL input. Alternatively just specify the MCLK always and let the driver figure out when to use the PLL.
Yes, got the point.
- if (da7210->master) {
/* In PLL master mode, use master mode PLL dividers */
switch (fout) {
case 2822400:
row_idx = MASTER_2822400_DIV_OFFSET;
break;
case 3072000:
row_idx = MASTER_3072000_DIV_OFFSET;
break;
default:
dev_err(codec_dai->dev,
"Unsupported PLL output frequency %d\n", fout);
return -EINVAL;
}
- } else {
/* In PLL slave mode, use SRM mode PLL dividers */
row_idx = SLAVE_SRM_DIV_OFFSET;
- }
You need checks elsewhere to make sure that the user doesn't try to reconfigure master/slave while the PLL is active.
AFAIK master/slave configuration is static(depending on the board configuration) and done via set_fmt() from hw_params() of platform driver. Can you please point me to an example where dynamic/runtime setting of I2S master/slave mode is supported?