linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally"
@ 2021-04-30 12:42 Shujun Wang
  2021-04-30 13:31 ` Rafael J. Wysocki
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Shujun Wang @ 2021-04-30 12:42 UTC (permalink / raw)
  To: rjw, lenb, linux-acpi, linux-kernel; +Cc: Shujun Wang

This reverts commit 7e4fdeafa61f2b653fcf9678f09935e55756aed2.
It may cause some NVMe device probes to fail, and the system may get
stuck when using an NVMe device as the root filesystem.

In the function nvme_pci_enable(struct nvme_dev *dev), as shown below,
readl(NVME_REG_CSTS) always returns -1 with the commit, which results in
the probe failed.

  if (readl(dev->bar + NVME_REG_CSTS) == -1) {
	result = -ENODEV;
	goto disable;
  }

dmesg:
  [    1.106280] nvme 0000:04:00.0: platform quirk: setting simple suspend
  [    1.109111] nvme nvme0: pci function 0000:04:00.0
  [    1.113066] nvme 0000:04:00.0: enabling device (0000 -> 0002)
  [    1.121040] nvme nvme0: Removing after probe failure status: -19

lspci:
  Non-Volatile memory controller: KIOXIA Corporation Device 0001

device uevent:
  DRIVER=nvme
  PCI_CLASS=10802
  PCI_ID=1E0F:0001
  PCI_SUBSYS_ID=1E0F:0001
  PCI_SLOT_NAME=0000:04:00.0
  MODALIAS=pci:v00001E0Fd00000001sv00001E0Fsd00000001bc01sc08i02

This patch was tested in Lenovo Thinkpad X1.

Signed-off-by: Shujun Wang <wsj20369@163.com>
---
 drivers/acpi/power.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index 56102eaaa2da..8bf10abeb2e0 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -1004,9 +1004,18 @@ void acpi_turn_off_unused_power_resources(void)
 	mutex_lock(&power_resource_list_lock);
 
 	list_for_each_entry_reverse(resource, &acpi_power_resource_list, list_node) {
+		int result, state;
+
 		mutex_lock(&resource->resource_lock);
 
-		if (!resource->ref_count) {
+		result = acpi_power_get_state(resource->device.handle, &state);
+		if (result) {
+			mutex_unlock(&resource->resource_lock);
+			continue;
+		}
+
+		if (state == ACPI_POWER_RESOURCE_STATE_ON
+		    && !resource->ref_count) {
 			dev_info(&resource->device.dev, "Turning OFF\n");
 			__acpi_power_off(resource);
 		}
-- 
2.25.1


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

* Re: [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally"
  2021-04-30 12:42 [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally" Shujun Wang
@ 2021-04-30 13:31 ` Rafael J. Wysocki
  2021-04-30 13:46   ` Rafael J. Wysocki
  2021-05-10  6:37 ` Zhang, Rui
  2021-05-14 14:05 ` Rafael J. Wysocki
  2 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2021-04-30 13:31 UTC (permalink / raw)
  To: Shujun Wang
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List, David Box

On Fri, Apr 30, 2021 at 2:43 PM Shujun Wang <wsj20369@163.com> wrote:
>
> This reverts commit 7e4fdeafa61f2b653fcf9678f09935e55756aed2.

OK, I'll revert that commit, thanks!

> It may cause some NVMe device probes to fail, and the system may get
> stuck when using an NVMe device as the root filesystem.
>
> In the function nvme_pci_enable(struct nvme_dev *dev), as shown below,
> readl(NVME_REG_CSTS) always returns -1 with the commit, which results in
> the probe failed.
>
>   if (readl(dev->bar + NVME_REG_CSTS) == -1) {
>         result = -ENODEV;
>         goto disable;
>   }
>
> dmesg:
>   [    1.106280] nvme 0000:04:00.0: platform quirk: setting simple suspend
>   [    1.109111] nvme nvme0: pci function 0000:04:00.0
>   [    1.113066] nvme 0000:04:00.0: enabling device (0000 -> 0002)
>   [    1.121040] nvme nvme0: Removing after probe failure status: -19
>
> lspci:
>   Non-Volatile memory controller: KIOXIA Corporation Device 0001
>
> device uevent:
>   DRIVER=nvme
>   PCI_CLASS=10802
>   PCI_ID=1E0F:0001
>   PCI_SUBSYS_ID=1E0F:0001
>   PCI_SLOT_NAME=0000:04:00.0
>   MODALIAS=pci:v00001E0Fd00000001sv00001E0Fsd00000001bc01sc08i02
>
> This patch was tested in Lenovo Thinkpad X1.
>
> Signed-off-by: Shujun Wang <wsj20369@163.com>
> ---
>  drivers/acpi/power.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
> index 56102eaaa2da..8bf10abeb2e0 100644
> --- a/drivers/acpi/power.c
> +++ b/drivers/acpi/power.c
> @@ -1004,9 +1004,18 @@ void acpi_turn_off_unused_power_resources(void)
>         mutex_lock(&power_resource_list_lock);
>
>         list_for_each_entry_reverse(resource, &acpi_power_resource_list, list_node) {
> +               int result, state;
> +
>                 mutex_lock(&resource->resource_lock);
>
> -               if (!resource->ref_count) {
> +               result = acpi_power_get_state(resource->device.handle, &state);
> +               if (result) {
> +                       mutex_unlock(&resource->resource_lock);
> +                       continue;
> +               }
> +
> +               if (state == ACPI_POWER_RESOURCE_STATE_ON
> +                   && !resource->ref_count) {
>                         dev_info(&resource->device.dev, "Turning OFF\n");
>                         __acpi_power_off(resource);
>                 }
> --
> 2.25.1
>

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

* Re: [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally"
  2021-04-30 13:31 ` Rafael J. Wysocki
