All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] ASoC: qdsp6: fixes and improve error reporting
@ 2021-11-16 11:47 ` Srinivas Kandagatla
  0 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie
  Cc: lgirdwood, perex, tiwai, pierre-louis.bossart, alsa-devel,
	linux-kernel, Srinivas Kandagatla

Hi Mark,

During testing on a laptop we found few issues with the existing q6dsp code, 
first 3 patches should fix these issues. Also during debug we found that
some of the errors reported are not very useful, so update error reporting
in two places.

Thanks,
srini

Srinivas Kandagatla (5):
  ASoC: qdsp6: qdsp6: q6prm: handle clk disable correctly
  ASoC: qdsp6: q6routing: Conditionally reset FrontEnd Mixer
  ASoC: qdsp6: q6asm: fix q6asm_dai_prepare error handling
  ASoC: qdsp6: q6adm: improve error reporting
  ASoC: qdsp6: q6routing: validate port id before setting up route

 sound/soc/qcom/qdsp6/audioreach.h |  4 +++
 sound/soc/qcom/qdsp6/q6adm.c      |  4 +--
 sound/soc/qcom/qdsp6/q6asm-dai.c  | 19 +++++++----
 sound/soc/qcom/qdsp6/q6prm.c      | 53 +++++++++++++++++++++++++++++--
 sound/soc/qcom/qdsp6/q6routing.c  | 12 ++++++-
 5 files changed, 81 insertions(+), 11 deletions(-)

-- 
2.21.0


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

* [PATCH 0/5] ASoC: qdsp6: fixes and improve error reporting
@ 2021-11-16 11:47 ` Srinivas Kandagatla
  0 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie; +Cc: alsa-devel, lgirdwood, linux-kernel, pierre-louis.bossart, tiwai

Hi Mark,

During testing on a laptop we found few issues with the existing q6dsp code, 
first 3 patches should fix these issues. Also during debug we found that
some of the errors reported are not very useful, so update error reporting
in two places.

Thanks,
srini

Srinivas Kandagatla (5):
  ASoC: qdsp6: qdsp6: q6prm: handle clk disable correctly
  ASoC: qdsp6: q6routing: Conditionally reset FrontEnd Mixer
  ASoC: qdsp6: q6asm: fix q6asm_dai_prepare error handling
  ASoC: qdsp6: q6adm: improve error reporting
  ASoC: qdsp6: q6routing: validate port id before setting up route

 sound/soc/qcom/qdsp6/audioreach.h |  4 +++
 sound/soc/qcom/qdsp6/q6adm.c      |  4 +--
 sound/soc/qcom/qdsp6/q6asm-dai.c  | 19 +++++++----
 sound/soc/qcom/qdsp6/q6prm.c      | 53 +++++++++++++++++++++++++++++--
 sound/soc/qcom/qdsp6/q6routing.c  | 12 ++++++-
 5 files changed, 81 insertions(+), 11 deletions(-)

-- 
2.21.0


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

* [PATCH 1/5] ASoC: qdsp6: qdsp6: q6prm: handle clk disable correctly
  2021-11-16 11:47 ` Srinivas Kandagatla
@ 2021-11-16 11:47   ` Srinivas Kandagatla
  -1 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie
  Cc: lgirdwood, perex, tiwai, pierre-louis.bossart, alsa-devel,
	linux-kernel, Srinivas Kandagatla

Q6PRM clks need to be disabled using PRM_CMD_RELEASE_HW_RSC dsp command
rather then using PRM_CMD_RSP_REQUEST_HW_RSC cmd with rate set to zero.

DSP will throw errors if we try to disable the clock using existing code.

Fix this by properly handling the clk release.

Fixes: 9a0e5d6fb16f ("ASoC: qdsp6: audioreach: add q6prm support")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/audioreach.h |  4 +++
 sound/soc/qcom/qdsp6/q6prm.c      | 53 +++++++++++++++++++++++++++++--
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
index 4f693a2660b5..3ee8bfcd0121 100644
--- a/sound/soc/qcom/qdsp6/audioreach.h
+++ b/sound/soc/qcom/qdsp6/audioreach.h
@@ -550,6 +550,10 @@ struct audio_hw_clk_cfg {
 	uint32_t clock_root;
 } __packed;
 
