Add 'external' to the libata.force kernel parameter

Hello there,

This patch replaces an earlier patch proposal, which added an 'external_port_map' kernel module parameter to the ahci driver:
https://lore.kernel.org/linux-ide/[email protected]/

This patch instead adds support for 'external' to the existing libata.force kernel parameter.

Normally we do not enable LPM on ports that are marked as external by firmware. However, as always, firmware can have bugs and can have forgotten to mark a port as external.

One workaround for such buggy firmware would be to modify the 'mobile_lpm_policy' kernel module parameter, however, that will affect the LPM policy for all ports.

This patch allows a user to mark one or more ports as hotplug capable (in case firmware has failed to do so). This way, you will still get LPM (power saving) on the ports that are not marked as external.

One example where this can be used, is for buggy devices such as the "HL-DT-ST BD-RE BU40N" Blu-Ray player, which does not reply to a regular COMRESET (the controller sees nothing as connected), instead it will send a hotplug event when, and only when the user presses the tray open button.

Not replying to a COMRESET is not spec compliant. If a port does not detect any device on a port, and LPM is enabled on that port, and the port is not marked as hotplug capable, then there should be no way that a device can be hotplugged later, so libata powers off the port/PHY to save power.

Adding 'external' to libata.force gives users a way to handles such non spec compliant devices in a more fine grained way (rather than using the big 'mobile_lpm_policy' hammer).

There does also exist a per port link_power_management_policy sysfs attribute, however, for many people, using libata.force (which can be either on the kernel command line, or as a kernel module parameter) is more convenient compared to writing udev rules.

Kind regards,
Niklas

Niklas Cassel (1):
ata: libata-core: Add 'external' to the libata.force kernel parameter

 .../admin-guide/kernel-parameters.txt         |  2 +
 drivers/ata/libata-core.c                     | 38 +++++++++++++++++++
 2 files changed, 40 insertions(+)

2.48.1

Commit ae1f3db006b7 ("ata: ahci: do not enable LPM on external ports") changed so that LPM is not enabled on external ports (hotplug-capable or eSATA ports).

This is because hotplug and LPM are mutually exclusive, see 7.3.1 Hot Plug Removal Detection and Power Management Interaction in AHCI 1.3.1.

This does require that firmware has set the appropate bits (HPCP or ESP) in PxCMD (which is a per port register in the AHCI controller).

If the firmware has failed to mark a port as hotplug-capable or eSATA in PxCMD, then there is currently not much a user can do.

If LPM is enabled on the port, hotplug insertions and removals will not be detected on that port.

In order to allow a user to fix up broken firmware, add 'external' to the libata.force kernel parameter.

libata.force can be specified either on the kernel command line, or as a kernel module parameter.

For more information, see Documentation/admin-guide/kernel-parameters.txt.

---
 .../admin-guide/kernel-parameters.txt         |  2 +
 drivers/ata/libata-core.c                     | 38 +++++++++++++++++++
 2 files changed, 40 insertions(+)

index 3872bc6ec49d..d9c55dcbf4a2 100644
@@ -3036,6 +3036,8 @@
 			* max_sec_lba48: Set or clear transfer size limit to
 			  65535 sectors.
 
+			* external: Mark port as external (hotplug-capable).
+
 			* [no]lpm: Enable or disable link power management.
 
 			* [no]setxfer: Indicate if transfer speed mode setting

index c085dd81ebe7..52b78d22c831 100644
@@ -88,6 +88,7 @@ struct ata_force_param {
 	unsigned int	xfer_mask;
 	unsigned int	quirk_on;
 	unsigned int	quirk_off;
+	unsigned int	pflags_on;
 	u16		lflags_on;
 	u16		lflags_off;
 };

@@ -331,6 +332,35 @@ void ata_force_cbl(struct ata_port *ap)
 	}
 }
 
+/**
+ *	ata_force_pflags - force port flags according to libata.force
+ *	@ap: ATA port of interest
+ *
+ *	Force port flags according to libata.force and whine about it.
+ *
+ *	LOCKING:
+ *	EH context.
+ */
+static void ata_force_pflags(struct ata_port *ap)
+{
+	int i;
+
+	for (i = ata_force_tbl_size - 1; i >= 0; i--) {
+		const struct ata_force_ent *fe = &ata_force_tbl[i];
+
+		if (fe->port != -1 && fe->port != ap->print_id)
+			continue;
+
+		/* let pflags stack */
+		if (fe->param.pflags_on) {
+			ap->pflags |= fe->param.pflags_on;
+			ata_port_notice(ap,
+					"FORCE: port flag 0x%x forced -> 0x%x\n",
+					fe->param.pflags_on, ap->pflags);
+		}
+	}
+}
+
 /**
  *	ata_force_link_limits - force link limits according to libata.force
  *	@link: ATA link of interest

@@ -486,6 +516,7 @@ static void ata_force_quirks(struct ata_device *dev)
 	}
 }
 #else
+static inline void ata_force_pflags(struct ata_port *ap) { }
 static inline void ata_force_link_limits(struct ata_link *link) { }
 static inline void ata_force_xfermask(struct ata_device *dev) { }
 static inline void ata_force_quirks(struct ata_device *dev) { }

@@ -5452,6 +5483,8 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
 #endif
 	ata_sff_port_init(ap);
 
+	ata_force_pflags(ap);
+
 	return ap;
 }
 EXPORT_SYMBOL_GPL(ata_port_alloc);

@@ -6264,6 +6297,9 @@ EXPORT_SYMBOL_GPL(ata_platform_remove_one);
 	{ "no" #name,	.lflags_on	= (flags) },	\
 	{ #name,	.lflags_off	= (flags) }
 
+#define force_pflag_on(name, flags)			\
+	{ #name,	.pflags_on	= (flags) }
+
 #define force_quirk_on(name, flag)			\
 	{ #name,	.quirk_on	= (flag) }
 

@@ -6323,6 +6359,8 @@ static const struct ata_force_param force_tbl[] __initconst = {
 	force_lflag_on(rstonce,		ATA_LFLAG_RST_ONCE),
 	force_lflag_onoff(dbdelay,	ATA_LFLAG_NO_DEBOUNCE_DELAY),
 
+	force_pflag_on(external,	ATA_PFLAG_EXTERNAL),
+
 	force_quirk_onoff(ncq,		ATA_QUIRK_NONCQ),
 	force_quirk_onoff(ncqtrim,	ATA_QUIRK_NO_NCQ_TRIM),
 	force_quirk_onoff(ncqati,	ATA_QUIRK_NO_NCQ_ON_ATI),

2.48.1

Looks good to me.

Damien Le Moal
Western Digital Research

Applied to libata/linux.git (for-6.15), thanks!

[1/1] ata: libata-core: Add 'external' to the libata.force kernel parameter

      https://git.kernel.org/libata/linux/c/deca4232

Kind regards,
Niklas