| Message ID | 20220324145905.4176-1-mario.limonciello@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | ata: Kconfig: Update SATA_LPM_POLICY default to "3" | expand |
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.
[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.
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
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
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(-)