[PATCH v2 0/8] scsi: libsas: sas address comparation refactor

[PATCH v2 0/8] scsi: libsas: sas address comparation refactor

[Jason Yan]

Sas address conversion and comparation is widely used in libsas and
drivers. However they are all opencoded and to avoid the line spill over
80 columns, are mostly split into multi-lines.

To make the code easier to read, introduce some helpers with clearer
semantics and replace the opencoded segments with them.

v1->v2:
  First factor out sas_find_attached_phy() and replace LLDDs's code
  	with it.
  Remove three too simple helpers.
  Rename the helpers with 'sas_' prefix.

Jason Yan (8):
  scsi: libsas: introduce sas_find_attached_phy() helper
  scsi: pm8001: use sas_find_attached_phy() instead of open coded
  scsi: mvsas: use sas_find_attached_phy() instead of open coded
  scsi: hisi_sas: use sas_find_attathed_phy() instead of open coded
  scsi: libsas: introduce sas address comparation helpers
  scsi: libsas: use sas_phy_match_dev_addr() instead of open coded
  scsi: libsas: use sas_phy_addr_same() instead of open coded
  scsi: libsas: use sas_phy_match_port_addr() instead of open coded

 drivers/scsi/hisi_sas/hisi_sas_main.c | 12 ++------
 drivers/scsi/libsas/sas_expander.c    | 40 ++++++++++++++++-----------
 drivers/scsi/libsas/sas_internal.h    | 17 ++++++++++++
 drivers/scsi/mvsas/mv_sas.c           | 15 +++-------
 drivers/scsi/pm8001/pm8001_sas.c      | 16 ++++-------
 include/scsi/libsas.h                 |  2 ++
 6 files changed, 54 insertions(+), 48 deletions(-)

-- 
2.31.1

[PATCH v2 1/8] scsi: libsas: introduce sas_find_attached_phy() helper

[Jason Yan]

LLDDs are implementing their own attached phy finding code repeatedly.
Factor it out to libsas.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/scsi/libsas/sas_expander.c | 17 +++++++++++++++++
 include/scsi/libsas.h              |  2 ++
 2 files changed, 19 insertions(+)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index fa2209080cc2..a5effff3d1c3 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2107,6 +2107,23 @@ int sas_ex_revalidate_domain(struct domain_device *port_dev)
 	return res;
 }
 
