[PATCH v2 0/4] ASoC: qcom: check ADSP version when setting clocks

[PATCH v2 0/4] ASoC: qcom: check ADSP version when setting clocks

[Otto Pflüger]

The apq8016_sbc driver currently works on APQ8016 and MSM8916 devices. It
should also work on MSM8909 (and newer SoCs like MSM8917 and MSM8953 if
the quinary MI2S line is added); however, newer devices with these SoCs
ship with newer firmware that uses a different interface for controlling
the digital codec and bit clocks, which causes the driver to fail because
it cannot set LPAIF_BIT_CLK.

In order to fix this problem, modify the LPAIF_* clock implementation in
the qdsp6 driver to use the newer clock API if a newer firmware version is
detected. This seems to be a better solution than exposing the firmware
version to other drivers like apq8016_sbc and forcing them to figure out
which clock to use.

On MSM8916, a hack is currently used to control the LPAIF_DIG_CLK directly
through the GCC driver, but on devices with the newer firmware, the
INTERNAL_DIGITAL_CODEC_CORE clock provided by q6afe-clocks in the qdsp6
driver can be used instead. Add a fallback to make this clock work with
the older firmware too, allowing one to specify it as the codec's "mclk"
in the device tree:

  compatible = "qcom,msm8916-wcd-digital-codec";
  clocks = <&xo_board>,
           <&q6afecc LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
                     LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
  clock-names = "ahbix", "mclk";
  assigned-clocks = <&q6afecc LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
                              LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
  assigned-clock-rates = <9600000>;

This works both on MSM8916 and on the newer SoCs mentioned above.

---
Changes in v2:
- Log unknown AVS versions
- Move some checks to q6afe-dai as suggested by Srinivas
- Also implement LPAIF_OSR_CLK for ports that have it

Otto Pflüger (4):
  ASoC: qcom: q6core: expose ADSP core firmware version
  ASoC: qcom: q6afe: provide fallback for digital codec clock
  ASoC: qcom: q6afe-dai: check ADSP version when setting sysclk
  ASoC: qcom: q6afe: remove "port already open" error

 sound/soc/qcom/qdsp6/q6afe-dai.c | 98 +++++++++++++++++++++++++++-----
 sound/soc/qcom/qdsp6/q6afe.c     | 30 +++++++++-
 sound/soc/qcom/qdsp6/q6core.c    | 65 +++++++++++++++++++++
 sound/soc/qcom/qdsp6/q6core.h    |  9 +++
 4 files changed, 185 insertions(+), 17 deletions(-)


base-commit: 66f1e1ea3548378ff6387b1ce0b40955d54e86aa
-- 
2.39.2

[PATCH v2 1/4] ASoC: qcom: q6core: expose ADSP core firmware version

[Otto Pflüger]

Add a q6core_get_adsp_version() function that returns the version of the
ADSP firmware (2.6, 2.7 or 2.8), also known as the AVS version (see [1]
in downstream kernel).

Some APIs differ between these versions, e.g. the AFE clock APIs.

[1]: https://github.com/msm8916-mainline/linux-downstream/blob/LA.BR.1.2.9.1_rb1.5/sound/soc/msm/qdsp6v2/q6core.c#L193

Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
 sound/soc/qcom/qdsp6/q6core.c | 65 +++++++++++++++++++++++++++++++++++
 sound/soc/qcom/qdsp6/q6core.h |  9 +++++
 2 files changed, 74 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6core.c b/sound/soc/qcom/qdsp6/q6core.c
index 49cfb32cd209..855ab4ff1e59 100644
--- a/sound/soc/qcom/qdsp6/q6core.c
+++ b/sound/soc/qcom/qdsp6/q6core.c
@@ -20,6 +20,9 @@
 #define AVCS_CMDRSP_ADSP_EVENT_GET_STATE	0x0001290D
 #define AVCS_GET_VERSIONS       0x00012905
 #define AVCS_GET_VERSIONS_RSP   0x00012906
+#define AVCS_CMDRSP_Q6_ID_2_6	0x00040000
+#define AVCS_CMDRSP_Q6_ID_2_7	0x00040001
+#define AVCS_CMDRSP_Q6_ID_2_8	0x00040002
 #define AVCS_CMD_GET_FWK_VERSION	0x001292c
 #define AVCS_CMDRSP_GET_FWK_VERSION	0x001292d
 
@@ -63,6 +66,7 @@ struct q6core {
 	bool get_state_supported;
 	bool get_version_supported;
 	bool is_version_requested;
+	enum q6core_version adsp_version;
 };
 
 static struct q6core *g_core;
@@ -108,6 +112,14 @@ static int q6core_callback(struct apr_device *adev, struct apr_resp_pkt *data)
 		if (!core->fwk_version)
 			return -ENOMEM;
 
+		/*
+		 * Since GET_VERSIONS is not called when GET_FWK_VERSION
+		 * is successful and these commands may return completely
+		 * different versions, assume that the version is 2.8 here.
+		 * Older versions do not support GET_FWK_VERSION and we do
+		 * not care if the version is newer than 2.8.
+		 */
+		core->adsp_version = Q6_ADSP_VERSION_2_8;
 		core->fwk_version_supported = true;
 		core->resp_received = true;
 
