All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drivers: core: Detach device from power domain on shutdown
@ 2020-12-01 21:30 Furquan Shaikh
  2020-12-15  4:56 ` Furquan Shaikh
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Furquan Shaikh @ 2020-12-01 21:30 UTC (permalink / raw)
  To: Rafael J . Wysocki
  Cc: Greg Kroah-Hartman, linux-kernel, linux-acpi, Furquan Shaikh

When the system is powered off or rebooted, devices are not detached
from their PM domain. This results in ACPI PM not being invoked and
hence PowerResouce _OFF method not being invoked for any of the
devices. Because the ACPI power resources are not turned off in case
of poweroff and reboot, it violates the power sequencing requirements
which impacts the reliability of the devices over the lifetime of the
platform. This is currently observed on all Chromebooks using ACPI.

In order to solve the above problem, this change detaches a device
from its PM domain whenever it is shutdown. This action is basically
analogous to ->remove() from driver model perspective. Detaching the
device from its PM domain ensures that the ACPI PM gets a chance to
turn off the power resources for the device thus complying with its
power sequencing requirements.

Signed-off-by: Furquan Shaikh <furquan@google.com>
---
 drivers/base/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index d661ada1518f..5823f1d719e1 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -23,6 +23,7 @@
 #include <linux/of_device.h>
 #include <linux/genhd.h>
 #include <linux/mutex.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/netdevice.h>
 #include <linux/sched/signal.h>
@@ -4057,6 +4058,8 @@ void device_shutdown(void)
 			dev->driver->shutdown(dev);
 		}
 
+		dev_pm_domain_detach(dev, true);
+
 		device_unlock(dev);
 		if (parent)
 			device_unlock(parent);
-- 
2.29.2.454.gaff20da3a2-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2020-12-01 21:30 [PATCH] drivers: core: Detach device from power domain on shutdown Furquan Shaikh
@ 2020-12-15  4:56 ` Furquan Shaikh
  2021-01-08 15:44   ` Greg Kroah-Hartman
  2021-01-08 15:49 ` Rafael J. Wysocki
  2021-01-12  9:55 ` Dmitry Osipenko
  2 siblings, 1 reply; 13+ messages in thread
From: Furquan Shaikh @ 2020-12-15  4:56 UTC (permalink / raw)
  To: Rafael J . Wysocki
  Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, ACPI Devel Maling List

On Tue, Dec 1, 2020 at 1:30 PM Furquan Shaikh <furquan@google.com> wrote:
>
> When the system is powered off or rebooted, devices are not detached
> from their PM domain. This results in ACPI PM not being invoked and
> hence PowerResouce _OFF method not being invoked for any of the
> devices. Because the ACPI power resources are not turned off in case
> of poweroff and reboot, it violates the power sequencing requirements
> which impacts the reliability of the devices over the lifetime of the
> platform. This is currently observed on all Chromebooks using ACPI.
>
> In order to solve the above problem, this change detaches a device
> from its PM domain whenever it is shutdown. This action is basically
> analogous to ->remove() from driver model perspective. Detaching the
> device from its PM domain ensures that the ACPI PM gets a chance to
> turn off the power resources for the device thus complying with its
> power sequencing requirements.
>
> Signed-off-by: Furquan Shaikh <furquan@google.com>
> ---
>  drivers/base/core.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index d661ada1518f..5823f1d719e1 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -23,6 +23,7 @@
>  #include <linux/of_device.h>
>  #include <linux/genhd.h>
>  #include <linux/mutex.h>
> +#include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/netdevice.h>
>  #include <linux/sched/signal.h>
> @@ -4057,6 +4058,8 @@ void device_shutdown(void)
>                         dev->driver->shutdown(dev);
>                 }
>
> +               dev_pm_domain_detach(dev, true);
> +
>                 device_unlock(dev);
>                 if (parent)
>                         device_unlock(parent);
> --
> 2.29.2.454.gaff20da3a2-goog
>

Hello,

Gentle ping. Just checking if there are any comments.

Thanks,
Furquan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2020-12-15  4:56 ` Furquan Shaikh
@ 2021-01-08 15:44   ` Greg Kroah-Hartman
  2021-01-08 15:49     ` Rafael J. Wysocki
  0 siblings, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-08 15:44 UTC (permalink / raw)
  To: Furquan Shaikh
  Cc: Rafael J . Wysocki, Linux Kernel Mailing List, ACPI Devel Maling List

