Re: [PATCH] ASoC: tlv320aic3x: add input clock selection

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1.1: Type: text/plain, Size: 838 bytes --]

On Tue, Jun 26, 2012 at 12:21:28PM +0200, Prchal Jiří wrote:

> +	/* set clock on MCLK or GPIO2 or BCLK */
> +	snd_soc_update_bits(codec, AIC3X_CLKGEN_CTRL_REG, PLLCLK_IN_MASK, clk_id);
> +	snd_soc_update_bits(codec, AIC3X_CLKGEN_CTRL_REG, CLKDIV_IN_MASK, clk_id);

Normally it's possible to set these separately.  Is there a reason why
they have to be the same (and if so why has the chip got separate
registers)?

> +#define PLLCLK_IN_MASK		0x30
> +#define CLKDIV_IN_MASK		0xc0
> +/* clock in source */
> +#define CLKIN_MCLK		0
> +#define CLKIN_GPIO2		1
> +#define CLKIN_BCLK		2

This doesn't look right - you use the clock source values directly above
but they need shifting to be used as if they're used directly they'll
always come out as zero.  It'd also be better to have some bounds
checking on the values.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]