@ 2021-04-30 13:46   ` Rafael J. Wysocki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2021-04-30 13:46 UTC (permalink / raw)
  To: Shujun Wang
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List, David Box

On Fri, Apr 30, 2021 at 3:31 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Fri, Apr 30, 2021 at 2:43 PM Shujun Wang <wsj20369@163.com> wrote:
> >
> > This reverts commit 7e4fdeafa61f2b653fcf9678f09935e55756aed2.
>
> OK, I'll revert that commit, thanks!

That said I'm assuming that this patch:

https://patchwork.kernel.org/project/linux-pm/patch/3219454.74lMxhSOWB@kreacher/

is present in the kernel that you have tested.

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

* RE: [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally"
  2021-04-30 12:42 [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally" Shujun Wang
  2021-04-30 13:31 ` Rafael J. Wysocki
@ 2021-05-10  6:37 ` Zhang, Rui
  2021-05-10 12:13   ` Rafael J. Wysocki
  2021-05-14 14:05 ` Rafael J. Wysocki
  2 siblings, 1 reply; 9+ messages in thread
From: Zhang, Rui @ 2021-05-10  6:37 UTC (permalink / raw)
  To: Shujun Wang, rjw, lenb, linux-acpi, linux-kernel

Hi, Shujun,

I'm experiencing similar problem, and it should be a BIOS problem, which can be fixed by a customized DSDT.
Can you please attach the full acpidump output on this machine? I just want to make sure if it is the same problem.

Thanks,
rui

> -----Original Message-----
> From: Shujun Wang <wsj20369@163.com>
> Sent: Friday, April 30, 2021 8:42 PM
> To: rjw@rjwysocki.net; lenb@kernel.org; linux-acpi@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: Shujun Wang <wsj20369@163.com>
> Subject: [PATCH] Revert "ACPI: power: Turn off unused power resources
> unconditionally"
> 
> This reverts commit 7e4fdeafa61f2b653fcf9678f09935e55756aed2.
> It may cause some NVMe device probes to fail, and the system may get stuck
> when using an NVMe device as the root filesystem.
> 
> In the function nvme_pci_enable(struct nvme_dev *dev), as shown below,
> readl(NVME_REG_CSTS) always returns -1 with the commit, which results in
> the probe failed.
> 
>   if (readl(dev->bar + NVME_REG_CSTS) == -1) {
> 	result = -ENODEV;
> 	goto disable;
>   }
> 
> dmesg:
>   [    1.106280] nvme 0000:04:00.0: platform quirk: setting simple suspend
>   [    1.109111] nvme nvme0: pci function 0000:04:00.0
>   [    1.113066] nvme 0000:04:00.0: enabling device (0000 -> 0002)
>   [    1.121040] nvme nvme0: Removing after probe failure status: -19
> 
> lspci:
>   Non-Volatile memory controller: KIOXIA Corporation Device 0001
> 
> device uevent:
>   DRIVER=nvme
>   PCI_CLASS=10802
>   PCI_ID=1E0F:0001
>   PCI_SUBSYS_ID=1E0F:0001
>   PCI_SLOT_NAME=0000:04:00.0
>   MODALIAS=pci:v00001E0Fd00000001sv00001E0Fsd00000001bc01sc08i02
> 
> This patch was tested in Lenovo Thinkpad X1.
> 
> Signed-off-by: Shujun Wang <wsj20369@163.com>
> ---
>  drivers/acpi/power.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c index
> 56102eaaa2da..8bf10abeb2e0 100644
> --- a/drivers/acpi/power.c
> +++ b/drivers/acpi/power.c
> @@ -1004,9 +1004,18 @@ void
> acpi_turn_off_unused_power_resources(void)
>  	mutex_lock(&power_resource_list_lock);
> 
>  	list_for_each_entry_reverse(resource, &acpi_power_resource_list,
> list_node) {
> +		int result, state;
> +
>  		mutex_lock(&resource->resource_lock);
> 
> -		if (!resource->ref_count) {
> +		result = acpi_power_get_state(resource->device.handle,
> &state);
> +		if (result) {
> +			mutex_unlock(&resource->resource_lock);
> +			continue;
> +		}
> +
> +		if (state == ACPI_POWER_RESOURCE_STATE_ON
> +		    && !resource->ref_count) {
>  			dev_info(&resource->device.dev, "Turning OFF\n");
>  			__acpi_power_off(resource);
>  		}
> --
> 2.25.1


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

* Re: [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally"
  2021-05-10  6:37 ` Zhang, Rui
