Re: [PATCH 3/6] ata: Fix SATA_MOBILE_LPM_POLICY description in Kconfig

[Damien Le Moal <dlemoal@kernel.org>]

On 6/27/25 15:03, Hannes Reinecke wrote:
> On 6/27/25 03:11, Damien Le Moal wrote:
>> Improve the description of the possible default SATA link power
>> management policies and add the missing description for policy 5.
>> No functional changes.
>>
>> Fixes: a5ec5a7bfd1f ("ata: ahci: Support state with min power but Partial low power state")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>> ---
>>   drivers/ata/Kconfig | 36 ++++++++++++++++++++++++++----------
>>   1 file changed, 26 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
>> index e00536b49552..1d53d7b568bd 100644
>> --- a/drivers/ata/Kconfig
>> +++ b/drivers/ata/Kconfig
>> @@ -117,23 +117,39 @@ config SATA_AHCI
>>   
>>   config SATA_MOBILE_LPM_POLICY
>>   	int "Default SATA Link Power Management policy"
>> -	range 0 4
>> +	range 0 5
>>   	default 3
>>   	depends on SATA_AHCI
>>   	help
>>   	  Select the Default SATA Link Power Management (LPM) policy to use
>>   	  for chipsets / "South Bridges" supporting low-power modes. Such
>>   	  chipsets are ubiquitous across laptops, desktops and servers.
>> -
>> -	  The value set has the following meanings:
>> +	  Each policy combines power saving states and features:
>> +	   - Partial: The Phy logic is powered but is in a reduced power
>> +                      state. The exit latency from this state is no longer than
>> +                      10us).
>> +	   - Slumber: The Phy logic is powered but is in an even lower power
>> +                      state. The exit latency from this state is potentially
>> +		      longer, but no longer than 10ms.
>> +	   - DevSleep: The Phy logic may be powered down. The exit latency from
>> +	               this state is no longer than 20 ms, unless otherwise
>> +		       specified by DETO in the device Identify Device Data log.
>> +	   - HIPM: Host Initiated Power Management (host automatic transisitons
>> +		   to partial and slumber).
>> +	   - DIPM: Device Initiated Power Management (device automatic
>> +		   transitions to partial and slumber).
>> +
>> +	  The possible values for the default SATA link power management
>> +	  policies are:
>>   		0 => Keep firmware settings
>> -		1 => Maximum performance
>> -		2 => Medium power
>> -		3 => Medium power with Device Initiated PM enabled
>> -		4 => Minimum power
>> -
>> -	  Note "Minimum power" is known to cause issues, including disk
>> -	  corruption, with some disks and should not be used.
>> +		1 => No power savings (maximum performance)
>> +		2 => HIPM (Partial)
>> +		3 => HIPM (Partial) and DIPM (Partial and Slumber)
>> +		4 => HIPM (Partial and DevSleep) and DIPM (Partial and Slumber)
>> +		5 => HIPM (Slumber and DevSleep) and DIPM (Partial and Slumber)
>> +
>> +	  Excluding the value 0, higher values represent policies with higher
>> +	  power savings.
>>   
>>   config SATA_AHCI_PLATFORM
>>   	tristate "Platform AHCI SATA support"
> 
> Hmm. Isn't it worth creating an official documentation somewhere in
> Documentation/* to have this available without having to look at the
> source code?

Yes, I am considering this.
I did not write it in the cover letter, but these patches are the first part of
a much bigger patch series that addresses a lot of LPM issues that we recently
discovered doing completely unrelated tests. Once that is out, documenting
everything will be a good thing to do.

> 
> Otherwise:
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> 
> Cheers,
> 
> Hannes


-- 
Damien Le Moal
Western Digital Research