Browse Source

audio-kernel: SSR/PDR fixes on audio-drivers.

Few Fixes on the drivers we are making are
1. snd_event is not resetting client info on deregister.
2. audio_prm is not providing the snd event up notification
3. gpr_lite reversing registration seq to avoid race condition
4. pinctrl-lpi reversing reg seq to avoid race condition.

Change-Id: I749de8874b33a528cb6b386d753f5d594139a577
Signed-off-by: Bharath Tirunagaru <[email protected]>
Bharath Tirunagaru 5 years ago
parent
commit
ac15eb7016
4 changed files with 71 additions and 84 deletions
  1. 0 40
      dsp/audio_prm.c
  2. 45 28
      ipc/gpr-lite.c
  3. 12 11
      soc/pinctrl-lpi.c
  4. 14 5
      soc/snd_event.c

+ 0 - 40
dsp/audio_prm.c

@@ -89,11 +89,6 @@ static int prm_gpr_send_pkt(struct gpr_pkt *pkt, wait_queue_head_t *wait)
 
 	mutex_lock(&g_prm.lock);
 	pr_debug("%s: enter",__func__);
-	if (!g_prm.is_adsp_up) {
-		pr_err("%s: ADSP is not up\n", __func__);
-		mutex_unlock(&g_prm.lock);
-		return -ENODEV;
-	}
 
 	if (g_prm.adev == NULL) {
 		pr_err("%s: apr is unregistered\n", __func__);
@@ -302,27 +297,6 @@ EXPORT_SYMBOL(audio_prm_set_lpass_clk_cfg);
 
 
 
-static int prm_ssr_enable(struct device *dev, void *data)
-{
-        mutex_lock(&g_prm.lock);
-        g_prm.is_adsp_up = true;
-        mutex_unlock(&g_prm.lock);
-	return 0;
-}
-
-static void prm_ssr_disable(struct device *dev, void *data)
-{
-        mutex_lock(&g_prm.lock);
-        g_prm.is_adsp_up = true;
-        mutex_unlock(&g_prm.lock);
-
-}
-
-static const struct snd_event_ops prm_ssr_ops = {
-        .enable = prm_ssr_enable,
-        .disable = prm_ssr_disable,
-};
-
 static int audio_prm_probe(struct gpr_device *adev)
 {
 	int ret = 0;
@@ -331,18 +305,11 @@ static int audio_prm_probe(struct gpr_device *adev)
 
 	mutex_init(&g_prm.lock);
 	g_prm.adev = adev;
-        g_prm.is_adsp_up = true;
 
 	init_waitqueue_head(&g_prm.wait);
 
-	ret = snd_event_client_register(&adev->dev, &prm_ssr_ops, NULL);
-	if (ret) {
-		pr_err("%s: Registration with snd event failed \n",__func__);
-		goto err;
-	}
 
 	pr_err("%s: prm probe success\n", __func__);
-err:
 	return ret;
 }
 
@@ -351,15 +318,8 @@ static int audio_prm_remove(struct gpr_device *adev)
 	int ret = 0;
 
 	mutex_lock(&g_prm.lock);
-	ret = snd_event_client_deregister(&adev->dev);
-	if (ret) {
-		pr_err("%s: Deregistration from SNF failed \n",__func__);
-		goto err;
-	}
 	g_prm.adev = NULL;
-	g_prm.is_adsp_up = false;
 	mutex_unlock(&g_prm.lock);
-err:
 	return ret;
 }
 

+ 45 - 28
ipc/gpr-lite.c

@@ -34,6 +34,7 @@ struct gpr {
 	spinlock_t svcs_lock;
 	struct idr svcs_idr;
 	int dest_domain_id;
+	struct work_struct notifier_reg_work;
 };
 
 static struct gpr_q6 q6;
