[alsa-devel] [PATCH] ASoC: da7210: Add support for PLL and SRM

[Mark Brown]

On Thu, Jan 12, 2012 at 05:54:21PM +0530, Ashish Chavan wrote:

> Hi,

Remember this is a changelog...

> +/**
> + * da7210_set_dai_clkdiv: Set codec input clock divider
> + * @param codec_dai	: pointer to codec DAI
> + * @param div_id	: DA7210 has only one clk_div, so div_id is always zero
> + * @param div		: Divider value (configured via control reg MCLK_RANGE)
> + * @return int		: Zero for success, negative error code for error
> + *
> + */
> +static int da7210_set_dai_clkdiv(struct snd_soc_dai *codec_dai,
> +				 int div_id, int div)

Why does the driver need the machine driver to manually configure clock
dividers?

> +	switch (fref) {
> +	case 12000000:
> +		mclk_offset = 0;
> +		break;

> +	switch (fout) {
> +	case 2822400:
> +		fsdm_offset = 0;
> +		break;

> +	if (da7210->master) {
> +		/* In PLL master mode, use Master mode PLL dividers */
> +		pll_div_offset = (mclk_offset * 2 + fsdm_offset) * 3;
> +		pll_div1 = da7210_pll_master_div[pll_div_offset];
> +		pll_div2 = da7210_pll_master_div[pll_div_offset + 1];
> +		pll_div3 = da7210_pll_master_div[pll_div_offset + 2];

This isn't great, you're indexing into a table of divisors using raw
numeric constants in a totally separate part of the code.  Worse, these
constants aren't even directly used but have a calculation applied to 
them apparently because this is really a multi dimensional array.  This
isn't great for either legibility or robustness.

Fix this to remove the use of magic numbers, for example by putting
fout and fref into the table and searching for them.