+struct audio_hw_clk_rel_cfg {
+	uint32_t clock_id;
+} __packed;
+
 #define PARAM_ID_HW_EP_POWER_MODE_CFG	0x8001176
 #define AR_HW_EP_POWER_MODE_0	0 /* default */
 #define AR_HW_EP_POWER_MODE_1	1 /* XO Shutdown allowed */
diff --git a/sound/soc/qcom/qdsp6/q6prm.c b/sound/soc/qcom/qdsp6/q6prm.c
index 82c40f2d4e1d..cda33ded29be 100644
--- a/sound/soc/qcom/qdsp6/q6prm.c
+++ b/sound/soc/qcom/qdsp6/q6prm.c
@@ -42,6 +42,12 @@ struct prm_cmd_request_rsc {
 	struct audio_hw_clk_cfg clock_id;
 } __packed;
 
+struct prm_cmd_release_rsc {
+	struct apm_module_param_data param_data;
+	uint32_t num_clk_id;
+	struct audio_hw_clk_rel_cfg clock_id;
+} __packed;
+
 static int q6prm_send_cmd_sync(struct q6prm *prm, struct gpr_pkt *pkt, uint32_t rsp_opcode)
 {
 	return audioreach_send_cmd_sync(prm->dev, prm->gdev, &prm->result, &prm->lock,
@@ -102,8 +108,8 @@ int q6prm_unvote_lpass_core_hw(struct device *dev, uint32_t hw_block_id, uint32_
 }
 EXPORT_SYMBOL_GPL(q6prm_unvote_lpass_core_hw);
 
-int q6prm_set_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root,
-			  unsigned int freq)
+static int q6prm_request_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root,
+				     unsigned int freq)
 {
 	struct q6prm *prm = dev_get_drvdata(dev->parent);
 	struct apm_module_param_data *param_data;
@@ -138,6 +144,49 @@ int q6prm_set_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_
 
 	return rc;
 }
+
+static int q6prm_release_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root,
+			  unsigned int freq)
+{
+	struct q6prm *prm = dev_get_drvdata(dev->parent);
+	struct apm_module_param_data *param_data;
+	struct prm_cmd_release_rsc *rel;
+	gpr_device_t *gdev = prm->gdev;
+	struct gpr_pkt *pkt;
+	int rc;
+
+	pkt = audioreach_alloc_cmd_pkt(sizeof(*rel), PRM_CMD_RELEASE_HW_RSC, 0, gdev->svc.id,
+				       GPR_PRM_MODULE_IID);
+	if (IS_ERR(pkt))
+		return PTR_ERR(pkt);
+
+	rel = (void *)pkt + GPR_HDR_SIZE + APM_CMD_HDR_SIZE;
+
+	param_data = &rel->param_data;
+
+	param_data->module_instance_id = GPR_PRM_MODULE_IID;
+	param_data->error_code = 0;
+	param_data->param_id = PARAM_ID_RSC_AUDIO_HW_CLK;
+	param_data->param_size = sizeof(*rel) - APM_MODULE_PARAM_DATA_SIZE;
+
+	rel->num_clk_id = 1;
+	rel->clock_id.clock_id = clk_id;
+
+	rc = q6prm_send_cmd_sync(prm, pkt, PRM_CMD_RSP_RELEASE_HW_RSC);
+
+	kfree(pkt);
+
+	return rc;
+}
+
+int q6prm_set_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root,
+			  unsigned int freq)
+{
+	if (freq)
+		return q6prm_request_lpass_clock(dev, clk_id, clk_attr, clk_attr, freq);
+
+	return q6prm_release_lpass_clock(dev, clk_id, clk_attr, clk_attr, freq);
+}
 EXPORT_SYMBOL_GPL(q6prm_set_lpass_clock);
 
 static int prm_callback(struct gpr_resp_pkt *data, void *priv, int op)