@ 2021-05-10 12:13   ` Rafael J. Wysocki
  2021-05-10 13:59     ` Zhang Rui
  0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2021-05-10 12:13 UTC (permalink / raw)
  To: Zhang, Rui, Shujun Wang; +Cc: rjw, lenb, linux-acpi, linux-kernel, David Box

On Mon, May 10, 2021 at 8:37 AM Zhang, Rui <rui.zhang@intel.com> wrote:
>
> Hi, Shujun,
>
> I'm experiencing similar problem, and it should be a BIOS problem,

Right, and I confused things.  Sorry about that.

If commit 7e4fdeafa61f2b653f ("ACPI: power: Turn off unused power
resources unconditionally") causes problems to happen, this means that
the platform firmware implementation doesn't follow the ACPI
specification.

> which can be fixed by a customized DSDT.
> Can you please attach the full acpidump output on this machine? I just want to make sure if it is the same problem.

Yes, please.

Rui, can you create a BZ for this please and can you both attach
dmidecode output from the affected systems?

I don't want to revert this commit completely, so the default behavior
is spec-compliant, but there can be a DMI-based blacklist for systems
having problems with it.

> > -----Original Message-----
> > From: Shujun Wang <wsj20369@163.com>
> > Sent: Friday, April 30, 2021 8:42 PM
> > To: rjw@rjwysocki.net; lenb@kernel.org; linux-acpi@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Cc: Shujun Wang <wsj20369@163.com>
> > Subject: [PATCH] Revert "ACPI: power: Turn off unused power resources
> > unconditionally"
> >
> > This reverts commit 7e4fdeafa61f2b653fcf9678f09935e55756aed2.
> > It may cause some NVMe device probes to fail, and the system may get stuck
> > when using an NVMe device as the root filesystem.
> >
> > In the function nvme_pci_enable(struct nvme_dev *dev), as shown below,
> > readl(NVME_REG_CSTS) always returns -1 with the commit, which results in
> > the probe failed.
> >
> >   if (readl(dev->bar + NVME_REG_CSTS) == -1) {
> >       result = -ENODEV;
> >       goto disable;
> >   }
> >
> > dmesg:
> >   [    1.106280] nvme 0000:04:00.0: platform quirk: setting simple suspend
> >   [    1.109111] nvme nvme0: pci function 0000:04:00.0
> >   [    1.113066] nvme 0000:04:00.0: enabling device (0000 -> 0002)
> >   [    1.121040] nvme nvme0: Removing after probe failure status: -19
> >
> > lspci:
> >   Non-Volatile memory controller: KIOXIA Corporation Device 0001
> >
> > device uevent:
> >   DRIVER=nvme
> >   PCI_CLASS=10802
> >   PCI_ID=1E0F:0001
> >   PCI_SUBSYS_ID=1E0F:0001
> >   PCI_SLOT_NAME=0000:04:00.0
> >   MODALIAS=pci:v00001E0Fd00000001sv00001E0Fsd00000001bc01sc08i02
> >
> > This patch was tested in Lenovo Thinkpad X1.
> >
> > Signed-off-by: Shujun Wang <wsj20369@163.com>
> > ---
> >  drivers/acpi/power.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c index
> > 56102eaaa2da..8bf10abeb2e0 100644
> > --- a/drivers/acpi/power.c
> > +++ b/drivers/acpi/power.c
> > @@ -1004,9 +1004,18 @@ void
> > acpi_turn_off_unused_power_resources(void)
> >       mutex_lock(&power_resource_list_lock);
> >
> >       list_for_each_entry_reverse(resource, &acpi_power_resource_list,
> > list_node) {
> > +             int result, state;
> > +
> >               mutex_lock(&resource->resource_lock);
> >
> > -             if (!resource->ref_count) {
> > +             result = acpi_power_get_state(resource->device.handle,
> > &state);
> > +             if (result) {
> > +                     mutex_unlock(&resource->resource_lock);
> > +                     continue;
> > +             }
> > +
> > +             if (state == ACPI_POWER_RESOURCE_STATE_ON
> > +                 && !resource->ref_count) {
> >                       dev_info(&resource->device.dev, "Turning OFF\n");
> >                       __acpi_power_off(resource);
> >               }
> > --
> > 2.25.1
>

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

* Re: [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally"
  2021-05-10 12:13   ` Rafael J. Wysocki
@ 2021-05-10 13:59     ` Zhang Rui
  2021-05-17  9:48       ` wsj20369
  0 siblings, 1 reply; 9+ messages in thread
From: Zhang Rui @ 2021-05-10 13:59 UTC (permalink / raw)
  To: Rafael J. Wysocki, Shujun Wang
  Cc: rjw, lenb, linux-acpi, linux-kernel, David Box

On Mon, 2021-05-10 at 14:13 +0200, Rafael J. Wysocki wrote:
> On Mon, May 10, 2021 at 8:37 AM Zhang, Rui <rui.zhang@intel.com>
> wrote:
> > 
> > Hi, Shujun,
> > 
> > I'm experiencing similar problem, and it should be a BIOS problem,
> 
> Right, and I confused things.  Sorry about that.
> 
> If commit 7e4fdeafa61f2b653f ("ACPI: power: Turn off unused power
> resources unconditionally") causes problems to happen, this means
> that
> the platform firmware implementation doesn't follow the ACPI
> specification.
> 
> > which can be fixed by a customized DSDT.
> > Can you please attach the full acpidump output on this machine? I
> > just want to make sure if it is the same problem.
> 
> Yes, please.
> 
> Rui, can you create a BZ for this please and can you both attach
> dmidecode output from the affected systems?
> 
> I don't want to revert this commit completely, so the default
> behavior
> is spec-compliant, but there can be a DMI-based blacklist for systems
> having problems with it.
> 
Done.
https://bugzilla.kernel.org/show_bug.cgi?id=213019

Shujun,
can you please attach the acpidump and dmidecode output in this bug
report?

thanks,
rui

> > > -----Original Message-----
> > > From: Shujun Wang <wsj20369@163.com>
> > > Sent: Friday, April 30, 2021 8:42 PM
> > > To: rjw@rjwysocki.net; lenb@kernel.org; 
> > > linux-acpi@vger.kernel.org; linux-
> > > kernel@vger.kernel.org
> > > Cc: Shujun Wang <wsj20369@163.com>
> > > Subject: [PATCH] Revert "ACPI: power: Turn off unused power
> > > resources
> > > unconditionally"
> > > 
> > > This reverts commit 7e4fdeafa61f2b653fcf9678f09935e55756aed2.
> > > It may cause some NVMe device probes to fail, and the system may
> > > get stuck
> > > when using an NVMe device as the root filesystem.
> > > 
> > > In the function nvme_pci_enable(struct nvme_dev *dev), as shown
> > > below,
> > > readl(NVME_REG_CSTS) always returns -1 with the commit, which
> > > results in
> > > the probe failed.
> > > 
> > >   if (readl(dev->bar + NVME_REG_CSTS) == -1) {
> > >       result = -ENODEV;
> > >       goto disable;
> > >   }
> > > 
> > > dmesg:
> > >   [    1.106280] nvme 0000:04:00.0: platform quirk: setting
> > > simple suspend
> > >   [    1.109111] nvme nvme0: pci function 0000:04:00.0
> > >   [    1.113066] nvme 0000:04:00.0: enabling device (0000 ->
> > > 0002)
> > >   [    1.121040] nvme nvme0: Removing after probe failure status:
> > > -19
> > > 
> > > lspci:
> > >   Non-Volatile memory controller: KIOXIA Corporation Device 0001
> > > 
> > > device uevent:
> > >   DRIVER=nvme
> > >   PCI_CLASS=10802
> > >   PCI_ID=1E0F:0001
> > >   PCI_SUBSYS_ID=1E0F:0001
> > >   PCI_SLOT_NAME=0000:04:00.0
> > >   MODALIAS=pci:v00001E0Fd00000001sv00001E0Fsd00000001bc01sc08i02
> > > 
> > > This patch was tested in Lenovo Thinkpad X1.
> > > 
> > > Signed-off-by: Shujun Wang <wsj20369@163.com>
> > > ---
> > >  drivers/acpi/power.c | 11 ++++++++++-
> > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c index
> > > 56102eaaa2da..8bf10abeb2e0 100644
> > > --- a/drivers/acpi/power.c
> > > +++ b/drivers/acpi/power.c
> > > @@ -1004,9 +1004,18 @@ void
> > > acpi_turn_off_unused_power_resources(void)
> > >       mutex_lock(&power_resource_list_lock);
> > > 
> > >       list_for_each_entry_reverse(resource,
> > > &acpi_power_resource_list,
> > > list_node) {
> > > +             int result, state;
> > > +
> > >               mutex_lock(&resource->resource_lock);
> > > 
> > > -             if (!resource->ref_count) {
> > > +             result = acpi_power_get_state(resource-
> > > >device.handle,
> > > &state);
> > > +             if (result) {
> > > +                     mutex_unlock(&resource->resource_lock);
> > > +                     continue;
> > > +             }
> > > +
> > > +             if (state == ACPI_POWER_RESOURCE_STATE_ON
> > > +                 && !resource->ref_count) {
> > >                       dev_info(&resource->device.dev, "Turning
> > > OFF\n");
> > >                       __acpi_power_off(resource);
> > >               }
> > > --
> > > 2.25.1


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

* Re: [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally"
  2021-04-30 12:42 [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally" Shujun Wang
  2021-04-30 13:31 ` Rafael J. Wysocki
  2021-05-10  6:37 ` Zhang, Rui
@ 2021-05-14 14:05 ` Rafael J. Wysocki
  2021-05-17  9:45   ` wsj20369
  2 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2021-05-14 14:05 UTC (permalink / raw)
  To: Shujun Wang
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List

On Fri, Apr 30, 2021 at 2:43 PM Shujun Wang <wsj20369@163.com> wrote:
>
> This reverts commit 7e4fdeafa61f2b653fcf9678f09935e55756aed2.
> It may cause some NVMe device probes to fail, and the system may get
> stuck when using an NVMe device as the root filesystem.
>
> In the function nvme_pci_enable(struct nvme_dev *dev), as shown below,
> readl(NVME_REG_CSTS) always returns -1 with the commit, which results in
> the probe failed.
>
>   if (readl(dev->bar + NVME_REG_CSTS) == -1) {
>         result = -ENODEV;
>         goto disable;
>   }
>
> dmesg:
>   [    1.106280] nvme 0000:04:00.0: platform quirk: setting simple suspend
>   [    1.109111] nvme nvme0: pci function 0000:04:00.0
>   [    1.113066] nvme 0000:04:00.0: enabling device (0000 -> 0002)
>   [    1.121040] nvme nvme0: Removing after probe failure status: -19
>
> lspci:
>   Non-Volatile memory controller: KIOXIA Corporation Device 0001
>
> device uevent:
>   DRIVER=nvme
>   PCI_CLASS=10802
>   PCI_ID=1E0F:0001
>   PCI_SUBSYS_ID=1E0F:0001
>   PCI_SLOT_NAME=0000:04:00.0
>   MODALIAS=pci:v00001E0Fd00000001sv00001E0Fsd00000001bc01sc08i02
>
> This patch was tested in Lenovo Thinkpad X1.

Please send me the dmidecode output from this machine or (better)
attach it at https://bugzilla.kernel.org/show_bug.cgi?id=213019

Thanks!

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

* Re:Re: [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally"
  2021-05-14 14:05 ` Rafael J. Wysocki
@ 2021-05-17  9:45   ` wsj20369
  0 siblings, 0 replies; 9+ messages in thread
From: wsj20369 @ 2021-05-17  9:45 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List



At 2021-05-14 22:05:06, "Rafael J. Wysocki" <rafael@kernel.org> wrote:
>On Fri, Apr 30, 2021 at 2:43 PM Shujun Wang <wsj20369@163.com> wrote:
>>
>> This reverts commit 7e4fdeafa61f2b653fcf9678f09935e55756aed2.
>> It may cause some NVMe device probes to fail, and the system may get
>> stuck when using an NVMe device as the root filesystem.
>>
>> In the function nvme_pci_enable(struct nvme_dev *dev), as shown below,
>> readl(NVME_REG_CSTS) always returns -1 with the commit, which results in
>> the probe failed.
>>
>>   if (readl(dev->bar + NVME_REG_CSTS) == -1) {
>>         result = -ENODEV;
>>         goto disable;
>>   }
>>
>> dmesg:
>>   [    1.106280] nvme 0000:04:00.0: platform quirk: setting simple suspend
>>   [    1.109111] nvme nvme0: pci function 0000:04:00.0
>>   [    1.113066] nvme 0000:04:00.0: enabling device (0000 -> 0002)
>>   [    1.121040] nvme nvme0: Removing after probe failure status: -19
>>
>> lspci:
>>   Non-Volatile memory controller: KIOXIA Corporation Device 0001
>>
>> device uevent:
>>   DRIVER=nvme
>>   PCI_CLASS=10802
>>   PCI_ID=1E0F:0001
>>   PCI_SUBSYS_ID=1E0F:0001
>>   PCI_SLOT_NAME=0000:04:00.0
>>   MODALIAS=pci:v00001E0Fd00000001sv00001E0Fsd00000001bc01sc08i02
>>
>> This patch was tested in Lenovo Thinkpad X1.
>
>Please send me the dmidecode output from this machine or (better)
>attach it at https://bugzilla.kernel.org/show_bug.cgi?id=213019
>
>Thanks!

Hi Rafael:

Sorry for late, I have attached the acpidump and dmidecode in the https://bugzilla.kernel.org/show_bug.cgi?id=213019

BR
Shujun Wang


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

* Re:Re: [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally"
  2021-05-10 13:59     ` Zhang Rui
@ 2021-05-17  9:48       ` wsj20369
  0 siblings, 0 replies; 9+ messages in thread
From: wsj20369 @ 2021-05-17  9:48 UTC (permalink / raw)
  To: Zhang Rui
  Cc: Rafael J. Wysocki, rjw, lenb, linux-acpi, linux-kernel, David Box

At 2021-05-10 21:59:03, "Zhang Rui" <rui.zhang@intel.com> wrote:
>On Mon, 2021-05-10 at 14:13 +0200, Rafael J. Wysocki wrote:
>> On Mon, May 10, 2021 at 8:37 AM Zhang, Rui <rui.zhang@intel.com>
>> wrote:
>> > 
>> > Hi, Shujun,
>> > 
>> > I'm experiencing similar problem, and it should be a BIOS problem,
>> 
>> Right, and I confused things.  Sorry about that.
>> 
>> If commit 7e4fdeafa61f2b653f ("ACPI: power: Turn off unused power
>> resources unconditionally") causes problems to happen, this means
>> that
>> the platform firmware implementation doesn't follow the ACPI
>> specification.
>> 
>> > which can be fixed by a customized DSDT.
>> > Can you please attach the full acpidump output on this machine? I
>> > just want to make sure if it is the same problem.
>> 
>> Yes, please.
>> 
>> Rui, can you create a BZ for this please and can you both attach
>> dmidecode output from the affected systems?
>> 
>> I don't want to revert this commit completely, so the default
>> behavior
>> is spec-compliant, but there can be a DMI-based blacklist for systems
>> having problems with it.
>> 
>Done.
>https://bugzilla.kernel.org/show_bug.cgi?id=213019
>
>Shujun,
>can you please attach the acpidump and dmidecode output in this bug
>report?
>
>thanks,
>rui

Hi Rui:

Sorry for late, I have attached the acpidump and dmidecode in the https://bugzilla.kernel.org/show_bug.cgi?id=213019

BR
Shujun Wang

>
>> > > -----Original Message-----
>> > > From: Shujun Wang <wsj20369@163.com>
>> > > Sent: Friday, April 30, 2021 8:42 PM
>> > > To: rjw@rjwysocki.net; lenb@kernel.org; 
>> > > linux-acpi@vger.kernel.org; linux-
>> > > kernel@vger.kernel.org
>> > > Cc: Shujun Wang <wsj20369@163.com>
>> > > Subject: [PATCH] Revert "ACPI: power: Turn off unused power
>> > > resources
>> > > unconditionally"
>> > > 
>> > > This reverts commit 7e4fdeafa61f2b653fcf9678f09935e55756aed2.
>> > > It may cause some NVMe device probes to fail, and the system may
>> > > get stuck
>> > > when using an NVMe device as the root filesystem.
>> > > 
>> > > In the function nvme_pci_enable(struct nvme_dev *dev), as shown
>> > > below,
>> > > readl(NVME_REG_CSTS) always returns -1 with the commit, which
>> > > results in
>> > > the probe failed.
>> > > 
>> > >   if (readl(dev->bar + NVME_REG_CSTS) == -1) {
>> > >       result = -ENODEV;
>> > >       goto disable;
>> > >   }
>> > > 
>> > > dmesg:
>> > >   [    1.106280] nvme 0000:04:00.0: platform quirk: setting
>> > > simple suspend
>> > >   [    1.109111] nvme nvme0: pci function 0000:04:00.0
>> > >   [    1.113066] nvme 0000:04:00.0: enabling device (0000 ->
>> > > 0002)
>> > >   [    1.121040] nvme nvme0: Removing after probe failure status:
>> > > -19
>> > > 
>> > > lspci:
>> > >   Non-Volatile memory controller: KIOXIA Corporation Device 0001
>> > > 
>> > > device uevent:
>> > >   DRIVER=nvme
>> > >   PCI_CLASS=10802
>> > >   PCI_ID=1E0F:0001
>> > >   PCI_SUBSYS_ID=1E0F:0001
>> > >   PCI_SLOT_NAME=0000:04:00.0
>> > >   MODALIAS=pci:v00001E0Fd00000001sv00001E0Fsd00000001bc01sc08i02
>> > > 
>> > > This patch was tested in Lenovo Thinkpad X1.
>> > > 
>> > > Signed-off-by: Shujun Wang <wsj20369@163.com>
>> > > ---
>> > >  drivers/acpi/power.c | 11 ++++++++++-
>> > >  1 file changed, 10 insertions(+), 1 deletion(-)
>> > > 
>> > > diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c index
>> > > 56102eaaa2da..8bf10abeb2e0 100644
>> > > --- a/drivers/acpi/power.c
>> > > +++ b/drivers/acpi/power.c
>> > > @@ -1004,9 +1004,18 @@ void
>> > > acpi_turn_off_unused_power_resources(void)
>> > >       mutex_lock(&power_resource_list_lock);
>> > > 
>> > >       list_for_each_entry_reverse(resource,
>> > > &acpi_power_resource_list,
>> > > list_node) {
>> > > +             int result, state;
>> > > +
>> > >               mutex_lock(&resource->resource_lock);
>> > > 
>> > > -             if (!resource->ref_count) {
>> > > +             result = acpi_power_get_state(resource-
>> > > >device.handle,
>> > > &state);
>> > > +             if (result) {
>> > > +                     mutex_unlock(&resource->resource_lock);
>> > > +                     continue;
>> > > +             }
>> > > +
>> > > +             if (state == ACPI_POWER_RESOURCE_STATE_ON
>> > > +                 && !resource->ref_count) {
>> > >                       dev_info(&resource->device.dev, "Turning
>> > > OFF\n");
>> > >                       __acpi_power_off(resource);
>> > >               }
>> > > --
>> > > 2.25.1

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

end of thread, other threads:[~2021-05-17 10:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-30 12:42 [PATCH] Revert "ACPI: power: Turn off unused power resources unconditionally" Shujun Wang
2021-04-30 13:31 ` Rafael J. Wysocki
2021-04-30 13:46   ` Rafael J. Wysocki
2021-05-10  6:37 ` Zhang, Rui
2021-05-10 12:13   ` Rafael J. Wysocki
2021-05-10 13:59     ` Zhang Rui
2021-05-17  9:48       ` wsj20369
2021-05-14 14:05 ` Rafael J. Wysocki
2021-05-17  9:45   ` wsj20369

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).