Re: [PATCH v2 2/2] ata: Use ACPI methods to power on ata ports

[Niklas Cassel <cassel@kernel.org>]

On Thu, Oct 09, 2025 at 11:24:49AM +0000, Markus Probst wrote:
> Some embedded devices, including many Synology NAS devices, have the
> ability to control whether a ATA port has power or not.

In V1, you mentioned that it was to control the SATA power supply,
now you mention the ATA port. I am confused.

If it is for the ATA port, then SATA already has support for this,
using PxSCTL.

How does this ACPI way to control power interact with the regular
way to control power for a port using PxSCTL?


> 
> 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. Also add a new function, ata_acpi_port_set_power_state(), that
> will be used to power on an ata port if usable ACPI power resources are
> found. It will be called right before probing the port, therefore the port
> will be powered on just in time.
> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  drivers/ata/libata-acpi.c | 70 +++++++++++++++++++++++++++++++++++++++
>  drivers/ata/libata-core.c |  2 ++
>  drivers/ata/libata-scsi.c |  1 +
>  drivers/ata/libata.h      |  4 +++
>  4 files changed, 77 insertions(+)
> 
> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
> index f2140fc06ba0..bba5ef49f055 100644
> --- a/drivers/ata/libata-acpi.c
> +++ b/drivers/ata/libata-acpi.c
> @@ -245,6 +245,76 @@ 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.

Like Damien mentioned earlier, please no C++ style comments.


Kind regards,
Niklas