@@ -115,6 +127,7 @@ static int q6core_callback(struct apr_device *adev, struct apr_resp_pkt *data)
 	}
 	case AVCS_GET_VERSIONS_RSP: {
 		struct avcs_cmdrsp_get_version *v;
+		int i;
 
 		v = data->payload;
 
@@ -125,6 +138,32 @@ static int q6core_callback(struct apr_device *adev, struct apr_resp_pkt *data)
 		if (!core->svc_version)
 			return -ENOMEM;
 
+		for (i = 0; i < g_core->svc_version->num_services; i++) {
+			struct avcs_svc_info *info;
+
+			info = &g_core->svc_version->svc_api_info[i];
+			if (info->service_id != APR_SVC_ADSP_CORE)
+				continue;
+
+			switch (info->version) {
+			case AVCS_CMDRSP_Q6_ID_2_6:
+				core->adsp_version = Q6_ADSP_VERSION_2_6;
+				break;
+			case AVCS_CMDRSP_Q6_ID_2_7:
+				core->adsp_version = Q6_ADSP_VERSION_2_7;
+				break;
+			case AVCS_CMDRSP_Q6_ID_2_8:
+				core->adsp_version = Q6_ADSP_VERSION_2_8;
+				break;
+			default:
+				dev_err(&adev->dev, "Unknown AVS version 0x%08x\n",
+					info->version);
+				break;
+			}
+
+			break;
+		}
+
 		core->get_version_supported = true;
 		core->resp_received = true;
 
@@ -293,6 +332,31 @@ int q6core_get_svc_api_info(int svc_id, struct q6core_svc_api_info *ainfo)
 }
 EXPORT_SYMBOL_GPL(q6core_get_svc_api_info);
 
+/**
+ * q6core_get_adsp_version() - Get the core version number.
+ *
+ * Return: version code or Q6_ADSP_VERSION_UNKNOWN on failure
+ */
+enum q6core_version q6core_get_adsp_version(void)
+{
+	int ret;
+
+	if (!g_core)
+		return Q6_ADSP_VERSION_UNKNOWN;
+
+	mutex_lock(&g_core->lock);
+	if (!g_core->is_version_requested) {
+		if (q6core_get_fwk_versions(g_core) == -ENOTSUPP)
+			q6core_get_svc_versions(g_core);
+		g_core->is_version_requested = true;
+	}
+	ret = g_core->adsp_version;
+	mutex_unlock(&g_core->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(q6core_get_adsp_version);
+
 /**
  * q6core_is_adsp_ready() - Get status of adsp
  *
@@ -334,6 +398,7 @@ static int q6core_probe(struct apr_device *adev)
 	dev_set_drvdata(&adev->dev, g_core);
 
 	mutex_init(&g_core->lock);
+	g_core->adsp_version = Q6_ADSP_VERSION_UNKNOWN;
 	g_core->adev = adev;
 	init_waitqueue_head(&g_core->wait);
 	return 0;
diff --git a/sound/soc/qcom/qdsp6/q6core.h b/sound/soc/qcom/qdsp6/q6core.h
index 4105b1d730be..472e06bf8efc 100644
--- a/sound/soc/qcom/qdsp6/q6core.h
+++ b/sound/soc/qcom/qdsp6/q6core.h
@@ -9,7 +9,16 @@ struct q6core_svc_api_info {
 	uint32_t api_branch_version;
 };
 
+/* Versions must be in order! */
+enum q6core_version {
+	Q6_ADSP_VERSION_UNKNOWN,
+	Q6_ADSP_VERSION_2_6,
+	Q6_ADSP_VERSION_2_7,
+	Q6_ADSP_VERSION_2_8,
+};
+
 bool q6core_is_adsp_ready(void);
+enum q6core_version q6core_get_adsp_version(void);
 int q6core_get_svc_api_info(int svc_id, struct q6core_svc_api_info *ainfo);
 
 #endif /* __Q6CORE_H__ */
-- 
2.39.2

[PATCH v2 2/4] ASoC: qcom: q6afe: provide fallback for digital codec clock

[Otto Pflüger]

When q6afe is used as a clock provider through q6afe-clocks.c, it uses
an interface for setting clocks that is not present in older firmware
versions. However, using Q6AFE_LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
as the codec MCLK in the device tree can be useful on older platforms
too. Provide a fallback that sets this clock using the old method when
an old firmware version is detected.

MSM8916 did not need this because of a workaround that controls this
clock directly through the GCC driver, but newer SoCs do not have this
clock in their GCC drivers because it is meant to be controlled by the
DSP.

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

diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
index 91d39f6ad0bd..f14d3b366aa4 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);
+		return ret;
+	}
+
 	cset.clk_set_minor_version = AFE_API_VERSION_CLOCK_SET;
 	cset.clk_id = clk_id;
 	cset.clk_freq_in_hz = freq;
-- 
2.39.2

[PATCH v2 3/4] ASoC: qcom: q6afe-dai: check ADSP version when setting sysclk

[Otto Pflüger]

Some older DSP firmware versions only provide AFE_PARAM_ID_*_CLK_CONFIG
requests for setting clocks, while newer ones only provide the
incompatible AFE_PARAM_ID_CLOCK_SET. The q6afe driver implements both,
but requires different clock IDs for the different firmware versions.

Implement the LPAIF_*_CLK clocks using newer clock IDs when the DSP
firmware does not support the old clocks so that users don't have to
care about the firmware version when setting clocks.

Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
 sound/soc/qcom/qdsp6/q6afe-dai.c | 98 +++++++++++++++++++++++++++-----
 1 file changed, 84 insertions(+), 14 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6afe-dai.c b/sound/soc/qcom/qdsp6/q6afe-dai.c
index a9c4f896a7df..c66f8ab41d5e 100644
--- a/sound/soc/qcom/qdsp6/q6afe-dai.c
+++ b/sound/soc/qcom/qdsp6/q6afe-dai.c
@@ -14,6 +14,7 @@
 #include <sound/pcm_params.h>
 #include "q6dsp-lpass-ports.h"
 #include "q6dsp-common.h"
+#include "q6core.h"
 #include "q6afe.h"
 
 
@@ -443,36 +444,105 @@ static int q6slim_set_channel_map(struct snd_soc_dai *dai,
 	return 0;
 }
 
