[PATCH 0/2] Fix GPR callback registration race during probe

[PATCH 0/2] Fix GPR callback registration race during probe

[Mohit Sharma]

GPR service drivers can issue synchronous commands directly from their
probe callbacks or indirectly while populating child devices. The GPR
bus currently publishes the service callback only after the client probe
returns. Consequently, a DSP response received during probe cannot be
dispatched and the command times out.

This was observed intermittently on Qualcomm Lemans and Monaco platforms
while q6apm child probing issued an APM shared-memory map command. The DSP
response reached the GPR receive path, but was discarded because the
service callback had not yet been installed.

The first patch prepares q6apm and q6prm for early callback publication by
ensuring callback-visible state is initialized before driver data is
published and safely ignoring packets received before that point. The
second patch publishes GPR callbacks before invoking client probes and
clears them again when probing fails. Existing APR callback ordering is
left unchanged.

Mohit Sharma (2):
  ASoC: qcom: q6apm: Guard callbacks until probe data is ready
  soc: qcom: apr: Publish GPR callback before client probe

 drivers/soc/qcom/apr.c          | 15 +++++++++++++--
 sound/soc/qcom/qdsp6/q6apm.c   |  6 ++++--
 sound/soc/qcom/qdsp6/q6prm.c   |  3 +++
 3 files changed, 20 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH 1/2] ASoC: qcom: q6apm: Guard callbacks until probe data is ready

[Mohit Sharma]

The GPR bus needs to make response callbacks available before invoking a
client probe because clients can issue synchronous commands from probe.
This creates a short interval where a callback can run before the client
has published its private data.

Make q6apm publish its driver data only after all callback-visible state
has been initialized. Have the q6apm and q6prm callbacks ignore packets
received before driver data is available.

Fixes: ec1471a898cc ("soc: qcom: apr: Add GPR support")
Cc: stable@vger.kernel.org
Co-developed-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
Signed-off-by: Mohit Sharma <mohit.sharma@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/q6apm.c | 6 ++++--
 sound/soc/qcom/qdsp6/q6prm.c | 3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
index 3ec3c5a18c7a..dbebb643cd5e 100644
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -872,8 +872,6 @@ static int apm_probe(gpr_device_t *gdev)
 	if (!apm)
 		return -ENOMEM;
 
-	dev_set_drvdata(dev, apm);
-
 	mutex_init(&apm->lock);
 	apm->dev = dev;
 	apm->gdev = gdev;
@@ -887,6 +885,7 @@ static int apm_probe(gpr_device_t *gdev)
 
 	idr_init(&apm->modules_idr);
 
+	dev_set_drvdata(dev, apm);
 	g_apm = apm;
 
 	q6apm_get_apm_state(apm);
@@ -930,6 +929,9 @@ static int apm_callback(const struct gpr_resp_pkt *data, void *priv, int op)
 	const struct gpr_hdr *hdr = &data->hdr;
 	int graph_id, is_pos_buf;
 
+	if (!apm)
+		return 0;
+
 	result = data->payload;
 
 	switch (hdr->opcode) {
diff --git a/sound/soc/qcom/qdsp6/q6prm.c b/sound/soc/qcom/qdsp6/q6prm.c
index 2042ff1e8ff1..5c927ebd81f4 100644
--- a/sound/soc/qcom/qdsp6/q6prm.c
+++ b/sound/soc/qcom/qdsp6/q6prm.c
@@ -194,6 +194,9 @@ static int prm_callback(const struct gpr_resp_pkt *data, void *priv, int op)
 	const struct gpr_ibasic_rsp_result_t *result;
 	const struct gpr_hdr *hdr = &data->hdr;
 
+	if (!prm)
+		return 0;
+
 	switch (hdr->opcode) {
 	case PRM_CMD_RSP_REQUEST_HW_RSC:
 	case PRM_CMD_RSP_RELEASE_HW_RSC:
-- 
2.34.1

[PATCH 2/2] soc: qcom: apr: Publish GPR callback before client probe

[Mohit Sharma]

A GPR service driver can issue synchronous commands from its probe path,
or populate child devices which issue commands before the parent probe
returns. The APR/GPR bus currently installs the GPR service callback only
after the service driver's probe has completed.

If a response arrives while the client probe is still running, GPR finds
the destination service but cannot dispatch the response because the
service callback is NULL. The client waiting for the response eventually
times out.

This was observed intermittently on Qualcomm Lemans and Monaco platforms
when q6apm child probing sent an APM shared-memory map command before the
parent probe returned. The failure has the following signature:

  qcom-apm ... CMD timeout for [100100c] opcode
  q6apm-dai ... Audio Start: Buffer Allocation failed rc = -110
  snd-sc8280xp ... ASoC: can't create pcm MultiMedia1 Playback :-110

Install the GPR callback before invoking the client probe so responses to
probe-time commands can be dispatched. Clear the callback again if probe
fails, and preserve the existing callback ordering for APR clients.

Fixes: ec1471a898cc ("soc: qcom: apr: Add GPR support")
Cc: stable@vger.kernel.org
Co-developed-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
Signed-off-by: Mohit Sharma <mohit.sharma@oss.qualcomm.com>
---
 drivers/soc/qcom/apr.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/apr.c b/drivers/soc/qcom/apr.c
index ea7f83916d8d..829e5c8fdf98 100644
--- a/drivers/soc/qcom/apr.c
+++ b/drivers/soc/qcom/apr.c
@@ -365,13 +365,24 @@ static int apr_device_probe(struct device *dev)
 {
 	struct apr_device *adev = to_apr_device(dev);
 	struct apr_driver *adrv = to_apr_driver(dev->driver);
+	struct packet_router *apr = dev_get_drvdata(dev->parent);
 	int ret;
 
+	if (apr->type == PR_TYPE_GPR)
+		adev->svc.callback = adrv->gpr_callback;
+
 	ret = adrv->probe(adev);
-	if (!ret)
+	if (ret) {
+		if (apr->type == PR_TYPE_GPR)
+			adev->svc.callback = NULL;
+
+		return ret;
+	}
+
+	if (apr->type != PR_TYPE_GPR)
 		adev->svc.callback = adrv->gpr_callback;
 
-	return ret;
+	return 0;
 }
 
 static void apr_device_remove(struct device *dev)
-- 
2.34.1