Skip to content

Commit a374cfb

Browse files
floatiousdamien-lemoal
authored andcommitted
ata: libata-eh: Keep DIPM disabled while modifying the allowed LPM states
Currently, it is possible that LPM is enabled while calling the set_lpm() callback. The current code performs a SET FEATURES command to disable DIPM if policy < ATA_LPM_MED_POWER_WITH_DIPM, this means that it will currently disable DIPM for policies: ATA_LPM_UNKNOWN, ATA_LPM_MAX_POWER, ATA_LPM_MED_POWER (but not for policy ATA_LPM_MED_POWER_WITH_DIPM). The code called after calling the set_lpm() callback will later perform a SET FEATURES command to enable DIPM, if policy >= ATA_LPM_MED_POWER_WITH_DIPM. As we can see DIPM will not be disabled before calling set_lpm() if the LPM policy is: ATA_LPM_MED_POWER_WITH_DIPM, ATA_LPM_MIN_POWER_WITH_PARTIAL, or ATA_LPM_MIN_POWER. Make sure that we always disable DIPM before calling the set_lpm() callback. This is because the set_lpm() callback is the function (for AHCI) that sets the proper bits in PxSCTL.IPM, reflecting the support of the HBA. PxSCTL.IPM controls the LPM states that the device is allowed to enter. If the device tries to enter a state disabled by PxSCTL.IPM, the host will NAK the transition. If we do not disable DIPM before modifying PxSCTL.IPM, it is possible that DIPM will try (and will be allowed to) enter a LPM state that the HBA does not support (since we have not yet written PxSCTL.IPM, the HBA wasn't able to NAK the transition). While at it, remove the guard of host support for DIPM around the disabling of DIPM. While it makes sense to take host support for DIPM into account when enabling DIPM, it makes zero sense to take host support into account when disabling DIPM. If the host does not support DIPM, that is an even bigger reason why DIPM should be disabled on the device side. Signed-off-by: Niklas Cassel <cassel@kernel.org> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
1 parent 6d915e2 commit a374cfb

File tree

1 file changed

+1
-2
lines changed

1 file changed

+1
-2
lines changed

drivers/ata/libata-eh.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3471,8 +3471,7 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
34713471
hints &= ~ATA_LPM_HIPM;
34723472

34733473
/* disable DIPM before changing link config */
3474-
if (policy < ATA_LPM_MED_POWER_WITH_DIPM &&
3475-
(dev_has_dipm && host_has_dipm)) {
3474+
if (dev_has_dipm) {
34763475
err_mask = ata_dev_set_feature(dev,
34773476
SETFEATURES_SATA_DISABLE, SATA_DIPM);
34783477
if (err_mask && err_mask != AC_ERR_DEV) {

0 commit comments

Comments
 (0)