diff mbox

platform/x86: intel-vbtn: Reset wakeup capable flag on removal

Message ID 1906705.kOn0fLT5aH@aspire.rjw.lan (mailing list archive)
State Accepted, archived
Delegated to: Andy Shevchenko
Headers show

Commit Message

Rafael J. Wysocki Feb. 28, 2018, 11:09 a.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The intel-vbtn device will not be able to wake up the system any more
after removing the notify handler provided by its driver, so make
its sysfs attributes reflect that.

Fixes: 91f9e850d465 (platform: x86: intel-vbtn: Wake up the system from suspend-to-idle)
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Resend with the correct platform-driver-x86 list address.

---
 drivers/platform/x86/intel-vbtn.c |    1 +
 1 file changed, 1 insertion(+)

Comments

Dmitry Torokhov May 15, 2018, 10:18 p.m. UTC | #1
On Wed, Feb 28, 2018 at 3:09 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The intel-vbtn device will not be able to wake up the system any more
> after removing the notify handler provided by its driver, so make
> its sysfs attributes reflect that.

Are there devices that can and should wake up system even after driver
is unbound from device? Should we move resetting wakeup flag into the
driver core so we do not have to do it in each individual driver?

>
> Fixes: 91f9e850d465 (platform: x86: intel-vbtn: Wake up the system from suspend-to-idle)
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> Resend with the correct platform-driver-x86 list address.
>
> ---
>  drivers/platform/x86/intel-vbtn.c |    1 +
>  1 file changed, 1 insertion(+)
>
> Index: linux-pm/drivers/platform/x86/intel-vbtn.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/x86/intel-vbtn.c
> +++ linux-pm/drivers/platform/x86/intel-vbtn.c
> @@ -154,6 +154,7 @@ static int intel_vbtn_remove(struct plat
>  {
>         acpi_handle handle = ACPI_HANDLE(&device->dev);
>
> +       device_init_wakeup(&device->dev, false);
>         acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, notify_handler);
>
>         /*
>
>

Thanks.
Rafael J. Wysocki May 16, 2018, 8:29 a.m. UTC | #2
On Wed, May 16, 2018 at 12:18 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Feb 28, 2018 at 3:09 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> The intel-vbtn device will not be able to wake up the system any more
>> after removing the notify handler provided by its driver, so make
>> its sysfs attributes reflect that.
>
> Are there devices that can and should wake up system even after driver
> is unbound from device?

Maybe, but then something needs to configure them for wakeup on suspend, right?

> Should we move resetting wakeup flag into the
> driver core so we do not have to do it in each individual driver?

It could be done in the driver core or bus type.  The bus type level
is a better place probably, because of PCI.

Anyway, the idea of this patch was to clear the bit in the same entity
that set it. :-)
Dmitry Torokhov May 16, 2018, 5:08 p.m. UTC | #3
On Wed, May 16, 2018 at 10:29:38AM +0200, Rafael J. Wysocki wrote:
> On Wed, May 16, 2018 at 12:18 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Feb 28, 2018 at 3:09 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >>
> >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>
> >> The intel-vbtn device will not be able to wake up the system any more
> >> after removing the notify handler provided by its driver, so make
> >> its sysfs attributes reflect that.
> >
> > Are there devices that can and should wake up system even after driver
> > is unbound from device?
> 
> Maybe, but then something needs to configure them for wakeup on suspend, right?

Yes. I don't know of any such instances though, that's why I asked.

> 
> > Should we move resetting wakeup flag into the
> > driver core so we do not have to do it in each individual driver?
> 
> It could be done in the driver core or bus type.  The bus type level
> is a better place probably, because of PCI.
> 
> Anyway, the idea of this patch was to clear the bit in the same entity
> that set it. :-)

Yeah, I understand. I was just thinking that with devm* API we are able
to remove most if not all remove() code in drivers, and similarly to
resetting driver data pointers, it would be great if we moved resetting
of wakeup flags into either bus implementation or device core itself.

Thanks.
diff mbox

Patch

Index: linux-pm/drivers/platform/x86/intel-vbtn.c
===================================================================
--- linux-pm.orig/drivers/platform/x86/intel-vbtn.c
+++ linux-pm/drivers/platform/x86/intel-vbtn.c
@@ -154,6 +154,7 @@  static int intel_vbtn_remove(struct plat
 {
 	acpi_handle handle = ACPI_HANDLE(&device->dev);
 
+	device_init_wakeup(&device->dev, false);
 	acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, notify_handler);
 
 	/*