[PATCH v3 00/10] Improve link power management

[PATCH v3 00/10] Improve link power management

[Damien Le Moal]

This patch series improves ATA link power management, mainly for the
AHCI driver. Follow-up patches will address libsas and AHCI platform
adapters.

These patches are a mix of code cleanup and LPM management improvements
around the application of an appropriate initial link power management
policy and preventing users or system daemons from changing a port link
power management policy through sysfs when the port or the device does
not support LPM policies or the port is an external port (and using LPM
would break the port hotplug capability).

Changes from v2:
 - Improved comments in ata_dev_config_lpm() and the commit message of
   patch 1
 - Improved commit message of patch 7
 - Added review tags

Changes from v1:
 - Improved commit message of patch 1 as suggested by Niklas.
 - Removed warning message in patch 7.
 - Changed warning message in patch 8 to be a debug message. Also fixed
   a typo in the commit message.
 - Changed message to be a single line in patch 10.

Damien Le Moal (10):
  ata: libata-core: Introduce ata_dev_config_lpm()
  ata: libata-core: Move device LPM quirk settings to ata_dev_config_lpm()
  ata: libata-core: Advertize device support for DIPM and HIPM features
  ata: libata-eh: Avoid unnecessary resets when revalidating devices
  ata: libata-sata: Disallow changing LPM state if not supported
  ata: ahci: Disable DIPM if host lacks support
  ata: ahci: Disallow LPM policy control for external ports
  ata: ahci: Disallow LPM policy control if not supported
  ata: libata-core: Reduce the number of messages signaling broken LPM
  ata: libata_eh: Add debug messages to ata_eh_link_set_lpm()

 drivers/ata/ahci.c        | 15 ++++++--
 drivers/ata/libata-core.c | 73 +++++++++++++++++++++++++++++----------
 drivers/ata/libata-eh.c   | 31 ++++++++++++++++-
 drivers/ata/libata-sata.c |  5 +++
 4 files changed, 103 insertions(+), 21 deletions(-)

-- 
2.50.0

[PATCH v3 01/10] ata: libata-core: Introduce ata_dev_config_lpm()

[Damien Le Moal]

If the port of a device does not support Device Initiated Power
Management (DIPM), that is, the port is flagged with ATA_FLAG_NO_DIPM,
the DIPM feature of a device should not be used. Though DIPM is disabled
by default on a device, the "Software Settings Preservation feature"
may keep DIPM enabled or DIPM may have been enabled by the system
firmware.

Introduce the function ata_dev_config_lpm() to always disable DIPM on a
device that supports this feature if the port of the device is flagged
with ATA_FLAG_NO_DIPM. ata_dev_config_lpm() is called from
ata_dev_configure(), ensuring that a device DIPM feature is disabled
when it cannot be used.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-core.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 3918ea624e0b..6df5e51ece21 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2790,6 +2790,30 @@ static void ata_dev_config_cpr(struct ata_device *dev)
 	kfree(buf);
 }
 
+/*
+ * Configure features related to link power management.
+ */
+static void ata_dev_config_lpm(struct ata_device *dev)
+{
+	struct ata_port *ap = dev->link->ap;
+	unsigned int err_mask;
+
+	/*
+	 * Device Initiated Power Management (DIPM) is normally disabled by
+	 * default on a device. However, DIPM may have been enabled and that
+	 * setting kept even after COMRESET because of the Software Settings
+	 * Preservation feature. So if the port does not support DIPM and the
+	 * device does, disable DIPM on the device.
+	 */
+	if (ap->flags & ATA_FLAG_NO_DIPM && ata_id_has_dipm(dev->id)) {
+		err_mask = ata_dev_set_feature(dev,
+					SETFEATURES_SATA_DISABLE, SATA_DIPM);
+		if (err_mask && err_mask != AC_ERR_DEV)
+			ata_dev_err(dev, "Disable DIPM failed, Emask 0x%x\n",
+				    err_mask);
+	}
+}
+
 static void ata_dev_print_features(struct ata_device *dev)
 {
 	if (!(dev->flags & ATA_DFLAG_FEATURES_MASK))
@@ -2963,6 +2987,7 @@ int ata_dev_configure(struct ata_device *dev)
 			ata_dev_config_chs(dev);
 		}
 
