All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] libata.force improvements
@ 2022-04-07 12:37 Damien Le Moal
  2022-04-07 12:37 ` [PATCH 1/5] ata: libata-core: cleanup ata_device_blacklist Damien Le Moal
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Damien Le Moal @ 2022-04-07 12:37 UTC (permalink / raw)
  To: linux-ide, Jonathan Corbet, linux-doc; +Cc: Sergey Shtylyov

This patch series improves the usefulness of the libata.force kernel
boot parameters to facilitate field debugging of ata drives and adapter
issues by extending the range of horkage flags and link flags that can
be controlled.

The first patch is a simple cleanup of the drive blacklist array.
Patch 2 refactors the declaration of the force_tbl array defining the
possible values that libata.force can take. Patch 3 and 4 extend this
array adding most horkage flags and one link flag. Finally, patch 5
updates the kernel documentation reflecting these changes.

Damien Le Moal (5):
  ata: libata-core: cleanup ata_device_blacklist
  ata: libata-core: Refactor force_tbl definition
  ata: libata-core: Improve link flags forced settings
  ata: libata-core: Allow forcing most horkage flags
  doc: admin-guide: Update libata kernel parameters

 .../admin-guide/kernel-parameters.txt         |  71 +++--
 drivers/ata/libata-core.c                     | 271 +++++++++++-------
 2 files changed, 217 insertions(+), 125 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/5] ata: libata-core: cleanup ata_device_blacklist
  2022-04-07 12:37 [PATCH 0/5] libata.force improvements Damien Le Moal
@ 2022-04-07 12:37 ` Damien Le Moal
  2022-04-24 17:29   ` Sergey Shtylyov
  2022-04-07 12:37 ` [PATCH 2/5] ata: libata-core: Refactor force_tbl definition Damien Le Moal
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Damien Le Moal @ 2022-04-07 12:37 UTC (permalink / raw)
  To: linux-ide, Jonathan Corbet, linux-doc; +Cc: Sergey Shtylyov

Remove the unneeded comma after the last field of the array entries.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/ata/libata-core.c | 96 +++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 48 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index cceedde51126..bc59c77b99b6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3898,7 +3898,7 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	/* Devices where NCQ should be avoided */
 	/* NCQ is slow */
 	{ "WDC WD740ADFD-00",	NULL,		ATA_HORKAGE_NONCQ },
-	{ "WDC WD740ADFD-00NLR1", NULL,		ATA_HORKAGE_NONCQ, },
+	{ "WDC WD740ADFD-00NLR1", NULL,		ATA_HORKAGE_NONCQ },
 	/* http://thread.gmane.org/gmane.linux.ide/14907 */
 	{ "FUJITSU MHT2060BH",	NULL,		ATA_HORKAGE_NONCQ },
 	/* NCQ is broken */
@@ -3924,23 +3924,23 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	/* drives which fail FPDMA_AA activation (some may freeze afterwards)
 	   the ST disks also have LPM issues */
 	{ "ST1000LM024 HN-M101MBB", NULL,	ATA_HORKAGE_BROKEN_FPDMA_AA |
-						ATA_HORKAGE_NOLPM, },
+						ATA_HORKAGE_NOLPM },
 	{ "VB0250EAVER",	"HPG7",		ATA_HORKAGE_BROKEN_FPDMA_AA },
 
 	/* Blacklist entries taken from Silicon Image 3124/3132
 	   Windows driver .inf file - also several Linux problem reports */
-	{ "HTS541060G9SA00",    "MB3OC60D",     ATA_HORKAGE_NONCQ, },
-	{ "HTS541080G9SA00",    "MB4OC60D",     ATA_HORKAGE_NONCQ, },
-	{ "HTS541010G9SA00",    "MBZOC60D",     ATA_HORKAGE_NONCQ, },
+	{ "HTS541060G9SA00",    "MB3OC60D",     ATA_HORKAGE_NONCQ },
+	{ "HTS541080G9SA00",    "MB4OC60D",     ATA_HORKAGE_NONCQ },
+	{ "HTS541010G9SA00",    "MBZOC60D",     ATA_HORKAGE_NONCQ },
 
 	/* https://bugzilla.kernel.org/show_bug.cgi?id=15573 */
-	{ "C300-CTFDDAC128MAG",	"0001",		ATA_HORKAGE_NONCQ, },
+	{ "C300-CTFDDAC128MAG",	"0001",		ATA_HORKAGE_NONCQ },
 
 	/* Sandisk SD7/8/9s lock up hard on large trims */
-	{ "SanDisk SD[789]*",	NULL,		ATA_HORKAGE_MAX_TRIM_128M, },
+	{ "SanDisk SD[789]*",	NULL,		ATA_HORKAGE_MAX_TRIM_128M },
 
 	/* devices which puke on READ_NATIVE_MAX */
-	{ "HDS724040KLSA80",	"KFAOA20N",	ATA_HORKAGE_BROKEN_HPA, },
+	{ "HDS724040KLSA80",	"KFAOA20N",	ATA_HORKAGE_BROKEN_HPA },
 	{ "WDC WD3200JD-00KLB0", "WD-WCAMR1130137", ATA_HORKAGE_BROKEN_HPA },
 	{ "WDC WD2500JD-00HBB0", "WD-WMAL71490727", ATA_HORKAGE_BROKEN_HPA },
 	{ "MAXTOR 6L080L4",	"A93.0500",	ATA_HORKAGE_BROKEN_HPA },
@@ -3949,22 +3949,22 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	{ "OCZ-VERTEX",		    "1.30",	ATA_HORKAGE_BROKEN_HPA },
 
 	/* Devices which report 1 sector over size HPA */
-	{ "ST340823A",		NULL,		ATA_HORKAGE_HPA_SIZE, },
-	{ "ST320413A",		NULL,		ATA_HORKAGE_HPA_SIZE, },
-	{ "ST310211A",		NULL,		ATA_HORKAGE_HPA_SIZE, },
+	{ "ST340823A",		NULL,		ATA_HORKAGE_HPA_SIZE },
+	{ "ST320413A",		NULL,		ATA_HORKAGE_HPA_SIZE },
+	{ "ST310211A",		NULL,		ATA_HORKAGE_HPA_SIZE },
 
 	/* Devices which get the IVB wrong */
-	{ "QUANTUM FIREBALLlct10 05", "A03.0900", ATA_HORKAGE_IVB, },
+	{ "QUANTUM FIREBALLlct10 05", "A03.0900", ATA_HORKAGE_IVB },
 	/* Maybe we should just blacklist TSSTcorp... */
-	{ "TSSTcorp CDDVDW SH-S202[HJN]", "SB0[01]",  ATA_HORKAGE_IVB, },
+	{ "TSSTcorp CDDVDW SH-S202[HJN]", "SB0[01]",  ATA_HORKAGE_IVB },
 
 	/* Devices that do not need bridging limits applied */
-	{ "MTRON MSP-SATA*",		NULL,	ATA_HORKAGE_BRIDGE_OK, },
-	{ "BUFFALO HD-QSU2/R5",		NULL,	ATA_HORKAGE_BRIDGE_OK, },
+	{ "MTRON MSP-SATA*",		NULL,	ATA_HORKAGE_BRIDGE_OK },
+	{ "BUFFALO HD-QSU2/R5",		NULL,	ATA_HORKAGE_BRIDGE_OK },
 
 	/* Devices which aren't very happy with higher link speeds */