On Mon, Dec 14, 2020 at 08:56:48PM -0800, Furquan Shaikh wrote:
> On Tue, Dec 1, 2020 at 1:30 PM Furquan Shaikh <furquan@google.com> wrote:
> >
> > When the system is powered off or rebooted, devices are not detached
> > from their PM domain. This results in ACPI PM not being invoked and
> > hence PowerResouce _OFF method not being invoked for any of the
> > devices. Because the ACPI power resources are not turned off in case
> > of poweroff and reboot, it violates the power sequencing requirements
> > which impacts the reliability of the devices over the lifetime of the
> > platform. This is currently observed on all Chromebooks using ACPI.
> >
> > In order to solve the above problem, this change detaches a device
> > from its PM domain whenever it is shutdown. This action is basically
> > analogous to ->remove() from driver model perspective. Detaching the
> > device from its PM domain ensures that the ACPI PM gets a chance to
> > turn off the power resources for the device thus complying with its
> > power sequencing requirements.
> >
> > Signed-off-by: Furquan Shaikh <furquan@google.com>
> > ---
> >  drivers/base/core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index d661ada1518f..5823f1d719e1 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/of_device.h>
> >  #include <linux/genhd.h>
> >  #include <linux/mutex.h>
> > +#include <linux/pm_domain.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/netdevice.h>
> >  #include <linux/sched/signal.h>
> > @@ -4057,6 +4058,8 @@ void device_shutdown(void)
> >                         dev->driver->shutdown(dev);
> >                 }
> >
> > +               dev_pm_domain_detach(dev, true);
> > +
> >                 device_unlock(dev);
> >                 if (parent)
> >                         device_unlock(parent);
> > --
> > 2.29.2.454.gaff20da3a2-goog
> >
> 
> Hello,
> 
> Gentle ping. Just checking if there are any comments.

