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 <[email protected]>
---
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
On Tue, Dec 1, 2020 at 1:30 PM Furquan Shaikh <[email protected]> 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 <[email protected]>
> ---
> 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
On Mon, Dec 14, 2020 at 08:56:48PM -0800, Furquan Shaikh wrote:
> On Tue, Dec 1, 2020 at 1:30 PM Furquan Shaikh <[email protected]> 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 <[email protected]>
> > ---
> > 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
On Tue, Dec 1, 2020 at 10:30 PM Furquan Shaikh <[email protected]> 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 <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
> 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
>
On Fri, Jan 8, 2021 at 4:43 PM Greg Kroah-Hartman
<[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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!
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 <[email protected]>
> ---
> 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.
On Tue, Jan 12, 2021 at 10:55 AM Dmitry Osipenko <[email protected]> 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 <[email protected]>
> > ---
> > 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.
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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
On Tue, Jan 12, 2021 at 5:09 AM Greg Kroah-Hartman
<[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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
13.01.2021 04:22, Furquan Shaikh пишет:
> On Tue, Jan 12, 2021 at 5:09 AM Greg Kroah-Hartman
> <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>> 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!
Hi Furquan,
On Wed, Jan 13, 2021 at 10:31 PM Dmitry Osipenko <[email protected]> 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
On Wed, Feb 3, 2021 at 6:37 PM Kai-Heng Feng
<[email protected]> wrote:
>
> Hi Furquan,
>
> On Wed, Jan 13, 2021 at 10:31 PM Dmitry Osipenko <[email protected]> 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
On Thu, Feb 4, 2021 at 12:09 PM Furquan Shaikh <[email protected]> wrote:
>
> On Wed, Feb 3, 2021 at 6:37 PM Kai-Heng Feng
> <[email protected]> wrote:
> >
> > Hi Furquan,
> >
> > On Wed, Jan 13, 2021 at 10:31 PM Dmitry Osipenko <[email protected]> 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