ata: Extend Samsung LPM quirk to AMD and fix force documentation
Subsystems (2)
Niklas Cassel
maintainercover ata: Extend Samsung LPM quirk to AMD and fix force documentation
Hello,
A user reports that a Samsung SSD 870 QVO 8TB connected to an AMD 600 Series chipset SATA controller times out on STANDBY IMMEDIATE during system suspend with med_power_with_dipm enabled. The command completes with max_performance instead.
Patch 1 extends the existing Samsung LPM quirk from ATI to AMD controllers, renaming it to ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD. It keeps the same drive model matches and leaves the NCQ quirk restricted to ATI. LPM behavior on other controller vendors, including Intel, is unchanged. The underlying controller/device interaction has not been root-caused.
Patch 2 separately corrects the libata.force parameter documentation:
the supported PIO range, the scope of ncqati, and the set-only behavior of max_sec_1024 and max_sec_lba48. It introduces no new force options.
Note that the ATI (and now AMD) LPM quirk never had a libata.force option, and neither do we want to add one.
If a user wants to force enable/disable lpm, they can use the existing [no]lpm libata.force parameter. We therefore do not have and documentation in kernel-parameters.txt to change.
Bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=221986
Kind regards,
Niklas
Niklas Cassel (2):
ata: libata-core: Extend Samsung LPM quirk to AMD controllers
ata: libata: Correct libata.force parameter documentation
Documentation/admin-guide/kernel-parameters.txt | 10 ++++------ drivers/ata/libata-core.c | 15 ++++++++------- include/linux/libata.h | 4 ++-- 3 files changed, 14 insertions(+), 15 deletions(-)
Signature · 1 line
2.55.0
Niklas Cassel
maintainerPATCH 1/2 ata: libata-core: Extend Samsung LPM quirk to AMD controllers
A Samsung SSD 870 QVO 8TB connected to an AMD 600 Series chipset SATA controller is reported to time out on STANDBY IMMEDIATE during system suspend with med_power_with_dipm enabled. The command completes when using max_performance instead.
The existing Samsung LPM quirk only matches ATI controllers, leaving AMD controllers unaffected. Rename it to ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD and extend the vendor check to AMD for the same Samsung SSD model patterns. Keep LPM behavior unchanged for other controller vendors, including Intel.
Leave ATA_QUIRK_NO_NCQ_ON_ATI restricted to ATI, since the reported AMD issue concerns LPM rather than NCQ.
--- drivers/ata/libata-core.c | 15 ++++++++------- include/linux/libata.h | 4 ++-- 2 files changed, 10 insertions(+), 9 deletions(-)
drivers/ata/libata-core.c +8 -7
@@ -2972,9 +2972,10 @@ static void ata_dev_config_lpm(struct ata_device *dev) (dev->id[ATA_ID_SATA_CAPABILITY] & 0xe) == 0x2) dev->quirks |= ATA_QUIRK_NOLPM; -/* ATI specific quirk */ -if ((dev->quirks &ATA_QUIRK_NO_LPM_ON_ATI ) && -ata_dev_check_adapter (dev,PCI_VENDOR_ID_ATI )) +/* ATI and AMD specific quirk */ +if ((dev->quirks &ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD ) && + (ata_dev_check_adapter (dev,PCI_VENDOR_ID_ATI ) || +ata_dev_check_adapter (dev,PCI_VENDOR_ID_AMD ))) dev->quirks |=ATA_QUIRK_NOLPM ; }
@@ -4136,7 +4137,7 @@ static const char * const ata_quirk_names[] = { [__ATA_QUIRK_MAX_SEC] = "maxsec", [__ATA_QUIRK_MAX_TRIM_128M] = "maxtrim128m", [__ATA_QUIRK_NO_NCQ_ON_ATI] = "noncqonati", - [__ATA_QUIRK_NO_LPM_ON_ATI] = "nolpmonati", + [__ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD] = "nolpmonatiandamd", [__ATA_QUIRK_NO_ID_DEV_LOG] = "noiddevlog", [__ATA_QUIRK_NO_LOG_DIR] = "nologdir", [__ATA_QUIRK_NO_FUA] = "nofua",
@@ -4420,15 +4421,15 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = { {"Samsung SSD 860*" ,NULL ,ATA_QUIRK_NO_NCQ_TRIM |ATA_QUIRK_ZERO_AFTER_TRIM |ATA_QUIRK_NO_NCQ_ON_ATI | - ATA_QUIRK_NO_LPM_ON_ATI }, + ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD }, { "Samsung SSD 870*", NULL, ATA_QUIRK_NO_NCQ_TRIM |ATA_QUIRK_ZERO_AFTER_TRIM |ATA_QUIRK_NO_NCQ_ON_ATI | - ATA_QUIRK_NO_LPM_ON_ATI }, + ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD }, { "SAMSUNG*MZ7LH*", NULL, ATA_QUIRK_NO_NCQ_TRIM |ATA_QUIRK_ZERO_AFTER_TRIM |ATA_QUIRK_NO_NCQ_ON_ATI | - ATA_QUIRK_NO_LPM_ON_ATI }, + ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD }, { "FCCT*M500*", NULL, ATA_QUIRK_NO_NCQ_TRIM | ATA_QUIRK_ZERO_AFTER_TRIM },
include/linux/libata.h +2 -2
@@ -76,7 +76,7 @@ enum ata_quirks { __ATA_QUIRK_MAX_SEC,/* Limit max sectors */ __ATA_QUIRK_MAX_TRIM_128M,/* Limit max trim size to 128M */ __ATA_QUIRK_NO_NCQ_ON_ATI,/* Disable NCQ on ATI chipset */ - __ATA_QUIRK_NO_LPM_ON_ATI,/* Disable LPM on ATI chipset */ + __ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD,/* Disable LPM on ATI and AMD chipsets */ __ATA_QUIRK_NO_ID_DEV_LOG,/* Identify device log missing */ __ATA_QUIRK_NO_LOG_DIR,/* Do not read log directory */ __ATA_QUIRK_NO_FUA, /* Do not use FUA */
@@ -115,7 +115,7 @@ enum {ATA_QUIRK_MAX_SEC =BIT_ULL (__ATA_QUIRK_MAX_SEC),ATA_QUIRK_MAX_TRIM_128M =BIT_ULL (__ATA_QUIRK_MAX_TRIM_128M),ATA_QUIRK_NO_NCQ_ON_ATI =BIT_ULL (__ATA_QUIRK_NO_NCQ_ON_ATI), -ATA_QUIRK_NO_LPM_ON_ATI =BIT_ULL (__ATA_QUIRK_NO_LPM_ON_ATI), +ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD =BIT_ULL (__ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD),ATA_QUIRK_NO_ID_DEV_LOG =BIT_ULL (__ATA_QUIRK_NO_ID_DEV_LOG),ATA_QUIRK_NO_LOG_DIR =BIT_ULL (__ATA_QUIRK_NO_LOG_DIR), ATA_QUIRK_NO_FUA = BIT_ULL(__ATA_QUIRK_NO_FUA),
Signature · 1 line
2.55.0
Re: [PATCH 1/2] ata: libata-core: Extend Samsung LPM quirk to AMD controllers
Quoted 11 lines · Niklas Cassel
A Samsung SSD 870 QVO 8TB connected to an AMD 600 Series chipset SATA controller is reported to time out on STANDBY IMMEDIATE during system suspend with
med_power_with_dipmenabled. The command completes when usingmax_performanceinstead.The existing Samsung LPM quirk only matches ATI controllers, leaving AMD controllers unaffected. Rename it to
ATA_QUIRK_NO_LPM_ON_ATI_AND_AMDand extend the vendor check to AMD for the same Samsung SSD model patterns. Keep LPM behavior unchanged for other controller vendors, including Intel.
Quoted 5 lines
Leave
ATA_QUIRK_NO_NCQ_ON_ATIrestricted to ATI, since the reported AMD issue concerns LPM rather than NCQ.
Quoted 72 lines
drivers/ata/libata-core.c | 15 ++++++++------- include/linux/libata.h | 4 ++-- 2 files changed, 10 insertions(+), 9 deletions(-)drivers/ata/libata-core.c +8
-7@@ -2972,9 +2972,10 @@ static void ata_dev_config_lpm(struct ata_device *dev) (dev->id[ATA_ID_SATA_CAPABILITY] & 0xe) == 0x2) dev->quirks |= ATA_QUIRK_NOLPM; -/* ATI specific quirk */ -if ((dev->quirks &ATA_QUIRK_NO_LPM_ON_ATI ) && -ata_dev_check_adapter (dev,PCI_VENDOR_ID_ATI )) +/* ATI and AMD specific quirk */ +if ((dev->quirks &ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD ) && + (ata_dev_check_adapter (dev,PCI_VENDOR_ID_ATI ) || +ata_dev_check_adapter (dev,PCI_VENDOR_ID_AMD ))) dev->quirks |=ATA_QUIRK_NOLPM ; }@@ -4136,7 +4137,7 @@ static const char * const ata_quirk_names[] = { [__ATA_QUIRK_MAX_SEC] = "maxsec", [__ATA_QUIRK_MAX_TRIM_128M] = "maxtrim128m", [__ATA_QUIRK_NO_NCQ_ON_ATI] = "noncqonati", - [__ATA_QUIRK_NO_LPM_ON_ATI] = "nolpmonati", + [__ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD] = "nolpmonatiandamd", [__ATA_QUIRK_NO_ID_DEV_LOG] = "noiddevlog", [__ATA_QUIRK_NO_LOG_DIR] = "nologdir", [__ATA_QUIRK_NO_FUA] = "nofua",@@ -4420,15 +4421,15 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = { {"Samsung SSD 860*" ,NULL ,ATA_QUIRK_NO_NCQ_TRIM |ATA_QUIRK_ZERO_AFTER_TRIM |ATA_QUIRK_NO_NCQ_ON_ATI | - ATA_QUIRK_NO_LPM_ON_ATI }, + ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD }, { "Samsung SSD 870*", NULL, ATA_QUIRK_NO_NCQ_TRIM |ATA_QUIRK_ZERO_AFTER_TRIM |ATA_QUIRK_NO_NCQ_ON_ATI | - ATA_QUIRK_NO_LPM_ON_ATI }, + ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD }, { "SAMSUNG*MZ7LH*", NULL, ATA_QUIRK_NO_NCQ_TRIM |ATA_QUIRK_ZERO_AFTER_TRIM |ATA_QUIRK_NO_NCQ_ON_ATI | - ATA_QUIRK_NO_LPM_ON_ATI }, + ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD }, { "FCCT*M500*", NULL, ATA_QUIRK_NO_NCQ_TRIM | ATA_QUIRK_ZERO_AFTER_TRIM },include/linux/libata.h +2
-2@@ -76,7 +76,7 @@ enum ata_quirks { __ATA_QUIRK_MAX_SEC,/* Limit max sectors */ __ATA_QUIRK_MAX_TRIM_128M,/* Limit max trim size to 128M */ __ATA_QUIRK_NO_NCQ_ON_ATI,/* Disable NCQ on ATI chipset */ - __ATA_QUIRK_NO_LPM_ON_ATI,/* Disable LPM on ATI chipset */ + __ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD,/* Disable LPM on ATI and AMD chipsets */ __ATA_QUIRK_NO_ID_DEV_LOG,/* Identify device log missing */ __ATA_QUIRK_NO_LOG_DIR,/* Do not read log directory */ __ATA_QUIRK_NO_FUA, /* Do not use FUA */@@ -115,7 +115,7 @@ enum {ATA_QUIRK_MAX_SEC =BIT_ULL (__ATA_QUIRK_MAX_SEC),ATA_QUIRK_MAX_TRIM_128M =BIT_ULL (__ATA_QUIRK_MAX_TRIM_128M),ATA_QUIRK_NO_NCQ_ON_ATI =BIT_ULL (__ATA_QUIRK_NO_NCQ_ON_ATI), -ATA_QUIRK_NO_LPM_ON_ATI =BIT_ULL (__ATA_QUIRK_NO_LPM_ON_ATI), +ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD =BIT_ULL (__ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD),ATA_QUIRK_NO_ID_DEV_LOG =BIT_ULL (__ATA_QUIRK_NO_ID_DEV_LOG),ATA_QUIRK_NO_LOG_DIR =BIT_ULL (__ATA_QUIRK_NO_LOG_DIR), ATA_QUIRK_NO_FUA = BIT_ULL(__ATA_QUIRK_NO_FUA),
Re: [PATCH 1/2] ata: libata-core: Extend Samsung LPM quirk to AMD controllers
In reply to Niklas CasselQuoted 11 lines · Niklas Cassel
A Samsung SSD 870 QVO 8TB connected to an AMD 600 Series chipset SATA controller is reported to time out on STANDBY IMMEDIATE during system suspend with
med_power_with_dipmenabled. The command completes when usingmax_performanceinstead.The existing Samsung LPM quirk only matches ATI controllers, leaving AMD controllers unaffected. Rename it to
ATA_QUIRK_NO_LPM_ON_ATI_AND_AMDand extend the vendor check to AMD for the same Samsung SSD model patterns. Keep LPM behavior unchanged for other controller vendors, including Intel.
Quoted 5 lines
Leave
ATA_QUIRK_NO_NCQ_ON_ATIrestricted to ATI, since the reported AMD issue concerns LPM rather than NCQ.
Looks OK.
Signature · 2 lines
Damien Le Moal Western Digital Research
Niklas Cassel
maintainerPATCH 2/2 ata: libata: Correct libata.force parameter documentation
In reply to Niklas CasselAlign the documented libata.force options with their implementation. The force table accepts PIO modes 0 through 6, not mode 7, and ncqati controls NCQ generally rather than only queued TRIM.
The max_sec_1024 and max_sec_lba48 options only set transfer size limits. Remove the misleading claim that they can also clear them.
--- Documentation/admin-guide/kernel-parameters.txt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +4 -6
@@ -3491,7 +3491,7 @@ Kernel parameters * SATA link speed limit: 1.5Gbps or 3.0Gbps. - * Transfer mode: pio[0-7], mwdma[0-4] and udma[0-7]. + * Transfer mode: pio[0-6], mwdma[0-4] and udma[0-7]. udma[/][16,25,33,44,66,100,133] notation is also allowed.
@@ -3509,7 +3509,7 @@ Kernel parameters * [no]ncqtrim: Enable or disable queued DSM TRIM. - * [no]ncqati: Enable or disable NCQ trim on ATI chipset. + * [no]ncqati: Enable or disable NCQ on ATI chipsets. * [no]trim: Enable or disable (unqueued) TRIM.
@@ -3540,11 +3540,9 @@ Kernel parameters * max_sec_128: Set transfer size limit to 128 sectors. - * max_sec_1024: Set or clear transfer size limit to - 1024 sectors. + * max_sec_1024: Set transfer size limit to 1024 sectors. - * max_sec_lba48: Set or clear transfer size limit to - 65535 sectors. + * max_sec_lba48: Set transfer size limit to 65535 sectors. * external: Mark port as external (hotplug-capable).
Signature · 1 line
2.55.0
Niklas Cassel
maintainerRe: [PATCH 2/2] ata: libata: Correct libata.force parameter documentation
Quoted 26 lines · Niklas Cassel
Align the documented libata.force options with their implementation. The force table accepts PIO modes 0 through 6, not mode 7, and ncqati controls NCQ generally rather than only queued TRIM.
The
max_sec_1024andmax_sec_lba48options only set transfer size limits. Remove the misleading claim that they can also clear them.Signed-off-by Niklas Cassel <[email protected]>--- Documentation/admin-guide/kernel-parameters.txt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)Documentation/admin-guide/kernel-parameters.txt +1
-1@@ -3491,7 +3491,7 @@ Kernel parameters * SATA link speed limit: 1.5Gbps or 3.0Gbps. - * Transfer mode: pio[0-7], mwdma[0-4] and udma[0-7]. + * Transfer mode: pio[0-6], mwdma[0-4] and udma[0-7]. udma[/][16,25,33,44,66,100,133] notation is also allowed.
Documentation/admin-guide/kernel-parameters.txt Kernel parameters · line 3512
Quoted 5 lines
Documentation/admin-guide/kernel-parameters.txt +1
-1@@ -3509,7 +3509,7 @@ Kernel parameters * [no]ncqtrim: Enable or disable queued DSM TRIM. - * [no]ncqati: Enable or disable NCQ trim on ATI chipset. + * [no]ncqati: Enable or disable NCQ on ATI chipsets.
Hmm.. I guess we could theoretically remove the [no]ncqati parameter from libata.force.
If a user wants to enable/disable ncq, they can use the [no]ncq libata.force parameter.
However, since this libata.force kernel parameter already exists, I guess it might actually be used by someone, so we can't really remove it without potentially breaking existing setups, so I guess the easiest thing is just to keep it, even if it is not strictly needed.
Kind regards,
Niklas
Damien Le Moal
maintainerRe: [PATCH 2/2] ata: libata: Correct libata.force parameter documentation
Quoted 38 lines · Niklas Cassel
Quoted 26 lines · Niklas Cassel
Align the documented libata.force options with their implementation. The force table accepts PIO modes 0 through 6, not mode 7, and ncqati controls NCQ generally rather than only queued TRIM.
The
max_sec_1024andmax_sec_lba48options only set transfer size limits. Remove the misleading claim that they can also clear them.Signed-off-by Niklas Cassel <[email protected]>--- Documentation/admin-guide/kernel-parameters.txt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)Documentation/admin-guide/kernel-parameters.txt +1
-1@@ -3491,7 +3491,7 @@ Kernel parameters * SATA link speed limit: 1.5Gbps or 3.0Gbps. - * Transfer mode: pio[0-7], mwdma[0-4] and udma[0-7]. + * Transfer mode: pio[0-6], mwdma[0-4] and udma[0-7]. udma[/][16,25,33,44,66,100,133] notation is also allowed.Documentation/admin-guide/kernel-parameters.txt Kernel parameters · line 3512
Quoted 5 lines
Documentation/admin-guide/kernel-parameters.txt +1
-1@@ -3509,7 +3509,7 @@ Kernel parameters * [no]ncqtrim: Enable or disable queued DSM TRIM. - * [no]ncqati: Enable or disable NCQ trim on ATI chipset. + * [no]ncqati: Enable or disable NCQ on ATI chipsets.Hmm.. I guess we could theoretically remove the [no]ncqati parameter from libata.force.
If a user wants to enable/disable ncq, they can use the [no]ncq libata.force parameter.
Quoted 4 lines
However, since this libata.force kernel parameter already exists, I guess it might actually be used by someone, so we can't really remove it without potentially breaking existing setups, so I guess the easiest thing is just to keep it, even if it is not strictly needed.
It can be needed on a machine that has multiple adapters from different vendors.
Signature · 2 lines
Damien Le Moal Western Digital Research
Niklas Cassel
maintainerRe: [PATCH 2/2] ata: libata: Correct libata.force parameter documentation
Quoted 6 lines · Damien Le Moal
Quoted 6 lines
Hmm.. I guess we could theoretically remove the [no]ncqati parameter from libata.force.
If a user wants to enable/disable ncq, they can use the [no]ncq libata.force parameter.
Quoted 6 lines
Quoted 4 lines
However, since this libata.force kernel parameter already exists, I guess it might actually be used by someone, so we can't really remove it without potentially breaking existing setups, so I guess the easiest thing is just to keep it, even if it is not strictly needed.
It can be needed on a machine that has multiple adapters from different vendors.
libata.force parameters can be applied per port, so even for machines with multiple adapters, it would have been sufficient IMO.
Kind regards,
Niklas
Re: [PATCH 2/2] ata: libata: Correct libata.force parameter documentation
In reply to Niklas CasselQuoted 8 lines · Niklas Cassel
Align the documented libata.force options with their implementation. The force table accepts PIO modes 0 through 6, not mode 7, and ncqati controls NCQ generally rather than only queued TRIM.
The
max_sec_1024andmax_sec_lba48options only set transfer size limits. Remove the misleading claim that they can also clear them.Signed-off-by Niklas Cassel <[email protected]>
Looks good.
Signature · 2 lines
Damien Le Moal Western Digital Research
Quoted 5 lines · Niklas Cassel
A user reports that a Samsung SSD 870 QVO 8TB connected to an AMD 600 Series chipset SATA controller times out on STANDBY IMMEDIATE during system suspend with
med_power_with_dipmenabled. The command completes withmax_performanceinstead.
Quoted 7 lines
Patch 1 extends the existing Samsung LPM quirk from ATI to AMD controllers, renaming it to
ATA_QUIRK_NO_LPM_ON_ATI_AND_AMD. It keeps the same drive model matches and leaves the NCQ quirk restricted to ATI. LPM behavior on other controller vendors, including Intel, is unchanged. The underlying controller/device interaction has not been root-caused.[...]
Applied to libata/linux.git (for-7.3-fixes), thanks!
[1/2] ata: libata-core: Extend Samsung LPM quirk to AMD controllers
https://git.kernel.org/libata/linux/c/e6bae503 [2/2] ata: libata: Correct libata.force parameter documentation https://git.kernel.org/libata/linux/c/88a0474d
Kind regards,
Niklas