Re: [PATCH 3/3] ata: ahci: Rename CONFIG_SATA_LPM_MOBILE_POLICY configuration item

["Limonciello, Mario" <mario.limonciello@amd.com>]

On 2/16/2022 06:44, Hans de Goede wrote:
> Hi,
> 
> On 2/16/22 03:59, Mario Limonciello wrote:
>> `CONFIG_SATA_LPM_MOBILE_POLICY` reflects a configuration to apply only to
>> mobile chipsets.  As some desktop boards may want to use this policy by
>> default as well, rename the configuration item to `SATA_LPM_POLICY`.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/ata/Kconfig | 6 +++---
>>   drivers/ata/ahci.c  | 2 +-
>>   drivers/ata/ahci.h  | 2 +-
>>   3 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
>> index cb54631fd950..52c086f155f0 100644
>> --- a/drivers/ata/Kconfig
>> +++ b/drivers/ata/Kconfig
>> @@ -115,14 +115,14 @@ config SATA_AHCI
>>   
>>   	  If unsure, say N.
>>   
>> -config SATA_MOBILE_LPM_POLICY
>> -	int "Default SATA Link Power Management policy for mobile chipsets"
>> +config SATA_LPM_POLICY
> 
> Maybe "config SATA_DEFAULT_LPM_POLICY" at least that matches the summary text better ?
> 
>> +	int "Default SATA Link Power Management policy for supported chipsets"
>>   	range 0 4
>>   	default 0
>>   	depends on SATA_AHCI
>>   	help
>>   	  Select the Default SATA Link Power Management (LPM) policy to use
>> -	  for mobile / laptop variants of chipsets / "South Bridges".
>> +	  for certain chipsets / "South Bridges".
> 
> Maybe:
> 
> for chipsets where using a different default policy then max_performance
> is deemed desirable.
> 
> Yeah no, that IMHO is no good either, but "certain" also is very much
> not helpful in this help text.
> 
> Looking at this patch I'm starting to think that maybe the maybe mobile was not
> that bad of a name after all :)
> 
> Maybe for v2 do s/mobile/low_power/ in each of the patches instead?

Sure, I'll do that and see how everything looks and reads.

> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>>   
>>   	  The value set has the following meanings:
>>   		0 => Keep firmware settings
>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>> index 5be889de9b1e..7a7fa1090bb0 100644
>> --- a/drivers/ata/ahci.c
>> +++ b/drivers/ata/ahci.c
>> @@ -1592,7 +1592,7 @@ static int ahci_init_msi(struct pci_dev *pdev, unsigned int n_ports,
>>   static void ahci_update_initial_lpm_policy(struct ata_port *ap,
>>   					   struct ahci_host_priv *hpriv)
>>   {
>> -	int policy = CONFIG_SATA_MOBILE_LPM_POLICY;
>> +	int policy = CONFIG_SATA_LPM_POLICY;
>>   
>>   
>>   	/* Ignore processing for chipsets that don't use policy */
>> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
>> index 1ad48e2fe573..5badbaca05a0 100644
>> --- a/drivers/ata/ahci.h
>> +++ b/drivers/ata/ahci.h
>> @@ -236,7 +236,7 @@ enum {
>>   	AHCI_HFLAG_NO_WRITE_TO_RO	= (1 << 24), /* don't write to read
>>   							only registers */
>>   	AHCI_HFLAG_USE_LPM_POLICY	= (1 << 25), /* chipset that should use
>> -							SATA_MOBILE_LPM_POLICY
>> +							SATA_LPM_POLICY
>>   							as default lpm_policy */
>>   	AHCI_HFLAG_SUSPEND_PHYS		= (1 << 26), /* handle PHYs during
>>   							suspend/resume */
>>
>