I'll wait for Rafael to ack this before taking it...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2020-12-01 21:30 [PATCH] drivers: core: Detach device from power domain on shutdown Furquan Shaikh
  2020-12-15  4:56 ` Furquan Shaikh
@ 2021-01-08 15:49 ` Rafael J. Wysocki
  2021-01-12  9:55 ` Dmitry Osipenko
  2 siblings, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2021-01-08 15:49 UTC (permalink / raw)
  To: Furquan Shaikh
  Cc: Rafael J . Wysocki, Greg Kroah-Hartman,
	Linux Kernel Mailing List, ACPI Devel Maling List

On Tue, Dec 1, 2020 at 10:30 PM Furquan Shaikh <furquan@google.com> wrote:
>
> When the system is powered off or rebooted, devices are not detached
> from their PM domain. This results in ACPI PM not being invoked and
> hence PowerResouce _OFF method not being invoked for any of the
> devices. Because the ACPI power resources are not turned off in case
> of poweroff and reboot, it violates the power sequencing requirements
> which impacts the reliability of the devices over the lifetime of the
> platform. This is currently observed on all Chromebooks using ACPI.
>
> In order to solve the above problem, this change detaches a device
> from its PM domain whenever it is shutdown. This action is basically
> analogous to ->remove() from driver model perspective. Detaching the
> device from its PM domain ensures that the ACPI PM gets a chance to
> turn off the power resources for the device thus complying with its
> power sequencing requirements.
>
> Signed-off-by: Furquan Shaikh <furquan@google.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/base/core.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index d661ada1518f..5823f1d719e1 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -23,6 +23,7 @@
>  #include <linux/of_device.h>
>  #include <linux/genhd.h>
>  #include <linux/mutex.h>
> +#include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/netdevice.h>
>  #include <linux/sched/signal.h>
> @@ -4057,6 +4058,8 @@ void device_shutdown(void)
>                         dev->driver->shutdown(dev);
>                 }
>
> +               dev_pm_domain_detach(dev, true);
> +
>                 device_unlock(dev);
>                 if (parent)
>                         device_unlock(parent);
> --
> 2.29.2.454.gaff20da3a2-goog
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2021-01-08 15:44   ` Greg Kroah-Hartman
@ 2021-01-08 15:49     ` Rafael J. Wysocki
  0 siblings, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2021-01-08 15:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Furquan Shaikh, Rafael J . Wysocki, Linux Kernel Mailing List,
	ACPI Devel Maling List

On Fri, Jan 8, 2021 at 4:43 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Dec 14, 2020 at 08:56:48PM -0800, Furquan Shaikh wrote:
> > On Tue, Dec 1, 2020 at 1:30 PM Furquan Shaikh <furquan@google.com> wrote:
> > >
> > > When the system is powered off or rebooted, devices are not detached
> > > from their PM domain. This results in ACPI PM not being invoked and
> > > hence PowerResouce _OFF method not being invoked for any of the
> > > devices. Because the ACPI power resources are not turned off in case
> > > of poweroff and reboot, it violates the power sequencing requirements
> > > which impacts the reliability of the devices over the lifetime of the
> > > platform. This is currently observed on all Chromebooks using ACPI.
> > >
> > > In order to solve the above problem, this change detaches a device
> > > from its PM domain whenever it is shutdown. This action is basically
> > > analogous to ->remove() from driver model perspective. Detaching the
> > > device from its PM domain ensures that the ACPI PM gets a chance to
> > > turn off the power resources for the device thus complying with its
> > > power sequencing requirements.
> > >
> > > Signed-off-by: Furquan Shaikh <furquan@google.com>
> > > ---
> > >  drivers/base/core.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > index d661ada1518f..5823f1d719e1 100644
> > > --- a/drivers/base/core.c
> > > +++ b/drivers/base/core.c
> > > @@ -23,6 +23,7 @@
> > >  #include <linux/of_device.h>
> > >  #include <linux/genhd.h>
> > >  #include <linux/mutex.h>
> > > +#include <linux/pm_domain.h>
> > >  #include <linux/pm_runtime.h>
> > >  #include <linux/netdevice.h>
> > >  #include <linux/sched/signal.h>
> > > @@ -4057,6 +4058,8 @@ void device_shutdown(void)
> > >                         dev->driver->shutdown(dev);
> > >                 }
> > >
> > > +               dev_pm_domain_detach(dev, true);
> > > +
> > >                 device_unlock(dev);
> > >                 if (parent)
> > >                         device_unlock(parent);
> > > --
> > > 2.29.2.454.gaff20da3a2-goog
> > >
> >
> > Hello,
> >
> > Gentle ping. Just checking if there are any comments.
>
> I'll wait for Rafael to ack this before taking it...

Done.

Cheers!

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2020-12-01 21:30 [PATCH] drivers: core: Detach device from power domain on shutdown Furquan Shaikh
  2020-12-15  4:56 ` Furquan Shaikh
  2021-01-08 15:49 ` Rafael J. Wysocki
@ 2021-01-12  9:55 ` Dmitry Osipenko
  2021-01-12 12:45   ` Rafael J. Wysocki
  2 siblings, 1 reply; 13+ messages in thread
From: Dmitry Osipenko @ 2021-01-12  9:55 UTC (permalink / raw)
  To: Furquan Shaikh, Rafael J . Wysocki, Thierry Reding
  Cc: Greg Kroah-Hartman, linux-kernel, linux-acpi, linux-tegra

02.12.2020 00:30, Furquan Shaikh пишет:
> When the system is powered off or rebooted, devices are not detached
> from their PM domain. This results in ACPI PM not being invoked and
> hence PowerResouce _OFF method not being invoked for any of the
> devices. Because the ACPI power resources are not turned off in case
> of poweroff and reboot, it violates the power sequencing requirements
> which impacts the reliability of the devices over the lifetime of the
> platform. This is currently observed on all Chromebooks using ACPI.
> 
> In order to solve the above problem, this change detaches a device
> from its PM domain whenever it is shutdown. This action is basically
> analogous to ->remove() from driver model perspective. Detaching the
> device from its PM domain ensures that the ACPI PM gets a chance to
> turn off the power resources for the device thus complying with its
> power sequencing requirements.
> 
> Signed-off-by: Furquan Shaikh <furquan@google.com>
> ---
>  drivers/base/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index d661ada1518f..5823f1d719e1 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -23,6 +23,7 @@
>  #include <linux/of_device.h>
>  #include <linux/genhd.h>
>  #include <linux/mutex.h>
> +#include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/netdevice.h>
>  #include <linux/sched/signal.h>
> @@ -4057,6 +4058,8 @@ void device_shutdown(void)
>  			dev->driver->shutdown(dev);
>  		}
>  
> +		dev_pm_domain_detach(dev, true);
> +
>  		device_unlock(dev);
>  		if (parent)
>  			device_unlock(parent);
> 

This patch broke system shutdown on NVIDIA Tegra using today's
linux-next because power domain can't be turned off until device drivers
handed control over device resets to the power domain of Power
Management controller on Tegra. This patch introduced the wrong
behaviour, apparently it should be made specific to ACPI only.

Please fix, thanks in advance.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2021-01-12  9:55 ` Dmitry Osipenko
@ 2021-01-12 12:45   ` Rafael J. Wysocki
  2021-01-12 13:10     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2021-01-12 12:45 UTC (permalink / raw)
  To: Dmitry Osipenko, Greg Kroah-Hartman
  Cc: Furquan Shaikh, Rafael J . Wysocki, Thierry Reding,
	Linux Kernel Mailing List, ACPI Devel Maling List, linux-tegra

On Tue, Jan 12, 2021 at 10:55 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 02.12.2020 00:30, Furquan Shaikh пишет:
> > When the system is powered off or rebooted, devices are not detached
> > from their PM domain. This results in ACPI PM not being invoked and
> > hence PowerResouce _OFF method not being invoked for any of the
> > devices. Because the ACPI power resources are not turned off in case
> > of poweroff and reboot, it violates the power sequencing requirements
> > which impacts the reliability of the devices over the lifetime of the
> > platform. This is currently observed on all Chromebooks using ACPI.
> >
> > In order to solve the above problem, this change detaches a device
> > from its PM domain whenever it is shutdown. This action is basically
> > analogous to ->remove() from driver model perspective. Detaching the
> > device from its PM domain ensures that the ACPI PM gets a chance to
> > turn off the power resources for the device thus complying with its
> > power sequencing requirements.
> >
> > Signed-off-by: Furquan Shaikh <furquan@google.com>
> > ---
> >  drivers/base/core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index d661ada1518f..5823f1d719e1 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/of_device.h>
> >  #include <linux/genhd.h>
> >  #include <linux/mutex.h>
> > +#include <linux/pm_domain.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/netdevice.h>
> >  #include <linux/sched/signal.h>
> > @@ -4057,6 +4058,8 @@ void device_shutdown(void)
> >                       dev->driver->shutdown(dev);
> >               }
> >
> > +             dev_pm_domain_detach(dev, true);
> > +
> >               device_unlock(dev);
> >               if (parent)
> >                       device_unlock(parent);
> >
>
> This patch broke system shutdown on NVIDIA Tegra using today's
> linux-next because power domain can't be turned off until device drivers
> handed control over device resets to the power domain of Power
> Management controller on Tegra. This patch introduced the wrong
> behaviour, apparently it should be made specific to ACPI only.
>
> Please fix, thanks in advance.

OK, so Greg please drop it.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2021-01-12 12:45   ` Rafael J. Wysocki
@ 2021-01-12 13:10     ` Greg Kroah-Hartman
  2021-01-13  1:22       ` Furquan Shaikh
  0 siblings, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-12 13:10 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Dmitry Osipenko, Furquan Shaikh, Thierry Reding,
	Linux Kernel Mailing List, ACPI Devel Maling List, linux-tegra

