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

[Jason Yan <yanaijie@huawei.com>]

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