Re: [PATCH] Input: hisi_powerkey: Enable system-wakeup for s2idle

[Ulf Hansson]

On Wed, 12 Mar 2025 at 17:37, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>
> On Tue, 11 Mar 2025 at 06:09, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote:
> >
> > Hi Ulf,
> >
> > On Thu, Mar 06, 2025 at 12:50:21PM +0100, Ulf Hansson wrote:
> > > To wake up the system from s2idle when pressing the power-button, let's
> > > convert from using pm_wakeup_event() to pm_wakeup_dev_event(), as it allows
> > > us to specify the "hard" in-parameter, which needs to be set for s2idle.
> >
> > I was looking at pm_wakeup_event() and pm_wakeup_dev_event() and I am
> > afraid I do not understand the distinction. Why would we want to
> > abort suspend by only by some wakeup sources and not by others? And why
> > does a driver need to know whether a system uses s2idle or some other
> > implementation of low power state?
>
> Good question!
>
> In regards to waking up. The CPU that wakes up from suspend-to-idle
> may actually decide to just go back to idle, without doing a full
> system resume - unless there is a wakeup that requires the system to
> resume.
>
> In suspend-to-ram a wakeup will always trigger a full system resume.
>
> In most cases a driver doesn't really need to distinguish between
> these cases, yet the wakeup APIs are designed to allow it. That's the
> reason why pm_system_wakeup() needs to be called (controlled by "hard"
> in-parameter to pm_wakeup_dev_event()).
>
> >
> > FWIW we have Chromebooks that use S0ix and Chromebooks that use S3 as
> > well as ARM Chromebooks and I do not think they use
> > pm_wakeup_dev_event() variant.
> >
> > I'm cc-ing Rafael to give us some guidance.
>
> Good, let's see if there is something I failed to explain.
>
> >
> > >
> > > Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> > > ---
> > >  drivers/input/misc/hisi_powerkey.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/input/misc/hisi_powerkey.c b/drivers/input/misc/hisi_powerkey.c
> > > index d3c293a95d32..d315017324d9 100644
> > > --- a/drivers/input/misc/hisi_powerkey.c
> > > +++ b/drivers/input/misc/hisi_powerkey.c
> > > @@ -30,7 +30,7 @@ static irqreturn_t hi65xx_power_press_isr(int irq, void *q)
> > >  {
> > >       struct input_dev *input = q;
> > >
> > > -     pm_wakeup_event(input->dev.parent, MAX_HELD_TIME);
> > > +     pm_wakeup_dev_event(input->dev.parent, MAX_HELD_TIME, true);
> > >       input_report_key(input, KEY_POWER, 1);
> > >       input_sync(input);
> > >
> > > --
> > > 2.43.0
> > >
> >
> > Thanks.
> >
> > --
> > Dmitry
>
> Kind regards
> Uffe

Dmitry, is there anything else I can do to make you queue this one?

S2idle is currently broken for Hikey and - it's rather annoying.

Kind regards
Uffe