Re: [PATCH v4 1/2] Input: enable i8042-level wakeup control

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Wed, Aug 03, 2011 at 09:04:56AM +0100, Daniel Drake wrote:
> On Wed, Aug 3, 2011 at 7:59 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > I am not sure that we should be marking input devices themselves as
> > wakeup capable - they are in no way physical devices. I'd stop at serio
> > level...
> 
> We need a way to make sure the device state is untouched during
> suspend/resume. Would you be happier if an input device looked at the
> device_may_wakeup() of its parent?

I think we should leave input core out of it completely, see below...

> 
> >>
> >> diff --git a/drivers/input/input.c b/drivers/input/input.c
> >> index da38d97..639aba7 100644
> >> --- a/drivers/input/input.c
> >> +++ b/drivers/input/input.c
> >> @@ -1588,6 +1588,9 @@ static int input_dev_suspend(struct device *dev)
> >>  {
> >>       struct input_dev *input_dev = to_input_dev(dev);
> >>
> >> +     if (device_may_wakeup(dev))
> >> +             return 0;
> >> +
> >
> > On suspend should we not try to shut off leds and sound?
> 
> Thats right. The setup is that the system appears to be running as
> normal, our display also remains on during suspend (and wifi, and so
> on). Bar the laptop's power LED which goes from always-on to flashing,
> the user is unaware that the laptop has suspended.

No, I do not believe we can do what you are doing in input core. You are
changing behavior for one platform and one (or 2 drivers) that will not
be matched (at least I don't think so) by anything else out there. What
will happen if you plug a random USB keyboard into OLPC box? Will it keep
leds powered?

> 
> >>       mutex_lock(&input_dev->mutex);
> >>
> >>       if (input_dev->users)
> >> @@ -1602,7 +1605,8 @@ static int input_dev_resume(struct device *dev)
> >>  {
> >>       struct input_dev *input_dev = to_input_dev(dev);
> >>
> >> -     input_reset_device(input_dev);
> >> +     if (!device_may_wakeup(dev))
> >> +             input_reset_device(input_dev);
> >>
> >
> > Does the controller wakes up the system on key release or only press? My
> > concern is with cases when we suspend with a key pressed and wake up
> > with it already released.
> 
> It wakes up on key press, but our EC buffers communication, so both
> the key press and key release event would be delivered in the above
> scenario.

Just to confirm, we 3 events will be delivered in this case:

1 - old key release
2 - wakeup key press
3 - wakeup key release

?

I also wonder what will happen with non-PS2 devices...

Instead of wiring it all through input core could we contain this in
atkbd and hgpk by registering pm_notifiers and ignoring certain requests
from input/serio cores during system state transition on OLPC only?

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html