All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"
@ 2022-03-24 14:59 Mario Limonciello
  2022-03-24 15:05 ` Christoph Hellwig
  0 siblings, 1 reply; 4+ messages in thread
From: Mario Limonciello @ 2022-03-24 14:59 UTC (permalink / raw)
  To: mario.limonciello, Damien Le Moal,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list

SATA_LPM_POLICY is a "new" configuration item, but it was renamed from
SATA_LPM_MOBILE_POLICY in commit 4dd4d3deb502 ("ata: ahci: Rename
CONFIG_SATA_LPM_MOBILE_POLICY configuration item")

Since the configuration item was renamed it was mentioned that people
might invisibly lose their chosen defaults for it.  This means it's a
good time to evaluate whether those defaults make sense still.

Historically this was set to "0" to prevent problems with power management
on very old drives.  However it's been observed that almost all modern
Linux distributions either set the policy to "3" in the kernel
configuration or update it to this via userspace policy changes. Update
the policy default in the kernel to "3" to match that behavior as well.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
We at least have confirmation from:
* Ubuntu
* RHEL/CentOS/SUSE
* Arch
* Gentoo
* Debian

 drivers/ata/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index e5641e6c52ee..3ffe14057ed2 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -118,7 +118,7 @@ config SATA_AHCI
 config SATA_LPM_POLICY
 	int "Default SATA Link Power Management policy for low power chipsets"
 	range 0 4
-	default 0
+	default 3
 	depends on SATA_AHCI
 	help
 	  Select the Default SATA Link Power Management (LPM) policy to use
-- 
2.34.1


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

* Re: [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"
  2022-03-24 14:59 [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3" Mario Limonciello
@ 2022-03-24 15:05 ` Christoph Hellwig
  2022-03-24 15:09   ` Limonciello, Mario
  0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2022-03-24 15:05 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Damien Le Moal,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list

What about just reverting the rename of the config symbol?
It is a bit misleadinḑ, but so are many other option nams.  And they
really aren't the user interface anyway, the help text and description
are the main user interface and can be changed at will.

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

* RE: [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"
  2022-03-24 15:05 ` Christoph Hellwig
@ 2022-03-24 15:09   ` Limonciello, Mario
  2022-03-28  0:37     ` Damien Le Moal
  0 siblings, 1 reply; 4+ messages in thread
From: Limonciello, Mario @ 2022-03-24 15:09 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Damien Le Moal,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list

[Public]

> -----Original Message-----
> From: Christoph Hellwig <hch@infradead.org>
> Sent: Thursday, March 24, 2022 10:05
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com>; open
> list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers) <linux-
> ide@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"
> 
> What about just reverting the rename of the config symbol?
> It is a bit misleadinḑ, but so are many other option nams.  And they
> really aren't the user interface anyway, the help text and description
> are the main user interface and can be changed at will.

It was the rename that prompted us to give it a closer look.
I think this patch should to change the default should happen either way.

If Damien or Linus wants to revert the rename, that's fine with me, but if
we land this patch then I think it should serve the intent of most people and
distros.

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

* Re: [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"
  2022-03-24 15:09   ` Limonciello, Mario
@ 2022-03-28  0:37     ` Damien Le Moal
  0 siblings, 0 replies; 4+ messages in thread
From: Damien Le Moal @ 2022-03-28  0:37 UTC (permalink / raw)
  To: Limonciello, Mario, Christoph Hellwig
  Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers), open list

On 3/25/22 00:09, Limonciello, Mario wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Christoph Hellwig <hch@infradead.org>
>> Sent: Thursday, March 24, 2022 10:05
>> To: Limonciello, Mario <Mario.Limonciello@amd.com>
>> Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com>; open
>> list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers) <linux-
>> ide@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
>> Subject: Re: [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"
>>
>> What about just reverting the rename of the config symbol?
>> It is a bit misleadinḑ, but so are many other option nams.  And they
>> really aren't the user interface anyway, the help text and description
>> are the main user interface and can be changed at will.
> 
> It was the rename that prompted us to give it a closer look.
> I think this patch should to change the default should happen either way.
> 
> If Damien or Linus wants to revert the rename, that's fine with me, but if
> we land this patch then I think it should serve the intent of most people and
> distros.

I am OK with the change of the default value to 3 since this is what most
distros have today anyway. Nevertheless, I think we should fallback on the
safe side with this and also revert the config name back to
CONFIG_SATA_LPM_MOBILE_POLICY to avoid any unintended side effects.

Please send a v2 with that rename and the default value change. As
Christoph suggested, the config option help text can be broader than what
the name suggests. I am thinking of something like this:

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index f27b12ba2ce7..ee3f96b5be70 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -125,14 +125,16 @@ config SATA_AHCI

 	  If unsure, say N.

-config SATA_LPM_POLICY
+config SATA_MOBILE_LPM_POLICY
 	int "Default SATA Link Power Management policy for low power chipsets"
 	range 0 4
-	default 0
+	default 3
 	depends on SATA_AHCI
 	help
 	  Select the Default SATA Link Power Management (LPM) policy to use
-	  for chipsets / "South Bridges" designated as supporting low power.
+	  for chipsets / "South Bridges" supporting low-power modes. Such
+	  chipsets are typically found on most laptops but desktops and
+	  servers now also widely use chipsets with low power modes support.

 	  The value set has the following meanings:
 		0 => Keep firmware settings



-- 
Damien Le Moal
Western Digital Research

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

end of thread, other threads:[~2022-03-28  0:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-24 14:59 [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3" Mario Limonciello
2022-03-24 15:05 ` Christoph Hellwig
2022-03-24 15:09   ` Limonciello, Mario
2022-03-28  0:37     ` Damien Le Moal

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.