Some embedded devices have the ability to control whether power is
provided to the disks via the SATA power connector or not. ACPI power
resources are usually off by default, thus making it unclear if the
specific power resource will retain its state after a restart. If power
resources are defined on ATA ports / devices in ACPI, we should stop the
disk on SYSTEM_RESTART, to ensure the disk will not lose power while
active.

Add a new function, ata_acpi_dev_manage_restart(), that will be used to
determine if a disk should be stopped before restarting the system. If a
usable ACPI power resource has been found, it is assumed that the disk
will lose power after a restart and should be stopped to avoid a power
failure.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/ata/libata-acpi.c | 28 ++++++++++++++++++++++++++++
 drivers/ata/libata-scsi.c |  1 +
 drivers/ata/libata.h      |  2 ++
 3 files changed, 31 insertions(+)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index f2140fc06ba0..f05c247d25bc 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -245,6 +245,34 @@ void ata_acpi_bind_dev(struct ata_device *dev)
 				   ata_acpi_dev_uevent);
 }
 
+/**
+ * ata_acpi_dev_manage_restart - if the disk should be stopped (spun down) on
+ * system restart.
+ * @dev: target ATA device
+ *
+ * RETURNS:
+ * 1 if the disk should be stopped, otherwise 0
+ */
+bool ata_acpi_dev_manage_restart(struct ata_device *dev)
+{
+	/* If the device is power manageable, we assume the disk loses power on
+	 * reboot.
+	 */
+
+	/* If `ATA_FLAG_ACPI_SATA` is set, the acpi fwnode is attached to the
+	 * `ata_device` instead of the `ata_port`.
+	 */
+	if (dev->link->ap->flags & ATA_FLAG_ACPI_SATA) {
+		if (!is_acpi_device_node(dev->tdev.fwnode))
+			return 0;
+		return acpi_bus_power_manageable(ACPI_HANDLE(&dev->tdev));
+	}
+
+	if (!is_acpi_device_node(dev->link->ap->tdev.fwnode))
+		return 0;
+	return acpi_bus_power_manageable(ACPI_HANDLE(&dev->link->ap->tdev));
+}
+
 /**
  * ata_acpi_dissociate - dissociate ATA host from ACPI objects
  * @host: target ATA host
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index b43a3196e2be..026122bb6f2f 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_acpi_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..af08bb9b40d0 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -130,6 +130,7 @@ 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 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 +141,7 @@ 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 bool ata_acpi_dev_manage_restart(struct ata_device *dev) { return 0; }
 #endif
 
 /* libata-scsi.c */
-- 
2.51.0

On 2025/10/23 9:52, Markus Probst wrote:
> Some embedded devices have the ability to control whether power is
> provided to the disks via the SATA power connector or not. ACPI power
> resources are usually off by default, thus making it unclear if the
> specific power resource will retain its state after a restart. If power
> resources are defined on ATA ports / devices in ACPI, we should stop the
> disk on SYSTEM_RESTART, to ensure the disk will not lose power while
> active.
> 
> Add a new function, ata_acpi_dev_manage_restart(), that will be used to
> determine if a disk should be stopped before restarting the system. If a
> usable ACPI power resource has been found, it is assumed that the disk
> will lose power after a restart and should be stopped to avoid a power
> failure.
> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  drivers/ata/libata-acpi.c | 28 ++++++++++++++++++++++++++++
>  drivers/ata/libata-scsi.c |  1 +
>  drivers/ata/libata.h      |  2 ++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
> index f2140fc06ba0..f05c247d25bc 100644
> --- a/drivers/ata/libata-acpi.c
> +++ b/drivers/ata/libata-acpi.c
> @@ -245,6 +245,34 @@ void ata_acpi_bind_dev(struct ata_device *dev)
>  				   ata_acpi_dev_uevent);
>  }
>  
> +/**
> + * ata_acpi_dev_manage_restart - if the disk should be stopped (spun down) on
> + * system restart.

Please align "system restart" to the "if" above.

> + * @dev: target ATA device
> + *
> + * RETURNS:
> + * 1 if the disk should be stopped, otherwise 0

s/1/true
s/0/false

And please terminate sentences with a period.

> + */
> +bool ata_acpi_dev_manage_restart(struct ata_device *dev)
> +{
> +	/* If the device is power manageable, we assume the disk loses power on
> +	 * reboot.
> +	 */

And then ? This seems incomplete...
Also please adhere to the normal multi-line comment style by starting the
multi-line comment block with "/*".

	/*
	 * If the device is power manageable, we assume the disk loses power on
	 * reboot.
	 */

> +
> +	/* If `ATA_FLAG_ACPI_SATA` is set, the acpi fwnode is attached to the
> +	 * `ata_device` instead of the `ata_port`.
> +	 */

No need for all the quote marks here.

> +	if (dev->link->ap->flags & ATA_FLAG_ACPI_SATA) {
> +		if (!is_acpi_device_node(dev->tdev.fwnode))
> +			return 0;

			return false;

> +		return acpi_bus_power_manageable(ACPI_HANDLE(&dev->tdev));
> +	}
> +
> +	if (!is_acpi_device_node(dev->link->ap->tdev.fwnode))
> +		return 0;

		return false;

> +	return acpi_bus_power_manageable(ACPI_HANDLE(&dev->link->ap->tdev));
> +}

Overall, I think the following is cleaner and simpler as it doe not repeat code:

bool ata_acpi_dev_manage_restart(struct ata_device *dev)
{
	struct device *tdev;

	/*
	 * If ATA_FLAG_ACPI_SATA is set, the acpi fwnode is attached to the
	 * ATA device instead of the port.
	 */
	if (dev->link->ap->flags & ATA_FLAG_ACPI_SATA)
		tdev = &dev->tdev;
	else
		tdev = &dev->link->ap->tdev;

	if (!is_acpi_device_node(tdev->fwnode))
		return false;

	return acpi_bus_power_manageable(ACPI_HANDLE(tdev));
}

> +
>  /**
>   * ata_acpi_dissociate - dissociate ATA host from ACPI objects
>   * @host: target ATA host
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index b43a3196e2be..026122bb6f2f 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_acpi_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..af08bb9b40d0 100644
> --- a/drivers/ata/libata.h
> +++ b/drivers/ata/libata.h
> @@ -130,6 +130,7 @@ 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 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 +141,7 @@ 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 bool ata_acpi_dev_manage_restart(struct ata_device *dev) { return 0; }
>  #endif
>  
>  /* libata-scsi.c */


-- 
Damien Le Moal
Western Digital Research