Re: [PATCH] ata: libata: disallow dev-initiated LPM transitions to unsupported states

[Damien Le Moal <dlemoal@kernel.org>]

On 9/5/23 17:26, Niklas Cassel wrote:
> On Tue, Sep 05, 2023 at 11:29:13AM +0900, Damien Le Moal wrote:
>> On 9/5/23 05:42, Niklas Cassel wrote:
>>> From: Niklas Cassel <niklas.cassel@wdc.com>
>>>
>>> In AHCI 1.3.1, the register description for CAP.SSC:
>>> "When cleared to ‘0’, software must not allow the HBA to initiate
>>> transitions to the Slumber state via agressive link power management nor
>>> the PxCMD.ICC field in each port, and the PxSCTL.IPM field in each port
>>> must be programmed to disallow device initiated Slumber requests."
>>>
>>> In AHCI 1.3.1, the register description for CAP.PSC:
>>> "When cleared to ‘0’, software must not allow the HBA to initiate
>>> transitions to the Partial state via agressive link power management nor
>>> the PxCMD.ICC field in each port, and the PxSCTL.IPM field in each port
>>> must be programmed to disallow device initiated Partial requests."
>>>
>>> Ensure that we always set the corresponding bits in PxSCTL.IPM, such that
>>> a device is not allowed to initiate transitions to power states which are
>>> unsupported by the HBA.
>>>
>>> DevSleep is always initiated by the HBA, however, for completeness, set the
>>> corresponding bit in PxSCTL.IPM such that agressive link power management
>>> cannot transition to DevSleep if DevSleep is not supported.
>>>
>>> sata_link_scr_lpm() is used by libahci, ata_piix and libata-pmp.
>>> However, only libahci has the ability to read the CAP/CAP2 register to see
>>> if these features are supported. Therefore, in order to not introduce any
>>> regressions on ata_piix or libata-pmp, create flags that indicate that the
>>> respective feature is NOT supported. This way, the behavior for ata_piix
>>> and libata-pmp should remain unchanged.
>>>
>>> This change is based on a patch originally submitted by Runa Guo-oc.
>>>
>>> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
>>
>> Looks good, but don't we need a Fixes tag for this ?
> 
> Sure
> 
> Fixes: 1152b2617a6e ("libata: implement sata_link_scr_lpm() and make ata_dev_set_feature() global")
> 
> Can you add when applying, or do you want me to resend?

OK. Will add that. No need to resend.

> 
> 
> Kind regards,
> Niklas

-- 
Damien Le Moal
Western Digital Research