[RFC PATCH 0/2] Make qcom-ngd-ctrl not wait indefinitely for already booted ADSP

[RFC PATCH 0/2] Make qcom-ngd-ctrl not wait indefinitely for already booted ADSP

[Yassine Oudjana]

Commit a899d324863a3 ("slimbus: qcom-ngd-ctrl: add Sub System Restart support")
made qcom-ngd-ctrl wait for ADSP to become ready before starting to do its work.
Due to how the SSR notifications currently work though, if qcom-ngd-ctrl probes
after ADSP boots and becomes ready, it never receives a QCOM_SSR_AFTER_POWERUP
event notification and keeps waiting indefinitely, making SLIMbus never come up.

This series makes qcom_register_ssr_notifier call the notifier_call of the newly
registered notifier block with the last SSR event received from the remoteproc,
basically reporting the event that qcom-ngd-ctrl missed by registering late,
stopping it from waiting for an event that has already happened.

I'm not sure if this approach would have any unwanted consequences in other
drivers relying on SSR events however, hence I'm sending this as a RFC. This
can also be considered a bug report, so if anyone has a better fix then I'd
appreciate it getting applied instead of this one.

The second patch is a general fix that became necessary after the first patch,
and should likely be applied anyway.

Yassine Oudjana (2):
  remoteproc: qcom: Report last event on SSR notifier registration
  slimbus: qcom-ngd-ctrl: Initialize ngd_up_work before it can be
    scheduled

 drivers/remoteproc/qcom_common.c | 17 +++++++++++++----
 drivers/slimbus/qcom-ngd-ctrl.c  |  3 ++-
 2 files changed, 15 insertions(+), 5 deletions(-)

-- 
2.36.0

[RFC PATCH 0/2] Make qcom-ngd-ctrl not wait indefinitely for already booted ADSP

[Yassine Oudjana]

Commit a899d324863a3 ("slimbus: qcom-ngd-ctrl: add Sub System Restart support")
made qcom-ngd-ctrl wait for ADSP to become ready before starting to do its work.
Due to how the SSR notifications currently work though, if qcom-ngd-ctrl probes
after ADSP boots and becomes ready, it never receives a QCOM_SSR_AFTER_POWERUP
event notification and keeps waiting indefinitely, making SLIMbus never come up.

This series makes qcom_register_ssr_notifier call the notifier_call of the newly
registered notifier block with the last SSR event received from the remoteproc,
basically reporting the event that qcom-ngd-ctrl missed by registering late,
stopping it from waiting for an event that has already happened.

I'm not sure if this approach would have any unwanted consequences in other
drivers relying on SSR events however, hence I'm sending this as a RFC. This
can also be considered a bug report, so if anyone has a better fix then I'd
appreciate it getting applied instead of this one.

The second patch is a general fix that became necessary after the first patch,
and should likely be applied anyway.

Yassine Oudjana (2):
  remoteproc: qcom: Report last event on SSR notifier registration
  slimbus: qcom-ngd-ctrl: Initialize ngd_up_work before it can be
    scheduled

 drivers/remoteproc/qcom_common.c | 17 +++++++++++++----
 drivers/slimbus/qcom-ngd-ctrl.c  |  3 ++-
 2 files changed, 15 insertions(+), 5 deletions(-)

-- 
2.36.0

[RFC PATCH 1/2] remoteproc: qcom: Report last event on SSR notifier registration

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Keep track of the last SSR event of a remoteproc, then report it
to each newly registered notifier block. This prevents drivers
from waiting indefinitely for an event that has already happened
before they registered their notifier blocks.

Fixes: a899d324863a3 ("slimbus: qcom-ngd-ctrl: add Sub System Restart support")
Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/remoteproc/qcom_common.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 4b91e3c9eafa..ff85ea564129 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -85,6 +85,7 @@ struct qcom_ssr_subsystem {
 	const char *name;
 	struct srcu_notifier_head notifier_list;
 	struct list_head list;
+	enum qcom_ssr_notify_type current_state;
 };
 
 static LIST_HEAD(qcom_ssr_subsystem_list);
@@ -392,6 +393,9 @@ void *qcom_register_ssr_notifier(const char *name, struct notifier_block *nb)
 
 	srcu_notifier_chain_register(&info->notifier_list, nb);
 
