On 4/25/22 03:34, 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. To be consistent with naming, the macro force_lflag() is
renamed to force_lflag_on().
Using force_lflag_onoff(), 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@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
---
drivers/ata/libata-core.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index dfdb23c2bbd6..e5a0e73b39d3 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);
}
}
}
@@ -6153,8 +6160,12 @@ EXPORT_SYMBOL_GPL(ata_platform_remove_one);
#define force_xfer(mode, shift) \
{ #mode, .xfer_mask = (1UL << (shift)) }
-#define force_lflag(name, flags) \
- { #name, .lflags = (flags) }
+#define force_lflag_on(name, 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) }
@@ -6209,10 +6220,11 @@ static const struct ata_force_param force_tbl[] __initconst = {
force_xfer(udma/133, ATA_SHIFT_UDMA + 6),
force_xfer(udma7, ATA_SHIFT_UDMA + 7),
- force_lflag(nohrst, ATA_LFLAG_NO_HRST),
- force_lflag(nosrst, ATA_LFLAG_NO_SRST),
- force_lflag(norst, ATA_LFLAG_NO_HRST | ATA_LFLAG_NO_SRST),
- force_lflag(rstonce, ATA_LFLAG_RST_ONCE),
+ 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_lflag_onoff(dbdelay, ATA_LFLAG_NO_DEBOUNCE_DELAY),
force_horkage_onoff(ncq, ATA_HORKAGE_NONCQ),
force_horkage_onoff(ncqtrim, ATA_HORKAGE_NO_NCQ_TRIM),
Hmm.
Personally, I'm not a fan of distinct 'flags_on' and 'flags_off'; I
always wonder what does happen if one sets the same value for both ...
And do we really need this? All settings above are just simple flags
which can be set or unset.
Sure, some flags are inverted, but still they are flags.
So why do we need the separation here?
Isn't it rather cosmetical?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
