diff mbox series

[v3,2/4] remoteproc: sysmon: Expose the shutdown result

Message ID 20201122054135.802935-3-bjorn.andersson@linaro.org
State Accepted
Commit 5c212aaf5457ca5bd99aba3ad29a4a17f8129939
Headers show
Series remoteproc: Improvement for the Qualcomm sysmon | expand

Commit Message

Bjorn Andersson Nov. 22, 2020, 5:41 a.m. UTC
A graceful shutdown of the Qualcomm remote processors where
traditionally performed by invoking a shared memory state signal and
waiting for the associated ack.

This was later superseded by the "sysmon" mechanism, where some form of
shared memory bus is used to send a "graceful shutdown request" message
and one of more signals comes back to indicate its success.

But when this newer mechanism is in effect the firmware is shut down by
the time the older mechanism, implemented in the remoteproc drivers,
attempts to perform a graceful shutdown - and as such it will never
receive an ack back.

This patch therefor track the success of the latest shutdown attempt in
sysmon and exposes a new function in the API that the remoteproc driver
can use to query the success and the necessity of invoking the older
mechanism.

Tested-by: Steev Klimaszewski <steev@kali.org>

Reviewed-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

---

Change since v2:
- None

 drivers/remoteproc/qcom_common.h |  6 +++
 drivers/remoteproc/qcom_sysmon.c | 82 ++++++++++++++++++++++++--------
 2 files changed, 69 insertions(+), 19 deletions(-)

-- 
2.28.0

Comments

Evan Green Dec. 7, 2020, 7:59 p.m. UTC | #1
On Sat, Nov 21, 2020 at 9:43 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>

> A graceful shutdown of the Qualcomm remote processors where

> traditionally performed by invoking a shared memory state signal and

> waiting for the associated ack.

>

> This was later superseded by the "sysmon" mechanism, where some form of

> shared memory bus is used to send a "graceful shutdown request" message

> and one of more signals comes back to indicate its success.

>

> But when this newer mechanism is in effect the firmware is shut down by

> the time the older mechanism, implemented in the remoteproc drivers,

> attempts to perform a graceful shutdown - and as such it will never

> receive an ack back.

>

> This patch therefor track the success of the latest shutdown attempt in

> sysmon and exposes a new function in the API that the remoteproc driver

> can use to query the success and the necessity of invoking the older

> mechanism.

>

> Tested-by: Steev Klimaszewski <steev@kali.org>

> Reviewed-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>

> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> ---

>

> Change since v2:

> - None

>

>  drivers/remoteproc/qcom_common.h |  6 +++

>  drivers/remoteproc/qcom_sysmon.c | 82 ++++++++++++++++++++++++--------

>  2 files changed, 69 insertions(+), 19 deletions(-)

>

> diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h

> index dfc641c3a98b..8ba9052955bd 100644

> --- a/drivers/remoteproc/qcom_common.h

> +++ b/drivers/remoteproc/qcom_common.h

> @@ -51,6 +51,7 @@ struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,

>                                            const char *name,

>                                            int ssctl_instance);

>  void qcom_remove_sysmon_subdev(struct qcom_sysmon *sysmon);

> +bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon);

>  #else

>  static inline struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,

>                                                          const char *name,

> @@ -62,6 +63,11 @@ static inline struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,

>  static inline void qcom_remove_sysmon_subdev(struct qcom_sysmon *sysmon)

>  {

>  }

> +

> +static inline bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon)

> +{

> +       return false;

> +}

>  #endif

>

>  #endif

> diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c

> index b37b111b15b3..a428b707a6de 100644

> --- a/drivers/remoteproc/qcom_sysmon.c

> +++ b/drivers/remoteproc/qcom_sysmon.c

