All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Bard Liao <bardliao@realtek.com>,
	Oder Chiou <oder_chiou@realtek.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	alsa-devel@alsa-project.org, Takashi Iwai <tiwai@suse.com>
Subject: [PATCH 01/19] ASoC: rt5640: Remove is_sys_clk_from_pll, it has ordering issues
Date: Tue,  8 May 2018 17:35:46 +0200	[thread overview]
Message-ID: <20180508153604.23711-2-hdegoede@redhat.com> (raw)
In-Reply-To: <20180508153604.23711-1-hdegoede@redhat.com>

is_sys_clk_from_pll() is used as a snd_soc_dapm_route.connected callback,
checking RT5640_GBL_CLK to determine if the sys-clk is PLL1 and thus the
PWR_PLL bit in reg PWR_ANLG2 must be set.

RT5640_GBL_CLK is changed by rt5640_set_dai_sysclk(), which gets called by
the pre_pmu / post_pmd functions of the "Platform Clock" dapm-supply.

This creates an ordering issue, during a dapm transition first all
connected() callbacks are called to build a list of supplies to enable
and then the complete list is walked to enable the supplies. Since the
connected() check happens before enabling any supplies,
is_sys_clk_from_pll() ends up deciding if the PWR_PLL bit should be set
based on the state the "Platform Clock" supply had *before* the transition.
This sometimes results in PWR_PLL being off, even though *after* the
transition PLL1 is configured as sys-clk.

This commit removes is_sys_clk_from_pll() instead simply setting / clearing
PWR_PLL in rt5640_set_dai_sysclk() based on the selected sys-clk, which
fixes this and as a bonus results in a nice cleanup.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5640.c | 29 ++++-------------------------
 1 file changed, 4 insertions(+), 25 deletions(-)

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index 05567426f211..140bad07429e 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -476,20 +476,6 @@ static int set_dmic_clk(struct snd_soc_dapm_widget *w,
 	return idx;
 }
 