+int sas_find_attached_phy(struct expander_device *ex_dev,
+			  struct domain_device *dev)
+{
+	struct ex_phy *phy;
+	int phy_id;
+
+	for (phy_id = 0; phy_id < ex_dev->num_phys; phy_id++) {
+		phy = &ex_dev->ex_phy[phy_id];
+		if (SAS_ADDR(phy->attached_sas_addr)
+			== SAS_ADDR(dev->sas_addr))
+			return phy_id;
+	}
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(sas_find_attached_phy);
+
 void sas_smp_handler(struct bsg_job *job, struct Scsi_Host *shost,
 		struct sas_rphy *rphy)
 {
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 2dbead74a2af..75faf2308eae 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -750,6 +750,8 @@ int sas_clear_task_set(struct domain_device *dev, u8 *lun);
 int sas_lu_reset(struct domain_device *dev, u8 *lun);
 int sas_query_task(struct sas_task *task, u16 tag);
 int sas_abort_task(struct sas_task *task, u16 tag);
+int sas_find_attached_phy(struct expander_device *ex_dev,
+			  struct domain_device *dev);
 
 void sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event,
 			   gfp_t gfp_flags);
-- 
2.31.1

[PATCH v2 2/8] scsi: pm8001: use sas_find_attached_phy() instead of open coded

[Jason Yan]

The attached phy finding is open coded. Now we can replace it with
sas_find_attached_phy().

Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/scsi/pm8001/pm8001_sas.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 8e3f2f9ddaac..d15a824bcea6 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -645,22 +645,16 @@ static int pm8001_dev_found_notify(struct domain_device *dev)
 	pm8001_device->dcompletion = &completion;
 	if (parent_dev && dev_is_expander(parent_dev->dev_type)) {
 		int phy_id;
-		struct ex_phy *phy;
-		for (phy_id = 0; phy_id < parent_dev->ex_dev.num_phys;
-		phy_id++) {
-			phy = &parent_dev->ex_dev.ex_phy[phy_id];
-			if (SAS_ADDR(phy->attached_sas_addr)
-				== SAS_ADDR(dev->sas_addr)) {
-				pm8001_device->attached_phy = phy_id;
-				break;
-			}
-		}
-		if (phy_id == parent_dev->ex_dev.num_phys) {
+
+		phy_id = sas_find_attached_phy(&parent_dev->ex_dev, dev);
+		if (phy_id == -ENODEV) {
 			pm8001_dbg(pm8001_ha, FAIL,
 				   "Error: no attached dev:%016llx at ex:%016llx.\n",
 				   SAS_ADDR(dev->sas_addr),
 				   SAS_ADDR(parent_dev->sas_addr));
 			res = -1;
+		} else {
+			pm8001_device->attached_phy = phy_id;
 		}
 	} else {
 		if (dev->dev_type == SAS_SATA_DEV) {
-- 
2.31.1

[PATCH v2 3/8] scsi: mvsas: use sas_find_attached_phy() instead of open coded

[Jason Yan]

The attached phy finding is open coded. Now we can replace it with
sas_find_attached_phy().

Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/scsi/mvsas/mv_sas.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index a6867dae0e7c..f6576ffabe9f 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -1190,23 +1190,16 @@ static int mvs_dev_found_notify(struct domain_device *dev, int lock)
 	mvi_device->sas_device = dev;
 	if (parent_dev && dev_is_expander(parent_dev->dev_type)) {
 		int phy_id;
-		u8 phy_num = parent_dev->ex_dev.num_phys;
-		struct ex_phy *phy;
-		for (phy_id = 0; phy_id < phy_num; phy_id++) {
-			phy = &parent_dev->ex_dev.ex_phy[phy_id];
-			if (SAS_ADDR(phy->attached_sas_addr) ==
-				SAS_ADDR(dev->sas_addr)) {
-				mvi_device->attached_phy = phy_id;
-				break;
-			}
-		}
 
-		if (phy_id == phy_num) {
+		phy_id = sas_find_attached_phy(&parent_dev->ex_dev, dev);
+		if (phy_id == -ENODEV) {
 			mv_printk("Error: no attached dev:%016llx"
 				"at ex:%016llx.\n",
 				SAS_ADDR(dev->sas_addr),
 				SAS_ADDR(parent_dev->sas_addr));
 			res = -1;
+		} else {
+			mvi_device->attached_phy = phy_id;
 		}
 	}
 
-- 
2.31.1

[PATCH v2 4/8] scsi: hisi_sas: use sas_find_attathed_phy() instead of open coded

[Jason Yan]

The attached phy finding is open coded. Now we can replace it with
sas_find_attached_phy().

Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 33af5b8dede2..995ccb13fb9d 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -792,17 +792,9 @@ static int hisi_sas_dev_found(struct domain_device *device)
 
 	if (parent_dev && dev_is_expander(parent_dev->dev_type)) {
 		int phy_no;
-		u8 phy_num = parent_dev->ex_dev.num_phys;
-		struct ex_phy *phy;
 
-		for (phy_no = 0; phy_no < phy_num; phy_no++) {
-			phy = &parent_dev->ex_dev.ex_phy[phy_no];
-			if (SAS_ADDR(phy->attached_sas_addr) ==
-				SAS_ADDR(device->sas_addr))
-				break;
-		}
-
-		if (phy_no == phy_num) {
+		phy_no = sas_find_attached_phy(&parent_dev->ex_dev, device);
+		if (phy_no == -ENODEV) {
 			dev_info(dev, "dev found: no attached "
 				 "dev:%016llx at ex:%016llx\n",
 				 SAS_ADDR(device->sas_addr),
-- 
2.31.1

[PATCH v2 5/8] scsi: libsas: introduce sas address comparation helpers

[Jason Yan]

Sas address comparation is widely used in libsas. However they are all
opencoded and to avoid the line spill over 80 columns, are mostly split
into multi-lines. Introduce some helpers to prepare some refactor.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/scsi/libsas/sas_internal.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 8d0ad3abc7b5..171ffad514ca 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -111,6 +111,23 @@ static inline void sas_smp_host_handler(struct bsg_job *job,
 }
 #endif
 
+static inline bool sas_phy_match_dev_addr(struct domain_device *dev,
+					 struct ex_phy *phy)
+{
+	return SAS_ADDR(dev->sas_addr) == SAS_ADDR(phy->attached_sas_addr);
+}
+
+static inline bool sas_phy_match_port_addr(struct asd_sas_port *port,
+					   struct ex_phy *phy)
+{
+	return SAS_ADDR(port->sas_addr) == SAS_ADDR(phy->attached_sas_addr);
+}
+
+static inline bool sas_phy_addr_same(struct ex_phy *p1, struct ex_phy *p2)
+{
+	return  SAS_ADDR(p1->attached_sas_addr) == SAS_ADDR(p2->attached_sas_addr);
+}
+
 static inline void sas_fail_probe(struct domain_device *dev, const char *func, int err)
 {
 	pr_warn("%s: for %s device %016llx returned %d\n",
-- 
2.31.1

Re: [PATCH v2 5/8] scsi: libsas: introduce sas address comparation helpers

[John Garry]

On 24/09/2022 08:34, Jason Yan wrote:
> +static inline bool sas_phy_match_dev_addr(struct domain_device *dev,
> +					 struct ex_phy *phy)
> +{
> +	return SAS_ADDR(dev->sas_addr) == SAS_ADDR(phy->attached_sas_addr);
> +}
> +
> +static inline bool sas_phy_match_port_addr(struct asd_sas_port *port,
> +					   struct ex_phy *phy)
> +{
> +	return SAS_ADDR(port->sas_addr) == SAS_ADDR(phy->attached_sas_addr);
> +}
> +
> +static inline bool sas_phy_addr_same(struct ex_phy *p1, struct ex_phy *p2)

nit: please use "match" or "same" consistently in the naming. My 
preference was for "match" to be used.

Thanks,
John

> +{
> +	return  SAS_ADDR(p1->attached_sas_addr) == SAS_ADDR(p2->attached_sas_addr);
> +}
> +

Re: [PATCH v2 5/8] scsi: libsas: introduce sas address comparation helpers

[Jason Yan]

On 2022/9/26 19:44, John Garry wrote:
> On 24/09/2022 08:34, Jason Yan wrote:
>> +static inline bool sas_phy_match_dev_addr(struct domain_device *dev,
>> +                     struct ex_phy *phy)
>> +{
>> +    return SAS_ADDR(dev->sas_addr) == SAS_ADDR(phy->attached_sas_addr);
>> +}
>> +
>> +static inline bool sas_phy_match_port_addr(struct asd_sas_port *port,
>> +                       struct ex_phy *phy)
>> +{
>> +    return SAS_ADDR(port->sas_addr) == SAS_ADDR(phy->attached_sas_addr);
>> +}
>> +
>> +static inline bool sas_phy_addr_same(struct ex_phy *p1, struct ex_phy 
>> *p2)
> 
> nit: please use "match" or "same" consistently in the naming. My 
> preference was for "match" to be used.
> 

OK.

Thanks,
Jason

> Thanks,
> John
> 
>> +{
>> +    return  SAS_ADDR(p1->attached_sas_addr) == 
>> SAS_ADDR(p2->attached_sas_addr);
>> +}
>> +
> 
> .

[PATCH v2 6/8] scsi: libsas: use sas_phy_match_dev_addr() instead of open coded

[Jason Yan]

The sas address comparation of domain device and expander phy is open
coded. Now we can replace it with sas_phy_match_dev_addr().

Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/scsi/libsas/sas_expander.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index a5effff3d1c3..b2b5103c3e76 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -738,9 +738,7 @@ static void sas_ex_get_linkrate(struct domain_device *parent,
 		    phy->phy_state == PHY_NOT_PRESENT)
 			continue;
 
-		if (SAS_ADDR(phy->attached_sas_addr) ==
-		    SAS_ADDR(child->sas_addr)) {
-
+		if (sas_phy_match_dev_addr(child, phy)) {
 			child->min_linkrate = min(parent->min_linkrate,
 						  phy->linkrate);
 			child->max_linkrate = max(parent->max_linkrate,
@@ -1012,8 +1010,7 @@ static int sas_ex_discover_dev(struct domain_device *dev, int phy_id)
 		sas_add_parent_port(dev, phy_id);
 		return 0;
 	}
-	if (dev->parent && (SAS_ADDR(ex_phy->attached_sas_addr) ==
-			    SAS_ADDR(dev->parent->sas_addr))) {
+	if (dev->parent && sas_phy_match_dev_addr(dev->parent, ex_phy)) {
 		sas_add_parent_port(dev, phy_id);
 		if (ex_phy->routing_attr == TABLE_ROUTING)
 			sas_configure_phy(dev, phy_id, dev->port->sas_addr, 1);
@@ -1312,7 +1309,7 @@ static int sas_check_parent_topology(struct domain_device *child)
 		    parent_phy->phy_state == PHY_NOT_PRESENT)
 			continue;
 
-		if (SAS_ADDR(parent_phy->attached_sas_addr) != SAS_ADDR(child->sas_addr))
+		if (sas_phy_match_dev_addr(child, parent_phy))
 			continue;
 
 		child_phy = &child_ex->ex_phy[parent_phy->attached_phy_id];
@@ -1522,8 +1519,7 @@ static int sas_configure_parent(struct domain_device *parent,
 		struct ex_phy *phy = &ex_parent->ex_phy[i];
 
 		if ((phy->routing_attr == TABLE_ROUTING) &&
-		    (SAS_ADDR(phy->attached_sas_addr) ==
-		     SAS_ADDR(child->sas_addr))) {
+		    sas_phy_match_dev_addr(child, phy)) {
 			res = sas_configure_phy(parent, i, sas_addr, include);
 			if (res)
 				return res;
@@ -1858,8 +1854,7 @@ static void sas_unregister_devs_sas_addr(struct domain_device *parent,
 	if (last) {
 		list_for_each_entry_safe(child, n,
 			&ex_dev->children, siblings) {
-			if (SAS_ADDR(child->sas_addr) ==
-			    SAS_ADDR(phy->attached_sas_addr)) {
+			if (sas_phy_match_dev_addr(child, phy)) {
 				set_bit(SAS_DEV_GONE, &child->state);
 				if (dev_is_expander(child->dev_type))
 					sas_unregister_ex_tree(parent->port, child);
@@ -1941,8 +1936,7 @@ static int sas_discover_new(struct domain_device *dev, int phy_id)
 	if (res)
 		return res;
 	list_for_each_entry(child, &dev->ex_dev.children, siblings) {
-		if (SAS_ADDR(child->sas_addr) ==
-		    SAS_ADDR(ex_phy->attached_sas_addr)) {
+		if (sas_phy_match_dev_addr(child, ex_phy)) {
 			if (dev_is_expander(child->dev_type))
 				res = sas_discover_bfs_by_root(child);
 			break;
@@ -2115,8 +2109,7 @@ int sas_find_attached_phy(struct expander_device *ex_dev,
 
 	for (phy_id = 0; phy_id < ex_dev->num_phys; phy_id++) {
 		phy = &ex_dev->ex_phy[phy_id];
-		if (SAS_ADDR(phy->attached_sas_addr)
-			== SAS_ADDR(dev->sas_addr))
+		if (sas_phy_match_dev_addr(dev, phy))
 			return phy_id;
 	}
 
-- 
2.31.1

Re: [PATCH v2 6/8] scsi: libsas: use sas_phy_match_dev_addr() instead of open coded

[John Garry]

On 24/09/2022 08:34, Jason Yan wrote:
> @@ -2115,8 +2109,7 @@ int sas_find_attached_phy(struct expander_device *ex_dev,
>   
>   	for (phy_id = 0; phy_id < ex_dev->num_phys; phy_id++) {
>   		phy = &ex_dev->ex_phy[phy_id];
> -		if (SAS_ADDR(phy->attached_sas_addr)
> -			== SAS_ADDR(dev->sas_addr))
> +		if (sas_phy_match_dev_addr(dev, phy))

It would be nice if the series was arranged such that 
sas_phy_match_dev_addr() is available when you introduce 
sas_phy_match_dev_addr()

>   			return phy_id;
>   	}

Thanks,
John

Re: [PATCH v2 6/8] scsi: libsas: use sas_phy_match_dev_addr() instead of open coded

[Jason Yan]

On 2022/9/26 19:43, John Garry wrote:
> On 24/09/2022 08:34, Jason Yan wrote:
>> @@ -2115,8 +2109,7 @@ int sas_find_attached_phy(struct expander_device 
>> *ex_dev,
>>       for (phy_id = 0; phy_id < ex_dev->num_phys; phy_id++) {
>>           phy = &ex_dev->ex_phy[phy_id];
>> -        if (SAS_ADDR(phy->attached_sas_addr)
>> -            == SAS_ADDR(dev->sas_addr))
>> +        if (sas_phy_match_dev_addr(dev, phy))
> 
> It would be nice if the series was arranged such that 
> sas_phy_match_dev_addr() is available when you introduce 
> sas_phy_match_dev_addr()


I assume you mean sas_phy_match_dev_addr() should available before 
introducing sas_find_attached_phy().

Yes, I can make that change.

Thanks,
Jason

> 
>>               return phy_id;
>>       }
> 
> Thanks,
> John
> .

[PATCH v2 7/8] scsi: libsas: use sas_phy_addr_same() instead of open coded

[Jason Yan]

The sas address comparation of expander phys is open coded. Now we can
replace it with sas_phy_addr_same().

Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/scsi/libsas/sas_expander.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index b2b5103c3e76..c9d738ad8176 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2058,8 +2058,7 @@ static int sas_rediscover(struct domain_device *dev, const int phy_id)
 
 			if (i == phy_id)
 				continue;
-			if (SAS_ADDR(phy->attached_sas_addr) ==
-			    SAS_ADDR(changed_phy->attached_sas_addr)) {
+			if (sas_phy_addr_same(phy, changed_phy)) {
 				last = false;
 				break;
 			}
-- 
2.31.1

[PATCH v2 8/8] scsi: libsas: use sas_phy_match_port_addr() instead of open coded

[Jason Yan]

The sas address comparation of asd_sas_port and expander phy is open
coded. Now we can replace it with sas_phy_match_port_addr().

Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/scsi/libsas/sas_expander.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index c9d738ad8176..129e5e365f4e 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1005,8 +1005,7 @@ static int sas_ex_discover_dev(struct domain_device *dev, int phy_id)
 	}
 
 	/* Parent and domain coherency */
-	if (!dev->parent && (SAS_ADDR(ex_phy->attached_sas_addr) ==
-			     SAS_ADDR(dev->port->sas_addr))) {
+	if (!dev->parent && sas_phy_match_port_addr(dev->port, ex_phy)) {
 		sas_add_parent_port(dev, phy_id);
 		return 0;
 	}
-- 
2.31.1

Re: [PATCH v2 0/8] scsi: libsas: sas address comparation refactor

[Jinpu Wang]

On Sat, Sep 24, 2022 at 9:24 AM Jason Yan <yanaijie@huawei.com> wrote:
>
> Sas address conversion and comparation is widely used in libsas and
> drivers. However they are all opencoded and to avoid the line spill over
> 80 columns, are mostly split into multi-lines.
>
> To make the code easier to read, introduce some helpers with clearer
> semantics and replace the opencoded segments with them.
>
> v1->v2:
>   First factor out sas_find_attached_phy() and replace LLDDs's code
>         with it.
>   Remove three too simple helpers.
>   Rename the helpers with 'sas_' prefix.
>
Hi Jason,

Thx for doing this.
> Jason Yan (8):
>   scsi: libsas: introduce sas_find_attached_phy() helper
>   scsi: pm8001: use sas_find_attached_phy() instead of open coded
>   scsi: mvsas: use sas_find_attached_phy() instead of open coded
>   scsi: hisi_sas: use sas_find_attathed_phy() instead of open coded
These 4 look good to me.
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
>   scsi: libsas: introduce sas address comparation helpers
>   scsi: libsas: use sas_phy_match_dev_addr() instead of open coded
>   scsi: libsas: use sas_phy_addr_same() instead of open coded
>   scsi: libsas: use sas_phy_match_port_addr() instead of open coded
These helpers are too simple to replace, we add more loc in the end.
>
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 12 ++------
>  drivers/scsi/libsas/sas_expander.c    | 40 ++++++++++++++++-----------
>  drivers/scsi/libsas/sas_internal.h    | 17 ++++++++++++
>  drivers/scsi/mvsas/mv_sas.c           | 15 +++-------
>  drivers/scsi/pm8001/pm8001_sas.c      | 16 ++++-------
>  include/scsi/libsas.h                 |  2 ++
>  6 files changed, 54 insertions(+), 48 deletions(-)
>
> --
> 2.31.1
>

Re: [PATCH v2 0/8] scsi: libsas: sas address comparation refactor

[Jason Yan]

On 2022/9/26 14:09, Jinpu Wang wrote:
> On Sat, Sep 24, 2022 at 9:24 AM Jason Yan <yanaijie@huawei.com> wrote:
>>
>> Sas address conversion and comparation is widely used in libsas and
>> drivers. However they are all opencoded and to avoid the line spill over
>> 80 columns, are mostly split into multi-lines.
>>
>> To make the code easier to read, introduce some helpers with clearer
>> semantics and replace the opencoded segments with them.
>>
>> v1->v2:
>>    First factor out sas_find_attached_phy() and replace LLDDs's code
>>          with it.
>>    Remove three too simple helpers.
>>    Rename the helpers with 'sas_' prefix.
>>
> Hi Jason,
> 
> Thx for doing this.
>> Jason Yan (8):
>>    scsi: libsas: introduce sas_find_attached_phy() helper
>>    scsi: pm8001: use sas_find_attached_phy() instead of open coded
>>    scsi: mvsas: use sas_find_attached_phy() instead of open coded
>>    scsi: hisi_sas: use sas_find_attathed_phy() instead of open coded
> These 4 look good to me.
> Reviewed-by: Jack Wang <jinpu.wang@ionos.com>

Hi Jack,
Thank you very much for the review.

>>    scsi: libsas: introduce sas address comparation helpers
>>    scsi: libsas: use sas_phy_match_dev_addr() instead of open coded
>>    scsi: libsas: use sas_phy_addr_same() instead of open coded
>>    scsi: libsas: use sas_phy_match_port_addr() instead of open coded
> These helpers are too simple to replace, we add more loc in the end.

The initial purpose to introduce these helpers is to stop cutting 
compare expressions into two lines and to make the code looks clean. We 
add more loc in the end because of function declaration and more blank 
lines between them.

Thanks,
Jason

>>
>>   drivers/scsi/hisi_sas/hisi_sas_main.c | 12 ++------
>>   drivers/scsi/libsas/sas_expander.c    | 40 ++++++++++++++++-----------
>>   drivers/scsi/libsas/sas_internal.h    | 17 ++++++++++++
>>   drivers/scsi/mvsas/mv_sas.c           | 15 +++-------
>>   drivers/scsi/pm8001/pm8001_sas.c      | 16 ++++-------
>>   include/scsi/libsas.h                 |  2 ++
>>   6 files changed, 54 insertions(+), 48 deletions(-)
>>
>> --
>> 2.31.1
>>
> .
>