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

[Hans de Goede <hdegoede@redhat.com>]

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?

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