* [PATCH v2 2/3] ata: ahci: Rename `AHCI_HFLAG_IS_MOBILE`
2022-02-25 6:11 [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile Mario Limonciello
@ 2022-02-25 6:11 ` Mario Limonciello
2022-02-25 6:11 ` [PATCH v2 3/3] ata: ahci: Rename CONFIG_SATA_LPM_MOBILE_POLICY configuration item Mario Limonciello
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Mario Limonciello @ 2022-02-25 6:11 UTC (permalink / raw)
To: Damien Le Moal
Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel, hdegoede, Mario Limonciello
`AHCI_HFLAG_IS_MOBILE` designates that a chipset should be using the
default link power management policy from a kernel configuration item.
As desktop chipsets may also be interested in this default policy
configuration, rename the flag to `AHCI_HFLAG_USE_LPM_POLICY` to more
accurately reflect that a chipset doesn't have to be mobile to adopt it.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
changes from v1->v2:
* rebase on changes from first patch
drivers/ata/ahci.c | 6 +++---
drivers/ata/ahci.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 995ef962eb92..a77e8154b5f0 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -136,7 +136,7 @@ static const struct ata_port_info ahci_port_info[] = {
.port_ops = &ahci_ops,
},
[board_ahci_low_power] = {
- AHCI_HFLAGS (AHCI_HFLAG_IS_MOBILE),
+ AHCI_HFLAGS (AHCI_HFLAG_USE_LPM_POLICY),
.flags = AHCI_FLAG_COMMON,
.pio_mask = ATA_PIO4,
.udma_mask = ATA_UDMA6,
@@ -1595,8 +1595,8 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap,
int policy = CONFIG_SATA_MOBILE_LPM_POLICY;
- /* Ignore processing for non mobile platforms */
- if (!(hpriv->flags & AHCI_HFLAG_IS_MOBILE))
+ /* Ignore processing for chipsets that don't use policy */
+ if (!(hpriv->flags & AHCI_HFLAG_USE_LPM_POLICY))
return;
/* user modified policy via module param */
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index eeac5482f1d1..1ad48e2fe573 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -235,7 +235,7 @@ enum {
AHCI_HFLAG_YES_ALPM = (1 << 23), /* force ALPM cap on */
AHCI_HFLAG_NO_WRITE_TO_RO = (1 << 24), /* don't write to read
only registers */
- AHCI_HFLAG_IS_MOBILE = (1 << 25), /* mobile chipset, use
+ AHCI_HFLAG_USE_LPM_POLICY = (1 << 25), /* chipset that should use
SATA_MOBILE_LPM_POLICY
as default lpm_policy */
AHCI_HFLAG_SUSPEND_PHYS = (1 << 26), /* handle PHYs during
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 3/3] ata: ahci: Rename CONFIG_SATA_LPM_MOBILE_POLICY configuration item
2022-02-25 6:11 [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile Mario Limonciello
2022-02-25 6:11 ` [PATCH v2 2/3] ata: ahci: Rename `AHCI_HFLAG_IS_MOBILE` Mario Limonciello
@ 2022-02-25 6:11 ` Mario Limonciello
2022-02-25 6:33 ` [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile Damien Le Moal
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Mario Limonciello @ 2022-02-25 6:11 UTC (permalink / raw)
To: Damien Le Moal
Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel, hdegoede, Mario Limonciello
`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>
---
changes from v1->v2:
* Reword documentation in Kconfig
* Change configuration item to match earlier patches
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..cb9e71b4ca4d 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
+ int "Default SATA Link Power Management policy for low power 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 chipsets / "South Bridges" designated as supporting low power.
The value set has the following meanings:
0 => Keep firmware settings
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index a77e8154b5f0..b84ba95be84a 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 */
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
2022-02-25 6:11 [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile Mario Limonciello
2022-02-25 6:11 ` [PATCH v2 2/3] ata: ahci: Rename `AHCI_HFLAG_IS_MOBILE` Mario Limonciello
2022-02-25 6:11 ` [PATCH v2 3/3] ata: ahci: Rename CONFIG_SATA_LPM_MOBILE_POLICY configuration item Mario Limonciello
@ 2022-02-25 6:33 ` Damien Le Moal
2022-02-25 9:24 ` Hans de Goede
2022-02-25 16:01 ` Christoph Hellwig
4 siblings, 0 replies; 13+ messages in thread
From: Damien Le Moal @ 2022-02-25 6:33 UTC (permalink / raw)
To: Mario Limonciello
Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel, hdegoede
On 2/25/22 15:11, Mario Limonciello wrote:
> This board definition was originally created for mobile devices to
> designate default link power managmeent policy to influence runtime
> power consumption.
>
> As this is interesting for more than just mobile designs, rename the
> board to `board_ahci_low_power` to make it clear it is about default
> policy.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> changes from v1->v2:
> * Rename to board_ahci_low_power
>
> drivers/ata/ahci.c | 96 +++++++++++++++++++++++-----------------------
> 1 file changed, 48 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index ab5811ef5a53..995ef962eb92 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -49,8 +49,8 @@ enum {
> enum board_ids {
> /* board IDs by feature in alphabetical order */
> board_ahci,
> + board_ahci_low_power,
Nit: please keep the alphabetical order.
> board_ahci_ign_iferr,
> - board_ahci_mobile,
> board_ahci_no_debounce_delay,
> board_ahci_nomsi,
> board_ahci_noncq,
> @@ -135,7 +135,7 @@ static const struct ata_port_info ahci_port_info[] = {
> .udma_mask = ATA_UDMA6,
> .port_ops = &ahci_ops,
> },
> - [board_ahci_mobile] = {
> + [board_ahci_low_power] = {
> AHCI_HFLAGS (AHCI_HFLAG_IS_MOBILE),
> .flags = AHCI_FLAG_COMMON,
> .pio_mask = ATA_PIO4,
> @@ -275,13 +275,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x2924), board_ahci }, /* ICH9 */
> { PCI_VDEVICE(INTEL, 0x2925), board_ahci }, /* ICH9 */
> { PCI_VDEVICE(INTEL, 0x2927), board_ahci }, /* ICH9 */
> - { PCI_VDEVICE(INTEL, 0x2929), board_ahci_mobile }, /* ICH9M */
> - { PCI_VDEVICE(INTEL, 0x292a), board_ahci_mobile }, /* ICH9M */
> - { PCI_VDEVICE(INTEL, 0x292b), board_ahci_mobile }, /* ICH9M */
> - { PCI_VDEVICE(INTEL, 0x292c), board_ahci_mobile }, /* ICH9M */
> - { PCI_VDEVICE(INTEL, 0x292f), board_ahci_mobile }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x2929), board_ahci_low_power }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x292a), board_ahci_low_power }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x292b), board_ahci_low_power }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x292c), board_ahci_low_power }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x292f), board_ahci_low_power }, /* ICH9M */
> { PCI_VDEVICE(INTEL, 0x294d), board_ahci }, /* ICH9 */
> - { PCI_VDEVICE(INTEL, 0x294e), board_ahci_mobile }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x294e), board_ahci_low_power }, /* ICH9M */
> { PCI_VDEVICE(INTEL, 0x502a), board_ahci }, /* Tolapai */
> { PCI_VDEVICE(INTEL, 0x502b), board_ahci }, /* Tolapai */
> { PCI_VDEVICE(INTEL, 0x3a05), board_ahci }, /* ICH10 */
> @@ -291,9 +291,9 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x3b23), board_ahci }, /* PCH AHCI */
> { PCI_VDEVICE(INTEL, 0x3b24), board_ahci }, /* PCH RAID */
> { PCI_VDEVICE(INTEL, 0x3b25), board_ahci }, /* PCH RAID */
> - { PCI_VDEVICE(INTEL, 0x3b29), board_ahci_mobile }, /* PCH M AHCI */
> + { PCI_VDEVICE(INTEL, 0x3b29), board_ahci_low_power }, /* PCH M AHCI */
> { PCI_VDEVICE(INTEL, 0x3b2b), board_ahci }, /* PCH RAID */
> - { PCI_VDEVICE(INTEL, 0x3b2c), board_ahci_mobile }, /* PCH M RAID */
> + { PCI_VDEVICE(INTEL, 0x3b2c), board_ahci_low_power }, /* PCH M RAID */
> { PCI_VDEVICE(INTEL, 0x3b2f), board_ahci }, /* PCH AHCI */
> { PCI_VDEVICE(INTEL, 0x19b0), board_ahci_pcs7 }, /* DNV AHCI */
> { PCI_VDEVICE(INTEL, 0x19b1), board_ahci_pcs7 }, /* DNV AHCI */
> @@ -316,9 +316,9 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x19cE), board_ahci_pcs7 }, /* DNV AHCI */
> { PCI_VDEVICE(INTEL, 0x19cF), board_ahci_pcs7 }, /* DNV AHCI */
> { PCI_VDEVICE(INTEL, 0x1c02), board_ahci }, /* CPT AHCI */
> - { PCI_VDEVICE(INTEL, 0x1c03), board_ahci_mobile }, /* CPT M AHCI */
> + { PCI_VDEVICE(INTEL, 0x1c03), board_ahci_low_power }, /* CPT M AHCI */
> { PCI_VDEVICE(INTEL, 0x1c04), board_ahci }, /* CPT RAID */
> - { PCI_VDEVICE(INTEL, 0x1c05), board_ahci_mobile }, /* CPT M RAID */
> + { PCI_VDEVICE(INTEL, 0x1c05), board_ahci_low_power }, /* CPT M RAID */
> { PCI_VDEVICE(INTEL, 0x1c06), board_ahci }, /* CPT RAID */
> { PCI_VDEVICE(INTEL, 0x1c07), board_ahci }, /* CPT RAID */
> { PCI_VDEVICE(INTEL, 0x1d02), board_ahci }, /* PBG AHCI */
> @@ -327,29 +327,29 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x2826), board_ahci }, /* PBG/Lewisburg RAID*/
> { PCI_VDEVICE(INTEL, 0x2323), board_ahci }, /* DH89xxCC AHCI */
> { PCI_VDEVICE(INTEL, 0x1e02), board_ahci }, /* Panther Point AHCI */
> - { PCI_VDEVICE(INTEL, 0x1e03), board_ahci_mobile }, /* Panther M AHCI */
> + { PCI_VDEVICE(INTEL, 0x1e03), board_ahci_low_power }, /* Panther M AHCI */
> { PCI_VDEVICE(INTEL, 0x1e04), board_ahci }, /* Panther Point RAID */
> { PCI_VDEVICE(INTEL, 0x1e05), board_ahci }, /* Panther Point RAID */
> { PCI_VDEVICE(INTEL, 0x1e06), board_ahci }, /* Panther Point RAID */
> - { PCI_VDEVICE(INTEL, 0x1e07), board_ahci_mobile }, /* Panther M RAID */
> + { PCI_VDEVICE(INTEL, 0x1e07), board_ahci_low_power }, /* Panther M RAID */
> { PCI_VDEVICE(INTEL, 0x1e0e), board_ahci }, /* Panther Point RAID */
> { PCI_VDEVICE(INTEL, 0x8c02), board_ahci }, /* Lynx Point AHCI */
> - { PCI_VDEVICE(INTEL, 0x8c03), board_ahci_mobile }, /* Lynx M AHCI */
> + { PCI_VDEVICE(INTEL, 0x8c03), board_ahci_low_power }, /* Lynx M AHCI */
> { PCI_VDEVICE(INTEL, 0x8c04), board_ahci }, /* Lynx Point RAID */
> - { PCI_VDEVICE(INTEL, 0x8c05), board_ahci_mobile }, /* Lynx M RAID */
> + { PCI_VDEVICE(INTEL, 0x8c05), board_ahci_low_power }, /* Lynx M RAID */
> { PCI_VDEVICE(INTEL, 0x8c06), board_ahci }, /* Lynx Point RAID */
> - { PCI_VDEVICE(INTEL, 0x8c07), board_ahci_mobile }, /* Lynx M RAID */
> + { PCI_VDEVICE(INTEL, 0x8c07), board_ahci_low_power }, /* Lynx M RAID */
> { PCI_VDEVICE(INTEL, 0x8c0e), board_ahci }, /* Lynx Point RAID */
> - { PCI_VDEVICE(INTEL, 0x8c0f), board_ahci_mobile }, /* Lynx M RAID */
> - { PCI_VDEVICE(INTEL, 0x9c02), board_ahci_mobile }, /* Lynx LP AHCI */
> - { PCI_VDEVICE(INTEL, 0x9c03), board_ahci_mobile }, /* Lynx LP AHCI */
> - { PCI_VDEVICE(INTEL, 0x9c04), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c05), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c06), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c07), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c0e), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c0f), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9dd3), board_ahci_mobile }, /* Cannon Lake PCH-LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x8c0f), board_ahci_low_power }, /* Lynx M RAID */
> + { PCI_VDEVICE(INTEL, 0x9c02), board_ahci_low_power }, /* Lynx LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x9c03), board_ahci_low_power }, /* Lynx LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x9c04), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c05), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c06), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c07), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c0e), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c0f), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9dd3), board_ahci_low_power }, /* Cannon Lake PCH-LP AHCI */
> { PCI_VDEVICE(INTEL, 0x1f22), board_ahci }, /* Avoton AHCI */
> { PCI_VDEVICE(INTEL, 0x1f23), board_ahci }, /* Avoton AHCI */
> { PCI_VDEVICE(INTEL, 0x1f24), board_ahci }, /* Avoton RAID */
> @@ -381,26 +381,26 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x8d66), board_ahci }, /* Wellsburg RAID */
> { PCI_VDEVICE(INTEL, 0x8d6e), board_ahci }, /* Wellsburg RAID */
> { PCI_VDEVICE(INTEL, 0x23a3), board_ahci }, /* Coleto Creek AHCI */
> - { PCI_VDEVICE(INTEL, 0x9c83), board_ahci_mobile }, /* Wildcat LP AHCI */
> - { PCI_VDEVICE(INTEL, 0x9c85), board_ahci_mobile }, /* Wildcat LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c87), board_ahci_mobile }, /* Wildcat LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c8f), board_ahci_mobile }, /* Wildcat LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c83), board_ahci_low_power }, /* Wildcat LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x9c85), board_ahci_low_power }, /* Wildcat LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c87), board_ahci_low_power }, /* Wildcat LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c8f), board_ahci_low_power }, /* Wildcat LP RAID */
> { PCI_VDEVICE(INTEL, 0x8c82), board_ahci }, /* 9 Series AHCI */
> - { PCI_VDEVICE(INTEL, 0x8c83), board_ahci_mobile }, /* 9 Series M AHCI */
> + { PCI_VDEVICE(INTEL, 0x8c83), board_ahci_low_power }, /* 9 Series M AHCI */
> { PCI_VDEVICE(INTEL, 0x8c84), board_ahci }, /* 9 Series RAID */
> - { PCI_VDEVICE(INTEL, 0x8c85), board_ahci_mobile }, /* 9 Series M RAID */
> + { PCI_VDEVICE(INTEL, 0x8c85), board_ahci_low_power }, /* 9 Series M RAID */
> { PCI_VDEVICE(INTEL, 0x8c86), board_ahci }, /* 9 Series RAID */
> - { PCI_VDEVICE(INTEL, 0x8c87), board_ahci_mobile }, /* 9 Series M RAID */
> + { PCI_VDEVICE(INTEL, 0x8c87), board_ahci_low_power }, /* 9 Series M RAID */
> { PCI_VDEVICE(INTEL, 0x8c8e), board_ahci }, /* 9 Series RAID */
> - { PCI_VDEVICE(INTEL, 0x8c8f), board_ahci_mobile }, /* 9 Series M RAID */
> - { PCI_VDEVICE(INTEL, 0x9d03), board_ahci_mobile }, /* Sunrise LP AHCI */
> - { PCI_VDEVICE(INTEL, 0x9d05), board_ahci_mobile }, /* Sunrise LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9d07), board_ahci_mobile }, /* Sunrise LP RAID */
> + { PCI_VDEVICE(INTEL, 0x8c8f), board_ahci_low_power }, /* 9 Series M RAID */
> + { PCI_VDEVICE(INTEL, 0x9d03), board_ahci_low_power }, /* Sunrise LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x9d05), board_ahci_low_power }, /* Sunrise LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9d07), board_ahci_low_power }, /* Sunrise LP RAID */
> { PCI_VDEVICE(INTEL, 0xa102), board_ahci }, /* Sunrise Point-H AHCI */
> - { PCI_VDEVICE(INTEL, 0xa103), board_ahci_mobile }, /* Sunrise M AHCI */
> + { PCI_VDEVICE(INTEL, 0xa103), board_ahci_low_power }, /* Sunrise M AHCI */
> { PCI_VDEVICE(INTEL, 0xa105), board_ahci }, /* Sunrise Point-H RAID */
> { PCI_VDEVICE(INTEL, 0xa106), board_ahci }, /* Sunrise Point-H RAID */
> - { PCI_VDEVICE(INTEL, 0xa107), board_ahci_mobile }, /* Sunrise M RAID */
> + { PCI_VDEVICE(INTEL, 0xa107), board_ahci_low_power }, /* Sunrise M RAID */
> { PCI_VDEVICE(INTEL, 0xa10f), board_ahci }, /* Sunrise Point-H RAID */
> { PCI_VDEVICE(INTEL, 0xa182), board_ahci }, /* Lewisburg AHCI*/
> { PCI_VDEVICE(INTEL, 0xa186), board_ahci }, /* Lewisburg RAID*/
> @@ -413,13 +413,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0xa356), board_ahci }, /* Cannon Lake PCH-H RAID */
> { PCI_VDEVICE(INTEL, 0x06d7), board_ahci }, /* Comet Lake-H RAID */
> { PCI_VDEVICE(INTEL, 0xa386), board_ahci }, /* Comet Lake PCH-V RAID */
> - { PCI_VDEVICE(INTEL, 0x0f22), board_ahci_mobile }, /* Bay Trail AHCI */
> - { PCI_VDEVICE(INTEL, 0x0f23), board_ahci_mobile }, /* Bay Trail AHCI */
> - { PCI_VDEVICE(INTEL, 0x22a3), board_ahci_mobile }, /* Cherry Tr. AHCI */
> - { PCI_VDEVICE(INTEL, 0x5ae3), board_ahci_mobile }, /* ApolloLake AHCI */
> - { PCI_VDEVICE(INTEL, 0x34d3), board_ahci_mobile }, /* Ice Lake LP AHCI */
> - { PCI_VDEVICE(INTEL, 0x02d3), board_ahci_mobile }, /* Comet Lake PCH-U AHCI */
> - { PCI_VDEVICE(INTEL, 0x02d7), board_ahci_mobile }, /* Comet Lake PCH RAID */
> + { PCI_VDEVICE(INTEL, 0x0f22), board_ahci_low_power }, /* Bay Trail AHCI */
> + { PCI_VDEVICE(INTEL, 0x0f23), board_ahci_low_power }, /* Bay Trail AHCI */
> + { PCI_VDEVICE(INTEL, 0x22a3), board_ahci_low_power }, /* Cherry Tr. AHCI */
> + { PCI_VDEVICE(INTEL, 0x5ae3), board_ahci_low_power }, /* ApolloLake AHCI */
> + { PCI_VDEVICE(INTEL, 0x34d3), board_ahci_low_power }, /* Ice Lake LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x02d3), board_ahci_low_power }, /* Comet Lake PCH-U AHCI */
> + { PCI_VDEVICE(INTEL, 0x02d7), board_ahci_low_power }, /* Comet Lake PCH RAID */
>
> /* JMicron 360/1/3/5/6, match class to avoid IDE function */
> { PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> @@ -447,7 +447,7 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(AMD, 0x7800), board_ahci }, /* AMD Hudson-2 */
> { PCI_VDEVICE(AMD, 0x7801), board_ahci_no_debounce_delay }, /* AMD Hudson-2 (AHCI mode) */
> { PCI_VDEVICE(AMD, 0x7900), board_ahci }, /* AMD CZ */
> - { PCI_VDEVICE(AMD, 0x7901), board_ahci_mobile }, /* AMD Green Sardine */
> + { PCI_VDEVICE(AMD, 0x7901), board_ahci_low_power }, /* AMD Green Sardine */
> /* AMD is using RAID class only for ahci controllers */
> { PCI_VENDOR_ID_AMD, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> PCI_CLASS_STORAGE_RAID << 8, 0xffffff, board_ahci },
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
2022-02-25 6:11 [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile Mario Limonciello
` (2 preceding siblings ...)
2022-02-25 6:33 ` [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile Damien Le Moal
@ 2022-02-25 9:24 ` Hans de Goede
2022-02-25 16:01 ` Christoph Hellwig
4 siblings, 0 replies; 13+ messages in thread
From: Hans de Goede @ 2022-02-25 9:24 UTC (permalink / raw)
To: Mario Limonciello, Damien Le Moal
Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel
Hi Mario,
On 2/25/22 07:11, Mario Limonciello wrote:
> This board definition was originally created for mobile devices to
> designate default link power managmeent policy to influence runtime
> power consumption.
>
> As this is interesting for more than just mobile designs, rename the
> board to `board_ahci_low_power` to make it clear it is about default
> policy.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Thanks, the entire series looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
for the series.
Regards,
Hans
> ---
> changes from v1->v2:
> * Rename to board_ahci_low_power
>
> drivers/ata/ahci.c | 96 +++++++++++++++++++++++-----------------------
> 1 file changed, 48 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index ab5811ef5a53..995ef962eb92 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -49,8 +49,8 @@ enum {
> enum board_ids {
> /* board IDs by feature in alphabetical order */
> board_ahci,
> + board_ahci_low_power,
> board_ahci_ign_iferr,
> - board_ahci_mobile,
> board_ahci_no_debounce_delay,
> board_ahci_nomsi,
> board_ahci_noncq,
> @@ -135,7 +135,7 @@ static const struct ata_port_info ahci_port_info[] = {
> .udma_mask = ATA_UDMA6,
> .port_ops = &ahci_ops,
> },
> - [board_ahci_mobile] = {
> + [board_ahci_low_power] = {
> AHCI_HFLAGS (AHCI_HFLAG_IS_MOBILE),
> .flags = AHCI_FLAG_COMMON,
> .pio_mask = ATA_PIO4,
> @@ -275,13 +275,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x2924), board_ahci }, /* ICH9 */
> { PCI_VDEVICE(INTEL, 0x2925), board_ahci }, /* ICH9 */
> { PCI_VDEVICE(INTEL, 0x2927), board_ahci }, /* ICH9 */
> - { PCI_VDEVICE(INTEL, 0x2929), board_ahci_mobile }, /* ICH9M */
> - { PCI_VDEVICE(INTEL, 0x292a), board_ahci_mobile }, /* ICH9M */
> - { PCI_VDEVICE(INTEL, 0x292b), board_ahci_mobile }, /* ICH9M */
> - { PCI_VDEVICE(INTEL, 0x292c), board_ahci_mobile }, /* ICH9M */
> - { PCI_VDEVICE(INTEL, 0x292f), board_ahci_mobile }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x2929), board_ahci_low_power }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x292a), board_ahci_low_power }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x292b), board_ahci_low_power }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x292c), board_ahci_low_power }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x292f), board_ahci_low_power }, /* ICH9M */
> { PCI_VDEVICE(INTEL, 0x294d), board_ahci }, /* ICH9 */
> - { PCI_VDEVICE(INTEL, 0x294e), board_ahci_mobile }, /* ICH9M */
> + { PCI_VDEVICE(INTEL, 0x294e), board_ahci_low_power }, /* ICH9M */
> { PCI_VDEVICE(INTEL, 0x502a), board_ahci }, /* Tolapai */
> { PCI_VDEVICE(INTEL, 0x502b), board_ahci }, /* Tolapai */
> { PCI_VDEVICE(INTEL, 0x3a05), board_ahci }, /* ICH10 */
> @@ -291,9 +291,9 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x3b23), board_ahci }, /* PCH AHCI */
> { PCI_VDEVICE(INTEL, 0x3b24), board_ahci }, /* PCH RAID */
> { PCI_VDEVICE(INTEL, 0x3b25), board_ahci }, /* PCH RAID */
> - { PCI_VDEVICE(INTEL, 0x3b29), board_ahci_mobile }, /* PCH M AHCI */
> + { PCI_VDEVICE(INTEL, 0x3b29), board_ahci_low_power }, /* PCH M AHCI */
> { PCI_VDEVICE(INTEL, 0x3b2b), board_ahci }, /* PCH RAID */
> - { PCI_VDEVICE(INTEL, 0x3b2c), board_ahci_mobile }, /* PCH M RAID */
> + { PCI_VDEVICE(INTEL, 0x3b2c), board_ahci_low_power }, /* PCH M RAID */
> { PCI_VDEVICE(INTEL, 0x3b2f), board_ahci }, /* PCH AHCI */
> { PCI_VDEVICE(INTEL, 0x19b0), board_ahci_pcs7 }, /* DNV AHCI */
> { PCI_VDEVICE(INTEL, 0x19b1), board_ahci_pcs7 }, /* DNV AHCI */
> @@ -316,9 +316,9 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x19cE), board_ahci_pcs7 }, /* DNV AHCI */
> { PCI_VDEVICE(INTEL, 0x19cF), board_ahci_pcs7 }, /* DNV AHCI */
> { PCI_VDEVICE(INTEL, 0x1c02), board_ahci }, /* CPT AHCI */
> - { PCI_VDEVICE(INTEL, 0x1c03), board_ahci_mobile }, /* CPT M AHCI */
> + { PCI_VDEVICE(INTEL, 0x1c03), board_ahci_low_power }, /* CPT M AHCI */
> { PCI_VDEVICE(INTEL, 0x1c04), board_ahci }, /* CPT RAID */
> - { PCI_VDEVICE(INTEL, 0x1c05), board_ahci_mobile }, /* CPT M RAID */
> + { PCI_VDEVICE(INTEL, 0x1c05), board_ahci_low_power }, /* CPT M RAID */
> { PCI_VDEVICE(INTEL, 0x1c06), board_ahci }, /* CPT RAID */
> { PCI_VDEVICE(INTEL, 0x1c07), board_ahci }, /* CPT RAID */
> { PCI_VDEVICE(INTEL, 0x1d02), board_ahci }, /* PBG AHCI */
> @@ -327,29 +327,29 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x2826), board_ahci }, /* PBG/Lewisburg RAID*/
> { PCI_VDEVICE(INTEL, 0x2323), board_ahci }, /* DH89xxCC AHCI */
> { PCI_VDEVICE(INTEL, 0x1e02), board_ahci }, /* Panther Point AHCI */
> - { PCI_VDEVICE(INTEL, 0x1e03), board_ahci_mobile }, /* Panther M AHCI */
> + { PCI_VDEVICE(INTEL, 0x1e03), board_ahci_low_power }, /* Panther M AHCI */
> { PCI_VDEVICE(INTEL, 0x1e04), board_ahci }, /* Panther Point RAID */
> { PCI_VDEVICE(INTEL, 0x1e05), board_ahci }, /* Panther Point RAID */
> { PCI_VDEVICE(INTEL, 0x1e06), board_ahci }, /* Panther Point RAID */
> - { PCI_VDEVICE(INTEL, 0x1e07), board_ahci_mobile }, /* Panther M RAID */
> + { PCI_VDEVICE(INTEL, 0x1e07), board_ahci_low_power }, /* Panther M RAID */
> { PCI_VDEVICE(INTEL, 0x1e0e), board_ahci }, /* Panther Point RAID */
> { PCI_VDEVICE(INTEL, 0x8c02), board_ahci }, /* Lynx Point AHCI */
> - { PCI_VDEVICE(INTEL, 0x8c03), board_ahci_mobile }, /* Lynx M AHCI */
> + { PCI_VDEVICE(INTEL, 0x8c03), board_ahci_low_power }, /* Lynx M AHCI */
> { PCI_VDEVICE(INTEL, 0x8c04), board_ahci }, /* Lynx Point RAID */
> - { PCI_VDEVICE(INTEL, 0x8c05), board_ahci_mobile }, /* Lynx M RAID */
> + { PCI_VDEVICE(INTEL, 0x8c05), board_ahci_low_power }, /* Lynx M RAID */
> { PCI_VDEVICE(INTEL, 0x8c06), board_ahci }, /* Lynx Point RAID */
> - { PCI_VDEVICE(INTEL, 0x8c07), board_ahci_mobile }, /* Lynx M RAID */
> + { PCI_VDEVICE(INTEL, 0x8c07), board_ahci_low_power }, /* Lynx M RAID */
> { PCI_VDEVICE(INTEL, 0x8c0e), board_ahci }, /* Lynx Point RAID */
> - { PCI_VDEVICE(INTEL, 0x8c0f), board_ahci_mobile }, /* Lynx M RAID */
> - { PCI_VDEVICE(INTEL, 0x9c02), board_ahci_mobile }, /* Lynx LP AHCI */
> - { PCI_VDEVICE(INTEL, 0x9c03), board_ahci_mobile }, /* Lynx LP AHCI */
> - { PCI_VDEVICE(INTEL, 0x9c04), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c05), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c06), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c07), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c0e), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c0f), board_ahci_mobile }, /* Lynx LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9dd3), board_ahci_mobile }, /* Cannon Lake PCH-LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x8c0f), board_ahci_low_power }, /* Lynx M RAID */
> + { PCI_VDEVICE(INTEL, 0x9c02), board_ahci_low_power }, /* Lynx LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x9c03), board_ahci_low_power }, /* Lynx LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x9c04), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c05), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c06), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c07), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c0e), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c0f), board_ahci_low_power }, /* Lynx LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9dd3), board_ahci_low_power }, /* Cannon Lake PCH-LP AHCI */
> { PCI_VDEVICE(INTEL, 0x1f22), board_ahci }, /* Avoton AHCI */
> { PCI_VDEVICE(INTEL, 0x1f23), board_ahci }, /* Avoton AHCI */
> { PCI_VDEVICE(INTEL, 0x1f24), board_ahci }, /* Avoton RAID */
> @@ -381,26 +381,26 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x8d66), board_ahci }, /* Wellsburg RAID */
> { PCI_VDEVICE(INTEL, 0x8d6e), board_ahci }, /* Wellsburg RAID */
> { PCI_VDEVICE(INTEL, 0x23a3), board_ahci }, /* Coleto Creek AHCI */
> - { PCI_VDEVICE(INTEL, 0x9c83), board_ahci_mobile }, /* Wildcat LP AHCI */
> - { PCI_VDEVICE(INTEL, 0x9c85), board_ahci_mobile }, /* Wildcat LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c87), board_ahci_mobile }, /* Wildcat LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9c8f), board_ahci_mobile }, /* Wildcat LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c83), board_ahci_low_power }, /* Wildcat LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x9c85), board_ahci_low_power }, /* Wildcat LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c87), board_ahci_low_power }, /* Wildcat LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9c8f), board_ahci_low_power }, /* Wildcat LP RAID */
> { PCI_VDEVICE(INTEL, 0x8c82), board_ahci }, /* 9 Series AHCI */
> - { PCI_VDEVICE(INTEL, 0x8c83), board_ahci_mobile }, /* 9 Series M AHCI */
> + { PCI_VDEVICE(INTEL, 0x8c83), board_ahci_low_power }, /* 9 Series M AHCI */
> { PCI_VDEVICE(INTEL, 0x8c84), board_ahci }, /* 9 Series RAID */
> - { PCI_VDEVICE(INTEL, 0x8c85), board_ahci_mobile }, /* 9 Series M RAID */
> + { PCI_VDEVICE(INTEL, 0x8c85), board_ahci_low_power }, /* 9 Series M RAID */
> { PCI_VDEVICE(INTEL, 0x8c86), board_ahci }, /* 9 Series RAID */
> - { PCI_VDEVICE(INTEL, 0x8c87), board_ahci_mobile }, /* 9 Series M RAID */
> + { PCI_VDEVICE(INTEL, 0x8c87), board_ahci_low_power }, /* 9 Series M RAID */
> { PCI_VDEVICE(INTEL, 0x8c8e), board_ahci }, /* 9 Series RAID */
> - { PCI_VDEVICE(INTEL, 0x8c8f), board_ahci_mobile }, /* 9 Series M RAID */
> - { PCI_VDEVICE(INTEL, 0x9d03), board_ahci_mobile }, /* Sunrise LP AHCI */
> - { PCI_VDEVICE(INTEL, 0x9d05), board_ahci_mobile }, /* Sunrise LP RAID */
> - { PCI_VDEVICE(INTEL, 0x9d07), board_ahci_mobile }, /* Sunrise LP RAID */
> + { PCI_VDEVICE(INTEL, 0x8c8f), board_ahci_low_power }, /* 9 Series M RAID */
> + { PCI_VDEVICE(INTEL, 0x9d03), board_ahci_low_power }, /* Sunrise LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x9d05), board_ahci_low_power }, /* Sunrise LP RAID */
> + { PCI_VDEVICE(INTEL, 0x9d07), board_ahci_low_power }, /* Sunrise LP RAID */
> { PCI_VDEVICE(INTEL, 0xa102), board_ahci }, /* Sunrise Point-H AHCI */
> - { PCI_VDEVICE(INTEL, 0xa103), board_ahci_mobile }, /* Sunrise M AHCI */
> + { PCI_VDEVICE(INTEL, 0xa103), board_ahci_low_power }, /* Sunrise M AHCI */
> { PCI_VDEVICE(INTEL, 0xa105), board_ahci }, /* Sunrise Point-H RAID */
> { PCI_VDEVICE(INTEL, 0xa106), board_ahci }, /* Sunrise Point-H RAID */
> - { PCI_VDEVICE(INTEL, 0xa107), board_ahci_mobile }, /* Sunrise M RAID */
> + { PCI_VDEVICE(INTEL, 0xa107), board_ahci_low_power }, /* Sunrise M RAID */
> { PCI_VDEVICE(INTEL, 0xa10f), board_ahci }, /* Sunrise Point-H RAID */
> { PCI_VDEVICE(INTEL, 0xa182), board_ahci }, /* Lewisburg AHCI*/
> { PCI_VDEVICE(INTEL, 0xa186), board_ahci }, /* Lewisburg RAID*/
> @@ -413,13 +413,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0xa356), board_ahci }, /* Cannon Lake PCH-H RAID */
> { PCI_VDEVICE(INTEL, 0x06d7), board_ahci }, /* Comet Lake-H RAID */
> { PCI_VDEVICE(INTEL, 0xa386), board_ahci }, /* Comet Lake PCH-V RAID */
> - { PCI_VDEVICE(INTEL, 0x0f22), board_ahci_mobile }, /* Bay Trail AHCI */
> - { PCI_VDEVICE(INTEL, 0x0f23), board_ahci_mobile }, /* Bay Trail AHCI */
> - { PCI_VDEVICE(INTEL, 0x22a3), board_ahci_mobile }, /* Cherry Tr. AHCI */
> - { PCI_VDEVICE(INTEL, 0x5ae3), board_ahci_mobile }, /* ApolloLake AHCI */
> - { PCI_VDEVICE(INTEL, 0x34d3), board_ahci_mobile }, /* Ice Lake LP AHCI */
> - { PCI_VDEVICE(INTEL, 0x02d3), board_ahci_mobile }, /* Comet Lake PCH-U AHCI */
> - { PCI_VDEVICE(INTEL, 0x02d7), board_ahci_mobile }, /* Comet Lake PCH RAID */
> + { PCI_VDEVICE(INTEL, 0x0f22), board_ahci_low_power }, /* Bay Trail AHCI */
> + { PCI_VDEVICE(INTEL, 0x0f23), board_ahci_low_power }, /* Bay Trail AHCI */
> + { PCI_VDEVICE(INTEL, 0x22a3), board_ahci_low_power }, /* Cherry Tr. AHCI */
> + { PCI_VDEVICE(INTEL, 0x5ae3), board_ahci_low_power }, /* ApolloLake AHCI */
> + { PCI_VDEVICE(INTEL, 0x34d3), board_ahci_low_power }, /* Ice Lake LP AHCI */
> + { PCI_VDEVICE(INTEL, 0x02d3), board_ahci_low_power }, /* Comet Lake PCH-U AHCI */
> + { PCI_VDEVICE(INTEL, 0x02d7), board_ahci_low_power }, /* Comet Lake PCH RAID */
>
> /* JMicron 360/1/3/5/6, match class to avoid IDE function */
> { PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> @@ -447,7 +447,7 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(AMD, 0x7800), board_ahci }, /* AMD Hudson-2 */
> { PCI_VDEVICE(AMD, 0x7801), board_ahci_no_debounce_delay }, /* AMD Hudson-2 (AHCI mode) */
> { PCI_VDEVICE(AMD, 0x7900), board_ahci }, /* AMD CZ */
> - { PCI_VDEVICE(AMD, 0x7901), board_ahci_mobile }, /* AMD Green Sardine */
> + { PCI_VDEVICE(AMD, 0x7901), board_ahci_low_power }, /* AMD Green Sardine */
> /* AMD is using RAID class only for ahci controllers */
> { PCI_VENDOR_ID_AMD, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> PCI_CLASS_STORAGE_RAID << 8, 0xffffff, board_ahci },
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
2022-02-25 6:11 [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile Mario Limonciello
` (3 preceding siblings ...)
2022-02-25 9:24 ` Hans de Goede
@ 2022-02-25 16:01 ` Christoph Hellwig
2022-02-25 16:04 ` Limonciello, Mario
4 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2022-02-25 16:01 UTC (permalink / raw)
To: Mario Limonciello
Cc: Damien Le Moal,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel, hdegoede
On Fri, Feb 25, 2022 at 12:11:11AM -0600, Mario Limonciello wrote:
> This board definition was originally created for mobile devices to
> designate default link power managmeent policy to influence runtime
> power consumption.
>
> As this is interesting for more than just mobile designs, rename the
> board to `board_ahci_low_power` to make it clear it is about default
> policy.
Is there any good reason to not just apply the policy to all devices
by default?
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
2022-02-25 16:01 ` Christoph Hellwig
@ 2022-02-25 16:04 ` Limonciello, Mario
2022-02-25 16:16 ` Hans de Goede
0 siblings, 1 reply; 13+ messages in thread
From: Limonciello, Mario @ 2022-02-25 16:04 UTC (permalink / raw)
To: Christoph Hellwig, hdegoede@redhat.com
Cc: Damien Le Moal,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel@molgen.mpg.de
[Public]
> On Fri, Feb 25, 2022 at 12:11:11AM -0600, Mario Limonciello wrote:
> > This board definition was originally created for mobile devices to
> > designate default link power managmeent policy to influence runtime
> > power consumption.
> >
> > As this is interesting for more than just mobile designs, rename the
> > board to `board_ahci_low_power` to make it clear it is about default
> > policy.
>
> Is there any good reason to not just apply the policy to all devices
> by default?
That sure would make this all cleaner.
I think Hans knows more of the history here than anyone else. I had
presumed there was some data loss scenarios with some of the older
chipsets.
Hans?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
2022-02-25 16:04 ` Limonciello, Mario
@ 2022-02-25 16:16 ` Hans de Goede
2022-02-25 16:19 ` Limonciello, Mario
2022-02-25 16:21 ` Paul Menzel
0 siblings, 2 replies; 13+ messages in thread
From: Hans de Goede @ 2022-02-25 16:16 UTC (permalink / raw)
To: Limonciello, Mario, Christoph Hellwig
Cc: Damien Le Moal,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel@molgen.mpg.de
Hi,
On 2/25/22 17:04, Limonciello, Mario wrote:
> [Public]
>
>> On Fri, Feb 25, 2022 at 12:11:11AM -0600, Mario Limonciello wrote:
>>> This board definition was originally created for mobile devices to
>>> designate default link power managmeent policy to influence runtime
>>> power consumption.
>>>
>>> As this is interesting for more than just mobile designs, rename the
>>> board to `board_ahci_low_power` to make it clear it is about default
>>> policy.
>>
>> Is there any good reason to not just apply the policy to all devices
>> by default?
>
> That sure would make this all cleaner.
>
> I think Hans knows more of the history here than anyone else. I had
> presumed there was some data loss scenarios with some of the older
> chipsets.
When I first introduced this change there were reports of crashes and
data corruption caused by setting the policy to min_power, these were
tied to some motherboards and/or to some drives.
This is the whole reason why I only enabled this on a subset of all the
AHCI chipsets.
At least on devices with a chipset which is currently marked as
mobile, the motherboard specific issues could be fixed with a BIOS
update. But I doubt that similar BIOS fixes have also been rolled
out to all desktop boards (and have been applied by all users),
and I also don't know about older boards.
So enabling this on all chipsets is definitely not without risks.
Regards,
Hans
>
> Hans?
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
2022-02-25 16:16 ` Hans de Goede
@ 2022-02-25 16:19 ` Limonciello, Mario
2022-02-25 21:07 ` Hans de Goede
2022-02-25 16:21 ` Paul Menzel
1 sibling, 1 reply; 13+ messages in thread
From: Limonciello, Mario @ 2022-02-25 16:19 UTC (permalink / raw)
To: Hans de Goede, Christoph Hellwig
Cc: Damien Le Moal,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel@molgen.mpg.de
[Public]
> On 2/25/22 17:04, Limonciello, Mario wrote:
> > [Public]
> >
> >> On Fri, Feb 25, 2022 at 12:11:11AM -0600, Mario Limonciello wrote:
> >>> This board definition was originally created for mobile devices to
> >>> designate default link power managmeent policy to influence runtime
> >>> power consumption.
> >>>
> >>> As this is interesting for more than just mobile designs, rename the
> >>> board to `board_ahci_low_power` to make it clear it is about default
> >>> policy.
> >>
> >> Is there any good reason to not just apply the policy to all devices
> >> by default?
> >
> > That sure would make this all cleaner.
> >
> > I think Hans knows more of the history here than anyone else. I had
> > presumed there was some data loss scenarios with some of the older
> > chipsets.
>
> When I first introduced this change there were reports of crashes and
> data corruption caused by setting the policy to min_power, these were
> tied to some motherboards and/or to some drives.
>
> This is the whole reason why I only enabled this on a subset of all the
> AHCI chipsets.
>
> At least on devices with a chipset which is currently marked as
> mobile, the motherboard specific issues could be fixed with a BIOS
> update. But I doubt that similar BIOS fixes have also been rolled
> out to all desktop boards (and have been applied by all users),
> and I also don't know about older boards.
>
> So enabling this on all chipsets is definitely not without risks.
>
This was before min_power_with_partial and min_power_with_dipm
were introduced though right? Maybe another way to look at this
is to drop the policy min_power, which overall is dangerous.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
2022-02-25 16:19 ` Limonciello, Mario
@ 2022-02-25 21:07 ` Hans de Goede
2022-02-25 21:13 ` Limonciello, Mario
0 siblings, 1 reply; 13+ messages in thread
From: Hans de Goede @ 2022-02-25 21:07 UTC (permalink / raw)
To: Limonciello, Mario, Christoph Hellwig
Cc: Damien Le Moal,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel@molgen.mpg.de
Hi,
On 2/25/22 17:19, Limonciello, Mario wrote:
> [Public]
>
>> On 2/25/22 17:04, Limonciello, Mario wrote:
>>> [Public]
>>>
>>>> On Fri, Feb 25, 2022 at 12:11:11AM -0600, Mario Limonciello wrote:
>>>>> This board definition was originally created for mobile devices to
>>>>> designate default link power managmeent policy to influence runtime
>>>>> power consumption.
>>>>>
>>>>> As this is interesting for more than just mobile designs, rename the
>>>>> board to `board_ahci_low_power` to make it clear it is about default
>>>>> policy.
>>>>
>>>> Is there any good reason to not just apply the policy to all devices
>>>> by default?
>>>
>>> That sure would make this all cleaner.
>>>
>>> I think Hans knows more of the history here than anyone else. I had
>>> presumed there was some data loss scenarios with some of the older
>>> chipsets.
>>
>> When I first introduced this change there were reports of crashes and
>> data corruption caused by setting the policy to min_power, these were
>> tied to some motherboards and/or to some drives.
>>
>> This is the whole reason why I only enabled this on a subset of all the
>> AHCI chipsets.
>>
>> At least on devices with a chipset which is currently marked as
>> mobile, the motherboard specific issues could be fixed with a BIOS
>> update. But I doubt that similar BIOS fixes have also been rolled
>> out to all desktop boards (and have been applied by all users),
>> and I also don't know about older boards.
>>
>> So enabling this on all chipsets is definitely not without risks.
>>
>
> This was before min_power_with_partial and min_power_with_dipm
> were introduced though right?
The issues where some laptops needed BIOS updates was with fedora
using min_power_with_dipm as default for mobile chipsets.
> Maybe another way to look at this
> is to drop the policy min_power, which overall is dangerous.
Maybe, see above. I'm not going to block this if people want
to give this a try, but it is going to require someone keeping
a very close look at any issues popping up and we must be
prepared to roll-back the change if necessary.
Regards,
Hans
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
2022-02-25 21:07 ` Hans de Goede
@ 2022-02-25 21:13 ` Limonciello, Mario
2022-02-25 21:19 ` Hans de Goede
0 siblings, 1 reply; 13+ messages in thread
From: Limonciello, Mario @ 2022-02-25 21:13 UTC (permalink / raw)
To: Hans de Goede, Christoph Hellwig
Cc: Damien Le Moal,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel@molgen.mpg.de
[AMD Official Use Only]
> >>>> On Fri, Feb 25, 2022 at 12:11:11AM -0600, Mario Limonciello wrote:
> >>>>> This board definition was originally created for mobile devices to
> >>>>> designate default link power managmeent policy to influence runtime
> >>>>> power consumption.
> >>>>>
> >>>>> As this is interesting for more than just mobile designs, rename the
> >>>>> board to `board_ahci_low_power` to make it clear it is about default
> >>>>> policy.
> >>>>
> >>>> Is there any good reason to not just apply the policy to all devices
> >>>> by default?
> >>>
> >>> That sure would make this all cleaner.
> >>>
> >>> I think Hans knows more of the history here than anyone else. I had
> >>> presumed there was some data loss scenarios with some of the older
> >>> chipsets.
> >>
> >> When I first introduced this change there were reports of crashes and
> >> data corruption caused by setting the policy to min_power, these were
> >> tied to some motherboards and/or to some drives.
> >>
> >> This is the whole reason why I only enabled this on a subset of all the
> >> AHCI chipsets.
> >>
> >> At least on devices with a chipset which is currently marked as
> >> mobile, the motherboard specific issues could be fixed with a BIOS
> >> update. But I doubt that similar BIOS fixes have also been rolled
> >> out to all desktop boards (and have been applied by all users),
> >> and I also don't know about older boards.
> >>
> >> So enabling this on all chipsets is definitely not without risks.
> >>
> >
> > This was before min_power_with_partial and min_power_with_dipm
> > were introduced though right?
>
> The issues where some laptops needed BIOS updates was with fedora
> using min_power_with_dipm as default for mobile chipsets.
>
Do you know if the drives actually supported slumber and partial?
I wonder if that was the real problem that they were being set when
they shouldn't be.
I added something for this in 2/2 in the RFC series you can look at.
> > Maybe another way to look at this
> > is to drop the policy min_power, which overall is dangerous.
>
> Maybe, see above. I'm not going to block this if people want
> to give this a try, but it is going to require someone keeping
> a very close look at any issues popping up and we must be
> prepared to roll-back the change if necessary.
>
Per Paul's suggestion I sent out v3 of this series and then I sent
out a separate RFC series (you're on CC). For this type of
thing if y'all think it makes sense to do.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
2022-02-25 21:13 ` Limonciello, Mario
@ 2022-02-25 21:19 ` Hans de Goede
0 siblings, 0 replies; 13+ messages in thread
From: Hans de Goede @ 2022-02-25 21:19 UTC (permalink / raw)
To: Limonciello, Mario, Christoph Hellwig
Cc: Damien Le Moal,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, pmenzel@molgen.mpg.de
Hi,
On 2/25/22 22:13, Limonciello, Mario wrote:
> [AMD Official Use Only]
>
>>>>>> On Fri, Feb 25, 2022 at 12:11:11AM -0600, Mario Limonciello wrote:
>>>>>>> This board definition was originally created for mobile devices to
>>>>>>> designate default link power managmeent policy to influence runtime
>>>>>>> power consumption.
>>>>>>>
>>>>>>> As this is interesting for more than just mobile designs, rename the
>>>>>>> board to `board_ahci_low_power` to make it clear it is about default
>>>>>>> policy.
>>>>>>
>>>>>> Is there any good reason to not just apply the policy to all devices
>>>>>> by default?
>>>>>
>>>>> That sure would make this all cleaner.
>>>>>
>>>>> I think Hans knows more of the history here than anyone else. I had
>>>>> presumed there was some data loss scenarios with some of the older
>>>>> chipsets.
>>>>
>>>> When I first introduced this change there were reports of crashes and
>>>> data corruption caused by setting the policy to min_power, these were
>>>> tied to some motherboards and/or to some drives.
>>>>
>>>> This is the whole reason why I only enabled this on a subset of all the
>>>> AHCI chipsets.
>>>>
>>>> At least on devices with a chipset which is currently marked as
>>>> mobile, the motherboard specific issues could be fixed with a BIOS
>>>> update. But I doubt that similar BIOS fixes have also been rolled
>>>> out to all desktop boards (and have been applied by all users),
>>>> and I also don't know about older boards.
>>>>
>>>> So enabling this on all chipsets is definitely not without risks.
>>>>
>>>
>>> This was before min_power_with_partial and min_power_with_dipm
>>> were introduced though right?
>>
>> The issues where some laptops needed BIOS updates was with fedora
>> using min_power_with_dipm as default for mobile chipsets.
>>
>
> Do you know if the drives actually supported slumber and partial?
> I wonder if that was the real problem that they were being set when
> they shouldn't be.>
> I added something for this in 2/2 in the RFC series you can look at.
Fedora defaults to ATA_LPM_MED_POWER_WITH_DIPM so patch 2/2 is
a no-op on Fedora; and IIRC (it has been a long time) the
need for BIOS updates on some mobile devices was with
standard Fedora kernels / settings.
Regards,
Hans
>
>>> Maybe another way to look at this
>>> is to drop the policy min_power, which overall is dangerous.
>>
>> Maybe, see above. I'm not going to block this if people want
>> to give this a try, but it is going to require someone keeping
>> a very close look at any issues popping up and we must be
>> prepared to roll-back the change if necessary.
>>
>
> Per Paul's suggestion I sent out v3 of this series and then I sent
> out a separate RFC series (you're on CC). For this type of
> thing if y'all think it makes sense to do.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] ata: ahci: Rename board_ahci_mobile
2022-02-25 16:16 ` Hans de Goede
2022-02-25 16:19 ` Limonciello, Mario
@ 2022-02-25 16:21 ` Paul Menzel
1 sibling, 0 replies; 13+ messages in thread
From: Paul Menzel @ 2022-02-25 16:21 UTC (permalink / raw)
To: Hans de Goede, Christoph Hellwig
Cc: Mario Limonciello, Damien Le Moal, linux-ide, linux-kernel
Dear Hans, dear Christoph,
Am 25.02.22 um 17:16 schrieb Hans de Goede:
> Hi,
>
> On 2/25/22 17:04, Limonciello, Mario wrote:
>> [Public]
>>
>>> On Fri, Feb 25, 2022 at 12:11:11AM -0600, Mario Limonciello wrote:
>>>> This board definition was originally created for mobile devices to
>>>> designate default link power managmeent policy to influence runtime
>>>> power consumption.
>>>>
>>>> As this is interesting for more than just mobile designs, rename the
>>>> board to `board_ahci_low_power` to make it clear it is about default
>>>> policy.
>>>
>>> Is there any good reason to not just apply the policy to all devices
>>> by default?
>>
>> That sure would make this all cleaner.
>>
>> I think Hans knows more of the history here than anyone else. I had
>> presumed there was some data loss scenarios with some of the older
>> chipsets.
>
> When I first introduced this change there were reports of crashes and
> data corruption caused by setting the policy to min_power, these were
> tied to some motherboards and/or to some drives.
>
> This is the whole reason why I only enabled this on a subset of all the
> AHCI chipsets.
>
> At least on devices with a chipset which is currently marked as
> mobile, the motherboard specific issues could be fixed with a BIOS
> update. But I doubt that similar BIOS fixes have also been rolled
> out to all desktop boards (and have been applied by all users),
> and I also don't know about older boards.
>
> So enabling this on all chipsets is definitely not without risks.
Exactly, even requiring to update the firmware would go against Linux’
no regression rule.
When new chipset are added from now on, we should ask the submitter to
test with LPM first though.
Mario’s patches look fine to me, and other changes should be done in
follow-up patches.
All are:
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 13+ messages in thread