On 7/28/2025 9:41 AM, Damien Le Moal wrote:
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 ?
Yes this could work. I quickly hacked the test to ignore writing policy
if this file returns 0.
Here is the state of the machine I am testing on.
/sys/class/scsi_host/host0/link_power_management_supported: 0
/sys/class/scsi_host/host1/link_power_management_supported: 0
/sys/class/scsi_host/host2/link_power_management_supported: 0
/sys/class/scsi_host/host3/link_power_management_supported: 0
/sys/class/scsi_host/host4/link_power_management_supported: 1
/sys/class/scsi_host/host5/link_power_management_supported: 1
/sys/class/scsi_host/host6/link_power_management_supported: 1
/sys/class/scsi_host/host7/link_power_management_supported: 1
Regards
Chaitanya
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;