On Tue, Jan 12, 2021 at 01:45:25PM +0100, Rafael J. Wysocki wrote:
> On Tue, Jan 12, 2021 at 10:55 AM Dmitry Osipenko <digetx@gmail.com> wrote:
> >
> > 02.12.2020 00:30, Furquan Shaikh пишет:
> > > When the system is powered off or rebooted, devices are not detached
> > > from their PM domain. This results in ACPI PM not being invoked and
> > > hence PowerResouce _OFF method not being invoked for any of the
> > > devices. Because the ACPI power resources are not turned off in case
> > > of poweroff and reboot, it violates the power sequencing requirements
> > > which impacts the reliability of the devices over the lifetime of the
> > > platform. This is currently observed on all Chromebooks using ACPI.
> > >
> > > In order to solve the above problem, this change detaches a device
> > > from its PM domain whenever it is shutdown. This action is basically
> > > analogous to ->remove() from driver model perspective. Detaching the
> > > device from its PM domain ensures that the ACPI PM gets a chance to
> > > turn off the power resources for the device thus complying with its
> > > power sequencing requirements.
> > >
> > > Signed-off-by: Furquan Shaikh <furquan@google.com>
> > > ---
> > >  drivers/base/core.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > index d661ada1518f..5823f1d719e1 100644
> > > --- a/drivers/base/core.c
> > > +++ b/drivers/base/core.c
> > > @@ -23,6 +23,7 @@
> > >  #include <linux/of_device.h>
> > >  #include <linux/genhd.h>
> > >  #include <linux/mutex.h>
> > > +#include <linux/pm_domain.h>
> > >  #include <linux/pm_runtime.h>
> > >  #include <linux/netdevice.h>
> > >  #include <linux/sched/signal.h>
> > > @@ -4057,6 +4058,8 @@ void device_shutdown(void)
> > >                       dev->driver->shutdown(dev);
> > >               }
> > >
> > > +             dev_pm_domain_detach(dev, true);
> > > +
> > >               device_unlock(dev);
> > >               if (parent)
> > >                       device_unlock(parent);
> > >
> >
> > This patch broke system shutdown on NVIDIA Tegra using today's
> > linux-next because power domain can't be turned off until device drivers
> > handed control over device resets to the power domain of Power
> > Management controller on Tegra. This patch introduced the wrong
> > behaviour, apparently it should be made specific to ACPI only.
> >
> > Please fix, thanks in advance.
> 
> OK, so Greg please drop it.

