* [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).