-static int is_sys_clk_from_pll(struct snd_soc_dapm_widget *source,
-			 struct snd_soc_dapm_widget *sink)
-{
-	struct snd_soc_component *component = snd_soc_dapm_to_component(source->dapm);
-	unsigned int val;
-
-	val = snd_soc_component_read32(component, RT5640_GLB_CLK);
-	val &= RT5640_SCLK_SRC_MASK;
-	if (val == RT5640_SCLK_SRC_PLL1)
-		return 1;
-	else
-		return 0;
-}
-
 static int is_using_asrc(struct snd_soc_dapm_widget *source,
 			 struct snd_soc_dapm_widget *sink)
 {
@@ -1071,9 +1057,6 @@ static int rt5640_hp_post_event(struct snd_soc_dapm_widget *w,
 }
 
 static const struct snd_soc_dapm_widget rt5640_dapm_widgets[] = {
-	SND_SOC_DAPM_SUPPLY("PLL1", RT5640_PWR_ANLG2,
-			RT5640_PWR_PLL_BIT, 0, NULL, 0),
-
 	/* ASRC */
 	SND_SOC_DAPM_SUPPLY_S("Stereo Filter ASRC", 1, RT5640_ASRC_1,
 			 15, 0, NULL, 0),
@@ -1427,22 +1410,18 @@ static const struct snd_soc_dapm_route rt5640_dapm_routes[] = {
 	{"Stereo ADC MIXL", "ADC1 Switch", "Stereo ADC L1 Mux"},
 	{"Stereo ADC MIXL", "ADC2 Switch", "Stereo ADC L2 Mux"},
 	{"Stereo ADC MIXL", NULL, "Stereo Filter"},
-	{"Stereo Filter", NULL, "PLL1", is_sys_clk_from_pll},
 
 	{"Stereo ADC MIXR", "ADC1 Switch", "Stereo ADC R1 Mux"},
 	{"Stereo ADC MIXR", "ADC2 Switch", "Stereo ADC R2 Mux"},
 	{"Stereo ADC MIXR", NULL, "Stereo Filter"},
-	{"Stereo Filter", NULL, "PLL1", is_sys_clk_from_pll},
 
 	{"Mono ADC MIXL", "ADC1 Switch", "Mono ADC L1 Mux"},
 	{"Mono ADC MIXL", "ADC2 Switch", "Mono ADC L2 Mux"},
 	{"Mono ADC MIXL", NULL, "Mono Left Filter"},
-	{"Mono Left Filter", NULL, "PLL1", is_sys_clk_from_pll},
 
 	{"Mono ADC MIXR", "ADC1 Switch", "Mono ADC R1 Mux"},
 	{"Mono ADC MIXR", "ADC2 Switch", "Mono ADC R2 Mux"},
 	{"Mono ADC MIXR", NULL, "Mono Right Filter"},
-	{"Mono Right Filter", NULL, "PLL1", is_sys_clk_from_pll},
 
 	{"IF2 ADC L", NULL, "Mono ADC MIXL"},
 	{"IF2 ADC R", NULL, "Mono ADC MIXR"},
@@ -1512,10 +1491,8 @@ static const struct snd_soc_dapm_route rt5640_dapm_routes[] = {
 	{"DIG MIXR", "DAC R1 Switch", "DAC MIXR"},
 
 	{"DAC L1", NULL, "Stereo DAC MIXL"},
-	{"DAC L1", NULL, "PLL1", is_sys_clk_from_pll},
 	{"DAC L1", NULL, "DAC L1 Power"},
 	{"DAC R1", NULL, "Stereo DAC MIXR"},
-	{"DAC R1", NULL, "PLL1", is_sys_clk_from_pll},
 	{"DAC R1", NULL, "DAC R1 Power"},
 
 	{"SPK MIXL", "REC MIXL Switch", "RECMIXL"},
@@ -1622,10 +1599,8 @@ static const struct snd_soc_dapm_route rt5640_specific_dapm_routes[] = {
 	{"DIG MIXL", "DAC L2 Switch", "DAC L2 Mux"},
 
 	{"DAC L2", NULL, "Mono DAC MIXL"},
-	{"DAC L2", NULL, "PLL1", is_sys_clk_from_pll},
 	{"DAC L2", NULL, "DAC L2 Power"},
 	{"DAC R2", NULL, "Mono DAC MIXR"},
-	{"DAC R2", NULL, "PLL1", is_sys_clk_from_pll},
 	{"DAC R2", NULL, "DAC R2 Power"},
 
 	{"SPK MIXL", "DAC L2 Switch", "DAC L2"},
@@ -1861,6 +1836,7 @@ static int rt5640_set_dai_sysclk(struct snd_soc_dai *dai,
 	struct snd_soc_component *component = dai->component;
 	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
 	unsigned int reg_val = 0;
+	unsigned int pll_bit = 0;
 
 	if (freq == rt5640->sysclk && clk_id == rt5640->sysclk_src)
 		return 0;
@@ -1871,6 +1847,7 @@ static int rt5640_set_dai_sysclk(struct snd_soc_dai *dai,
 		break;
 	case RT5640_SCLK_S_PLL1:
 		reg_val |= RT5640_SCLK_SRC_PLL1;
+		pll_bit |= RT5640_PWR_PLL;
 		break;
 	case RT5640_SCLK_S_RCCLK:
 		reg_val |= RT5640_SCLK_SRC_RCCLK;
@@ -1879,6 +1856,8 @@ static int rt5640_set_dai_sysclk(struct snd_soc_dai *dai,
 		dev_err(component->dev, "Invalid clock id (%d)\n", clk_id);
 		return -EINVAL;
 	}
+	snd_soc_component_update_bits(component, RT5640_PWR_ANLG2,
+		RT5640_PWR_PLL, pll_bit);
 	snd_soc_component_update_bits(component, RT5640_GLB_CLK,
 		RT5640_SCLK_SRC_MASK, reg_val);
 	rt5640->sysclk = freq;
-- 
2.17.0

  reply	other threads:[~2018-05-08 15:36 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-08 15:35 [PATCH 00/19] ASoC: rt5640: Add jack-detect and button-press support Hans de Goede
2018-05-08 15:35 ` Hans de Goede [this message]
2018-05-08 15:35 ` [PATCH 02/19] ASoC: rt5640: Add devicetree-bindings for dmic, jack-detect Hans de Goede
2018-05-08 15:35 ` [PATCH 03/19] ASoC: rt5640: Remove unused rt5640_platform_data Hans de Goede
2018-05-11  2:16   ` Mark Brown
2018-05-11  2:52   ` Applied "ASoC: rt5640: Remove unused rt5640_platform_data" to the asoc tree Mark Brown
2018-05-08 15:35 ` [PATCH 04/19] ASoC: rt5640: Move checking of device-properties to component probe callback Hans de Goede
2018-05-11  2:51   ` Applied "ASoC: rt5640: Move checking of device-properties to component probe callback" to the asoc tree Mark Brown
2018-05-08 15:35 ` [PATCH 05/19] ASoC: rt5640: Allow specifying dmic data pins through device-properties Hans de Goede
2018-05-11  2:51   ` Applied "ASoC: rt5640: Allow specifying dmic data pins through device-properties" to the asoc tree Mark Brown
2018-05-08 15:35 ` [PATCH 06/19] ASoC: rt5640: Add jack-detect support Hans de Goede
2018-05-11  2:50   ` Applied "ASoC: rt5640: Add jack-detect support" to the asoc tree Mark Brown
2018-05-08 15:35 ` [PATCH 07/19] ASoC: rt5640: Add button press support Hans de Goede
2018-05-11  2:50   ` Applied "ASoC: rt5640: Add button press support" to the asoc tree Mark Brown
2018-05-08 15:35 ` [PATCH 08/19] ASoC: Intel: bytcr_rt5640: Configure PLL1 before using it Hans de Goede
2018-05-11  2:48   ` Applied "ASoC: Intel: bytcr_rt5640: Configure PLL1 before using it" to the asoc tree Mark Brown
2018-05-08 15:35 ` [PATCH 09/19] ASoC: Intel: bytcr_rt5640: Use device-property for differential mics Hans de Goede
2018-05-11  2:48   ` Applied "ASoC: Intel: bytcr_rt5640: Use device-property for differential mics" to the asoc tree Mark Brown
2018-05-08 15:35 ` [PATCH 10/19] ASoC: Intel: bytcr_rt5640: Use device properties for setting up dmic Hans de Goede
2018-05-11  2:25   ` Mark Brown
2018-05-08 15:35 ` [PATCH 11/19] ASoC: Intel: bytcr_rt5640: Fix Dell Venue 8 5830 Pro quirk Hans de Goede
2018-05-17 16:39   ` Applied "ASoC: Intel: bytcr_rt5640: Fix Dell Venue 8 5830 Pro quirk" to the asoc tree Mark Brown
2018-05-08 15:35 ` [PATCH 12/19] ASoC: Intel: bytcr_rt5640: Enable jack detection Hans de Goede
2018-05-17 16:39   ` Applied "ASoC: Intel: bytcr_rt5640: Enable jack detection" to the asoc tree Mark Brown
2018-05-08 15:35 ` [PATCH 13/19] ASoC: Intel: bytcr_rt5640: Change BYTCR default input to IN3 Hans de Goede
2018-05-17 16:39   ` Applied "ASoC: Intel: bytcr_rt5640: Change BYTCR default input to IN3" to the asoc tree Mark Brown
2018-05-08 15:35 ` [PATCH 14/19] ASoC: Intel: bytcr_rt5640: Unify BYTCR input defaults Hans de Goede
2018-05-17 16:39   ` Applied "ASoC: Intel: bytcr_rt5640: Unify BYTCR input defaults" to the asoc tree Mark Brown
2018-05-08 15:36 ` [PATCH 15/19] ASoC: Intel: bytcr_rt5640: Add default jack-detect settings Hans de Goede
2018-05-17 16:39   ` Applied "ASoC: Intel: bytcr_rt5640: Add default jack-detect settings" to the asoc tree Mark Brown
2018-05-08 15:36 ` [PATCH 16/19] ASoC: Intel: bytcr_rt5640: Sort DMI quirk list alphabetically Hans de Goede
2018-05-17 16:39   ` Applied "ASoC: Intel: bytcr_rt5640: Sort DMI quirk list alphabetically" to the asoc tree Mark Brown
2018-05-08 15:36 ` [PATCH 17/19] ASoC: Intel: bytcr_rt5640: Use dmi_first_match() for DMI quirk handling Hans de Goede
2018-05-17 16:38   ` Applied "ASoC: Intel: bytcr_rt5640: Use dmi_first_match() for DMI quirk handling" to the asoc tree Mark Brown
2018-05-08 15:36 ` [PATCH 18/19] ASoC: Intel: bytcr_rt5640: Add quirks for various devices Hans de Goede
2018-05-08 15:36 ` [PATCH 19/19] ASoC: Intel: bytcr_rt5640: Set card long_name based on quirks Hans de Goede
2018-05-08 18:35   ` Pierre-Louis Bossart
2018-05-10 10:27     ` Hans de Goede
2018-05-10 15:00       ` Pierre-Louis Bossart
2018-05-10 15:48         ` Hans de Goede
2018-05-10 17:46           ` Pierre-Louis Bossart
2018-05-10 18:01             ` Hans de Goede
2018-05-12 21:21               ` Pierre-Louis Bossart
2018-05-13  7:11                 ` Takashi Iwai
2018-05-13  7:28                   ` Hans de Goede
2018-05-18 15:55           ` Hans de Goede
2018-05-18 16:21             ` Pierre-Louis Bossart
2018-05-17 16:38   ` Applied "ASoC: Intel: bytcr_rt5640: Set card long_name based on quirks" to the asoc tree Mark Brown
2018-05-08 18:42 ` [PATCH 00/19] ASoC: rt5640: Add jack-detect and button-press support Pierre-Louis Bossart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180508153604.23711-2-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=bardliao@realtek.com \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=oder_chiou@realtek.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.