-- 
2.21.0


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

* [PATCH 1/5] ASoC: qdsp6: qdsp6: q6prm: handle clk disable correctly
@ 2021-11-16 11:47   ` Srinivas Kandagatla
  0 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie; +Cc: alsa-devel, lgirdwood, linux-kernel, pierre-louis.bossart, tiwai

Q6PRM clks need to be disabled using PRM_CMD_RELEASE_HW_RSC dsp command
rather then using PRM_CMD_RSP_REQUEST_HW_RSC cmd with rate set to zero.

DSP will throw errors if we try to disable the clock using existing code.

Fix this by properly handling the clk release.

Fixes: 9a0e5d6fb16f ("ASoC: qdsp6: audioreach: add q6prm support")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/audioreach.h |  4 +++
 sound/soc/qcom/qdsp6/q6prm.c      | 53 +++++++++++++++++++++++++++++--
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
index 4f693a2660b5..3ee8bfcd0121 100644
--- a/sound/soc/qcom/qdsp6/audioreach.h
+++ b/sound/soc/qcom/qdsp6/audioreach.h
@@ -550,6 +550,10 @@ struct audio_hw_clk_cfg {
 	uint32_t clock_root;
 } __packed;
 
+struct audio_hw_clk_rel_cfg {
+	uint32_t clock_id;
+} __packed;
+
 #define PARAM_ID_HW_EP_POWER_MODE_CFG	0x8001176
 #define AR_HW_EP_POWER_MODE_0	0 /* default */
 #define AR_HW_EP_POWER_MODE_1	1 /* XO Shutdown allowed */
diff --git a/sound/soc/qcom/qdsp6/q6prm.c b/sound/soc/qcom/qdsp6/q6prm.c
index 82c40f2d4e1d..cda33ded29be 100644
--- a/sound/soc/qcom/qdsp6/q6prm.c
+++ b/sound/soc/qcom/qdsp6/q6prm.c
@@ -42,6 +42,12 @@ struct prm_cmd_request_rsc {
 	struct audio_hw_clk_cfg clock_id;
 } __packed;
 
+struct prm_cmd_release_rsc {
+	struct apm_module_param_data param_data;
+	uint32_t num_clk_id;
+	struct audio_hw_clk_rel_cfg clock_id;
+} __packed;
+
 static int q6prm_send_cmd_sync(struct q6prm *prm, struct gpr_pkt *pkt, uint32_t rsp_opcode)
 {
 	return audioreach_send_cmd_sync(prm->dev, prm->gdev, &prm->result, &prm->lock,
@@ -102,8 +108,8 @@ int q6prm_unvote_lpass_core_hw(struct device *dev, uint32_t hw_block_id, uint32_
 }
 EXPORT_SYMBOL_GPL(q6prm_unvote_lpass_core_hw);
 
-int q6prm_set_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root,
-			  unsigned int freq)
+static int q6prm_request_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root,
+				     unsigned int freq)
 {
 	struct q6prm *prm = dev_get_drvdata(dev->parent);
 	struct apm_module_param_data *param_data;
@@ -138,6 +144,49 @@ int q6prm_set_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_
 
 	return rc;
 }
+
+static int q6prm_release_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root,
+			  unsigned int freq)
+{
+	struct q6prm *prm = dev_get_drvdata(dev->parent);
+	struct apm_module_param_data *param_data;
+	struct prm_cmd_release_rsc *rel;
+	gpr_device_t *gdev = prm->gdev;
+	struct gpr_pkt *pkt;
+	int rc;
+
+	pkt = audioreach_alloc_cmd_pkt(sizeof(*rel), PRM_CMD_RELEASE_HW_RSC, 0, gdev->svc.id,
+				       GPR_PRM_MODULE_IID);
+	if (IS_ERR(pkt))
+		return PTR_ERR(pkt);
+
+	rel = (void *)pkt + GPR_HDR_SIZE + APM_CMD_HDR_SIZE;
+
+	param_data = &rel->param_data;
+
+	param_data->module_instance_id = GPR_PRM_MODULE_IID;
+	param_data->error_code = 0;
+	param_data->param_id = PARAM_ID_RSC_AUDIO_HW_CLK;
+	param_data->param_size = sizeof(*rel) - APM_MODULE_PARAM_DATA_SIZE;
+
+	rel->num_clk_id = 1;
+	rel->clock_id.clock_id = clk_id;
+
+	rc = q6prm_send_cmd_sync(prm, pkt, PRM_CMD_RSP_RELEASE_HW_RSC);
+
+	kfree(pkt);
+
+	return rc;
+}
+
+int q6prm_set_lpass_clock(struct device *dev, int clk_id, int clk_attr, int clk_root,
+			  unsigned int freq)
+{
+	if (freq)
+		return q6prm_request_lpass_clock(dev, clk_id, clk_attr, clk_attr, freq);
+
+	return q6prm_release_lpass_clock(dev, clk_id, clk_attr, clk_attr, freq);
+}
 EXPORT_SYMBOL_GPL(q6prm_set_lpass_clock);
 
 static int prm_callback(struct gpr_resp_pkt *data, void *priv, int op)
-- 
2.21.0


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

* [PATCH 2/5] ASoC: qdsp6: q6routing: Conditionally reset FrontEnd Mixer
  2021-11-16 11:47 ` Srinivas Kandagatla
