Re: [PATCH 2/6] scsi: libsas: delete wrapper function sas_discover_end_dev()

John Garry
2022-12-05T08:59:57Z

On 04/12/2022 08:16, Jason Yan wrote:
> After commit 0558f33c06bb ("scsi: libsas: direct call probe and destruct")
> this function is only a wrapper of sas_notify_lldd_dev_found(). And the
> function name does not reflect the real purpose of this function now.

Why is this? Maybe add "dev_found" to the name could help.

> Remove it and call sas_notify_lldd_dev_found() directly. The log is also
> changed accordingly.
>
> Cc: John Garry <john.g.garry@oracle.com>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> ---
> drivers/scsi/libsas/sas_discover.c | 13 +------------
> drivers/scsi/libsas/sas_expander.c | 4 ++--
> include/scsi/libsas.h | 1 -
> 3 files changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
> index d5bc1314c341..efc6bf95bb67 100644
> --- a/drivers/scsi/libsas/sas_discover.c
> +++ b/drivers/scsi/libsas/sas_discover.c
> @@ -269,17 +269,6 @@ static void sas_resume_devices(struct work_struct *work)
> sas_resume_sata(port);
> }
>
> -/**
> - * sas_discover_end_dev - discover an end device (SSP, etc)
> - * @dev: pointer to domain device of interest
> - *
> - * See comment in sas_discover_sata().
> - */
> -int sas_discover_end_dev(struct domain_device *dev)
> -{
> - return sas_notify_lldd_dev_found(dev);
> -}
> -
> /* ---------- Device registration and unregistration ---------- */
>
> void sas_free_device(struct kref *kref)
> @@ -447,7 +436,7 @@ static void sas_discover_domain(struct work_struct *work)
>
> switch (dev->dev_type) {
> case SAS_END_DEVICE:
> - error = sas_discover_end_dev(dev);
> + error = sas_notify_lldd_dev_found(dev);

For me, personally, I prefer consistent API name, like
sas_discover_end_dev() and sas_discover_sata(), even if
sas_discover_end_dev() is just a wrapper.

> break;
> case SAS_EDGE_EXPANDER_DEVICE:
> case SAS_FANOUT_EXPANDER_DEVICE:
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index a04cad620e93..aa8ea3b1f2e4 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -855,9 +855,9 @@ static struct domain_device *sas_ex_discover_end_dev(
>
> list_add_tail(&child->disco_list_node, &parent->port->disco_list);
>
> - res = sas_discover_end_dev(child);
> + res = sas_notify_lldd_dev_found(child);
> if (res) {
> - pr_notice("sas_discover_end_dev() for device %016llx at %016llx:%02d returned 0x%x\n",
> + pr_notice("notify lldd for device %016llx at %016llx:%02d returned 0x%x\n",
> SAS_ADDR(child->sas_addr),
> SAS_ADDR(parent->sas_addr), phy_id, res);
> goto out_list_del;
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index 1aee3d0ebbb2..87682390fb76 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -736,7 +736,6 @@ void sas_init_disc(struct sas_discovery *disc, struct asd_sas_port *);
> void sas_discover_event(struct asd_sas_port *, enum discover_event ev);
>
> int sas_discover_sata(struct domain_device *);
> -int sas_discover_end_dev(struct domain_device *);
>
> void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *);
>