Re: [PATCH v2 04/13] ata,scsi: Remove useless wrappers ata_sas_tport_{add,delete}()

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

On 6/27/24 03:00, Niklas Cassel wrote:
> Remove useless wrappers ata_sas_tport_add() and ata_sas_tport_delete().

I am not a big fan of this patch. I would prefer to keep everything in
libata-transport local to libata and have the exports done for what libsas needs
using the wrappers, even if some of them are very trivial...

That also allows keeping the naming consistent with the ata_sas_ prefix for
everything that libsas uses from libata.

John ? Thoughts ?

> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/ata/libata-sata.c          | 12 ------------
>  drivers/ata/libata-transport.c     |  2 ++
>  drivers/ata/libata-transport.h     |  3 ---
>  drivers/scsi/libsas/sas_ata.c      |  2 +-
>  drivers/scsi/libsas/sas_discover.c |  2 +-
>  include/linux/libata.h             |  4 ++--
>  6 files changed, 6 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> index 9e047bf912b1..e7991595bfe5 100644
> --- a/drivers/ata/libata-sata.c
> +++ b/drivers/ata/libata-sata.c
> @@ -1241,18 +1241,6 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host,
>  }
>  EXPORT_SYMBOL_GPL(ata_sas_port_alloc);
>  
> -int ata_sas_tport_add(struct device *parent, struct ata_port *ap)
> -{
> -	return ata_tport_add(parent, ap);
> -}
> -EXPORT_SYMBOL_GPL(ata_sas_tport_add);
> -
> -void ata_sas_tport_delete(struct ata_port *ap)
> -{
> -	ata_tport_delete(ap);
> -}
> -EXPORT_SYMBOL_GPL(ata_sas_tport_delete);
> -
>  /**
>   *	ata_sas_device_configure - Default device_configure routine for libata
>   *				   devices
> diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
> index 3e49a877500e..d24f201c0ab2 100644
> --- a/drivers/ata/libata-transport.c
> +++ b/drivers/ata/libata-transport.c
> @@ -265,6 +265,7 @@ void ata_tport_delete(struct ata_port *ap)
>  	transport_destroy_device(dev);
>  	put_device(dev);
>  }
> +EXPORT_SYMBOL_GPL(ata_tport_delete);
>  
>  static const struct device_type ata_port_sas_type = {
>  	.name = ATA_PORT_TYPE_NAME,
> @@ -329,6 +330,7 @@ int ata_tport_add(struct device *parent,
>  	put_device(dev);
>  	return error;
>  }
> +EXPORT_SYMBOL_GPL(ata_tport_add);
>  
>  /**
>   *     ata_port_classify - determine device type based on ATA-spec signature
> diff --git a/drivers/ata/libata-transport.h b/drivers/ata/libata-transport.h
> index 08a57fb9dc61..50cd2cbe8eea 100644
> --- a/drivers/ata/libata-transport.h
> +++ b/drivers/ata/libata-transport.h
> @@ -8,9 +8,6 @@ extern struct scsi_transport_template *ata_scsi_transport_template;
>  int ata_tlink_add(struct ata_link *link);
>  void ata_tlink_delete(struct ata_link *link);
>  
> -int ata_tport_add(struct device *parent, struct ata_port *ap);
> -void ata_tport_delete(struct ata_port *ap);
> -
>  struct scsi_transport_template *ata_attach_transport(void);
>  void ata_release_transport(struct scsi_transport_template *t);
>  
> diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
> index 4c69fc63c119..1c2400c96ebd 100644
> --- a/drivers/scsi/libsas/sas_ata.c
> +++ b/drivers/scsi/libsas/sas_ata.c
> @@ -608,7 +608,7 @@ int sas_ata_init(struct domain_device *found_dev)
>  	ap->cbl = ATA_CBL_SATA;
>  	ap->scsi_host = shost;
>  
> -	rc = ata_sas_tport_add(ata_host->dev, ap);
> +	rc = ata_tport_add(ata_host->dev, ap);
>  	if (rc)
>  		goto destroy_port;
>  
> diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
> index 8fb7c41c0962..6e01ddec10c9 100644
> --- a/drivers/scsi/libsas/sas_discover.c
> +++ b/drivers/scsi/libsas/sas_discover.c
> @@ -300,7 +300,7 @@ void sas_free_device(struct kref *kref)
>  		kfree(dev->ex_dev.ex_phy);
>  
>  	if (dev_is_sata(dev) && dev->sata_dev.ap) {
> -		ata_sas_tport_delete(dev->sata_dev.ap);
> +		ata_tport_delete(dev->sata_dev.ap);
>  		kfree(dev->sata_dev.ap);
>  		ata_host_put(dev->sata_dev.ata_host);
>  		dev->sata_dev.ata_host = NULL;
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 13fb41d25da6..581e166615fa 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1249,8 +1249,8 @@ extern int ata_slave_link_init(struct ata_port *ap);
>  extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
>  					   struct ata_port_info *, struct Scsi_Host *);
>  extern void ata_port_probe(struct ata_port *ap);
> -extern int ata_sas_tport_add(struct device *parent, struct ata_port *ap);
> -extern void ata_sas_tport_delete(struct ata_port *ap);
> +extern int ata_tport_add(struct device *parent, struct ata_port *ap);
> +extern void ata_tport_delete(struct ata_port *ap);
>  int ata_sas_device_configure(struct scsi_device *sdev, struct queue_limits *lim,
>  		struct ata_port *ap);
>  extern int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap);

-- 
Damien Le Moal
Western Digital Research