+static int q6afe_get_bit_clk_id(unsigned int dai_id)
+{
+	switch (dai_id) {
+	case PRIMARY_MI2S_RX:
+	case PRIMARY_MI2S_TX:
+		return Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT;
+	case SECONDARY_MI2S_RX:
+	case SECONDARY_MI2S_TX:
+		return Q6AFE_LPASS_CLK_ID_SEC_MI2S_IBIT;
+	case TERTIARY_MI2S_RX:
+	case TERTIARY_MI2S_TX:
+		return Q6AFE_LPASS_CLK_ID_TER_MI2S_IBIT;
+	case QUATERNARY_MI2S_RX:
+	case QUATERNARY_MI2S_TX:
+		return Q6AFE_LPASS_CLK_ID_TER_MI2S_IBIT;
+	case QUINARY_MI2S_RX:
+	case QUINARY_MI2S_TX:
+		return Q6AFE_LPASS_CLK_ID_QUI_MI2S_IBIT;
+
+	case PRIMARY_TDM_RX_0 ... PRIMARY_TDM_TX_7:
+		return Q6AFE_LPASS_CLK_ID_PRI_TDM_IBIT;
+	case SECONDARY_TDM_RX_0 ... SECONDARY_TDM_TX_7:
+		return Q6AFE_LPASS_CLK_ID_SEC_TDM_IBIT;
+	case TERTIARY_TDM_RX_0 ... TERTIARY_TDM_TX_7:
+		return Q6AFE_LPASS_CLK_ID_TER_TDM_IBIT;
+	case QUATERNARY_TDM_RX_0 ... QUATERNARY_TDM_TX_7:
+		return Q6AFE_LPASS_CLK_ID_QUAD_TDM_IBIT;
+	case QUINARY_TDM_RX_0 ... QUINARY_TDM_TX_7:
+		return Q6AFE_LPASS_CLK_ID_QUIN_TDM_IBIT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int q6afe_get_osr_clk_id(unsigned int dai_id)
+{
+	switch (dai_id) {
+	case QUINARY_MI2S_RX:
+	case QUINARY_MI2S_TX:
+		return Q6AFE_LPASS_CLK_ID_QUI_MI2S_OSR;
+
+	case QUINARY_TDM_RX_0 ... QUINARY_TDM_TX_7:
+		return Q6AFE_LPASS_CLK_ID_QUIN_TDM_OSR;
+
+	default:
+		return -EINVAL;
+	}
+}
+
 static int q6afe_mi2s_set_sysclk(struct snd_soc_dai *dai,
 		int clk_id, unsigned int freq, int dir)
 {
 	struct q6afe_dai_data *dai_data = dev_get_drvdata(dai->dev);
 	struct q6afe_port *port = dai_data->port[dai->id];
+	int clk_src;
+	int clk_root;
+	bool use_new_clks = q6core_get_adsp_version() >= Q6_ADSP_VERSION_2_7;
 
 	switch (clk_id) {
 	case LPAIF_DIG_CLK:
-		return q6afe_port_set_sysclk(port, clk_id, 0, 5, freq, dir);
+		if (use_new_clks) {
+			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;
+		} else {
+			clk_src = 0;
+			clk_root = 5;
+		}
+		break;
 	case LPAIF_BIT_CLK:
 	case LPAIF_OSR_CLK:
-		return q6afe_port_set_sysclk(port, clk_id,
-					     Q6AFE_LPASS_CLK_SRC_INTERNAL,
-					     Q6AFE_LPASS_CLK_ROOT_DEFAULT,
-					     freq, dir);
+		if (use_new_clks) {
+			clk_src = Q6AFE_LPASS_CLK_ATTRIBUTE_COUPLE_NO;
+			clk_root = Q6AFE_LPASS_CLK_ROOT_DEFAULT;
+			if (clk_id == LPAIF_OSR_CLK)
+				clk_id = q6afe_get_osr_clk_id(dai->id);
+			else
+				clk_id = q6afe_get_bit_clk_id(dai->id);
+		} else {
+			clk_src = Q6AFE_LPASS_CLK_SRC_INTERNAL;
+			clk_root = Q6AFE_LPASS_CLK_ROOT_DEFAULT;
+		}
+		break;
 	case Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT ... Q6AFE_LPASS_CLK_ID_QUI_MI2S_OSR:
 	case Q6AFE_LPASS_CLK_ID_MCLK_1 ... Q6AFE_LPASS_CLK_ID_INT_MCLK_1:
 	case Q6AFE_LPASS_CLK_ID_WSA_CORE_MCLK ... Q6AFE_LPASS_CLK_ID_VA_CORE_2X_MCLK:
-		return q6afe_port_set_sysclk(port, clk_id,
-					     Q6AFE_LPASS_CLK_ATTRIBUTE_COUPLE_NO,
-					     Q6AFE_LPASS_CLK_ROOT_DEFAULT,
-					     freq, dir);
+		clk_src = Q6AFE_LPASS_CLK_ATTRIBUTE_COUPLE_NO;
+		clk_root = Q6AFE_LPASS_CLK_ROOT_DEFAULT;
+		break;
 	case Q6AFE_LPASS_CLK_ID_PRI_TDM_IBIT ... Q6AFE_LPASS_CLK_ID_QUIN_TDM_EBIT:
-		return q6afe_port_set_sysclk(port, clk_id,
-					     Q6AFE_LPASS_CLK_ATTRIBUTE_INVERT_COUPLE_NO,
-					     Q6AFE_LPASS_CLK_ROOT_DEFAULT,
-					     freq, dir);
+		clk_src = Q6AFE_LPASS_CLK_ATTRIBUTE_INVERT_COUPLE_NO;
+		clk_root = Q6AFE_LPASS_CLK_ROOT_DEFAULT;
+		break;
+	default:
+		return 0;
 	}
 
-	return 0;
+	return q6afe_port_set_sysclk(port, clk_id, clk_src, clk_root, freq, dir);
 }
 
 static const struct snd_soc_dapm_route q6afe_dapm_routes[] = {
-- 
2.39.2

[PATCH v2 4/4] ASoC: qcom: q6afe: remove "port already open" error

[Otto Pflüger]

The clock fallback for older firmware versions now represents a use case
for having multiple references to a port. Stop logging an error when a
port is requested more than once because this does not indicate a problem
anymore and causes unnecessary spam in dmesg.

Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
 sound/soc/qcom/qdsp6/q6afe.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
index f14d3b366aa4..56b1407c30bb 100644
--- a/sound/soc/qcom/qdsp6/q6afe.c
+++ b/sound/soc/qcom/qdsp6/q6afe.c
@@ -1592,10 +1592,8 @@ struct q6afe_port *q6afe_port_get_from_id(struct device *dev, int id)
 
 	/* if port is multiple times bind/unbind before callback finishes */
 	port = q6afe_find_port(afe, id);
-	if (port) {
-		dev_err(dev, "AFE Port already open\n");
+	if (port)
 		return port;
-	}
 
 	port_id = port_maps[id].port_id;
 
-- 
2.39.2

Re: [PATCH v2 3/4] ASoC: qcom: q6afe-dai: check ADSP version when setting sysclk

[Srinivas Kandagatla]

Thanks Otto for the patches.

On 29/10/2023 16:57, Otto Pflüger wrote:
> Some older DSP firmware versions only provide AFE_PARAM_ID_*_CLK_CONFIG
> requests for setting clocks, while newer ones only provide the
> incompatible AFE_PARAM_ID_CLOCK_SET. The q6afe driver implements both,
> but requires different clock IDs for the different firmware versions.
> 
> Implement the LPAIF_*_CLK clocks using newer clock IDs when the DSP
> firmware does not support the old clocks so that users don't have to
> care about the firmware version when setting clocks.
> 
> Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
> ---
>   sound/soc/qcom/qdsp6/q6afe-dai.c | 98 +++++++++++++++++++++++++++-----
>   1 file changed, 84 insertions(+), 14 deletions(-)
> 
> diff --git a/sound/soc/qcom/qdsp6/q6afe-dai.c b/sound/soc/qcom/qdsp6/q6afe-dai.c
> index a9c4f896a7df..c66f8ab41d5e 100644
> --- a/sound/soc/qcom/qdsp6/q6afe-dai.c
> +++ b/sound/soc/qcom/qdsp6/q6afe-dai.c
> @@ -14,6 +14,7 @@
>   #include <sound/pcm_params.h>
>   #include "q6dsp-lpass-ports.h"
>   #include "q6dsp-common.h"
> +#include "q6core.h"
>   #include "q6afe.h"
>   
>   
> @@ -443,36 +444,105 @@ static int q6slim_set_channel_map(struct snd_soc_dai *dai,
>   	return 0;
>   }
>   
> +static int q6afe_get_bit_clk_id(unsigned int dai_id)
> +{
> +	switch (dai_id) {
> +	case PRIMARY_MI2S_RX:
> +	case PRIMARY_MI2S_TX:
> +		return Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT;
> +	case SECONDARY_MI2S_RX:
> +	case SECONDARY_MI2S_TX:
> +		return Q6AFE_LPASS_CLK_ID_SEC_MI2S_IBIT;
> +	case TERTIARY_MI2S_RX:
> +	case TERTIARY_MI2S_TX:
> +		return Q6AFE_LPASS_CLK_ID_TER_MI2S_IBIT;
> +	case QUATERNARY_MI2S_RX:
> +	case QUATERNARY_MI2S_TX:
> +		return Q6AFE_LPASS_CLK_ID_TER_MI2S_IBIT;
> +	case QUINARY_MI2S_RX:
> +	case QUINARY_MI2S_TX:
> +		return Q6AFE_LPASS_CLK_ID_QUI_MI2S_IBIT;
> +
> +	case PRIMARY_TDM_RX_0 ... PRIMARY_TDM_TX_7:
> +		return Q6AFE_LPASS_CLK_ID_PRI_TDM_IBIT;
> +	case SECONDARY_TDM_RX_0 ... SECONDARY_TDM_TX_7:
> +		return Q6AFE_LPASS_CLK_ID_SEC_TDM_IBIT;
> +	case TERTIARY_TDM_RX_0 ... TERTIARY_TDM_TX_7:
> +		return Q6AFE_LPASS_CLK_ID_TER_TDM_IBIT;
> +	case QUATERNARY_TDM_RX_0 ... QUATERNARY_TDM_TX_7:
> +		return Q6AFE_LPASS_CLK_ID_QUAD_TDM_IBIT;
> +	case QUINARY_TDM_RX_0 ... QUINARY_TDM_TX_7:
> +		return Q6AFE_LPASS_CLK_ID_QUIN_TDM_IBIT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int q6afe_get_osr_clk_id(unsigned int dai_id)
> +{
> +	switch (dai_id) {
> +	case QUINARY_MI2S_RX:
> +	case QUINARY_MI2S_TX:
> +		return Q6AFE_LPASS_CLK_ID_QUI_MI2S_OSR;
> +
> +	case QUINARY_TDM_RX_0 ... QUINARY_TDM_TX_7:
> +		return Q6AFE_LPASS_CLK_ID_QUIN_TDM_OSR;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>   static int q6afe_mi2s_set_sysclk(struct snd_soc_dai *dai,
>   		int clk_id, unsigned int freq, int dir)
>   {
>   	struct q6afe_dai_data *dai_data = dev_get_drvdata(dai->dev);
>   	struct q6afe_port *port = dai_data->port[dai->id];
> +	int clk_src;
> +	int clk_root;
> +	bool use_new_clks = q6core_get_adsp_version() >= Q6_ADSP_VERSION_2_7;
>   
>   	switch (clk_id) {
>   	case LPAIF_DIG_CLK:
> -		return q6afe_port_set_sysclk(port, clk_id, 0, 5, freq, dir);
> +		if (use_new_clks) {
> +			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;
Why clk id is changed ? isn't machine driver passing the correct clk id?

> +		} else {
> +			clk_src = 0;
> +			clk_root = 5;
> +		}

> +		break;
>   	case LPAIF_BIT_CLK:
>   	case LPAIF_OSR_CLK:
> -		return q6afe_port_set_sysclk(port, clk_id,
> -					     Q6AFE_LPASS_CLK_SRC_INTERNAL,
> -					     Q6AFE_LPASS_CLK_ROOT_DEFAULT,
> -					     freq, dir);
> +		if (use_new_clks) {
> +			clk_src = Q6AFE_LPASS_CLK_ATTRIBUTE_COUPLE_NO;
> +			clk_root = Q6AFE_LPASS_CLK_ROOT_DEFAULT;
> +			if (clk_id == LPAIF_OSR_CLK)
> +				clk_id = q6afe_get_osr_clk_id(dai->id);
> +			else
> +				clk_id = q6afe_get_bit_clk_id(dai->id);

same comment, why cant the machine driver pass correct clk id?

This exactly like calling

snd_soc_dai_set_sysclk(cpu_dai, 
Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT, 
MI2S_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);

from machine driver for PRIM_MI2S bit clk and simillar for OSR.


--srini


> +		} else {
> +			clk_src = Q6AFE_LPASS_CLK_SRC_INTERNAL;
> +			clk_root = Q6AFE_LPASS_CLK_ROOT_DEFAULT;
> +		}
> +		break;
>   	case Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT ... Q6AFE_LPASS_CLK_ID_QUI_MI2S_OSR:
>   	case Q6AFE_LPASS_CLK_ID_MCLK_1 ... Q6AFE_LPASS_CLK_ID_INT_MCLK_1:
>   	case Q6AFE_LPASS_CLK_ID_WSA_CORE_MCLK ... Q6AFE_LPASS_CLK_ID_VA_CORE_2X_MCLK:
> -		return q6afe_port_set_sysclk(port, clk_id,
> -					     Q6AFE_LPASS_CLK_ATTRIBUTE_COUPLE_NO,
> -					     Q6AFE_LPASS_CLK_ROOT_DEFAULT,
> -					     freq, dir);
> +		clk_src = Q6AFE_LPASS_CLK_ATTRIBUTE_COUPLE_NO;
> +		clk_root = Q6AFE_LPASS_CLK_ROOT_DEFAULT;
> +		break;
>   	case Q6AFE_LPASS_CLK_ID_PRI_TDM_IBIT ... Q6AFE_LPASS_CLK_ID_QUIN_TDM_EBIT:
> -		return q6afe_port_set_sysclk(port, clk_id,
> -					     Q6AFE_LPASS_CLK_ATTRIBUTE_INVERT_COUPLE_NO,
> -					     Q6AFE_LPASS_CLK_ROOT_DEFAULT,
> -					     freq, dir);
> +		clk_src = Q6AFE_LPASS_CLK_ATTRIBUTE_INVERT_COUPLE_NO;
> +		clk_root = Q6AFE_LPASS_CLK_ROOT_DEFAULT;
> +		break;
> +	default:
> +		return 0;
>   	}
>   
> -	return 0;
> +	return q6afe_port_set_sysclk(port, clk_id, clk_src, clk_root, freq, dir);
>   }
>   
>   static const struct snd_soc_dapm_route q6afe_dapm_routes[] = {

Re: [PATCH v2 1/4] ASoC: qcom: q6core: expose ADSP core firmware version

[Srinivas Kandagatla]

Thanks Otto for the patches.

On 29/10/2023 16:57, Otto Pflüger wrote:
> Add a q6core_get_adsp_version() function that returns the version of the
> ADSP firmware (2.6, 2.7 or 2.8), also known as the AVS version (see [1]
> in downstream kernel).
> 
> Some APIs differ between these versions, e.g. the AFE clock APIs.
> 
> [1]: https://github.com/msm8916-mainline/linux-downstream/blob/LA.BR.1.2.9.1_rb1.5/sound/soc/msm/qdsp6v2/q6core.c#L193
> 
> Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
> ---
>   sound/soc/qcom/qdsp6/q6core.c | 65 +++++++++++++++++++++++++++++++++++
>   sound/soc/qcom/qdsp6/q6core.h |  9 +++++
>   2 files changed, 74 insertions(+)
> 
> diff --git a/sound/soc/qcom/qdsp6/q6core.c b/sound/soc/qcom/qdsp6/q6core.c
> index 49cfb32cd209..855ab4ff1e59 100644
> --- a/sound/soc/qcom/qdsp6/q6core.c
> +++ b/sound/soc/qcom/qdsp6/q6core.c
> @@ -20,6 +20,9 @@
>   #define AVCS_CMDRSP_ADSP_EVENT_GET_STATE	0x0001290D
>   #define AVCS_GET_VERSIONS       0x00012905
>   #define AVCS_GET_VERSIONS_RSP   0x00012906
> +#define AVCS_CMDRSP_Q6_ID_2_6	0x00040000
> +#define AVCS_CMDRSP_Q6_ID_2_7	0x00040001
> +#define AVCS_CMDRSP_Q6_ID_2_8	0x00040002
>   #define AVCS_CMD_GET_FWK_VERSION	0x001292c
>   #define AVCS_CMDRSP_GET_FWK_VERSION	0x001292d
>   
> @@ -63,6 +66,7 @@ struct q6core {
>   	bool get_state_supported;
>   	bool get_version_supported;
>   	bool is_version_requested;
> +	enum q6core_version adsp_version;

May be rename this to just api_version would more readable than 
adsp_Version.

>   };
>   
>   static struct q6core *g_core;
> @@ -108,6 +112,14 @@ static int q6core_callback(struct apr_device *adev, struct apr_resp_pkt *data)
>   		if (!core->fwk_version)
>   			return -ENOMEM;
>   
> +		/*
> +		 * Since GET_VERSIONS is not called when GET_FWK_VERSION
> +		 * is successful and these commands may return completely
> +		 * different versions, assume that the version is 2.8 here.
> +		 * Older versions do not support GET_FWK_VERSION and we do
> +		 * not care if the version is newer than 2.8.
> +		 */
> +		core->adsp_version = Q6_ADSP_VERSION_2_8;
>   		core->fwk_version_supported = true;
>   		core->resp_received = true;
>   
> @@ -115,6 +127,7 @@ static int q6core_callback(struct apr_device *adev, struct apr_resp_pkt *data)
>   	}
>   	case AVCS_GET_VERSIONS_RSP: {
>   		struct avcs_cmdrsp_get_version *v;
> +		int i;
>   
>   		v = data->payload;
>   
> @@ -125,6 +138,32 @@ static int q6core_callback(struct apr_device *adev, struct apr_resp_pkt *data)
>   		if (!core->svc_version)
>   			return -ENOMEM;
>   
> +		for (i = 0; i < g_core->svc_version->num_services; i++) {

We have two places where the api info is stored based on adsp version, 
g_core->fwk_version->num_services and g_core->svc_version->num_services
this code is only going thru one list which will not work as expected in 
some revisions of the firmware.

> +			struct avcs_svc_info *info;
> +
> +			info = &g_core->svc_version->svc_api_info[i];
> +			if (info->service_id != APR_SVC_ADSP_CORE)
> +				continue;
> +
> +			switch (info->version) {
> +			case AVCS_CMDRSP_Q6_ID_2_6:
> +				core->adsp_version = Q6_ADSP_VERSION_2_6;
> +				break;
> +			case AVCS_CMDRSP_Q6_ID_2_7:
> +				core->adsp_version = Q6_ADSP_VERSION_2_7;
> +				break;
> +			case AVCS_CMDRSP_Q6_ID_2_8:
> +				core->adsp_version = Q6_ADSP_VERSION_2_8;
> +				break;
> +			default:
> +				dev_err(&adev->dev, "Unknown AVS version 0x%08x\n",
> +					info->version);


> +				break;
> +			}
> +
> +			break;
> +		}
> +
>   		core->get_version_supported = true;
>   		core->resp_received = true;
>   
> @@ -293,6 +332,31 @@ int q6core_get_svc_api_info(int svc_id, struct q6core_svc_api_info *ainfo)
>   }
>   EXPORT_SYMBOL_GPL(q6core_get_svc_api_info);
>   
> +/**
> + * q6core_get_adsp_version() - Get the core version number.
> + *
> + * Return: version code or Q6_ADSP_VERSION_UNKNOWN on failure
> + */
> +enum q6core_version q6core_get_adsp_version(void)
> +{
> +	int ret;
> +
> +	if (!g_core)
> +		return Q6_ADSP_VERSION_UNKNOWN;
> +
> +	mutex_lock(&g_core->lock);
> +	if (!g_core->is_version_requested) {
> +		if (q6core_get_fwk_versions(g_core) == -ENOTSUPP)
> +			q6core_get_svc_versions(g_core);
> +		g_core->is_version_requested = true;
> +	}
> +	ret = g_core->adsp_version;
> +	mutex_unlock(&g_core->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(q6core_get_adsp_version);
> +
>   /**
>    * q6core_is_adsp_ready() - Get status of adsp
>    *
> @@ -334,6 +398,7 @@ static int q6core_probe(struct apr_device *adev)
>   	dev_set_drvdata(&adev->dev, g_core);
>   
>   	mutex_init(&g_core->lock);
> +	g_core->adsp_version = Q6_ADSP_VERSION_UNKNOWN;
>   	g_core->adev = adev;
>   	init_waitqueue_head(&g_core->wait);
>   	return 0;
> diff --git a/sound/soc/qcom/qdsp6/q6core.h b/sound/soc/qcom/qdsp6/q6core.h
> index 4105b1d730be..472e06bf8efc 100644
> --- a/sound/soc/qcom/qdsp6/q6core.h
> +++ b/sound/soc/qcom/qdsp6/q6core.h
> @@ -9,7 +9,16 @@ struct q6core_svc_api_info {
>   	uint32_t api_branch_version;
>   };
>   
> +/* Versions must be in order! */
> +enum q6core_version {
> +	Q6_ADSP_VERSION_UNKNOWN,
> +	Q6_ADSP_VERSION_2_6,
> +	Q6_ADSP_VERSION_2_7,
> +	Q6_ADSP_VERSION_2_8,
> +};
> +
>   bool q6core_is_adsp_ready(void);
> +enum q6core_version q6core_get_adsp_version(void);
>   int q6core_get_svc_api_info(int svc_id, struct q6core_svc_api_info *ainfo);
>   
>   #endif /* __Q6CORE_H__ */

Re: [PATCH v2 2/4] ASoC: qcom: q6afe: provide fallback for digital codec clock

[Srinivas Kandagatla]

Thanks Otto for the patch,

On 29/10/2023 16:57, Otto Pflüger wrote:
> When q6afe is used as a clock provider through q6afe-clocks.c, it uses
> an interface for setting clocks that is not present in older firmware
> versions. However, using Q6AFE_LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
> as the codec MCLK in the device tree can be useful on older platforms
> too. Provide a fallback that sets this clock using the old method when
> an old firmware version is detected.
> 
> MSM8916 did not need this because of a workaround that controls this
> clock directly through the GCC driver, but newer SoCs do not have this
> clock in their GCC drivers because it is meant to be controlled by the
> DSP.
> 
> Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
> ---
>   sound/soc/qcom/qdsp6/q6afe.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
> index 91d39f6ad0bd..f14d3b366aa4 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);

This redirection of clks looks totally confusing and hacky.
Why can not we do this from machine driver based something like this:

if (q6core_get_adsp_version() < Q6_ADSP_VERSION_2_7)
	ret = snd_soc_dai_set_sysclk(cpu_dai, LPAIF_DIG_CLK, freq, 0);



--srini
> +		return ret;
> +	}
> +
>   	cset.clk_set_minor_version = AFE_API_VERSION_CLOCK_SET;
>   	cset.clk_id = clk_id;
>   	cset.clk_freq_in_hz = freq;

Re: [PATCH v2 2/4] ASoC: qcom: q6afe: provide fallback for digital codec clock

[Stephan Gerhold]

Hi Srini,

On Fri, Nov 17, 2023 at 01:26:43PM +0000, Srinivas Kandagatla wrote:
> On 29/10/2023 16:57, Otto Pflüger wrote:
> > When q6afe is used as a clock provider through q6afe-clocks.c, it uses
> > an interface for setting clocks that is not present in older firmware
> > versions. However, using Q6AFE_LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
> > as the codec MCLK in the device tree can be useful on older platforms
> > too. Provide a fallback that sets this clock using the old method when
> > an old firmware version is detected.
> > 
> > MSM8916 did not need this because of a workaround that controls this
> > clock directly through the GCC driver, but newer SoCs do not have this
> > clock in their GCC drivers because it is meant to be controlled by the
> > DSP.
> > 
> > Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
> > ---
> >   sound/soc/qcom/qdsp6/q6afe.c | 26 ++++++++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> > 
> > diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
> > index 91d39f6ad0bd..f14d3b366aa4 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);
> 
> This redirection of clks looks totally confusing and hacky.
> Why can not we do this from machine driver based something like this:
> 
> if (q6core_get_adsp_version() < Q6_ADSP_VERSION_2_7)
> 	ret = snd_soc_dai_set_sysclk(cpu_dai, LPAIF_DIG_CLK, freq, 0);
> 

Unfortunately this doesn't work for the digital codec clock. This clock
is consumed and dynamically controlled by the msm8916-wcd-digital driver
via the clk subsystem, not via ASoC sysclks.

Using q6afe-clocks a typical setup in the DT would look like:

	lpass_codec: audio-codec@771c000 {
		compatible = "qcom,msm8916-wcd-digital-codec";
		reg = <0x0771c000 0x400>;
		clocks = <&xo_board>,
			 <&q6afecc LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
				   LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
		clock-names = "ahbix-clk", "mclk";
		#sound-dai-cells = <1>;
	};

However, this works only for newer firmware versions (>= 2.7), not for
older firmware versions controlling this clock via LPAIF_DIG_CLK.

We need a way to describe this independent from the firmware version in
the DT, since there are SoCs such as MSM8909 where both firmware
versions have been used (perhaps even on the same device depending on
which firmware version you have installed).

Any solution involving the machine driver will inevitably end up in a
"chicken-and-egg" problem: The machine driver needs the codec drivers to
probe successfully, but the msm8916-wcd-digital driver first needs the
digital codec clock (mclk) to probe successfully.

I agree that these kind of checks in the qdsp6 code are a bit hacky, but
in my opinion it's still far better than exposing this firmware
detection problem to the device tree.

Do you see any other straightforward way to solve this for the digital
codec clock consumed by msm8916-wcd-digital?

Thanks,
Stephan

Re: [PATCH v2 2/4] ASoC: qcom: q6afe: provide fallback for digital codec clock

[Srinivas Kandagatla]

Hi Stephan,

On 04/12/2023 10:34, Stephan Gerhold wrote:
> Hi Srini,
> 
> On Fri, Nov 17, 2023 at 01:26:43PM +0000, Srinivas Kandagatla wrote:
>> On 29/10/2023 16:57, Otto Pflüger wrote:
>>> When q6afe is used as a clock provider through q6afe-clocks.c, it uses
>>> an interface for setting clocks that is not present in older firmware
>>> versions. However, using Q6AFE_LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
>>> as the codec MCLK in the device tree can be useful on older platforms
>>> too. Provide a fallback that sets this clock using the old method when
>>> an old firmware version is detected.
>>>
>>> MSM8916 did not need this because of a workaround that controls this
>>> clock directly through the GCC driver, but newer SoCs do not have this
>>> clock in their GCC drivers because it is meant to be controlled by the
>>> DSP.
>>>
>>> Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
>>> ---
>>>    sound/soc/qcom/qdsp6/q6afe.c | 26 ++++++++++++++++++++++++++
>>>    1 file changed, 26 insertions(+)
>>>
>>> diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
>>> index 91d39f6ad0bd..f14d3b366aa4 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);
>>
>> This redirection of clks looks totally confusing and hacky.
>> Why can not we do this from machine driver based something like this:
>>
>> if (q6core_get_adsp_version() < Q6_ADSP_VERSION_2_7)
>> 	ret = snd_soc_dai_set_sysclk(cpu_dai, LPAIF_DIG_CLK, freq, 0);
>>
> 
> Unfortunately this doesn't work for the digital codec clock. This clock
> is consumed and dynamically controlled by the msm8916-wcd-digital driver
> via the clk subsystem, not via ASoC sysclks.

Thankyou for explaining this, I see the issue, older firmware versions 
do not support setting internal digital codec core clock via 
AFE_PARAM_ID_CLOCK_SET.

we could do a better job here to allow NULL port arguments to 
q6afe_port_set_param_v2 and make this look much better.

Could you also add asoc mailing list in CC, I somehow missed these emails.


something like this should be much cleaner:

--------------------------------------->cut<----------------------------------

diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
index 91d39f6ad0bd..8dec2c6c7bba 100644
--- a/sound/soc/qcom/qdsp6/q6afe.c
+++ b/sound/soc/qcom/qdsp6/q6afe.c
@@ -1031,14 +1031,13 @@ static int q6afe_port_set_param(struct 
q6afe_port *port, void *data,
                                psize, port->token);
  }

-static int q6afe_port_set_param_v2(struct q6afe_port *port, void *data,
+static int q6afe_port_set_param_v2(struct q6afe *afe, struct q6afe_port 
*port, void *data,
                                    int param_id, int module_id, int psize)
  {
         struct afe_port_cmd_set_param_v2 *param;
         struct afe_port_param_data_v2 *pdata;
-       struct q6afe *afe = port->afe;
         struct apr_pkt *pkt;
-       u16 port_id = port->id;
+       u16 port_id = port ? port->id : 0;
         int ret, pkt_size;
         void *p, *pl;

@@ -1059,7 +1058,7 @@ static int q6afe_port_set_param_v2(struct 
q6afe_port *port, void *data,
         pkt->hdr.pkt_size = pkt_size;
         pkt->hdr.src_port = 0;
         pkt->hdr.dest_port = 0;
-       pkt->hdr.token = port->token;
+       pkt->hdr.token = port ? port->token : 0;
         pkt->hdr.opcode = AFE_PORT_CMD_SET_PARAM_V2;

         param->port_id = port_id;
@@ -1083,7 +1082,7 @@ static int q6afe_port_set_param_v2(struct 
q6afe_port *port, void *data,
  static int q6afe_port_set_lpass_clock(struct q6afe_port *port,
                                  struct afe_clk_cfg *cfg)
  {
-       return q6afe_port_set_param_v2(port, cfg,
+       return q6afe_port_set_param_v2(port->afe, port, cfg,
                                        AFE_PARAM_ID_LPAIF_CLK_CONFIG,
                                        AFE_MODULE_AUDIO_DEV_INTERFACE,
                                        sizeof(*cfg));
@@ -1099,7 +1098,7 @@ static int q6afe_set_lpass_clock_v2(struct 
q6afe_port *port,
  static int q6afe_set_digital_codec_core_clock(struct q6afe_port *port,
                                               struct 
afe_digital_clk_cfg *cfg)
  {
-       return q6afe_port_set_param_v2(port, cfg,
+       return q6afe_port_set_param_v2(port->afe, port, cfg,
 
AFE_PARAM_ID_INT_DIGITAL_CDC_CLK_CONFIG,
                                        AFE_MODULE_AUDIO_DEV_INTERFACE,
                                        sizeof(*cfg));
@@ -1117,6 +1116,22 @@ int q6afe_set_lpass_clock(struct device *dev, int 
clk_id, int attri,
         cset.clk_attri = attri;
         cset.clk_root = clk_root;
         cset.enable = !!freq;
+       if (q6core_geti_adsp_version() < Q6_ADSP_VERSION_2_7) {
+               struct afe_digital_clk_cfg dcfg = {0,};
+
+               switch (clk_id) {
+               /* DSP firmware versions below 2.7 do not support 
configuring internal
+                * digital codec core clk using AFE_PARAM_ID_CLOCK_SET */
+               case: Q6AFE_LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE:
+                       dcfg.i2s_cfg_minor_version = 
AFE_API_VERSION_I2S_CONFIG;
+                       dcfg.clk_val = freq;
+                       dcfg.clk_root = 5;
+                       return q6afe_set_digital_codec_core_clock(port, 
&dcfg);
+
+               default:
+                     break;
+               }
+       }

         return q6afe_set_param(afe, NULL, &cset, AFE_PARAM_ID_CLOCK_SET,
                                AFE_MODULE_CLOCK_SET, sizeof(cset),
@@ -1493,7 +1508,7 @@ int q6afe_port_start(struct q6afe_port *port)
         int pkt_size;
         void *p;

-       ret  = q6afe_port_set_param_v2(port, &port->port_cfg, param_id,
+       ret  = q6afe_port_set_param_v2(afe, port, &port->port_cfg, param_id,
                                        AFE_MODULE_AUDIO_DEV_INTERFACE,
                                        sizeof(port->port_cfg));
         if (ret) {
@@ -1503,7 +1518,7 @@ int q6afe_port_start(struct q6afe_port *port)
         }

         if (port->scfg) {
-               ret  = q6afe_port_set_param_v2(port, port->scfg,
+               ret  = q6afe_port_set_param_v2(afe, port, port->scfg,
 
AFE_PARAM_ID_PORT_SLOT_MAPPING_CONFIG,
                                         AFE_MODULE_TDM, 
sizeof(*port->scfg));
                 if (ret) {

--------------------------------------->cut<----------------------------------

Thanks,
Srini

> 
> Using q6afe-clocks a typical setup in the DT would look like:
> 
> 	lpass_codec: audio-codec@771c000 {
> 		compatible = "qcom,msm8916-wcd-digital-codec";
> 		reg = <0x0771c000 0x400>;
> 		clocks = <&xo_board>,
> 			 <&q6afecc LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
> 				   LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
> 		clock-names = "ahbix-clk", "mclk";
> 		#sound-dai-cells = <1>;
> 	};
> 
> However, this works only for newer firmware versions (>= 2.7), not for
> older firmware versions controlling this clock via LPAIF_DIG_CLK.
> 
> We need a way to describe this independent from the firmware version in
> the DT, since there are SoCs such as MSM8909 where both firmware
> versions have been used (perhaps even on the same device depending on
> which firmware version you have installed).
> 
> Any solution involving the machine driver will inevitably end up in a
> "chicken-and-egg" problem: The machine driver needs the codec drivers to
> probe successfully, but the msm8916-wcd-digital driver first needs the
> digital codec clock (mclk) to probe successfully.
> 
> I agree that these kind of checks in the qdsp6 code are a bit hacky, but
> in my opinion it's still far better than exposing this firmware
> detection problem to the device tree.
> 
> Do you see any other straightforward way to solve this for the digital
> codec clock consumed by msm8916-wcd-digital?
> 
> Thanks,
> Stephan