-	{ "WD My Book",			NULL,	ATA_HORKAGE_1_5_GBPS, },
-	{ "Seagate FreeAgent GoFlex",	NULL,	ATA_HORKAGE_1_5_GBPS, },
+	{ "WD My Book",			NULL,	ATA_HORKAGE_1_5_GBPS },
+	{ "Seagate FreeAgent GoFlex",	NULL,	ATA_HORKAGE_1_5_GBPS },
 
 	/*
 	 * Devices which choke on SETXFER.  Applies only if both the
@@ -3982,54 +3982,54 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	/* 512GB MX100 with MU01 firmware has both queued TRIM and LPM issues */
 	{ "Crucial_CT512MX100*",	"MU01",	ATA_HORKAGE_NO_NCQ_TRIM |
 						ATA_HORKAGE_ZERO_AFTER_TRIM |
-						ATA_HORKAGE_NOLPM, },
+						ATA_HORKAGE_NOLPM },
 	/* 512GB MX100 with newer firmware has only LPM issues */
 	{ "Crucial_CT512MX100*",	NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM |
-						ATA_HORKAGE_NOLPM, },
+						ATA_HORKAGE_NOLPM },
 
 	/* 480GB+ M500 SSDs have both queued TRIM and LPM issues */
 	{ "Crucial_CT480M500*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM |
 						ATA_HORKAGE_ZERO_AFTER_TRIM |
-						ATA_HORKAGE_NOLPM, },
+						ATA_HORKAGE_NOLPM },
 	{ "Crucial_CT960M500*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM |
 						ATA_HORKAGE_ZERO_AFTER_TRIM |
-						ATA_HORKAGE_NOLPM, },
+						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, },
-	{ "SAMSUNG MZ7TD256HAFV-000L9", NULL,       ATA_HORKAGE_NOLPM, },
-	{ "SAMSUNG MZ7TE512HMHP-000L1", "EXT06L0Q", ATA_HORKAGE_NOLPM, },
+	{ "SAMSUNG MZMPC128HBFU-000MV", "CXM14M1Q", ATA_HORKAGE_NOLPM },
+	{ "SAMSUNG SSD PM830 mSATA *",  "CXM13D1Q", ATA_HORKAGE_NOLPM },
+	{ "SAMSUNG MZ7TD256HAFV-000L9", NULL,       ATA_HORKAGE_NOLPM },
+	{ "SAMSUNG MZ7TE512HMHP-000L1", "EXT06L0Q", ATA_HORKAGE_NOLPM },
 
 	/* devices that don't properly handle queued TRIM commands */
 	{ "Micron_M500IT_*",		"MU01",	ATA_HORKAGE_NO_NCQ_TRIM |
-						ATA_HORKAGE_ZERO_AFTER_TRIM, },
+						ATA_HORKAGE_ZERO_AFTER_TRIM },
 	{ "Micron_M500_*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM |
-						ATA_HORKAGE_ZERO_AFTER_TRIM, },
+						ATA_HORKAGE_ZERO_AFTER_TRIM },
 	{ "Crucial_CT*M500*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM |
-						ATA_HORKAGE_ZERO_AFTER_TRIM, },
+						ATA_HORKAGE_ZERO_AFTER_TRIM },
 	{ "Micron_M5[15]0_*",		"MU01",	ATA_HORKAGE_NO_NCQ_TRIM |
-						ATA_HORKAGE_ZERO_AFTER_TRIM, },
+						ATA_HORKAGE_ZERO_AFTER_TRIM },
 	{ "Crucial_CT*M550*",		"MU01",	ATA_HORKAGE_NO_NCQ_TRIM |
-						ATA_HORKAGE_ZERO_AFTER_TRIM, },
+						ATA_HORKAGE_ZERO_AFTER_TRIM },
 	{ "Crucial_CT*MX100*",		"MU01",	ATA_HORKAGE_NO_NCQ_TRIM |
-						ATA_HORKAGE_ZERO_AFTER_TRIM, },
+						ATA_HORKAGE_ZERO_AFTER_TRIM },
 	{ "Samsung SSD 840*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM |
-						ATA_HORKAGE_ZERO_AFTER_TRIM, },
+						ATA_HORKAGE_ZERO_AFTER_TRIM },
 	{ "Samsung SSD 850*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM |
-						ATA_HORKAGE_ZERO_AFTER_TRIM, },
+						ATA_HORKAGE_ZERO_AFTER_TRIM },
 	{ "Samsung SSD 860*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM |
 						ATA_HORKAGE_ZERO_AFTER_TRIM |
-						ATA_HORKAGE_NO_NCQ_ON_ATI, },
+						ATA_HORKAGE_NO_NCQ_ON_ATI },
 	{ "Samsung SSD 870*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM |
 						ATA_HORKAGE_ZERO_AFTER_TRIM |
-						ATA_HORKAGE_NO_NCQ_ON_ATI, },
+						ATA_HORKAGE_NO_NCQ_ON_ATI },
 	{ "FCCT*M500*",			NULL,	ATA_HORKAGE_NO_NCQ_TRIM |
-						ATA_HORKAGE_ZERO_AFTER_TRIM, },
+						ATA_HORKAGE_ZERO_AFTER_TRIM },
 
 	/* devices that don't properly handle TRIM commands */
-	{ "SuperSSpeed S238*",		NULL,	ATA_HORKAGE_NOTRIM, },
-	{ "M88V29*",			NULL,	ATA_HORKAGE_NOTRIM, },
+	{ "SuperSSpeed S238*",		NULL,	ATA_HORKAGE_NOTRIM },
+	{ "M88V29*",			NULL,	ATA_HORKAGE_NOTRIM },
 
 	/*
 	 * As defined, the DRAT (Deterministic Read After Trim) and RZAT
@@ -4047,16 +4047,16 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	 * The intel 510 drive has buggy DRAT/RZAT. Explicitly exclude
 	 * that model before whitelisting all other intel SSDs.
 	 */
