Re: [PATCH v1] remoteproc: core: do pm relax when not first crash

[Aiqun(Maria) Yu]

On 9/17/2022 1:05 AM, Mathieu Poirier wrote:
> On Tue, 13 Sept 2022 at 05:03, Aiqun(Maria) Yu <quic_aiquny@xxxxxxxxxxx> wrote:
> > Hi Mathieu,
> >
> > pm_awake and pm_relax needed to be used as a pair. There is chance that
> > pm_relax is not being called, and make the device always in cannot
> > suspend state.
> >
> > On 9/10/2022 3:23 AM, Mathieu Poirier wrote:
> > > Hi Maria,
> > >
> > > On Fri, Sep 09, 2022 at 04:33:33PM +0800, Maria Yu wrote:
> > > > Even if it is not first crash, need to relax the pm
> > > > wakelock otherwise the device will stay awake.
> > >
> > > The goal is exactly to keep the device awake...
> > >
> > > > Signed-off-by: Maria Yu <quic_aiquny@xxxxxxxxxxx>
> > > > ---
> > > >    drivers/remoteproc/remoteproc_core.c | 1 +
> > > >    1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> > > > index e5279ed9a8d7..30078043e939 100644
> > > > --- a/drivers/remoteproc/remoteproc_core.c
> > > > +++ b/drivers/remoteproc/remoteproc_core.c
> > > > @@ -1956,6 +1956,7 @@ 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);
> > > > +            pm_relax(rproc->dev.parent);
> > >
> > > If we are here it means that rproc_crash_handler_work() has already been called
> > > _and_ that a recovery is in process.  When the first crash handler completes
> > > pm_relax() will be called and the device will go to sleep as expected.
> > If the rproc->state cannot be changed to running state, the device will
> > always be awake from this return.
> > Also APROC_OFFLINE state can be given in other path like an shutdown
> > request is issued.
> >
> > While this patch is not considering carefully as well, I think I need to
> > upload a new patchset with an ordered workqueue to make each work have
> > each pm_relax before return.
> > what do you think?
>
> I was travelling this week and as such did not have the time to
> follow-up with this thread, something I will do next week.
Thx for follow up. I have new patchset posted on this thread.
After reconsideration, extra action can be done only for RPROC_OFFLINE 
state. Pls check the newest v4 patchset on this thread.
> > > Thanks,
> > > Mathieu
> > >
> > > >               return;
> > > >       }
> > > >
> > > > --
> > > > 2.7.4
> >
> >
> > --
> > Thx and BRs,
> > Aiqun(Maria) Yu


--
Thx and BRs,
Aiqun(Maria) Yu