+	/* Notify newly registered notifier block of current remoteproc state */
+	nb->notifier_call(nb, info->current_state, NULL);
+
 	return &info->notifier_list;
 }
 EXPORT_SYMBOL_GPL(qcom_register_ssr_notifier);
@@ -420,8 +424,9 @@ static int ssr_notify_prepare(struct rproc_subdev *subdev)
 		.crashed = false,
 	};
 
+	ssr->info->current_state = QCOM_SSR_BEFORE_POWERUP;
 	srcu_notifier_call_chain(&ssr->info->notifier_list,
-				 QCOM_SSR_BEFORE_POWERUP, &data);
+				 ssr->info->current_state, &data);
 	return 0;
 }
 
@@ -433,8 +438,9 @@ static int ssr_notify_start(struct rproc_subdev *subdev)
 		.crashed = false,
 	};
 
+	ssr->info->current_state = QCOM_SSR_AFTER_POWERUP;
 	srcu_notifier_call_chain(&ssr->info->notifier_list,
-				 QCOM_SSR_AFTER_POWERUP, &data);
+				 ssr->info->current_state, &data);
 	return 0;
 }
 
@@ -446,8 +452,9 @@ static void ssr_notify_stop(struct rproc_subdev *subdev, bool crashed)
 		.crashed = crashed,
 	};
 
+	ssr->info->current_state = QCOM_SSR_BEFORE_SHUTDOWN;
 	srcu_notifier_call_chain(&ssr->info->notifier_list,
-				 QCOM_SSR_BEFORE_SHUTDOWN, &data);
+				 ssr->info->current_state, &data);
 }
 
 static void ssr_notify_unprepare(struct rproc_subdev *subdev)
@@ -458,8 +465,9 @@ static void ssr_notify_unprepare(struct rproc_subdev *subdev)
 		.crashed = false,
 	};
 
+	ssr->info->current_state = QCOM_SSR_AFTER_SHUTDOWN;
 	srcu_notifier_call_chain(&ssr->info->notifier_list,
-				 QCOM_SSR_AFTER_SHUTDOWN, &data);
+				 ssr->info->current_state, &data);
 }
 
 /**
@@ -484,6 +492,7 @@ void qcom_add_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr,
 	}
 
 	ssr->info = info;
+	ssr->info->current_state = QCOM_SSR_BEFORE_POWERUP;
 	ssr->subdev.prepare = ssr_notify_prepare;
 	ssr->subdev.start = ssr_notify_start;
 	ssr->subdev.stop = ssr_notify_stop;
-- 
2.36.0

[RFC PATCH 1/2] remoteproc: qcom: Report last event on SSR notifier registration

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Keep track of the last SSR event of a remoteproc, then report it
to each newly registered notifier block. This prevents drivers
from waiting indefinitely for an event that has already happened
before they registered their notifier blocks.

Fixes: a899d324863a3 ("slimbus: qcom-ngd-ctrl: add Sub System Restart support")
Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/remoteproc/qcom_common.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 4b91e3c9eafa..ff85ea564129 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -85,6 +85,7 @@ struct qcom_ssr_subsystem {
 	const char *name;
 	struct srcu_notifier_head notifier_list;
 	struct list_head list;
+	enum qcom_ssr_notify_type current_state;
 };
 
 static LIST_HEAD(qcom_ssr_subsystem_list);
@@ -392,6 +393,9 @@ void *qcom_register_ssr_notifier(const char *name, struct notifier_block *nb)
 
 	srcu_notifier_chain_register(&info->notifier_list, nb);
 
+	/* Notify newly registered notifier block of current remoteproc state */
+	nb->notifier_call(nb, info->current_state, NULL);
+
 	return &info->notifier_list;
 }
 EXPORT_SYMBOL_GPL(qcom_register_ssr_notifier);
@@ -420,8 +424,9 @@ static int ssr_notify_prepare(struct rproc_subdev *subdev)
 		.crashed = false,
 	};
 
+	ssr->info->current_state = QCOM_SSR_BEFORE_POWERUP;
 	srcu_notifier_call_chain(&ssr->info->notifier_list,
-				 QCOM_SSR_BEFORE_POWERUP, &data);
+				 ssr->info->current_state, &data);
 	return 0;
 }
 
@@ -433,8 +438,9 @@ static int ssr_notify_start(struct rproc_subdev *subdev)
 		.crashed = false,
 	};
 
