From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755986AbcATXCE (ORCPT ); Wed, 20 Jan 2016 18:02:04 -0500 Received: from mail-lb0-f175.google.com ([209.85.217.175]:34995 "EHLO mail-lb0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754740AbcATXCB (ORCPT ); Wed, 20 Jan 2016 18:02:01 -0500 MIME-Version: 1.0 In-Reply-To: References: <20160118021138.GA20498@dtor-ws> <2182115.GafOXid1Dx@vostro.rjw.lan> <20160120004546.GD31589@dtor-ws> Date: Thu, 21 Jan 2016 00:01:59 +0100 X-Google-Sender-Auth: RDRur7wd77bDIvomV_f1K_ctxHU Message-ID: Subject: Re: [PATCH] driver-core: platform: automatically mark wakeup devices From: "Rafael J. Wysocki" To: "Rafael J. Wysocki" Cc: Dmitry Torokhov , "Rafael J. Wysocki" , Greg Kroah-Hartman , Rob Herring , Grant Likely , Linus Walleij , Thierry Reding , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , lkml Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jan 20, 2016 at 2:51 PM, Rafael J. Wysocki wrote: > On Wed, Jan 20, 2016 at 3:40 AM, Rafael J. Wysocki wrote: >> On Wed, Jan 20, 2016 at 1:45 AM, Dmitry Torokhov >> 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