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...
- 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.
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.
- 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.