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