@ 2021-11-16 11:47   ` Srinivas Kandagatla
  -1 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie
  Cc: lgirdwood, perex, tiwai, pierre-louis.bossart, alsa-devel,
	linux-kernel, Srinivas Kandagatla

Stream IDs are reused across multiple BackEnd mixers, do not reset the
stream mixers if they are not already set for that particular FrontEnd.

Ex:
amixer cset iface=MIXER,name='SLIMBUS_0_RX Audio Mixer MultiMedia1' 1

would set the MultiMedia1 steam for SLIMBUS_0_RX, however doing below
command will reset previously setup MultiMedia1 stream, because both of them
are using MultiMedia1 PCM stream.

amixer cset iface=MIXER,name='SLIMBUS_2_RX Audio Mixer MultiMedia1' 0

reset the FrontEnd Mixers conditionally to fix this issue.

This is more noticeable in desktop setup, where in alsactl tries to restore
the alsa state and overwriting the previous mixer settings.

Fixes: e3a33673e845 ("ASoC: qdsp6: q6routing: Add q6routing driver")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6routing.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c
index 3390ebef9549..243b8179e59d 100644
--- a/sound/soc/qcom/qdsp6/q6routing.c
+++ b/sound/soc/qcom/qdsp6/q6routing.c
@@ -495,7 +495,11 @@ static int msm_routing_put_audio_mixer(struct snd_kcontrol *kcontrol,
 		session->port_id = be_id;
 		snd_soc_dapm_mixer_update_power(dapm, kcontrol, 1, update);
 	} else {
-		session->port_id = -1;
+		if (session->port_id == be_id) {
+			session->port_id = -1;
+			return 0;
+		}
+
 		snd_soc_dapm_mixer_update_power(dapm, kcontrol, 0, update);
 	}
 
-- 
2.21.0


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

* [PATCH 2/5] ASoC: qdsp6: q6routing: Conditionally reset FrontEnd Mixer
@ 2021-11-16 11:47   ` Srinivas Kandagatla
  0 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie; +Cc: alsa-devel, lgirdwood, linux-kernel, pierre-louis.bossart, tiwai

Stream IDs are reused across multiple BackEnd mixers, do not reset the
stream mixers if they are not already set for that particular FrontEnd.

Ex:
amixer cset iface=MIXER,name='SLIMBUS_0_RX Audio Mixer MultiMedia1' 1

would set the MultiMedia1 steam for SLIMBUS_0_RX, however doing below
command will reset previously setup MultiMedia1 stream, because both of them
are using MultiMedia1 PCM stream.