-	{ "INTEL*SSDSC2MH*",		NULL,	0, },
-
-	{ "Micron*",			NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM, },
-	{ "Crucial*",			NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM, },
-	{ "INTEL*SSD*", 		NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM, },
-	{ "SSD*INTEL*",			NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM, },
-	{ "Samsung*SSD*",		NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM, },
-	{ "SAMSUNG*SSD*",		NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM, },
-	{ "SAMSUNG*MZ7KM*",		NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM, },
-	{ "ST[1248][0248]0[FH]*",	NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM, },
+	{ "INTEL*SSDSC2MH*",		NULL,	0 },
+
+	{ "Micron*",			NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM },
+	{ "Crucial*",			NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM },
+	{ "INTEL*SSD*", 		NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM },
+	{ "SSD*INTEL*",			NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM },
+	{ "Samsung*SSD*",		NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM },
+	{ "SAMSUNG*SSD*",		NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM },
+	{ "SAMSUNG*MZ7KM*",		NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM },
+	{ "ST[1248][0248]0[FH]*",	NULL,	ATA_HORKAGE_ZERO_AFTER_TRIM },
 
 	/*
 	 * Some WD SATA-I drives spin up and down erratically when the link
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/5] ata: libata-core: Refactor force_tbl definition
  2022-04-07 12:37 [PATCH 0/5] libata.force improvements Damien Le Moal
  2022-04-07 12:37 ` [PATCH 1/5] ata: libata-core: cleanup ata_device_blacklist Damien Le Moal
@ 2022-04-07 12:37 ` Damien Le Moal
  2022-04-24 17:53   ` Sergey Shtylyov
  2022-04-07 12:37 ` [PATCH 3/5] ata: libata-core: Improve link flags forced settings Damien Le Moal
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Damien Le Moal @ 2022-04-07 12:37 UTC (permalink / raw)
  To: linux-ide, Jonathan Corbet, linux-doc; +Cc: Sergey Shtylyov

Introduce the macro definitions force_cbl(), force_spd_limit(),
force_xfer(), force_lflag_on(), force_horkage_on() and
force_horkage_onoff() to define with a more compact syntax the struct
ata_force_param entries in the force_tbl array defined in the function
ata_parse_force_one().
To reduce the indentation of the array declaration, force_tbl definition
is also moved out of the ata_parse_force_one() function.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/ata/libata-core.c | 139 ++++++++++++++++++++++----------------
 1 file changed, 81 insertions(+), 58 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bc59c77b99b6..c0cf42ffcc38 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6143,67 +6143,90 @@ int ata_platform_remove_one(struct platform_device *pdev)
 EXPORT_SYMBOL_GPL(ata_platform_remove_one);
 
 #ifdef CONFIG_ATA_FORCE
+
+#define force_cbl(name, flag)				\
+	{ # name,	.cbl		= (flag) }
+
+#define force_spd_limit(spd, val)			\
+	{ # spd,	.spd_limit	= (val) }
+
+#define force_xfer(mode, shift)				\
+	{ # mode,	.xfer_mask	= (1UL << (shift)) }
+
+#define force_lflag_on(name, flags)			\
+	{ # name,	.lflags		= (flags) }
+
+#define force_horkage_on(name, flag)			\
+	{ # name,	.horkage_on	= (flag) }
+
+#define force_horkage_onoff(name, flag)			\
+	{ "no" # name,	.horkage_on	= (flag) },	\
+	{ # name,	.horkage_off	= (flag) }
+
+static const struct ata_force_param force_tbl[] __initconst = {
+	force_cbl(40c,			ATA_CBL_PATA40),
+	force_cbl(80c,			ATA_CBL_PATA80),
+	force_cbl(short40c,		ATA_CBL_PATA40_SHORT),
+	force_cbl(unk,			ATA_CBL_PATA_UNK),
+	force_cbl(ign,			ATA_CBL_PATA_IGN),
+	force_cbl(sata,			ATA_CBL_SATA),
+
+	force_spd_limit(1.5Gbps,	1),
+	force_spd_limit(3.0Gbps,	2),
+
+	force_xfer(pio0,		ATA_SHIFT_PIO + 0),
+	force_xfer(pio1,		ATA_SHIFT_PIO + 1),
+	force_xfer(pio2,		ATA_SHIFT_PIO + 2),
+	force_xfer(pio3,		ATA_SHIFT_PIO + 3),
+	force_xfer(pio4,		ATA_SHIFT_PIO + 4),
+	force_xfer(pio5,		ATA_SHIFT_PIO + 5),
+	force_xfer(pio6,		ATA_SHIFT_PIO + 6),
+	force_xfer(mwdma0,		ATA_SHIFT_MWDMA + 0),
+	force_xfer(mwdma1,		ATA_SHIFT_MWDMA + 1),
+	force_xfer(mwdma2,		ATA_SHIFT_MWDMA + 2),
+	force_xfer(mwdma3,		ATA_SHIFT_MWDMA + 3),
+	force_xfer(mwdma4,		ATA_SHIFT_MWDMA + 4),
+	force_xfer(udma0,		ATA_SHIFT_UDMA + 0),
+	force_xfer(udma16,		ATA_SHIFT_UDMA + 0),
+	force_xfer(udma/16,		ATA_SHIFT_UDMA + 0),
+	force_xfer(udma1,		ATA_SHIFT_UDMA + 1),
+	force_xfer(udma25,		ATA_SHIFT_UDMA + 1),
+	force_xfer(udma/25,		ATA_SHIFT_UDMA + 1),
+	force_xfer(udma2,		ATA_SHIFT_UDMA + 2),
+	force_xfer(udma33,		ATA_SHIFT_UDMA + 2),
+	force_xfer(udma/33,		ATA_SHIFT_UDMA + 2),
+	force_xfer(udma3,		ATA_SHIFT_UDMA + 3),
+	force_xfer(udma44,		ATA_SHIFT_UDMA + 3),
+	force_xfer(udma/44,		ATA_SHIFT_UDMA + 3),
+	force_xfer(udma4,		ATA_SHIFT_UDMA + 4),
+	force_xfer(udma66,		ATA_SHIFT_UDMA + 4),
+	force_xfer(udma/66,		ATA_SHIFT_UDMA + 4),
+	force_xfer(udma5,		ATA_SHIFT_UDMA + 5),
+	force_xfer(udma100,		ATA_SHIFT_UDMA + 5),
+	force_xfer(udma/100,		ATA_SHIFT_UDMA + 5),
+	force_xfer(udma6,		ATA_SHIFT_UDMA + 6),
+	force_xfer(udma133,		ATA_SHIFT_UDMA + 6),
+	force_xfer(udma/133,		ATA_SHIFT_UDMA + 6),
+	force_xfer(udma7,		ATA_SHIFT_UDMA + 7),
+
+	force_lflag_on(nohrst,		ATA_LFLAG_NO_HRST),
+	force_lflag_on(nosrst,		ATA_LFLAG_NO_SRST),
+	force_lflag_on(norst,		ATA_LFLAG_NO_HRST | ATA_LFLAG_NO_SRST),
+	force_lflag_on(rstonce,		ATA_LFLAG_RST_ONCE),
+
+	force_horkage_onoff(ncq,	ATA_HORKAGE_NONCQ),
+	force_horkage_onoff(ncqtrim,	ATA_HORKAGE_NO_NCQ_TRIM),
+	force_horkage_onoff(ncqati,	ATA_HORKAGE_NO_NCQ_ON_ATI),
+
+	force_horkage_on(dump_id,	ATA_HORKAGE_DUMP_ID),
+	force_horkage_on(atapi_dmadir,	ATA_HORKAGE_ATAPI_DMADIR),
+	force_horkage_on(disable,	ATA_HORKAGE_DISABLE)
+};
+
 static int __init ata_parse_force_one(char **cur,
 				      struct ata_force_ent *force_ent,
 				      const char **reason)
 {
-	static const struct ata_force_param force_tbl[] __initconst = {
-		{ "40c",	.cbl		= ATA_CBL_PATA40 },
-		{ "80c",	.cbl		= ATA_CBL_PATA80 },
-		{ "short40c",	.cbl		= ATA_CBL_PATA40_SHORT },
-		{ "unk",	.cbl		= ATA_CBL_PATA_UNK },
-		{ "ign",	.cbl		= ATA_CBL_PATA_IGN },
-		{ "sata",	.cbl		= ATA_CBL_SATA },
-		{ "1.5Gbps",	.spd_limit	= 1 },
-		{ "3.0Gbps",	.spd_limit	= 2 },
-		{ "noncq",	.horkage_on	= ATA_HORKAGE_NONCQ },
-		{ "ncq",	.horkage_off	= ATA_HORKAGE_NONCQ },
-		{ "noncqtrim",	.horkage_on	= ATA_HORKAGE_NO_NCQ_TRIM },
-		{ "ncqtrim",	.horkage_off	= ATA_HORKAGE_NO_NCQ_TRIM },
-		{ "noncqati",	.horkage_on	= ATA_HORKAGE_NO_NCQ_ON_ATI },
-		{ "ncqati",	.horkage_off	= ATA_HORKAGE_NO_NCQ_ON_ATI },
-		{ "dump_id",	.horkage_on	= ATA_HORKAGE_DUMP_ID },
-		{ "pio0",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 0) },
-		{ "pio1",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 1) },
-		{ "pio2",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 2) },
-		{ "pio3",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 3) },
-		{ "pio4",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 4) },
-		{ "pio5",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 5) },
-		{ "pio6",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 6) },
-		{ "mwdma0",	.xfer_mask	= 1 << (ATA_SHIFT_MWDMA + 0) },
-		{ "mwdma1",	.xfer_mask	= 1 << (ATA_SHIFT_MWDMA + 1) },
-		{ "mwdma2",	.xfer_mask	= 1 << (ATA_SHIFT_MWDMA + 2) },
-		{ "mwdma3",	.xfer_mask	= 1 << (ATA_SHIFT_MWDMA + 3) },
-		{ "mwdma4",	.xfer_mask	= 1 << (ATA_SHIFT_MWDMA + 4) },
-		{ "udma0",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 0) },
-		{ "udma16",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 0) },
-		{ "udma/16",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 0) },
-		{ "udma1",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 1) },
-		{ "udma25",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 1) },
-		{ "udma/25",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 1) },
-		{ "udma2",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 2) },
-		{ "udma33",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 2) },
-		{ "udma/33",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 2) },
-		{ "udma3",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 3) },
-		{ "udma44",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 3) },
-		{ "udma/44",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 3) },
-		{ "udma4",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 4) },
-		{ "udma66",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 4) },
-		{ "udma/66",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 4) },
-		{ "udma5",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 5) },
-		{ "udma100",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 5) },
-		{ "udma/100",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 5) },
-		{ "udma6",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 6) },
-		{ "udma133",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 6) },
-		{ "udma/133",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 6) },
-		{ "udma7",	.xfer_mask	= 1 << (ATA_SHIFT_UDMA + 7) },
-		{ "nohrst",	.lflags		= ATA_LFLAG_NO_HRST },
-		{ "nosrst",	.lflags		= ATA_LFLAG_NO_SRST },
-		{ "norst",	.lflags		= ATA_LFLAG_NO_HRST | ATA_LFLAG_NO_SRST },
-		{ "rstonce",	.lflags		= ATA_LFLAG_RST_ONCE },
-		{ "atapi_dmadir", .horkage_on	= ATA_HORKAGE_ATAPI_DMADIR },
-		{ "disable",	.horkage_on	= ATA_HORKAGE_DISABLE },
-	};
 	char *start = *cur, *p = *cur;
 	char *id, *val, *endp;
 	const struct ata_force_param *match_fp = NULL;
@@ -6285,7 +6308,7 @@ static void __init ata_parse_force_param(void)
 	int last_port = -1, last_device = -1;
 	char *p, *cur, *next;
 
-	/* calculate maximum number of params and allocate force_tbl */
+	/* Calculate maximum number of params and allocate ata_force_tbl */
 	for (p = ata_force_param_buf; *p; p++)
 		if (*p == ',')
 			size++;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/5] ata: libata-core: Improve link flags forced settings
  2022-04-07 12:37 [PATCH 0/5] libata.force improvements Damien Le Moal
  2022-04-07 12:37 ` [PATCH 1/5] ata: libata-core: cleanup ata_device_blacklist Damien Le Moal
  2022-04-07 12:37 ` [PATCH 2/5] ata: libata-core: Refactor force_tbl definition Damien Le Moal
