Re: [PATCH 1/5] ata: ahci: move marking of external port earlier

[Damien Le Moal <dlemoal@kernel.org>]

On 2/2/24 01:14, Niklas Cassel wrote:
> Move the marking of an external port earlier in the call chain.
> This is needed for further cleanups.
> No functional change intended.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/ata/ahci.c    | 14 ++++++++++++++
>  drivers/ata/libahci.c |  7 -------
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index da2e74fce2d9..157ab88bdf75 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1642,6 +1642,17 @@ static int ahci_init_msi(struct pci_dev *pdev, unsigned int n_ports,
>  	return pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
>  }
>  
> +static void ahci_mark_external_port(struct ata_port *ap)
> +{
> +	struct ahci_host_priv *hpriv = ap->host->private_data;
> +	void __iomem *port_mmio = ahci_port_base(ap);
> +	u32 tmp;
> +
> +	tmp = readl(port_mmio + PORT_CMD);
> +	if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS))
> +		ap->pflags |= ATA_PFLAG_EXTERNAL;
> +}
> +
>  static void ahci_update_initial_lpm_policy(struct ata_port *ap,
>  					   struct ahci_host_priv *hpriv)
>  {
> @@ -1934,6 +1945,9 @@ 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 */
> +		ahci_mark_external_port(ap);

Nit: the comment is rather useless as the function name is clear. It is also
rather incorrect since this is for both esata as well as hot-plug capable ports
(later in the series). So maybe drop it ?

Otherwise, looks OK to me.

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

> +
>  		ahci_update_initial_lpm_policy(ap, hpriv);
>  
>  		/* disabled/not-implemented port */
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 1a63200ea437..fca376f03c9e 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -1280,10 +1280,8 @@ static void ahci_port_init(struct device *dev, struct ata_port *ap,
>  			   int port_no, void __iomem *mmio,
>  			   void __iomem *port_mmio)
>  {
> -	struct ahci_host_priv *hpriv = ap->host->private_data;
>  	const char *emsg = NULL;
>  	int rc;
> -	u32 tmp;
>  
>  	/* make sure port is not active */
>  	rc = ahci_deinit_port(ap, &emsg);
> @@ -1291,11 +1289,6 @@ static void ahci_port_init(struct device *dev, struct ata_port *ap,
>  		dev_warn(dev, "%s (%d)\n", emsg, rc);
>  
>  	ahci_port_clear_pending_irq(ap);
> -
> -	/* mark esata ports */
> -	tmp = readl(port_mmio + PORT_CMD);
> -	if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS))
> -		ap->pflags |= ATA_PFLAG_EXTERNAL;
>  }
>  
>  void ahci_init_controller(struct ata_host *host)

-- 
Damien Le Moal
Western Digital Research