Re: [BUG] ata: ahci: regression in f7870e8d345c disables DevSleep for ACS-5 drives on PSC=0/SSC=0/SDS=1 controllers

Re: [BUG] ata: ahci: regression in f7870e8d345c disables DevSleep for ACS-5 drives on PSC=0/SSC=0/SDS=1 controllers

[Damien Le Moal]

On 2026/04/05 21:01, Patrick Zima wrote:
> Commit f7870e8d345c ("ata: ahci: Disable DIPM if host lacks support") introduced
>   a regression: it disables DIPM — and thereby DevSleep — on drives behind AHCI
>   controllers that have CAP.PSC=0 and CAP.SSC=0 but CAP2.SDS=1 (DevSleep capable).
>   The guard condition is too broad; it treats the absence of Partial/Slumber state
>   bits as grounds to disable DIPM, but DevSleep does not depend on Partial or
>   Slumber being exposed to the OS — only on SDS=1 in CAP2.
> 
>   --- The logic flaw ---
> 
>   The new code (simplified):
> 
>     if (!(cap & HOST_CAP_PART) || !(cap & HOST_CAP_SSC))
>         set ATA_FLAG_NO_DIPM on port
>         → SETFEATURES DISABLE DIPM to all drives with word 78 bit 3 set
> 
>   On a controller with PSC=0, SSC=0 but SDS=1, this fires unconditionally,
>   even though the controller is fully capable of negotiating DevSleep.
>   The flag ATA_FLAG_NO_DIPM then also suppresses ALPM port behaviour at the
>   kernel level — re-enabling DIPM at the drive level via SETFEATURES after the
>   fact does not restore DevSleep.
> 
>   --- Suggested fix ---
> 
>   Gate the DIPM-disable path on the absence of DevSleep support:
> 
>     if (!(cap & HOST_CAP_PART) && !(cap & HOST_CAP_SSC)
>   +     && !(cap2 & HOST_CAP2_SDS)
>         && (ap->flags & ATA_FLAG_DIPM)) {
> 
>   If the controller can do DevSleep (SDS=1), there is no reason to disable
>   DIPM — the power state sequence Active → DevSleep does not require Partial
>   or Slumber to be present.

Instead of sending what looks like an AI generated report, what about you
actually generate a proper patch from the suggested fix and test it ? And send
said patch if the suggested fix works ?


-- 
Damien Le Moal
Western Digital Research

Re: [BUG] ata: ahci: regression in f7870e8d345c disables DevSleep for ACS-5 drives on PSC=0/SSC=0/SDS=1 controllers

[Niklas Cassel]

On Mon, Apr 06, 2026 at 09:30:34AM +0200, Damien Le Moal wrote:
> On 2026/04/05 21:01, Patrick Zima wrote:
> > Commit f7870e8d345c ("ata: ahci: Disable DIPM if host lacks support") introduced
> >   a regression: it disables DIPM — and thereby DevSleep — on drives behind AHCI
> >   controllers that have CAP.PSC=0 and CAP.SSC=0 but CAP2.SDS=1 (DevSleep capable).
> >   The guard condition is too broad; it treats the absence of Partial/Slumber state
> >   bits as grounds to disable DIPM, but DevSleep does not depend on Partial or
> >   Slumber being exposed to the OS — only on SDS=1 in CAP2.
> > 
> >   --- The logic flaw ---
> > 
> >   The new code (simplified):
> > 
> >     if (!(cap & HOST_CAP_PART) || !(cap & HOST_CAP_SSC))
> >         set ATA_FLAG_NO_DIPM on port
> >         → SETFEATURES DISABLE DIPM to all drives with word 78 bit 3 set
> > 
> >   On a controller with PSC=0, SSC=0 but SDS=1, this fires unconditionally,
> >   even though the controller is fully capable of negotiating DevSleep.
> >   The flag ATA_FLAG_NO_DIPM then also suppresses ALPM port behaviour at the
> >   kernel level — re-enabling DIPM at the drive level via SETFEATURES after the
> >   fact does not restore DevSleep.
> > 
> >   --- Suggested fix ---
> > 
> >   Gate the DIPM-disable path on the absence of DevSleep support:
> > 
> >     if (!(cap & HOST_CAP_PART) && !(cap & HOST_CAP_SSC)
> >   +     && !(cap2 & HOST_CAP2_SDS)
> >         && (ap->flags & ATA_FLAG_DIPM)) {
> > 
> >   If the controller can do DevSleep (SDS=1), there is no reason to disable
> >   DIPM — the power state sequence Active → DevSleep does not require Partial
> >   or Slumber to be present.
> 
> Instead of sending what looks like an AI generated report, what about you
> actually generate a proper patch from the suggested fix and test it ? And send
> said patch if the suggested fix works ?

From AHCI 1.3.1, 8.3.1.4 Software Requirements and Precedence:
"If CAP.SSC or CAP.PSC is cleared to ‘0’, software should disable
device-initiated power management by issuing the appropriate SET FEATURES
command to the device."


From AHCI 1.3.1, 8.5 Device Sleep (DEVSLP) Feature:
"Entry to the DevSleep interface state may be accomplished autonomously by
the HBA via the aggressive Device Sleep mechanism or through software
utilizing the PxCMD.ICC field (similar to how Partial and Slumber are
managed). Device Sleep is never initiated by the device."


Since DevSleep is never initiated by the device, doing a SET FEATURES to
disable device-initiated power management should be irrelavant to how
DevSleep works.


I'm not saying that there is not a problem with DevSleep, but the AI
generated analysis and suggested fix seems totally wrong.


Kind regards,
Niklas

Re: [BUG] ata: ahci: regression in f7870e8d345c disables DevSleep for ACS-5 drives on PSC=0/SSC=0/SDS=1 controllers

[Niklas Cassel]

Hello Patrick,

On Mon, Apr 06, 2026 at 05:51:39PM +0200, Patrick Zima wrote:
> You are right, (and yes to Damien, I had AI consolidate my findings)
> I need to investigate further - my drives definitely hibernate with kernel -18 and remain sleeping only with -20 
> 
> but ...
> - the commit doesn't really seem to affect my controller ASM1164 (AHCI_HFLAG_USE_LPM_POLICY) as it seems to fire for Intel mobile chipsets only                                             
> - still, the libata.ko and libahci.ko are identical between my two kernel versions — no LPM code changed for this controller
> - I initially followed the path: link_power_management_policy reverting to keep_firmware_settings after afirst reboot after -20 kernel installed
> 
> To sum it up, my initial result, leading me to file the bug was wrong
> 
> Sorry

No need to say sorry, it sounds like you have a regression on real hardware
(which was not clear from your previous mail).

Please provide as much information as possible, so we can help you find the
root cause.

Can you please test with the latest upstream kernel? 7.0-rc7 to see if the
problem still exists there? (Your distro usually provides the config it is
using in /boot/config-*, and it makes sense to use the same config as your
distro.)


Kind regards,
Niklas