Re: [PATCH] ata: libata-core: Add ATA_HORKAGE_NOLPM for Apacer AS340

[Niklas Cassel <cassel@kernel.org>]

On Thu, May 30, 2024 at 11:27:04PM +0200, Niklas Cassel wrote:
> Commit 7627a0edef54 ("ata: ahci: Drop low power policy board type")
> dropped the board_ahci_low_power board type, and instead enables LPM if:
> -The AHCI controller reports that it supports LPM (Partial/Slumber), and
> -CONFIG_SATA_MOBILE_LPM_POLICY != 0, and
> -The port is not defined as external in the per port PxCMD register, and
> -The port is not defined as hotplug capable in the per port PxCMD
>  register.
> 
> Partial and Slumber LPM states can either be initiated by HIPM or DIPM.
> 
> For HIPM (host initiated power management) to get enabled, both the AHCI
> controller and the drive have to report that they support HIPM.
> 
> For DIPM (device initiated power management) to get enabled, only the
> drive has to report that it supports DIPM. However, the HBA will reject
> device requests to enter LPM states which the HBA does not support.
> 
> The problem is that Apacer AS340 drives do not handle low power modes
> correctly. The problem was most likely not seen before because no one
> had used this drive with a AHCI controller with LPM enabled.
> 
> Add a quirk so that we do not enable LPM for this drive, since we see
> command timeouts if we do (even though the drive claims to support DIPM).
> 
> Fixes: 7627a0edef54 ("ata: ahci: Drop low power policy board type")
> Cc: stable@vger.kernel.org
> Reported-by: Tim Teichmann <teichmanntim@outlook.de>
> Closes: https://lore.kernel.org/linux-ide/87bk4pbve8.ffs@tglx/
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> On the system reporting this issue, the HBA supports SALP (HIPM) and
> LPM states Partial and Slumber.
> 
> This drive only supports DIPM but not HIPM, however, that should not
> matter, as a DIPM request from the device still has to be acked by the
> HBA, and according to AHCI 1.3.1, section 5.3.2.11 P:Idle, if the link
> layer has negotiated to low power state based on device power management
> request, the HBA will jump to state PM:LowPower.
> 
> In PM:LowPower, the HBA will automatically request to wake the link
> (exit from Partial/Slumber) when a new command is queued (by writing to
> PxCI). Thus, there should be no need for host software to request an
> explicit wakeup (by writing PxCMD.ICC to 1).
> 
> Therefore, even with only DIPM supported/enabled, we shouldn't see command
> timeouts with the current code. Also, only enabling only DIPM (by
> modifying the AHCI driver) with another drive (which support both DIPM
> and HIPM), shows no errors. Thus, it seems like the drive is the problem.
> 
>  drivers/ata/libata-core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 4f35aab81a0a..25b400f1c3de 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4155,6 +4155,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
>  						ATA_HORKAGE_ZERO_AFTER_TRIM |
>  						ATA_HORKAGE_NOLPM },
>  
> +	/* Apacer models with LPM issues */
> +	{ "Apacer AS340*",		NULL,	ATA_HORKAGE_NOLPM },
> +
>  	/* These specific Samsung models/firmware-revs do not handle LPM well */
>  	{ "SAMSUNG MZMPC128HBFU-000MV", "CXM14M1Q", ATA_HORKAGE_NOLPM },
>  	{ "SAMSUNG SSD PM830 mSATA *",  "CXM13D1Q", ATA_HORKAGE_NOLPM },
> -- 
> 2.45.1
> 

One interesting fact which:
Apacer AS340, CT240BX500SSD1, and R3SL240G all have in common:
their SSD controller is made by Silicon Motion:
https://smarthdd.com/database/Apacer-AS340-120GB/U1014A0/
https://smarthdd.com/database/CT240BX500SSD1/M6CR013/
https://smarthdd.com/database/R3SL240G/P0422C/

Not sure if that is relevant or not...


Kind regards,
Niklas