+		ata_dev_config_lpm(dev);
 		ata_dev_config_fua(dev);
 		ata_dev_config_devslp(dev);
 		ata_dev_config_sense_reporting(dev);
-- 
2.50.0

[PATCH v3 02/10] ata: libata-core: Move device LPM quirk settings to ata_dev_config_lpm()

[Damien Le Moal]

Move the various cases of setting the ATA_QUIRK_NOLPM quirk flag for a
device in ata_dev_configure() to the function ata_dev_config_lpm().
This allows having all LPM related settings in one place to facilitate
maintenance.

No functional changes.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c | 43 +++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 6df5e51ece21..33b2ffd05af7 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2798,6 +2798,32 @@ static void ata_dev_config_lpm(struct ata_device *dev)
 	struct ata_port *ap = dev->link->ap;
 	unsigned int err_mask;
 
+	if (ap->flags & ATA_FLAG_NO_LPM) {
+		/*
+		 * When the port does not support LPM, we cannot support it on
+		 * the device either.
+		 */
+		dev->quirks |= ATA_QUIRK_NOLPM;
+	} else {
+		/*
+		 * Some WD SATA-1 drives have issues with LPM, turn on NOLPM for
+		 * them.
+		 */
+		if ((dev->quirks & ATA_QUIRK_WD_BROKEN_LPM) &&
+		    (dev->id[ATA_ID_SATA_CAPABILITY] & 0xe) == 0x2)
+			dev->quirks |= ATA_QUIRK_NOLPM;
+
+		/* ATI specific quirk */
+		if ((dev->quirks & ATA_QUIRK_NO_LPM_ON_ATI) &&
+		    ata_dev_check_adapter(dev, PCI_VENDOR_ID_ATI))
+			dev->quirks |= ATA_QUIRK_NOLPM;
+	}
+
+	if (dev->quirks & ATA_QUIRK_NOLPM) {
+		ata_dev_warn(dev, "LPM support broken, forcing max_power\n");
+		ap->target_lpm_policy = ATA_LPM_MAX_POWER;
+	}
+
 	/*
 	 * Device Initiated Power Management (DIPM) is normally disabled by
 	 * default on a device. However, DIPM may have been enabled and that
@@ -2884,23 +2910,6 @@ int ata_dev_configure(struct ata_device *dev)
 	if (rc)
 		return rc;
 
-	/* some WD SATA-1 drives have issues with LPM, turn on NOLPM for them */
-	if ((dev->quirks & ATA_QUIRK_WD_BROKEN_LPM) &&
-	    (id[ATA_ID_SATA_CAPABILITY] & 0xe) == 0x2)
-		dev->quirks |= ATA_QUIRK_NOLPM;
-
-	if (dev->quirks & ATA_QUIRK_NO_LPM_ON_ATI &&
-	    ata_dev_check_adapter(dev, PCI_VENDOR_ID_ATI))
-		dev->quirks |= ATA_QUIRK_NOLPM;
-
-	if (ap->flags & ATA_FLAG_NO_LPM)
-		dev->quirks |= ATA_QUIRK_NOLPM;
-
-	if (dev->quirks & ATA_QUIRK_NOLPM) {
-		ata_dev_warn(dev, "LPM support broken, forcing max_power\n");
-		dev->link->ap->target_lpm_policy = ATA_LPM_MAX_POWER;
-	}
-
 	/* let ACPI work its magic */
 	rc = ata_acpi_on_devcfg(dev);
 	if (rc)
-- 
2.50.0

[PATCH v3 03/10] ata: libata-core: Advertize device support for DIPM and HIPM features

[Damien Le Moal]