amixer cset iface=MIXER,name='SLIMBUS_2_RX Audio Mixer MultiMedia1' 0

reset the FrontEnd Mixers conditionally to fix this issue.

This is more noticeable in desktop setup, where in alsactl tries to restore
the alsa state and overwriting the previous mixer settings.

Fixes: e3a33673e845 ("ASoC: qdsp6: q6routing: Add q6routing driver")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6routing.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c
index 3390ebef9549..243b8179e59d 100644
--- a/sound/soc/qcom/qdsp6/q6routing.c
+++ b/sound/soc/qcom/qdsp6/q6routing.c
@@ -495,7 +495,11 @@ static int msm_routing_put_audio_mixer(struct snd_kcontrol *kcontrol,
 		session->port_id = be_id;
 		snd_soc_dapm_mixer_update_power(dapm, kcontrol, 1, update);
 	} else {
-		session->port_id = -1;
+		if (session->port_id == be_id) {
+			session->port_id = -1;
+			return 0;
+		}
+
 		snd_soc_dapm_mixer_update_power(dapm, kcontrol, 0, update);
 	}
 
-- 
2.21.0


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

* [PATCH 3/5] ASoC: qdsp6: q6asm: fix q6asm_dai_prepare error handling
  2021-11-16 11:47 ` Srinivas Kandagatla
@ 2021-11-16 11:47   ` Srinivas Kandagatla
  -1 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie
  Cc: lgirdwood, perex, tiwai, pierre-louis.bossart, alsa-devel,
	linux-kernel, Srinivas Kandagatla

Error handling in q6asm_dai_prepare() seems to be completely broken,
Fix this by handling it properly.

