Re: [PATCH 2/3] ASoC: qcom: q6afe: check ADSP version when setting clocks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks Otto for the patch,
some comments below

On 14/10/2023 18:26, Otto Pflüger wrote:
There are two APIs for setting clocks: the old one that uses
AFE_PARAM_ID_INT_DIGITAL_CDC_CLK_CONFIG and AFE_PARAM_ID_LPAIF_CLK_CONFIG,
and the new one which uses AFE_PARAM_ID_CLOCK_SET.

ADSP firmware version 2.6 only provides the old API, while newer
firmware versions only provide the new API.

Implement LPAIF_BIT_CLK and LPAIF_DIG_CLK for both APIs so that users
don't have to care about the firmware version. Also fall back to
setting AFE_PARAM_ID_INT_DIGITAL_CDC_CLK_CONFIG in q6afe_set_lpass_clock
when setting the new Q6AFE_LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
clock is requested to allow specifying it in the device tree on older
platforms too.

Signed-off-by: Otto Pflüger <otto.pflueger@xxxxxxxxx>
---
  sound/soc/qcom/qdsp6/q6afe.c | 81 ++++++++++++++++++++++++++++++++++++
  1 file changed, 81 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
index 91d39f6ad0bd..87bdf741e5f6 100644
--- a/sound/soc/qcom/qdsp6/q6afe.c
+++ b/sound/soc/qcom/qdsp6/q6afe.c
@@ -1111,6 +1111,32 @@ int q6afe_set_lpass_clock(struct device *dev, int clk_id, int attri,
  	struct q6afe *afe = dev_get_drvdata(dev->parent);
  	struct afe_clk_set cset = {0,};
+ /*
+	 * v2 clocks specified in the device tree may not be supported by the
+	 * firmware. If this is the digital codec core clock, fall back to the
+	 * old method for setting it.
+	 */
+	if (q6core_get_adsp_version() < Q6_ADSP_VERSION_2_7) {
+		struct q6afe_port *port;
+		struct afe_digital_clk_cfg dcfg = {0,};
+		int ret;
+
+		if (clk_id != Q6AFE_LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE)
+			return -EINVAL;
+

<---
+		port = q6afe_port_get_from_id(dev, PRIMARY_MI2S_RX);
+		if (IS_ERR(port))
+			return PTR_ERR(port);
+
+		dcfg.i2s_cfg_minor_version = AFE_API_VERSION_I2S_CONFIG;
+		dcfg.clk_val = freq;
+		dcfg.clk_root = 5;
+		ret = q6afe_set_digital_codec_core_clock(port, &dcfg);
+
+		q6afe_port_put(port);
--->

Could you pl explain what are we doing in this snippet?

Isn't this what is exactly done in q6afe_mi2s_set_sysclk(LPAIF_DIG_CLK...)




+		return ret;
+	}
+
  	cset.clk_set_minor_version = AFE_API_VERSION_CLOCK_SET;
  	cset.clk_id = clk_id;
  	cset.clk_freq_in_hz = freq;
@@ -1124,6 +1150,41 @@ int q6afe_set_lpass_clock(struct device *dev, int clk_id, int attri,
  }
  EXPORT_SYMBOL_GPL(q6afe_set_lpass_clock);
...

  int q6afe_port_set_sysclk(struct q6afe_port *port, int clk_id,
  			  int clk_src, int clk_root,
  			  unsigned int freq, int dir)
@@ -1133,6 +1194,26 @@ int q6afe_port_set_sysclk(struct q6afe_port *port, int clk_id,
  	struct afe_digital_clk_cfg dcfg = {0,};
  	int ret;
+ if (q6core_get_adsp_version() >= Q6_ADSP_VERSION_2_7) {
+		/* Always use the new clock API on newer platforms. */
+		switch (clk_id) {
+		case LPAIF_DIG_CLK:
+			clk_src = Q6AFE_LPASS_CLK_ATTRIBUTE_COUPLE_NO;
+			clk_root = Q6AFE_LPASS_CLK_ROOT_DEFAULT;
+			clk_id = Q6AFE_LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE;
+			break;
+		case LPAIF_BIT_CLK:
+			clk_src = Q6AFE_LPASS_CLK_ATTRIBUTE_COUPLE_NO;
+			clk_root = Q6AFE_LPASS_CLK_ROOT_DEFAULT;
+			clk_id = q6afe_get_v2_bit_clk_id(port);
+			if (clk_id < 0)
+				return clk_id;
+			break;
+		default:
+			break;
+		}
+	}

This should be probably done in machine driver or q6afe-dai, not in q6afe.


+
  	switch (clk_id) {
  	case LPAIF_DIG_CLK:
  		dcfg.i2s_cfg_minor_version = AFE_API_VERSION_I2S_CONFIG;



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux