* [PATCH 1/2] ASoC: tas2552: Fix PM sequencing @ 2014-07-18 17:31 ` Dan Murphy 0 siblings, 0 replies; 16+ messages in thread From: Dan Murphy @ 2014-07-18 17:31 UTC (permalink / raw) To: linux-sound; +Cc: linux-kernel, alsa-devel, broonie, Dan Murphy In the pm suspend/resume it is better to disable the GPIO after the regmap_cache setting calls so that if the call is interrupted the new reg values will be cached and set on resume. Also add pm_runtime_put in the remove call. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- sound/soc/codecs/tas2552.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c index f0760af..a3ae394 100644 --- a/sound/soc/codecs/tas2552.c +++ b/sound/soc/codecs/tas2552.c @@ -239,12 +239,12 @@ static int tas2552_runtime_suspend(struct device *dev) tas2552_sw_shutdown(tas2552, 0); - if (tas2552->enable_gpio) - gpiod_set_value(tas2552->enable_gpio, 0); - regcache_cache_only(tas2552->regmap, true); regcache_mark_dirty(tas2552->regmap); + if (tas2552->enable_gpio) + gpiod_set_value(tas2552->enable_gpio, 0); + return 0; } @@ -382,6 +382,8 @@ static int tas2552_codec_remove(struct snd_soc_codec *codec) { struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec); + pm_runtime_put(codec->dev); + if (tas2552->enable_gpio) gpiod_set_value(tas2552->enable_gpio, 0); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/2] ASoC: tas2552: Fix PM sequencing @ 2014-07-18 17:31 ` Dan Murphy 0 siblings, 0 replies; 16+ messages in thread From: Dan Murphy @ 2014-07-18 17:31 UTC (permalink / raw) To: linux-sound; +Cc: linux-kernel, alsa-devel, broonie, Dan Murphy In the pm suspend/resume it is better to disable the GPIO after the regmap_cache setting calls so that if the call is interrupted the new reg values will be cached and set on resume. Also add pm_runtime_put in the remove call. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- sound/soc/codecs/tas2552.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c index f0760af..a3ae394 100644 --- a/sound/soc/codecs/tas2552.c +++ b/sound/soc/codecs/tas2552.c @@ -239,12 +239,12 @@ static int tas2552_runtime_suspend(struct device *dev) tas2552_sw_shutdown(tas2552, 0); - if (tas2552->enable_gpio) - gpiod_set_value(tas2552->enable_gpio, 0); - regcache_cache_only(tas2552->regmap, true); regcache_mark_dirty(tas2552->regmap); + if (tas2552->enable_gpio) + gpiod_set_value(tas2552->enable_gpio, 0); + return 0; } @@ -382,6 +382,8 @@ static int tas2552_codec_remove(struct snd_soc_codec *codec) { struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec); + pm_runtime_put(codec->dev); + if (tas2552->enable_gpio) gpiod_set_value(tas2552->enable_gpio, 0); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/2] ASoC: tas2552: Fix PM sequencing @ 2014-07-18 17:31 ` Dan Murphy 0 siblings, 0 replies; 16+ messages in thread From: Dan Murphy @ 2014-07-18 17:31 UTC (permalink / raw) To: linux-sound; +Cc: linux-kernel, alsa-devel, broonie, Dan Murphy In the pm suspend/resume it is better to disable the GPIO after the regmap_cache setting calls so that if the call is interrupted the new reg values will be cached and set on resume. Also add pm_runtime_put in the remove call. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- sound/soc/codecs/tas2552.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c index f0760af..a3ae394 100644 --- a/sound/soc/codecs/tas2552.c +++ b/sound/soc/codecs/tas2552.c @@ -239,12 +239,12 @@ static int tas2552_runtime_suspend(struct device *dev) tas2552_sw_shutdown(tas2552, 0); - if (tas2552->enable_gpio) - gpiod_set_value(tas2552->enable_gpio, 0); - regcache_cache_only(tas2552->regmap, true); regcache_mark_dirty(tas2552->regmap); + if (tas2552->enable_gpio) + gpiod_set_value(tas2552->enable_gpio, 0); + return 0; } @@ -382,6 +382,8 @@ static int tas2552_codec_remove(struct snd_soc_codec *codec) { struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec); + pm_runtime_put(codec->dev); + if (tas2552->enable_gpio) gpiod_set_value(tas2552->enable_gpio, 0); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL 2014-07-18 17:31 ` Dan Murphy (?) @ 2014-07-18 17:31 ` Dan Murphy -1 siblings, 0 replies; 16+ messages in thread From: Dan Murphy @ 2014-07-18 17:31 UTC (permalink / raw) To: linux-sound; +Cc: linux-kernel, alsa-devel, broonie, Dan Murphy Add DAPM calls to enable/disable the Class D amp. Also add a DAPM call to turn off the PLL upon the stream completing. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- sound/soc/codecs/tas2552.c | 58 +++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c index a3ae394..3fdb173 100644 --- a/sound/soc/codecs/tas2552.c +++ b/sound/soc/codecs/tas2552.c @@ -78,6 +78,40 @@ struct tas2552_data { unsigned int mclk; }; + +static int tas2552_pll_disable(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + if (event == SND_SOC_DAPM_POST_PMD) + snd_soc_update_bits(w->codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0); + + return 0; +} + +static int tas2552_class_d_en(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + switch (event) { + case SND_SOC_DAPM_PRE_PMU: + snd_soc_update_bits(w->codec, TAS2552_CFG_2, + TAS2552_CLASSD_EN_MASK, TAS2552_CLASSD_EN_MASK); + break; + case SND_SOC_DAPM_POST_PMD: + snd_soc_update_bits(w->codec, TAS2552_CFG_2, + TAS2552_CLASSD_EN_MASK, 0); + break; + } + + return 0; +} + +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = +{ +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en), +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en), +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable), +}; + static void tas2552_sw_shutdown(struct tas2552_data *tas_data, int sw_shutdown) { u8 cfg1_reg; @@ -101,10 +135,6 @@ static int tas2552_hw_params(struct snd_pcm_substream *substream, int d; u8 p, j; - /* Turn on Class D amplifier */ - snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK, - TAS2552_CLASSD_EN); - if (!tas2552->mclk) return -EINVAL; @@ -149,7 +179,6 @@ static int tas2552_hw_params(struct snd_pcm_substream *substream, snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, TAS2552_PLL_ENABLE); - return 0; } @@ -269,19 +298,10 @@ static const struct dev_pm_ops tas2552_pm = { NULL) }; -static void tas2552_shutdown(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct snd_soc_codec *codec = dai->codec; - - snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0); -} - static struct snd_soc_dai_ops tas2552_speaker_dai_ops = { .hw_params = tas2552_hw_params, .set_sysclk = tas2552_set_dai_sysclk, .set_fmt = tas2552_set_dai_fmt, - .shutdown = tas2552_shutdown, .digital_mute = tas2552_mute, }; @@ -321,6 +341,7 @@ static const struct reg_default tas2552_init_regs[] = { static int tas2552_codec_probe(struct snd_soc_codec *codec) { struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec); + struct snd_soc_dapm_context *dapm = &codec->dapm; int ret; tas2552->codec = codec; @@ -362,9 +383,12 @@ static int tas2552_codec_probe(struct snd_soc_codec *codec) goto patch_fail; } - snd_soc_write(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN | - TAS2552_BOOST_EN | TAS2552_APT_EN | - TAS2552_LIM_EN); + snd_soc_write(codec, TAS2552_CFG_2, TAS2552_BOOST_EN | + TAS2552_APT_EN | TAS2552_LIM_EN); + + snd_soc_dapm_new_controls(dapm, tas2552_dapm_widgets, + ARRAY_SIZE(tas2552_dapm_widgets)); + return 0; patch_fail: -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL @ 2014-07-18 17:31 ` Dan Murphy 0 siblings, 0 replies; 16+ messages in thread From: Dan Murphy @ 2014-07-18 17:31 UTC (permalink / raw) To: linux-sound; +Cc: linux-kernel, alsa-devel, broonie, Dan Murphy Add DAPM calls to enable/disable the Class D amp. Also add a DAPM call to turn off the PLL upon the stream completing. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- sound/soc/codecs/tas2552.c | 58 +++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c index a3ae394..3fdb173 100644 --- a/sound/soc/codecs/tas2552.c +++ b/sound/soc/codecs/tas2552.c @@ -78,6 +78,40 @@ struct tas2552_data { unsigned int mclk; }; + +static int tas2552_pll_disable(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + if (event = SND_SOC_DAPM_POST_PMD) + snd_soc_update_bits(w->codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0); + + return 0; +} + +static int tas2552_class_d_en(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + switch (event) { + case SND_SOC_DAPM_PRE_PMU: + snd_soc_update_bits(w->codec, TAS2552_CFG_2, + TAS2552_CLASSD_EN_MASK, TAS2552_CLASSD_EN_MASK); + break; + case SND_SOC_DAPM_POST_PMD: + snd_soc_update_bits(w->codec, TAS2552_CFG_2, + TAS2552_CLASSD_EN_MASK, 0); + break; + } + + return 0; +} + +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] +{ +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en), +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en), +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable), +}; + static void tas2552_sw_shutdown(struct tas2552_data *tas_data, int sw_shutdown) { u8 cfg1_reg; @@ -101,10 +135,6 @@ static int tas2552_hw_params(struct snd_pcm_substream *substream, int d; u8 p, j; - /* Turn on Class D amplifier */ - snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK, - TAS2552_CLASSD_EN); - if (!tas2552->mclk) return -EINVAL; @@ -149,7 +179,6 @@ static int tas2552_hw_params(struct snd_pcm_substream *substream, snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, TAS2552_PLL_ENABLE); - return 0; } @@ -269,19 +298,10 @@ static const struct dev_pm_ops tas2552_pm = { NULL) }; -static void tas2552_shutdown(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct snd_soc_codec *codec = dai->codec; - - snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0); -} - static struct snd_soc_dai_ops tas2552_speaker_dai_ops = { .hw_params = tas2552_hw_params, .set_sysclk = tas2552_set_dai_sysclk, .set_fmt = tas2552_set_dai_fmt, - .shutdown = tas2552_shutdown, .digital_mute = tas2552_mute, }; @@ -321,6 +341,7 @@ static const struct reg_default tas2552_init_regs[] = { static int tas2552_codec_probe(struct snd_soc_codec *codec) { struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec); + struct snd_soc_dapm_context *dapm = &codec->dapm; int ret; tas2552->codec = codec; @@ -362,9 +383,12 @@ static int tas2552_codec_probe(struct snd_soc_codec *codec) goto patch_fail; } - snd_soc_write(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN | - TAS2552_BOOST_EN | TAS2552_APT_EN | - TAS2552_LIM_EN); + snd_soc_write(codec, TAS2552_CFG_2, TAS2552_BOOST_EN | + TAS2552_APT_EN | TAS2552_LIM_EN); + + snd_soc_dapm_new_controls(dapm, tas2552_dapm_widgets, + ARRAY_SIZE(tas2552_dapm_widgets)); + return 0; patch_fail: -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL @ 2014-07-18 17:31 ` Dan Murphy 0 siblings, 0 replies; 16+ messages in thread From: Dan Murphy @ 2014-07-18 17:31 UTC (permalink / raw) To: linux-sound; +Cc: alsa-devel, broonie, linux-kernel, Dan Murphy Add DAPM calls to enable/disable the Class D amp. Also add a DAPM call to turn off the PLL upon the stream completing. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- sound/soc/codecs/tas2552.c | 58 +++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c index a3ae394..3fdb173 100644 --- a/sound/soc/codecs/tas2552.c +++ b/sound/soc/codecs/tas2552.c @@ -78,6 +78,40 @@ struct tas2552_data { unsigned int mclk; }; + +static int tas2552_pll_disable(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + if (event == SND_SOC_DAPM_POST_PMD) + snd_soc_update_bits(w->codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0); + + return 0; +} + +static int tas2552_class_d_en(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + switch (event) { + case SND_SOC_DAPM_PRE_PMU: + snd_soc_update_bits(w->codec, TAS2552_CFG_2, + TAS2552_CLASSD_EN_MASK, TAS2552_CLASSD_EN_MASK); + break; + case SND_SOC_DAPM_POST_PMD: + snd_soc_update_bits(w->codec, TAS2552_CFG_2, + TAS2552_CLASSD_EN_MASK, 0); + break; + } + + return 0; +} + +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = +{ +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en), +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en), +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable), +}; + static void tas2552_sw_shutdown(struct tas2552_data *tas_data, int sw_shutdown) { u8 cfg1_reg; @@ -101,10 +135,6 @@ static int tas2552_hw_params(struct snd_pcm_substream *substream, int d; u8 p, j; - /* Turn on Class D amplifier */ - snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN_MASK, - TAS2552_CLASSD_EN); - if (!tas2552->mclk) return -EINVAL; @@ -149,7 +179,6 @@ static int tas2552_hw_params(struct snd_pcm_substream *substream, snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, TAS2552_PLL_ENABLE); - return 0; } @@ -269,19 +298,10 @@ static const struct dev_pm_ops tas2552_pm = { NULL) }; -static void tas2552_shutdown(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct snd_soc_codec *codec = dai->codec; - - snd_soc_update_bits(codec, TAS2552_CFG_2, TAS2552_PLL_ENABLE, 0); -} - static struct snd_soc_dai_ops tas2552_speaker_dai_ops = { .hw_params = tas2552_hw_params, .set_sysclk = tas2552_set_dai_sysclk, .set_fmt = tas2552_set_dai_fmt, - .shutdown = tas2552_shutdown, .digital_mute = tas2552_mute, }; @@ -321,6 +341,7 @@ static const struct reg_default tas2552_init_regs[] = { static int tas2552_codec_probe(struct snd_soc_codec *codec) { struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec); + struct snd_soc_dapm_context *dapm = &codec->dapm; int ret; tas2552->codec = codec; @@ -362,9 +383,12 @@ static int tas2552_codec_probe(struct snd_soc_codec *codec) goto patch_fail; } - snd_soc_write(codec, TAS2552_CFG_2, TAS2552_CLASSD_EN | - TAS2552_BOOST_EN | TAS2552_APT_EN | - TAS2552_LIM_EN); + snd_soc_write(codec, TAS2552_CFG_2, TAS2552_BOOST_EN | + TAS2552_APT_EN | TAS2552_LIM_EN); + + snd_soc_dapm_new_controls(dapm, tas2552_dapm_widgets, + ARRAY_SIZE(tas2552_dapm_widgets)); + return 0; patch_fail: -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL 2014-07-18 17:31 ` Dan Murphy @ 2014-07-21 11:53 ` Mark Brown -1 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2014-07-21 11:53 UTC (permalink / raw) To: Dan Murphy; +Cc: linux-sound, linux-kernel, alsa-devel [-- Attachment #1: Type: text/plain, Size: 1054 bytes --] On Fri, Jul 18, 2014 at 12:31:08PM -0500, Dan Murphy wrote: > +static int tas2552_class_d_en(struct snd_soc_dapm_widget *w, > + struct snd_kcontrol *kcontrol, int event) > +{ > + switch (event) { > + case SND_SOC_DAPM_PRE_PMU: > + snd_soc_update_bits(w->codec, TAS2552_CFG_2, > + TAS2552_CLASSD_EN_MASK, TAS2552_CLASSD_EN_MASK); > + break; > + case SND_SOC_DAPM_POST_PMD: > + snd_soc_update_bits(w->codec, TAS2552_CFG_2, > + TAS2552_CLASSD_EN_MASK, 0); > + break; > + } > + > + return 0; > +} > + > +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = > +{ > +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en), > +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en), > +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable), > +}; This seems broken, having to use _PRE or _POST widgets for simple register writes (or almost anything really) should never be required and error prone - what is this actually trying to do? I'd expect the class D to be a PGA or OUTPUT widget and the PLL to be a SUPPLY widget. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL @ 2014-07-21 11:53 ` Mark Brown 0 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2014-07-21 11:53 UTC (permalink / raw) To: Dan Murphy; +Cc: linux-sound, linux-kernel, alsa-devel [-- Attachment #1: Type: text/plain, Size: 1054 bytes --] On Fri, Jul 18, 2014 at 12:31:08PM -0500, Dan Murphy wrote: > +static int tas2552_class_d_en(struct snd_soc_dapm_widget *w, > + struct snd_kcontrol *kcontrol, int event) > +{ > + switch (event) { > + case SND_SOC_DAPM_PRE_PMU: > + snd_soc_update_bits(w->codec, TAS2552_CFG_2, > + TAS2552_CLASSD_EN_MASK, TAS2552_CLASSD_EN_MASK); > + break; > + case SND_SOC_DAPM_POST_PMD: > + snd_soc_update_bits(w->codec, TAS2552_CFG_2, > + TAS2552_CLASSD_EN_MASK, 0); > + break; > + } > + > + return 0; > +} > + > +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = > +{ > +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en), > +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en), > +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable), > +}; This seems broken, having to use _PRE or _POST widgets for simple register writes (or almost anything really) should never be required and error prone - what is this actually trying to do? I'd expect the class D to be a PGA or OUTPUT widget and the PLL to be a SUPPLY widget. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL 2014-07-21 11:53 ` Mark Brown (?) @ 2014-07-25 18:15 ` Murphy, Dan -1 siblings, 0 replies; 16+ messages in thread From: Murphy, Dan @ 2014-07-25 18:15 UTC (permalink / raw) To: Mark Brown; +Cc: alsa-devel, linux-kernel, linux-sound Mark On 07/21/2014 06:54 AM, Mark Brown wrote: On Fri, Jul 18, 2014 at 12:31:08PM -0500, Dan Murphy wrote: +static int tas2552_class_d_en(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + switch (event) { + case SND_SOC_DAPM_PRE_PMU: + snd_soc_update_bits(w->codec, TAS2552_CFG_2, + TAS2552_CLASSD_EN_MASK, TAS2552_CLASSD_EN_MASK); + break; + case SND_SOC_DAPM_POST_PMD: + snd_soc_update_bits(w->codec, TAS2552_CFG_2, + TAS2552_CLASSD_EN_MASK, 0); + break; + } + + return 0; +} + +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = +{ +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en), +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en), +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable), +}; This seems broken, having to use _PRE or _POST widgets for simple register writes (or almost anything really) should never be required and error prone - what is this actually trying to do? I'd expect the class D to be a PGA or OUTPUT widget and the PLL to be a SUPPLY widget. I need a little help here. I am not seeing the PLL being disabled or Class D being disabled when I am using the DAPM calls My implementation is like this. And I use set these widgets and audio map in the call back functions in the struct snd_soc_codec_driver. When I look at the registers after stream playback I see that the TAS2552_CFG_2 register is untouched static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = { SND_SOC_DAPM_AIF_IN("DAC IN", NULL, 0, SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_DAC("DAC", NULL, SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_OUT_DRV("ClassD Enable", TAS2552_CFG_2, 7, 0, NULL, 0), SND_SOC_DAPM_OUT_DRV("ClassD Disable", TAS2552_CFG_2, 7, 1, NULL, 0), SND_SOC_DAPM_SUPPLY("PLL Disable", TAS2552_CFG_2, 3, 1, NULL, 0), SND_SOC_DAPM_OUTPUT("OUT") }; static const struct snd_soc_dapm_route tas2552_audio_map[] = { {"DAC", NULL, "DAC IN"}, {"ClassD Enable", NULL, "DAC"}, {"OUT", NULL, "ClassD Enable"}, {"ClassD Disable", NULL, "OUT"}, {"PLL Disable", NULL, "ClassD Disable"} }; Any information would help here Thanks in advance -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL 2014-07-21 11:53 ` Mark Brown @ 2014-07-25 18:54 ` Murphy, Dan -1 siblings, 0 replies; 16+ messages in thread From: Murphy, Dan @ 2014-07-25 18:54 UTC (permalink / raw) To: Mark Brown; +Cc: linux-sound, linux-kernel, alsa-devel On 07/21/2014 06:54 AM, Mark Brown wrote: > On Fri, Jul 18, 2014 at 12:31:08PM -0500, Dan Murphy wrote: > >> +static int tas2552_class_d_en(struct snd_soc_dapm_widget *w, >> + struct snd_kcontrol *kcontrol, int event) >> +{ >> + switch (event) { >> + case SND_SOC_DAPM_PRE_PMU: >> + snd_soc_update_bits(w->codec, TAS2552_CFG_2, >> + TAS2552_CLASSD_EN_MASK, TAS2552_CLASSD_EN_MASK); >> + break; >> + case SND_SOC_DAPM_POST_PMD: >> + snd_soc_update_bits(w->codec, TAS2552_CFG_2, >> + TAS2552_CLASSD_EN_MASK, 0); >> + break; >> + } >> + >> + return 0; >> +} >> + >> +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = >> +{ >> +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en), >> +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en), >> +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable), >> +}; > > This seems broken, having to use _PRE or _POST widgets for simple > register writes (or almost anything really) should never be required and > error prone - what is this actually trying to do? I'd expect the class > D to be a PGA or OUTPUT widget and the PLL to be a SUPPLY widget. > I need a little help here. I am not seeing the PLL being disabled or Class D being disabled when I am using the DAPM calls My implementation is like this. And I use set these widgets and audio map in the call back functions in the struct snd_soc_codec_driver. When I look at the registers after stream playback I see that the TAS2552_CFG_2 register is untouched static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = { SND_SOC_DAPM_AIF_IN("DAC IN", NULL, 0, SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_DAC("DAC", NULL, SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_OUT_DRV("ClassD Enable", TAS2552_CFG_2, 7, 0, NULL, 0), SND_SOC_DAPM_OUT_DRV("ClassD Disable", TAS2552_CFG_2, 7, 1, NULL, 0), SND_SOC_DAPM_SUPPLY("PLL Disable", TAS2552_CFG_2, 3, 1, NULL, 0), SND_SOC_DAPM_OUTPUT("OUT") }; static const struct snd_soc_dapm_route tas2552_audio_map[] = { {"DAC", NULL, "DAC IN"}, {"ClassD Enable", NULL, "DAC"}, {"OUT", NULL, "ClassD Enable"}, {"ClassD Disable", NULL, "OUT"}, {"PLL Disable", NULL, "ClassD Disable"} }; Any information would help here Thanks in advance -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL @ 2014-07-25 18:54 ` Murphy, Dan 0 siblings, 0 replies; 16+ messages in thread From: Murphy, Dan @ 2014-07-25 18:54 UTC (permalink / raw) To: Mark Brown; +Cc: linux-sound, linux-kernel, alsa-devel On 07/21/2014 06:54 AM, Mark Brown wrote: > On Fri, Jul 18, 2014 at 12:31:08PM -0500, Dan Murphy wrote: > >> +static int tas2552_class_d_en(struct snd_soc_dapm_widget *w, >> + struct snd_kcontrol *kcontrol, int event) >> +{ >> + switch (event) { >> + case SND_SOC_DAPM_PRE_PMU: >> + snd_soc_update_bits(w->codec, TAS2552_CFG_2, >> + TAS2552_CLASSD_EN_MASK, TAS2552_CLASSD_EN_MASK); >> + break; >> + case SND_SOC_DAPM_POST_PMD: >> + snd_soc_update_bits(w->codec, TAS2552_CFG_2, >> + TAS2552_CLASSD_EN_MASK, 0); >> + break; >> + } >> + >> + return 0; >> +} >> + >> +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] >> +{ >> +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en), >> +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en), >> +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable), >> +}; > > This seems broken, having to use _PRE or _POST widgets for simple > register writes (or almost anything really) should never be required and > error prone - what is this actually trying to do? I'd expect the class > D to be a PGA or OUTPUT widget and the PLL to be a SUPPLY widget. > I need a little help here. I am not seeing the PLL being disabled or Class D being disabled when I am using the DAPM calls My implementation is like this. And I use set these widgets and audio map in the call back functions in the struct snd_soc_codec_driver. When I look at the registers after stream playback I see that the TAS2552_CFG_2 register is untouched static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] { SND_SOC_DAPM_AIF_IN("DAC IN", NULL, 0, SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_DAC("DAC", NULL, SND_SOC_NOPM, 0, 0), SND_SOC_DAPM_OUT_DRV("ClassD Enable", TAS2552_CFG_2, 7, 0, NULL, 0), SND_SOC_DAPM_OUT_DRV("ClassD Disable", TAS2552_CFG_2, 7, 1, NULL, 0), SND_SOC_DAPM_SUPPLY("PLL Disable", TAS2552_CFG_2, 3, 1, NULL, 0), SND_SOC_DAPM_OUTPUT("OUT") }; static const struct snd_soc_dapm_route tas2552_audio_map[] = { {"DAC", NULL, "DAC IN"}, {"ClassD Enable", NULL, "DAC"}, {"OUT", NULL, "ClassD Enable"}, {"ClassD Disable", NULL, "OUT"}, {"PLL Disable", NULL, "ClassD Disable"} }; Any information would help here Thanks in advance -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL 2014-07-25 18:54 ` Murphy, Dan @ 2014-07-29 18:51 ` Mark Brown -1 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2014-07-29 18:51 UTC (permalink / raw) To: Murphy, Dan; +Cc: linux-sound, linux-kernel, alsa-devel [-- Attachment #1: Type: text/plain, Size: 2056 bytes --] On Fri, Jul 25, 2014 at 06:54:19PM +0000, Murphy, Dan wrote: > On 07/21/2014 06:54 AM, Mark Brown wrote: > > On Fri, Jul 18, 2014 at 12:31:08PM -0500, Dan Murphy wrote: > >> +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = > >> +{ > >> +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en), > >> +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en), > >> +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable), > >> +}; > > This seems broken, having to use _PRE or _POST widgets for simple > > register writes (or almost anything really) should never be required and > > error prone - what is this actually trying to do? I'd expect the class > > D to be a PGA or OUTPUT widget and the PLL to be a SUPPLY widget. > I need a little help here. > I am not seeing the PLL being disabled or Class D being disabled when > I am using the DAPM calls > My implementation is like this. And I use set these widgets and audio > map in the call back functions in the struct snd_soc_codec_driver. > When I look at the registers after stream playback I see that > the TAS2552_CFG_2 register is untouched > static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = > { > SND_SOC_DAPM_AIF_IN("DAC IN", NULL, 0, SND_SOC_NOPM, 0, 0), > SND_SOC_DAPM_DAC("DAC", NULL, SND_SOC_NOPM, 0, 0), > SND_SOC_DAPM_OUT_DRV("ClassD Enable", TAS2552_CFG_2, 7, 0, NULL, 0), > SND_SOC_DAPM_OUT_DRV("ClassD Disable", TAS2552_CFG_2, 7, 1, NULL, 0), > SND_SOC_DAPM_SUPPLY("PLL Disable", TAS2552_CFG_2, 3, 1, NULL, 0), > SND_SOC_DAPM_OUTPUT("OUT") > }; > static const struct snd_soc_dapm_route tas2552_audio_map[] = { > {"DAC", NULL, "DAC IN"}, > {"ClassD Enable", NULL, "DAC"}, > {"OUT", NULL, "ClassD Enable"}, > {"ClassD Disable", NULL, "OUT"}, > {"PLL Disable", NULL, "ClassD Disable"} > }; This looks very confused, why do you have separate enable and disable things - what do you think the effect of the above should be? That's probably what's going wrong. I'd expect to see one DAPM widget for the class D and one for the PLL. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL @ 2014-07-29 18:51 ` Mark Brown 0 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2014-07-29 18:51 UTC (permalink / raw) To: Murphy, Dan; +Cc: linux-sound, linux-kernel, alsa-devel [-- Attachment #1: Type: text/plain, Size: 2056 bytes --] On Fri, Jul 25, 2014 at 06:54:19PM +0000, Murphy, Dan wrote: > On 07/21/2014 06:54 AM, Mark Brown wrote: > > On Fri, Jul 18, 2014 at 12:31:08PM -0500, Dan Murphy wrote: > >> +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = > >> +{ > >> +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en), > >> +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en), > >> +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable), > >> +}; > > This seems broken, having to use _PRE or _POST widgets for simple > > register writes (or almost anything really) should never be required and > > error prone - what is this actually trying to do? I'd expect the class > > D to be a PGA or OUTPUT widget and the PLL to be a SUPPLY widget. > I need a little help here. > I am not seeing the PLL being disabled or Class D being disabled when > I am using the DAPM calls > My implementation is like this. And I use set these widgets and audio > map in the call back functions in the struct snd_soc_codec_driver. > When I look at the registers after stream playback I see that > the TAS2552_CFG_2 register is untouched > static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] = > { > SND_SOC_DAPM_AIF_IN("DAC IN", NULL, 0, SND_SOC_NOPM, 0, 0), > SND_SOC_DAPM_DAC("DAC", NULL, SND_SOC_NOPM, 0, 0), > SND_SOC_DAPM_OUT_DRV("ClassD Enable", TAS2552_CFG_2, 7, 0, NULL, 0), > SND_SOC_DAPM_OUT_DRV("ClassD Disable", TAS2552_CFG_2, 7, 1, NULL, 0), > SND_SOC_DAPM_SUPPLY("PLL Disable", TAS2552_CFG_2, 3, 1, NULL, 0), > SND_SOC_DAPM_OUTPUT("OUT") > }; > static const struct snd_soc_dapm_route tas2552_audio_map[] = { > {"DAC", NULL, "DAC IN"}, > {"ClassD Enable", NULL, "DAC"}, > {"OUT", NULL, "ClassD Enable"}, > {"ClassD Disable", NULL, "OUT"}, > {"PLL Disable", NULL, "ClassD Disable"} > }; This looks very confused, why do you have separate enable and disable things - what do you think the effect of the above should be? That's probably what's going wrong. I'd expect to see one DAPM widget for the class D and one for the PLL. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] ASoC: tas2552: Fix PM sequencing 2014-07-18 17:31 ` Dan Murphy @ 2014-07-21 12:19 ` Mark Brown -1 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2014-07-21 12:19 UTC (permalink / raw) To: Dan Murphy; +Cc: linux-sound, linux-kernel, alsa-devel [-- Attachment #1: Type: text/plain, Size: 436 bytes --] On Fri, Jul 18, 2014 at 12:31:07PM -0500, Dan Murphy wrote: > In the pm suspend/resume it is better > to disable the GPIO after the regmap_cache > setting calls so that if the call is interrupted > the new reg values will be cached and set on resume. > > Also add pm_runtime_put in the remove call. Applied, thanks, but please don't mix multiple changes into one commit like this especially when there's no textual overlap. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] ASoC: tas2552: Fix PM sequencing @ 2014-07-21 12:19 ` Mark Brown 0 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2014-07-21 12:19 UTC (permalink / raw) To: Dan Murphy; +Cc: linux-sound, linux-kernel, alsa-devel [-- Attachment #1: Type: text/plain, Size: 436 bytes --] On Fri, Jul 18, 2014 at 12:31:07PM -0500, Dan Murphy wrote: > In the pm suspend/resume it is better > to disable the GPIO after the regmap_cache > setting calls so that if the call is interrupted > the new reg values will be cached and set on resume. > > Also add pm_runtime_put in the remove call. Applied, thanks, but please don't mix multiple changes into one commit like this especially when there's no textual overlap. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] ASoC: tas2552: Fix PM sequencing 2014-07-21 12:19 ` Mark Brown (?) @ 2014-07-21 13:23 ` Murphy, Dan -1 siblings, 0 replies; 16+ messages in thread From: Murphy, Dan @ 2014-07-21 13:23 UTC (permalink / raw) To: Mark Brown; +Cc: linux-sound, linux-kernel, alsa-devel Mark On 07/21/2014 07:19 AM, Mark Brown wrote: > On Fri, Jul 18, 2014 at 12:31:07PM -0500, Dan Murphy wrote: >> In the pm suspend/resume it is better >> to disable the GPIO after the regmap_cache >> setting calls so that if the call is interrupted >> the new reg values will be cached and set on resume. >> >> Also add pm_runtime_put in the remove call. > Applied, thanks, but please don't mix multiple changes into one commit > like this especially when there's no textual overlap. I usually don't mix changes like this as I like to have a single change per commit. But I find each maintainer is a little different but now I know for next time. Thanks -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-07-29 18:52 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-07-18 17:31 [PATCH 1/2] ASoC: tas2552: Fix PM sequencing Dan Murphy 2014-07-18 17:31 ` Dan Murphy 2014-07-18 17:31 ` Dan Murphy 2014-07-18 17:31 ` [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL Dan Murphy 2014-07-18 17:31 ` Dan Murphy 2014-07-18 17:31 ` Dan Murphy 2014-07-21 11:53 ` Mark Brown 2014-07-21 11:53 ` Mark Brown 2014-07-25 18:15 ` Murphy, Dan 2014-07-25 18:54 ` Murphy, Dan 2014-07-25 18:54 ` Murphy, Dan 2014-07-29 18:51 ` Mark Brown 2014-07-29 18:51 ` Mark Brown 2014-07-21 12:19 ` [PATCH 1/2] ASoC: tas2552: Fix PM sequencing Mark Brown 2014-07-21 12:19 ` Mark Brown 2014-07-21 13:23 ` Murphy, Dan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.