All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Drop mobile board support
@ 2022-05-24 17:05 Mario Limonciello
  2022-05-24 17:05 ` [PATCH 1/3] ata: ahci: Drop low power policy board type Mario Limonciello
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Mario Limonciello @ 2022-05-24 17:05 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list, hdegoede, Mario Limonciello

The AHCI driver currently has a large list of boards that are distinguished
as mobile or not mobile controlling LPM policy.  This approach hasn't
scaled very well as new boards need to be added and all new boards do
support LPM.

Furthermore it was shown recently in some bug reports that some of the
controllers included in this list actually are used in multiple
products spanning different product types.  This means that the
assumptions about "mobile" or "low-power" support don't make sense at
all anyway.

This series drops that distinction and then also adds documentation to
make the module parameter for configuring it more accessible.

v2->v3
 * Add documentation
 * Rename parameter
 * Drop patch for users protecting from themselves, this will be taken over
   by Runa Guo-Oc's series

Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1971576
Link: https://lore.kernel.org/linux-ide/1650534217-14052-2-git-send-email-RunaGuo-oc@zhaoxin.com/

Mario Limonciello (3):
  ata: ahci: Drop low power policy board type
  ata: ahci: Rename module parameter for lpm policy
  ahci: Document the loss of hotplug by new LPM policy

 .../admin-guide/kernel-parameters.txt         |  18 +++
 drivers/ata/Kconfig                           |   5 +-
 drivers/ata/ahci.c                            | 115 ++++++++----------
 drivers/ata/ahci.h                            |   7 +-
 4 files changed, 73 insertions(+), 72 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH 1/3] ata: ahci: Drop low power policy board type
  2022-05-24 17:05 [PATCH v3 0/3] Drop mobile board support Mario Limonciello
@ 2022-05-24 17:05 ` Mario Limonciello
  2022-05-24 17:05 ` [PATCH 2/3] ata: ahci: Rename module parameter for lpm policy Mario Limonciello
  2022-05-24 17:05 ` [PATCH 3/3] ahci: Document the loss of hotplug by new LPM policy Mario Limonciello
  2 siblings, 0 replies; 7+ messages in thread
From: Mario Limonciello @ 2022-05-24 17:05 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list, hdegoede, Mario Limonciello, Christoph Hellwig,
	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>
Acked-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/ata/Kconfig |   5 +--
 drivers/ata/ahci.c  | 105 +++++++++++++++++++-------------------------
 drivers/ata/ahci.h  |   7 +--
 3 files changed, 50 insertions(+), 67 deletions(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index bb45a9c00514..8ad73a30f405 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -116,15 +116,14 @@ config SATA_AHCI
 	  If unsure, say N.
 
 config SATA_MOBILE_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
 	help
 	  Select the Default SATA Link Power Management (LPM) policy to use
 	  for chipsets / "South Bridges" supporting low-power modes. Such
-	  chipsets are typically found on most laptops but desktops and
-	  servers now also widely use chipsets supporting low power modes.
+	  chipsets are ubiquitous across laptops, desktops and servers.
 
 	  The value set has the following meanings:
 		0 => Keep firmware settings
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index c1eca72b4575..c39cdf6a44f1 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 */
@@ -326,29 +318,29 @@ static const struct pci_device_id ahci_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, 0x1d06), board_ahci }, /* PBG 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 */
@@ -382,26 +374,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*/
@@ -414,13 +406,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,
@@ -448,7 +440,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 },
@@ -1598,11 +1590,6 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap,
 {
 	int policy = CONFIG_SATA_MOBILE_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 ad11a4c52fbe..a085228ce4ff 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -235,12 +235,9 @@ 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_MOBILE_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_NO_SXS		= (1 << 28), /* SXS not supported */
+	AHCI_HFLAG_NO_SXS		= (1 << 26), /* SXS not supported */
 
 	/* ap->flags bits */
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH 2/3] ata: ahci: Rename module parameter for lpm policy
  2022-05-24 17:05 [PATCH v3 0/3] Drop mobile board support Mario Limonciello
  2022-05-24 17:05 ` [PATCH 1/3] ata: ahci: Drop low power policy board type Mario Limonciello
@ 2022-05-24 17:05 ` Mario Limonciello
  2022-05-25  9:03   ` Christoph Hellwig
  2022-05-24 17:05 ` [PATCH 3/3] ahci: Document the loss of hotplug by new LPM policy Mario Limonciello
  2 siblings, 1 reply; 7+ messages in thread
