diff mbox series

ASoC: rt5682: move clk related code to rt5682_i2c_probe

Message ID 20210929054344.12112-1-jack.yu@realtek.com (mailing list archive)
State Accepted
Commit 57589f82762e40bdaa975d840fa2bc5157b5be95
Headers show
Series ASoC: rt5682: move clk related code to rt5682_i2c_probe | expand

Commit Message

Jack Yu Sept. 29, 2021, 5:43 a.m. UTC
The DAI clock is only used in I2S mode, to make it clear
and to fix clock resource release issue, we move CCF clock
related code to rt5682_i2c_probe to fix clock
register/unregister issue.

Signed-off-by: Jack Yu <jack.yu@realtek.com>
---
 sound/soc/codecs/rt5682-i2c.c | 22 +++++++++++
 sound/soc/codecs/rt5682.c     | 70 +++++++++++++----------------------
 sound/soc/codecs/rt5682.h     |  3 ++
 3 files changed, 51 insertions(+), 44 deletions(-)

Comments

Mark Brown Sept. 30, 2021, 2:58 p.m. UTC | #1
On Wed, 29 Sep 2021 13:43:44 +0800, Jack Yu wrote:
> The DAI clock is only used in I2S mode, to make it clear
> and to fix clock resource release issue, we move CCF clock
> related code to rt5682_i2c_probe to fix clock
> register/unregister issue.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: rt5682: move clk related code to rt5682_i2c_probe
      commit: 57589f82762e40bdaa975d840fa2bc5157b5be95

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Kai Vehmanen Oct. 5, 2021, 11:23 a.m. UTC | #2
Hi,

On Wed, 29 Sep 2021, Jack Yu wrote:

> The DAI clock is only used in I2S mode, to make it clear
> and to fix clock resource release issue, we move CCF clock
> related code to rt5682_i2c_probe to fix clock
> register/unregister issue.

this patch is causing regressions on some devices in SOF CI:
https://sof-ci.01.org/linuxpr/PR3192/build6477/devicetest/?model=CML_HEL_RT5682&testcase=verify-kernel-boot-log

Reverting this patch and the test passes.

--cut--
[    2.725780] kernel: rt5682 i2c-10EC5682:00: sysclk/dai not set correctly
[    2.725854] kernel: general protection fault, probably for non-canonical address 0x2bd63a3afec92100: 0000 [#1] SMP NOPTI
[    2.725864] kernel: CPU: 2 PID: 80 Comm: kworker/u8:2 Not tainted 5.15.0-rc4-pr3192-6477-default #7c8961c8
[    2.725870] kernel: Hardware name: Google Helios/Helios, BIOS  01/21/2020
[    2.725874] kernel: Workqueue: events_unbound async_run_entry_fn
[    2.725882] kernel: RIP: 0010:clk_core_get_parent_by_index+0x4a/0x90
[    2.725889] kernel: Code: 8d 2c 92 48 c1 e5 03 4c 8d 24 28 49 8b 44 24 08 48 85 c0 74 0c 5b 5d 41 5c c3 5b 31 c0 5d 41 5c c3 49 8b 04 24 48 85 c0 74 26 <48> 8b 00 48 85 c0 74 e3 48 3d 00 f0 ff ff 77 05 49 89 44 24 08 48
[    2.725899] kernel: RSP: 0018:ffff9cf2c07c7c00 EFLAGS: 00010206
[    2.725903] kernel: RAX: 2bd63a3afec92100 RBX: ffff92ca837cbb00 RCX: 00000000f3c74d52
[    2.725908] kernel: RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff92ca837cbb00
[    2.725913] kernel: RBP: 0000000000000000 R08: 00000000ffffffff R09: 0000000000000000
[    2.725918] kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff92ca861aeec0
[    2.725922] kernel: R13: ffff92ca8cf84420 R14: ffff92ca861aeee8 R15: ffff92ca837cbb00
[    2.725927] kernel: FS:  0000000000000000(0000) GS:ffff92cbd6200000(0000) knlGS:0000000000000000
[    2.725933] kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    2.725937] kernel: CR2: 00007eff771a8400 CR3: 0000000194a24006 CR4: 00000000003706e0
[    2.725942] kernel: Call Trace:
[    2.725947] kernel:  __clk_register+0x465/0x7e0
[    2.725952] kernel:  ? clk_hw_unregister+0x10/0x10
[    2.725958] kernel:  clk_hw_register+0x19/0x40
[    2.725963] kernel:  devm_clk_hw_register+0x41/0x80
[    2.725969] kernel:  rt5682_register_dai_clks+0x8e/0x130 [snd_soc_rt5682]
[    2.725979] kernel:  rt5682_i2c_probe+0x484/0x600 [snd_soc_rt5682_i2c]
[    2.725987] kernel:  ? rt5682_irq+0x40/0x40 [snd_soc_rt5682_i2c]
[    2.725992] kernel:  i2c_device_probe+0x314/0x340
--cut--

