diff mbox

[06/15] qedf: Add fka_period SCSI host attribute to show fip keep alive period.

Message ID 20170523131931.1777-7-chad.dupuis@cavium.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Dupuis, Chad May 23, 2017, 1:19 p.m. UTC 
Expose this information for interested applications.

Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
---
 drivers/scsi/qedf/qedf_attr.c | 60 +++++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 19 deletions(-)

Comments

Bart Van Assche May 24, 2017, 4:34 p.m. UTC | #1 
On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> Expose this information for interested applications.
> 
> Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
> ---
>  drivers/scsi/qedf/qedf_attr.c | 60 +++++++++++++++++++++++++++++--------------
>  1 file changed, 41 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/scsi/qedf/qedf_attr.c b/drivers/scsi/qedf/qedf_attr.c
> index 1349f8a..68e2b77 100644
> --- a/drivers/scsi/qedf/qedf_attr.c
> +++ b/drivers/scsi/qedf/qedf_attr.c
> @@ -8,6 +8,25 @@
>   */
>  #include "qedf.h"
>  
> +inline bool qedf_is_vport(struct qedf_ctx *qedf)
> +{
> +	return (!(qedf->lport->vport == NULL));
> +}

Have you considered to write the return statement as follows?

	return qedf->lport->vport != NULL;

Checkpatch should have recommended not to use parentheses in the return
statement.

> +
> +/* Get base qedf for physical port from vport */
> +static struct qedf_ctx *qedf_get_base_qedf(struct qedf_ctx *qedf)
> +{
> +	struct fc_lport *lport;
> +	struct fc_lport *base_lport;
> +
> +	if (!(qedf_is_vport(qedf)))
> +		return NULL;
> +
> +	lport = qedf->lport;
> +	base_lport = shost_priv(vport_to_shost(lport->vport));
> +	return (struct qedf_ctx *)(lport_priv(base_lport));
> +}

lport_priv() returns a void pointer so the cast in the return statement is not
necessary.

> +static ssize_t
> +qedf_fka_period_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct fc_lport *lport = shost_priv(class_to_shost(dev));
> +	struct qedf_ctx *qedf = lport_priv(lport);
> +	int fka_period = -1;
> +
> +	if (qedf_is_vport(qedf))
> +		qedf = qedf_get_base_qedf(qedf);
> +
> +	if (!qedf->ctlr.sel_fcf)
> +		goto out;
> +
> +	fka_period = qedf->ctlr.sel_fcf->fka_period;
> +
> +out:
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", fka_period);
> +}

Do we really need a goto statement to skip a single statement? How about the
following:

	if (qedf->ctlr.sel_fcf)
		fka_period = qedf->ctlr.sel_fcf->fka_period;

	return scnprintf(buf, PAGE_SIZE, "%d\n", fka_period);

or this:

	return scnprintf(buf, PAGE_SIZE, "%d\n", qedf->ctlr.sel_fcf ?
                         qedf->ctlr.sel_fcf->fka_period : -1);

Bart.
diff mbox

Patch

diff --git a/drivers/scsi/qedf/qedf_attr.c b/drivers/scsi/qedf/qedf_attr.c
index 1349f8a..68e2b77 100644
--- a/drivers/scsi/qedf/qedf_attr.c
+++ b/drivers/scsi/qedf/qedf_attr.c
@@ -8,6 +8,25 @@ 
  */
 #include "qedf.h"
 
+inline bool qedf_is_vport(struct qedf_ctx *qedf)
+{
+	return (!(qedf->lport->vport == NULL));
+}
+
+/* Get base qedf for physical port from vport */
+static struct qedf_ctx *qedf_get_base_qedf(struct qedf_ctx *qedf)
+{
+	struct fc_lport *lport;
+	struct fc_lport *base_lport;
+
+	if (!(qedf_is_vport(qedf)))
+		return NULL;
+
+	lport = qedf->lport;
+	base_lport = shost_priv(vport_to_shost(lport->vport));
+	return (struct qedf_ctx *)(lport_priv(base_lport));
+}
+
 static ssize_t
 qedf_fcoe_mac_show(struct device *dev,
 	struct device_attribute *attr, char *buf)
@@ -26,34 +45,37 @@ 
 	return scnprintf(buf, PAGE_SIZE, "%pM\n", fcoe_mac);
 }
 
+static ssize_t
+qedf_fka_period_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct fc_lport *lport = shost_priv(class_to_shost(dev));
+	struct qedf_ctx *qedf = lport_priv(lport);
+	int fka_period = -1;
+
+	if (qedf_is_vport(qedf))
+		qedf = qedf_get_base_qedf(qedf);
+
+	if (!qedf->ctlr.sel_fcf)
+		goto out;
+
+	fka_period = qedf->ctlr.sel_fcf->fka_period;
+
+out:
+	return scnprintf(buf, PAGE_SIZE, "%d\n", fka_period);
+}
+
 static DEVICE_ATTR(fcoe_mac, S_IRUGO, qedf_fcoe_mac_show, NULL);
+static DEVICE_ATTR(fka_period, S_IRUGO, qedf_fka_period_show, NULL);
 
 struct device_attribute *qedf_host_attrs[] = {
 	&dev_attr_fcoe_mac,
+	&dev_attr_fka_period,
 	NULL,
 };
 
 extern const struct qed_fcoe_ops *qed_ops;
 
-inline bool qedf_is_vport(struct qedf_ctx *qedf)
-{
-	return (!(qedf->lport->vport == NULL));
-}
-
-/* Get base qedf for physical port from vport */
-static struct qedf_ctx *qedf_get_base_qedf(struct qedf_ctx *qedf)
-{
-	struct fc_lport *lport;
-	struct fc_lport *base_lport;
-
-	if (!(qedf_is_vport(qedf)))
-		return NULL;
-
-	lport = qedf->lport;
-	base_lport = shost_priv(vport_to_shost(lport->vport));
-	return (struct qedf_ctx *)(lport_priv(base_lport));
-}
-
 void qedf_capture_grc_dump(struct qedf_ctx *qedf)
 {
 	struct qedf_ctx *base_qedf;