@ 2022-04-07 12:37 ` Damien Le Moal
  2022-04-24 18:01   ` Sergey Shtylyov
  2022-04-07 12:37 ` [PATCH 4/5] ata: libata-core: Allow forcing most horkage flags Damien Le Moal
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Damien Le Moal @ 2022-04-07 12:37 UTC (permalink / raw)
  To: linux-ide, Jonathan Corbet, linux-doc; +Cc: Sergey Shtylyov

Similarly to the horkage flags, introduce the force_lflag_onoff() macro
to define struct ata_force_param entries of the force_tbl array that
allow turning on or off a link flag using the libata.force boot
parameter. Using this new helper macro, define a new force_tbl entry
for the ATA_LFLAG_NO_DEBOUNCE_DELAY link flag, thus allowing testing if
an adapter requires a link debounce delay or not.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/ata/libata-core.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index c0cf42ffcc38..75856f4210d7 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -96,7 +96,8 @@ struct ata_force_param {
 	unsigned long	xfer_mask;
 	unsigned int	horkage_on;
 	unsigned int	horkage_off;
-	u16		lflags;
+	u16		lflags_on;
+	u16		lflags_off;
 };
 
 struct ata_force_ent {
@@ -386,11 +387,17 @@ static void ata_force_link_limits(struct ata_link *link)
 		}
 
 		/* let lflags stack */
-		if (fe->param.lflags) {
-			link->flags |= fe->param.lflags;
+		if (fe->param.lflags_on) {
+			link->flags |= fe->param.lflags_on;
 			ata_link_notice(link,
 					"FORCE: link flag 0x%x forced -> 0x%x\n",
-					fe->param.lflags, link->flags);
+					fe->param.lflags_on, link->flags);
+		}
+		if (fe->param.lflags_off) {
+			link->flags &= ~fe->param.lflags_off;
+			ata_link_notice(link,
+				"FORCE: link flag 0x%x cleared -> 0x%x\n",
+				fe->param.lflags_off, link->flags);
 		}
 	}
 }
@@ -6154,7 +6161,11 @@ EXPORT_SYMBOL_GPL(ata_platform_remove_one);
 	{ # mode,	.xfer_mask	= (1UL << (shift)) }
 
 #define force_lflag_on(name, flags)			\
-	{ # name,	.lflags		= (flags) }
+	{ # name,	.lflags_on	= (flags) }
+
+#define force_lflag_onoff(name, flags)			\
+	{ "no" # name,	.lflags_on	= (flags) },	\
+	{ # name,	.lflags_off	= (flags) }
 
 #define force_horkage_on(name, flag)			\
 	{ # name,	.horkage_on	= (flag) }
@@ -6213,6 +6224,7 @@ static const struct ata_force_param force_tbl[] __initconst = {
 	force_lflag_on(nosrst,		ATA_LFLAG_NO_SRST),
 	force_lflag_on(norst,		ATA_LFLAG_NO_HRST | ATA_LFLAG_NO_SRST),
 	force_lflag_on(rstonce,		ATA_LFLAG_RST_ONCE),
+	force_lflag_onoff(dbdelay,	ATA_LFLAG_NO_DEBOUNCE_DELAY),
 
 	force_horkage_onoff(ncq,	ATA_HORKAGE_NONCQ),
 	force_horkage_onoff(ncqtrim,	ATA_HORKAGE_NO_NCQ_TRIM),
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/5] ata: libata-core: Allow forcing most horkage flags
  2022-04-07 12:37 [PATCH 0/5] libata.force improvements Damien Le Moal
                   ` (2 preceding siblings ...)
  2022-04-07 12:37 ` [PATCH 3/5] ata: libata-core: Improve link flags forced settings Damien Le Moal
@ 2022-04-07 12:37 ` Damien Le Moal
  2022-04-24 18:17   ` Sergey Shtylyov
  2022-04-07 12:37 ` [PATCH 5/5] doc: admin-guide: Update libata kernel parameters Damien Le Moal
  2022-04-12  0:45 ` [PATCH 0/5] libata.force improvements Damien Le Moal
  5 siblings, 1 reply; 15+ messages in thread
From: Damien Le Moal @ 2022-04-07 12:37 UTC (permalink / raw)
  To: linux-ide, Jonathan Corbet, linux-doc; +Cc: Sergey Shtylyov

To facilitate debugging of drive issues in the field without kernel
changes (e.g. temporary test patches), add an entry for most horkage
flags in the force_tbl array to allow controlling these horkage
settings with the libata.force kernel boot parameter.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/ata/libata-core.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 75856f4210d7..121cb55a219a 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6230,9 +6230,27 @@ static const struct ata_force_param force_tbl[] __initconst = {
 	force_horkage_onoff(ncqtrim,	ATA_HORKAGE_NO_NCQ_TRIM),
 	force_horkage_onoff(ncqati,	ATA_HORKAGE_NO_NCQ_ON_ATI),
 
-	force_horkage_on(dump_id,	ATA_HORKAGE_DUMP_ID),
+	force_horkage_onoff(trim,	ATA_HORKAGE_NOTRIM),
+	force_horkage_onoff(trim_zero,	ATA_HORKAGE_ZERO_AFTER_TRIM),
+	force_horkage_on(max_trim_128m, ATA_HORKAGE_MAX_TRIM_128M),
+
+	force_horkage_onoff(dma,	ATA_HORKAGE_NODMA),
 	force_horkage_on(atapi_dmadir,	ATA_HORKAGE_ATAPI_DMADIR),
-	force_horkage_on(disable,	ATA_HORKAGE_DISABLE)
+	force_horkage_on(atapi_mod16_dma, ATA_HORKAGE_ATAPI_MOD16_DMA),
+
+	force_horkage_onoff(dma_log,	ATA_HORKAGE_NO_DMA_LOG),
+	force_horkage_onoff(id_dev_log,	ATA_HORKAGE_NO_ID_DEV_LOG),
+	force_horkage_onoff(log_dir,	ATA_HORKAGE_NO_LOG_DIR),
+
+	force_horkage_on(max_sec_128,	ATA_HORKAGE_MAX_SEC_128),
+	force_horkage_on(max_sec_1024,	ATA_HORKAGE_MAX_SEC_1024),
+	force_horkage_on(max_sec_lba48,	ATA_HORKAGE_MAX_SEC_LBA48),
+
+	force_horkage_onoff(lpm,	ATA_HORKAGE_NOLPM),
+	force_horkage_onoff(setxfer,	ATA_HORKAGE_NOSETXFER),
+	force_horkage_on(dump_id,	ATA_HORKAGE_DUMP_ID),
+
+	force_horkage_on(disable,	ATA_HORKAGE_DISABLE),
 };
 
 static int __init ata_parse_force_one(char **cur,
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/5] doc: admin-guide: Update libata kernel parameters
  2022-04-07 12:37 [PATCH 0/5] libata.force improvements Damien Le Moal
                   ` (3 preceding siblings ...)
  2022-04-07 12:37 ` [PATCH 4/5] ata: libata-core: Allow forcing most horkage flags Damien Le Moal
@ 2022-04-07 12:37 ` Damien Le Moal
  2022-04-25 12:10   ` Sergey Shtylyov
  2022-04-12  0:45 ` [PATCH 0/5] libata.force improvements Damien Le Moal
  5 siblings, 1 reply; 15+ messages in thread
