Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [alsa-devel] [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks
@ 2020-02-14  7:53 derek.fang
  2020-02-14  7:53 ` [alsa-devel] [PATCH 2/2] ASoC: rt5682: Add DAI clock binding info for WCLK/BCLK CCF usage derek.fang
  2020-02-14 12:38 ` [alsa-devel] [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks Mark Brown
  0 siblings, 2 replies; 5+ messages in thread
From: derek.fang @ 2020-02-14  7:53 UTC (permalink / raw)
  To: broonie, lgirdwood
  Cc: oder_chiou, jack.yu, alsa-devel, lars, albertchen, derek.fang,
	shumingf, flove

From: "derek.fang" <derek.fang@realtek.com>

There is a need to use RT5682 as DAI clock master for other codecs
within a system, which means that the DAI clocks are required to
remain, regardless of whether the RT5682 is actually running
playback/capture.

Signed-off-by: derek.fang <derek.fang@realtek.com>
---
 include/sound/rt5682.h    |   8 +
 sound/soc/codecs/rt5682.c | 389 +++++++++++++++++++++++++++++++++++++++++++++-
 sound/soc/codecs/rt5682.h |   4 +-
 3 files changed, 397 insertions(+), 4 deletions(-)

diff --git a/include/sound/rt5682.h b/include/sound/rt5682.h
index bc2c317..6bf0e35 100644
--- a/include/sound/rt5682.h
+++ b/include/sound/rt5682.h
@@ -24,6 +24,12 @@ enum rt5682_jd_src {
 	RT5682_JD1,
 };
 
+enum rt5682_dai_clks {
+	RT5682_DAI_WCLK_IDX,
+	RT5682_DAI_BCLK_IDX,
+	RT5682_DAI_NUM_CLKS,
+};
+
 struct rt5682_platform_data {
 
 	int ldo1_en; /* GPIO for LDO1_EN */
@@ -32,6 +38,8 @@ struct rt5682_platform_data {
 	enum rt5682_dmic1_clk_pin dmic1_clk_pin;
 	enum rt5682_jd_src jd_src;
 	unsigned int btndet_delay;
+
+	const char *dai_clk_names[RT5682_DAI_NUM_CLKS];
 };
 
 #endif
diff --git a/sound/soc/codecs/rt5682.c b/sound/soc/codecs/rt5682.c
index 9fbb386..197e615 100644
--- a/sound/soc/codecs/rt5682.c
+++ b/sound/soc/codecs/rt5682.c
@@ -27,6 +27,9 @@
 #include <sound/soc-dapm.h>
 #include <sound/initval.h>
 #include <sound/tlv.h>
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
 #include <sound/rt5682.h>
 
 #include "rl6231.h"
@@ -45,6 +48,8 @@ static const struct rt5682_platform_data i2s_default_platform_data = {
 	.dmic1_clk_pin = RT5682_DMIC1_CLK_GPIO3,
 	.jd_src = RT5682_JD1,
 	.btndet_delay = 16,
+	.dai_clk_names[RT5682_DAI_WCLK_IDX] = "rt5682-dai-wclk",
+	.dai_clk_names[RT5682_DAI_BCLK_IDX] = "rt5682-dai-bclk",
 };
 
 struct rt5682_priv {
@@ -58,6 +63,13 @@ struct rt5682_priv {
 	struct mutex calibrate_mutex;
 	bool is_sdw;
 
+#ifdef CONFIG_COMMON_CLK
+	struct clk_hw dai_clks_hw[RT5682_DAI_NUM_CLKS];
+	struct clk_lookup *dai_clks_lookup[RT5682_DAI_NUM_CLKS];
+	struct clk *dai_clks[RT5682_DAI_NUM_CLKS];
+	struct clk *mclk;
+#endif
+
 	int sysclk;
 	int sysclk_src;
 	int lrck[RT5682_AIFS];
@@ -921,6 +933,7 @@ static int rt5682_headset_detect(struct snd_soc_component *component,
 		int jack_insert)
 {
 	struct rt5682_priv *rt5682 = snd_soc_component_get_drvdata(component);
+	struct snd_soc_dapm_context *dapm = &component->dapm;
 	unsigned int val, count;
 
 	if (jack_insert) {
@@ -963,8 +976,13 @@ static int rt5682_headset_detect(struct snd_soc_component *component,
 		rt5682_enable_push_button_irq(component, false);
 		snd_soc_component_update_bits(component, RT5682_CBJ_CTRL_1,
 			RT5682_TRIG_JD_MASK, RT5682_TRIG_JD_LOW);
-		snd_soc_component_update_bits(component, RT5682_PWR_ANLG_1,
-			RT5682_PWR_VREF2 | RT5682_PWR_MB, 0);
+		if (snd_soc_dapm_get_pin_status(dapm, "MICBIAS"))
+			snd_soc_component_update_bits(component,
+				RT5682_PWR_ANLG_1, RT5682_PWR_VREF2, 0);
+		else
+			snd_soc_component_update_bits(component,
+				RT5682_PWR_ANLG_1,
+				RT5682_PWR_VREF2 | RT5682_PWR_MB, 0);
 		snd_soc_component_update_bits(component, RT5682_PWR_ANLG_3,
 			RT5682_PWR_CBJ, 0);
 
@@ -1633,6 +1651,7 @@ static const struct snd_soc_dapm_widget rt5682_dapm_widgets[] = {
 		rt5655_set_verf, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU),
 	SND_SOC_DAPM_SUPPLY("Vref2", RT5682_PWR_ANLG_1, RT5682_PWR_VREF2_BIT, 0,
 		NULL, 0),
+	SND_SOC_DAPM_SUPPLY("MICBIAS", SND_SOC_NOPM, 0, 0, NULL, 0),
 
 	/* ASRC */
 	SND_SOC_DAPM_SUPPLY_S("DAC STO1 ASRC", 1, RT5682_PLL_TRACK_1,
@@ -2459,12 +2478,362 @@ static int rt5682_set_bias_level(struct snd_soc_component *component,
 	return 0;
 }
 
+#ifdef CONFIG_COMMON_CLK
+#define CLK_PLL2_FIN 48000000
+#define CLK_PLL2_FOUT 24576000
+#define CLK_48 48000
+
+static bool rt5682_clk_check(struct rt5682_priv *rt5682)
+{
+	if (!rt5682->master[RT5682_AIF1]) {
+		dev_err(rt5682->component->dev, "sysclk/dai not set correctly\n");
+		return false;
+	}
+	return true;
+}
+
+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);
+
+	if (!rt5682_clk_check(rt5682))
+		return -EINVAL;
+
+	snd_soc_dapm_mutex_lock(dapm);
+
+	snd_soc_dapm_force_enable_pin_unlocked(dapm, "MICBIAS");
+	snd_soc_component_update_bits(component, RT5682_PWR_ANLG_1,
+				RT5682_PWR_MB, RT5682_PWR_MB);
+	snd_soc_dapm_force_enable_pin_unlocked(dapm, "I2S1");
+	snd_soc_dapm_force_enable_pin_unlocked(dapm, "PLL2F");
+	snd_soc_dapm_force_enable_pin_unlocked(dapm, "PLL2B");
+	snd_soc_dapm_sync_unlocked(dapm);
+
+	snd_soc_dapm_mutex_unlock(dapm);
+
+	return 0;
+}
+
+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);
+
+	if (!rt5682_clk_check(rt5682))
+		return;
+
+	snd_soc_dapm_mutex_lock(dapm);
+
+	snd_soc_dapm_disable_pin_unlocked(dapm, "MICBIAS");
+	if (!rt5682->jack_type)
+		snd_soc_component_update_bits(component, RT5682_PWR_ANLG_1,
+				RT5682_PWR_MB, 0);
+	snd_soc_dapm_disable_pin_unlocked(dapm, "I2S1");
+	snd_soc_dapm_disable_pin_unlocked(dapm, "PLL2F");
+	snd_soc_dapm_disable_pin_unlocked(dapm, "PLL2B");
+	snd_soc_dapm_sync_unlocked(dapm);
+
+	snd_soc_dapm_mutex_unlock(dapm);
+}
+
+static unsigned long rt5682_wclk_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	struct rt5682_priv *rt5682 =
+		container_of(hw, struct rt5682_priv,
+			     dai_clks_hw[RT5682_DAI_WCLK_IDX]);
+
+	if (!rt5682_clk_check(rt5682))
+		return 0;
+
+	return CLK_48;
+}
+
+static long rt5682_wclk_round_rate(struct clk_hw *hw, unsigned long rate,
+				   unsigned long *parent_rate)
+{
+	struct rt5682_priv *rt5682 =
+		container_of(hw, struct rt5682_priv,
+			     dai_clks_hw[RT5682_DAI_WCLK_IDX]);
+
+	if (!rt5682_clk_check(rt5682))
+		return -EINVAL;
+
+	return CLK_48;
+}
+
+static int rt5682_wclk_set_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_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 clk *parent_clk;
+	const char * const clk_name = __clk_get_name(hw->clk);
+	int pre_div;
+
+	if (!rt5682_clk_check(rt5682))
+		return -EINVAL;
+
+	parent_clk = clk_get_parent(hw->clk);
+	if (!parent_clk)
+		dev_warn(component->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",
+			clk_name, CLK_PLL2_FIN);
+
+	if (rate != CLK_48) {
+		dev_warn(component->dev, "clk %s only support %d Hz output\n",
+			clk_name, CLK_48);
+		rate = CLK_48;
+	}
+
+	rt5682_set_component_pll(component, RT5682_PLL2, RT5682_PLL2_S_MCLK,
+		CLK_PLL2_FIN, CLK_PLL2_FOUT);
+
+	rt5682_set_component_sysclk(component, RT5682_SCLK_S_PLL2, 0,
+		CLK_PLL2_FOUT, SND_SOC_CLOCK_IN);
+
+	pre_div = rl6231_get_clk_info(rt5682->sysclk, rate);
+
+	snd_soc_component_update_bits(component, RT5682_ADDA_CLK_1,
+		RT5682_I2S_M_DIV_MASK | RT5682_I2S_CLK_SRC_MASK,
+		pre_div << RT5682_I2S_M_DIV_SFT |
+		(rt5682->sysclk_src) << RT5682_I2S_CLK_SRC_SFT);
+
+	return 0;
+}
+
+static unsigned long rt5682_bclk_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_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;
+	unsigned int bclks_per_wclk;
+
+	snd_soc_component_read(component, RT5682_TDM_TCON_CTRL,
+				&bclks_per_wclk);
+
+	switch (bclks_per_wclk & RT5682_TDM_BCLK_MS1_MASK) {
+	case RT5682_TDM_BCLK_MS1_256:
+		return parent_rate * 256;
+	case RT5682_TDM_BCLK_MS1_128:
+		return parent_rate * 128;
+	case RT5682_TDM_BCLK_MS1_64:
+		return parent_rate * 64;
+	case RT5682_TDM_BCLK_MS1_32:
+		return parent_rate * 32;
+	default:
+		return 0;
+	}
+}
+
+static unsigned long rt5682_bclk_get_factor(unsigned long rate,
+					    unsigned long parent_rate)
+{
+	unsigned long factor;
+
+	factor = rate / parent_rate;
+	if (factor < 64)
+		return 32;
+	else if (factor < 128)
+		return 64;
+	else if (factor < 256)
+		return 128;
+	else
+		return 256;
+}
+
+static long rt5682_bclk_round_rate(struct clk_hw *hw, unsigned long rate,
+				   unsigned long *parent_rate)
+{
+	struct rt5682_priv *rt5682 =
+		container_of(hw, struct rt5682_priv,
+			     dai_clks_hw[RT5682_DAI_BCLK_IDX]);
+	unsigned long factor;
+
+	if (!*parent_rate || !rt5682_clk_check(rt5682))
+		return -EINVAL;
+
+	/*
+	 * BCLK rates are set as a multiplier of WCLK in HW.
+	 * We don't allow changing the parent WCLK. We just do
+	 * some rounding down based on the parent WCLK rate
+	 * and find the appropriate multiplier of BCLK to
+	 * get the rounded down BCLK value.
+	 */
+	factor = rt5682_bclk_get_factor(rate, *parent_rate);
+
+	return *parent_rate * factor;
+}
+
+static int rt5682_bclk_set_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_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_dai *dai = NULL;
+	unsigned long factor;
+
+	if (!rt5682_clk_check(rt5682))
+		return -EINVAL;
+
+	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",
+			RT5682_AIF1);
+		return -ENODEV;
+	}
+
+	return rt5682_set_bclk1_ratio(dai, factor);
+}
+
+static const struct clk_ops rt5682_dai_clk_ops[RT5682_DAI_NUM_CLKS] = {
+	[RT5682_DAI_WCLK_IDX] = {
+		.prepare = rt5682_wclk_prepare,
+		.unprepare = rt5682_wclk_unprepare,
+		.recalc_rate = rt5682_wclk_recalc_rate,
+		.round_rate = rt5682_wclk_round_rate,
+		.set_rate = rt5682_wclk_set_rate,
+	},
+	[RT5682_DAI_BCLK_IDX] = {
+		.recalc_rate = rt5682_bclk_recalc_rate,
+		.round_rate = rt5682_bclk_round_rate,
+		.set_rate = rt5682_bclk_set_rate,
+	},
+};
+
+static int rt5682_register_dai_clks(struct snd_soc_component *component)
+{
+	struct device *dev = component->dev;
+	struct rt5682_priv *rt5682 = snd_soc_component_get_drvdata(component);
+	struct rt5682_platform_data *pdata = &rt5682->pdata;
+	struct clk_init_data init;
+	struct clk *dai_clk;
+	struct clk_lookup *dai_clk_lookup;
+	struct clk_hw *dai_clk_hw;
+	const char *parent_name;
+	int i, ret;
+
+	for (i = 0; i < RT5682_DAI_NUM_CLKS; ++i) {
+		dai_clk_hw = &rt5682->dai_clks_hw[i];
+
+		switch (i) {
+		case RT5682_DAI_WCLK_IDX:
+			/* Make MCLK the parent of WCLK */
+			if (rt5682->mclk) {
+				parent_name = __clk_get_name(rt5682->mclk);
+				init.parent_names = &parent_name;
+				init.num_parents = 1;
+			} else {
+				init.parent_names = NULL;
+				init.num_parents = 0;
+			}
+			break;
+		case RT5682_DAI_BCLK_IDX:
+			/* Make WCLK the parent of BCLK */
+			parent_name = __clk_get_name(
+				rt5682->dai_clks[RT5682_DAI_WCLK_IDX]);
+			init.parent_names = &parent_name;
+			init.num_parents = 1;
+			break;
+		default:
+			dev_err(dev, "Invalid clock index\n");
+			ret = -EINVAL;
+			goto err;
+		}
+
+		init.name = pdata->dai_clk_names[i];
+		init.ops = &rt5682_dai_clk_ops[i];
+		init.flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_GATE;
+		dai_clk_hw->init = &init;
+
+		dai_clk = devm_clk_register(dev, dai_clk_hw);
+		if (IS_ERR(dai_clk)) {
+			dev_warn(dev, "Failed to register %s: %ld\n",
+				 init.name, PTR_ERR(dai_clk));
+			ret = PTR_ERR(dai_clk);
+			goto err;
+		}
+		rt5682->dai_clks[i] = dai_clk;
+
+		if (dev->of_node) {
+			devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+						    dai_clk_hw);
+		} else {
+			dai_clk_lookup = clkdev_create(dai_clk, init.name,
+						       "%s", dev_name(dev));
+			if (!dai_clk_lookup) {
+				ret = -ENOMEM;
+				goto err;
+			} else {
+				rt5682->dai_clks_lookup[i] = dai_clk_lookup;
+			}
+		}
+	}
+
+	return 0;
+
+err:
+	do {
+		if (rt5682->dai_clks_lookup[i])
+			clkdev_drop(rt5682->dai_clks_lookup[i]);
+	} while (i-- > 0);
+
+	return ret;
+}
+#endif /* CONFIG_COMMON_CLK */
+
 static int rt5682_probe(struct snd_soc_component *component)
 {
 	struct rt5682_priv *rt5682 = snd_soc_component_get_drvdata(component);
 
+#ifdef CONFIG_COMMON_CLK
+	int ret;
+#endif
 	rt5682->component = component;
 
+#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
+
 	return 0;
 }
 
