diff mbox series

[1/7] scsi: libsas: introduce sas address conversion and comparation helpers

Message ID 20220917104311.1878250-2-yanaijie@huawei.com
State New
Headers show
Series scsi: libsas: sas address comparation refactor | expand

Commit Message

Jason Yan Sept. 17, 2022, 10:43 a.m. UTC
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. Introduce some helpers to
prepare some refactor.

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

Comments

John Garry Sept. 22, 2022, 2:14 p.m. UTC | #1
On 17/09/2022 11:43, Jason Yan 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. Introduce some helpers to
> prepare some refactor.
> 
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> ---
>   include/scsi/libsas.h | 32 ++++++++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)
> 
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index 2dbead74a2af..382aedf31fa4 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -648,6 +648,38 @@ static inline bool sas_is_internal_abort(struct sas_task *task)
>   	return task->task_proto == SAS_PROTOCOL_INTERNAL_ABORT;
>   }
>   
> +static inline unsigned long long ex_phy_addr(struct ex_phy *phy)

This is a public header, so I would hope that any function would have 
"sas_" prefix

> +{
> +	return SAS_ADDR(phy->attached_sas_addr);
> +}
> +
> +static inline unsigned long long dev_addr(struct domain_device *dev)
> +{
> +	return SAS_ADDR(dev->sas_addr);
> +}
> +
> +static inline unsigned long long port_addr(struct asd_sas_port *port)
> +{
> +	return SAS_ADDR(port->sas_addr);

As below, I don't really see how these simple functions help much

> +}
> +
> +static inline bool dev_and_phy_addr_same(struct domain_device *dev,
> +					 struct ex_phy *phy)
> +{
> +	return dev_addr(dev) == ex_phy_addr(phy);
> +}
> +
> +static inline bool port_and_phy_addr_same(struct asd_sas_port *port,
> +					  struct ex_phy *phy)

I'd say sas_phy_match_port_addr() could be a better name.

> +{
> +	return port_addr(port) == ex_phy_addr(phy);

I think the following is just as good:

	return SAS_ADDR(port->sas_addr) == SAS_ADDR(phy->attached_sas_addr)

port_addr() is only used once AFAICS, so the code would not be less concise

> +}
> +
> +static inline bool ex_phy_addr_same(struct ex_phy *phy1, struct ex_phy *phy2)
> +{
> +	return  ex_phy_addr(phy1) ==  ex_phy_addr(phy2);

nit: 2x double whitespace

> +}
> +
>   struct sas_domain_function_template {
>   	/* The class calls these to notify the LLDD of an event. */
>   	void (*lldd_port_formed)(struct asd_sas_phy *);

Thanks,
John
diff mbox series

Patch

diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 2dbead74a2af..382aedf31fa4 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -648,6 +648,38 @@  static inline bool sas_is_internal_abort(struct sas_task *task)
 	return task->task_proto == SAS_PROTOCOL_INTERNAL_ABORT;
 }
 
+static inline unsigned long long ex_phy_addr(struct ex_phy *phy)
+{
+	return SAS_ADDR(phy->attached_sas_addr);
+}
+
+static inline unsigned long long dev_addr(struct domain_device *dev)
+{
+	return SAS_ADDR(dev->sas_addr);
+}
+
+static inline unsigned long long port_addr(struct asd_sas_port *port)
+{
+	return SAS_ADDR(port->sas_addr);
+}
+
+static inline bool dev_and_phy_addr_same(struct domain_device *dev,
+					 struct ex_phy *phy)
+{
+	return dev_addr(dev) == ex_phy_addr(phy);
+}
+
+static inline bool port_and_phy_addr_same(struct asd_sas_port *port,
+					  struct ex_phy *phy)
+{
+	return port_addr(port) == ex_phy_addr(phy);
+}
+
+static inline bool ex_phy_addr_same(struct ex_phy *phy1, struct ex_phy *phy2)
+{
+	return  ex_phy_addr(phy1) ==  ex_phy_addr(phy2);
+}
+
 struct sas_domain_function_template {
 	/* The class calls these to notify the LLDD of an event. */
 	void (*lldd_port_formed)(struct asd_sas_phy *);