Support power resources in ata ports

[PATCH 1/2] Add manage_restart device attribute to scsi_disk

Posted by Markus Probst 2 months, 1 week ago

there is already manage_shutdown, manage_system_start_stop and
manage_runtime_start_stop device attributes that allows the high-level
device driver (sd) manage the device power state, expect for the
system_state SYSTEM_RESTART. With this device attribute, it is possible to
let the high-level device driver (sd) manage the device power state for
SYSTEM_RESTART too.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/scsi/sd.c          | 35 ++++++++++++++++++++++++++++++++++-
 include/scsi/scsi_device.h |  6 ++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 5b8668accf8e..a3e9c2e9d9f4 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -318,6 +318,36 @@ static ssize_t manage_shutdown_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(manage_shutdown);
 
+static ssize_t manage_restart_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct scsi_disk *sdkp = to_scsi_disk(dev);
+	struct scsi_device *sdp = sdkp->device;
+
+	return sysfs_emit(buf, "%u\n", sdp->manage_restart);
+}
+
+
+static ssize_t manage_restart_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	struct scsi_disk *sdkp = to_scsi_disk(dev);
+	struct scsi_device *sdp = sdkp->device;
+	bool v;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (kstrtobool(buf, &v))
+		return -EINVAL;
+
+	sdp->manage_restart = v;
+
+	return count;
+}
+static DEVICE_ATTR_RW(manage_restart);
+
 static ssize_t
 allow_restart_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