From: Damien Le Moal @ 2022-04-07 12:37 UTC (permalink / raw)
  To: linux-ide, Jonathan Corbet, linux-doc; +Cc: Sergey Shtylyov

Cleanup the text text describing the libata.force boot parameter and
update the list of the values to include all supported horkage and link
flag that can be forced.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 .../admin-guide/kernel-parameters.txt         | 71 ++++++++++++++-----
 1 file changed, 55 insertions(+), 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 3f1cc5e317ed..30734a610b92 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2622,14 +2622,14 @@
 			when set.
 			Format: <int>
 
-	libata.force=	[LIBATA] Force configurations.  The format is comma-
-			separated list of "[ID:]VAL" where ID is
-			PORT[.DEVICE].  PORT and DEVICE are decimal numbers
-			matching port, link or device.  Basically, it matches
-			the ATA ID string printed on console by libata.  If
-			the whole ID part is omitted, the last PORT and DEVICE
-			values are used.  If ID hasn't been specified yet, the
-			configuration applies to all ports, links and devices.
+	libata.force=	[LIBATA] Force configurations.  The format is a comma-
+			separated list of "[ID:]VAL" where ID is PORT[.DEVICE].
+			PORT and DEVICE are decimal numbers matching port, link
+			or device.  Basically, it matches the ATA ID string
+			printed on console by libata.  If the whole ID part is
+			omitted, the last PORT and DEVICE values are used.  If
+			ID hasn't been specified yet, the configuration applies
+			to all ports, links and devices.
 
 			If only DEVICE is omitted, the parameter applies to
 			the port and all links and devices behind it.  DEVICE
@@ -2639,7 +2639,7 @@
 			host link and device attached to it.
 
 			The VAL specifies the configuration to force.  As long
-			as there's no ambiguity shortcut notation is allowed.
+			as there is no ambiguity, shortcut notation is allowed.
 			For example, both 1.5 and 1.5G would work for 1.5Gbps.
 			The following configurations can be forced.
 
@@ -2652,19 +2652,58 @@
 			  udma[/][16,25,33,44,66,100,133] notation is also
 			  allowed.
 
+			* nohrst, nosrst, norst: suppress hard, soft and both
+			  resets.
+
+			* rstonce: only attempt one reset during hot-unplug
+			  link recovery.
+
+			* [no]dbdelay: Enable or disable the extra 200ms delay
+			  before debouncing a link PHY and device presence
+			  detection.
+
 			* [no]ncq: Turn on or off NCQ.
 
