[PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"

[PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"

[Mario Limonciello]

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

Re: [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"

[Christoph Hellwig]

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.

RE: [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"

[Limonciello, Mario]

[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.

Re: [PATCH] ata: Kconfig: Update SATA_LPM_POLICY default to "3"

[Damien Le Moal]

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