@@ -91,13 +92,13 @@ int gpr_send_pkt(struct gpr_device *adev, struct gpr_pkt *pkt)
 
 	if(!adev)
 	{
-		pr_err("%s: enter pointer adev[%p] \n", __func__, adev);
+		pr_err("%s: enter pointer adev[%pK] \n", __func__, adev);
 		return -EINVAL;
 	}
 
 	if(!(adev->dev.parent))
 	{
-		pr_err("%s: enter pointer adev->dev.parent[%p] \n",
+		pr_err("%s: enter pointer adev->dev.parent[%pK] \n",
 			__func__, adev->dev.parent);
 		return -EINVAL;
 	}
@@ -179,14 +180,14 @@ static const struct snd_event_ops gpr_ssr_ops = {
 
 static void gpr_adsp_down(unsigned long opcode)
 {
-	dev_dbg(gpr_priv->dev,"%s: Q6 is Down\n", __func__);
-	snd_event_notify(gpr_priv->dev, SND_EVENT_DOWN);
+	dev_info(gpr_priv->dev,"%s: Q6 is Down\n", __func__);
 	gpr_set_q6_state(GPR_SUBSYS_DOWN);
+	snd_event_notify(gpr_priv->dev, SND_EVENT_DOWN);
 }
 
 static void gpr_adsp_up(void)
 {
-	dev_dbg(gpr_priv->dev,"%s: Q6 is Up\n", __func__);
+	dev_info(gpr_priv->dev,"%s: Q6 is Up\n", __func__);
 	gpr_set_q6_state(GPR_SUBSYS_LOADED);
 	snd_event_notify(gpr_priv->dev, SND_EVENT_UP);
 }
@@ -456,6 +457,23 @@ static void of_register_gpr_devices(struct device *dev)
 	}
 }
 
+static void gpr_notifier_register(struct work_struct *work)
+{
+	if (GPR_DOMAIN_ADSP == gpr_priv->dest_domain_id) {
+		gpr_subsys_notif_register("gpr_adsp",
+				       AUDIO_NOTIFIER_ADSP_DOMAIN,
+				       &adsp_service_nb);
+	} else if (GPR_DOMAIN_MODEM == gpr_priv->dest_domain_id) {
+		gpr_subsys_notif_register("gpr_modem",
+				       AUDIO_NOTIFIER_MODEM_DOMAIN,
+				       &modem_service_nb);
+	}
+
+	dev_info(gpr_priv->dev, "%s: registered via subsys_notif_register for domain id(%d)",
+		__func__, gpr_priv->dest_domain_id  );
+	return;
+}
+
 static int gpr_probe(struct rpmsg_device *rpdev)
 {
 	struct device *dev = &rpdev->dev;
@@ -471,34 +489,11 @@ static int gpr_probe(struct rpmsg_device *rpdev)
 	gpr_priv->is_initial_boot = true;
 	spin_unlock(&gpr_priv->gpr_lock);
 
-	ret = of_property_read_u32(dev->of_node, "reg", &gpr_priv->dest_domain_id);
-	if (ret) {
-		dev_err(dev, "GPR Domain ID not specified in DT\n");
-		return ret;
-	}
-	if (GPR_DOMAIN_ADSP == gpr_priv->dest_domain_id) {
-		gpr_subsys_notif_register("gpr_adsp",
-				       AUDIO_NOTIFIER_ADSP_DOMAIN,
-				       &adsp_service_nb);
-	} else if (GPR_DOMAIN_MODEM == gpr_priv->dest_domain_id) {
-		gpr_subsys_notif_register("gpr_modem",
-				       AUDIO_NOTIFIER_MODEM_DOMAIN,
-				       &modem_service_nb);
-	} else {
-		dev_err(dev, "%s: invalid dest_domain_id %s\n", __func__,
-		  gpr_priv->dest_domain_id);
-		return -EINVAL;
-	}
-
-	dev_info(dev, "%s: registered via subsys_notif_register for domain id(%d)",
-		__func__, gpr_priv->dest_domain_id  );
-
 	dev_set_drvdata(dev, gpr_priv);
 	gpr_priv->ch = rpdev->ept;
 	gpr_priv->dev = dev;
 	spin_lock_init(&gpr_priv->svcs_lock);
 	idr_init(&gpr_priv->svcs_idr);
-	of_register_gpr_devices(dev);
 
 	ret = snd_event_client_register(&rpdev->dev, &gpr_ssr_ops, NULL);
 	if (ret) {
@@ -507,6 +502,28 @@ static int gpr_probe(struct rpmsg_device *rpdev)
 		ret = 0;
 	}
 
+	of_register_gpr_devices(dev);
+
+	INIT_WORK(&gpr_priv->notifier_reg_work, gpr_notifier_register);
+
+	ret = of_property_read_u32(dev->of_node, "reg", &gpr_priv->dest_domain_id);
+	if (ret) {
+		dev_err(dev, "GPR Domain ID not specified in DT\n");
+		return ret;
+	}
+
+	if (GPR_DOMAIN_ADSP == gpr_priv->dest_domain_id ||
+		GPR_DOMAIN_MODEM == gpr_priv->dest_domain_id) {
+		schedule_work(&gpr_priv->notifier_reg_work);
+	} else {
+		dev_err(dev, "%s: invalid dest_domain_id %s\n", __func__,
+		  gpr_priv->dest_domain_id);
+		return -EINVAL;
+	}
+
+	dev_info(dev, "%s: gpr-lite probe success\n",
+		__func__);
+
 	return 0;
 }
 

