diff mbox series

ata: libata-core: Add ATA_HORKAGE_NOLPM for Apacer AS340

Message ID 20240530212703.561517-2-cassel@kernel.org
State New
Headers show
Series ata: libata-core: Add ATA_HORKAGE_NOLPM for Apacer AS340 | expand

Commit Message

Niklas Cassel May 30, 2024, 9:27 p.m. UTC
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(+)

Comments

Niklas Cassel May 30, 2024, 9:55 p.m. UTC | #1
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
Damien Le Moal May 31, 2024, midnight UTC | #2
On 5/31/24 6:27 AM, 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>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Damien Le Moal May 31, 2024, 12:01 a.m. UTC | #3
On 5/31/24 6:55 AM, Niklas Cassel wrote:
> 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...

Well, I guess that probably explains why they all have the same issue with LPM.
Mika Westerberg May 31, 2024, 12:59 p.m. UTC | #4
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>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Niklas Cassel May 31, 2024, 1:17 p.m. UTC | #5
On Thu, 30 May 2024 23:27:04 +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.
> 
> [...]

Applied, thanks!

[1/1] ata: libata-core: Add ATA_HORKAGE_NOLPM for Apacer AS340
      commit: 3cb648c4dd3e8dde800fb3659250ed11f2d9efa5

Best regards,
diff mbox series

Patch

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 },