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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aOejov5d_TlVkueH@ryzen>
Date: Thu, 9 Oct 2025 13:59:30 +0200
From: Niklas Cassel <cassel@...nel.org>
To: Markus Probst <markus.probst@...teo.de>
Cc: Damien Le Moal <dlemoal@...nel.org>,
	"James E . J . Bottomley" <James.Bottomley@...senpartnership.com>,
	"Martin K . Petersen" <martin.petersen@...cle.com>,
	linux-ide@...r.kernel.org, linux-scsi@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] ata: Use ACPI methods to power on ata ports

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@...teo.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