[2/2] ACPI: x86: Add more systems to quirk list for forcing StorageD3Enable

Message ID 20230213213537.6121-2-mario.limonciello@amd.com
State New
Headers
Series [1/2] ACPI: x86: Add more systems to quirk list for forcing StorageD3Enable |

Commit Message

Mario Limonciello Feb. 13, 2023, 9:35 p.m. UTC
  commit 018d6711c26e4 ("ACPI: x86: Add a quirk for Dell Inspiron 14 2-in-1
for StorageD3Enable") introduced a quirk to allow a system with ambiguous
use of _ADR 0 to force StorageD3Enable.

It is reported that Vostro 5626 suffers same symptoms. Add this other
system to the list as well.

Suggested-by: dbilios@stdio.gr
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217003
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/x86/utils.c | 7 +++++++
 1 file changed, 7 insertions(+)
  

Comments

Rafael J. Wysocki Feb. 14, 2023, 2:48 p.m. UTC | #1
On Mon, Feb 13, 2023 at 10:35 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> commit 018d6711c26e4 ("ACPI: x86: Add a quirk for Dell Inspiron 14 2-in-1
> for StorageD3Enable") introduced a quirk to allow a system with ambiguous
> use of _ADR 0 to force StorageD3Enable.
>
> It is reported that Vostro 5626 suffers same symptoms. Add this other
> system to the list as well.
>
> Suggested-by: dbilios@stdio.gr
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217003
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Can you please combine these two patches into one?

Or at least make the subjects differ?

> ---
>  drivers/acpi/x86/utils.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
> index 3409ce6513afa..b1d24718f73d7 100644
> --- a/drivers/acpi/x86/utils.c
> +++ b/drivers/acpi/x86/utils.c
> @@ -214,6 +214,7 @@ static const struct dmi_system_id force_storage_d3_dmi[] = {
>                  * but .NVME is needed to get StorageD3Enable node
>                  * https://bugzilla.kernel.org/show_bug.cgi?id=216440
>                  * https://bugzilla.kernel.org/show_bug.cgi?id=216773
> +                * https://bugzilla.kernel.org/show_bug.cgi?id=217003
>                  */
>                 .matches = {
>                         DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> @@ -232,6 +233,12 @@ static const struct dmi_system_id force_storage_d3_dmi[] = {
>                         DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 16 5625"),
>                 }
>         },
> +       {
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 5625"),
> +               }
> +       },
>         {}
>  };
>
> --
> 2.34.1
>
  
Mario Limonciello Feb. 14, 2023, 2:52 p.m. UTC | #2
[AMD Official Use Only - General]



> -----Original Message-----
> From: Rafael J. Wysocki <rafael@kernel.org>
> Sent: Tuesday, February 14, 2023 08:49
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: rafael@kernel.org; linux-kernel@vger.kernel.org; dbilios@stdio.gr; Len
> Brown <lenb@kernel.org>; linux-acpi@vger.kernel.org
> Subject: Re: [PATCH 2/2] ACPI: x86: Add more systems to quirk list for forcing
> StorageD3Enable
> 
> On Mon, Feb 13, 2023 at 10:35 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> >
> > commit 018d6711c26e4 ("ACPI: x86: Add a quirk for Dell Inspiron 14 2-in-1
> > for StorageD3Enable") introduced a quirk to allow a system with ambiguous
> > use of _ADR 0 to force StorageD3Enable.
> >
> > It is reported that Vostro 5626 suffers same symptoms. Add this other
> > system to the list as well.
> >
> > Suggested-by: dbilios@stdio.gr
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217003
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> Can you please combine these two patches into one?
> 
> Or at least make the subjects differ?

I left them split so that we could revert one if the BIOS is fixed on one
in the future.  Happy to respin with separate subjects.

But before I respin; something else I want to offer as an idea to avoid this list
growing.

They all use the same CPU.  So we could just add the CPU model that all of these
use to the force StorageD3Enable list and take out the whole (growing) list.

Which way would you prefer?

> 
> > ---
> >  drivers/acpi/x86/utils.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
> > index 3409ce6513afa..b1d24718f73d7 100644
> > --- a/drivers/acpi/x86/utils.c
> > +++ b/drivers/acpi/x86/utils.c
> > @@ -214,6 +214,7 @@ static const struct dmi_system_id
> force_storage_d3_dmi[] = {
> >                  * but .NVME is needed to get StorageD3Enable node
> >                  * https://bugzilla.kernel.org/show_bug.cgi?id=216440
> >                  * https://bugzilla.kernel.org/show_bug.cgi?id=216773
> > +                * https://bugzilla.kernel.org/show_bug.cgi?id=217003
> >                  */
> >                 .matches = {
> >                         DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > @@ -232,6 +233,12 @@ static const struct dmi_system_id
> force_storage_d3_dmi[] = {
> >                         DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 16 5625"),
> >                 }
> >         },
> > +       {
> > +               .matches = {
> > +                       DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > +                       DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 5625"),
> > +               }
> > +       },
> >         {}
> >  };
> >
> > --
> > 2.34.1
> >
  
Rafael J. Wysocki Feb. 14, 2023, 2:58 p.m. UTC | #3
On Tue, Feb 14, 2023 at 3:52 PM Limonciello, Mario
<Mario.Limonciello@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
>
>
> > -----Original Message-----
> > From: Rafael J. Wysocki <rafael@kernel.org>
> > Sent: Tuesday, February 14, 2023 08:49
> > To: Limonciello, Mario <Mario.Limonciello@amd.com>
> > Cc: rafael@kernel.org; linux-kernel@vger.kernel.org; dbilios@stdio.gr; Len
> > Brown <lenb@kernel.org>; linux-acpi@vger.kernel.org
> > Subject: Re: [PATCH 2/2] ACPI: x86: Add more systems to quirk list for forcing
> > StorageD3Enable
> >
> > On Mon, Feb 13, 2023 at 10:35 PM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> > >
> > > commit 018d6711c26e4 ("ACPI: x86: Add a quirk for Dell Inspiron 14 2-in-1
> > > for StorageD3Enable") introduced a quirk to allow a system with ambiguous
> > > use of _ADR 0 to force StorageD3Enable.
> > >
> > > It is reported that Vostro 5626 suffers same symptoms. Add this other
> > > system to the list as well.
> > >
> > > Suggested-by: dbilios@stdio.gr
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217003
> > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >
> > Can you please combine these two patches into one?
> >
> > Or at least make the subjects differ?
>
> I left them split so that we could revert one if the BIOS is fixed on one
> in the future.  Happy to respin with separate subjects.
>
> But before I respin; something else I want to offer as an idea to avoid this list
> growing.
>
> They all use the same CPU.  So we could just add the CPU model that all of these
> use to the force StorageD3Enable list and take out the whole (growing) list.

Well, I'd rather do this, but in the next cycle.

> Which way would you prefer?

For this merge window, please respin the current patch series and make
the changes as per the above early after 6.3-rc1.

It may be still necessary to pull them into 6.3-rc later, if more
instances are reported or discovered, but we'll see.
  
Mario Limonciello Feb. 14, 2023, 3 p.m. UTC | #4
[AMD Official Use Only - General]



> -----Original Message-----
> From: Rafael J. Wysocki <rafael@kernel.org>
> Sent: Tuesday, February 14, 2023 08:59
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>; linux-kernel@vger.kernel.org;
> dbilios@stdio.gr; Len Brown <lenb@kernel.org>; linux-acpi@vger.kernel.org
> Subject: Re: [PATCH 2/2] ACPI: x86: Add more systems to quirk list for forcing
> StorageD3Enable
> 
> On Tue, Feb 14, 2023 at 3:52 PM Limonciello, Mario
> <Mario.Limonciello@amd.com> wrote:
> >
> > [AMD Official Use Only - General]
> >
> >
> >
> > > -----Original Message-----
> > > From: Rafael J. Wysocki <rafael@kernel.org>
> > > Sent: Tuesday, February 14, 2023 08:49
> > > To: Limonciello, Mario <Mario.Limonciello@amd.com>
> > > Cc: rafael@kernel.org; linux-kernel@vger.kernel.org; dbilios@stdio.gr;
> Len
> > > Brown <lenb@kernel.org>; linux-acpi@vger.kernel.org
> > > Subject: Re: [PATCH 2/2] ACPI: x86: Add more systems to quirk list for
> forcing
> > > StorageD3Enable
> > >
> > > On Mon, Feb 13, 2023 at 10:35 PM Mario Limonciello
> > > <mario.limonciello@amd.com> wrote:
> > > >
> > > > commit 018d6711c26e4 ("ACPI: x86: Add a quirk for Dell Inspiron 14 2-in-
> 1
> > > > for StorageD3Enable") introduced a quirk to allow a system with
> ambiguous
> > > > use of _ADR 0 to force StorageD3Enable.
> > > >
> > > > It is reported that Vostro 5626 suffers same symptoms. Add this other
> > > > system to the list as well.
> > > >
> > > > Suggested-by: dbilios@stdio.gr
> > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217003
> > > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > >
> > > Can you please combine these two patches into one?
> > >
> > > Or at least make the subjects differ?
> >
> > I left them split so that we could revert one if the BIOS is fixed on one
> > in the future.  Happy to respin with separate subjects.
> >
> > But before I respin; something else I want to offer as an idea to avoid this
> list
> > growing.
> >
> > They all use the same CPU.  So we could just add the CPU model that all of
> these
> > use to the force StorageD3Enable list and take out the whole (growing) list.
> 
> Well, I'd rather do this, but in the next cycle.
> 
> > Which way would you prefer?
> 
> For this merge window, please respin the current patch series and make
> the changes as per the above early after 6.3-rc1.
> 
> It may be still necessary to pull them into 6.3-rc later, if more
> instances are reported or discovered, but we'll see.

Sure, thanks.  I'll rework it for this and wait until 6.3-rc1 to send it.
  

Patch

diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
index 3409ce6513afa..b1d24718f73d7 100644
--- a/drivers/acpi/x86/utils.c
+++ b/drivers/acpi/x86/utils.c
@@ -214,6 +214,7 @@  static const struct dmi_system_id force_storage_d3_dmi[] = {
 		 * but .NVME is needed to get StorageD3Enable node
 		 * https://bugzilla.kernel.org/show_bug.cgi?id=216440
 		 * https://bugzilla.kernel.org/show_bug.cgi?id=216773
+		 * https://bugzilla.kernel.org/show_bug.cgi?id=217003
 		 */
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
@@ -232,6 +233,12 @@  static const struct dmi_system_id force_storage_d3_dmi[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 16 5625"),
 		}
 	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 5625"),
+		}
+	},
 	{}
 };