[alsa-devel] [PATCH 1/2] ASoC: sgtl5000: Fix wrong register name in restore

[Dong Aisheng-B29396]

> -----Original Message-----
> From: zengzm.kernel at gmail.com [mailto:zengzm.kernel at gmail.com]
> Sent: Monday, January 16, 2012 3:16 PM
> To: alsa-devel at alsa-project.org
> Cc: Dong Aisheng-B29396; broonie at opensource.wolfsonmicro.com;
> julia.lawall at lip6.fr; festevam at gmail.com; w.sang at pengutronix.de;
> shawn.guo at linaro.org; lrg at ti.com; zengzm.kernel at gmail.com
> Subject: [PATCH 1/2] ASoC: sgtl5000: Fix wrong register name in restore
> Importance: High
> 
> From: Zeng Zhaoming <zengzm.kernel at gmail.com>
> 
> Correct SGTL5000_CHIP_CLK_CTRL to SGTL5000_CHIP_REF_CTRL in
> sgtl5000_restore_regs(), and add comment to explain the restore order.
> 
The change is ok to me.
One minor thing:

> Signed-off-by: Zeng Zhaoming <zengzm.kernel at gmail.com>
> ---
>  sound/soc/codecs/sgtl5000.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c index
> fc9b127..71df1fe 100644
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -987,12 +987,12 @@ static int sgtl5000_restore_regs(struct snd_soc_codec
> *codec)
>  	/* restore regular registers */
>  	for (reg = 0; reg <= SGTL5000_CHIP_SHORT_CTRL; reg += 2) {
> 
> -		/* this regs depends on the others */
> +		/* These regs should restore in particular order */
>  		if (reg == SGTL5000_CHIP_ANA_POWER ||
>  			reg == SGTL5000_CHIP_CLK_CTRL ||
>  			reg == SGTL5000_CHIP_LINREG_CTRL ||
>  			reg == SGTL5000_CHIP_LINE_OUT_CTRL ||
> -			reg == SGTL5000_CHIP_CLK_CTRL)
> +			reg == SGTL5000_CHIP_REF_CTRL)
>  			continue;
> 
>  		snd_soc_write(codec, reg, cache[reg]); @@ -1005,6 +1005,13 @@
> static int sgtl5000_restore_regs(struct snd_soc_codec *codec)
>  	/*
>  	 * restore power and other regs according
>  	 * to set_power() and set_clock()
There's no such functions in this driver.
Can we change the comment to something like:
"restore power and other regs according to the power and clock setting sequence".
That's easier to understand.

> +	 * the order of restore is:
> +	 * 1. SGTL5000_CHIP_CLK_CTRL MCLK_FREQ bits (1:0) should be restore after
> +	 *    SGTL5000_CHIP_ANA_POWER PLL bits set
> +	 * 2. SGTL5000_CHIP_LINREG_CTRL should be set before
> +	 *    SGTL5000_CHIP_ANA_POWER LINREG_D restored
> +	 * 3. SGTL5000_CHIP_REF_CTRL controls Analog Ground Voltage,
> +	 *    I prefer to resotre it after SGTL5000_CHIP_ANA_POWER restored
>  	 */
>  	snd_soc_write(codec, SGTL5000_CHIP_LINREG_CTRL,
>  			cache[SGTL5000_CHIP_LINREG_CTRL]);
> --
> 1.7.6.1.385.gb7fcd0
> 

Regards
Dong Aisheng