Re: libahci driver and power switching HDD on newer kernels

[Niklas Cassel <cassel@kernel.org>]

On Tue, Sep 24, 2024 at 12:42:10PM +0200, W wrote:
> > 
> > Given that you had 6.4.12 working OK, it is likely some commit that introduced a
> > regression. If you can git bisect it, we will have a better idea how to remove
> > the regression.
> 
> Please take a look at bugzilla report:
> https://bugzilla.kernel.org/show_bug.cgi?id=219296 - there are the details.
> 
> I'm wondering what is the better way for communication - here on mailing
> list or put the comments in bugzilla ticket?
> Probably here will be better idea...
> 
> W
> 

Hello W,

Could you please try the following patch,
and see if it helps:


From dba01b7d68fffc26f3abf3252296082311a767a0 Mon Sep 17 00:00:00 2001
From: Niklas Cassel <cassel@kernel.org>
Date: Wed, 2 Oct 2024 21:40:41 +0200
Subject: [PATCH] ata: libata: do not spin down disk on PM event freeze

Currently, ata_eh_handle_port_suspend() will return early if
ATA_PFLAG_PM_PENDING is not set, or if the PM event has flag
PM_EVENT_RESUME set.

This means that the following PM callbacks:
.suspend = ata_port_pm_suspend,
.freeze = ata_port_pm_freeze,
.poweroff = ata_port_pm_poweroff,
.runtime_suspend = ata_port_runtime_suspend,
will actually make ata_eh_handle_port_suspend() perform some work.

ata_eh_handle_port_suspend() will spin down the disks (by calling
ata_dev_power_set_standby()), regardless of the PM event.

Documentation/driver-api/pm/devices.rst, section "Entering Hibernation",
explicitly mentions that .freeze() does not have to be put the device in
a low-power state, and actually recommends not doing so. Thus, let's not
spin down the disk for the .freeze() callback. (The disk will instead be
spun down during the succeeding .poweroff() callback.)

Fixes: aa3998dbeb3a ("ata: libata-scsi: Disable scsi device manage_system_start_stop")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-eh.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 3f0144e7dc80..45a0d9af2d54 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -4099,10 +4099,20 @@ static void ata_eh_handle_port_suspend(struct ata_port *ap)
 
        WARN_ON(ap->pflags & ATA_PFLAG_SUSPENDED);
 
-       /* Set all devices attached to the port in standby mode */
-       ata_for_each_link(link, ap, HOST_FIRST) {
-               ata_for_each_dev(dev, link, ENABLED)
-                       ata_dev_power_set_standby(dev);
+       /*
+        * We will reach this point for all of the PM events:
+        * PM_EVENT_SUSPEND (if runtime pm, PM_EVENT_AUTO will also be set)
+        * PM_EVENT_FREEZE, and PM_EVENT_HIBERNATE.
+        *
+        * We do not want to perform disk spin down for PM_EVENT_FREEZE.
+        * (Spin down will be performed by the succeeding PM_EVENT_HIBERNATE.)
+        */
+       if (!(ap->pm_mesg.event & PM_EVENT_FREEZE)) {
+               /* Set all devices attached to the port in standby mode */
+               ata_for_each_link(link, ap, HOST_FIRST) {
+                       ata_for_each_dev(dev, link, ENABLED)
+                               ata_dev_power_set_standby(dev);
+               }
        }
 
        /*
-- 
2.46.2