| Message ID | 1663236256-52289-1-git-send-email-quic_aiquny@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] remoteproc: core: do pm relax when in RPROC_OFFLINE | expand |
Hi, On 9/15/2022 3:34 PM, Maria Yu wrote: > RPROC_OFFLINE state indicate there is no recovery process > is in progress and no chance to do the pm_relax. > Because when recovering from crash, rproc->lock is hold and s/hold/held ? > state is RPROC_CRASHED -> RPROC_OFFLINE -> RPROC_RUNNING, > and then unlock rproc->lock. > When the state is in RPROC_OFFLINE it means separate request > of rproc_stop was done and no need to hold the wakeup source > in crash handler to recover any more. > > Signed-off-by: Maria Yu <quic_aiquny@quicinc.com> > --- > drivers/remoteproc/remoteproc_core.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index e5279ed9a8d7..247ced6b0655 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -1956,6 +1956,17 @@ static void rproc_crash_handler_work(struct work_struct *work) > if (rproc->state == RPROC_CRASHED || rproc->state == RPROC_OFFLINE) { > /* handle only the first crash detected */ > mutex_unlock(&rproc->lock); > + /* > + * RPROC_OFFLINE state indicate there is no recovery process > + * is in progress and no chance to have pm_relax in place. > + * Because when recovering from crash, rproc->lock is hold and ..ditto.. > + * state is RPROC_CRASHED -> RPROC_OFFLINE -> RPROC_RUNNING, > + * and then unlock rproc->lock. > + * RPROC_OFFLINE is only an intermediate state in recovery > + * process. > + */ > + if (rproc->state == RPROC_OFFLINE) > + pm_relax(rproc->dev.parent); Looks good, apart from minor nit. Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com> -Mukesh > return; > } >
On 9/15/2022 8:47 PM, Mukesh Ojha wrote: > Hi, > > On 9/15/2022 3:34 PM, Maria Yu wrote: >> RPROC_OFFLINE state indicate there is no recovery process >> is in progress and no chance to do the pm_relax. >> Because when recovering from crash, rproc->lock is hold and > > s/hold/held ? > Thanks. >> state is RPROC_CRASHED -> RPROC_OFFLINE -> RPROC_RUNNING, >> and then unlock rproc->lock. >> When the state is in RPROC_OFFLINE it means separate request >> of rproc_stop was done and no need to hold the wakeup source >> in crash handler to recover any more. >> >> Signed-off-by: Maria Yu <quic_aiquny@quicinc.com> >> --- >> drivers/remoteproc/remoteproc_core.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/remoteproc/remoteproc_core.c >> b/drivers/remoteproc/remoteproc_core.c >> index e5279ed9a8d7..247ced6b0655 100644 >> --- a/drivers/remoteproc/remoteproc_core.c >> +++ b/drivers/remoteproc/remoteproc_core.c >> @@ -1956,6 +1956,17 @@ static void rproc_crash_handler_work(struct >> work_struct *work) >> if (rproc->state == RPROC_CRASHED || rproc->state == >> RPROC_OFFLINE) { >> /* handle only the first crash detected */ >> mutex_unlock(&rproc->lock); >> + /* >> + * RPROC_OFFLINE state indicate there is no recovery process >> + * is in progress and no chance to have pm_relax in place. >> + * Because when recovering from crash, rproc->lock is hold and > > ..ditto.. > >> + * state is RPROC_CRASHED -> RPROC_OFFLINE -> RPROC_RUNNING, >> + * and then unlock rproc->lock. >> + * RPROC_OFFLINE is only an intermediate state in recovery >> + * process. >> + */ >> + if (rproc->state == RPROC_OFFLINE) >> + pm_relax(rproc->dev.parent); > > > Looks good, apart from minor nit. :) > > Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com> > > -Mukesh >> return; >> }
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index e5279ed9a8d7..247ced6b0655 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -1956,6 +1956,17 @@ static void rproc_crash_handler_work(struct work_struct *work) if (rproc->state == RPROC_CRASHED || rproc->state == RPROC_OFFLINE) { /* handle only the first crash detected */ mutex_unlock(&rproc->lock); + /* + * RPROC_OFFLINE state indicate there is no recovery process + * is in progress and no chance to have pm_relax in place. + * Because when recovering from crash, rproc->lock is hold and + * state is RPROC_CRASHED -> RPROC_OFFLINE -> RPROC_RUNNING, + * and then unlock rproc->lock. + * RPROC_OFFLINE is only an intermediate state in recovery + * process. + */ + if (rproc->state == RPROC_OFFLINE) + pm_relax(rproc->dev.parent); return; }
RPROC_OFFLINE state indicate there is no recovery process is in progress and no chance to do the pm_relax. Because when recovering from crash, rproc->lock is hold and state is RPROC_CRASHED -> RPROC_OFFLINE -> RPROC_RUNNING, and then unlock rproc->lock. When the state is in RPROC_OFFLINE it means separate request of rproc_stop was done and no need to hold the wakeup source in crash handler to recover any more. Signed-off-by: Maria Yu <quic_aiquny@quicinc.com> --- drivers/remoteproc/remoteproc_core.c | 11 +++++++++++ 1 file changed, 11 insertions(+)