On 1/16/25 23:36, Niklas Cassel wrote:
> 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 the 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 a module parameter
> 'external_port_map' for the 'ahci' driver.
Maybe we should call the parameter "force_external_port_map" to make it clear
that this is a user forced setup ?
Otherwise looks good to me.
>
> The external_port_map parameter accepts 2 different formats:
> - external_port_map=<map>
> This applies the same map to all AHCI controllers
> present in the system. This format is convenient for small systems
> that have only a single AHCI controller.
> - external_port_map=<pci_dev>=<map>,<pci_dev>=map,...
> This applies the specified maps only to the PCI device listed. The
> <pci_dev> field is a regular PCI device ID (domain:bus:dev.func).
> This ID can be seen following "ahci" in the kernel messages. E.g.
> for "ahci 0000:01:00.0: 2/2 ports implemented (port mask 0x3)", the
> <pci_dev> field is "0000:01:00.0".
>
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
> drivers/ata/ahci.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 92b08d3a0c3c..ec2bc5f17b96 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -676,6 +676,17 @@ MODULE_PARM_DESC(mask_port_map,
> "where <pci_dev> is the PCI ID of an AHCI controller in the "
> "form \"domain:bus:dev.func\"");
>
> +static char *ahci_external_port_map;
> +module_param_named(external_port_map, ahci_external_port_map, charp, 0444);
> +MODULE_PARM_DESC(external_port_map,
> + "32-bits port map to force set one or more ports as external. "
> + "Valid values are: "
> + "\"<map>\" to apply the same map to all AHCI controller "
> + "devices, and \"<pci_dev>=<map>,<pci_dev>=<map>,...\" to "
> + "specify different maps for the controllers specified, "
> + "where <pci_dev> is the PCI ID of an AHCI controller in the "
> + "form \"domain:bus:dev.func\"");
> +
> typedef void (*port_map_callback_t)(struct device *dev,
> struct ahci_host_priv *hpriv, char *mask_s);
>
> @@ -758,6 +769,34 @@ static void ahci_get_port_map_mask(struct device *dev,
> ahci_apply_port_map_mask);
> }
>
> +static void ahci_apply_external_port_map(struct device *dev,
> + struct ahci_host_priv *hpriv,
> + char *mask_s)
> +{
> + struct ata_host *host = dev_get_drvdata(dev);
> + unsigned long port_map;
> + unsigned int map;
> + int i;
> +
> + if (kstrtouint(mask_s, 0, &map)) {
> + dev_err(dev, "Invalid external port map\n");
> + return;
> + }
> +
> + port_map = map;
> + for_each_set_bit(i, &port_map, AHCI_MAX_PORTS) {
> + if (i < host->n_ports)
> + host->ports[i]->pflags |= ATA_PFLAG_EXTERNAL;
> + }
> +}
> +
> +static void ahci_get_external_port_map(struct device *dev,
> + struct ahci_host_priv *hpriv)
> +{
> + ahci_get_port_map_helper(dev, hpriv, ahci_external_port_map,
> + ahci_apply_external_port_map);
> +}
> +
> static void ahci_pci_save_initial_config(struct pci_dev *pdev,
> struct ahci_host_priv *hpriv)
> {
> @@ -2020,6 +2059,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (pi.flags & ATA_FLAG_EM)
> ahci_reset_em(host);
>
> + /* Handle external port map passed as module parameter. */
> + if (ahci_external_port_map)
> + ahci_get_external_port_map(&pdev->dev, hpriv);
> +
> for (i = 0; i < host->n_ports; i++) {
> struct ata_port *ap = host->ports[i];
>
--
Damien Le Moal
Western Digital Research