@@ -2472,6 +2841,15 @@ static void rt5682_remove(struct snd_soc_component *component)
 {
 	struct rt5682_priv *rt5682 = snd_soc_component_get_drvdata(component);
 
+#ifdef CONFIG_COMMON_CLK
+	int i;
+
+	for (i = RT5682_DAI_NUM_CLKS - 1; i >= 0; --i) {
+		if (rt5682->dai_clks_lookup[i])
+			clkdev_drop(rt5682->dai_clks_lookup[i]);
+	}
+#endif
+
 	rt5682_reset(rt5682);
 }
 
@@ -2606,6 +2984,13 @@ static int rt5682_parse_dt(struct rt5682_priv *rt5682, struct device *dev)
 	rt5682->pdata.ldo1_en = of_get_named_gpio(dev->of_node,
 		"realtek,ldo1-en-gpios", 0);
 
+	if (device_property_read_string_array(dev, "clock-output-names",
+					      rt5682->pdata.dai_clk_names,
+					      RT5682_DAI_NUM_CLKS) < 0)
+		dev_warn(dev, "Using default DAI clk names: %s, %s\n",
+			 rt5682->pdata.dai_clk_names[RT5682_DAI_WCLK_IDX],
+			 rt5682->pdata.dai_clk_names[RT5682_DAI_BCLK_IDX]);
+
 	return 0;
 }
 