-			* [no]ncqtrim: Turn off queued DSM TRIM.
+			* [no]ncqtrim: Enable or disable queued DSM TRIM.
+
+			* [no]ncqati: Enable or disable NCQ trim on ATI chipset.
+
+			* [no]trim: Enable or disable (unqueued) TRIM.
+
+			* [no]trim_zero: Indicate if TRIM command zeroes data.
+
+			* max_trim_128m: Set 128M maximum trim size limit.
+
+			* [no]dma: Turn on or off DMA transfers.
+
+			* atapi_dmadir: Enable ATAPI DMADIR bridge support.
+
+			* atapi_mod16_dma: Enable the use of ATAPI DMA for
+			  commands that are not a multiple of 16 bytes.
+
+			* [no]dma_log: Enable or disable the use of the
+			  READ LOG DMA EXT command to access logs.
+
+			* [no]id_dev_log: Enable or disable access to the
+			  identify device data log.
+
+			* [no]log_dir: Enable or disable access to the general
+			  purpose log directory.
+
+			* max_sec_128: Set transfer size limit to 128 sectors.
+
+			* max_sec_1024: Set or clear transfer size limit to
+			  1024 sectors.
 
-			* nohrst, nosrst, norst: suppress hard, soft
-			  and both resets.
+			* max_sec_lba48: Set or clear transfer size limit to
+			  65535 sectors.
 
-			* rstonce: only attempt one reset during
-			  hot-unplug link recovery
+			* [no]lpm: Enable or disable link power management.
 
-			* dump_id: dump IDENTIFY data.
+			* [no]setxfer: Indicate if transfer speed mode setting
+			  should be skipped.
 
-			* atapi_dmadir: Enable ATAPI DMADIR bridge support
+			* dump_id: Dump IDENTIFY data.
 
 			* disable: Disable this device.
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/5] libata.force improvements
  2022-04-07 12:37 [PATCH 0/5] libata.force improvements Damien Le Moal
                   ` (4 preceding siblings ...)
  2022-04-07 12:37 ` [PATCH 5/5] doc: admin-guide: Update libata kernel parameters Damien Le Moal
@ 2022-04-12  0:45 ` Damien Le Moal
  2022-04-17 18:10   ` Sergey Shtylyov
  5 siblings, 1 reply; 15+ messages in thread
From: Damien Le Moal @ 2022-04-12  0:45 UTC (permalink / raw)
  To: linux-ide; +Cc: Sergey Shtylyov

On 4/7/22 21:37, Damien Le Moal wrote:
> This patch series improves the usefulness of the libata.force kernel
> boot parameters to facilitate field debugging of ata drives and adapter
> issues by extending the range of horkage flags and link flags that can
> be controlled.
> 
> The first patch is a simple cleanup of the drive blacklist array.
> Patch 2 refactors the declaration of the force_tbl array defining the
> possible values that libata.force can take. Patch 3 and 4 extend this
> array adding most horkage flags and one link flag. Finally, patch 5
> updates the kernel documentation reflecting these changes.
> 
> Damien Le Moal (5):
>   ata: libata-core: cleanup ata_device_blacklist
>   ata: libata-core: Refactor force_tbl definition
>   ata: libata-core: Improve link flags forced settings
>   ata: libata-core: Allow forcing most horkage flags
>   doc: admin-guide: Update libata kernel parameters
> 
>  .../admin-guide/kernel-parameters.txt         |  71 +++--
>  drivers/ata/libata-core.c                     | 271 +++++++++++-------
>  2 files changed, 217 insertions(+), 125 deletions(-)
> 

Sergey,

Could you review this and/or give it a try ?

-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/5] libata.force improvements
  2022-04-12  0:45 ` [PATCH 0/5] libata.force improvements Damien Le Moal
@ 2022-04-17 18:10   ` Sergey Shtylyov
  0 siblings, 0 replies; 15+ messages in thread
From: Sergey Shtylyov @ 2022-04-17 18:10 UTC (permalink / raw)
  To: Damien Le Moal, linux-ide

On 4/12/22 3:45 AM, Damien Le Moal wrote:

>> This patch series improves the usefulness of the libata.force kernel
>> boot parameters to facilitate field debugging of ata drives and adapter
>> issues by extending the range of horkage flags and link flags that can
>> be controlled.
>>
>> The first patch is a simple cleanup of the drive blacklist array.
>> Patch 2 refactors the declaration of the force_tbl array defining the
>> possible values that libata.force can take. Patch 3 and 4 extend this
>> array adding most horkage flags and one link flag. Finally, patch 5
>> updates the kernel documentation reflecting these changes.
>>
>> Damien Le Moal (5):
>>   ata: libata-core: cleanup ata_device_blacklist
>>   ata: libata-core: Refactor force_tbl definition
>>   ata: libata-core: Improve link flags forced settings
>>   ata: libata-core: Allow forcing most horkage flags
>>   doc: admin-guide: Update libata kernel parameters
>>
>>  .../admin-guide/kernel-parameters.txt         |  71 +++--
>>  drivers/ata/libata-core.c                     | 271 +++++++++++-------
>>  2 files changed, 217 insertions(+), 125 deletions(-)
>>
> 
> Sergey,
> 
> Could you review this and/or give it a try ?

   Sorry, I'm a bit overloaded at work, couldn't find the time for that (yet?)...

MBR, Sergey

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/5] ata: libata-core: cleanup ata_device_blacklist
  2022-04-07 12:37 ` [PATCH 1/5] ata: libata-core: cleanup ata_device_blacklist Damien Le Moal
@ 2022-04-24 17:29   ` Sergey Shtylyov
  0 siblings, 0 replies; 15+ messages in thread
From: Sergey Shtylyov @ 2022-04-24 17:29 UTC (permalink / raw)
  To: Damien Le Moal, linux-ide, Jonathan Corbet, linux-doc

Hello!

On 4/7/22 3:37 PM, Damien Le Moal wrote:

> Remove the unneeded comma after the last field of the array entries.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  drivers/ata/libata-core.c | 96 +++++++++++++++++++--------------------
>  1 file changed, 48 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index cceedde51126..bc59c77b99b6 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
[...]

Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>

