All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ASoC: rt5682: move clk related code to rt5682_i2c_probe
@ 2021-09-29  5:43 Jack Yu
  2021-09-30 14:58 ` Mark Brown
  2021-10-05 11:23 ` Kai Vehmanen
  0 siblings, 2 replies; 3+ messages in thread
From: Jack Yu @ 2021-09-29  5:43 UTC (permalink / raw)
  To: broonie, lgirdwood
  Cc: oder_chiou, Jack Yu, alsa-devel, lars, wenst, derek.fang,
	shumingf, flove

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(-)

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];
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ASoC: rt5682: move clk related code to rt5682_i2c_probe
  2021-09-29  5:43 [PATCH] ASoC: rt5682: move clk related code to rt5682_i2c_probe Jack Yu
@ 2021-09-30 14:58 ` Mark Brown
  2021-10-05 11:23 ` Kai Vehmanen
  1 sibling, 0 replies; 3+ messages in thread
From: Mark Brown @ 2021-09-30 14:58 UTC (permalink / raw)
  To: Jack Yu, lgirdwood
  Cc: oder_chiou, alsa-devel, lars, wenst, Mark Brown, derek.fang,
	shumingf, flove

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ASoC: rt5682: move clk related code to rt5682_i2c_probe
  2021-09-29  5:43 [PATCH] ASoC: rt5682: move clk related code to rt5682_i2c_probe Jack Yu
  2021-09-30 14:58 ` Mark Brown
@ 2021-10-05 11:23 ` Kai Vehmanen
  1 sibling, 0 replies; 3+ messages in thread
From: Kai Vehmanen @ 2021-10-05 11:23 UTC (permalink / raw)
  To: Jack Yu
  Cc: oder_chiou, alsa-devel, lars, wenst, lgirdwood,
	pierre-louis.bossart, broonie, derek.fang, shumingf, flove

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-10-05 11:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  5:43 [PATCH] ASoC: rt5682: move clk related code to rt5682_i2c_probe Jack Yu
2021-09-30 14:58 ` Mark Brown
2021-10-05 11:23 ` Kai Vehmanen

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.