diff mbox series

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

Message ID 20221204081643.3835966-3-yanaijie@huawei.com (mailing list archive)
State Superseded
Headers show
Series scsi: libsas: Some coding style fixes and cleanups | expand

Commit Message

Jason Yan Dec. 4, 2022, 8:16 a.m. UTC
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.
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(-)

Comments

John Garry Dec. 5, 2022, 8:57 a.m. UTC | #1
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 *);
>
Jason Yan Dec. 8, 2022, 6:56 a.m. UTC | #2
On 2022/12/5 16:57, John Garry wrote:
> 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.

Fair enough. I was just thinking that this API name is not proper now
because it is only notifying the lldd.

I will drop this patch if you insist.

Thanks,
Jason
diff mbox series

Patch

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);
 		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 *);