> On Jun 19, 2017, at 2:22 PM, James Smart <james.sm...@broadcom.com> wrote:
> 
> On 6/16/2017 3:47 PM, Himanshu Madhani wrote:
>> From: Duane Grigsby <duane.grig...@cavium.com>
>> 
>> This code provides the interfaces to register remote and local ports
>> of FC4 type 0x28 with the FC-NVMe transport and transports the
>> requests (FC-NVMe FC link services and FC-NVMe commands IUs) to the
>> fabric. It also provides the support for allocating h/w queues and
>> aborting FC-NVMe FC requests.
>> 
>> Signed-off-by: Darren Trapp <darren.tr...@cavium.com>
>> Signed-off-by: Duane Grigsby <duane.grig...@cavium.com>
>> Signed-off-by: Anil Gurumurthy <anil.gurumur...@cavium.com>
>> Signed-off-by: Giridhar Malavali <giridhar.malav...@cavium.com>
>> Signed-off-by: Himanshu Madhani <himanshu.madh...@cavium.com>
>> 
>> +#if (IS_ENABLED(CONFIG_NVME_FC))
>> +extern int nvme_fc_register_remoteport(struct nvme_fc_local_port *,
>> +    struct nvme_fc_port_info *, struct nvme_fc_remote_port **);
>> +extern int nvme_fc_register_localport(struct nvme_fc_port_info *,
>> +    struct nvme_fc_port_template *, struct device *,
>> +    struct nvme_fc_local_port **);
>> +extern int nvme_fc_unregister_localport(struct nvme_fc_local_port *);
>> +#endif
> 
> There should be no need for these. They are in the 
> include/linux/nvme-fc-driver.h header.
> 

Agree. Will remove it 

> 
>> +static void qla_nvme_sp_ls_done(void *ptr, int res)
>> +{
>> +    srb_t *sp = ptr;
>> +    struct srb_iocb *nvme;
>> +    struct nvmefc_ls_req   *fd;
>> +    struct nvme_private *priv;
>> +
>> +    if (atomic_read(&sp->ref_count) == 0) {
>> +            ql_log(ql_log_warn, sp->fcport->vha, 0x2123,
>> +                "SP reference-count to ZERO on LS_done -- sp=%p.\n", sp);
>> +            return;
>> +    }
>> +
>> +    if (!atomic_dec_and_test(&sp->ref_count))
>> +            return;
>> +
>> +    if (res)
>> +            res = NVME_SC_FC_TRANSPORT_ERROR;
> 
> LS request failures status should be -Exxx values, not nvme status codes 
> (which don't apply to LS's).
> The api probably wasn't super clear on this.
> 

Will fix up in v2.

>> +
>> +    nvme = &sp->u.iocb_cmd;
>> +    fd = nvme->u.nvme.desc;
>> +    priv = fd->private;
>> +    priv->comp_status = res;
>> +    schedule_work(&priv->ls_work);
>> +    /* work schedule doesn't need the sp */
>> +    qla2x00_rel_sp(sp);
>> +}
>> +
>> +static void qla_nvme_sp_done(void *ptr, int res)
>> +{
>> +    srb_t *sp = ptr;
>> +    struct srb_iocb *nvme;
>> +    struct nvmefc_fcp_req *fd;
>> +
>> +    nvme = &sp->u.iocb_cmd;
>> +    fd = nvme->u.nvme.desc;
>> +
>> +    if (!atomic_dec_and_test(&sp->ref_count))
>> +            return;
>> +
>> +    if (!(sp->fcport->nvme_flag & NVME_FLAG_REGISTERED))
>> +            goto rel;
>> +
>> +    if (unlikely((nvme->u.nvme.comp_status) || res))
>> +            fd->status = NVME_SC_FC_TRANSPORT_ABORTED;
>> +    else
>> +            fd->status = 0;
> 
> FCP req failures status should be -Exxx values, not nvme status codes. This 
> is clear in the api header.
> 

Sure. Will fix this in v2. 

> -- james
> 



Reply via email to