Re: [PATCH v2 4/5] ata: ahci: do not enable LPM on external ports

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

On 2/7/24 06:13, Niklas Cassel wrote:
> AHCI 1.3.3, 7.3.1.1 Software Flow for Hot Plug Removal Detection states:
> "To reliably detect hot plug removals, software must disable interface
> power management.
> 
> Software should perform the following initialization on a port after a
> device is attached:
> -Set PxSCTL.IPM to 3h to disable interface power management state
>  transitions.
> -Set PxCMD.ALPE to ‘0’ to disable aggressive power management.
> -Ensure PxIE.PRCE is set to ‘1’ to enable interrupts on hot plug removals.
> -Disable device initiated interface power management by issuing the
>  appropriate SET FEATURES command."
> 
> Further, AHCI 1.3.3, 7.3 Native Hot Plug Support states:
> "The HBA shall set the PxSERR.DIAG.X bit to ‘1’ when a COMINIT is received
> from the device. Hot plug insertions are detected via the PxIS.PCS bit
> that directly reflects the PxSERR.DIAG.X bit. The HBA shall set the
> PxSERR.DIAG.N bit to ‘1’ when the HBA’s internal PhyRdy signal changes
> state.
> 
> Hot plug removals are detected via the PxIS.PRCS bit that directly
> reflects the PxSERR.DIAG.N bit. Note that PxSERR.DIAG.N is also set
> to ‘1’ on insertions and during interface power management entry/exit."
> 
> ahci_set_lpm() already disables the PxIS.PRCS interrupt if setting a
> LPM policy != ATA_LPM_MAX_POWER, so we cannot detect hot plug removals
> when LPM policy != ATA_LPM_MAX_POWER.
> 
> We do have PxIS.PCS interrupt enabled even for LPM policy !=
> ATA_LPM_MAX_POWER, so we should theoretically still be able to detect
> hot plug insertions even when LPM is enabled.
> 
> However, in practise, for LPM policy ATA_LPM_MED_POWER_WITH_DIPM,
> ATA_LPM_MIN_POWER_WITH_PARTIAL, and ATA_LPM_MIN_POWER, if there is
> no link enabled, sata_link_scr_lpm() will set SControl.DET = 0x4,
> which will transition the port to the "P:Offline" state.
> 
> The P:Offline mode is described in SATA Gold 3.5a:
> 4.1.1.103 Phy offline:
> "In this mode the host Phy is forced off and the host Phy does not
> recognize nor respond to COMINIT or COMWAKE. This mode is entered by
> setting the DET field of the SControl register to 0100b. This is a
> mechanism for the host to turn off its Phy."
> 
> So in the P:Offline state the PHY does not recognize the unsolicited
> COMINIT which is sent on a hot plug insertion.
> 
> While we could change sata_link_scr_lpm() to never power off an external
> port for LPM policy != ATA_LPM_MAX_POWER (in order be able to handle hot
> plug insertions), we still would not be able to handle hot plug removals.
> 
> Thus, simply modify ahci_update_initial_lpm_policy() to not enable LPM if
> the port advertises itself as an external port, as this function is
> already being used to set/override the initial LPM policy.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research