MBR, Sergey

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] ata: libata-core: Refactor force_tbl definition
  2022-04-07 12:37 ` [PATCH 2/5] ata: libata-core: Refactor force_tbl definition Damien Le Moal
@ 2022-04-24 17:53   ` Sergey Shtylyov
  2022-04-25  1:34     ` Damien Le Moal
  0 siblings, 1 reply; 15+ messages in thread
From: Sergey Shtylyov @ 2022-04-24 17:53 UTC (permalink / raw)
  To: Damien Le Moal, linux-ide, Jonathan Corbet, linux-doc

On 4/7/22 3:37 PM, Damien Le Moal wrote:

> Introduce the macro definitions force_cbl(), force_spd_limit(),
> force_xfer(), force_lflag_on(), force_horkage_on() and
> force_horkage_onoff() to define with a more compact syntax the struct
> ata_force_param entries in the force_tbl array defined in the function
> ata_parse_force_one().
> To reduce the indentation of the array declaration, force_tbl definition
> is also moved out of the ata_parse_force_one() function.

   Some entries are reordered too... :-)

> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index bc59c77b99b6..c0cf42ffcc38 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -6143,67 +6143,90 @@ int ata_platform_remove_one(struct platform_device *pdev)
>  EXPORT_SYMBOL_GPL(ata_platform_remove_one);
>  
>  #ifdef CONFIG_ATA_FORCE
> +
> +#define force_cbl(name, flag)				\
> +	{ # name,	.cbl		= (flag) }

   Why not #name here and below?

> +
> +#define force_spd_limit(spd, val)			\
> +	{ # spd,	.spd_limit	= (val) }
> +
> +#define force_xfer(mode, shift)				\
> +	{ # mode,	.xfer_mask	= (1UL << (shift)) }
> +
> +#define force_lflag_on(name, flags)			\

   Not just force_lflag()?

> +	{ # name,	.lflags		= (flags) }
> +
> +#define force_horkage_on(name, flag)			\
> +	{ # name,	.horkage_on	= (flag) }
> +
> +#define force_horkage_onoff(name, flag)			\
> +	{ "no" # name,	.horkage_on	= (flag) },	\
> +	{ # name,	.horkage_off	= (flag) }
> +
[...]
> @@ -6285,7 +6308,7 @@ static void __init ata_parse_force_param(void)
>  	int last_port = -1, last_device = -1;
>  	char *p, *cur, *next;
>  
> -	/* calculate maximum number of params and allocate force_tbl */
> +	/* Calculate maximum number of params and allocate ata_force_tbl */

   Drove-by change? :-)

[...]

Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>

MBR, Sergey

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/5] ata: libata-core: Improve link flags forced settings
  2022-04-07 12:37 ` [PATCH 3/5] ata: libata-core: Improve link flags forced settings Damien Le Moal
@ 2022-04-24 18:01   ` Sergey Shtylyov
  0 siblings, 0 replies; 15+ messages in thread
From: Sergey Shtylyov @ 2022-04-24 18:01 UTC (permalink / raw)
  To: Damien Le Moal, linux-ide, Jonathan Corbet, linux-doc

On 4/7/22 3:37 PM, Damien Le Moal wrote:

> Similarly to the horkage flags, introduce the force_lflag_onoff() macro
> to define struct ata_force_param entries of the force_tbl array that
> allow turning on or off a link flag using the libata.force boot
> parameter. Using this new helper macro, define a new force_tbl entry
> for the ATA_LFLAG_NO_DEBOUNCE_DELAY link flag, thus allowing testing if
> an adapter requires a link debounce delay or not.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  drivers/ata/libata-core.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index c0cf42ffcc38..75856f4210d7 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
[...]
> @@ -6154,7 +6161,11 @@ EXPORT_SYMBOL_GPL(ata_platform_remove_one);
>  	{ # mode,	.xfer_mask	= (1UL << (shift)) }
>  
>  #define force_lflag_on(name, flags)			\
> -	{ # name,	.lflags		= (flags) }
> +	{ # name,	.lflags_on	= (flags) }

   The same comment about #name...

> +
> +#define force_lflag_onoff(name, flags)			\
> +	{ "no" # name,	.lflags_on	= (flags) },	\
> +	{ # name,	.lflags_off	= (flags) }
>  
>  #define force_horkage_on(name, flag)			\
>  	{ # name,	.horkage_on	= (flag) }
[...]

Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>

MBR, Sergey

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/5] ata: libata-core: Allow forcing most horkage flags
  2022-04-07 12:37 ` [PATCH 4/5] ata: libata-core: Allow forcing most horkage flags Damien Le Moal
@ 2022-04-24 18:17   ` Sergey Shtylyov
  2022-04-25  1:15     ` Damien Le Moal
  0 siblings, 1 reply; 15+ messages in thread
From: Sergey Shtylyov @ 2022-04-24 18:17 UTC (permalink / raw)
  To: Damien Le Moal, linux-ide, Jonathan Corbet, linux-doc

On 4/7/22 3:37 PM, Damien Le Moal wrote:

> To facilitate debugging of drive issues in the field without kernel
> changes (e.g. temporary test patches), add an entry for most horkage
> flags in the force_tbl array to allow controlling these horkage
> settings with the libata.force kernel boot parameter.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  drivers/ata/libata-core.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 75856f4210d7..121cb55a219a 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -6230,9 +6230,27 @@ static const struct ata_force_param force_tbl[] __initconst = {
>  	force_horkage_onoff(ncqtrim,	ATA_HORKAGE_NO_NCQ_TRIM),
>  	force_horkage_onoff(ncqati,	ATA_HORKAGE_NO_NCQ_ON_ATI),
>  
> -	force_horkage_on(dump_id,	ATA_HORKAGE_DUMP_ID),
> +	force_horkage_onoff(trim,	ATA_HORKAGE_NOTRIM),
> +	force_horkage_onoff(trim_zero,	ATA_HORKAGE_ZERO_AFTER_TRIM),
> +	force_horkage_on(max_trim_128m, ATA_HORKAGE_MAX_TRIM_128M),
> +
> +	force_horkage_onoff(dma,	ATA_HORKAGE_NODMA),
>  	force_horkage_on(atapi_dmadir,	ATA_HORKAGE_ATAPI_DMADIR),
> -	force_horkage_on(disable,	ATA_HORKAGE_DISABLE)
> +	force_horkage_on(atapi_mod16_dma, ATA_HORKAGE_ATAPI_MOD16_DMA),
> +
> +	force_horkage_onoff(dma_log,	ATA_HORKAGE_NO_DMA_LOG),
> +	force_horkage_onoff(id_dev_log,	ATA_HORKAGE_NO_ID_DEV_LOG),
> +	force_horkage_onoff(log_dir,	ATA_HORKAGE_NO_LOG_DIR),

   Underscores in the names with "no" (without underscore) would look inconsistent,
wouldn't they? Maybe drop the underscores here?

[...]

Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>

MBR, Sergey

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/5] ata: libata-core: Allow forcing most horkage flags
  2022-04-24 18:17   ` Sergey Shtylyov
@ 2022-04-25  1:15     ` Damien Le Moal
  0 siblings, 0 replies; 15+ messages in thread
From: Damien Le Moal @ 2022-04-25  1:15 UTC (permalink / raw)
  To: Sergey Shtylyov, linux-ide, Jonathan Corbet, linux-doc

