On Tue, 2017-05-23 at 06:19 -0700, Dupuis, Chad wrote:
> Expose this information for interested applications.
> 
> Signed-off-by: Chad Dupuis <chad.dup...@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.

Reply via email to