Messages in this thread | | | Date | Tue, 10 Nov 2020 17:01:34 -0800 | From | rishabhb@codeauro ... | Subject | Re: [PATCH v2 2/4] remoteproc: sysmon: Expose the shutdown result |
| |
On 2020-11-04 20:50, Bjorn Andersson 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. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Changes since v1: > - New patch > > 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 38f63c968fa8..1c42f00010d3 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; > } > > /** > @@ -508,6 +531,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; > @@ -519,9 +545,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) > @@ -679,6 +707,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
Reviewed-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
| |