linux-kernel - Re: [PATCH 3/3] ata: ahci: Rename CONFIG_SATA_LPM_MOBILE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fa6f84e9-3a17-c97d-285a-82051481ca11@amd.com>
Date:   Wed, 16 Feb 2022 07:41:08 -0600
From:   "Limonciello, Mario" <mario.limonciello@....com>
To:     Hans de Goede <hdegoede@...hat.com>,
        Damien Le Moal <damien.lemoal@...nsource.wdc.com>
Cc:     "open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers)" 
        <linux-ide@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>, pmenzel@...gen.mpg.de
Subject: Re: [PATCH 3/3] ata: ahci: Rename CONFIG_SATA_LPM_MOBILE_POLICY
 configuration item

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@....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 */
>>
>