+ 12 - 11
soc/pinctrl-lpi.c

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2016-2020 The Linux Foundation. All rights reserved.
  */
 
 #include <linux/gpio.h>
@@ -772,13 +772,6 @@ static int lpi_pinctrl_probe(struct platform_device *pdev)
 
 	lpi_dev = &pdev->dev;
 	lpi_dev_up = true;
-	ret = audio_notifier_register("lpi_tlmm", AUDIO_NOTIFIER_ADSP_DOMAIN,
-				      &service_nb);
-	if (ret < 0) {
-		pr_err("%s: Audio notifier register failed ret = %d\n",
-			__func__, ret);
-		goto err_range;
-	}
 
 	ret = snd_event_client_register(dev, &lpi_pinctrl_ssr_ops, NULL);
 	if (!ret) {
@@ -786,7 +779,15 @@ static int lpi_pinctrl_probe(struct platform_device *pdev)
 	} else {
 		dev_err(dev, "%s: snd_event registration failed, ret [%d]\n",
 			__func__, ret);
-		goto err_snd_evt;
+		goto err_range;
+	}
+
+	ret = audio_notifier_register("lpi_tlmm", AUDIO_NOTIFIER_ADSP_DOMAIN,
+				      &service_nb);
+	if (ret < 0) {
+		pr_err("%s: Audio notifier register failed ret = %d\n",
+			__func__, ret);
+		goto err_range;
 	}
 
 	/* Register LPASS core hw vote */
@@ -819,8 +820,8 @@ static int lpi_pinctrl_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_snd_evt:
-	audio_notifier_deregister("lpi_tlmm");
+//err_snd_evt:
+//	audio_notifier_deregister("lpi_tlmm");
 err_range:
 	gpiochip_remove(&state->chip);
 err_chip:

+ 14 - 5
soc/snd_event.c

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2018, 2020 The Linux Foundation. All rights reserved.
  */
 
 #include <linux/platform_device.h>
@@ -232,6 +232,9 @@ int snd_event_client_deregister(struct device *dev)
 		return -EINVAL;
 	}
 
+	dev_dbg(dev, "%s: removing client to SND event FW \n",
+		__func__);
+
 	mutex_lock(&snd_event_mutex);
 	if (list_empty(&snd_event_client_list)) {
 		dev_dbg(dev, "%s: No SND client registered\n", __func__);
@@ -248,7 +251,7 @@ int snd_event_client_deregister(struct device *dev)
 
 	c->state = false;
 
-	if (master && master->clients_found) {
+	if (master) {
 		struct snd_event_client *d;
 		bool dev_found = false;
 
@@ -259,9 +262,12 @@ int snd_event_client_deregister(struct device *dev)
 				break;
 			}
 		}
-		if (dev_found) {
-			ret = check_and_update_fwk_state();
-			master->clients_found = false;
+		if (dev_found ) {
+			if(master->clients_found) {
+				ret = check_and_update_fwk_state();
+				master->clients_found = false;
+			}
+			master->clients->cl_arr[i].dev = NULL;
 		}
 	}
 
@@ -462,6 +468,9 @@ int snd_event_notify(struct device *dev, unsigned int state)
 		return -EINVAL;
 	}
 
+	dev_dbg(dev, "%s: snd_event_notify (state %u)\n",
+		__func__, state);
+
 	mutex_lock(&snd_event_mutex);
 	if (list_empty(&snd_event_client_list) && !master) {
 		dev_err(dev, "%s: No device registered\n", __func__);