On Mon, Jun 30, 2025 at 03:26:35PM +0900, Damien Le Moal wrote:
> Commit fa997b0576c9 ("ata: ahci: Do not enable LPM if no LPM states are
> supported by the HBA") introduced an early return in
> ahci_update_initial_lpm_policy() to ensure that the target_lpm_policy
> of ports belonging to a host that does not support the Partial, Slumber
> and DevSleep power states is unchanged and remains set to
> ATA_LPM_UNKNOWN and thus prevents the execution of
> ata_eh_link_set_lpm().
>
> However, a user or a system daemon (e.g. systemd-udevd) may still
> attempt changing the LPM policy through the sysfs
> link_power_management_policy of the host.
>
> Improve this to prevent sysfs LPM policy changes by setting the flag
> ATA_FLOAG_NO_LPM for the port of such host, and initialize the port
> target_lpm_policy to ATA_LPM_MAX_POWER to guarantee that no unsupported
> low power state is being used on the port and its link.
>
> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> ---
> drivers/ata/ahci.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 34698ae39f55..737f5d1bde11 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1793,7 +1793,10 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap)
> if ((ap->host->flags & ATA_HOST_NO_PART) &&
> (ap->host->flags & ATA_HOST_NO_SSC) &&
> (ap->host->flags & ATA_HOST_NO_DEVSLP)) {
> - ata_port_dbg(ap, "no LPM states supported, not enabling LPM\n");
> + ata_port_warn(ap,
> + "No LPM states supported, forcing LPM max_power\n");
Do we really want this to be a warning?
I don't think there is anything wrong with an HBA that does not support any
LPM states, so it seems a bit excessive to give a warning about it.
e.g. [PATCH 06/10] ata: ahci: Disable DIPM if host lacks support
was only a at_port_dbg().
Or, if you really want to keep this warning, then perhaps we should move this
if-statement
(and the ap->pflags & ATA_PFLAG_EXTERNAL if-statement)
below the
update_policy:
label
And change it to:
if (policy != ATA_LPM_MAX_POWER &&
(ap->host->flags & ATA_HOST_NO_PART) &&
(ap->host->flags & ATA_HOST_NO_SSC) &&
(ap->host->flags & ATA_HOST_NO_DEVSLP)) {
(and add the same policy != ATA_LPM_MAX_POWER &&
guard to the ap->pflags & ATA_PFLAG_EXTERNAL if-statement)
But I think that I prefer to just keep it as ata_port_dbg().
Kind regards,
Niklas