Now reverted, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2021-01-12 13:10     ` Greg Kroah-Hartman
@ 2021-01-13  1:22       ` Furquan Shaikh
  2021-01-13 14:30         ` Dmitry Osipenko
  0 siblings, 1 reply; 13+ messages in thread
From: Furquan Shaikh @ 2021-01-13  1:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Dmitry Osipenko, Thierry Reding,
	Linux Kernel Mailing List, ACPI Devel Maling List, linux-tegra

On Tue, Jan 12, 2021 at 5:09 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jan 12, 2021 at 01:45:25PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 12, 2021 at 10:55 AM Dmitry Osipenko <digetx@gmail.com> wrote:
> > >
> > > 02.12.2020 00:30, Furquan Shaikh пишет:
> > > > When the system is powered off or rebooted, devices are not detached
> > > > from their PM domain. This results in ACPI PM not being invoked and
> > > > hence PowerResouce _OFF method not being invoked for any of the
> > > > devices. Because the ACPI power resources are not turned off in case
> > > > of poweroff and reboot, it violates the power sequencing requirements
> > > > which impacts the reliability of the devices over the lifetime of the
> > > > platform. This is currently observed on all Chromebooks using ACPI.
> > > >
> > > > In order to solve the above problem, this change detaches a device
> > > > from its PM domain whenever it is shutdown. This action is basically
> > > > analogous to ->remove() from driver model perspective. Detaching the
> > > > device from its PM domain ensures that the ACPI PM gets a chance to
> > > > turn off the power resources for the device thus complying with its
> > > > power sequencing requirements.
> > > >
> > > > Signed-off-by: Furquan Shaikh <furquan@google.com>
> > > > ---
> > > >  drivers/base/core.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > > index d661ada1518f..5823f1d719e1 100644
> > > > --- a/drivers/base/core.c
> > > > +++ b/drivers/base/core.c
> > > > @@ -23,6 +23,7 @@
> > > >  #include <linux/of_device.h>
> > > >  #include <linux/genhd.h>
> > > >  #include <linux/mutex.h>
> > > > +#include <linux/pm_domain.h>
> > > >  #include <linux/pm_runtime.h>
> > > >  #include <linux/netdevice.h>
> > > >  #include <linux/sched/signal.h>
> > > > @@ -4057,6 +4058,8 @@ void device_shutdown(void)
> > > >                       dev->driver->shutdown(dev);
> > > >               }
> > > >
> > > > +             dev_pm_domain_detach(dev, true);
> > > > +
> > > >               device_unlock(dev);
> > > >               if (parent)
> > > >                       device_unlock(parent);
> > > >
> > >
> > > This patch broke system shutdown on NVIDIA Tegra using today's
> > > linux-next because power domain can't be turned off until device drivers
> > > handed control over device resets to the power domain of Power
> > > Management controller on Tegra. This patch introduced the wrong
> > > behaviour, apparently it should be made specific to ACPI only.
> > >
> > > Please fix, thanks in advance.

Sorry about the breakage. I am working on an alternate solution that
Rafael suggested.

> >
> > OK, so Greg please drop it.
>
> Now reverted, thanks.

Thanks Greg!

>
> greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2021-01-13  1:22       ` Furquan Shaikh
@ 2021-01-13 14:30         ` Dmitry Osipenko
  2021-02-04  2:37           ` Kai-Heng Feng
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Osipenko @ 2021-01-13 14:30 UTC (permalink / raw)
  To: Furquan Shaikh, Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Thierry Reding, Linux Kernel Mailing List,
	ACPI Devel Maling List, linux-tegra