Fixes: 2a9e92d371db ("ASoC: qdsp6: q6asm: Add q6asm dai driver")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6asm-dai.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c
index 46f365528d50..b74b67720ef4 100644
--- a/sound/soc/qcom/qdsp6/q6asm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6asm-dai.c
@@ -269,9 +269,7 @@ static int q6asm_dai_prepare(struct snd_soc_component *component,
 
 	if (ret < 0) {
 		dev_err(dev, "%s: q6asm_open_write failed\n", __func__);
-		q6asm_audio_client_free(prtd->audio_client);
-		prtd->audio_client = NULL;
-		return -ENOMEM;
+		goto open_err;
 	}
 
 	prtd->session_id = q6asm_get_session_id(prtd->audio_client);
@@ -279,7 +277,7 @@ static int q6asm_dai_prepare(struct snd_soc_component *component,
 			      prtd->session_id, substream->stream);
 	if (ret) {
 		dev_err(dev, "%s: stream reg failed ret:%d\n", __func__, ret);
-		return ret;
+		goto routing_err;
 	}
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
@@ -301,10 +299,19 @@ static int q6asm_dai_prepare(struct snd_soc_component *component,
 	}
 	if (ret < 0)
 		dev_info(dev, "%s: CMD Format block failed\n", __func__);
+	else
+		prtd->state = Q6ASM_STREAM_RUNNING;
 
-	prtd->state = Q6ASM_STREAM_RUNNING;
+	return ret;
 
-	return 0;
+routing_err:
+	q6asm_cmd(prtd->audio_client, prtd->stream_id,  CMD_CLOSE);
+open_err:
+	q6asm_unmap_memory_regions(substream->stream, prtd->audio_client);
+	q6asm_audio_client_free(prtd->audio_client);
+	prtd->audio_client = NULL;
+
+	return ret;
 }
 
 static int q6asm_dai_trigger(struct snd_soc_component *component,
-- 
2.21.0


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

* [PATCH 3/5] ASoC: qdsp6: q6asm: fix q6asm_dai_prepare error handling
@ 2021-11-16 11:47   ` Srinivas Kandagatla
  0 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie; +Cc: alsa-devel, lgirdwood, linux-kernel, pierre-louis.bossart, tiwai

Error handling in q6asm_dai_prepare() seems to be completely broken,
Fix this by handling it properly.

Fixes: 2a9e92d371db ("ASoC: qdsp6: q6asm: Add q6asm dai driver")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6asm-dai.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c
index 46f365528d50..b74b67720ef4 100644
--- a/sound/soc/qcom/qdsp6/q6asm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6asm-dai.c
@@ -269,9 +269,7 @@ static int q6asm_dai_prepare(struct snd_soc_component *component,
 
 	if (ret < 0) {
 		dev_err(dev, "%s: q6asm_open_write failed\n", __func__);
-		q6asm_audio_client_free(prtd->audio_client);
-		prtd->audio_client = NULL;
-		return -ENOMEM;
+		goto open_err;
 	}
 
 	prtd->session_id = q6asm_get_session_id(prtd->audio_client);
@@ -279,7 +277,7 @@ static int q6asm_dai_prepare(struct snd_soc_component *component,
 			      prtd->session_id, substream->stream);
 	if (ret) {
 		dev_err(dev, "%s: stream reg failed ret:%d\n", __func__, ret);
-		return ret;
+		goto routing_err;
 	}
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
@@ -301,10 +299,19 @@ static int q6asm_dai_prepare(struct snd_soc_component *component,
 	}
 	if (ret < 0)
 		dev_info(dev, "%s: CMD Format block failed\n", __func__);
+	else
+		prtd->state = Q6ASM_STREAM_RUNNING;
 
-	prtd->state = Q6ASM_STREAM_RUNNING;
+	return ret;
 
-	return 0;
+routing_err:
+	q6asm_cmd(prtd->audio_client, prtd->stream_id,  CMD_CLOSE);
+open_err:
+	q6asm_unmap_memory_regions(substream->stream, prtd->audio_client);
+	q6asm_audio_client_free(prtd->audio_client);
+	prtd->audio_client = NULL;
+
+	return ret;
 }
 
 static int q6asm_dai_trigger(struct snd_soc_component *component,
-- 
2.21.0


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

* [PATCH 4/5] ASoC: qdsp6: q6adm: improve error reporting
  2021-11-16 11:47 ` Srinivas Kandagatla
@ 2021-11-16 11:47   ` Srinivas Kandagatla
  -1 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie
  Cc: lgirdwood, perex, tiwai, pierre-louis.bossart, alsa-devel,
	linux-kernel, Srinivas Kandagatla

reset value for port is -1 so printing an hex would not give us very
useful debug information, so use %d instead.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6adm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6adm.c b/sound/soc/qcom/qdsp6/q6adm.c
index 3d831b635524..72c5719f1d25 100644
--- a/sound/soc/qcom/qdsp6/q6adm.c
+++ b/sound/soc/qcom/qdsp6/q6adm.c
@@ -390,7 +390,7 @@ struct q6copp *q6adm_open(struct device *dev, int port_id, int path, int rate,
 	int ret = 0;
 
 	if (port_id < 0) {
-		dev_err(dev, "Invalid port_id 0x%x\n", port_id);
+		dev_err(dev, "Invalid port_id %d\n", port_id);
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -508,7 +508,7 @@ int q6adm_matrix_map(struct device *dev, int path,
 		int port_idx = payload_map.port_id[i];
 
 		if (port_idx < 0) {
-			dev_err(dev, "Invalid port_id 0x%x\n",
+			dev_err(dev, "Invalid port_id %d\n",
 				payload_map.port_id[i]);
 			kfree(pkt);
 			return -EINVAL;
-- 
2.21.0


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

* [PATCH 4/5] ASoC: qdsp6: q6adm: improve error reporting
@ 2021-11-16 11:47   ` Srinivas Kandagatla
  0 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie; +Cc: alsa-devel, lgirdwood, linux-kernel, pierre-louis.bossart, tiwai

reset value for port is -1 so printing an hex would not give us very
useful debug information, so use %d instead.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6adm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6adm.c b/sound/soc/qcom/qdsp6/q6adm.c
index 3d831b635524..72c5719f1d25 100644
--- a/sound/soc/qcom/qdsp6/q6adm.c
+++ b/sound/soc/qcom/qdsp6/q6adm.c
@@ -390,7 +390,7 @@ struct q6copp *q6adm_open(struct device *dev, int port_id, int path, int rate,
 	int ret = 0;
 
 	if (port_id < 0) {
-		dev_err(dev, "Invalid port_id 0x%x\n", port_id);
+		dev_err(dev, "Invalid port_id %d\n", port_id);
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -508,7 +508,7 @@ int q6adm_matrix_map(struct device *dev, int path,
 		int port_idx = payload_map.port_id[i];
 
 		if (port_idx < 0) {
-			dev_err(dev, "Invalid port_id 0x%x\n",
+			dev_err(dev, "Invalid port_id %d\n",
 				payload_map.port_id[i]);
 			kfree(pkt);
 			return -EINVAL;
-- 
2.21.0


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

* [PATCH 5/5] ASoC: qdsp6: q6routing: validate port id before setting up route
  2021-11-16 11:47 ` Srinivas Kandagatla
@ 2021-11-16 11:47   ` Srinivas Kandagatla
  -1 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie
  Cc: lgirdwood, perex, tiwai, pierre-louis.bossart, alsa-devel,
	linux-kernel, Srinivas Kandagatla

Validate port id before it starts sending commands to dsp this would
make error handling simpler.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6routing.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c
index 243b8179e59d..cd74681e811e 100644
--- a/sound/soc/qcom/qdsp6/q6routing.c
+++ b/sound/soc/qcom/qdsp6/q6routing.c
@@ -372,6 +372,12 @@ int q6routing_stream_open(int fedai_id, int perf_mode,
 	}
 
 	session = &routing_data->sessions[stream_id - 1];
+	if (session->port_id < 0) {
+		dev_err(routing_data->dev, "Routing not setup for MultiMedia%d Session\n",
+			session->fedai_id);
+		return -EINVAL;
+	}
+
 	pdata = &routing_data->port_data[session->port_id];
 
 	mutex_lock(&routing_data->lock);
-- 
2.21.0


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

* [PATCH 5/5] ASoC: qdsp6: q6routing: validate port id before setting up route
@ 2021-11-16 11:47   ` Srinivas Kandagatla
  0 siblings, 0 replies; 14+ messages in thread
From: Srinivas Kandagatla @ 2021-11-16 11:47 UTC (permalink / raw)
  To: broonie; +Cc: alsa-devel, lgirdwood, linux-kernel, pierre-louis.bossart, tiwai

Validate port id before it starts sending commands to dsp this would
make error handling simpler.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6routing.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c
index 243b8179e59d..cd74681e811e 100644
--- a/sound/soc/qcom/qdsp6/q6routing.c
+++ b/sound/soc/qcom/qdsp6/q6routing.c
@@ -372,6 +372,12 @@ int q6routing_stream_open(int fedai_id, int perf_mode,
 	}
 
 	session = &routing_data->sessions[stream_id - 1];
+	if (session->port_id < 0) {
+		dev_err(routing_data->dev, "Routing not setup for MultiMedia%d Session\n",
+			session->fedai_id);
+		return -EINVAL;
+	}
+
 	pdata = &routing_data->port_data[session->port_id];
 
 	mutex_lock(&routing_data->lock);
-- 
2.21.0


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

* Re: [PATCH 0/5] ASoC: qdsp6: fixes and improve error reporting
  2021-11-16 11:47 ` Srinivas Kandagatla
@ 2021-11-16 17:48   ` Mark Brown
  -1 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2021-11-16 17:48 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: linux-kernel, perex, tiwai, pierre-louis.bossart, alsa-devel, lgirdwood

On Tue, 16 Nov 2021 11:47:16 +0000, Srinivas Kandagatla wrote:
> During testing on a laptop we found few issues with the existing q6dsp code,
> first 3 patches should fix these issues. Also during debug we found that
> some of the errors reported are not very useful, so update error reporting
> in two places.
> 
> Thanks,
> srini
> 
> [...]

Applied to

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

Thanks!

[1/5] ASoC: qdsp6: qdsp6: q6prm: handle clk disable correctly
      commit: 2f20640491edda3c03eb6b899d0b92630d3d4c63
[2/5] ASoC: qdsp6: q6routing: Conditionally reset FrontEnd Mixer
      commit: 861afeac7990587588d057b2c0b3222331c3da29
[3/5] ASoC: qdsp6: q6asm: fix q6asm_dai_prepare error handling
      commit: 721a94b4352dc8e47bff90b549a0118c39776756
[4/5] ASoC: qdsp6: q6adm: improve error reporting
      commit: 0a270471d68533f59c5cfd631a3fce31a3b17144
[5/5] ASoC: qdsp6: q6routing: validate port id before setting up route
      commit: 6712c2e18c06b0976559fd4bd47774b243038e9c

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] 14+ messages in thread

* Re: [PATCH 0/5] ASoC: qdsp6: fixes and improve error reporting
@ 2021-11-16 17:48   ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2021-11-16 17:48 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: alsa-devel, lgirdwood, linux-kernel, pierre-louis.bossart, tiwai

On Tue, 16 Nov 2021 11:47:16 +0000, Srinivas Kandagatla wrote:
> During testing on a laptop we found few issues with the existing q6dsp code,
> first 3 patches should fix these issues. Also during debug we found that
> some of the errors reported are not very useful, so update error reporting
> in two places.
> 
> Thanks,
> srini
> 
> [...]

Applied to

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

Thanks!

[1/5] ASoC: qdsp6: qdsp6: q6prm: handle clk disable correctly
      commit: 2f20640491edda3c03eb6b899d0b92630d3d4c63
[2/5] ASoC: qdsp6: q6routing: Conditionally reset FrontEnd Mixer
      commit: 861afeac7990587588d057b2c0b3222331c3da29
[3/5] ASoC: qdsp6: q6asm: fix q6asm_dai_prepare error handling
      commit: 721a94b4352dc8e47bff90b549a0118c39776756
[4/5] ASoC: qdsp6: q6adm: improve error reporting
      commit: 0a270471d68533f59c5cfd631a3fce31a3b17144
[5/5] ASoC: qdsp6: q6routing: validate port id before setting up route
      commit: 6712c2e18c06b0976559fd4bd47774b243038e9c

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] 14+ messages in thread

end of thread, other threads:[~2021-11-16 17:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16 11:47 [PATCH 0/5] ASoC: qdsp6: fixes and improve error reporting Srinivas Kandagatla
2021-11-16 11:47 ` Srinivas Kandagatla
2021-11-16 11:47 ` [PATCH 1/5] ASoC: qdsp6: qdsp6: q6prm: handle clk disable correctly Srinivas Kandagatla
2021-11-16 11:47   ` Srinivas Kandagatla
2021-11-16 11:47 ` [PATCH 2/5] ASoC: qdsp6: q6routing: Conditionally reset FrontEnd Mixer Srinivas Kandagatla
2021-11-16 11:47   ` Srinivas Kandagatla
2021-11-16 11:47 ` [PATCH 3/5] ASoC: qdsp6: q6asm: fix q6asm_dai_prepare error handling Srinivas Kandagatla
2021-11-16 11:47   ` Srinivas Kandagatla
2021-11-16 11:47 ` [PATCH 4/5] ASoC: qdsp6: q6adm: improve error reporting Srinivas Kandagatla
2021-11-16 11:47   ` Srinivas Kandagatla
2021-11-16 11:47 ` [PATCH 5/5] ASoC: qdsp6: q6routing: validate port id before setting up route Srinivas Kandagatla
2021-11-16 11:47   ` Srinivas Kandagatla
2021-11-16 17:48 ` [PATCH 0/5] ASoC: qdsp6: fixes and improve error reporting Mark Brown
2021-11-16 17:48   ` Mark Brown

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.