From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755848AbcARQWL (ORCPT ); Mon, 18 Jan 2016 11:22:11 -0500 Received: from mail-ob0-f177.google.com ([209.85.214.177]:32816 "EHLO mail-ob0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755796AbcARQWI (ORCPT ); Mon, 18 Jan 2016 11:22:08 -0500 MIME-Version: 1.0 In-Reply-To: <2182115.GafOXid1Dx@vostro.rjw.lan> References: <20160118021138.GA20498@dtor-ws> <2182115.GafOXid1Dx@vostro.rjw.lan> Date: Mon, 18 Jan 2016 08:22:08 -0800 Message-ID: Subject: Re: [PATCH] driver-core: platform: automatically mark wakeup devices From: Dmitry Torokhov To: "Rafael J. Wysocki" Cc: Greg Kroah-Hartman , Rob Herring , Grant Likely , Linus Walleij , Thierry Reding , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , lkml , "Rafael J. Wysocki" 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 Mon, Jan 18, 2016 at 6:47 AM, Rafael J. Wysocki wrote: > On Sunday, January 17, 2016 06:11:38 PM Dmitry Torokhov wrote: >> When probing platform drivers let's check if corresponding devices have >> "wakeup-source" property defined (either in device tree, ACPI, or static >> platform properties) and automatically enable such devices as wakeup >> sources for the system. This will help us standardize on the name for this >> property and reduce amount of boilerplate code in the drivers. > > ACPI has other ways of telling the OS that the device is wakeup-capable, > but I guess the property in question can be used too (as long as it is > consistent with the other methods). I was thinking that down the road ACPI can wire its internal wakeup knowledge into generic device property. Unfortunately at the moment most drivers go like this: - if it is device tree platform: good, we'll take the data from device tree property and set up driver behavior accordingly; - if it is legacy board setup: OK, take for driver-specific platform data; - ACPI... hmm... dunno... enable wakeup and if it is not correct the system will just not wake up, not the best but not terribly wrong either. > >> Signed-off-by: Dmitry Torokhov >> --- >> drivers/base/platform.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/base/platform.c b/drivers/base/platform.c >> index 1dd6d3b..d14071a 100644 >> --- a/drivers/base/platform.c >> +++ b/drivers/base/platform.c >> @@ -514,9 +514,14 @@ static int platform_drv_probe(struct device *_dev) >> >> ret = dev_pm_domain_attach(_dev, true); >> if (ret != -EPROBE_DEFER && drv->probe) { >> + bool wakeup = device_property_read_bool(_dev, "wakeup-source"); >> + >> + device_init_wakeup(_dev, wakeup); > > But I'm wondering if this should be device_set_wakeup_capable(dev, true) rather? > > device_init_wakeup() additionally sets the default in sysfs to "do wakeup" > which in principle may unblock spurious wakeups on some systems. Why would we not want enable wakeup for devices where system (platform) tells us that they are wakeup sources? This is how most of the drivers I am involved in behave. All of them use device_init_wakeup() and then we adjust the behavior depending on runtime state, i.e. do not wake up from touchpad when lid is closed (which is useful if lid is flexible and may interact with touchpad if slammed down hard enough). > >> ret = drv->probe(dev); >> - if (ret) >> + if (ret) { >> + device_init_wakeup(_dev, false); >> dev_pm_domain_detach(_dev, true); >> + } >> } >> >> if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) { >> @@ -540,6 +545,8 @@ static int platform_drv_remove(struct device *_dev) >> >> if (drv->remove) >> ret = drv->remove(dev); >> + >> + device_init_wakeup(_dev, false); >> dev_pm_domain_detach(_dev, true); >> >> return ret; >> > Thanks. -- Dmitry