[RFC PATCH 3/4] ASoC: qcom: q6apm: synchronize readiness query and provider lifetime

[Zhang Jiaxi]

From: Jiaxi Zhang <z1529105815@xxxxxxxxxxx>

The private SPF-state query ignores its synchronous command error, while
the bool API accepts a stale state or even a negative allocation error as
ready. Its q6prm probe caller consequently skips EPROBE_DEFER.

The public Graphservices GET_SPF_STATE response defines READY as 1.
Require this query to succeed and return that exact state. Protect the APM
pointer for the entire query and state snapshot, serialize its publication
and removal, and clear it on failed component registration or population.
Run the initial probe query before publishing the provider; its error is
still ignored by probe, so this does not add probe-time error recovery.
Removal waits for a readiness query to finish before clearing the pointer
and returning to devres cleanup. The callback never takes this global mutex.

The existing per-APM command mutex remains responsible for command/result
serialization and waiting, and is released before the synchronous command
returns. The global mutex protects this public query's provider and
snapshot without recursively taking the command mutex. It does not keep
later SPF replies from overwriting the state or add reply correlation.
APR callback draining, graph lifetime and DSP restart recovery remain
separate lifecycle problems.

Based on sound.git for-next
62d9f9ffdfd44e88010412bf8f23732f0a93a9be. No target hardware result is claimed.

Link: https://github.com/AudioReach/audioreach-graphservices/blob/042cb60b4e510fd30a944ef940a2c79af57aa7e3/spf/api/apm/apm_api.h#L1002
Signed-off-by: Jiaxi Zhang <z1529105815@xxxxxxxxxxx>

---
diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -6,6 +6,7 @@
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/sched.h>
@@ -28,6 +29,7 @@
 #define APM_GRAPH_MGMT_PSIZE(p, n) ALIGN(struct_size(p, sub_graph_id_list, n), 8)
 
 static struct q6apm *g_apm;
+static DEFINE_MUTEX(g_apm_lock);
 
 int q6apm_send_cmd_sync(struct q6apm *apm, struct gpr_pkt *pkt,
 			uint32_t rsp_opcode)
@@ -149,17 +151,19 @@
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
-	q6apm_send_cmd_sync(apm, pkt, APM_CMD_RSP_GET_SPF_STATE);
-
-	return apm->state;
+	return q6apm_send_cmd_sync(apm, pkt, APM_CMD_RSP_GET_SPF_STATE);
 }
 
 bool q6apm_is_adsp_ready(void)
 {
+	bool ready = false;
+
+	mutex_lock(&g_apm_lock);
 	if (g_apm)
-		return q6apm_get_apm_state(g_apm);
-
-	return false;
+		ready = !q6apm_get_apm_state(g_apm) && g_apm->state == 1;
+	mutex_unlock(&g_apm_lock);
+
+	return ready;
 }
 EXPORT_SYMBOL_GPL(q6apm_is_adsp_ready);
 
@@ -862,6 +866,14 @@
 	.remove_order   = SND_SOC_COMP_ORDER_LAST,
 };
 
+static void q6apm_clear_global(struct q6apm *apm)
+{
+	mutex_lock(&g_apm_lock);
+	if (g_apm == apm)
+		g_apm = NULL;
+	mutex_unlock(&g_apm_lock);
+}
+
 static int apm_probe(gpr_device_t *gdev)
 {
 	struct device *dev = &gdev->dev;
@@ -887,26 +899,31 @@
 
 	idr_init(&apm->modules_idr);
 
+	q6apm_get_apm_state(apm);
+
+	mutex_lock(&g_apm_lock);
 	g_apm = apm;
-
-	q6apm_get_apm_state(apm);
+	mutex_unlock(&g_apm_lock);
 
 	ret = snd_soc_register_component(dev, &q6apm_audio_component, NULL, 0);
 	if (ret < 0) {
 		dev_err(dev, "failed to register q6apm: %d\n", ret);
+		q6apm_clear_global(apm);
 		return ret;
 	}
 
 	ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
-	if (ret)
+	if (ret) {
+		q6apm_clear_global(apm);
 		snd_soc_unregister_component(dev);
+	}
 
 	return ret;
 }
 
 static void apm_remove(gpr_device_t *gdev)
 {
-	g_apm = NULL;
+	q6apm_clear_global(dev_get_drvdata(&gdev->dev));
 	of_platform_depopulate(&gdev->dev);
 	snd_soc_unregister_component(&gdev->dev);
 }