From: Mario Limonciello @ 2022-05-24 17:05 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list, hdegoede, Mario Limonciello

The LPM policy is applied to more chipsets than just mobile designs.
Update the module parameter to drop the word "mobile" to make this
clearer to users.

Also, document this renamed parameter in the admin-guide for users
as it was missing previously.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 11 +++++++++++
 drivers/ata/ahci.c                              | 10 +++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 9336d98fc670..9e6bd212004d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -268,6 +268,17 @@
 			try_unsupported: try to drive unsupported chipsets
 				(may crash computer or cause data corruption)
 
+	ahci.lpm_policy= [AHCI]
+			The Default SATA Link Power Management (LPM) policy to use
+			for chipsets / "South Bridges" supporting low-power modes.
+
+			The value set has the following meanings:
+			0 => Keep firmware settings
+			1 => Maximum performance
+			2 => Medium power
+			3 => Medium power with Device Initiated PM enabled
+			4 => Minimum power
+
 	ALSA		[HW,ALSA]
 			See Documentation/sound/alsa-configuration.rst
 
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index c39cdf6a44f1..1e2febea2f28 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -640,9 +640,9 @@ static int marvell_enable = 1;
 module_param(marvell_enable, int, 0644);
 MODULE_PARM_DESC(marvell_enable, "Marvell SATA via AHCI (1 = enabled)");
 
-static int mobile_lpm_policy = -1;
-module_param(mobile_lpm_policy, int, 0644);
-MODULE_PARM_DESC(mobile_lpm_policy, "Default LPM policy for mobile chipsets");
+static int lpm_policy = -1;
+module_param(lpm_policy, int, 0644);
+MODULE_PARM_DESC(lpm_policy, "Default LPM policy");
 
 static void ahci_pci_save_initial_config(struct pci_dev *pdev,
 					 struct ahci_host_priv *hpriv)
@@ -1591,8 +1591,8 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap,
 	int policy = CONFIG_SATA_MOBILE_LPM_POLICY;
 
 	/* user modified policy via module param */
-	if (mobile_lpm_policy != -1) {
-		policy = mobile_lpm_policy;
+	if (lpm_policy != -1) {
+		policy = lpm_policy;
 		goto update_policy;
 	}
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH 3/3] ahci: Document the loss of hotplug by new LPM policy
  2022-05-24 17:05 [PATCH v3 0/3] Drop mobile board support Mario Limonciello
  2022-05-24 17:05 ` [PATCH 1/3] ata: ahci: Drop low power policy board type Mario Limonciello
  2022-05-24 17:05 ` [PATCH 2/3] ata: ahci: Rename module parameter for lpm policy Mario Limonciello
@ 2022-05-24 17:05 ` Mario Limonciello
  2022-05-25  9:04   ` Christoph Hellwig
  2 siblings, 1 reply; 7+ messages in thread
From: Mario Limonciello @ 2022-05-24 17:05 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list, hdegoede, Mario Limonciello

Per AHCI spec v1.3.1, "7.3 Native Hot Plug Support", once LPM is
enabled hotplug support needs to be disabled.

The LPM code always followed this and disabled the port when no
drives were connected, but as more machines will be exposed to
this code it might be an unexpected behavior to some users.

Add a note to parameter documentation to explain the new behavior.

Link: https://bugs.launchpad.net/bugs/1971576
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 9e6bd212004d..4dcd9a3ba4a5 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -279,6 +279,13 @@
 			3 => Medium power with Device Initiated PM enabled
 			4 => Minimum power
 
