* [PATCH] sound/sgtl5000: fix codec register initial values and mask
@ 2013-06-11 8:12 Oskar Schirmer
2013-06-11 12:20 ` Fabio Estevam
0 siblings, 1 reply; 7+ messages in thread
From: Oskar Schirmer @ 2013-06-11 8:12 UTC (permalink / raw)
To: Liam Girdwood
Cc: Andrew Morton, linux-kernel, Zeng Zhaoming, alsa-devel,
Takashi Iwai, Jaroslav Kysela, Fabio Estevam, Wolfram Sang,
Oskar Schirmer
According to documentation bit 3:2 in register SSS_CTRL are
reserved and zero, so initially setting the register to 0x0008
does not make much sense. Instead, bit 4 should be marked set,
as this is the power up default.
Further, mask computation in declarative part is obviously wrong:
Fix FRAC DIVISOR to provide an 11 bit mask correctly.
Signed-off-by: Oskar Schirmer <oskar@scara.com>
---
sound/soc/codecs/sgtl5000.c | 2 +-
sound/soc/codecs/sgtl5000.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index 92bbfec..ea47938 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -37,7 +37,7 @@
static const u16 sgtl5000_regs[SGTL5000_MAX_REG_OFFSET] = {
[SGTL5000_CHIP_CLK_CTRL] = 0x0008,
[SGTL5000_CHIP_I2S_CTRL] = 0x0010,
- [SGTL5000_CHIP_SSS_CTRL] = 0x0008,
+ [SGTL5000_CHIP_SSS_CTRL] = 0x0010,
[SGTL5000_CHIP_DAC_VOL] = 0x3c3c,
[SGTL5000_CHIP_PAD_STRENGTH] = 0x015f,
[SGTL5000_CHIP_ANA_HP_CTRL] = 0x1818,
diff --git a/sound/soc/codecs/sgtl5000.h b/sound/soc/codecs/sgtl5000.h
index 8a9f435..d3a68bb 100644
--- a/sound/soc/codecs/sgtl5000.h
+++ b/sound/soc/codecs/sgtl5000.h
@@ -347,7 +347,7 @@
#define SGTL5000_PLL_INT_DIV_MASK 0xf800
#define SGTL5000_PLL_INT_DIV_SHIFT 11
#define SGTL5000_PLL_INT_DIV_WIDTH 5
-#define SGTL5000_PLL_FRAC_DIV_MASK 0x0700
+#define SGTL5000_PLL_FRAC_DIV_MASK 0x07ff
#define SGTL5000_PLL_FRAC_DIV_SHIFT 0
#define SGTL5000_PLL_FRAC_DIV_WIDTH 11
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] sound/sgtl5000: fix codec register initial values and mask
2013-06-11 8:12 [PATCH] sound/sgtl5000: fix codec register initial values and mask Oskar Schirmer
@ 2013-06-11 12:20 ` Fabio Estevam
2013-06-11 15:22 ` [PATCH v2] " Oskar Schirmer
0 siblings, 1 reply; 7+ messages in thread
From: Fabio Estevam @ 2013-06-11 12:20 UTC (permalink / raw)
To: Oskar Schirmer
Cc: Liam Girdwood, Andrew Morton, linux-kernel, Zeng Zhaoming,
alsa-devel, Takashi Iwai, Jaroslav Kysela, Wolfram Sang
Hi Oskar,
On 06/11/2013 05:12 AM, Oskar Schirmer wrote:
> According to documentation bit 3:2 in register SSS_CTRL are
> reserved and zero, so initially setting the register to 0x0008
> does not make much sense. Instead, bit 4 should be marked set,
> as this is the power up default.
>
> Further, mask computation in declarative part is obviously wrong:
> Fix FRAC DIVISOR to provide an 11 bit mask correctly.
>
> Signed-off-by: Oskar Schirmer <oskar@scara.com>
Please rebase your patch against Mark Brown's topic/sgtl5000 branch:
https://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/log/?h=topic/sgtl5000
After that, you can add my:
Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] sound/sgtl5000: fix codec register initial values and mask
2013-06-11 12:20 ` Fabio Estevam
@ 2013-06-11 15:22 ` Oskar Schirmer
2013-06-11 16:01 ` Fabio Estevam
0 siblings, 1 reply; 7+ messages in thread
From: Oskar Schirmer @ 2013-06-11 15:22 UTC (permalink / raw)
To: Liam Girdwood
Cc: Andrew Morton, linux-kernel, Zeng Zhaoming, alsa-devel,
Takashi Iwai, Jaroslav Kysela, Fabio Estevam, Wolfram Sang,
Oskar Schirmer
According to documentation bit 3:2 in register SSS_CTRL are
reserved and zero, so initially setting the register to 0x0008
does not make much sense. Instead, bit 4 should be marked set,
as this is the power up default.
Further, mask computation in declarative part is obviously wrong:
Fix FRAC DIVISOR to provide an 11 bit mask correctly.
Signed-off-by: Oskar Schirmer <oskar@scara.com>
Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
---
sound/soc/codecs/sgtl5000.c | 2 +-
sound/soc/codecs/sgtl5000.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index d441559..10092ba 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -38,7 +38,7 @@
static const struct reg_default sgtl5000_reg_defaults[] = {
{ SGTL5000_CHIP_CLK_CTRL, 0x0008 },
{ SGTL5000_CHIP_I2S_CTRL, 0x0010 },
- { SGTL5000_CHIP_SSS_CTRL, 0x0008 },
+ { SGTL5000_CHIP_SSS_CTRL, 0x0010 },
{ SGTL5000_CHIP_DAC_VOL, 0x3c3c },
{ SGTL5000_CHIP_PAD_STRENGTH, 0x015f },
{ SGTL5000_CHIP_ANA_HP_CTRL, 0x1818 },
diff --git a/sound/soc/codecs/sgtl5000.h b/sound/soc/codecs/sgtl5000.h
index 4b69229..52bd843 100644
--- a/sound/soc/codecs/sgtl5000.h
+++ b/sound/soc/codecs/sgtl5000.h
@@ -347,7 +347,7 @@
#define SGTL5000_PLL_INT_DIV_MASK 0xf800
#define SGTL5000_PLL_INT_DIV_SHIFT 11
#define SGTL5000_PLL_INT_DIV_WIDTH 5
-#define SGTL5000_PLL_FRAC_DIV_MASK 0x0700
+#define SGTL5000_PLL_FRAC_DIV_MASK 0x07ff
#define SGTL5000_PLL_FRAC_DIV_SHIFT 0
#define SGTL5000_PLL_FRAC_DIV_WIDTH 11
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] sound/sgtl5000: fix codec register initial values and mask
2013-06-11 15:22 ` [PATCH v2] " Oskar Schirmer
@ 2013-06-11 16:01 ` Fabio Estevam
2013-06-12 16:08 ` Mark Brown
2013-06-19 13:16 ` [PATCH v3] sound/sgtl5000: fix codec register initial values and handling Oskar Schirmer
0 siblings, 2 replies; 7+ messages in thread
From: Fabio Estevam @ 2013-06-11 16:01 UTC (permalink / raw)
To: Oskar Schirmer
Cc: Liam Girdwood, Andrew Morton, linux-kernel, Zeng Zhaoming,
alsa-devel, Takashi Iwai, Jaroslav Kysela, Mark Brown
On 06/11/2013 12:22 PM, Oskar Schirmer wrote:
> According to documentation bit 3:2 in register SSS_CTRL are
> reserved and zero, so initially setting the register to 0x0008
> does not make much sense. Instead, bit 4 should be marked set,
> as this is the power up default.
>
> Further, mask computation in declarative part is obviously wrong:
> Fix FRAC DIVISOR to provide an 11 bit mask correctly.
>
> Signed-off-by: Oskar Schirmer <oskar@scara.com>
> Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
Looks good, but please copy Mark Brown. Just added him in Cc now.
> ---
> sound/soc/codecs/sgtl5000.c | 2 +-
> sound/soc/codecs/sgtl5000.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
> index d441559..10092ba 100644
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -38,7 +38,7 @@
> static const struct reg_default sgtl5000_reg_defaults[] = {
> { SGTL5000_CHIP_CLK_CTRL, 0x0008 },
> { SGTL5000_CHIP_I2S_CTRL, 0x0010 },
> - { SGTL5000_CHIP_SSS_CTRL, 0x0008 },
> + { SGTL5000_CHIP_SSS_CTRL, 0x0010 },
> { SGTL5000_CHIP_DAC_VOL, 0x3c3c },
> { SGTL5000_CHIP_PAD_STRENGTH, 0x015f },
> { SGTL5000_CHIP_ANA_HP_CTRL, 0x1818 },
> diff --git a/sound/soc/codecs/sgtl5000.h b/sound/soc/codecs/sgtl5000.h
> index 4b69229..52bd843 100644
> --- a/sound/soc/codecs/sgtl5000.h
> +++ b/sound/soc/codecs/sgtl5000.h
> @@ -347,7 +347,7 @@
> #define SGTL5000_PLL_INT_DIV_MASK 0xf800
> #define SGTL5000_PLL_INT_DIV_SHIFT 11
> #define SGTL5000_PLL_INT_DIV_WIDTH 5
> -#define SGTL5000_PLL_FRAC_DIV_MASK 0x0700
> +#define SGTL5000_PLL_FRAC_DIV_MASK 0x07ff
> #define SGTL5000_PLL_FRAC_DIV_SHIFT 0
> #define SGTL5000_PLL_FRAC_DIV_WIDTH 11
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] sound/sgtl5000: fix codec register initial values and mask
2013-06-11 16:01 ` Fabio Estevam
@ 2013-06-12 16:08 ` Mark Brown
2013-06-19 13:16 ` [PATCH v3] sound/sgtl5000: fix codec register initial values and handling Oskar Schirmer
1 sibling, 0 replies; 7+ messages in thread
From: Mark Brown @ 2013-06-12 16:08 UTC (permalink / raw)
To: Fabio Estevam
Cc: Oskar Schirmer, Liam Girdwood, Andrew Morton, linux-kernel,
Zeng Zhaoming, alsa-devel, Takashi Iwai, Jaroslav Kysela
[-- Attachment #1: Type: text/plain, Size: 814 bytes --]
On Tue, Jun 11, 2013 at 01:01:57PM -0300, Fabio Estevam wrote:
> On 06/11/2013 12:22 PM, Oskar Schirmer wrote:
> >According to documentation bit 3:2 in register SSS_CTRL are
> >reserved and zero, so initially setting the register to 0x0008
> >does not make much sense. Instead, bit 4 should be marked set,
> >as this is the power up default.
> >
> >Further, mask computation in declarative part is obviously wrong:
> >Fix FRAC DIVISOR to provide an 11 bit mask correctly.
> >
> >Signed-off-by: Oskar Schirmer <oskar@scara.com>
> >Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
> Looks good, but please copy Mark Brown. Just added him in Cc now.
Please also use subject lines matching the normal style for the
subsystem. I can't apply this unless someone sends me a copy properly
(not quoted and so on).
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3] sound/sgtl5000: fix codec register initial values and handling
2013-06-11 16:01 ` Fabio Estevam
2013-06-12 16:08 ` Mark Brown
@ 2013-06-19 13:16 ` Oskar Schirmer
2013-06-19 15:26 ` Mark Brown
1 sibling, 1 reply; 7+ messages in thread
From: Oskar Schirmer @ 2013-06-19 13:16 UTC (permalink / raw)
To: Fabio Estevam
Cc: Andrew Morton, linux-kernel, Zeng Zhaoming, alsa-devel,
Liam Girdwood, Takashi Iwai, Jaroslav Kysela, Wolfram Sang,
broonie, Oskar Schirmer
According to documentation bit 3:2 in register SSS_CTRL are
reserved and zero, so initially setting the register to 0x0008
does not make much sense. Instead, bit 4 should be marked set,
as this is the power up default.
Further, mask computation in declarative part is obviously wrong:
Fix FRAC DIVISOR to provide an 11 bit mask correctly.
Next, powering down PLL before switching to a mode that
does not use it, is a bad idea. So first set the mode control,
then power down PLL.
Signed-off-by: Oskar Schirmer <oskar@scara.com>
---
sound/soc/codecs/sgtl5000.c | 11 +++++++----
sound/soc/codecs/sgtl5000.h | 2 +-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index 92bbfec..6162e19 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -38,7 +38,7 @@
static const struct reg_default sgtl5000_reg_defaults[] = {
{ SGTL5000_CHIP_CLK_CTRL, 0x0008 },
{ SGTL5000_CHIP_I2S_CTRL, 0x0010 },
- { SGTL5000_CHIP_SSS_CTRL, 0x0008 },
+ { SGTL5000_CHIP_SSS_CTRL, 0x0010 },
{ SGTL5000_CHIP_DAC_VOL, 0x3c3c },
{ SGTL5000_CHIP_PAD_STRENGTH, 0x015f },
{ SGTL5000_CHIP_ANA_HP_CTRL, 0x1818 },
@@ -644,16 +644,19 @@ static int sgtl5000_set_clock(struct snd_soc_codec *codec, int frame_rate)
snd_soc_update_bits(codec, SGTL5000_CHIP_ANA_POWER,
SGTL5000_PLL_POWERUP | SGTL5000_VCOAMP_POWERUP,
SGTL5000_PLL_POWERUP | SGTL5000_VCOAMP_POWERUP);
+
+ /* if using pll, clk_ctrl must be set after pll power up */
+ snd_soc_write(codec, SGTL5000_CHIP_CLK_CTRL, clk_ctl);
} else {
+ /* otherwise, clk_ctrl must be set before pll power down */
+ snd_soc_write(codec, SGTL5000_CHIP_CLK_CTRL, clk_ctl);
+
/* power down pll */
snd_soc_update_bits(codec, SGTL5000_CHIP_ANA_POWER,
SGTL5000_PLL_POWERUP | SGTL5000_VCOAMP_POWERUP,
0);
}
- /* if using pll, clk_ctrl must be set after pll power up */
- snd_soc_write(codec, SGTL5000_CHIP_CLK_CTRL, clk_ctl);
-
return 0;
}
diff --git a/sound/soc/codecs/sgtl5000.h b/sound/soc/codecs/sgtl5000.h
index 8a9f435..d3a68bb 100644
--- a/sound/soc/codecs/sgtl5000.h
+++ b/sound/soc/codecs/sgtl5000.h
@@ -347,7 +347,7 @@
#define SGTL5000_PLL_INT_DIV_MASK 0xf800
#define SGTL5000_PLL_INT_DIV_SHIFT 11
#define SGTL5000_PLL_INT_DIV_WIDTH 5
-#define SGTL5000_PLL_FRAC_DIV_MASK 0x0700
+#define SGTL5000_PLL_FRAC_DIV_MASK 0x07ff
#define SGTL5000_PLL_FRAC_DIV_SHIFT 0
#define SGTL5000_PLL_FRAC_DIV_WIDTH 11
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] sound/sgtl5000: fix codec register initial values and handling
2013-06-19 13:16 ` [PATCH v3] sound/sgtl5000: fix codec register initial values and handling Oskar Schirmer
@ 2013-06-19 15:26 ` Mark Brown
0 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2013-06-19 15:26 UTC (permalink / raw)
To: Oskar Schirmer
Cc: Fabio Estevam, Andrew Morton, linux-kernel, Zeng Zhaoming,
alsa-devel, Liam Girdwood, Takashi Iwai, Jaroslav Kysela,
Wolfram Sang
[-- Attachment #1: Type: text/plain, Size: 525 bytes --]
On Wed, Jun 19, 2013 at 01:16:06PM +0000, Oskar Schirmer wrote:
This is a set of three unrelated changes, they should be sent as three
separate patches.
> Further, mask computation in declarative part is obviously wrong:
> Fix FRAC DIVISOR to provide an 11 bit mask correctly.
What makes you say that this is obviously wrong? It's not entirely
clear.
> -#define SGTL5000_PLL_FRAC_DIV_MASK 0x0700
> +#define SGTL5000_PLL_FRAC_DIV_MASK 0x07ff
Based on your changelog I was expecting a mask with only two bits set
here?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-06-19 15:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-11 8:12 [PATCH] sound/sgtl5000: fix codec register initial values and mask Oskar Schirmer
2013-06-11 12:20 ` Fabio Estevam
2013-06-11 15:22 ` [PATCH v2] " Oskar Schirmer
2013-06-11 16:01 ` Fabio Estevam
2013-06-12 16:08 ` Mark Brown
2013-06-19 13:16 ` [PATCH v3] sound/sgtl5000: fix codec register initial values and handling Oskar Schirmer
2013-06-19 15:26 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).