diff --git a/sound/soc/codecs/rt5682.h b/sound/soc/codecs/rt5682.h
index 465c99b..f82126a 100644
--- a/sound/soc/codecs/rt5682.h
+++ b/sound/soc/codecs/rt5682.h
@@ -841,8 +841,8 @@
 #define RT5682_TDM_M_LP_INV			(0x1 << 1)
 #define RT5682_TDM_MS_MASK			(0x1 << 0)
 #define RT5682_TDM_MS_SFT			0
-#define RT5682_TDM_MS_M				(0x0 << 0)
-#define RT5682_TDM_MS_S				(0x1 << 0)
+#define RT5682_TDM_MS_S				(0x0 << 0)
+#define RT5682_TDM_MS_M				(0x1 << 0)
 
 /* Global Clock Control (0x0080) */
 #define RT5682_SCLK_SRC_MASK			(0x7 << 13)
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* [alsa-devel] [PATCH 2/2] ASoC: rt5682: Add DAI clock binding info for WCLK/BCLK CCF usage
  2020-02-14  7:53 [alsa-devel] [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks derek.fang
@ 2020-02-14  7:53 ` derek.fang
  2020-02-14 12:38 ` [alsa-devel] [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks Mark Brown
  1 sibling, 0 replies; 5+ messages in thread
From: derek.fang @ 2020-02-14  7:53 UTC (permalink / raw)
  To: broonie, lgirdwood
  Cc: oder_chiou, jack.yu, alsa-devel, lars, albertchen, derek.fang,
	shumingf, flove

From: "derek.fang" <derek.fang@realtek.com>

This patch describes that rt5682 can expose WCLK and BCLK clocks
and how to use.

Signed-off-by: derek.fang <derek.fang@realtek.com>
---
 Documentation/devicetree/bindings/sound/rt5682.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/rt5682.txt b/Documentation/devicetree/bindings/sound/rt5682.txt
index 30e927a2..ac98151 100644
--- a/Documentation/devicetree/bindings/sound/rt5682.txt
+++ b/Documentation/devicetree/bindings/sound/rt5682.txt
@@ -32,6 +32,12 @@ Optional properties:
   The delay time is realtek,btndet-delay value multiple of 8.192 ms.
   If absent, the default is 16.
 
+- #clock-cells : Should be set to '<1>',  wclk and bclk sources provided.
+- clock-output-names : Name given for DAI clocks output.
+
+- clocks : phandle and clock specifier for codec MCLK.
+- clock-names : Clock name string for 'clocks' attribute, should be "mclk".
+
 Pins on the device (for linking into audio routes) for RT5682:
 
   * DMIC L1
@@ -53,4 +59,10 @@ rt5682 {
 	realtek,dmic1-clk-pin = <1>;
 	realtek,jd-src = <1>;
 	realtek,btndet-delay = <16>;
+
+	#clock-cells = <1>;
+	clock-output-names = "rt5682-dai-wclk", "rt5682-dai-bclk";
+
+	clocks = <&osc>;
+	clock-names = "mclk";
 };
-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* Re: [alsa-devel] [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks
  2020-02-14  7:53 [alsa-devel] [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks derek.fang
  2020-02-14  7:53 ` [alsa-devel] [PATCH 2/2] ASoC: rt5682: Add DAI clock binding info for WCLK/BCLK CCF usage derek.fang
@ 2020-02-14 12:38 ` Mark Brown
       [not found]   ` <b9cfe003aeb34216b59c9a22c4601bf1@realtek.com>
  1 sibling, 1 reply; 5+ messages in thread
From: Mark Brown @ 2020-02-14 12:38 UTC (permalink / raw)
  To: derek.fang
  Cc: oder_chiou, jack.yu, alsa-devel, lars, lgirdwood, albertchen,
	shumingf, flove

[-- Attachment #1.1: Type: text/plain, Size: 668 bytes --]

On Fri, Feb 14, 2020 at 03:53:33PM +0800, derek.fang@realtek.com wrote:

> +	parent_clk = clk_get_parent(hw->clk);
> +	if (!parent_clk)
> +		dev_warn(component->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",
> +			clk_name, CLK_PLL2_FIN);
> +
> +	if (rate != CLK_48) {
> +		dev_warn(component->dev, "clk %s only support %d Hz output\n",
> +			clk_name, CLK_48);
> +		rate = CLK_48;
> +	}

Are these genuine restrictions of the hardware or is this just being
hard coded in the driver?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* Re: [alsa-devel] [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks
       [not found]   ` <b9cfe003aeb34216b59c9a22c4601bf1@realtek.com>
@ 2020-02-17 12:36     ` Mark Brown
  2020-02-18  6:33       ` Derek [方德義]
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2020-02-17 12:36 UTC (permalink / raw)
  To: Derek [方德義]
  Cc: Oder Chiou, Jack Yu, alsa-devel, lars, lgirdwood, Albert Chen,
	Shuming [范書銘], Flove(HsinFu)

[-- Attachment #1.1: Type: text/plain, Size: 1029 bytes --]

On Mon, Feb 17, 2020 at 10:59:55AM +0000, Derek [方德義] wrote:
> > Subject: Re: [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks

> > > +	if (parent_rate != CLK_PLL2_FIN)
> > > +		dev_warn(component->dev, "clk %s only support %d Hz input\n",
> > > +			clk_name, CLK_PLL2_FIN);

> > > +	if (rate != CLK_48) {
> > > +		dev_warn(component->dev, "clk %s only support %d Hz output\n",
> > > +			clk_name, CLK_48);
> > > +		rate = CLK_48;
> > > +	}

> > Are these genuine restrictions of the hardware or is this just being
> > hard coded in the driver?

> It's hard coded for an application case.
> Generate a 48kHz clk with a parent 48 MHz clk.

I see...  obviously this is really not good quality of implementation
although it doesn't actually break any external interfaces so I guess we
could take it.  I would at least want to see some clear comments in both
the code and the changelog explaining that this limitation is just a
temporary hack until a better implementation is done though.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* RE: [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks
  2020-02-17 12:36     ` Mark Brown
@ 2020-02-18  6:33       ` Derek [方德義]
  0 siblings, 0 replies; 5+ messages in thread
From: Derek [方德義] @ 2020-02-18  6:33 UTC (permalink / raw)
  To: Mark Brown
  Cc: Oder Chiou, Jack Yu, alsa-devel, lars, lgirdwood, Albert Chen,
	Shuming [范書銘], Flove(HsinFu)

> Subject: Re: [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks
> 
> On Mon, Feb 17, 2020 at 10:59:55AM +0000, Derek [方德義] wrote:
> > > Subject: Re: [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks
> 
> > > > +	if (parent_rate != CLK_PLL2_FIN)
> > > > +		dev_warn(component->dev, "clk %s only support %d Hz
> input\n",
> > > > +			clk_name, CLK_PLL2_FIN);
> 
> > > > +	if (rate != CLK_48) {
> > > > +		dev_warn(component->dev, "clk %s only support %d Hz
> output\n",
> > > > +			clk_name, CLK_48);
> > > > +		rate = CLK_48;
> > > > +	}
> 
> > > Are these genuine restrictions of the hardware or is this just being
> > > hard coded in the driver?
> 
> > It's hard coded for an application case.
> > Generate a 48kHz clk with a parent 48 MHz clk.
> 
> I see...  obviously this is really not good quality of implementation
> although it doesn't actually break any external interfaces so I guess we
> could take it.  I would at least want to see some clear comments in both
> the code and the changelog explaining that this limitation is just a
> temporary hack until a better implementation is done though.

Ok. Thanks for the advice.
I will add more comments both in the source and commit to clearly describe
the driver support and limitation, and then send the PATCH v2.

------Please consider the environment before printing this e-mail.

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14  7:53 [alsa-devel] [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks derek.fang
2020-02-14  7:53 ` [alsa-devel] [PATCH 2/2] ASoC: rt5682: Add DAI clock binding info for WCLK/BCLK CCF usage derek.fang
2020-02-14 12:38 ` [alsa-devel] [PATCH 1/2] ASoC: rt5682: Add CCF usage for providing I2S clks Mark Brown
     [not found]   ` <b9cfe003aeb34216b59c9a22c4601bf1@realtek.com>
2020-02-17 12:36     ` Mark Brown
2020-02-18  6:33       ` Derek [方德義]

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git