@@ -654,6 +684,7 @@ static struct attribute *sd_disk_attrs[] = {
 	&dev_attr_manage_system_start_stop.attr,
 	&dev_attr_manage_runtime_start_stop.attr,
 	&dev_attr_manage_shutdown.attr,
+	&dev_attr_manage_restart.attr,
 	&dev_attr_protection_type.attr,
 	&dev_attr_protection_mode.attr,
 	&dev_attr_app_tag_own.attr,
@@ -4175,7 +4206,9 @@ static void sd_shutdown(struct device *dev)
 	    (system_state == SYSTEM_POWER_OFF &&
 	     sdkp->device->manage_shutdown) ||
 	    (system_state == SYSTEM_RUNNING &&
-	     sdkp->device->manage_runtime_start_stop)) {
+	     sdkp->device->manage_runtime_start_stop) ||
+	    (system_state == SYSTEM_RESTART &&
+	     sdkp->device->manage_restart)) {
 		sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
 		sd_start_stop_device(sdkp, 0);
 	}
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 6d6500148c4b..c7e657ac8b6d 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -178,6 +178,12 @@ struct scsi_device {
 	 */
 	unsigned manage_shutdown:1;
 
+	/*
+	 * If true, let the high-level device driver (sd) manage the device
+	 * power state for system restart (reboot) operations.
+	 */
+	unsigned manage_restart:1;
+
 	/*
 	 * If set and if the device is runtime suspended, ask the high-level
 	 * device driver (sd) to force a runtime resume of the device.
-- 
2.49.1

Re: [PATCH 1/2] Add manage_restart device attribute to scsi_disk

Posted by Damien Le Moal 2 months, 1 week ago

On 10/6/25 04:06, Markus Probst wrote:
> there is already manage_shutdown, manage_system_start_stop and
> manage_runtime_start_stop device attributes that allows the high-level
> device driver (sd) manage the device power state, expect for the
> system_state SYSTEM_RESTART. With this device attribute, it is possible to
> let the high-level device driver (sd) manage the device power state for
> SYSTEM_RESTART too.
> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  drivers/scsi/sd.c          | 35 ++++++++++++++++++++++++++++++++++-
>  include/scsi/scsi_device.h |  6 ++++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 5b8668accf8e..a3e9c2e9d9f4 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -318,6 +318,36 @@ static ssize_t manage_shutdown_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(manage_shutdown);
>  
> +static ssize_t manage_restart_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct scsi_disk *sdkp = to_scsi_disk(dev);
> +	struct scsi_device *sdp = sdkp->device;

sdp is not really needed.

> +
> +	return sysfs_emit(buf, "%u\n", sdp->manage_restart);
> +}
> +
> +
> +static ssize_t manage_restart_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t count)
> +{
> +	struct scsi_disk *sdkp = to_scsi_disk(dev);
> +	struct scsi_device *sdp = sdkp->device;

Same here.

> +	bool v;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EACCES;
> +
> +	if (kstrtobool(buf, &v))
> +		return -EINVAL;
> +
> +	sdp->manage_restart = v;
> +
> +	return count;
> +}
> +static DEVICE_ATTR_RW(manage_restart);
> +
>  static ssize_t
>  allow_restart_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
> @@ -654,6 +684,7 @@ static struct attribute *sd_disk_attrs[] = {
>  	&dev_attr_manage_system_start_stop.attr,
>  	&dev_attr_manage_runtime_start_stop.attr,
>  	&dev_attr_manage_shutdown.attr,
> +	&dev_attr_manage_restart.attr,
>  	&dev_attr_protection_type.attr,
>  	&dev_attr_protection_mode.attr,
>  	&dev_attr_app_tag_own.attr,
> @@ -4175,7 +4206,9 @@ static void sd_shutdown(struct device *dev)
>  	    (system_state == SYSTEM_POWER_OFF &&
>  	     sdkp->device->manage_shutdown) ||
>  	    (system_state == SYSTEM_RUNNING &&
> -	     sdkp->device->manage_runtime_start_stop)) {
> +	     sdkp->device->manage_runtime_start_stop) ||
> +	    (system_state == SYSTEM_RESTART &&
> +	     sdkp->device->manage_restart)) {
>  		sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
>  		sd_start_stop_device(sdkp, 0);
>  	}
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 6d6500148c4b..c7e657ac8b6d 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -178,6 +178,12 @@ struct scsi_device {
>  	 */
>  	unsigned manage_shutdown:1;
>  
> +	/*
> +	 * If true, let the high-level device driver (sd) manage the device
> +	 * power state for system restart (reboot) operations.

What about cold boot ? Same is needed, no ? The name "manage_restart" is a bit
confusing since we already have manage_system_start_stop,
manage_runtime_start_stop and manage_shutdown. I do not have a better suggestion
though.

> +	 */
> +	unsigned manage_restart:1;
> +
>  	/*
>  	 * If set and if the device is runtime suspended, ask the high-level
>  	 * device driver (sd) to force a runtime resume of the device.


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/2] Add manage_restart device attribute to scsi_disk

Posted by Markus Probst 2 months, 1 week ago

On Mon, 2025-10-06 at 09:58 +0900, Damien Le Moal wrote:
> On 10/6/25 04:06, Markus Probst wrote:
> > there is already manage_shutdown, manage_system_start_stop and
> > manage_runtime_start_stop device attributes that allows the high-
> > level
> > device driver (sd) manage the device power state, expect for the
> > system_state SYSTEM_RESTART. With this device attribute, it is
> > possible to
> > let the high-level device driver (sd) manage the device power state
> > for
> > SYSTEM_RESTART too.
> > 
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> >  drivers/scsi/sd.c          | 35
> > ++++++++++++++++++++++++++++++++++-
> >  include/scsi/scsi_device.h |  6 ++++++
> >  2 files changed, 40 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> > index 5b8668accf8e..a3e9c2e9d9f4 100644
> > --- a/drivers/scsi/sd.c
> > +++ b/drivers/scsi/sd.c
> > @@ -318,6 +318,36 @@ static ssize_t manage_shutdown_store(struct
> > device *dev,
> >  }
> >  static DEVICE_ATTR_RW(manage_shutdown);
> >  
> > +static ssize_t manage_restart_show(struct device *dev,
> > +				   struct device_attribute *attr,
> > char *buf)
> > +{
> > +	struct scsi_disk *sdkp = to_scsi_disk(dev);
> > +	struct scsi_device *sdp = sdkp->device;
> 
> sdp is not really needed.

Then it would be inconsistent with manage_shutdown_*,
manage_runtime_start_stop_*, manage_system_start_stop_*, there it has
the sdp variable declared.

> 
> > +
> > +	return sysfs_emit(buf, "%u\n", sdp->manage_restart);
> > +}
> > +
> > +
> > +static ssize_t manage_restart_store(struct device *dev,
> > +				    struct device_attribute *attr,
> > +				    const char *buf, size_t count)
> > +{
> > +	struct scsi_disk *sdkp = to_scsi_disk(dev);
> > +	struct scsi_device *sdp = sdkp->device;
> 
> Same here.
> 
> > +	bool v;
> > +
> > +	if (!capable(CAP_SYS_ADMIN))
> > +		return -EACCES;
> > +
> > +	if (kstrtobool(buf, &v))
> > +		return -EINVAL;
> > +
> > +	sdp->manage_restart = v;
> > +
> > +	return count;
> > +}
> > +static DEVICE_ATTR_RW(manage_restart);
> > +
> >  static ssize_t
> >  allow_restart_show(struct device *dev, struct device_attribute
> > *attr, char *buf)
> >  {
> > @@ -654,6 +684,7 @@ static struct attribute *sd_disk_attrs[] = {
> >  	&dev_attr_manage_system_start_stop.attr,
> >  	&dev_attr_manage_runtime_start_stop.attr,
> >  	&dev_attr_manage_shutdown.attr,
> > +	&dev_attr_manage_restart.attr,
> >  	&dev_attr_protection_type.attr,
> >  	&dev_attr_protection_mode.attr,
> >  	&dev_attr_app_tag_own.attr,
> > @@ -4175,7 +4206,9 @@ static void sd_shutdown(struct device *dev)
> >  	    (system_state == SYSTEM_POWER_OFF &&
> >  	     sdkp->device->manage_shutdown) ||
> >  	    (system_state == SYSTEM_RUNNING &&
> > -	     sdkp->device->manage_runtime_start_stop)) {
> > +	     sdkp->device->manage_runtime_start_stop) ||
> > +	    (system_state == SYSTEM_RESTART &&
> > +	     sdkp->device->manage_restart)) {
> >  		sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
> >  		sd_start_stop_device(sdkp, 0);
> >  	}
> > diff --git a/include/scsi/scsi_device.h
> > b/include/scsi/scsi_device.h
> > index 6d6500148c4b..c7e657ac8b6d 100644
> > --- a/include/scsi/scsi_device.h
> > +++ b/include/scsi/scsi_device.h
> > @@ -178,6 +178,12 @@ struct scsi_device {
> >  	 */
> >  	unsigned manage_shutdown:1;
> >  
> > +	/*
> > +	 * If true, let the high-level device driver (sd) manage
> > the device
> > +	 * power state for system restart (reboot) operations.
> 
> What about cold boot ? Same is needed, no ?
Not sure what you mean with cold boot here.

> The name "manage_restart" is a bit
> confusing since we already have manage_system_start_stop,
> manage_runtime_start_stop and manage_shutdown. I do not have a better
> suggestion
> though.
In that case, we could remove the DEVICE_ATTR_RW(manage_restart), so it
would not be exposed to userspace yet and can later by changed without
breaking userspace?
> 
> > +	 */
> > +	unsigned manage_restart:1;
> > +
> >  	/*
> >  	 * If set and if the device is runtime suspended, ask the
> > high-level
> >  	 * device driver (sd) to force a runtime resume of the
> > device.
> 

Thanks
- Markus Probst

[PATCH 2/2] Power on ata ports defined in ACPI before probing ports

Posted by Markus Probst 2 months, 1 week ago

some devices (for example every synology nas with "deep sleep" support)
have the ability to power on and off each ata port individually. This
turns the specific port on, if power manageable in acpi, before probing
it.

This can later be extended to power down the ata ports (removing power
from a disk) while the disk is spin down.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/ata/libata-acpi.c | 68 +++++++++++++++++++++++++++++++++++++++
 drivers/ata/libata-core.c | 21 ++++++++++++
 drivers/ata/libata-scsi.c |  1 +
 drivers/ata/libata.h      |  4 +++
 include/linux/libata.h    |  1 +
 5 files changed, 95 insertions(+)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index f2140fc06ba0..891b59bfe29f 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -245,6 +245,74 @@ void ata_acpi_bind_dev(struct ata_device *dev)
 				   ata_acpi_dev_uevent);
 }
 
+/**
+ * ata_acpi_dev_manage_restart - if the disk should be stopped (spin down) on system restart.
+ * @dev: target ATA device
+ *
+ * RETURNS:
+ * true if the disk should be stopped, otherwise false
+ */
+bool ata_acpi_dev_manage_restart(struct ata_device *dev)
+{
+	// If the device is power manageable and we assume the disk loses power on reboot.
+	if (dev->link->ap->flags & ATA_FLAG_ACPI_SATA) {
+		if (!is_acpi_device_node(dev->tdev.fwnode))
+			return false;
+		return acpi_bus_power_manageable(ACPI_HANDLE(&dev->tdev));
+	}
+
+	if (!is_acpi_device_node(dev->link->ap->tdev.fwnode))
+		return false;
+	return acpi_bus_power_manageable(ACPI_HANDLE(&dev->link->ap->tdev));
+}
+
+/**
+ * ata_acpi_port_set_power_state - set the power state of the ata port
+ * @ap: target ATA port
+ *
+ * This function is called at the begin of ata_port_probe.
+ */
+void ata_acpi_port_set_power_state(struct ata_port *ap, bool enable)
+{
+	acpi_handle handle;
+	unsigned char state;
+	int i;
+
+	if (libata_noacpi)
+		return;
+
+	if (enable)
+		state = ACPI_STATE_D0;
+	else
+		state = ACPI_STATE_D3_COLD;
+
+	if (ap->flags & ATA_FLAG_ACPI_SATA) {
+		for (i = 0; i < ATA_MAX_DEVICES; i++) {
+			if (!is_acpi_device_node(ap->link.device[i].tdev.fwnode))
+				continue;
+			handle = ACPI_HANDLE(&ap->link.device[i].tdev);
+			if (!acpi_bus_power_manageable(handle))
+				continue;
+			if (!acpi_bus_set_power(handle, state))
+				ata_dev_info(&ap->link.device[i], "acpi: power was set to %d\n",
+					     enable);
+			else
+				ata_dev_info(&ap->link.device[i], "acpi: power failed to be set\n");
+		}
+	} else {
+		if (!is_acpi_device_node(ap->tdev.fwnode))
+			return;
+		handle = ACPI_HANDLE(&ap->tdev);
+		if (!acpi_bus_power_manageable(handle))
+			return;
+
+		if (!acpi_bus_set_power(handle, state))
+			ata_port_info(ap, "acpi: power was set to %d\n", enable);
+		else
+			ata_port_info(ap, "acpi: power failed to be set\n");
+	}
+}
+
 /**
  * ata_acpi_dissociate - dissociate ATA host from ACPI objects
  * @host: target ATA host
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index ff53f5f029b4..a52b916af14c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5899,11 +5899,32 @@ void ata_host_init(struct ata_host *host, struct device *dev,
 }
 EXPORT_SYMBOL_GPL(ata_host_init);
 
+static inline void ata_port_set_power_state(struct ata_port *ap, bool enable)
+{
+	ata_acpi_port_set_power_state(ap, enable);
+	// Maybe add a way with device tree and regulators too?
+}
+
+/**
+ * ata_dev_manage_restart - if the disk should be stopped (spin down) on system restart.
+ * @dev: target ATA device
+ *
+ * RETURNS:
+ * true if the disk should be stopped, otherwise false
+ */
+bool ata_dev_manage_restart(struct ata_device *dev)
+{
+	return ata_acpi_dev_manage_restart(dev);
+}
+EXPORT_SYMBOL_GPL(ata_dev_manage_restart);
+
 void ata_port_probe(struct ata_port *ap)
 {
 	struct ata_eh_info *ehi = &ap->link.eh_info;
 	unsigned long flags;
 
+	ata_port_set_power_state(ap, true);
+
 	/* kick EH for boot probing */
 	spin_lock_irqsave(ap->lock, flags);
 
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2ded5e476d6e..52297d9e3dc2 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1095,6 +1095,7 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct queue_limits *lim,
 		 */
 		sdev->manage_runtime_start_stop = 1;
 		sdev->manage_shutdown = 1;
+		sdev->manage_restart = ata_dev_manage_restart(dev);
 		sdev->force_runtime_start_on_system_start = 1;
 	}
 
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index e5b977a8d3e1..28cb652d99bc 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -130,6 +130,8 @@ extern void ata_acpi_on_disable(struct ata_device *dev);
 extern void ata_acpi_set_state(struct ata_port *ap, pm_message_t state);
 extern void ata_acpi_bind_port(struct ata_port *ap);
 extern void ata_acpi_bind_dev(struct ata_device *dev);
+extern void ata_acpi_port_set_power_state(struct ata_port *ap, bool enable);
+extern bool ata_acpi_dev_manage_restart(struct ata_device *dev);
 extern acpi_handle ata_dev_acpi_handle(struct ata_device *dev);
 #else
 static inline void ata_acpi_dissociate(struct ata_host *host) { }
@@ -140,6 +142,8 @@ static inline void ata_acpi_set_state(struct ata_port *ap,
 				      pm_message_t state) { }
 static inline void ata_acpi_bind_port(struct ata_port *ap) {}
 static inline void ata_acpi_bind_dev(struct ata_device *dev) {}
+static inline void ata_acpi_port_set_power_state(struct ata_port *ap, bool enable) {}
+static inline bool ata_acpi_dev_manage_restart(struct ata_device *dev) { return false; }
 #endif
 
 /* libata-scsi.c */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 0620dd67369f..af5974e91e1d 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1302,6 +1302,7 @@ extern int sata_link_debounce(struct ata_link *link,
 extern int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy,
 			     bool spm_wakeup);
 extern int ata_slave_link_init(struct ata_port *ap);
+extern bool ata_dev_manage_restart(struct ata_device *dev);
 extern void ata_port_probe(struct ata_port *ap);
 extern struct ata_port *ata_port_alloc(struct ata_host *host);
 extern void ata_port_free(struct ata_port *ap);
-- 
2.49.1