Skip to contentSkip to conversation

libata: Fix HPA horkage to target whole model series not just a few devices

PATCH In review Ioannis Barkas 3 participants 3 messages Sep 13 – Sep 15 libata

lore
Share & tools
lore

Ioannis Barkas

author
#0 lore raw

PATCH 1/1 libata: Fix HPA horkage to target whole model series not just a few devices

Fixes 16c55b038033 (\libata: implement BROKEN_HPA horkage and apply it to affected drives")
Link https://bugzilla.kernel.org/show_bug.cgi?id=8888
Signed-off-by Nick Barkas [email protected]
Signed-off-by Ioannis Barkas <[email protected]>
---
 drivers/ata/libata-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/ata/libata-core.c +2 -2
index 59e87b987d..b23e1c3e4c 100644
@@ -4248,8 +4248,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
 
 	/* devices which puke on READ_NATIVE_MAX */
 	{ "HDS724040KLSA80",	"KFAOA20N",	ATA_QUIRK_BROKEN_HPA },
-	{ "WDC WD3200JD-00KLB0", "WD-WCAMR1130137", ATA_QUIRK_BROKEN_HPA },
-	{ "WDC WD2500JD-00HBB0", "WD-WMAL71490727", ATA_QUIRK_BROKEN_HPA },
+	{ "WDC WD3200JD-00KLB0", "08.05J08", ATA_QUIRK_BROKEN_HPA },
+	{ "WDC WD2500JD-00HBB0", "08.02D08", ATA_QUIRK_BROKEN_HPA },
 	{ "MAXTOR 6L080L4",	"A93.0500",	ATA_QUIRK_BROKEN_HPA },
 
 	/* this one allows HPA unlocking but fails IOs on the area */
Signature · 1 line
2.53.0
AI review
#1 lore raw
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Fixing the HPA quirk matching causes these drives to lose their required ATA_QUIRK_WD_BROKEN_LPM…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Fixing the HPA quirk matching causes these drives to lose their required ATA_QUIRK_WD_BROKEN_LPM flag due to early loop termination.
- [Low] Malformed Fixes and Signed-off-by tags in the commit message.
--

commit 38c888241e3a93997eedcbdd5bda6cc7281c77f5
Author: Ioannis Barkas <[email protected]>

libata: Fix HPA horkage to target whole model series not just a few devices

This commit updates the __ata_dev_quirks table to target an entire model series for the ATA_QUIRK_BROKEN_HPA quirk instead of specific device serial numbers. It resolves an issue where the HPA horkage was not applied to all affected drives.

Link https://bugzilla.kernel.org/show_bug.cgi?id=8888
Quoted 5 lines
Fixes 16c55b038033 (\libata: implement BROKEN_HPA horkage and apply it to affected drives")
Link https://bugzilla.kernel.org/show_bug.cgi?id=8888
Signed-off-by Nick Barkas [email protected]

[Severity: Low]
Does this commit message violate formatting guidelines?

Automated tooling might fail to parse the Fixes tag due to the invalid backslash escape instead of an opening quote, and the Signed-off-by tag is missing angle brackets around the email address.

drivers/ata/libata-core.c static const struct ata_dev_quirks_entry __ata_dev_quirks[] = { · line 4252

Quoted 12 lines
drivers/ata/libata-core.c +2 -2
index 59e87b987d73e..b23e1c3e4cd99 100644
@@ -4248,8 +4248,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
 
 	/* devices which puke on READ_NATIVE_MAX */
 	{ "HDS724040KLSA80",	"KFAOA20N",	ATA_QUIRK_BROKEN_HPA },
-	{ "WDC WD3200JD-00KLB0", "WD-WCAMR1130137", ATA_QUIRK_BROKEN_HPA },
-	{ "WDC WD2500JD-00HBB0", "WD-WMAL71490727", ATA_QUIRK_BROKEN_HPA },
+	{ "WDC WD3200JD-00KLB0", "08.05J08", ATA_QUIRK_BROKEN_HPA },
+	{ "WDC WD2500JD-00HBB0", "08.02D08", ATA_QUIRK_BROKEN_HPA },

[Severity: High]
Does this change cause these drives to lose their required ATA_QUIRK_WD_BROKEN_LPM flag?

During device initialization, ata_dev_quirks() iterates through the __ata_dev_quirks table and returns immediately upon the first match. By fixing the firmware string to correctly match these devices here, the function will now exit early and skip the subsequent ATA_QUIRK_WD_BROKEN_LPM entry that these drives previously relied on:

    { "WDC WD3200JD-*",     NULL,   ATA_QUIRK_WD_BROKEN_LPM },

Without this flag, Link Power Management issues on these specific WD drives could cause command timeouts, NCQ errors, and potential drive dropouts.

Quoted 1 line
drivers/ata/libata-core.c +0 -0
 	{ "MAXTOR 6L080L4",	"A93.0500",	ATA_QUIRK_BROKEN_HPA },
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
maintainer
#2 lore raw
In reply to Ioannis Barkas
Quoted 1 line · Ioannis Barkas
Fixes 16c55b038033 (\libata: implement BROKEN_HPA horkage and apply it to affected drives")

As indicated by Sashiko, this is not formatted correctly.

Quoted 1 line

Drop this blank line too please.

Quoted 2 lines
Link https://bugzilla.kernel.org/show_bug.cgi?id=8888

And this one as well.

Also please write a proper commit message that explains what you are changing. Because saying "target whole model series" in the commit title but changing the quirk entries to still have a FW revision does not do that at all.

Quoted 21 lines
Signed-off-by Nick Barkas [email protected]
Signed-off-by Ioannis Barkas <[email protected]>
---
 drivers/ata/libata-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/ata/libata-core.c +2 -2
index 59e87b987d..b23e1c3e4c 100644
@@ -4248,8 +4248,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
 
 	/* devices which puke on READ_NATIVE_MAX */
 	{ "HDS724040KLSA80",	"KFAOA20N",	ATA_QUIRK_BROKEN_HPA },
-	{ "WDC WD3200JD-00KLB0", "WD-WCAMR1130137", ATA_QUIRK_BROKEN_HPA },
-	{ "WDC WD2500JD-00HBB0", "WD-WMAL71490727", ATA_QUIRK_BROKEN_HPA },
+	{ "WDC WD3200JD-00KLB0", "08.05J08", ATA_QUIRK_BROKEN_HPA },
+	{ "WDC WD2500JD-00HBB0", "08.02D08", ATA_QUIRK_BROKEN_HPA },
 	{ "MAXTOR 6L080L4",	"A93.0500",	ATA_QUIRK_BROKEN_HPA },
 
 	/* this one allows HPA unlocking but fails IOs on the area */
Signature · 2 lines
Damien Le Moal
Western Digital Research