> @@ -44,6 +44,7 @@ struct qcom_sysmon {

>         struct mutex lock;

>

>         bool ssr_ack;

> +       bool shutdown_acked;

>

>         struct qmi_handle qmi;

>         struct sockaddr_qrtr ssctl;

> @@ -115,10 +116,13 @@ static void sysmon_send_event(struct qcom_sysmon *sysmon,

>  /**

>   * sysmon_request_shutdown() - request graceful shutdown of remote

>   * @sysmon:    sysmon context

> + *

> + * Return: boolean indicator of the remote processor acking the request

>   */

> -static void sysmon_request_shutdown(struct qcom_sysmon *sysmon)

> +static bool sysmon_request_shutdown(struct qcom_sysmon *sysmon)

>  {

>         char *req = "ssr:shutdown";

> +       bool acked = false;

>         int ret;

>

>         mutex_lock(&sysmon->lock);

> @@ -141,9 +145,13 @@ static void sysmon_request_shutdown(struct qcom_sysmon *sysmon)

>         if (!sysmon->ssr_ack)

>                 dev_err(sysmon->dev,

>                         "unexpected response to sysmon shutdown request\n");

> +       else

> +               acked = true;

>

>  out_unlock:

>         mutex_unlock(&sysmon->lock);

> +

> +       return acked;

>  }

>

>  static int sysmon_callback(struct rpmsg_device *rpdev, void *data, int count,

> @@ -297,14 +305,33 @@ static struct qmi_msg_handler qmi_indication_handler[] = {

>         {}

>  };

>

> +static bool ssctl_request_shutdown_wait(struct qcom_sysmon *sysmon)

> +{

> +       int ret;

> +

> +       ret = wait_for_completion_timeout(&sysmon->shutdown_comp, 10 * HZ);

> +       if (ret)

> +               return true;

> +

> +       ret = try_wait_for_completion(&sysmon->ind_comp);

> +       if (ret)

> +               return true;

> +

> +       dev_err(sysmon->dev, "timeout waiting for shutdown ack\n");

> +       return false;

> +}

> +

>  /**

>   * ssctl_request_shutdown() - request shutdown via SSCTL QMI service

>   * @sysmon:    sysmon context

> + *

> + * Return: boolean indicator of the remote processor acking the request

>   */

> -static void ssctl_request_shutdown(struct qcom_sysmon *sysmon)

> +static bool ssctl_request_shutdown(struct qcom_sysmon *sysmon)

>  {

>         struct ssctl_shutdown_resp resp;

>         struct qmi_txn txn;

> +       bool acked = false;

>         int ret;

>

>         reinit_completion(&sysmon->ind_comp);

> @@ -312,7 +339,7 @@ static void ssctl_request_shutdown(struct qcom_sysmon *sysmon)

>         ret = qmi_txn_init(&sysmon->qmi, &txn, ssctl_shutdown_resp_ei, &resp);

>         if (ret < 0) {

>                 dev_err(sysmon->dev, "failed to allocate QMI txn\n");

> -               return;

> +               return false;

>         }

>

>         ret = qmi_send_request(&sysmon->qmi, &sysmon->ssctl, &txn,

> @@ -320,27 +347,23 @@ static void ssctl_request_shutdown(struct qcom_sysmon *sysmon)

>         if (ret < 0) {

>                 dev_err(sysmon->dev, "failed to send shutdown request\n");

>                 qmi_txn_cancel(&txn);

> -               return;

> +               return false;

>         }

>

>         ret = qmi_txn_wait(&txn, 5 * HZ);

> -       if (ret < 0)

> +       if (ret < 0) {

>                 dev_err(sysmon->dev, "failed receiving QMI response\n");

> -       else if (resp.resp.result)

> +       } else if (resp.resp.result) {

>                 dev_err(sysmon->dev, "shutdown request failed\n");

> -       else

> +       } else {

>                 dev_dbg(sysmon->dev, "shutdown request completed\n");

> -

> -       if (sysmon->shutdown_irq > 0) {

> -               ret = wait_for_completion_timeout(&sysmon->shutdown_comp,

> -                                                 10 * HZ);

> -               if (!ret) {

> -                       ret = try_wait_for_completion(&sysmon->ind_comp);

> -                       if (!ret)

> -                               dev_err(sysmon->dev,

> -                                       "timeout waiting for shutdown ack\n");

> -               }

> +               acked = true;

>         }

> +

> +       if (sysmon->shutdown_irq > 0)

> +               return ssctl_request_shutdown_wait(sysmon);

> +

> +       return acked;

>  }

>

>  /**

> @@ -510,6 +533,9 @@ static void sysmon_stop(struct rproc_subdev *subdev, bool crashed)

>                 .subsys_name = sysmon->name,

>                 .ssr_event = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN

>         };

> +       bool acked;

> +

> +       sysmon->shutdown_acked = false;

>

>         mutex_lock(&sysmon->state_lock);

>         sysmon->state = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN;

> @@ -521,9 +547,11 @@ static void sysmon_stop(struct rproc_subdev *subdev, bool crashed)

>                 return;

>

>         if (sysmon->ssctl_version)


> -               ssctl_request_shutdown(sysmon);

> +               acked = ssctl_request_shutdown(sysmon);

>         else if (sysmon->ept)

> -               sysmon_request_shutdown(sysmon);

> +               acked = sysmon_request_shutdown(sysmon);

> +

> +       sysmon->shutdown_acked = acked;


Guenter noticed that the 0-day bot complains about acked being
potentially uninitialized here. He put a fix for us into Chrome OS:

https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2577656

Bjorn, do you want to tweak the patch in your tree?
-Evan
Bjorn Andersson Dec. 7, 2020, 8:06 p.m. UTC | #2
On Mon, Dec 7, 2020 at 2:00 PM Evan Green <evgreen@chromium.org> wrote:
>

> On Sat, Nov 21, 2020 at 9:43 PM Bjorn Andersson

> <bjorn.andersson@linaro.org> wrote:

> >

> > A graceful shutdown of the Qualcomm remote processors where

> > traditionally performed by invoking a shared memory state signal and

> > waiting for the associated ack.

> >

> > This was later superseded by the "sysmon" mechanism, where some form of

> > shared memory bus is used to send a "graceful shutdown request" message

> > and one of more signals comes back to indicate its success.

> >

> > But when this newer mechanism is in effect the firmware is shut down by

> > the time the older mechanism, implemented in the remoteproc drivers,

> > attempts to perform a graceful shutdown - and as such it will never

> > receive an ack back.

> >

> > This patch therefor track the success of the latest shutdown attempt in

> > sysmon and exposes a new function in the API that the remoteproc driver

> > can use to query the success and the necessity of invoking the older

> > mechanism.

> >

> > Tested-by: Steev Klimaszewski <steev@kali.org>

> > Reviewed-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>

> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> > ---

> >

> > Change since v2:

> > - None

> >

> >  drivers/remoteproc/qcom_common.h |  6 +++

> >  drivers/remoteproc/qcom_sysmon.c | 82 ++++++++++++++++++++++++--------

> >  2 files changed, 69 insertions(+), 19 deletions(-)

> >

> > diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h

> > index dfc641c3a98b..8ba9052955bd 100644

> > --- a/drivers/remoteproc/qcom_common.h

> > +++ b/drivers/remoteproc/qcom_common.h

> > @@ -51,6 +51,7 @@ struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,

> >                                            const char *name,

> >                                            int ssctl_instance);

> >  void qcom_remove_sysmon_subdev(struct qcom_sysmon *sysmon);

> > +bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon);

> >  #else

> >  static inline struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,

> >                                                          const char *name,

> > @@ -62,6 +63,11 @@ static inline struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,

> >  static inline void qcom_remove_sysmon_subdev(struct qcom_sysmon *sysmon)

> >  {

> >  }

> > +

> > +static inline bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon)

> > +{

> > +       return false;

> > +}

> >  #endif

> >

> >  #endif

> > diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c

> > index b37b111b15b3..a428b707a6de 100644

> > --- a/drivers/remoteproc/qcom_sysmon.c

> > +++ b/drivers/remoteproc/qcom_sysmon.c

> > @@ -44,6 +44,7 @@ struct qcom_sysmon {

> >         struct mutex lock;

> >

> >         bool ssr_ack;

> > +       bool shutdown_acked;

> >

> >         struct qmi_handle qmi;

> >         struct sockaddr_qrtr ssctl;

> > @@ -115,10 +116,13 @@ static void sysmon_send_event(struct qcom_sysmon *sysmon,

> >  /**

> >   * sysmon_request_shutdown() - request graceful shutdown of remote

> >   * @sysmon:    sysmon context

> > + *

> > + * Return: boolean indicator of the remote processor acking the request

> >   */

> > -static void sysmon_request_shutdown(struct qcom_sysmon *sysmon)

> > +static bool sysmon_request_shutdown(struct qcom_sysmon *sysmon)

> >  {

> >         char *req = "ssr:shutdown";

> > +       bool acked = false;

> >         int ret;

> >

> >         mutex_lock(&sysmon->lock);

> > @@ -141,9 +145,13 @@ static void sysmon_request_shutdown(struct qcom_sysmon *sysmon)

> >         if (!sysmon->ssr_ack)

> >                 dev_err(sysmon->dev,

> >                         "unexpected response to sysmon shutdown request\n");

> > +       else

> > +               acked = true;

> >

> >  out_unlock:

> >         mutex_unlock(&sysmon->lock);

> > +

> > +       return acked;

> >  }

> >

> >  static int sysmon_callback(struct rpmsg_device *rpdev, void *data, int count,

> > @@ -297,14 +305,33 @@ static struct qmi_msg_handler qmi_indication_handler[] = {

> >         {}

> >  };

> >

> > +static bool ssctl_request_shutdown_wait(struct qcom_sysmon *sysmon)

> > +{

> > +       int ret;

> > +

> > +       ret = wait_for_completion_timeout(&sysmon->shutdown_comp, 10 * HZ);

> > +       if (ret)

> > +               return true;

> > +

> > +       ret = try_wait_for_completion(&sysmon->ind_comp);

> > +       if (ret)

> > +               return true;

> > +

> > +       dev_err(sysmon->dev, "timeout waiting for shutdown ack\n");

> > +       return false;

> > +}

> > +

> >  /**

> >   * ssctl_request_shutdown() - request shutdown via SSCTL QMI service

> >   * @sysmon:    sysmon context

> > + *

> > + * Return: boolean indicator of the remote processor acking the request

> >   */

> > -static void ssctl_request_shutdown(struct qcom_sysmon *sysmon)

> > +static bool ssctl_request_shutdown(struct qcom_sysmon *sysmon)

> >  {

> >         struct ssctl_shutdown_resp resp;

> >         struct qmi_txn txn;

> > +       bool acked = false;

> >         int ret;

> >

> >         reinit_completion(&sysmon->ind_comp);

> > @@ -312,7 +339,7 @@ static void ssctl_request_shutdown(struct qcom_sysmon *sysmon)

> >         ret = qmi_txn_init(&sysmon->qmi, &txn, ssctl_shutdown_resp_ei, &resp);

> >         if (ret < 0) {

> >                 dev_err(sysmon->dev, "failed to allocate QMI txn\n");

> > -               return;

> > +               return false;

> >         }

> >

> >         ret = qmi_send_request(&sysmon->qmi, &sysmon->ssctl, &txn,

> > @@ -320,27 +347,23 @@ static void ssctl_request_shutdown(struct qcom_sysmon *sysmon)

> >         if (ret < 0) {

> >                 dev_err(sysmon->dev, "failed to send shutdown request\n");

> >                 qmi_txn_cancel(&txn);

> > -               return;

> > +               return false;

> >         }

> >

> >         ret = qmi_txn_wait(&txn, 5 * HZ);

> > -       if (ret < 0)

> > +       if (ret < 0) {

> >                 dev_err(sysmon->dev, "failed receiving QMI response\n");

> > -       else if (resp.resp.result)

> > +       } else if (resp.resp.result) {

> >                 dev_err(sysmon->dev, "shutdown request failed\n");

> > -       else

> > +       } else {

> >                 dev_dbg(sysmon->dev, "shutdown request completed\n");

> > -

> > -       if (sysmon->shutdown_irq > 0) {

> > -               ret = wait_for_completion_timeout(&sysmon->shutdown_comp,

> > -                                                 10 * HZ);

> > -               if (!ret) {

> > -                       ret = try_wait_for_completion(&sysmon->ind_comp);

> > -                       if (!ret)

> > -                               dev_err(sysmon->dev,

> > -                                       "timeout waiting for shutdown ack\n");

> > -               }

> > +               acked = true;

> >         }

> > +

> > +       if (sysmon->shutdown_irq > 0)

> > +               return ssctl_request_shutdown_wait(sysmon);

> > +

> > +       return acked;

> >  }

> >

> >  /**

> > @@ -510,6 +533,9 @@ static void sysmon_stop(struct rproc_subdev *subdev, bool crashed)

> >                 .subsys_name = sysmon->name,

> >                 .ssr_event = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN

> >         };

> > +       bool acked;

> > +

> > +       sysmon->shutdown_acked = false;

> >

> >         mutex_lock(&sysmon->state_lock);

> >         sysmon->state = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN;

> > @@ -521,9 +547,11 @@ static void sysmon_stop(struct rproc_subdev *subdev, bool crashed)

> >                 return;

> >

> >         if (sysmon->ssctl_version)

>

> > -               ssctl_request_shutdown(sysmon);

> > +               acked = ssctl_request_shutdown(sysmon);

> >         else if (sysmon->ept)

> > -               sysmon_request_shutdown(sysmon);

> > +               acked = sysmon_request_shutdown(sysmon);

> > +

> > +       sysmon->shutdown_acked = acked;

>

> Guenter noticed that the 0-day bot complains about acked being

> potentially uninitialized here. He put a fix for us into Chrome OS:

>

> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2577656

>

> Bjorn, do you want to tweak the patch in your tree?


No, I prefer not to force push to the tree. I did however merge and
push out Arnd's fixup to this. You can find it here:

https://git.kernel.org/pub/scm/linux/kernel/git/andersson/remoteproc.git/commit/?h=for-next&id=9d7b4a40387d0f91512a74caed6654ffa23d5ce4

Regards,
Bjorn
diff mbox series

Patch

diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
index dfc641c3a98b..8ba9052955bd 100644
--- a/drivers/remoteproc/qcom_common.h
+++ b/drivers/remoteproc/qcom_common.h
@@ -51,6 +51,7 @@  struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
 					   const char *name,
 					   int ssctl_instance);
 void qcom_remove_sysmon_subdev(struct qcom_sysmon *sysmon);
+bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon);
 #else
 static inline struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
 							 const char *name,
@@ -62,6 +63,11 @@  static inline struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
 static inline void qcom_remove_sysmon_subdev(struct qcom_sysmon *sysmon)
 {
 }
+
+static inline bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon)
+{
+	return false;
+}
 #endif
 
 #endif
diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
index b37b111b15b3..a428b707a6de 100644
--- a/drivers/remoteproc/qcom_sysmon.c
+++ b/drivers/remoteproc/qcom_sysmon.c
@@ -44,6 +44,7 @@  struct qcom_sysmon {
 	struct mutex lock;
 
 	bool ssr_ack;
+	bool shutdown_acked;
 
 	struct qmi_handle qmi;
 	struct sockaddr_qrtr ssctl;
@@ -115,10 +116,13 @@  static void sysmon_send_event(struct qcom_sysmon *sysmon,
 /**
  * sysmon_request_shutdown() - request graceful shutdown of remote
  * @sysmon:	sysmon context
+ *
+ * Return: boolean indicator of the remote processor acking the request
  */
-static void sysmon_request_shutdown(struct qcom_sysmon *sysmon)
+static bool sysmon_request_shutdown(struct qcom_sysmon *sysmon)
 {
 	char *req = "ssr:shutdown";
+	bool acked = false;
 	int ret;
 
 	mutex_lock(&sysmon->lock);
@@ -141,9 +145,13 @@  static void sysmon_request_shutdown(struct qcom_sysmon *sysmon)
 	if (!sysmon->ssr_ack)
 		dev_err(sysmon->dev,
 			"unexpected response to sysmon shutdown request\n");
+	else
+		acked = true;
 
 out_unlock:
 	mutex_unlock(&sysmon->lock);
+
+	return acked;
 }
 
 static int sysmon_callback(struct rpmsg_device *rpdev, void *data, int count,
@@ -297,14 +305,33 @@  static struct qmi_msg_handler qmi_indication_handler[] = {
 	{}
 };
 
+static bool ssctl_request_shutdown_wait(struct qcom_sysmon *sysmon)
+{
+	int ret;
+
+	ret = wait_for_completion_timeout(&sysmon->shutdown_comp, 10 * HZ);
+	if (ret)
+		return true;
+
+	ret = try_wait_for_completion(&sysmon->ind_comp);
+	if (ret)
+		return true;
+
+	dev_err(sysmon->dev, "timeout waiting for shutdown ack\n");
+	return false;
+}
+
 /**
  * ssctl_request_shutdown() - request shutdown via SSCTL QMI service
  * @sysmon:	sysmon context
+ *
+ * Return: boolean indicator of the remote processor acking the request
  */
-static void ssctl_request_shutdown(struct qcom_sysmon *sysmon)
+static bool ssctl_request_shutdown(struct qcom_sysmon *sysmon)
 {
 	struct ssctl_shutdown_resp resp;
 	struct qmi_txn txn;
+	bool acked = false;
 	int ret;
 
 	reinit_completion(&sysmon->ind_comp);
@@ -312,7 +339,7 @@  static void ssctl_request_shutdown(struct qcom_sysmon *sysmon)
 	ret = qmi_txn_init(&sysmon->qmi, &txn, ssctl_shutdown_resp_ei, &resp);
 	if (ret < 0) {
 		dev_err(sysmon->dev, "failed to allocate QMI txn\n");
-		return;
+		return false;
 	}
 
 	ret = qmi_send_request(&sysmon->qmi, &sysmon->ssctl, &txn,
@@ -320,27 +347,23 @@  static void ssctl_request_shutdown(struct qcom_sysmon *sysmon)
 	if (ret < 0) {
 		dev_err(sysmon->dev, "failed to send shutdown request\n");
 		qmi_txn_cancel(&txn);
-		return;
+		return false;
 	}
 
 	ret = qmi_txn_wait(&txn, 5 * HZ);
-	if (ret < 0)
+	if (ret < 0) {
 		dev_err(sysmon->dev, "failed receiving QMI response\n");
-	else if (resp.resp.result)
+	} else if (resp.resp.result) {
 		dev_err(sysmon->dev, "shutdown request failed\n");
-	else
+	} else {
 		dev_dbg(sysmon->dev, "shutdown request completed\n");
-
-	if (sysmon->shutdown_irq > 0) {
-		ret = wait_for_completion_timeout(&sysmon->shutdown_comp,
-						  10 * HZ);
-		if (!ret) {
-			ret = try_wait_for_completion(&sysmon->ind_comp);
-			if (!ret)
-				dev_err(sysmon->dev,
-					"timeout waiting for shutdown ack\n");
-		}
+		acked = true;
 	}
+
+	if (sysmon->shutdown_irq > 0)
+		return ssctl_request_shutdown_wait(sysmon);
+
+	return acked;
 }
 
 /**
@@ -510,6 +533,9 @@  static void sysmon_stop(struct rproc_subdev *subdev, bool crashed)
 		.subsys_name = sysmon->name,
 		.ssr_event = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN
 	};
+	bool acked;
+
+	sysmon->shutdown_acked = false;
 
 	mutex_lock(&sysmon->state_lock);
 	sysmon->state = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN;
@@ -521,9 +547,11 @@  static void sysmon_stop(struct rproc_subdev *subdev, bool crashed)
 		return;
 
 	if (sysmon->ssctl_version)
-		ssctl_request_shutdown(sysmon);
+		acked = ssctl_request_shutdown(sysmon);
 	else if (sysmon->ept)
-		sysmon_request_shutdown(sysmon);
+		acked = sysmon_request_shutdown(sysmon);
+
+	sysmon->shutdown_acked = acked;
 }
 
 static void sysmon_unprepare(struct rproc_subdev *subdev)
@@ -681,6 +709,22 @@  void qcom_remove_sysmon_subdev(struct qcom_sysmon *sysmon)
 }
 EXPORT_SYMBOL_GPL(qcom_remove_sysmon_subdev);
 
+/**
+ * qcom_sysmon_shutdown_acked() - query the success of the last shutdown
+ * @sysmon:	sysmon context
+ *
+ * When sysmon is used to request a graceful shutdown of the remote processor
+ * this can be used by the remoteproc driver to query the success, in order to
+ * know if it should fall back to other means of requesting a shutdown.
+ *
+ * Return: boolean indicator of the success of the last shutdown request
+ */
+bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon)
+{
+	return sysmon && sysmon->shutdown_acked;
+}
+EXPORT_SYMBOL_GPL(qcom_sysmon_shutdown_acked);
+
 /**
  * sysmon_probe() - probe sys_mon channel
  * @rpdev:	rpmsg device handle