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

[Limonciello, Mario]

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