* [RFC 1/2] ata: ahci: Drop low power policy board type @ 2022-02-25 18:10 Mario Limonciello 2022-02-25 18:10 ` [RFC 2/2] ata: ahci: Protect users from setting policies their drives don't support Mario Limonciello 2022-02-25 21:13 ` [RFC 1/2] ata: ahci: Drop low power policy board type Hans de Goede 0 siblings, 2 replies; 6+ messages in thread From: Mario Limonciello @ 2022-02-25 18:10 UTC (permalink / raw) To: Damien Le Moal Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers), open list, hdegoede, Mario Limonciello, Christoph Hellwig The low power policy board type was introduced to allow systems to get into deep states reliably. Before it was introduced `min_power` was causing problems for a number of drives. New power policies `min_power_with_partial` and `med_power_with_dipm` have been introduced which provide a more stable baseline for systems. Suggested-by: Christoph Hellwig <hch@infradead.org> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/ata/Kconfig | 2 +- drivers/ata/ahci.c | 105 +++++++++++++++++++------------------------- drivers/ata/ahci.h | 9 ++-- 3 files changed, 50 insertions(+), 66 deletions(-) diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index cb9e71b4ca4d..3624d56d32b4 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -116,7 +116,7 @@ config SATA_AHCI If unsure, say N. config SATA_LPM_POLICY - int "Default SATA Link Power Management policy for low power chipsets" + int "Default SATA Link Power Management policy" range 0 4 default 0 depends on SATA_AHCI diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index ffaad9eaa79d..17d757ad7111 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -50,7 +50,6 @@ enum board_ids { /* board IDs by feature in alphabetical order */ board_ahci, board_ahci_ign_iferr, - board_ahci_low_power, board_ahci_no_debounce_delay, board_ahci_nomsi, board_ahci_noncq, @@ -135,13 +134,6 @@ static const struct ata_port_info ahci_port_info[] = { .udma_mask = ATA_UDMA6, .port_ops = &ahci_ops, }, - [board_ahci_low_power] = { - AHCI_HFLAGS (AHCI_HFLAG_USE_LPM_POLICY), - .flags = AHCI_FLAG_COMMON, - .pio_mask = ATA_PIO4, - .udma_mask = ATA_UDMA6, - .port_ops = &ahci_ops, - }, [board_ahci_no_debounce_delay] = { .flags = AHCI_FLAG_COMMON, .link_flags = ATA_LFLAG_NO_DEBOUNCE_DELAY, @@ -275,13 +267,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_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, 0x2929), board_ahci }, /* ICH9M */ + { PCI_VDEVICE(INTEL, 0x292a), board_ahci }, /* ICH9M */ + { PCI_VDEVICE(INTEL, 0x292b), board_ahci }, /* ICH9M */ + { PCI_VDEVICE(INTEL, 0x292c), board_ahci }, /* ICH9M */ + { PCI_VDEVICE(INTEL, 0x292f), board_ahci }, /* ICH9M */ { PCI_VDEVICE(INTEL, 0x294d), board_ahci }, /* ICH9 */ - { PCI_VDEVICE(INTEL, 0x294e), board_ahci_low_power }, /* ICH9M */ + { PCI_VDEVICE(INTEL, 0x294e), board_ahci }, /* ICH9M */ { PCI_VDEVICE(INTEL, 0x502a), board_ahci }, /* Tolapai */ { PCI_VDEVICE(INTEL, 0x502b), board_ahci }, /* Tolapai */ { PCI_VDEVICE(INTEL, 0x3a05), board_ahci }, /* ICH10 */ @@ -291,9 +283,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_low_power }, /* PCH M AHCI */ + { PCI_VDEVICE(INTEL, 0x3b29), board_ahci }, /* PCH M AHCI */ { PCI_VDEVICE(INTEL, 0x3b2b), board_ahci }, /* PCH RAID */ - { PCI_VDEVICE(INTEL, 0x3b2c), board_ahci_low_power }, /* PCH M RAID */ + { PCI_VDEVICE(INTEL, 0x3b2c), board_ahci }, /* 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 +308,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_low_power }, /* CPT M AHCI */ + { PCI_VDEVICE(INTEL, 0x1c03), board_ahci }, /* CPT M AHCI */ { PCI_VDEVICE(INTEL, 0x1c04), board_ahci }, /* CPT RAID */ - { PCI_VDEVICE(INTEL, 0x1c05), board_ahci_low_power }, /* CPT M RAID */ + { PCI_VDEVICE(INTEL, 0x1c05), board_ahci }, /* 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 +319,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_low_power }, /* Panther M AHCI */ + { PCI_VDEVICE(INTEL, 0x1e03), board_ahci }, /* 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_low_power }, /* Panther M RAID */ + { PCI_VDEVICE(INTEL, 0x1e07), board_ahci }, /* 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_low_power }, /* Lynx M AHCI */ + { PCI_VDEVICE(INTEL, 0x8c03), board_ahci }, /* Lynx M AHCI */ { PCI_VDEVICE(INTEL, 0x8c04), board_ahci }, /* Lynx Point RAID */ - { PCI_VDEVICE(INTEL, 0x8c05), board_ahci_low_power }, /* Lynx M RAID */ + { PCI_VDEVICE(INTEL, 0x8c05), board_ahci }, /* Lynx M RAID */ { PCI_VDEVICE(INTEL, 0x8c06), board_ahci }, /* Lynx Point RAID */ - { PCI_VDEVICE(INTEL, 0x8c07), board_ahci_low_power }, /* Lynx M RAID */ + { PCI_VDEVICE(INTEL, 0x8c07), board_ahci }, /* Lynx M RAID */ { PCI_VDEVICE(INTEL, 0x8c0e), board_ahci }, /* Lynx Point RAID */ - { 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, 0x8c0f), board_ahci }, /* Lynx M RAID */ + { PCI_VDEVICE(INTEL, 0x9c02), board_ahci }, /* Lynx LP AHCI */ + { PCI_VDEVICE(INTEL, 0x9c03), board_ahci }, /* Lynx LP AHCI */ + { PCI_VDEVICE(INTEL, 0x9c04), board_ahci }, /* Lynx LP RAID */ + { PCI_VDEVICE(INTEL, 0x9c05), board_ahci }, /* Lynx LP RAID */ + { PCI_VDEVICE(INTEL, 0x9c06), board_ahci }, /* Lynx LP RAID */ + { PCI_VDEVICE(INTEL, 0x9c07), board_ahci }, /* Lynx LP RAID */ + { PCI_VDEVICE(INTEL, 0x9c0e), board_ahci }, /* Lynx LP RAID */ + { PCI_VDEVICE(INTEL, 0x9c0f), board_ahci }, /* Lynx LP RAID */ + { PCI_VDEVICE(INTEL, 0x9dd3), board_ahci }, /* 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 +373,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_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, 0x9c83), board_ahci }, /* Wildcat LP AHCI */ + { PCI_VDEVICE(INTEL, 0x9c85), board_ahci }, /* Wildcat LP RAID */ + { PCI_VDEVICE(INTEL, 0x9c87), board_ahci }, /* Wildcat LP RAID */ + { PCI_VDEVICE(INTEL, 0x9c8f), board_ahci }, /* Wildcat LP RAID */ { PCI_VDEVICE(INTEL, 0x8c82), board_ahci }, /* 9 Series AHCI */ - { PCI_VDEVICE(INTEL, 0x8c83), board_ahci_low_power }, /* 9 Series M AHCI */ + { PCI_VDEVICE(INTEL, 0x8c83), board_ahci }, /* 9 Series M AHCI */ { PCI_VDEVICE(INTEL, 0x8c84), board_ahci }, /* 9 Series RAID */ - { PCI_VDEVICE(INTEL, 0x8c85), board_ahci_low_power }, /* 9 Series M RAID */ + { PCI_VDEVICE(INTEL, 0x8c85), board_ahci }, /* 9 Series M RAID */ { PCI_VDEVICE(INTEL, 0x8c86), board_ahci }, /* 9 Series RAID */ - { PCI_VDEVICE(INTEL, 0x8c87), board_ahci_low_power }, /* 9 Series M RAID */ + { PCI_VDEVICE(INTEL, 0x8c87), board_ahci }, /* 9 Series M RAID */ { PCI_VDEVICE(INTEL, 0x8c8e), board_ahci }, /* 9 Series 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, 0x8c8f), board_ahci }, /* 9 Series M RAID */ + { PCI_VDEVICE(INTEL, 0x9d03), board_ahci }, /* Sunrise LP AHCI */ + { PCI_VDEVICE(INTEL, 0x9d05), board_ahci }, /* Sunrise LP RAID */ + { PCI_VDEVICE(INTEL, 0x9d07), board_ahci }, /* Sunrise LP RAID */ { PCI_VDEVICE(INTEL, 0xa102), board_ahci }, /* Sunrise Point-H AHCI */ - { PCI_VDEVICE(INTEL, 0xa103), board_ahci_low_power }, /* Sunrise M AHCI */ + { PCI_VDEVICE(INTEL, 0xa103), board_ahci }, /* 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_low_power }, /* Sunrise M RAID */ + { PCI_VDEVICE(INTEL, 0xa107), board_ahci }, /* 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 +405,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_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 */ + { PCI_VDEVICE(INTEL, 0x0f22), board_ahci }, /* Bay Trail AHCI */ + { PCI_VDEVICE(INTEL, 0x0f23), board_ahci }, /* Bay Trail AHCI */ + { PCI_VDEVICE(INTEL, 0x22a3), board_ahci }, /* Cherry Tr. AHCI */ + { PCI_VDEVICE(INTEL, 0x5ae3), board_ahci }, /* ApolloLake AHCI */ + { PCI_VDEVICE(INTEL, 0x34d3), board_ahci }, /* Ice Lake LP AHCI */ + { PCI_VDEVICE(INTEL, 0x02d3), board_ahci }, /* Comet Lake PCH-U AHCI */ + { PCI_VDEVICE(INTEL, 0x02d7), board_ahci }, /* 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 +439,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_low_power }, /* AMD Green Sardine */ + { PCI_VDEVICE(AMD, 0x7901), board_ahci }, /* 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 }, @@ -1594,11 +1586,6 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap, { int policy = CONFIG_SATA_LPM_POLICY; - - /* Ignore processing for chipsets that don't use policy */ - if (!(hpriv->flags & AHCI_HFLAG_USE_LPM_POLICY)) - return; - /* user modified policy via module param */ if (mobile_lpm_policy != -1) { policy = mobile_lpm_policy; diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h index 5badbaca05a0..a2ca78e1df5c 100644 --- a/drivers/ata/ahci.h +++ b/drivers/ata/ahci.h @@ -235,14 +235,11 @@ 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_USE_LPM_POLICY = (1 << 25), /* chipset that should use - SATA_LPM_POLICY - as default lpm_policy */ - AHCI_HFLAG_SUSPEND_PHYS = (1 << 26), /* handle PHYs during + AHCI_HFLAG_SUSPEND_PHYS = (1 << 25), /* handle PHYs during suspend/resume */ - AHCI_HFLAG_IGN_NOTSUPP_POWER_ON = (1 << 27), /* ignore -EOPNOTSUPP + AHCI_HFLAG_IGN_NOTSUPP_POWER_ON = (1 << 26), /* ignore -EOPNOTSUPP from phy_power_on() */ - AHCI_HFLAG_NO_SXS = (1 << 28), /* SXS not supported */ + AHCI_HFLAG_NO_SXS = (1 << 27), /* SXS not supported */ /* ap->flags bits */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC 2/2] ata: ahci: Protect users from setting policies their drives don't support 2022-02-25 18:10 [RFC 1/2] ata: ahci: Drop low power policy board type Mario Limonciello @ 2022-02-25 18:10 ` Mario Limonciello 2022-02-25 21:20 ` Hans de Goede 2022-02-25 21:13 ` [RFC 1/2] ata: ahci: Drop low power policy board type Hans de Goede 1 sibling, 1 reply; 6+ messages in thread From: Mario Limonciello @ 2022-02-25 18:10 UTC (permalink / raw) To: Damien Le Moal Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers), open list, hdegoede, Mario Limonciello As the default low power policy applies to more chipsets and drives, it's important to make sure that drives actually support the policy that a user selected in their kernel configuration. If the drive doesn't support slumber, don't let the default policy for the ATA port be `min_power` or `min_power_with_partial`. Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/ata/ahci.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 17d757ad7111..af8999453084 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -1584,8 +1584,16 @@ 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) { + struct pci_dev *pdev = to_pci_dev(ap->host->dev); int policy = CONFIG_SATA_LPM_POLICY; + if (policy >= ATA_LPM_MIN_POWER_WITH_PARTIAL && + !(hpriv->cap & HOST_CAP_SSC)) { + dev_warn(&pdev->dev, + "This drive doesn't support slumber; ignoring default SATA policy\n"); + return; + } + /* user modified policy via module param */ if (mobile_lpm_policy != -1) { policy = mobile_lpm_policy; -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC 2/2] ata: ahci: Protect users from setting policies their drives don't support 2022-02-25 18:10 ` [RFC 2/2] ata: ahci: Protect users from setting policies their drives don't support Mario Limonciello @ 2022-02-25 21:20 ` Hans de Goede 2022-02-25 21:24 ` Limonciello, Mario 0 siblings, 1 reply; 6+ messages in thread From: Hans de Goede @ 2022-02-25 21:20 UTC (permalink / raw) To: Mario Limonciello, Damien Le Moal Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers), open list Hi, On 2/25/22 19:10, Mario Limonciello wrote: > As the default low power policy applies to more chipsets and drives, it's > important to make sure that drives actually support the policy that a user > selected in their kernel configuration. > > If the drive doesn't support slumber, don't let the default policy for the > ATA port be `min_power` or `min_power_with_partial`. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/ata/ahci.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c > index 17d757ad7111..af8999453084 100644 > --- a/drivers/ata/ahci.c > +++ b/drivers/ata/ahci.c > @@ -1584,8 +1584,16 @@ 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) > { > + struct pci_dev *pdev = to_pci_dev(ap->host->dev); > int policy = CONFIG_SATA_LPM_POLICY; > > + if (policy >= ATA_LPM_MIN_POWER_WITH_PARTIAL && > + !(hpriv->cap & HOST_CAP_SSC)) { > + dev_warn(&pdev->dev, > + "This drive doesn't support slumber; ignoring default SATA policy\n"); > + return; > + } > + Don't the capabilties get checked later when the policy gets applied ? At least I think they do get checked later, but I have not looked at this code for years ... ? Regards, Hans > /* user modified policy via module param */ > if (mobile_lpm_policy != -1) { > policy = mobile_lpm_policy; ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [RFC 2/2] ata: ahci: Protect users from setting policies their drives don't support 2022-02-25 21:20 ` Hans de Goede @ 2022-02-25 21:24 ` Limonciello, Mario 2022-02-26 11:23 ` Hans de Goede 0 siblings, 1 reply; 6+ messages in thread From: Limonciello, Mario @ 2022-02-25 21:24 UTC (permalink / raw) To: Hans de Goede, Damien Le Moal Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers), open list [AMD Official Use Only] > -----Original Message----- > From: Hans de Goede <hdegoede@redhat.com> > Sent: Friday, February 25, 2022 15:20 > To: Limonciello, Mario <Mario.Limonciello@amd.com>; Damien Le Moal > <damien.lemoal@opensource.wdc.com> > Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers) <linux- > ide@vger.kernel.org>; open list <linux-kernel@vger.kernel.org> > Subject: Re: [RFC 2/2] ata: ahci: Protect users from setting policies their > drives don't support > > Hi, > > On 2/25/22 19:10, Mario Limonciello wrote: > > As the default low power policy applies to more chipsets and drives, it's > > important to make sure that drives actually support the policy that a user > > selected in their kernel configuration. > > > > If the drive doesn't support slumber, don't let the default policy for the > > ATA port be `min_power` or `min_power_with_partial`. > > > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > > --- > > drivers/ata/ahci.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c > > index 17d757ad7111..af8999453084 100644 > > --- a/drivers/ata/ahci.c > > +++ b/drivers/ata/ahci.c > > @@ -1584,8 +1584,16 @@ 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) > > { > > + struct pci_dev *pdev = to_pci_dev(ap->host->dev); > > int policy = CONFIG_SATA_LPM_POLICY; > > > > + if (policy >= ATA_LPM_MIN_POWER_WITH_PARTIAL && > > + !(hpriv->cap & HOST_CAP_SSC)) { > > + dev_warn(&pdev->dev, > > + "This drive doesn't support slumber; ignoring default > SATA policy\n"); > > + return; > > + } > > + > > Don't the capabilties get checked later when the policy gets applied ? > > At least I think they do get checked later, but I have not looked > at this code for years ... ? There's a bunch of layers of indirection so I might have missed something, but I didn't see anything in sata_link_scr_lpm or anywhere else for that matter that actually checked HOST_CAP_SSC. > > Regards, > > Hans > > > > /* user modified policy via module param */ > > if (mobile_lpm_policy != -1) { > > policy = mobile_lpm_policy; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 2/2] ata: ahci: Protect users from setting policies their drives don't support 2022-02-25 21:24 ` Limonciello, Mario @ 2022-02-26 11:23 ` Hans de Goede 0 siblings, 0 replies; 6+ messages in thread From: Hans de Goede @ 2022-02-26 11:23 UTC (permalink / raw) To: Limonciello, Mario, Damien Le Moal Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers), open list Hi, On 2/25/22 22:24, Limonciello, Mario wrote: > [AMD Official Use Only] > > > >> -----Original Message----- >> From: Hans de Goede <hdegoede@redhat.com> >> Sent: Friday, February 25, 2022 15:20 >> To: Limonciello, Mario <Mario.Limonciello@amd.com>; Damien Le Moal >> <damien.lemoal@opensource.wdc.com> >> Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers) <linux- >> ide@vger.kernel.org>; open list <linux-kernel@vger.kernel.org> >> Subject: Re: [RFC 2/2] ata: ahci: Protect users from setting policies their >> drives don't support >> >> Hi, >> >> On 2/25/22 19:10, Mario Limonciello wrote: >>> As the default low power policy applies to more chipsets and drives, it's >>> important to make sure that drives actually support the policy that a user >>> selected in their kernel configuration. >>> >>> If the drive doesn't support slumber, don't let the default policy for the >>> ATA port be `min_power` or `min_power_with_partial`. >>> >>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >>> --- >>> drivers/ata/ahci.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c >>> index 17d757ad7111..af8999453084 100644 >>> --- a/drivers/ata/ahci.c >>> +++ b/drivers/ata/ahci.c >>> @@ -1584,8 +1584,16 @@ 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) >>> { >>> + struct pci_dev *pdev = to_pci_dev(ap->host->dev); >>> int policy = CONFIG_SATA_LPM_POLICY; >>> >>> + if (policy >= ATA_LPM_MIN_POWER_WITH_PARTIAL && >>> + !(hpriv->cap & HOST_CAP_SSC)) { >>> + dev_warn(&pdev->dev, >>> + "This drive doesn't support slumber; ignoring default >> SATA policy\n"); >>> + return; >>> + } >>> + >> >> Don't the capabilties get checked later when the policy gets applied ? >> >> At least I think they do get checked later, but I have not looked >> at this code for years ... ? > > There's a bunch of layers of indirection so I might have missed something, > but I didn't see anything in sata_link_scr_lpm or anywhere else for that > matter that actually checked HOST_CAP_SSC. Hmm, ok. Note that the user can still change the policy with an echo to sysfs. So I think it would be better to do a fix where HOST_CAP_SSC gets checked when the features are actually being enabled. Or at least also at a HOST_CAP_SSC check to the sysfs write functions. Regards, Hans >>> /* user modified policy via module param */ >>> if (mobile_lpm_policy != -1) { >>> policy = mobile_lpm_policy; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 1/2] ata: ahci: Drop low power policy board type 2022-02-25 18:10 [RFC 1/2] ata: ahci: Drop low power policy board type Mario Limonciello 2022-02-25 18:10 ` [RFC 2/2] ata: ahci: Protect users from setting policies their drives don't support Mario Limonciello @ 2022-02-25 21:13 ` Hans de Goede 1 sibling, 0 replies; 6+ messages in thread From: Hans de Goede @ 2022-02-25 21:13 UTC (permalink / raw) To: Mario Limonciello, Damien Le Moal Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers), open list, Christoph Hellwig Hi, On 2/25/22 19:10, Mario Limonciello wrote: > The low power policy board type was introduced to allow systems > to get into deep states reliably. Before it was introduced `min_power` > was causing problems for a number of drives. New power policies > `min_power_with_partial` and `med_power_with_dipm` have been introduced > which provide a more stable baseline for systems. > > Suggested-by: Christoph Hellwig <hch@infradead.org> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> As I already said in the other email thread about this: 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 > --- > drivers/ata/Kconfig | 2 +- > drivers/ata/ahci.c | 105 +++++++++++++++++++------------------------- > drivers/ata/ahci.h | 9 ++-- > 3 files changed, 50 insertions(+), 66 deletions(-) > > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig > index cb9e71b4ca4d..3624d56d32b4 100644 > --- a/drivers/ata/Kconfig > +++ b/drivers/ata/Kconfig > @@ -116,7 +116,7 @@ config SATA_AHCI > If unsure, say N. > > config SATA_LPM_POLICY > - int "Default SATA Link Power Management policy for low power chipsets" > + int "Default SATA Link Power Management policy" > range 0 4 > default 0 > depends on SATA_AHCI > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c > index ffaad9eaa79d..17d757ad7111 100644 > --- a/drivers/ata/ahci.c > +++ b/drivers/ata/ahci.c > @@ -50,7 +50,6 @@ enum board_ids { > /* board IDs by feature in alphabetical order */ > board_ahci, > board_ahci_ign_iferr, > - board_ahci_low_power, > board_ahci_no_debounce_delay, > board_ahci_nomsi, > board_ahci_noncq, > @@ -135,13 +134,6 @@ static const struct ata_port_info ahci_port_info[] = { > .udma_mask = ATA_UDMA6, > .port_ops = &ahci_ops, > }, > - [board_ahci_low_power] = { > - AHCI_HFLAGS (AHCI_HFLAG_USE_LPM_POLICY), > - .flags = AHCI_FLAG_COMMON, > - .pio_mask = ATA_PIO4, > - .udma_mask = ATA_UDMA6, > - .port_ops = &ahci_ops, > - }, > [board_ahci_no_debounce_delay] = { > .flags = AHCI_FLAG_COMMON, > .link_flags = ATA_LFLAG_NO_DEBOUNCE_DELAY, > @@ -275,13 +267,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_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, 0x2929), board_ahci }, /* ICH9M */ > + { PCI_VDEVICE(INTEL, 0x292a), board_ahci }, /* ICH9M */ > + { PCI_VDEVICE(INTEL, 0x292b), board_ahci }, /* ICH9M */ > + { PCI_VDEVICE(INTEL, 0x292c), board_ahci }, /* ICH9M */ > + { PCI_VDEVICE(INTEL, 0x292f), board_ahci }, /* ICH9M */ > { PCI_VDEVICE(INTEL, 0x294d), board_ahci }, /* ICH9 */ > - { PCI_VDEVICE(INTEL, 0x294e), board_ahci_low_power }, /* ICH9M */ > + { PCI_VDEVICE(INTEL, 0x294e), board_ahci }, /* ICH9M */ > { PCI_VDEVICE(INTEL, 0x502a), board_ahci }, /* Tolapai */ > { PCI_VDEVICE(INTEL, 0x502b), board_ahci }, /* Tolapai */ > { PCI_VDEVICE(INTEL, 0x3a05), board_ahci }, /* ICH10 */ > @@ -291,9 +283,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_low_power }, /* PCH M AHCI */ > + { PCI_VDEVICE(INTEL, 0x3b29), board_ahci }, /* PCH M AHCI */ > { PCI_VDEVICE(INTEL, 0x3b2b), board_ahci }, /* PCH RAID */ > - { PCI_VDEVICE(INTEL, 0x3b2c), board_ahci_low_power }, /* PCH M RAID */ > + { PCI_VDEVICE(INTEL, 0x3b2c), board_ahci }, /* 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 +308,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_low_power }, /* CPT M AHCI */ > + { PCI_VDEVICE(INTEL, 0x1c03), board_ahci }, /* CPT M AHCI */ > { PCI_VDEVICE(INTEL, 0x1c04), board_ahci }, /* CPT RAID */ > - { PCI_VDEVICE(INTEL, 0x1c05), board_ahci_low_power }, /* CPT M RAID */ > + { PCI_VDEVICE(INTEL, 0x1c05), board_ahci }, /* 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 +319,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_low_power }, /* Panther M AHCI */ > + { PCI_VDEVICE(INTEL, 0x1e03), board_ahci }, /* 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_low_power }, /* Panther M RAID */ > + { PCI_VDEVICE(INTEL, 0x1e07), board_ahci }, /* 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_low_power }, /* Lynx M AHCI */ > + { PCI_VDEVICE(INTEL, 0x8c03), board_ahci }, /* Lynx M AHCI */ > { PCI_VDEVICE(INTEL, 0x8c04), board_ahci }, /* Lynx Point RAID */ > - { PCI_VDEVICE(INTEL, 0x8c05), board_ahci_low_power }, /* Lynx M RAID */ > + { PCI_VDEVICE(INTEL, 0x8c05), board_ahci }, /* Lynx M RAID */ > { PCI_VDEVICE(INTEL, 0x8c06), board_ahci }, /* Lynx Point RAID */ > - { PCI_VDEVICE(INTEL, 0x8c07), board_ahci_low_power }, /* Lynx M RAID */ > + { PCI_VDEVICE(INTEL, 0x8c07), board_ahci }, /* Lynx M RAID */ > { PCI_VDEVICE(INTEL, 0x8c0e), board_ahci }, /* Lynx Point RAID */ > - { 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, 0x8c0f), board_ahci }, /* Lynx M RAID */ > + { PCI_VDEVICE(INTEL, 0x9c02), board_ahci }, /* Lynx LP AHCI */ > + { PCI_VDEVICE(INTEL, 0x9c03), board_ahci }, /* Lynx LP AHCI */ > + { PCI_VDEVICE(INTEL, 0x9c04), board_ahci }, /* Lynx LP RAID */ > + { PCI_VDEVICE(INTEL, 0x9c05), board_ahci }, /* Lynx LP RAID */ > + { PCI_VDEVICE(INTEL, 0x9c06), board_ahci }, /* Lynx LP RAID */ > + { PCI_VDEVICE(INTEL, 0x9c07), board_ahci }, /* Lynx LP RAID */ > + { PCI_VDEVICE(INTEL, 0x9c0e), board_ahci }, /* Lynx LP RAID */ > + { PCI_VDEVICE(INTEL, 0x9c0f), board_ahci }, /* Lynx LP RAID */ > + { PCI_VDEVICE(INTEL, 0x9dd3), board_ahci }, /* 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 +373,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_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, 0x9c83), board_ahci }, /* Wildcat LP AHCI */ > + { PCI_VDEVICE(INTEL, 0x9c85), board_ahci }, /* Wildcat LP RAID */ > + { PCI_VDEVICE(INTEL, 0x9c87), board_ahci }, /* Wildcat LP RAID */ > + { PCI_VDEVICE(INTEL, 0x9c8f), board_ahci }, /* Wildcat LP RAID */ > { PCI_VDEVICE(INTEL, 0x8c82), board_ahci }, /* 9 Series AHCI */ > - { PCI_VDEVICE(INTEL, 0x8c83), board_ahci_low_power }, /* 9 Series M AHCI */ > + { PCI_VDEVICE(INTEL, 0x8c83), board_ahci }, /* 9 Series M AHCI */ > { PCI_VDEVICE(INTEL, 0x8c84), board_ahci }, /* 9 Series RAID */ > - { PCI_VDEVICE(INTEL, 0x8c85), board_ahci_low_power }, /* 9 Series M RAID */ > + { PCI_VDEVICE(INTEL, 0x8c85), board_ahci }, /* 9 Series M RAID */ > { PCI_VDEVICE(INTEL, 0x8c86), board_ahci }, /* 9 Series RAID */ > - { PCI_VDEVICE(INTEL, 0x8c87), board_ahci_low_power }, /* 9 Series M RAID */ > + { PCI_VDEVICE(INTEL, 0x8c87), board_ahci }, /* 9 Series M RAID */ > { PCI_VDEVICE(INTEL, 0x8c8e), board_ahci }, /* 9 Series 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, 0x8c8f), board_ahci }, /* 9 Series M RAID */ > + { PCI_VDEVICE(INTEL, 0x9d03), board_ahci }, /* Sunrise LP AHCI */ > + { PCI_VDEVICE(INTEL, 0x9d05), board_ahci }, /* Sunrise LP RAID */ > + { PCI_VDEVICE(INTEL, 0x9d07), board_ahci }, /* Sunrise LP RAID */ > { PCI_VDEVICE(INTEL, 0xa102), board_ahci }, /* Sunrise Point-H AHCI */ > - { PCI_VDEVICE(INTEL, 0xa103), board_ahci_low_power }, /* Sunrise M AHCI */ > + { PCI_VDEVICE(INTEL, 0xa103), board_ahci }, /* 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_low_power }, /* Sunrise M RAID */ > + { PCI_VDEVICE(INTEL, 0xa107), board_ahci }, /* 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 +405,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_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 */ > + { PCI_VDEVICE(INTEL, 0x0f22), board_ahci }, /* Bay Trail AHCI */ > + { PCI_VDEVICE(INTEL, 0x0f23), board_ahci }, /* Bay Trail AHCI */ > + { PCI_VDEVICE(INTEL, 0x22a3), board_ahci }, /* Cherry Tr. AHCI */ > + { PCI_VDEVICE(INTEL, 0x5ae3), board_ahci }, /* ApolloLake AHCI */ > + { PCI_VDEVICE(INTEL, 0x34d3), board_ahci }, /* Ice Lake LP AHCI */ > + { PCI_VDEVICE(INTEL, 0x02d3), board_ahci }, /* Comet Lake PCH-U AHCI */ > + { PCI_VDEVICE(INTEL, 0x02d7), board_ahci }, /* 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 +439,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_low_power }, /* AMD Green Sardine */ > + { PCI_VDEVICE(AMD, 0x7901), board_ahci }, /* 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 }, > @@ -1594,11 +1586,6 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap, > { > int policy = CONFIG_SATA_LPM_POLICY; > > - > - /* Ignore processing for chipsets that don't use policy */ > - if (!(hpriv->flags & AHCI_HFLAG_USE_LPM_POLICY)) > - return; > - > /* user modified policy via module param */ > if (mobile_lpm_policy != -1) { > policy = mobile_lpm_policy; > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h > index 5badbaca05a0..a2ca78e1df5c 100644 > --- a/drivers/ata/ahci.h > +++ b/drivers/ata/ahci.h > @@ -235,14 +235,11 @@ 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_USE_LPM_POLICY = (1 << 25), /* chipset that should use > - SATA_LPM_POLICY > - as default lpm_policy */ > - AHCI_HFLAG_SUSPEND_PHYS = (1 << 26), /* handle PHYs during > + AHCI_HFLAG_SUSPEND_PHYS = (1 << 25), /* handle PHYs during > suspend/resume */ > - AHCI_HFLAG_IGN_NOTSUPP_POWER_ON = (1 << 27), /* ignore -EOPNOTSUPP > + AHCI_HFLAG_IGN_NOTSUPP_POWER_ON = (1 << 26), /* ignore -EOPNOTSUPP > from phy_power_on() */ > - AHCI_HFLAG_NO_SXS = (1 << 28), /* SXS not supported */ > + AHCI_HFLAG_NO_SXS = (1 << 27), /* SXS not supported */ > > /* ap->flags bits */ > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-26 11:23 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-25 18:10 [RFC 1/2] ata: ahci: Drop low power policy board type Mario Limonciello 2022-02-25 18:10 ` [RFC 2/2] ata: ahci: Protect users from setting policies their drives don't support Mario Limonciello 2022-02-25 21:20 ` Hans de Goede 2022-02-25 21:24 ` Limonciello, Mario 2022-02-26 11:23 ` Hans de Goede 2022-02-25 21:13 ` [RFC 1/2] ata: ahci: Drop low power policy board type Hans de Goede
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).