On 4/25/22 03:17, Sergey Shtylyov wrote:
> On 4/7/22 3:37 PM, Damien Le Moal wrote:
> 
>> To facilitate debugging of drive issues in the field without kernel
>> changes (e.g. temporary test patches), add an entry for most horkage
>> flags in the force_tbl array to allow controlling these horkage
>> settings with the libata.force kernel boot parameter.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
>> ---
>>  drivers/ata/libata-core.c | 22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index 75856f4210d7..121cb55a219a 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -6230,9 +6230,27 @@ static const struct ata_force_param force_tbl[] __initconst = {
>>  	force_horkage_onoff(ncqtrim,	ATA_HORKAGE_NO_NCQ_TRIM),
>>  	force_horkage_onoff(ncqati,	ATA_HORKAGE_NO_NCQ_ON_ATI),
>>  
>> -	force_horkage_on(dump_id,	ATA_HORKAGE_DUMP_ID),
>> +	force_horkage_onoff(trim,	ATA_HORKAGE_NOTRIM),
>> +	force_horkage_onoff(trim_zero,	ATA_HORKAGE_ZERO_AFTER_TRIM),
>> +	force_horkage_on(max_trim_128m, ATA_HORKAGE_MAX_TRIM_128M),
>> +
>> +	force_horkage_onoff(dma,	ATA_HORKAGE_NODMA),
>>  	force_horkage_on(atapi_dmadir,	ATA_HORKAGE_ATAPI_DMADIR),
>> -	force_horkage_on(disable,	ATA_HORKAGE_DISABLE)
>> +	force_horkage_on(atapi_mod16_dma, ATA_HORKAGE_ATAPI_MOD16_DMA),
>> +
>> +	force_horkage_onoff(dma_log,	ATA_HORKAGE_NO_DMA_LOG),
>> +	force_horkage_onoff(id_dev_log,	ATA_HORKAGE_NO_ID_DEV_LOG),
>> +	force_horkage_onoff(log_dir,	ATA_HORKAGE_NO_LOG_DIR),
> 
>    Underscores in the names with "no" (without underscore) would look inconsistent,
> wouldn't they? Maybe drop the underscores here?

Yep. Done.

> 
> [...]
> 
> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> MBR, Sergey


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] ata: libata-core: Refactor force_tbl definition
  2022-04-24 17:53   ` Sergey Shtylyov
@ 2022-04-25  1:34     ` Damien Le Moal
  0 siblings, 0 replies; 15+ messages in thread
From: Damien Le Moal @ 2022-04-25  1:34 UTC (permalink / raw)
  To: Sergey Shtylyov, linux-ide, Jonathan Corbet, linux-doc

On 4/25/22 02:53, Sergey Shtylyov wrote:
> On 4/7/22 3:37 PM, Damien Le Moal wrote:
> 
>> Introduce the macro definitions force_cbl(), force_spd_limit(),
>> force_xfer(), force_lflag_on(), force_horkage_on() and
>> force_horkage_onoff() to define with a more compact syntax the struct
>> ata_force_param entries in the force_tbl array defined in the function
>> ata_parse_force_one().
>> To reduce the indentation of the array declaration, force_tbl definition
>> is also moved out of the ata_parse_force_one() function.
> 
>    Some entries are reordered too... :-)
> 
>> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> 
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index bc59c77b99b6..c0cf42ffcc38 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -6143,67 +6143,90 @@ int ata_platform_remove_one(struct platform_device *pdev)
>>  EXPORT_SYMBOL_GPL(ata_platform_remove_one);
>>  
>>  #ifdef CONFIG_ATA_FORCE
>> +
>> +#define force_cbl(name, flag)				\
>> +	{ # name,	.cbl		= (flag) }
> 
>    Why not #name here and below?
> 
>> +
>> +#define force_spd_limit(spd, val)			\
>> +	{ # spd,	.spd_limit	= (val) }
>> +
>> +#define force_xfer(mode, shift)				\
>> +	{ # mode,	.xfer_mask	= (1UL << (shift)) }
>> +
>> +#define force_lflag_on(name, flags)			\
> 
>    Not just force_lflag()?

Patch 3 adds force_lflag_onoff(), so I added the on version here. Will
move this change to patch 3.

> 
>> +	{ # name,	.lflags		= (flags) }
>> +
>> +#define force_horkage_on(name, flag)			\
>> +	{ # name,	.horkage_on	= (flag) }
>> +
>> +#define force_horkage_onoff(name, flag)			\
>> +	{ "no" # name,	.horkage_on	= (flag) },	\
>> +	{ # name,	.horkage_off	= (flag) }
>> +
> [...]
>> @@ -6285,7 +6308,7 @@ static void __init ata_parse_force_param(void)
>>  	int last_port = -1, last_device = -1;
>>  	char *p, *cur, *next;
>>  
>> -	/* calculate maximum number of params and allocate force_tbl */
>> +	/* Calculate maximum number of params and allocate ata_force_tbl */
> 
>    Drove-by change? :-)

Yeah, just a comment bug I caught along the way. I do not think it
deserves its own patch. Will mention it in the commit message.

> 
> [...]
> 
> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> MBR, Sergey


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/5] doc: admin-guide: Update libata kernel parameters
  2022-04-07 12:37 ` [PATCH 5/5] doc: admin-guide: Update libata kernel parameters Damien Le Moal
@ 2022-04-25 12:10   ` Sergey Shtylyov
  0 siblings, 0 replies; 15+ messages in thread
From: Sergey Shtylyov @ 2022-04-25 12:10 UTC (permalink / raw)
  To: Damien Le Moal, linux-ide, Jonathan Corbet, linux-doc

On 4/7/22 3:37 PM, Damien Le Moal wrote:

> Cleanup the text text describing the libata.force boot parameter and
                   ^^^^           
   Once is enough. :-)

> update the list of the values to include all supported horkage and link
> flag that can be forced.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  .../admin-guide/kernel-parameters.txt         | 71 ++++++++++++++-----
>  1 file changed, 55 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 3f1cc5e317ed..30734a610b92 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
[...]
> @@ -2639,7 +2639,7 @@
>  			host link and device attached to it.
>  
>  			The VAL specifies the configuration to force.  As long
> -			as there's no ambiguity shortcut notation is allowed.
> +			as there is no ambiguity, shortcut notation is allowed.

   Dunno what you have against more informal style in the kernel... :-)

[...]
> @@ -2652,19 +2652,58 @@
[...]
> -			* dump_id: dump IDENTIFY data.
> +			* [no]setxfer: Indicate if transfer speed mode setting
> +			  should be skipped.
                                ^ (not)

[...]

Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>

MBR, Sergey

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-04-25 12:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-07 12:37 [PATCH 0/5] libata.force improvements Damien Le Moal
2022-04-07 12:37 ` [PATCH 1/5] ata: libata-core: cleanup ata_device_blacklist Damien Le Moal
2022-04-24 17:29   ` Sergey Shtylyov
2022-04-07 12:37 ` [PATCH 2/5] ata: libata-core: Refactor force_tbl definition Damien Le Moal
2022-04-24 17:53   ` Sergey Shtylyov
2022-04-25  1:34     ` Damien Le Moal
2022-04-07 12:37 ` [PATCH 3/5] ata: libata-core: Improve link flags forced settings Damien Le Moal
2022-04-24 18:01   ` Sergey Shtylyov
2022-04-07 12:37 ` [PATCH 4/5] ata: libata-core: Allow forcing most horkage flags Damien Le Moal
2022-04-24 18:17   ` Sergey Shtylyov
2022-04-25  1:15     ` Damien Le Moal
2022-04-07 12:37 ` [PATCH 5/5] doc: admin-guide: Update libata kernel parameters Damien Le Moal
2022-04-25 12:10   ` Sergey Shtylyov
2022-04-12  0:45 ` [PATCH 0/5] libata.force improvements Damien Le Moal
2022-04-17 18:10   ` Sergey Shtylyov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.