| Message ID | 20220225061113.223920-1-mario.limonciello@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] ata: ahci: Rename board_ahci_mobile | expand |
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 },
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 },
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?
[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?
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? >
[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.
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
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
[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.
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.
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 },
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(-)