Message-ID: <CAJZ5v0jWABjBEErP5zHXW=S-a-5SD7zP79cwwgS3DjEyG5bCEA@mail.gmail.com>
Date:	Thu, 21 Jan 2016 00:01:59 +0100
From:	"Rafael J. Wysocki" <rafael@...nel.org>
To:	"Rafael J. Wysocki" <rafael@...nel.org>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Rob Herring <robh@...nel.org>,
	Grant Likely <grant.likely@...aro.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Thierry Reding <treding@...dia.com>,
	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] driver-core: platform: automatically mark wakeup devices

On Wed, Jan 20, 2016 at 2:51 PM, Rafael J. Wysocki <rafael@...nel.org> wrote:
> On Wed, Jan 20, 2016 at 3:40 AM, Rafael J. Wysocki <rafael@...nel.org> wrote:
>> On Wed, Jan 20, 2016 at 1:45 AM, Dmitry Torokhov
>> <dmitry.torokhov@...il.com> wrote:
>
> [cut]
>
>>>> > I am not aware of any drivers denoting itself as being wakeup sources
>>>> > and not enabling wakeup. Do you have examples?
>>>>
>>>> Yes, I do.
>>>>
>>>> i8042 is one of them, exactly to avoid enabling those devices to do
>>>> system wakeup on systems where they were not enabled to do that in the
>>>> past.
>>>
>>> Ah, yes, you added it to i8042. Well, maybe that is an argument for not
>>> automatically enabling wakeup on ACPI systems because that was the
>>> default behavior for them. OF/board platforms have different default
>>> behavior. I guess if we do not do anything besides what the patch is
>>> doing, then ACPI will not have the property I propose, so they will be
>>> capable but not enabled, and OF/boards will have the property and will
>>> be capable and enabled.
>>
>> The defaults to use should generally depend on two things IMO: On what
>> the firmware tells us to use (given the argument that the firmware
>> people probably have a good reason to tell us what they are telling
>> us) and on what we were doing for this class of devices in the past.
>> Ignoring the last bit generally leads to regressions in the cases when
>> the firmware people are wrong after all, so I'm always cautious about
>> this.  And this rule applies to DT as well as to ACPI, although the DT
>> people usually won't admit it. :-)
>
> OK, I realize that the example I gave was not really a good one,
> because the wakeup part was added to i8042 to support wakeup from
> suspend-to-idle which is really special and the implementation is
> based on what the driver was doing previously.

Which is not to say that there are no good (or at least better) examples.

USB HID devices pretty much universally support remote wakeup
officially, but enough of them cause problems to happen while using it
(spurious wakeups etc) that we don't enable them for system wakeup by
default.  Plus some of them are cordless mice and such and you really
need to be careful to avoid waking up a sleeping system by accident
with them (when enabled).

Ethernet NICs support wakeup signaling, but at least some of them are
not enabled for system wakeup by default.  Moreover, there are
multiple ways to trigger the wakeup there that the user needs to
choose from in the first place.

Some wireless adapters support WoW, but enabling them to wake up the
system from sleep by default would be asking for troubles.

All in all, combining the information that the device can signal
wakeup with the requirement to enable it to wake up the system from
sleep states by default doesn't look like a good idea to me.

> But this also makes me think that the problem is really more
> complicated than it may seem to be, so what about starting over and
> looking at the wakeup problem in general?
>
> To that end, there are two categories of wakeup support in devices.
> The first one is support for something called "remote wakeup" in USB
> and which means that the device is able to generate wakeup signals
> when it (the device) is suspended and the rest of the system can
> generally be considered as working.  I'll use the "remote wakeup" term
> in general for the lack of a better one.
>
> Remote wakeup support is used in runtime PM and suspend-to-idle (which
> essentially uses the same kind of hardware mechanics, but in a
> different way).
>
> Note, though, that "device is suspended" need not map 1:1 onto a
> particular hardware state.  What it really means is that either it has
> been suspended using the runtime PM framework, or all devices have
> gone through the device suspend sequence during suspend-to-idle.  The
> hardware state the device ends up in depends on the driver/bus type/PM
> domain combination and is generally expected to be low-power.
>
> It is easy in PCI or USB and on ACPI systems where low-power states of
> devices are well defined and the connection between them and the
> ability to generate wakeup signals is known.  It is not so easy in the
> other cases, though.  My personal opinion is that if wakeup support is
> required, the device should be put into the lowest-power state from
> which it can generate wakeup signals.  That very well may be the
> full-power state of it if that's the only suitable one.
>
> If that point of view is adopted, then any device that (a) can take
> input and (b) uses interrupts can do remote wakeup.  We don't really
> have a good way to express that in the driver core.
>
> The second category of wakeup support is for platform-assisted
> low-power states of the whole system where there's a big switch (or a
> bunch of them) allowing multiple things to be powered off in one go
> from the OS perspective.
>
> I'll write about that in the next message, as I need to take care of
> some urgent stuff now.

Continuing.

If the device in question belongs to the part of the system powered
off by the big switch, it has to be provided with some special
"wakeup" power source so it can generate signals.  There needs to be a
physical line (or bus or similar) that will be activated when wakeup
is signaled by the device.  The activation of it needs to be noticed
by something and turned into a signal that eventually will wake up the
CPU (or make it start executing a power-up sequence or equivalent).
All of that generally requires specific setup.

The device has to be left in a state in which it can use the "wakeup"
power source in the first place.  The "wakeup" power source for it has
to be turned on.  The signal "line" needs to be prepared for
activation by the device's wakeup signals.  The part of the system
responsible for waking up the CPU when that "line" is active has to be
prepared too.  All this means that generally it is not enough to say
"things are wired up properly" for the right stuff to happen.

Traditionally, we set the can_wakeup bit if (a) there is an interface
we can use to set up those things (either by accessing some registers
available to us or by invoking the platform firmware to do it) and (b)
the device is hooked up to that interface in a known way.  IOW, that
bit is inferred from what we can find out about the device's
configuration rather that just taken from the firmware for granted.

Now, this generally may not be the right approach.

Maybe we just don't need the can_wakeup bit at all.  Maybe we should
just create the "wakeup" sysfs attribute for all devices, let user
space set it the way it wants and handle it on the best effort basis.
That is, it will wake up from the sleep states it can wake up from,
but it may not wake up from any of them if there's no support.
Granted, we really can't guarantee that it will work anyway (even if
the firmware exposes the interface to us, it may not be correctly
implemented etc and there's no way to verify that upfront).  And we
may allow the same attribute to be used for disabling remote wakeup
for runtime PM if someone wants to.

In that case, though, we really won't need the firmware to tell us
whether or not the device is "wakeup-capable".  We will always treat
it this way and if it turns out that something is missing, wakeup will
just not work.

Thanks,
Rafael

Powered by blists - more mailing lists