13.01.2021 04:22, Furquan Shaikh пишет:
> On Tue, Jan 12, 2021 at 5:09 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>>
>> On Tue, Jan 12, 2021 at 01:45:25PM +0100, Rafael J. Wysocki wrote:
>>> On Tue, Jan 12, 2021 at 10:55 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>>>>
>>>> 02.12.2020 00:30, Furquan Shaikh пишет:
>>>>> When the system is powered off or rebooted, devices are not detached
>>>>> from their PM domain. This results in ACPI PM not being invoked and
>>>>> hence PowerResouce _OFF method not being invoked for any of the
>>>>> devices. Because the ACPI power resources are not turned off in case
>>>>> of poweroff and reboot, it violates the power sequencing requirements
>>>>> which impacts the reliability of the devices over the lifetime of the
>>>>> platform. This is currently observed on all Chromebooks using ACPI.
>>>>>
>>>>> In order to solve the above problem, this change detaches a device
>>>>> from its PM domain whenever it is shutdown. This action is basically
>>>>> analogous to ->remove() from driver model perspective. Detaching the
>>>>> device from its PM domain ensures that the ACPI PM gets a chance to
>>>>> turn off the power resources for the device thus complying with its
>>>>> power sequencing requirements.
>>>>>
>>>>> Signed-off-by: Furquan Shaikh <furquan@google.com>
>>>>> ---
>>>>>  drivers/base/core.c | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>>>>> index d661ada1518f..5823f1d719e1 100644
>>>>> --- a/drivers/base/core.c
>>>>> +++ b/drivers/base/core.c
>>>>> @@ -23,6 +23,7 @@
>>>>>  #include <linux/of_device.h>
>>>>>  #include <linux/genhd.h>
>>>>>  #include <linux/mutex.h>
>>>>> +#include <linux/pm_domain.h>
>>>>>  #include <linux/pm_runtime.h>
>>>>>  #include <linux/netdevice.h>
>>>>>  #include <linux/sched/signal.h>
>>>>> @@ -4057,6 +4058,8 @@ void device_shutdown(void)
>>>>>                       dev->driver->shutdown(dev);
>>>>>               }
>>>>>
>>>>> +             dev_pm_domain_detach(dev, true);
>>>>> +
>>>>>               device_unlock(dev);
>>>>>               if (parent)
>>>>>                       device_unlock(parent);
>>>>>
>>>>
>>>> This patch broke system shutdown on NVIDIA Tegra using today's
>>>> linux-next because power domain can't be turned off until device drivers
>>>> handed control over device resets to the power domain of Power
>>>> Management controller on Tegra. This patch introduced the wrong
>>>> behaviour, apparently it should be made specific to ACPI only.
>>>>
>>>> Please fix, thanks in advance.
> 
> Sorry about the breakage. I am working on an alternate solution that
> Rafael suggested.
> 
>>>
>>> OK, so Greg please drop it.
>>
>> Now reverted, thanks.
> 
> Thanks Greg!

