Re: [PATCH 1/1] [ALSA]TLV320AIC23B: Support more sample rates

[Mark Brown <broonie@sirena.org.uk>]

On Fri, Oct 31, 2008 at 02:20:34PM -0700, Troy Kisky wrote:
> Add support for more sample rates, different crystals
> and split playback/capture rates.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> Acked-by: Arun KS <arunks@mistralsolutions.com>

This looks good, thanks - it's great to see more drivers moving away
from fixed tables for clock sources.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

but a few minor coding standard things:

> +	for (i = 0; i < 4; i++) {
> +		int base = mclk / bosr_usb_divisor_table[i];
> +		int mask = sr_valid_mask[i];

...

> +		for (j = 0; j < 16; j++, mask >>= 1) {

...

> +			adc = base * sr_adc_mult_table[j];
> +			dac = base * sr_dac_mult_table[j];

It'd be better if these loops used ARRAY_SIZE() for the limits rather
than having magic numbers.

> +static int set_sample_rate_control(struct snd_soc_codec *codec, int mclk,
> +		u32 sample_rate_adc, u32 sample_rate_dac)
> +{
> +	/* Search for the right sample rate */
> +	int data = find_rate(mclk, sample_rate_adc, sample_rate_dac);
> +	if (data < 0) {
> +		printk(KERN_ERR "%s:Invalid rate %u,%u requested\n",
> +				__func__, sample_rate_adc, sample_rate_dac);
> +		return -EINVAL;
> +	}
> +	tlv320aic23_write(codec, TLV320AIC23_SRATE, data);
> +	if (1) {
> +		int adc, dac;
> +		get_current_sample_rates(codec, mclk, &adc, &dac);
> +		printk(KERN_DEBUG "actual samplerate = %u,%u reg=%x\n",
> +			adc, dac, data);
> +	}

There's no need for the if () statement here - you can just open a new
block.  I guess this is supposed to be for debug purposes only?  If so
then it'd be better to surround it in #ifdef DEBUG.