Modify ata_dev_print_features() to advertize if a device supports the
Device Initiated Power Management (DIPM) and Host Initiated Power
Management (HIPM) features.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 33b2ffd05af7..4619e66ae100 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2846,11 +2846,13 @@ static void ata_dev_print_features(struct ata_device *dev)
 		return;
 
 	ata_dev_info(dev,
-		     "Features:%s%s%s%s%s%s%s%s\n",
+		     "Features:%s%s%s%s%s%s%s%s%s%s\n",
 		     dev->flags & ATA_DFLAG_FUA ? " FUA" : "",
 		     dev->flags & ATA_DFLAG_TRUSTED ? " Trust" : "",
 		     dev->flags & ATA_DFLAG_DA ? " Dev-Attention" : "",
 		     dev->flags & ATA_DFLAG_DEVSLP ? " Dev-Sleep" : "",
+		     ata_id_has_hipm(dev->id) ? " HIPM" : "",
+		     ata_id_has_dipm(dev->id) ? " DIPM" : "",
 		     dev->flags & ATA_DFLAG_NCQ_SEND_RECV ? " NCQ-sndrcv" : "",
 		     dev->flags & ATA_DFLAG_NCQ_PRIO ? " NCQ-prio" : "",
 		     dev->flags & ATA_DFLAG_CDL ? " CDL" : "",
-- 
2.50.0

[PATCH v3 04/10] ata: libata-eh: Avoid unnecessary resets when revalidating devices

[Damien Le Moal]

In ata_eh_revalidate_and_attach(), a link LPM policy is always
set to ATA_LPM_MAX_POWER before calling ata_dev_revalidate() to ensure
that the call to ata_phys_link_offline() does not return true, thus
causing an unnecessary device reset. This change was introduced
with commit 71d7b6e51ad3 ("ata: libata-eh: avoid needless hard reset
when revalidating link").

However, setting the link LPM policy to ATA_LPM_MAX_POWER may be
visible only after some time, depending on the power state the link was
in. E.g. transitioning out of the Partial state should take no longer
than a few microseconds, but transitioning out of the Slumber or
DevSleep state may take several milliseconds. So despite the changes
introduced with commit 71d7b6e51ad3 ("ata: libata-eh: avoid needless
hard reset when revalidating link"), we can still endup with
ata_phys_link_offline() seeing a link SCR_STATUS register signaling that
the device is present (DET is equal to 1h) but that the link PHY is
still in a low power mode (e.g. IPM is 2h, signaling "Interface in
Partial power management state"). In such cases, ata_phys_link_offline()
returns true, causing an EIO return for ata_eh_revalidate_and_attach()
and a device reset.

Avoid such unnecessary device resets by introducing a relaxed version
of the link offline test implemented by ata_phys_link_offline() with
the new helper function ata_eh_link_established(). This functions
returns true if for the link SCR_STATUS register we see that:
 - A device is still present, that is, the DET field is 1h (Device
   presence detected but Phy communication not established) or 3h
   (Device presence detected and Phy communication established).
 - Communication is established, that is, the IPM field is not 0h,
   indicating that the PHY is online or in a low power state.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-eh.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index f98d5123e1e4..7f5d13f9ca73 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2071,6 +2071,33 @@ static void ata_eh_get_success_sense(struct ata_link *link)
 	ata_eh_done(link, dev, ATA_EH_GET_SUCCESS_SENSE);
 }
 
+/*
+ * Check if a link is established. This is a relaxed version of
+ * ata_phys_link_online() which accounts for the fact that this is potentially
+ * called after changing the link power management policy, which may not be
+ * reflected immediately in the SSTAUS register (e.g., we may still be seeing
+ * the PHY in partial, slumber or devsleep Partial power management state.
+ * So check that:
+ * - A device is still present, that is, DET is 1h (Device presence detected
+ *   but Phy communication not established) or 3h (Device presence detected and
+ *   Phy communication established)
+ * - Communication is established, that is, IPM is not 0h, indicating that PHY
+ *   is online or in a low power state.
+ */
+static bool ata_eh_link_established(struct ata_link *link)
+{
+	u32 sstatus;
+	u8 det, ipm;
+
+	if (sata_scr_read(link, SCR_STATUS, &sstatus))
+		return false;
+
+	det = sstatus & 0x0f;
+	ipm = (sstatus >> 8) & 0x0f;
+
+	return (det & 0x01) && ipm;
+}
+
 /**
  *	ata_eh_link_set_lpm - configure SATA interface power management
  *	@link: link to configure
@@ -3275,7 +3302,7 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
 					goto err;
 			}
 
-			if (ata_phys_link_offline(ata_dev_phys_link(dev))) {
+			if (!ata_eh_link_established(ata_dev_phys_link(dev))) {
 				rc = -EIO;
 				goto err;
 			}
-- 
2.50.0

[PATCH v3 05/10] ata: libata-sata: Disallow changing LPM state if not supported

[Damien Le Moal]

Modify ata_scsi_lpm_store() to return an error if a user attempts to set
a link power management policy for a port that does not support LPM,
that is, ports flagged with ATA_FLAG_NO_LPM.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-sata.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index cb46ce276bb1..47169c469f43 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -924,6 +924,11 @@ static ssize_t ata_scsi_lpm_store(struct device *device,
 
 	spin_lock_irqsave(ap->lock, flags);
 
+	if (ap->flags & ATA_FLAG_NO_LPM) {
+		count = -EOPNOTSUPP;
+		goto out_unlock;
+	}
+
 	ata_for_each_link(link, ap, EDGE) {
 		ata_for_each_dev(dev, &ap->link, ENABLED) {
 			if (dev->quirks & ATA_QUIRK_NOLPM) {
-- 
2.50.0

[PATCH v3 06/10] ata: ahci: Disable DIPM if host lacks support

[Damien Le Moal]

The AHCI specification version 1.3.1 section 8.3.1.4 (Software
Requirements and Precedence) states that:

If CAP.SSC or CAP.PSC is cleared to ‘0’, software should disable
device-initiated power management by issuing the appropriate SET
FEATURES command to the device.

To satisfy this constraint and force ata_dev_configure to disable the
device DIPM feature, modify ahci_update_initial_lpm_policy() to set the
ATA_FLAG_NO_DIPM flag on ports that have a host with either the
ATA_HOST_NO_PART flag set or the ATA_HOST_NO_SSC flag set.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/ahci.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 1b4151d95888..0760fa47d90e 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1780,6 +1780,13 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap)
 		return;
 	}
 
+	/* If no Partial or no Slumber, we cannot support DIPM. */
+	if ((ap->host->flags & ATA_HOST_NO_PART) ||
+	    (ap->host->flags & ATA_HOST_NO_SSC)) {
+		ata_port_dbg(ap, "Host does not support DIPM\n");
+		ap->flags |= ATA_FLAG_NO_DIPM;
+	}
+
 	/* If no LPM states are supported by the HBA, do not bother with LPM */
 	if ((ap->host->flags & ATA_HOST_NO_PART) &&
 	    (ap->host->flags & ATA_HOST_NO_SSC) &&
-- 
2.50.0

[PATCH v3 07/10] ata: ahci: Disallow LPM policy control for external ports

[Damien Le Moal]

Commit ae1f3db006b7 ("ata: ahci: do not enable LPM on external ports")
added an early return in ahci_update_initial_lpm_policy() for all
ports flagged as external with the ATA_PFLAG_EXTERNAL port flag (e.g.
eSATA ports or hotplug capable ports) so that the target_lpm_policy of
these ports is unchanged and set to ATA_LPM_UNKNOWN. thus forcing libata
EH to not be called for external port. The goal of this change is to
preserve the initial power management policy to not break the hotplug
capability of external ports.

However, this change is incomplete as users or system daemon (e.g.
systemd-udevd) can still apply the system preferred power management
policy through sysfs, thus potentially unknowingly breaking the port
hotplug capability.

Modify ahci_update_initial_lpm_policy() to flag external ports with
ATA_FLAG_NO_LPM to prevent changes to the LPM policy by users through
the sysfs link_power_management_policy host attribute. Also set the
target_lpm_policy of external ports to ATA_LPM_MAX_POWER to ensure
that the port is not in a low power state preventing hotplug operations.

Since commit 439d47608bb3 ("ata: libata: Print if port is external on
boot") introduced an unconditional print on port probe signaling that a
port is external, the debug message signaling that fact and that LPM
will not be enabled is removed.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/ahci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 0760fa47d90e..a061aa3299ef 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1776,7 +1776,8 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap)
 	 * LPM if the port advertises itself as an external port.
 	 */
 	if (ap->pflags & ATA_PFLAG_EXTERNAL) {
-		ata_port_dbg(ap, "external port, not enabling LPM\n");
+		ap->flags |= ATA_FLAG_NO_LPM;
+		ap->target_lpm_policy = ATA_LPM_MAX_POWER;
 		return;
 	}
 
-- 
2.50.0

[PATCH v3 08/10] ata: ahci: Disallow LPM policy control if not supported

[Damien Le Moal]

Commit fa997b0576c9 ("ata: ahci: Do not enable LPM if no LPM states are
supported by the HBA") introduced an early return in
ahci_update_initial_lpm_policy() to ensure that the target_lpm_policy
of ports belonging to a host that does not support the Partial, Slumber
and DevSleep power states is unchanged and remains set to
ATA_LPM_UNKNOWN and thus prevents the execution of
ata_eh_link_set_lpm().

However, a user or a system daemon (e.g. systemd-udevd) may still
attempt changing the LPM policy through the sysfs
link_power_management_policy of the host.

Improve this to prevent sysfs LPM policy changes by setting the flag
ATA_FLAG_NO_LPM for the port of such host, and initialize the port
target_lpm_policy to ATA_LPM_MAX_POWER to guarantee that no unsupported
low power state is being used on the port and its link.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/ahci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index a061aa3299ef..5558e9f7b85d 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1792,7 +1792,10 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap)
 	if ((ap->host->flags & ATA_HOST_NO_PART) &&
 	    (ap->host->flags & ATA_HOST_NO_SSC) &&
 	    (ap->host->flags & ATA_HOST_NO_DEVSLP)) {
-		ata_port_dbg(ap, "no LPM states supported, not enabling LPM\n");
+		ata_port_dbg(ap,
+			"No LPM states supported, forcing LPM max_power\n");
+		ap->flags |= ATA_FLAG_NO_LPM;
+		ap->target_lpm_policy = ATA_LPM_MAX_POWER;
 		return;
 	}
 
-- 
2.50.0

[PATCH v3 09/10] ata: libata-core: Reduce the number of messages signaling broken LPM

[Damien Le Moal]

ata_dev_config_lpm() prints the message "LPM support broken, forcing
max_power" for devices that have the ATA_QUIRK_NOLPM quirk flag set.
This messages is repeated for every device revalidation, which is not
necessary, but also erroneously printed for devices without a broken LPM
support when connected to a port that does not support LPM (e.g. because
the port is an external one with hotplug capability).

Since in all cases the device port target_lpm_policy is set to
ATA_LPM_MAX_POWER, avoid the "LPM broken" message repetition and
erroneous output by generating it only if the port target_lpm_policy is
not already set to ATA_LPM_MAX_POWER.

This change will suppress the "LPM broken" message for genuine cases of
a device having broken LPM if the initial LPM policy is set to
ATA_LPM_MAX_POWER through CONFIG_SATA_MOBILE_LPM_POLICY. This is not a
problem as the ATA_LPM_MAX_POWER policy implies that LPM is disabled and
unused, which is safe for devices with broken LPM.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 4619e66ae100..7f6cebe61b33 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2819,7 +2819,8 @@ static void ata_dev_config_lpm(struct ata_device *dev)
 			dev->quirks |= ATA_QUIRK_NOLPM;
 	}
 
-	if (dev->quirks & ATA_QUIRK_NOLPM) {
+	if (dev->quirks & ATA_QUIRK_NOLPM &&
+	    ap->target_lpm_policy != ATA_LPM_MAX_POWER) {
 		ata_dev_warn(dev, "LPM support broken, forcing max_power\n");
 		ap->target_lpm_policy = ATA_LPM_MAX_POWER;
 	}
-- 
2.50.0

[PATCH v3 10/10] ata: libata_eh: Add debug messages to ata_eh_link_set_lpm()

[Damien Le Moal]

To facilitate field debugging of link power management related issues,
add a debug message to ata_eh_link_set_lpm() to easily track LPM policy
changes done from EH context, that is, during device scan and
revalidation, error handling, and when a policy change is issued through
a host sysfs link_power_management_policy attribute.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-eh.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 7f5d13f9ca73..0f9c30f9bc1e 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2140,6 +2140,8 @@ static int ata_eh_link_set_lpm(struct ata_link *link,
 	if (WARN_ON_ONCE(policy == ATA_LPM_UNKNOWN))
 		return 0;
 
+	ata_link_dbg(link, "Set LPM policy: %d -> %d\n", old_policy, policy);
+
 	/*
 	 * DIPM is enabled only for ATA_LPM_MIN_POWER,
 	 * ATA_LPM_MIN_POWER_WITH_PARTIAL, and ATA_LPM_MED_POWER_WITH_DIPM, as
-- 
2.50.0

Re: [PATCH v3 01/10] ata: libata-core: Introduce ata_dev_config_lpm()

[Hannes Reinecke]

On 7/1/25 14:53, Damien Le Moal wrote:
> If the port of a device does not support Device Initiated Power
> Management (DIPM), that is, the port is flagged with ATA_FLAG_NO_DIPM,
> the DIPM feature of a device should not be used. Though DIPM is disabled
> by default on a device, the "Software Settings Preservation feature"
> may keep DIPM enabled or DIPM may have been enabled by the system
> firmware.
> 
> Introduce the function ata_dev_config_lpm() to always disable DIPM on a
> device that supports this feature if the port of the device is flagged
> with ATA_FLAG_NO_DIPM. ata_dev_config_lpm() is called from
> ata_dev_configure(), ensuring that a device DIPM feature is disabled
> when it cannot be used.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> Reviewed-by: Niklas Cassel <cassel@kernel.org>
> ---
>   drivers/ata/libata-core.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v3 00/10] Improve link power management

[Niklas Cassel]

On Tue, 01 Jul 2025 21:53:11 +0900, Damien Le Moal wrote:
> This patch series improves ATA link power management, mainly for the
> AHCI driver. Follow-up patches will address libsas and AHCI platform
> adapters.
> 
> These patches are a mix of code cleanup and LPM management improvements
> around the application of an appropriate initial link power management
> policy and preventing users or system daemons from changing a port link
> power management policy through sysfs when the port or the device does
> not support LPM policies or the port is an external port (and using LPM
> would break the port hotplug capability).
> 
> [...]

Applied to libata/linux.git (for-6.17), thanks!

[01/10] ata: libata-core: Introduce ata_dev_config_lpm()
        https://git.kernel.org/libata/linux/c/d3601218
[02/10] ata: libata-core: Move device LPM quirk settings to ata_dev_config_lpm()
        https://git.kernel.org/libata/linux/c/d99a9142
[03/10] ata: libata-core: Advertize device support for DIPM and HIPM features
        https://git.kernel.org/libata/linux/c/b1f5af54
[04/10] ata: libata-eh: Avoid unnecessary resets when revalidating devices
        https://git.kernel.org/libata/linux/c/4371fe1b
[05/10] ata: libata-sata: Disallow changing LPM state if not supported
        https://git.kernel.org/libata/linux/c/413e800c
[06/10] ata: ahci: Disable DIPM if host lacks support
        https://git.kernel.org/libata/linux/c/f7870e8d
[07/10] ata: ahci: Disallow LPM policy control for external ports
        https://git.kernel.org/libata/linux/c/4edf1505
[08/10] ata: ahci: Disallow LPM policy control if not supported
        https://git.kernel.org/libata/linux/c/65b2c92f
[09/10] ata: libata-core: Reduce the number of messages signaling broken LPM
        https://git.kernel.org/libata/linux/c/3b50dd4c
[10/10] ata: libata_eh: Add debug messages to ata_eh_link_set_lpm()
        https://git.kernel.org/libata/linux/c/cb35d3b6

Kind regards,
Niklas