Full log at the link ("dmesg" tab).

Br, Kai
diff mbox series

Patch

diff --git a/sound/soc/codecs/rt5682-i2c.c b/sound/soc/codecs/rt5682-i2c.c
index b9d5d7a0975b..a30e42932cf7 100644
--- a/sound/soc/codecs/rt5682-i2c.c
+++ b/sound/soc/codecs/rt5682-i2c.c
@@ -139,6 +139,8 @@  static int rt5682_i2c_probe(struct i2c_client *i2c,
 
 	i2c_set_clientdata(i2c, rt5682);
 
+	rt5682->i2c_dev = &i2c->dev;
+
 	rt5682->pdata = i2s_default_platform_data;
 
 	if (pdata)
@@ -276,6 +278,26 @@  static int rt5682_i2c_probe(struct i2c_client *i2c,
 			dev_err(&i2c->dev, "Failed to reguest IRQ: %d\n", ret);
 	}
 
+#ifdef CONFIG_COMMON_CLK
+	/* Check if MCLK provided */
+	rt5682->mclk = devm_clk_get(&i2c->dev, "mclk");
+	if (IS_ERR(rt5682->mclk)) {
+		if (PTR_ERR(rt5682->mclk) != -ENOENT) {
+			ret = PTR_ERR(rt5682->mclk);
+			return ret;
+		}
+		rt5682->mclk = NULL;
+	}
+
+	/* Register CCF DAI clock control */
+	ret = rt5682_register_dai_clks(rt5682);
+	if (ret)
+		return ret;
+
+	/* Initial setup for CCF */
+	rt5682->lrck[RT5682_AIF1] = 48000;
+#endif
+
 	return devm_snd_soc_register_component(&i2c->dev,
 					       &rt5682_soc_component_dev,
 					       rt5682_dai, ARRAY_SIZE(rt5682_dai));
diff --git a/sound/soc/codecs/rt5682.c b/sound/soc/codecs/rt5682.c
index 12113c2dcae2..914fe7debc05 100644
--- a/sound/soc/codecs/rt5682.c
+++ b/sound/soc/codecs/rt5682.c
@@ -2510,7 +2510,7 @@  static int rt5682_set_bias_level(struct snd_soc_component *component,
 static bool rt5682_clk_check(struct rt5682_priv *rt5682)
 {
 	if (!rt5682->master[RT5682_AIF1]) {
-		dev_dbg(rt5682->component->dev, "sysclk/dai not set correctly\n");
+		dev_dbg(rt5682->i2c_dev, "sysclk/dai not set correctly\n");
 		return false;
 	}
 	return true;
@@ -2521,13 +2521,15 @@  static int rt5682_wclk_prepare(struct clk_hw *hw)
 	struct rt5682_priv *rt5682 =
 		container_of(hw, struct rt5682_priv,
 			     dai_clks_hw[RT5682_DAI_WCLK_IDX]);
-	struct snd_soc_component *component = rt5682->component;
-	struct snd_soc_dapm_context *dapm =
-			snd_soc_component_get_dapm(component);
+	struct snd_soc_component *component;
+	struct snd_soc_dapm_context *dapm;
 
 	if (!rt5682_clk_check(rt5682))
 		return -EINVAL;
 
+	component = rt5682->component;
+	dapm = snd_soc_component_get_dapm(component);
+
 	snd_soc_dapm_mutex_lock(dapm);
 
 	snd_soc_dapm_force_enable_pin_unlocked(dapm, "MICBIAS");
@@ -2557,13 +2559,15 @@  static void rt5682_wclk_unprepare(struct clk_hw *hw)
 	struct rt5682_priv *rt5682 =
 		container_of(hw, struct rt5682_priv,
 			     dai_clks_hw[RT5682_DAI_WCLK_IDX]);
-	struct snd_soc_component *component = rt5682->component;
-	struct snd_soc_dapm_context *dapm =
-			snd_soc_component_get_dapm(component);
+	struct snd_soc_component *component;
+	struct snd_soc_dapm_context *dapm;
 
 	if (!rt5682_clk_check(rt5682))
 		return;
 
+	component = rt5682->component;
+	dapm = snd_soc_component_get_dapm(component);
+
 	snd_soc_dapm_mutex_lock(dapm);
 
 	snd_soc_dapm_disable_pin_unlocked(dapm, "MICBIAS");
@@ -2587,7 +2591,6 @@  static unsigned long rt5682_wclk_recalc_rate(struct clk_hw *hw,
 	struct rt5682_priv *rt5682 =
 		container_of(hw, struct rt5682_priv,
 			     dai_clks_hw[RT5682_DAI_WCLK_IDX]);
-	struct snd_soc_component *component = rt5682->component;
 	const char * const clk_name = clk_hw_get_name(hw);
 
 	if (!rt5682_clk_check(rt5682))
@@ -2597,7 +2600,7 @@  static unsigned long rt5682_wclk_recalc_rate(struct clk_hw *hw,
 	 */
 	if (rt5682->lrck[RT5682_AIF1] != CLK_48 &&
 	    rt5682->lrck[RT5682_AIF1] != CLK_44) {
-		dev_warn(component->dev, "%s: clk %s only support %d or %d Hz output\n",
+		dev_warn(rt5682->i2c_dev, "%s: clk %s only support %d or %d Hz output\n",
 			__func__, clk_name, CLK_44, CLK_48);
 		return 0;
 	}
@@ -2611,7 +2614,6 @@  static long rt5682_wclk_round_rate(struct clk_hw *hw, unsigned long rate,
 	struct rt5682_priv *rt5682 =
 		container_of(hw, struct rt5682_priv,
 			     dai_clks_hw[RT5682_DAI_WCLK_IDX]);
-	struct snd_soc_component *component = rt5682->component;
 	const char * const clk_name = clk_hw_get_name(hw);
 
 	if (!rt5682_clk_check(rt5682))
@@ -2621,7 +2623,7 @@  static long rt5682_wclk_round_rate(struct clk_hw *hw, unsigned long rate,
 	 * It will force to 48kHz if not both.
 	 */
 	if (rate != CLK_48 && rate != CLK_44) {
-		dev_warn(component->dev, "%s: clk %s only support %d or %d Hz output\n",
+		dev_warn(rt5682->i2c_dev, "%s: clk %s only support %d or %d Hz output\n",
 			__func__, clk_name, CLK_44, CLK_48);
 		rate = CLK_48;
 	}
@@ -2635,7 +2637,7 @@  static int rt5682_wclk_set_rate(struct clk_hw *hw, unsigned long rate,
 	struct rt5682_priv *rt5682 =
 		container_of(hw, struct rt5682_priv,
 			     dai_clks_hw[RT5682_DAI_WCLK_IDX]);
-	struct snd_soc_component *component = rt5682->component;
+	struct snd_soc_component *component;
 	struct clk_hw *parent_hw;
 	const char * const clk_name = clk_hw_get_name(hw);
 	int pre_div;
@@ -2644,6 +2646,8 @@  static int rt5682_wclk_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (!rt5682_clk_check(rt5682))
 		return -EINVAL;
 
+	component = rt5682->component;
+
 	/*
 	 * Whether the wclk's parent clk (mclk) exists or not, please ensure
 	 * it is fixed or set to 48MHz before setting wclk rate. It's a
@@ -2653,12 +2657,12 @@  static int rt5682_wclk_set_rate(struct clk_hw *hw, unsigned long rate,
 	 */
 	parent_hw = clk_hw_get_parent(hw);
 	if (!parent_hw)
-		dev_warn(component->dev,
+		dev_warn(rt5682->i2c_dev,
 			"Parent mclk of wclk not acquired in driver. Please ensure mclk was provided as %d Hz.\n",
 			CLK_PLL2_FIN);
 
 	if (parent_rate != CLK_PLL2_FIN)
-		dev_warn(component->dev, "clk %s only support %d Hz input\n",
+		dev_warn(rt5682->i2c_dev, "clk %s only support %d Hz input\n",
 			clk_name, CLK_PLL2_FIN);
 
 	/*
@@ -2690,10 +2694,9 @@  static unsigned long rt5682_bclk_recalc_rate(struct clk_hw *hw,
 	struct rt5682_priv *rt5682 =
 		container_of(hw, struct rt5682_priv,
 			     dai_clks_hw[RT5682_DAI_BCLK_IDX]);
-	struct snd_soc_component *component = rt5682->component;
 	unsigned int bclks_per_wclk;
 
-	bclks_per_wclk = snd_soc_component_read(component, RT5682_TDM_TCON_CTRL);
+	regmap_read(rt5682->regmap, RT5682_TDM_TCON_CTRL, &bclks_per_wclk);
 
 	switch (bclks_per_wclk & RT5682_TDM_BCLK_MS1_MASK) {
 	case RT5682_TDM_BCLK_MS1_256:
@@ -2754,20 +2757,22 @@  static int rt5682_bclk_set_rate(struct clk_hw *hw, unsigned long rate,
 	struct rt5682_priv *rt5682 =
 		container_of(hw, struct rt5682_priv,
 			     dai_clks_hw[RT5682_DAI_BCLK_IDX]);
-	struct snd_soc_component *component = rt5682->component;
+	struct snd_soc_component *component;
 	struct snd_soc_dai *dai;
 	unsigned long factor;
 
 	if (!rt5682_clk_check(rt5682))
 		return -EINVAL;
 
+	component = rt5682->component;
+
 	factor = rt5682_bclk_get_factor(rate, parent_rate);
 
 	for_each_component_dais(component, dai)
 		if (dai->id == RT5682_AIF1)
 			break;
 	if (!dai) {
-		dev_err(component->dev, "dai %d not found in component\n",
+		dev_err(rt5682->i2c_dev, "dai %d not found in component\n",
 			RT5682_AIF1);
 		return -ENODEV;
 	}
@@ -2790,10 +2795,9 @@  static const struct clk_ops rt5682_dai_clk_ops[RT5682_DAI_NUM_CLKS] = {
 	},
 };
 
-static int rt5682_register_dai_clks(struct snd_soc_component *component)
+int rt5682_register_dai_clks(struct rt5682_priv *rt5682)
 {
-	struct device *dev = component->dev;
-	struct rt5682_priv *rt5682 = snd_soc_component_get_drvdata(component);
+	struct device *dev = rt5682->i2c_dev;
 	struct rt5682_platform_data *pdata = &rt5682->pdata;
 	struct clk_hw *dai_clk_hw;
 	int i, ret;
@@ -2851,6 +2855,7 @@  static int rt5682_register_dai_clks(struct snd_soc_component *component)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(rt5682_register_dai_clks);
 #endif /* CONFIG_COMMON_CLK */
 
 static int rt5682_probe(struct snd_soc_component *component)
@@ -2860,9 +2865,6 @@  static int rt5682_probe(struct snd_soc_component *component)
 	unsigned long time;
 	struct snd_soc_dapm_context *dapm = &component->dapm;
 
-#ifdef CONFIG_COMMON_CLK
-	int ret;
-#endif
 	rt5682->component = component;
 
 	if (rt5682->is_sdw) {
@@ -2874,26 +2876,6 @@  static int rt5682_probe(struct snd_soc_component *component)
 			dev_err(&slave->dev, "Initialization not complete, timed out\n");
 			return -ETIMEDOUT;
 		}
-	} else {
-#ifdef CONFIG_COMMON_CLK
-		/* Check if MCLK provided */
-		rt5682->mclk = devm_clk_get(component->dev, "mclk");
-		if (IS_ERR(rt5682->mclk)) {
-			if (PTR_ERR(rt5682->mclk) != -ENOENT) {
-				ret = PTR_ERR(rt5682->mclk);
-				return ret;
-			}
-			rt5682->mclk = NULL;
-		}
-
-		/* Register CCF DAI clock control */
-		ret = rt5682_register_dai_clks(component);
-		if (ret)
-			return ret;
-
-		/* Initial setup for CCF */
-		rt5682->lrck[RT5682_AIF1] = CLK_48;
-#endif
 	}
 
 	snd_soc_dapm_disable_pin(dapm, "MICBIAS");
diff --git a/sound/soc/codecs/rt5682.h b/sound/soc/codecs/rt5682.h
index b59221048ebf..262512babc50 100644
--- a/sound/soc/codecs/rt5682.h
+++ b/sound/soc/codecs/rt5682.h
@@ -1408,6 +1408,7 @@  enum {
 
 struct rt5682_priv {
 	struct snd_soc_component *component;
+	struct device *i2c_dev;
 	struct rt5682_platform_data pdata;
 	struct regmap *regmap;
 	struct regmap *sdw_regmap;
@@ -1462,6 +1463,8 @@  void rt5682_calibrate(struct rt5682_priv *rt5682);
 void rt5682_reset(struct rt5682_priv *rt5682);
 int rt5682_parse_dt(struct rt5682_priv *rt5682, struct device *dev);
 
+int rt5682_register_dai_clks(struct rt5682_priv *rt5682);
+
 #define RT5682_REG_NUM 318
 extern const struct reg_default rt5682_reg[RT5682_REG_NUM];