Thank you all for addressing this problem!


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2021-01-13 14:30         ` Dmitry Osipenko
@ 2021-02-04  2:37           ` Kai-Heng Feng
  2021-02-04  4:09             ` Furquan Shaikh
  0 siblings, 1 reply; 13+ messages in thread
From: Kai-Heng Feng @ 2021-02-04  2:37 UTC (permalink / raw)
  To: Furquan Shaikh
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Thierry Reding,
	Linux Kernel Mailing List, ACPI Devel Maling List, linux-tegra,
	Dmitry Osipenko

Hi Furquan,

On Wed, Jan 13, 2021 at 10:31 PM Dmitry Osipenko <digetx@gmail.com> wrote:
[snipped]
> Thank you all for addressing this problem!

Are you still working on the alternate solution? This patch can
address S5 power consumption issue for some laptops:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1912935

Kai-Heng

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2021-02-04  2:37           ` Kai-Heng Feng
@ 2021-02-04  4:09             ` Furquan Shaikh
  2021-02-04  5:19               ` Kai-Heng Feng
  0 siblings, 1 reply; 13+ messages in thread
From: Furquan Shaikh @ 2021-02-04  4:09 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Thierry Reding,
	Linux Kernel Mailing List, ACPI Devel Maling List, linux-tegra,
	Dmitry Osipenko

On Wed, Feb 3, 2021 at 6:37 PM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> Hi Furquan,
>
> On Wed, Jan 13, 2021 at 10:31 PM Dmitry Osipenko <digetx@gmail.com> wrote:
> [snipped]
> > Thank you all for addressing this problem!
>
> Are you still working on the alternate solution?

Yes, it is in my pipeline, but I have been distracted because of some
other high priority tasks. I plan to push something for review in ~3-4
weeks.

> This patch can
> address S5 power consumption issue for some laptops:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1912935
>
> Kai-Heng

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drivers: core: Detach device from power domain on shutdown
  2021-02-04  4:09             ` Furquan Shaikh
@ 2021-02-04  5:19               ` Kai-Heng Feng
  0 siblings, 0 replies; 13+ messages in thread
From: Kai-Heng Feng @ 2021-02-04  5:19 UTC (permalink / raw)
  To: Furquan Shaikh
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Thierry Reding,
	Linux Kernel Mailing List, ACPI Devel Maling List, linux-tegra,
	Dmitry Osipenko

On Thu, Feb 4, 2021 at 12:09 PM Furquan Shaikh <furquan@google.com> wrote:
>
> On Wed, Feb 3, 2021 at 6:37 PM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
> >
> > Hi Furquan,
> >
> > On Wed, Jan 13, 2021 at 10:31 PM Dmitry Osipenko <digetx@gmail.com> wrote:
> > [snipped]
> > > Thank you all for addressing this problem!
> >
> > Are you still working on the alternate solution?
>
> Yes, it is in my pipeline, but I have been distracted because of some
> other high priority tasks. I plan to push something for review in ~3-4
> weeks.

Please Cc me in the revised patch.
Thanks for your work.

Kai-Heng

>
> > This patch can
> > address S5 power consumption issue for some laptops:
> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1912935
> >
> > Kai-Heng

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-02-04  5:20 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-01 21:30 [PATCH] drivers: core: Detach device from power domain on shutdown Furquan Shaikh
2020-12-15  4:56 ` Furquan Shaikh
2021-01-08 15:44   ` Greg Kroah-Hartman
2021-01-08 15:49     ` Rafael J. Wysocki
2021-01-08 15:49 ` Rafael J. Wysocki
2021-01-12  9:55 ` Dmitry Osipenko
2021-01-12 12:45   ` Rafael J. Wysocki
2021-01-12 13:10     ` Greg Kroah-Hartman
2021-01-13  1:22       ` Furquan Shaikh
2021-01-13 14:30         ` Dmitry Osipenko
2021-02-04  2:37           ` Kai-Heng Feng
2021-02-04  4:09             ` Furquan Shaikh
2021-02-04  5:19               ` Kai-Heng Feng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.