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