Re: Regression on linux-next (next-20250708)

Damien Le Moal
2025-07-28T04:14:10Z

[-- Attachment #1: Type: text/plain, Size: 3551 bytes --]

On 7/25/25 3:43 PM, Borah, Chaitanya Kumar wrote:
> For some context in our kms_pm_rpm tests, we enable min_power policy for SATA
> so that we can reach deep runtime power states and restore the original policy
> after finishing. [5][6]
>
> IIUC, the above change is based on spec and not something which can be
> reverted. So as I see it, we have to drop this code path for external ports.
> However I am not sure if we can achieve deep power states without enforcing it
> through the sysfs entry.
>
> Atleast for the basic-rte subtest, the test passes if we comment out the
> functions controlling the SATA ports. We will need more testing to determine if
> this approach work. Any thoughts on it?
>
> Also, are there other ways to detect a port is external other than receiving
> EOPNOTSUPP on the sysfs write?

The attached patch adds the "link_power_management_supported" sysfs device
attribute for drives connected to AHCI. Would that work for you ?

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 229429ba5027..495fa096dd65 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -1089,6 +1089,7 @@ static struct ata_port_operations ich_pata_ops = {
};

static struct attribute *piix_sidpr_shost_attrs[] = {
+ &dev_attr_link_power_management_supported.attr,
&dev_attr_link_power_management_policy.attr,
NULL
};
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index b335fb7e5cb4..c79abdfcd7a9 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -111,6 +111,7 @@ static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
static DEVICE_ATTR(em_message_supported, S_IRUGO, ahci_show_em_supported, NULL);

static struct attribute *ahci_shost_attrs[] = {
+ &dev_attr_link_power_management_supported.attr,
&dev_attr_link_power_management_policy.attr,
&dev_attr_em_message_type.attr,
&dev_attr_em_message.attr,
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index 0708686ca58a..82a1a72e47bf 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -900,6 +900,18 @@ static const char *ata_lpm_policy_names[] = {
[ATA_LPM_MIN_POWER] = "min_power",
};

+static ssize_t ata_scsi_lpm_supported_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct Scsi_Host *shost = class_to_shost(dev);
+ struct ata_port *ap = ata_shost_to_port(shost);
+
+ return sysfs_emit(buf, "%d\n", !(ap->flags & ATA_FLAG_NO_LPM));
+}
+DEVICE_ATTR(link_power_management_supported, S_IRUGO,
+ ata_scsi_lpm_supported_show, NULL);
+EXPORT_SYMBOL_GPL(dev_attr_link_power_management_supported);
+
static ssize_t ata_scsi_lpm_store(struct device *device,
struct device_attribute *attr,
const char *buf, size_t count)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 1c0580627dbb..e9a6f37bd7f9 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -547,6 +547,7 @@ typedef void (*ata_postreset_fn_t)(struct ata_link *link,
unsigned int *classes)

extern struct device_attribute dev_attr_unload_heads;
#ifdef CONFIG_SATA_HOST
+extern struct device_attribute dev_attr_link_power_management_supported;
extern struct device_attribute dev_attr_link_power_management_policy;
extern struct device_attribute dev_attr_ncq_prio_supported;
extern struct device_attribute dev_attr_ncq_prio_enable;

--
Damien Le Moal
Western Digital Research

[-- Attachment #2: 0001-ata-libata-sata-Add-link_power_management_supported-.patch --]
[-- Type: text/x-patch, Size: 3338 bytes --]

From e9937d86d37a6c1a9584ef232613d5a8a5a3b6a1 Mon Sep 17 00:00:00 2001
From: Damien Le Moal <dlemoal@kernel.org>
Date: Mon, 28 Jul 2025 13:04:29 +0900
Subject: [PATCH] ata: libata-sata: Add link_power_management_supported
attribute

A port link power management policy can be controlled using the
link_power_management_policy sysfs device attribute. However, this
attribute exist also for device and ports that do not support LPM and in
such case, attempting to change the LPM policy will fail with
-EOPNOTSUPP.

Introduce the new sysfs link_power_management_supported device attribute
to indicate to the user if a port/device supports LPM and the attribute
link_power_management_policy can be used.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
drivers/ata/ata_piix.c | 1 +
drivers/ata/libahci.c | 1 +
drivers/ata/libata-sata.c | 12 ++++++++++++
include/linux/libata.h | 1 +
4 files changed, 15 insertions(+)

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 229429ba5027..495fa096dd65 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -1089,6 +1089,7 @@ static struct ata_port_operations ich_pata_ops = {
};

static struct attribute *piix_sidpr_shost_attrs[] = {
+ &dev_attr_link_power_management_supported.attr,
&dev_attr_link_power_management_policy.attr,
NULL
};
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index b335fb7e5cb4..c79abdfcd7a9 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -111,6 +111,7 @@ static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
static DEVICE_ATTR(em_message_supported, S_IRUGO, ahci_show_em_supported, NULL);

static struct attribute *ahci_shost_attrs[] = {
+ &dev_attr_link_power_management_supported.attr,
&dev_attr_link_power_management_policy.attr,
&dev_attr_em_message_type.attr,
&dev_attr_em_message.attr,
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index 0708686ca58a..82a1a72e47bf 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -900,6 +900,18 @@ static const char *ata_lpm_policy_names[] = {
[ATA_LPM_MIN_POWER] = "min_power",
};

+static ssize_t ata_scsi_lpm_supported_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct Scsi_Host *shost = class_to_shost(dev);
+ struct ata_port *ap = ata_shost_to_port(shost);
+
+ return sysfs_emit(buf, "%d\n", !(ap->flags & ATA_FLAG_NO_LPM));
+}
+DEVICE_ATTR(link_power_management_supported, S_IRUGO,
+ ata_scsi_lpm_supported_show, NULL);
+EXPORT_SYMBOL_GPL(dev_attr_link_power_management_supported);
+
static ssize_t ata_scsi_lpm_store(struct device *device,
struct device_attribute *attr,
const char *buf, size_t count)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 1c0580627dbb..e9a6f37bd7f9 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -547,6 +547,7 @@ typedef void (*ata_postreset_fn_t)(struct ata_link *link, unsigned int *classes)

extern struct device_attribute dev_attr_unload_heads;
#ifdef CONFIG_SATA_HOST
+extern struct device_attribute dev_attr_link_power_management_supported;
extern struct device_attribute dev_attr_link_power_management_policy;
extern struct device_attribute dev_attr_ncq_prio_supported;
extern struct device_attribute dev_attr_ncq_prio_enable;
--
2.50.1