+			NOTE: Enabling LPM when no drive is connected will disable
+			the port which means hotplug will not work.
+
+			If hotplug is an important use case, this can be modified
+			at runtime by changing
+			/sys/module/ahci/parameters/lpm_policy
+
 	ALSA		[HW,ALSA]
 			See Documentation/sound/alsa-configuration.rst
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] ata: ahci: Rename module parameter for lpm policy
  2022-05-24 17:05 ` [PATCH 2/3] ata: ahci: Rename module parameter for lpm policy Mario Limonciello
@ 2022-05-25  9:03   ` Christoph Hellwig
  2022-05-25 11:11     ` Damien Le Moal
  0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2022-05-25  9:03 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Damien Le Moal,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list, hdegoede

On Tue, May 24, 2022 at 12:05:07PM -0500, Mario Limonciello wrote:
> The LPM policy is applied to more chipsets than just mobile designs.
> Update the module parameter to drop the word "mobile" to make this
> clearer to users.
> 
> Also, document this renamed parameter in the admin-guide for users
> as it was missing previously.

Even if the name is confusing I don't think we should rename it as
that breaks existing setups.  I think just updating the documentation
is good enough, but if you feel strongly we can just add the new name
while keeping the old one as an alias.

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] ahci: Document the loss of hotplug by new LPM policy
  2022-05-24 17:05 ` [PATCH 3/3] ahci: Document the loss of hotplug by new LPM policy Mario Limonciello
@ 2022-05-25  9:04   ` Christoph Hellwig
  0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2022-05-25  9:04 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Damien Le Moal,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list, hdegoede

On Tue, May 24, 2022 at 12:05:08PM -0500, Mario Limonciello wrote:
> Per AHCI spec v1.3.1, "7.3 Native Hot Plug Support", once LPM is
> enabled hotplug support needs to be disabled.
> 
> The LPM code always followed this and disabled the port when no
> drives were connected, but as more machines will be exposed to
> this code it might be an unexpected behavior to some users.
> 
> Add a note to parameter documentation to explain the new behavior.
> 
> Link: https://bugs.launchpad.net/bugs/1971576
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9e6bd212004d..4dcd9a3ba4a5 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -279,6 +279,13 @@
>  			3 => Medium power with Device Initiated PM enabled
>  			4 => Minimum power
>  
> +			NOTE: Enabling LPM when no drive is connected will disable
> +			the port which means hotplug will not work.
> +
> +			If hotplug is an important use case, this can be modified

This has two overly long lines.

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] ata: ahci: Rename module parameter for lpm policy
  2022-05-25  9:03   ` Christoph Hellwig
@ 2022-05-25 11:11     ` Damien Le Moal
  0 siblings, 0 replies; 7+ messages in thread
From: Damien Le Moal @ 2022-05-25 11:11 UTC (permalink / raw)
  To: Christoph Hellwig, Mario Limonciello
  Cc: open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list, hdegoede

On 5/25/22 18:03, Christoph Hellwig wrote:
> On Tue, May 24, 2022 at 12:05:07PM -0500, Mario Limonciello wrote:
>> The LPM policy is applied to more chipsets than just mobile designs.
>> Update the module parameter to drop the word "mobile" to make this
>> clearer to users.
>>
>> Also, document this renamed parameter in the admin-guide for users
>> as it was missing previously.
> 
> Even if the name is confusing I don't think we should rename it as
> that breaks existing setups.  I think just updating the documentation
> is good enough, but if you feel strongly we can just add the new name
> while keeping the old one as an alias.

+1 for the alias.

The other thing that this series need is to add the changes of the LPM
policy initialization according to the adapter capabilities (your patch
and the patches from Runa) as well as accounting for the eventual nolpm
link horkage flag. Otherwise, I fear that this change will generate many
regressions.

-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-05-25 11:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-24 17:05 [PATCH v3 0/3] Drop mobile board support Mario Limonciello
2022-05-24 17:05 ` [PATCH 1/3] ata: ahci: Drop low power policy board type Mario Limonciello
2022-05-24 17:05 ` [PATCH 2/3] ata: ahci: Rename module parameter for lpm policy Mario Limonciello
2022-05-25  9:03   ` Christoph Hellwig
2022-05-25 11:11     ` Damien Le Moal
2022-05-24 17:05 ` [PATCH 3/3] ahci: Document the loss of hotplug by new LPM policy Mario Limonciello
2022-05-25  9:04   ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.