diff mbox series

[2/5] ata: ahci: a hotplug capable port is an external port

Message ID 20240201161507.1147521-3-cassel@kernel.org
State New
Headers show
Series drop low power policy board type | expand

Commit Message

Niklas Cassel Feb. 1, 2024, 4:14 p.m. UTC
A hotplug capable port is an external port, so mark it as such.

We even say this ourselves in libata-scsi.c:
/* set scsi removable (RMB) bit per ata bit, or if the
 * AHCI port says it's external (Hotplug-capable, eSATA).
 */

This also matches the terminology used in AHCI 1.3.1
(the keyword to search for is "externally accessible").

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/ahci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Damien Le Moal Feb. 2, 2024, 1:54 a.m. UTC | #1
On 2/2/24 01:14, Niklas Cassel wrote:
> A hotplug capable port is an external port, so mark it as such.
> 
> We even say this ourselves in libata-scsi.c:
> /* set scsi removable (RMB) bit per ata bit, or if the
>  * AHCI port says it's external (Hotplug-capable, eSATA).
>  */
> 
> This also matches the terminology used in AHCI 1.3.1
> (the keyword to search for is "externally accessible").
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/ata/ahci.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 157ab88bdf75..8c8403bae36f 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1649,7 +1649,8 @@ static void ahci_mark_external_port(struct ata_port *ap)
>  	u32 tmp;
>  
>  	tmp = readl(port_mmio + PORT_CMD);
> -	if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS))
> +	if (((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) ||
> +	    (tmp & PORT_CMD_HPCP))
>  		ap->pflags |= ATA_PFLAG_EXTERNAL;
>  }
>  
> @@ -1945,7 +1946,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		if (ap->flags & ATA_FLAG_EM)
>  			ap->em_message_type = hpriv->em_msg_type;
>  
> -		/* mark esata ports */
> +		/* mark external ports (hotplug-capable, eSATA) */

OK. So you fix the comment here... Maybe move that comment inside
ahci_mark_external_port() to make it clear which port are considered "external"
and remove the comment here ?

Otherwise, looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

>  		ahci_mark_external_port(ap);
>  
>  		ahci_update_initial_lpm_policy(ap, hpriv);
diff mbox series

Patch

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 157ab88bdf75..8c8403bae36f 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1649,7 +1649,8 @@  static void ahci_mark_external_port(struct ata_port *ap)
 	u32 tmp;
 
 	tmp = readl(port_mmio + PORT_CMD);
-	if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS))
+	if (((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) ||
+	    (tmp & PORT_CMD_HPCP))
 		ap->pflags |= ATA_PFLAG_EXTERNAL;
 }
 
@@ -1945,7 +1946,7 @@  static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		if (ap->flags & ATA_FLAG_EM)
 			ap->em_message_type = hpriv->em_msg_type;
 
-		/* mark esata ports */
+		/* mark external ports (hotplug-capable, eSATA) */
 		ahci_mark_external_port(ap);
 
 		ahci_update_initial_lpm_policy(ap, hpriv);