+	ssr->info->current_state = QCOM_SSR_AFTER_POWERUP;
 	srcu_notifier_call_chain(&ssr->info->notifier_list,
-				 QCOM_SSR_AFTER_POWERUP, &data);
+				 ssr->info->current_state, &data);
 	return 0;
 }
 
@@ -446,8 +452,9 @@ static void ssr_notify_stop(struct rproc_subdev *subdev, bool crashed)
 		.crashed = crashed,
 	};
 
+	ssr->info->current_state = QCOM_SSR_BEFORE_SHUTDOWN;
 	srcu_notifier_call_chain(&ssr->info->notifier_list,
-				 QCOM_SSR_BEFORE_SHUTDOWN, &data);
+				 ssr->info->current_state, &data);
 }
 
 static void ssr_notify_unprepare(struct rproc_subdev *subdev)
@@ -458,8 +465,9 @@ static void ssr_notify_unprepare(struct rproc_subdev *subdev)
 		.crashed = false,
 	};
 
+	ssr->info->current_state = QCOM_SSR_AFTER_SHUTDOWN;
 	srcu_notifier_call_chain(&ssr->info->notifier_list,
-				 QCOM_SSR_AFTER_SHUTDOWN, &data);
+				 ssr->info->current_state, &data);
 }
 
 /**
@@ -484,6 +492,7 @@ void qcom_add_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr,
 	}
 
 	ssr->info = info;
+	ssr->info->current_state = QCOM_SSR_BEFORE_POWERUP;
 	ssr->subdev.prepare = ssr_notify_prepare;
 	ssr->subdev.start = ssr_notify_start;
 	ssr->subdev.stop = ssr_notify_stop;
-- 
2.36.0

[RFC PATCH 2/2] slimbus: qcom-ngd-ctrl: Initialize ngd_up_work before it can be scheduled

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

ngd_up_work can be scheduled by the SSR notifier, which is registered
before it is initialized. Move initialization of ngd_up_work before
SSR notifier registration.

Fixes: a899d324863a3 ("slimbus: qcom-ngd-ctrl: add Sub System Restart support")
Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/slimbus/qcom-ngd-ctrl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index 7040293c2ee8..73b9abba305f 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -1490,7 +1490,6 @@ static int qcom_slim_ngd_probe(struct platform_device *pdev)
 	}
 
 	INIT_WORK(&ctrl->m_work, qcom_slim_ngd_master_worker);
-	INIT_WORK(&ctrl->ngd_up_work, qcom_slim_ngd_up_worker);
 	ctrl->mwq = create_singlethread_workqueue("ngd_master");
 	if (!ctrl->mwq) {
 		dev_err(&pdev->dev, "Failed to start master worker\n");
@@ -1539,6 +1538,8 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	INIT_WORK(&ctrl->ngd_up_work, qcom_slim_ngd_up_worker);
+
 	ctrl->nb.notifier_call = qcom_slim_ngd_ssr_notify;
 	ctrl->notifier = qcom_register_ssr_notifier("lpass", &ctrl->nb);
 	if (IS_ERR(ctrl->notifier))
-- 
2.36.0

[RFC PATCH 2/2] slimbus: qcom-ngd-ctrl: Initialize ngd_up_work before it can be scheduled

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

ngd_up_work can be scheduled by the SSR notifier, which is registered
before it is initialized. Move initialization of ngd_up_work before
SSR notifier registration.

Fixes: a899d324863a3 ("slimbus: qcom-ngd-ctrl: add Sub System Restart support")
Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/slimbus/qcom-ngd-ctrl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index 7040293c2ee8..73b9abba305f 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -1490,7 +1490,6 @@ static int qcom_slim_ngd_probe(struct platform_device *pdev)
 	}
 
 	INIT_WORK(&ctrl->m_work, qcom_slim_ngd_master_worker);
-	INIT_WORK(&ctrl->ngd_up_work, qcom_slim_ngd_up_worker);
 	ctrl->mwq = create_singlethread_workqueue("ngd_master");
 	if (!ctrl->mwq) {
 		dev_err(&pdev->dev, "Failed to start master worker\n");
@@ -1539,6 +1538,8 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	INIT_WORK(&ctrl->ngd_up_work, qcom_slim_ngd_up_worker);
+
 	ctrl->nb.notifier_call = qcom_slim_ngd_ssr_notify;
 	ctrl->notifier = qcom_register_ssr_notifier("lpass", &ctrl->nb);
 	if (IS_ERR(ctrl->notifier))
-- 
2.36.0