Re: [PATCH v2 3/6] qla2xxx: Add FC-NVMe F/W initialization and transport registration

[Madhani, Himanshu]

Hi Johannes, 

> On Jun 22, 2017, at 2:46 AM, Johannes Thumshirn  wrote:
> 
> On Wed, Jun 21, 2017 at 01:48:43PM -0700, Madhani, Himanshu wrote:
> [...]
>> +wait_queue_head_t nvme_ls_waitQ;
> 
> Can you please lower-case the 'Q' in waitQ IFF you have to re-send the series?

sure.

> 
> [...]
>> +wait_queue_head_t nvme_waitQ;
> 
> Ditto
> 
Ack

> [...]
>> +wait_queue_head_t nvme_waitQ;
> 
> And here as well.

Ack

> 
>> diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
>> index 6fbee11c1a18..c6af45f7d5d6 100644
>> --- a/drivers/scsi/qla2xxx/qla_gbl.h
>> +++ b/drivers/scsi/qla2xxx/qla_gbl.h
>> @@ -10,6 +10,16 @@
>> #include 
>> 
>> /*
>> + * Global functions prototype in qla_nvme.c source file.
>> + */
>> +extern void qla_nvme_register_hba(scsi_qla_host_t *);
>> +extern int  qla_nvme_register_remote(scsi_qla_host_t *, fc_port_t *);
>> +extern void qla_nvme_delete(scsi_qla_host_t *);
>> +extern void qla_nvme_abort(struct qla_hw_data *, srb_t *sp);
>> +extern void qla24xx_nvme_ls4_iocb(scsi_qla_host_t *, struct pt_ls4_request 
>> *,
>> +struct req_que *);
> 
> You're still not convinced of the idea of headers, heh ;-)
> Especially as you have a qla_nvme.h.
> 
> […]
> 

if this is not *must* i’ll like to post patch for this with other patches that 
I am going to queue up during rc1 phase. 

>> +INIT_WORK(>nvme_del_work, qla_nvme_unregister_remote_port);
>> +rport = kzalloc(sizeof(*rport), GFP_KERNEL);
>> +if (!rport) {
>> +ql_log(ql_log_warn, vha, 0x2101,
>> +"%s: unable to alloc memory\n", __func__);
> 
> kzalloc() will warn you about a failed allocation, no need to double it.
> See also:
> http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
> 
> […]

Ack. 

>> +ret = nvme_fc_register_remoteport(vha->nvme_local_port, >req,
>> +>nvme_remote_port);
>> +if (ret) {
>> +ql_log(ql_log_warn, vha, 0x212e,
>> +"Failed to register remote port. Transport returned %d\n",
>> +ret);
>> +return ret;
>> +}
>> +
>> +fcport->nvme_remote_port->private = fcport;
> 
> I think I already said that in the last review, but can you please move the 
> fcport->nvme_remote_port->private = fcport;
> assingment _above_ the nvme_fc_register_remoteport() call.
> 

I saw your response to James that this is okay for FC NVMe code.

> [...]
> 
>> +vha = (struct scsi_qla_host *)lport->private;
> 
> No need to cast from void *
>> +ql_log(ql_log_info, vha, 0x2104,
>> +"%s: handle %p, idx =%d, qsize %d\n",
>> +__func__, handle, qidx, qsize);
> 
> Btw, sometime in the future you could change your ql_log() thingies to the
> kernel's dyndebug facility.
> 
> […]

Thanks for the suggestions. I’ll bring it up to team and we can slowly convert 
these to kernel’s
dynamic debugging facility. 


>> +rval = ha->isp_ops->abort_command(sp);
>> +if (rval != QLA_SUCCESS)
>> +ql_log(ql_log_warn, fcport->vha, 0x2125,
>> +"%s: failed to abort LS command for SP:%p rval=%x\n",
>> +__func__, sp, rval);
>> +
>> +ql_dbg(ql_dbg_io, fcport->vha, 0x212b,
>> +"%s: aborted sp:%p on fcport:%p\n", __func__, sp, fcport);
> 
> If you insinst in having these two messages ("failed to abort" and "aborted")
> can you at least fold it into one print statement.
> 

I’ll send follow up patch for this cleanup, if its okay with you? 

>> +rval = ha->isp_ops->abort_command(sp);
>> +if (!rval)
>> +ql_log(ql_log_warn, fcport->vha, 0x2127,
>> +"%s: failed to abort command for SP:%p rval=%x\n",
>> +__func__, sp, rval);
>> +
>> +ql_dbg(ql_dbg_io, fcport->vha, 0x2126,
>> +"%s: aborted sp:%p on fcport:%p\n", __func__, sp, fcport);
> 
> Ditto.
> 

Agree. Will fold this into cleanup patch. 

> [...]
> 
> 
>> +/* Setup qpair pointers */
>> +req = qpair->req;
>> +tot_dsds = fd->sg_cnt;
>> +
>> +/* Acquire qpair specific lock */
>> +spin_lock_irqsave(>qp_lock, flags);
>> +
>> +/* Check for room in outstanding command list. */
>> +handle = req->current_outstanding_cmd;
> 
> I've just seen this in qla2xxx_start_scsi_mq() and
> qla2xxx_dif_start_scsi_mq() and was about to send you an RFC patch. But
> here it is for completeness in the nvme version as well:
> 
> You save a pointer to the req_que from you qpair and _afterwards_ you grab
> the qp_lock. What prevents someone from changing the request internals
> underneath you?
> 
> Like this:
> 
> CPU0   CPU1
> req = qpair->req;
> qla2xxx_delete_qpair(vha, qpair);
> `-> ret = qla25xx_delete_req_que(vha, 
> qpair->req);
> spin_lock_irqsave(>qp_lock, flags);
> handle = req->current_outstanding_cmd;
> 
> Oh and btw, neither qla2xxx_delete_qpair() nor 

Re: [PATCH v2 2/5] dt-bindings: scsi: ufs: add document for hi3660-ufs

[Bu Tao]

在 2017/6/23 9:05, Subhash Jadavani 写道:

On 2017-06-22 04:51, Arnd Bergmann wrote:

On Thu, Jun 22, 2017 at 1:44 PM, Bu Tao  wrote:

在 2017/6/17 5:51, Arnd Bergmann 写道:

On Fri, Jun 16, 2017 at 8:51 AM, Bu Tao  wrote:

+Optional properties for board device:
+- ufs-hi3660-use-rate-B: specifies UFS rate-B
+- ufs-hi3660-broken-fastauto   : specifies no fastauto
+- ufs-hi3660-use-HS-GEAR3  : specifies UFS HS-GEAR3
+- ufs-hi3660-use-HS-GEAR2  : specifies UFS HS-GEAR2
+- ufs-hi3660-use-HS-GEAR1  : specifies UFS HS-GEAR1
+- ufs-hi3660-broken-clk-gate-bypass: specifies no clk-gate
+- ufs-hi3660-use-one-line  : specifies UFS use one line work
+- reset-gpio   : specifies to reset devices



Some of these sound rather generic and might apply to UFS
implementations
other than hi3660, so I'd suggest adding them to the base ufs
binding with
a generic name instead.

Any DT properties that might be useful across multiple implementations
should be parsed in generic code that gets called by the individual
drivers,
and then the properties that are specific to the integration work
done by
hisilicon should be prefixed with "hisilicon,", but not normally
with the
SoC name: it is quite possible that another SoC will be derived from
this
chip and it should reuse the properties.



I do not know wheher other SoC need to use the optional properties as
abover. So here the name of the optional properties has "hi3660".


They should not have "hi3660" in their names either way, independent
of where they are used.



Yes, i agree with Arnd that SoCs might also need these so please make
these properties generic (put them under
Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt) and also move
their parsing code in generic driver (ufshcd.c or ufshcd-pltfrm.c).


Thanks for your comments. I will modify this and update soon.



(note: this is different from the value of the "compatible" property
that
is meant to be as specific as possible".

Also, please clarify how your binding relates to the ufshcd binding
in Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt: does
hi3660 implement any registers that are shared with ufshcd, or does
it use the same physical interface with a different register set?


No, only show how to use the dt-binding for hi3660 SoC


My question was about the hardware: does hi3660 implement ufshcd
or not?

  Arnd

Re: [PATCH 0/4] g_NCR5380: PDMA fixes and cleanup

[Finn Thain]

On Thu, 22 Jun 2017, Ondrej Zary wrote:

> Works only with HDD on non-DTC chips. CD-ROM hangs. DTC hangs even with 
> HDD. The PDMA code really needs to be fixed.
> 

Does this patch help? It should be applied on top of this series of 4.

diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index 4c31cb316a38..95ae8edbecbc 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -481,6 +481,30 @@ static void generic_NCR5380_release_resources(struct 
Scsi_Host *instance)
release_mem_region(base, region_size);
 }
 
+/* wait_for_53c80_access - wait for 53C80 registers to become accessible
+ * @hostdata: scsi host private data
+ *
+ * The registers within the 53C80 logic block are inaccessible until
+ * bit 7 in the 53C400 control status register gets asserted.
+ */
+
+static int wait_for_53c80_access(struct NCR5380_hostdata *hostdata)
+{
+   int count = 1;
+
+   do {
+   udelay(4); /* DTC436 chip hangs without this */
+   if (NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)
+   return 0;
+   } while (--count > 0);
+
+   scmd_printk(KERN_ERR, hostdata->connected,
+   "53c80 registers not accessible, device will be reset\n");
+   NCR5380_write(hostdata->c400_ctl_status, CSR_RESET);
+   NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
+   return -1;
+}
+
 /**
  * generic_NCR5380_pread - pseudo DMA receive
  * @hostdata: scsi host private data
@@ -493,33 +517,19 @@ static void generic_NCR5380_release_resources(struct 
Scsi_Host *instance)
 static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata,
 unsigned char *dst, int len)
 {
-   int start, retries;
-   u8 csr, basr;
+   int result;
+   int start;
 
NCR5380_write(hostdata->c400_ctl_status, CSR_BASE | CSR_TRANS_DIR);
NCR5380_write(hostdata->c400_blk_cnt, len / 128);
 
for (start = 0; start < len; start += 128) {
-   retries = 1;
-   while (1) { /* monitor IRQ while waiting for host buffer */
-   csr = NCR5380_read(hostdata->c400_ctl_status);
-   if (!(csr & CSR_HOST_BUF_NOT_RDY))
-   break;
-   if (csr & CSR_GATED_53C80_IRQ) {
-   basr = NCR5380_read(BUS_AND_STATUS_REG);
-   if (!(basr & BASR_PHASE_MATCH) ||
-   (basr & BASR_BUSY_ERROR)) {
-   printk("basr=0x%02x csr=0x%02x at 
start=%d\n", basr, csr, start);
-   goto out_wait;
-   }
-   }
-   if (retries-- < 1) {
-   shost_printk(KERN_ERR, hostdata->host, 
"53C400r: host buffer not ready in time\n");
-   NCR5380_write(hostdata->c400_ctl_status, 
CSR_RESET);
-   NCR5380_write(hostdata->c400_ctl_status, 
CSR_BASE);
-   goto out_wait;
-   }
-   }
+   if (NCR5380_poll_politely2(hostdata, hostdata->c400_ctl_status,
+  CSR_HOST_BUF_NOT_RDY, 0,
+  hostdata->c400_ctl_status,
+  CSR_GATED_53C80_IRQ,
+  CSR_GATED_53C80_IRQ, HZ / 64) < 0)
+   break;
 
if (hostdata->io_port && hostdata->io_width == 2)
insw(hostdata->io_port + hostdata->c400_host_buf,
@@ -532,24 +542,14 @@ static inline int generic_NCR5380_pread(struct 
NCR5380_hostdata *hostdata,
hostdata->io + NCR53C400_host_buffer, 128);
}
 
-out_wait:
-   /* wait for 53C80 registers to be available */
-   retries = 1;
-   while (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)) {
-   if (retries-- < 1) {
-   shost_printk(KERN_ERR, hostdata->host, "53C400r: 53C80 
registers not ready in time\n");
-   NCR5380_write(hostdata->c400_ctl_status, CSR_RESET);
-   NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
-   break;
-   }
-   }
+   result = wait_for_53c80_access(hostdata);
 
if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_END_DMA_TRANSFER))
pr_err("%s: No end dma signal (%d/%d)\n", __func__, start, len);
 
-   hostdata->pdma_residual = len - start;
+   hostdata->pdma_residual = NCR5380_read(hostdata->c400_blk_cnt) * 128;
 
-   return 0;
+   return result;
 }
 
 /**
@@ -564,41 +564,19 @@ static inline int generic_NCR5380_pread(struct 
NCR5380_hostdata *hostdata,
 static inline int 

Re: [PATCH v2 2/5] dt-bindings: scsi: ufs: add document for hi3660-ufs

[Subhash Jadavani]

On 2017-06-22 04:51, Arnd Bergmann wrote:

On Thu, Jun 22, 2017 at 1:44 PM, Bu Tao  wrote:

在 2017/6/17 5:51, Arnd Bergmann 写道:

On Fri, Jun 16, 2017 at 8:51 AM, Bu Tao  wrote:

+Optional properties for board device:
+- ufs-hi3660-use-rate-B: specifies UFS rate-B
+- ufs-hi3660-broken-fastauto   : specifies no fastauto
+- ufs-hi3660-use-HS-GEAR3  : specifies UFS HS-GEAR3
+- ufs-hi3660-use-HS-GEAR2  : specifies UFS HS-GEAR2
+- ufs-hi3660-use-HS-GEAR1  : specifies UFS HS-GEAR1
+- ufs-hi3660-broken-clk-gate-bypass: specifies no clk-gate
+- ufs-hi3660-use-one-line  : specifies UFS use one line work
+- reset-gpio   : specifies to reset devices



Some of these sound rather generic and might apply to UFS 
implementations
other than hi3660, so I'd suggest adding them to the base ufs binding 
with

a generic name instead.

Any DT properties that might be useful across multiple 
implementations

should be parsed in generic code that gets called by the individual
drivers,
and then the properties that are specific to the integration work 
done by
hisilicon should be prefixed with "hisilicon,", but not normally with 
the
SoC name: it is quite possible that another SoC will be derived from 
this

chip and it should reuse the properties.



I do not know wheher other SoC need to use the optional properties as
abover. So here the name of the optional properties has "hi3660".


They should not have "hi3660" in their names either way, independent
of where they are used.



Yes, i agree with Arnd that SoCs might also need these so please make 
these properties generic (put them under 
Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt) and also move 
their parsing code in generic driver (ufshcd.c or ufshcd-pltfrm.c).




(note: this is different from the value of the "compatible" property 
that

is meant to be as specific as possible".

Also, please clarify how your binding relates to the ufshcd binding
in Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt: does
hi3660 implement any registers that are shared with ufshcd, or does
it use the same physical interface with a different register set?


No, only show how to use the dt-binding for hi3660 SoC


My question was about the hardware: does hi3660 implement ufshcd
or not?

  Arnd


--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 0/4] g_NCR5380: PDMA fixes and cleanup

[Ondrej Zary]

On Thursday 15 June 2017 14:17:56 Finn Thain wrote:
> Ondrej, would you please test this patch series? One of your patches
> has been modified slightly and the two I wrote are untested.

Works only with HDD on non-DTC chips. CD-ROM hangs. DTC hangs even with HDD.
The PDMA code really needs to be fixed.

-- 
Ondrej Zary

Re: [PATCH 09/17] cxlflash: Create character device to provide host management interface

[Matthew R. Ochs]

> On Jun 21, 2017, at 9:15 PM, Uma Krishnan  wrote:
> 
> The cxlflash driver currently lacks host management interface. Future
> devices supported by cxlflash will provide a variety of host-wide
> management functions. Examples include LUN provisioning, hardware debug
> support, and firmware download.
> 
> In order to provide a way to manage the device, a character device will
> be created during probe of each adapter. This device will support a set of
> ioctls defined in the SISLite specification from which administrators can
> manage the adapter.
> 
> Signed-off-by: Uma Krishnan 

Acked-by: Matthew R. Ochs 

Re: [PATCH 08/17] cxlflash: Add scsi command abort handler

[Matthew R. Ochs]

> On Jun 21, 2017, at 9:15 PM, Uma Krishnan  wrote:
> 
> To date, CXL flash devices do not support a single command abort operation.
> Instead, the SISLite specification provides a context reset operation to
> cleanup all pending commands for a given context.
> 
> When a context reset is successful, it is guaranteed that the AFU has
> aborted all currently pending I/O. This sequence is less invasive than a
> device or host reset and can be executed to support scsi command abort
> requests. Add eh_abort_handler callback support to process command timeouts
> and abort requests.
> 
> Signed-off-by: Uma Krishnan 

Acked-by: Matthew R. Ochs 

Re: [PATCH 06/17] cxlflash: Track pending scsi commands in each hardware queue

[Matthew R. Ochs]

> On Jun 21, 2017, at 9:14 PM, Uma Krishnan  wrote:
> 
> Currently, there is no book keeping of the pending scsi commands in the
> cxlflash driver. This lack of tracking in-flight requests is too
> restrictive and requires a heavy-hammer reset each time an adapter error is
> encountered. Additionally, it does not allow for commands to be properly
> retried.
> 
> In order to avoid this problem and to better handle error path command
> cleanup, introduce a linked list for each hardware queue that tracks
> pending commands.
> 
> Signed-off-by: Uma Krishnan 

Acked-by: Matthew R. Ochs 

Re: [PATCH 07/17] cxlflash: Flush pending commands in cleanup path

[Matthew R. Ochs]

> On Jun 21, 2017, at 9:14 PM, Uma Krishnan  wrote:
> 
> When the AFU is reset in an error path, pending scsi commands can be
> silently dropped without completion or a formal abort. This puts the onus
> on the cxlflash driver to notify mid-layer and indicating that the command
> can be retried.
> 
> Once the card has been quiesced, the hardware send queue lock is acquired
> to prevent any data movement while the pending commands are processed.
> 
> Signed-off-by: Uma Krishnan 

Acked-by: Matthew R. Ochs 

Re: [PATCH 05/17] cxlflash: Handle AFU sync failures

[Matthew R. Ochs]

> On Jun 21, 2017, at 9:14 PM, Uma Krishnan  wrote:
> 
> AFU sync operations are not currently evaluated for failure. This is
> acceptable for paths where there is not a dependency on the AFU being
> consistent with the host. Examples include link reset events and LUN
> cleanup operations. On paths where there is a dependency, such as a LUN
> open, a sync failure should be acted upon.
> 
> In the event of AFU sync failures, either log or cleanup as appropriate for
> operations that are dependent on a successful sync completion.
> 
> Update documentation to reflect behavior in the event of an AFU sync
> failure.
> 
> Signed-off-by: Uma Krishnan 

Acked-by: Matthew R. Ochs 

Re: [PATCH 03/17] cxlflash: Reset hardware queue context via specified register

[Matthew R. Ochs]

> On Jun 21, 2017, at 9:14 PM, Uma Krishnan  wrote:
> 
> Per the SISLite specification, context_reset() writes 0x1 to the LSB of the
> reset register. When the AFU processes this reset request, it is expected
> to clear the bit after reset is complete. The current implementation simply
> checks that the entire value read back is not 1, instead of masking off the
> LSB and evaluating it for a change to 0. Should the AFU manipulate other
> bits during the reset (reading back a value of 0xF for example), successful
> completion will be prematurely indicated given the existing logic.
> 
> Additionally, in the event that the context reset operation fails, there
> does not currently exist a way to provide feedback to the initiator of the
> reset. This poses a problem for the rare case that a context reset fails as
> the caller will proceed on the assumption that all is well.
> 
> To remedy these issues, refactor the context reset routine to only mask off
> the LSB when evaluating for success and return status to the caller. Also
> update the context reset handler parameters to pass a hardware queue
> reference instead of a single command to better reflect that the entire
> queue associated with the context is impacted by the reset.
> 
> Signed-off-by: Uma Krishnan 

Acked-by: Matthew R. Ochs 

Re: [PATCH 04/17] cxlflash: Schedule asynchronous reset of the host

[Matthew R. Ochs]

> On Jun 21, 2017, at 9:14 PM, Uma Krishnan  wrote:
> 
> A context reset failure indicates the AFU is in a bad state. At present,
> when such a situation occurs, no further action is taken. This leaves the
> adapter in an unusable state with no recoverable actions.
> 
> To avoid this situation, context reset failures will be escalated to a host
> reset operation. This will be done asynchronously to allow the acting
> thread to return to the user with a failure.
> 
> Signed-off-by: Uma Krishnan 

Acked-by: Matthew R. Ochs 

Re: [PATCH 02/17] cxlflash: Update cxlflash_afu_sync() to return errno

[Matthew R. Ochs]

> On Jun 21, 2017, at 9:13 PM, Uma Krishnan  wrote:
> 
> The cxlflash_afu_sync() routine returns a negative one to indicate any kind
> of failure. This makes it impossible to establish why the error occurred.
> 
> Update the return codes to clearly indicate the failure cause to the
> caller.
> 
> Signed-off-by: Uma Krishnan 

Acked-by: Matthew R. Ochs 

Re: [PATCH 01/17] cxlflash: Combine the send queue locks

[Matthew R. Ochs]

> On Jun 21, 2017, at 9:13 PM, Uma Krishnan  wrote:
> 
> Currently there are separate spin locks for the two supported I/O queueing
> models. This makes it difficult to serialize with paths outside the enqueue
> path.
> 
> As a design simplification and to support serialization with enqueue
> operations, move to only a single lock that is used for enqueueing
> regardless of the queueing model.
> 
> Signed-off-by: Uma Krishnan 

Acked-by: Matthew R. Ochs 

[GIT PULL] SCSI fixes for 4.12-rc6

[James Bottomley]

Two fixes to remove spurious WARN_ONs from the new(ish) qedi driver. 
 The driver already prints a warning message, there's no need to panic
users by printing something that looks like an oops as well.

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes

The short changelog is:

Manish Rangankar (2):
  scsi: qedi: Remove WARN_ON from clear task context.
  scsi: qedi: Remove WARN_ON for untracked cleanup.

And the diffstat:

 drivers/scsi/qedi/qedi_fw.c   | 1 -
 drivers/scsi/qedi/qedi_main.c | 4 +---
 2 files changed, 1 insertion(+), 4 deletions(-)


With full diff below.

James

---

diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
index 8bc7ee1a8ca8..507512cc478b 100644
--- a/drivers/scsi/qedi/qedi_fw.c
+++ b/drivers/scsi/qedi/qedi_fw.c
@@ -870,7 +870,6 @@ static void qedi_process_cmd_cleanup_resp(struct qedi_ctx 
*qedi,
QEDI_ERR(>dbg_ctx,
 "Delayed or untracked cleanup response, itt=0x%x, 
tid=0x%x, cid=0x%x, task=%p\n",
 protoitt, cqe->itid, qedi_conn->iscsi_conn_id, task);
-   WARN_ON(1);
}
 }
 
diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index 09a294634bc7..879d3b7462f9 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -1499,11 +1499,9 @@ int qedi_get_task_idx(struct qedi_ctx *qedi)
 
 void qedi_clear_task_idx(struct qedi_ctx *qedi, int idx)
 {
-   if (!test_and_clear_bit(idx, qedi->task_idx_map)) {
+   if (!test_and_clear_bit(idx, qedi->task_idx_map))
QEDI_ERR(>dbg_ctx,
 "FW task context, already cleared, tid=0x%x\n", idx);
-   WARN_ON(1);
-   }
 }
 
 void qedi_update_itt_map(struct qedi_ctx *qedi, u32 tid, u32 proto_itt,

Re: [PATCH v2 3/6] qla2xxx: Add FC-NVMe F/W initialization and transport registration

[Johannes Thumshirn]

On Thu, Jun 22, 2017 at 10:48:46AM -0700, James Smart wrote:
> He can't move it. the fcport->nvme_remote_port pointer is set by the
> nvme_fc_register_remoteport() routine (if return status is 0).

Gah, that's kind of wired. Literly _all_ of the Kernel's register_xxx()
funtions have a semantic that after the registration is done the object can be
used and thus assigning private pointer afterwards is an error. Damn I didn't
realize this in the nmve-fc review.

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] lpfc: Fix nvme io stoppage after link bounce

[Johannes Thumshirn]

On Thu, Jun 22, 2017 at 08:52:14AM -0700, James Smart wrote:
> well, the primary reason is the code that was removed was bogus to begin
> with. Abort of an io/exchange, if its active, needs to occur on the link
> regardless of the ndlp state. This it simply needed to occur. As not checks
> on ndlp state are needed, there is no race.

Thanks for the explanation

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

KINDLY REPLY URGENTLY

[IBRAHIM KABORE]

Dear Friend

I am contacting you on a business deal of $9,500,000.00 Million United States 
Dollars, ready for transfer into your own personal account and if we make this 
claim, we will share it on the ratio of 50% / 50% basis, I would like to assure 
you that it be 100% risk free and it will be legally backed up with government 
approval. Once you are interested to transact this business with me, kindly 
give me your consent response immediately.

Hoping to hear from you.

My regards,
Mr Ibrahim Kabore
EMAIL,ibrahim.kab...@hotmail.com

[PATCH] scsi: hptiop: make function hptiop_iop_request_callback_itl static

[Colin King]

From: Colin Ian King 

The function hptiop_iop_request_callback_itl does not need to be in
global scope, so make it static.

Cleans up sparse warning:
"symbol 'hptiop_iop_request_callback_itl' was not declared. Should it
 be static?"

Signed-off-by: Colin Ian King 
---
 drivers/scsi/hptiop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
index db17ad15b0c1..7226226f7383 100644
--- a/drivers/scsi/hptiop.c
+++ b/drivers/scsi/hptiop.c
@@ -800,7 +800,7 @@ static void hptiop_host_request_callback_itl(struct 
hptiop_hba *hba, u32 _tag)
hptiop_finish_scsi_req(hba, tag, req);
 }
 
-void hptiop_iop_request_callback_itl(struct hptiop_hba *hba, u32 tag)
+static void hptiop_iop_request_callback_itl(struct hptiop_hba *hba, u32 tag)
 {
struct hpt_iop_request_header __iomem *req;
struct hpt_iop_request_ioctl_command __iomem *p;
-- 
2.11.0

Re: [PATCH] lpfc: Fix nvme io stoppage after link bounce

[James Smart]

On 6/22/2017 12:41 AM, Johannes Thumshirn wrote:

On Wed, Jun 21, 2017 at 10:37:58AM -0700, James Smart wrote:
[...]

-   /*
-* Catch race where our node has transitioned, but the
-* transport is still transitioning.
-*/



The patch looks fine but can you please explain why the above race can't
happen anymore?

Thanks,
Reviewed-by: Johannes Thumshirn 



well, the primary reason is the code that was removed was bogus to begin 
with. Abort of an io/exchange, if its active, needs to occur on the link 
regardless of the ndlp state. This it simply needed to occur. As not 
checks on ndlp state are needed, there is no race.


-- james

Re: [PATCH] scsi: default to scsi-mq

[Martin Wilck]

Hello Christoph,

On Fri, 2017-06-16 at 10:27 +0200, Christoph Hellwig wrote:
> Remove the SCSI_MQ_DEFAULT config option and default to the blk-mq
> I/O
> path now that we had plenty of testing, and have I/O schedulers for
> blk-mq.  The module option to disable the blk-mq path is kept around
> for now.

Could you explain why you remove the option entirely rather than just
changing the default? Some distributions may wish to keep the ability
to choose the compiled-in default.

Regards,
Martin

-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH] qla2xxx: Protect access to qpair members with qpair->qp_lock

[John Garry]

On 22/06/2017 14:43, Johannes Thumshirn wrote:

In qla2xx_start_scsi_mq() and qla2xx_dif_start_scsi_mq() we grab the
qpair->qp_lock but do access members of the qpair before having the lock.
Re-order the locking sequence to have all read and write access to qpair
members under the qpair->qp_lock.

Signed-off-by: Johannes Thumshirn 
---
 drivers/scsi/qla2xxx/qla_iocb.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 8404f17f3c6c..425ca1646a9a 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -1770,10 +1770,6 @@ qla2xxx_start_scsi_mq(srb_t *sp)
struct qla_hw_data *ha = vha->hw;
struct qla_qpair *qpair = sp->qpair;

-   /* Setup qpair pointers */
-   rsp = qpair->rsp;
-   req = qpair->req;


Can you check the call to qla2x00_marker() before the spinlock grab, 
which takes rsp and req as parameters?



-
/* So we know we haven't pci_map'ed anything yet */
tot_dsds = 0;

@@ -1788,6 +1784,10 @@ qla2xxx_start_scsi_mq(srb_t *sp)
/* Acquire qpair specific lock */
spin_lock_irqsave(>qp_lock, flags);

+   /* Setup qpair pointers */
+   rsp = qpair->rsp;
+   req = qpair->req;
+
/* Check for room in outstanding command list. */
handle = req->current_outstanding_cmd;
for (index = 1; index < req->num_outstanding_cmds; index++) {
@@ -1924,24 +1924,33 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp)

 #define QDSS_GOT_Q_SPACE   BIT_0

+   /* Acquire ring specific lock */
+   spin_lock_irqsave(>qp_lock, flags);
+
/* Check for host side state */
if (!qpair->online) {
cmd->result = DID_NO_CONNECT << 16;
+   spin_unlock_irqrestore(>qp_lock, flags);
return QLA_INTERFACE_ERROR;
}

if (!qpair->difdix_supported &&
scsi_get_prot_op(cmd) != SCSI_PROT_NORMAL) {
cmd->result = DID_NO_CONNECT << 16;
+   spin_unlock_irqrestore(>qp_lock, flags);
return QLA_INTERFACE_ERROR;
}

+   spin_unlock_irqrestore(>qp_lock, flags);
+
/* Only process protection or >16 cdb in this routine */
if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) {
if (cmd->cmd_len <= 16)
return qla2xxx_start_scsi_mq(sp);
}

+   spin_lock_irqsave(>qp_lock, flags);
+
/* Setup qpair pointers */
rsp = qpair->rsp;
req = qpair->req;
@@ -1957,9 +1966,6 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp)
vha->marker_needed = 0;
}

-   /* Acquire ring specific lock */
-   spin_lock_irqsave(>qp_lock, flags);
-
/* Check for room in outstanding command list. */
handle = req->current_outstanding_cmd;
for (index = 1; index < req->num_outstanding_cmds; index++) {

Re: [PATCH] qla2xxx: Protect access to qpair members with qpair->qp_lock

[Johannes Thumshirn]

On Thu, Jun 22, 2017 at 03:19:27PM +0100, John Garry wrote:
> On 22/06/2017 14:43, Johannes Thumshirn wrote:
> >In qla2xx_start_scsi_mq() and qla2xx_dif_start_scsi_mq() we grab the
> >qpair->qp_lock but do access members of the qpair before having the lock.
> >Re-order the locking sequence to have all read and write access to qpair
> >members under the qpair->qp_lock.
> >
> >Signed-off-by: Johannes Thumshirn 
> >---
> > drivers/scsi/qla2xxx/qla_iocb.c | 20 +---
> > 1 file changed, 13 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/scsi/qla2xxx/qla_iocb.c 
> >b/drivers/scsi/qla2xxx/qla_iocb.c
> >index 8404f17f3c6c..425ca1646a9a 100644
> >--- a/drivers/scsi/qla2xxx/qla_iocb.c
> >+++ b/drivers/scsi/qla2xxx/qla_iocb.c
> >@@ -1770,10 +1770,6 @@ qla2xxx_start_scsi_mq(srb_t *sp)
> > struct qla_hw_data *ha = vha->hw;
> > struct qla_qpair *qpair = sp->qpair;
> >
> >-/* Setup qpair pointers */
> >-rsp = qpair->rsp;
> >-req = qpair->req;
> 
> Can you check the call to qla2x00_marker() before the spinlock grab, which
> takes rsp and req as parameters?

Good catch, thanks.

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] qla2xxx: Protect access to qpair members with qpair->qp_lock

[Hannes Reinecke]

On 06/22/2017 03:43 PM, Johannes Thumshirn wrote:
> In qla2xx_start_scsi_mq() and qla2xx_dif_start_scsi_mq() we grab the
> qpair->qp_lock but do access members of the qpair before having the lock.
> Re-order the locking sequence to have all read and write access to qpair
> members under the qpair->qp_lock.
> 
> Signed-off-by: Johannes Thumshirn 
> ---
>  drivers/scsi/qla2xxx/qla_iocb.c | 20 +---
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
> index 8404f17f3c6c..425ca1646a9a 100644
> --- a/drivers/scsi/qla2xxx/qla_iocb.c
> +++ b/drivers/scsi/qla2xxx/qla_iocb.c
> @@ -1770,10 +1770,6 @@ qla2xxx_start_scsi_mq(srb_t *sp)
>   struct qla_hw_data *ha = vha->hw;
>   struct qla_qpair *qpair = sp->qpair;
>  
> - /* Setup qpair pointers */
> - rsp = qpair->rsp;
> - req = qpair->req;
> -
>   /* So we know we haven't pci_map'ed anything yet */
>   tot_dsds = 0;
>  
> @@ -1788,6 +1784,10 @@ qla2xxx_start_scsi_mq(srb_t *sp)
>   /* Acquire qpair specific lock */
>   spin_lock_irqsave(>qp_lock, flags);
>  
> + /* Setup qpair pointers */
> + rsp = qpair->rsp;
> + req = qpair->req;
> +
>   /* Check for room in outstanding command list. */
>   handle = req->current_outstanding_cmd;
>   for (index = 1; index < req->num_outstanding_cmds; index++) {
> @@ -1924,24 +1924,33 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp)
>  
>  #define QDSS_GOT_Q_SPACE BIT_0
>  
> + /* Acquire ring specific lock */
> + spin_lock_irqsave(>qp_lock, flags);
> +
>   /* Check for host side state */
>   if (!qpair->online) {
>   cmd->result = DID_NO_CONNECT << 16;
> + spin_unlock_irqrestore(>qp_lock, flags);
>   return QLA_INTERFACE_ERROR;
>   }
>  
>   if (!qpair->difdix_supported &&
>   scsi_get_prot_op(cmd) != SCSI_PROT_NORMAL) {
>   cmd->result = DID_NO_CONNECT << 16;
> + spin_unlock_irqrestore(>qp_lock, flags);
>   return QLA_INTERFACE_ERROR;
>   }
>  
> + spin_unlock_irqrestore(>qp_lock, flags);
> +
>   /* Only process protection or >16 cdb in this routine */
>   if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) {
>   if (cmd->cmd_len <= 16)
>   return qla2xxx_start_scsi_mq(sp);
>   }
>  
Not sure if that is required; at worst we're missing a device being
online; difdix support is probably not changing that often to be
warranted being protected with a lock here.

So I'd rather omit spinlocks here.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH] qla2xxx: Protect access to qpair members with qpair->qp_lock

[Johannes Thumshirn]

In qla2xx_start_scsi_mq() and qla2xx_dif_start_scsi_mq() we grab the
qpair->qp_lock but do access members of the qpair before having the lock.
Re-order the locking sequence to have all read and write access to qpair
members under the qpair->qp_lock.

Signed-off-by: Johannes Thumshirn 
---
 drivers/scsi/qla2xxx/qla_iocb.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 8404f17f3c6c..425ca1646a9a 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -1770,10 +1770,6 @@ qla2xxx_start_scsi_mq(srb_t *sp)
struct qla_hw_data *ha = vha->hw;
struct qla_qpair *qpair = sp->qpair;
 
-   /* Setup qpair pointers */
-   rsp = qpair->rsp;
-   req = qpair->req;
-
/* So we know we haven't pci_map'ed anything yet */
tot_dsds = 0;
 
@@ -1788,6 +1784,10 @@ qla2xxx_start_scsi_mq(srb_t *sp)
/* Acquire qpair specific lock */
spin_lock_irqsave(>qp_lock, flags);
 
+   /* Setup qpair pointers */
+   rsp = qpair->rsp;
+   req = qpair->req;
+
/* Check for room in outstanding command list. */
handle = req->current_outstanding_cmd;
for (index = 1; index < req->num_outstanding_cmds; index++) {
@@ -1924,24 +1924,33 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp)
 
 #define QDSS_GOT_Q_SPACE   BIT_0
 
+   /* Acquire ring specific lock */
+   spin_lock_irqsave(>qp_lock, flags);
+
/* Check for host side state */
if (!qpair->online) {
cmd->result = DID_NO_CONNECT << 16;
+   spin_unlock_irqrestore(>qp_lock, flags);
return QLA_INTERFACE_ERROR;
}
 
if (!qpair->difdix_supported &&
scsi_get_prot_op(cmd) != SCSI_PROT_NORMAL) {
cmd->result = DID_NO_CONNECT << 16;
+   spin_unlock_irqrestore(>qp_lock, flags);
return QLA_INTERFACE_ERROR;
}
 
+   spin_unlock_irqrestore(>qp_lock, flags);
+
/* Only process protection or >16 cdb in this routine */
if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL) {
if (cmd->cmd_len <= 16)
return qla2xxx_start_scsi_mq(sp);
}
 
+   spin_lock_irqsave(>qp_lock, flags);
+
/* Setup qpair pointers */
rsp = qpair->rsp;
req = qpair->req;
@@ -1957,9 +1966,6 @@ qla2xxx_dif_start_scsi_mq(srb_t *sp)
vha->marker_needed = 0;
}
 
-   /* Acquire ring specific lock */
-   spin_lock_irqsave(>qp_lock, flags);
-
/* Check for room in outstanding command list. */
handle = req->current_outstanding_cmd;
for (index = 1; index < req->num_outstanding_cmds; index++) {
-- 
2.12.3

Re: [PATCH v2 2/5] dt-bindings: scsi: ufs: add document for hi3660-ufs

[Bu Tao]

在 2017/6/22 20:15, Arnd Bergmann 写道:

On Thu, Jun 22, 2017 at 1:58 PM, Bu Tao  wrote:

在 2017/6/22 19:51, Arnd Bergmann 写道:

On Thu, Jun 22, 2017 at 1:44 PM, Bu Tao  wrote:

在 2017/6/17 5:51, Arnd Bergmann 写道:

On Fri, Jun 16, 2017 at 8:51 AM, Bu Tao  wrote:


I do not know wheher other SoC need to use the optional properties as
abover. So here the name of the optional properties has "hi3660".



They should not have "hi3660" in their names either way, independent
of where they are used.



Oh, change the "hi3660" to "hisilicon"?
e.g. ufs-hi3660-use-rate-B  -->  ufs-hisilicon-use-rate-B


No, just 'use-rate-B', no prefix for this.


(note: this is different from the value of the "compatible" property
that
is meant to be as specific as possible".

Also, please clarify how your binding relates to the ufshcd binding
in Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt: does
hi3660 implement any registers that are shared with ufshcd, or does
it use the same physical interface with a different register set?



No, only show how to use the dt-binding for hi3660 SoC



My question was about the hardware: does hi3660 implement ufshcd
or not?



YES


Ok, then the properties should be documented as optional in the
Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt file for anything
that has a proper interpretation in the context of the generic ufshcd
driver.

  Arnd



OK I will modify this and update the patch soon.

Re: [PATCH v2 2/5] dt-bindings: scsi: ufs: add document for hi3660-ufs

[Arnd Bergmann]

On Thu, Jun 22, 2017 at 1:58 PM, Bu Tao  wrote:
> 在 2017/6/22 19:51, Arnd Bergmann 写道:
>> On Thu, Jun 22, 2017 at 1:44 PM, Bu Tao  wrote:
>>> 在 2017/6/17 5:51, Arnd Bergmann 写道:
 On Fri, Jun 16, 2017 at 8:51 AM, Bu Tao  wrote:
>>>
>>> I do not know wheher other SoC need to use the optional properties as
>>> abover. So here the name of the optional properties has "hi3660".
>>
>>
>> They should not have "hi3660" in their names either way, independent
>> of where they are used.
>
>
> Oh, change the "hi3660" to "hisilicon"?
> e.g. ufs-hi3660-use-rate-B  -->  ufs-hisilicon-use-rate-B

No, just 'use-rate-B', no prefix for this.

 (note: this is different from the value of the "compatible" property
 that
 is meant to be as specific as possible".

 Also, please clarify how your binding relates to the ufshcd binding
 in Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt: does
 hi3660 implement any registers that are shared with ufshcd, or does
 it use the same physical interface with a different register set?
>>>
>>>
>>> No, only show how to use the dt-binding for hi3660 SoC
>>
>>
>> My question was about the hardware: does hi3660 implement ufshcd
>> or not?
>
>
> YES

Ok, then the properties should be documented as optional in the
Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt file for anything
that has a proper interpretation in the context of the generic ufshcd
driver.

  Arnd

[PATCH] block: cciss: constify cciss_host_attr_group and cciss_dev_attr_group

[Arvind Yadav]

File size before:
   textdata bss dec hex filename
  409111432 304   42647a697 drivers/block/cciss.o

File size After adding 'const':
   textdata bss dec hex filename
  410391304 304   42647a697 drivers/block/cciss.o

Signed-off-by: Arvind Yadav 
---
 drivers/block/cciss.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index cd37550..8932cf6 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -853,7 +853,7 @@ static ssize_t cciss_show_usage_count(struct device *dev,
NULL
 };
 
-static struct attribute_group cciss_host_attr_group = {
+static const struct attribute_group cciss_host_attr_group = {
.attrs = cciss_host_attrs,
 };
 
@@ -879,7 +879,7 @@ static ssize_t cciss_show_usage_count(struct device *dev,
NULL
 };
 
-static struct attribute_group cciss_dev_attr_group = {
+static const struct attribute_group cciss_dev_attr_group = {
.attrs = cciss_dev_attrs,
 };
 
-- 
1.9.1

Re: [PATCH v2 2/5] dt-bindings: scsi: ufs: add document for hi3660-ufs

[Bu Tao]

在 2017/6/22 19:51, Arnd Bergmann 写道:

On Thu, Jun 22, 2017 at 1:44 PM, Bu Tao  wrote:

在 2017/6/17 5:51, Arnd Bergmann 写道:

On Fri, Jun 16, 2017 at 8:51 AM, Bu Tao  wrote:

+Optional properties for board device:
+- ufs-hi3660-use-rate-B: specifies UFS rate-B
+- ufs-hi3660-broken-fastauto   : specifies no fastauto
+- ufs-hi3660-use-HS-GEAR3  : specifies UFS HS-GEAR3
+- ufs-hi3660-use-HS-GEAR2  : specifies UFS HS-GEAR2
+- ufs-hi3660-use-HS-GEAR1  : specifies UFS HS-GEAR1
+- ufs-hi3660-broken-clk-gate-bypass: specifies no clk-gate
+- ufs-hi3660-use-one-line  : specifies UFS use one line work
+- reset-gpio   : specifies to reset devices



Some of these sound rather generic and might apply to UFS implementations
other than hi3660, so I'd suggest adding them to the base ufs binding with
a generic name instead.

Any DT properties that might be useful across multiple implementations
should be parsed in generic code that gets called by the individual
drivers,
and then the properties that are specific to the integration work done by
hisilicon should be prefixed with "hisilicon,", but not normally with the
SoC name: it is quite possible that another SoC will be derived from this
chip and it should reuse the properties.



I do not know wheher other SoC need to use the optional properties as
abover. So here the name of the optional properties has "hi3660".


They should not have "hi3660" in their names either way, independent
of where they are used.


Oh, change the "hi3660" to "hisilicon"?
e.g. ufs-hi3660-use-rate-B  -->  ufs-hisilicon-use-rate-B



(note: this is different from the value of the "compatible" property that
is meant to be as specific as possible".

Also, please clarify how your binding relates to the ufshcd binding
in Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt: does
hi3660 implement any registers that are shared with ufshcd, or does
it use the same physical interface with a different register set?


No, only show how to use the dt-binding for hi3660 SoC


My question was about the hardware: does hi3660 implement ufshcd
or not?


YES


  Arnd

Re: [PATCH v2 2/5] dt-bindings: scsi: ufs: add document for hi3660-ufs

[Bu Tao]

在 2017/6/17 5:51, Arnd Bergmann 写道:

On Fri, Jun 16, 2017 at 8:51 AM, Bu Tao  wrote:

add ufs node document for hi3660

Signed-off-by: Bu Tao 
---
 .../devicetree/bindings/ufs/hi3660-ufs.txt | 58 ++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ufs/hi3660-ufs.txt

diff --git a/Documentation/devicetree/bindings/ufs/hi3660-ufs.txt 
b/Documentation/devicetree/bindings/ufs/hi3660-ufs.txt
new file mode 100644
index ..461afc8ef017
--- /dev/null
+++ b/Documentation/devicetree/bindings/ufs/hi3660-ufs.txt
@@ -0,0 +1,58 @@
+* Hisilicon Universal Flash Storage (UFS) Host Controller
+
+UFS nodes are defined to describe on-chip UFS hardware macro.
+Each UFS Host Controller should have its own node.
+
+Required properties:
+- compatible: compatible list, contains one of the following -
+   "hisilicon,hi3660-ufs" for hisi ufs host controller
+present on Hi3660 chipset.
+- reg   : should contain UFS register address space & UFS SYS CTRL 
register address,
+- interrupt-parent  : interrupt device
+- interrupts: interrupt number
+- clocks   : List of phandle and clock specifier pairs
+- clock-names   : List of clock input name strings sorted in the same
+ order as the clocks property. "clk_ref", "clk_phy" is 
optional
+- resets: reset node register, one reset the clk and the other 
reset the controller
+- reset-names   : describe reset node register
+
+Optional properties for board device:
+- ufs-hi3660-use-rate-B: specifies UFS rate-B
+- ufs-hi3660-broken-fastauto   : specifies no fastauto
+- ufs-hi3660-use-HS-GEAR3  : specifies UFS HS-GEAR3
+- ufs-hi3660-use-HS-GEAR2  : specifies UFS HS-GEAR2
+- ufs-hi3660-use-HS-GEAR1  : specifies UFS HS-GEAR1
+- ufs-hi3660-broken-clk-gate-bypass: specifies no clk-gate
+- ufs-hi3660-use-one-line  : specifies UFS use one line work
+- reset-gpio   : specifies to reset devices


Some of these sound rather generic and might apply to UFS implementations
other than hi3660, so I'd suggest adding them to the base ufs binding with
a generic name instead.

Any DT properties that might be useful across multiple implementations
should be parsed in generic code that gets called by the individual drivers,
and then the properties that are specific to the integration work done by
hisilicon should be prefixed with "hisilicon,", but not normally with the
SoC name: it is quite possible that another SoC will be derived from this
chip and it should reuse the properties.


I do not know wheher other SoC need to use the optional properties as 
abover. So here the name of the optional properties has "hi3660".


(note: this is different from the value of the "compatible" property that
is meant to be as specific as possible".

Also, please clarify how your binding relates to the ufshcd binding
in Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt: does
hi3660 implement any registers that are shared with ufshcd, or does
it use the same physical interface with a different register set?


No, only show how to use the dt-binding for hi3660 SoC


   Arnd

Re: [PATCH v2 2/5] dt-bindings: scsi: ufs: add document for hi3660-ufs

[Arnd Bergmann]

On Thu, Jun 22, 2017 at 1:44 PM, Bu Tao  wrote:
> 在 2017/6/17 5:51, Arnd Bergmann 写道:
>> On Fri, Jun 16, 2017 at 8:51 AM, Bu Tao  wrote:
>>> +Optional properties for board device:
>>> +- ufs-hi3660-use-rate-B: specifies UFS rate-B
>>> +- ufs-hi3660-broken-fastauto   : specifies no fastauto
>>> +- ufs-hi3660-use-HS-GEAR3  : specifies UFS HS-GEAR3
>>> +- ufs-hi3660-use-HS-GEAR2  : specifies UFS HS-GEAR2
>>> +- ufs-hi3660-use-HS-GEAR1  : specifies UFS HS-GEAR1
>>> +- ufs-hi3660-broken-clk-gate-bypass: specifies no clk-gate
>>> +- ufs-hi3660-use-one-line  : specifies UFS use one line work
>>> +- reset-gpio   : specifies to reset devices
>>
>>
>> Some of these sound rather generic and might apply to UFS implementations
>> other than hi3660, so I'd suggest adding them to the base ufs binding with
>> a generic name instead.
>>
>> Any DT properties that might be useful across multiple implementations
>> should be parsed in generic code that gets called by the individual
>> drivers,
>> and then the properties that are specific to the integration work done by
>> hisilicon should be prefixed with "hisilicon,", but not normally with the
>> SoC name: it is quite possible that another SoC will be derived from this
>> chip and it should reuse the properties.
>
>
> I do not know wheher other SoC need to use the optional properties as
> abover. So here the name of the optional properties has "hi3660".

They should not have "hi3660" in their names either way, independent
of where they are used.

>> (note: this is different from the value of the "compatible" property that
>> is meant to be as specific as possible".
>>
>> Also, please clarify how your binding relates to the ufshcd binding
>> in Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt: does
>> hi3660 implement any registers that are shared with ufshcd, or does
>> it use the same physical interface with a different register set?
>
> No, only show how to use the dt-binding for hi3660 SoC

My question was about the hardware: does hi3660 implement ufshcd
or not?

  Arnd

[PATCH] scsi: cxgb4i: assign rxqs in round robin mode

[Varun Prakash]

Assign rxq to TCP connections in round robin mode
to use all available rxqs.

Signed-off-by: Varun Prakash 
---
 drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 5 -
 drivers/scsi/cxgbi/libcxgbi.h  | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c 
b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 0aae094..fa99471 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -1606,6 +1606,7 @@ static int init_act_open(struct cxgbi_sock *csk)
struct neighbour *n = NULL;
void *daddr;
unsigned int step;
+   unsigned int rxq_idx;
unsigned int size, size6;
unsigned int linkspeed;
unsigned int rcv_winf, snd_winf;
@@ -1684,7 +1685,9 @@ static int init_act_open(struct cxgbi_sock *csk)
step = lldi->ntxq / lldi->nchan;
csk->txq_idx = cxgb4_port_idx(ndev) * step;
step = lldi->nrxq / lldi->nchan;
-   csk->rss_qid = lldi->rxq_ids[cxgb4_port_idx(ndev) * step];
+   rxq_idx = (cxgb4_port_idx(ndev) * step) + (cdev->rxq_idx_cntr % step);
+   cdev->rxq_idx_cntr++;
+   csk->rss_qid = lldi->rxq_ids[rxq_idx];
linkspeed = ((struct port_info *)netdev_priv(ndev))->link_cfg.speed;
csk->snd_win = cxgb4i_snd_win;
csk->rcv_win = cxgb4i_rcv_win;
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 37f07aa..31a5816 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -477,6 +477,7 @@ struct cxgbi_device {
unsigned int skb_rx_extra;  /* for msg coalesced mode */
unsigned int tx_max_size;
unsigned int rx_max_size;
+   unsigned int rxq_idx_cntr;
struct cxgbi_ports_map pmap;
 
void (*dev_ddp_cleanup)(struct cxgbi_device *);
-- 
2.0.2

Re: [PATCH 1/2] hpsa: limit transfer length to 1MB

[Johannes Thumshirn]

On Thu, Jun 22, 2017 at 05:58:44PM +0800, Yadan Fan wrote:
> The hpsa firmware will bypass the cache for any request larger
> than 1MB, so we should cap the request size to avoid any
> performance degradation in SLE12-SP2.
Upstream doesn't care too much about SLES. 

> References: bsc#1025461

and our internal Bugzilla IDs


How about:
The hpsa firmware will bypass the cache for any requests larger than 1MB, so
we shoould cap the request size to avoid any performance regressions in
kernels later than 4.3 

The same applies for Patch 2/2

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 2/2] smartpqi: limit transfer length to 1MB

[Yadan Fan]

The smartpqi firmware will bypass the cache for any request larger
than 1MB, so we should cap the request size to avoid any
performance degradation on SLE12-SP2

This degradation is caused from d2be537c3ba3568acd79cd178327b842e60d035e,
which changed max_sectors_kb to 1280k, but the hardware is able to
work fine with it, so the true fix should be from smartpqi driver.

Reference: bsc#1025461

Signed-off-by: Yadan Fan 
---
 drivers/scsi/smartpqi/smartpqi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index b673825f46b5..9513357bd472 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -630,7 +630,7 @@ struct pqi_encryption_info {
 };
 
 #define PQI_MAX_OUTSTANDING_REQUESTS   ((u32)~0)
-#define PQI_MAX_TRANSFER_SIZE  (4 * 1024U * 1024U)
+#define PQI_MAX_TRANSFER_SIZE  (1024U * 1024U)
 
 #define RAID_MAP_MAX_ENTRIES   1024
 
-- 
2.12.3

[PATCH 1/2] hpsa: limit transfer length to 1MB

[Yadan Fan]

The hpsa firmware will bypass the cache for any request larger
than 1MB, so we should cap the request size to avoid any
performance degradation in SLE12-SP2.

This degradation is caused from d2be537c3ba3568acd79cd178327b842e60d035e,
which changed max_sectors_kb to 1280k, but the hardware is able to
work fine with it, so the true fix should be from hpsa driver.

References: bsc#1025461

Signed-off-by: Yadan Fan 
---
 drivers/scsi/hpsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 73daace478cb..b4c0bbea680c 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -952,7 +952,7 @@ static struct scsi_host_template hpsa_driver_template = {
 #endif
.sdev_attrs = hpsa_sdev_attrs,
.shost_attrs = hpsa_shost_attrs,
-   .max_sectors = 8192,
+   .max_sectors = 1024,
.no_write_same = 1,
 };
 
-- 
2.12.3

Re: [PATCH v2 5/6] qla2xxx: Use FC-NMVe FC4 type for FDMI registration

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 4/6] qla2xxx: Send FC4 type NVMe to the management server

[Johannes Thumshirn]

On Wed, Jun 21, 2017 at 01:48:44PM -0700, Madhani, Himanshu wrote:

Looks good,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 3/6] qla2xxx: Add FC-NVMe F/W initialization and transport registration

[Johannes Thumshirn]

On Wed, Jun 21, 2017 at 01:48:43PM -0700, Madhani, Himanshu wrote:
[...]
> + wait_queue_head_t nvme_ls_waitQ;

Can you please lower-case the 'Q' in waitQ IFF you have to re-send the series?

[...]
> + wait_queue_head_t nvme_waitQ;

Ditto

[...]
> + wait_queue_head_t nvme_waitQ;

And here as well.

> diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
> index 6fbee11c1a18..c6af45f7d5d6 100644
> --- a/drivers/scsi/qla2xxx/qla_gbl.h
> +++ b/drivers/scsi/qla2xxx/qla_gbl.h
> @@ -10,6 +10,16 @@
>  #include 
>  
>  /*
> + * Global functions prototype in qla_nvme.c source file.
> + */
> +extern void qla_nvme_register_hba(scsi_qla_host_t *);
> +extern int  qla_nvme_register_remote(scsi_qla_host_t *, fc_port_t *);
> +extern void qla_nvme_delete(scsi_qla_host_t *);
> +extern void qla_nvme_abort(struct qla_hw_data *, srb_t *sp);
> +extern void qla24xx_nvme_ls4_iocb(scsi_qla_host_t *, struct pt_ls4_request *,
> +struct req_que *);

You're still not convinced of the idea of headers, heh ;-)
Especially as you have a qla_nvme.h.

[...]

> + INIT_WORK(>nvme_del_work, qla_nvme_unregister_remote_port);
> + rport = kzalloc(sizeof(*rport), GFP_KERNEL);
> + if (!rport) {
> + ql_log(ql_log_warn, vha, 0x2101,
> + "%s: unable to alloc memory\n", __func__);

kzalloc() will warn you about a failed allocation, no need to double it.
See also:
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf

[...]

> + ret = nvme_fc_register_remoteport(vha->nvme_local_port, >req,
> + >nvme_remote_port);
> + if (ret) {
> + ql_log(ql_log_warn, vha, 0x212e,
> + "Failed to register remote port. Transport returned %d\n",
> + ret);
> + return ret;
> + }
> +
> + fcport->nvme_remote_port->private = fcport;

I think I already said that in the last review, but can you please move the 
fcport->nvme_remote_port->private = fcport;
assingment _above_ the nvme_fc_register_remoteport() call.

[...]

> + vha = (struct scsi_qla_host *)lport->private;

No need to cast from void *
> + ql_log(ql_log_info, vha, 0x2104,
> + "%s: handle %p, idx =%d, qsize %d\n",
> + __func__, handle, qidx, qsize);

Btw, sometime in the future you could change your ql_log() thingies to the
kernel's dyndebug facility.

[...]

> + rval = ha->isp_ops->abort_command(sp);
> + if (rval != QLA_SUCCESS)
> + ql_log(ql_log_warn, fcport->vha, 0x2125,
> + "%s: failed to abort LS command for SP:%p rval=%x\n",
> + __func__, sp, rval);
> +
> + ql_dbg(ql_dbg_io, fcport->vha, 0x212b,
> + "%s: aborted sp:%p on fcport:%p\n", __func__, sp, fcport);

If you insinst in having these two messages ("failed to abort" and "aborted")
can you at least fold it into one print statement.

> + rval = ha->isp_ops->abort_command(sp);
> + if (!rval)
> + ql_log(ql_log_warn, fcport->vha, 0x2127,
> + "%s: failed to abort command for SP:%p rval=%x\n",
> + __func__, sp, rval);
> +
> + ql_dbg(ql_dbg_io, fcport->vha, 0x2126,
> + "%s: aborted sp:%p on fcport:%p\n", __func__, sp, fcport);

Ditto.

[...]


> + /* Setup qpair pointers */
> + req = qpair->req;
> + tot_dsds = fd->sg_cnt;
> +
> + /* Acquire qpair specific lock */
> + spin_lock_irqsave(>qp_lock, flags);
> +
> + /* Check for room in outstanding command list. */
> + handle = req->current_outstanding_cmd;

I've just seen this in qla2xxx_start_scsi_mq() and
qla2xxx_dif_start_scsi_mq() and was about to send you an RFC patch. But
here it is for completeness in the nvme version as well:

You save a pointer to the req_que from you qpair and _afterwards_ you grab
the qp_lock. What prevents someone from changing the request internals
underneath you?

Like this:

CPU0   CPU1
req = qpair->req;
 qla2xxx_delete_qpair(vha, qpair);
 `-> ret = qla25xx_delete_req_que(vha, 
qpair->req);
spin_lock_irqsave(>qp_lock, flags);
handle = req->current_outstanding_cmd;

Oh and btw, neither qla2xxx_delete_qpair() nor qla25xx_delete_req_que() grab
the qp_lock.

I think this is something work re-thinking. Maybe you can identify the blocks
accessing struct members which need to be touched under a lock and extract
them into a helper function wich calls lockdep_assert_held(). No must just and
idea.

[...]
> +
> + /* Load data segments */
> + for_each_sg(sgl, sg, tot_dsds, i) {

Do you really need the whole loop under a spin_lock_irqsave()? If the sglist
has a lot of entries (i.e. becasue we couldn't cluster it) we're in risk to
trigger a NMI watchdog soft-lockup WARN_ON(). You need to grab the lock when
accessing req's members but the rest of the loop? This applies to
qla24xx_build_scsi_iocbs() for SCSI as well.

[...]

> + 

Re: [PATCH] lpfc: fix refcount error on node list

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] lpfc: Fix nvme io stoppage after link bounce

[Johannes Thumshirn]

On Wed, Jun 21, 2017 at 10:37:58AM -0700, James Smart wrote:
[...]
> - /*
> -  * Catch race where our node has transitioned, but the
> -  * transport is still transitioning.
> -  */


The patch looks fine but can you please explain why the above race can't
happen anymore?

Thanks,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 6/6] qla2xxx: Update Driver version to 10.00.00.00-k

[Hannes Reinecke]

On 06/21/2017 10:48 PM, Madhani, Himanshu wrote:
> From: Himanshu Madhani 
> 
> Signed-off-by: Himanshu Madhani 
> Reviewed-by: Johannes Thumshirn 
> Reviewed-by: James Smart 
> ---
>  drivers/scsi/qla2xxx/qla_version.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_version.h 
> b/drivers/scsi/qla2xxx/qla_version.h
> index dcbb9bb05e99..005a378f7fab 100644
> --- a/drivers/scsi/qla2xxx/qla_version.h
> +++ b/drivers/scsi/qla2xxx/qla_version.h
> @@ -7,9 +7,9 @@
>  /*
>   * Driver version
>   */
> -#define QLA2XXX_VERSION  "9.01.00.00-k"
> +#define QLA2XXX_VERSION  "10.00.00.00-k"
>  
> -#define QLA_DRIVER_MAJOR_VER 9
> -#define QLA_DRIVER_MINOR_VER 1
> +#define QLA_DRIVER_MAJOR_VER 10
> +#define QLA_DRIVER_MINOR_VER 0
>  #define QLA_DRIVER_PATCH_VER 0
>  #define QLA_DRIVER_BETA_VER  0
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 5/6] qla2xxx: Use FC-NMVe FC4 type for FDMI registration

[Hannes Reinecke]

On 06/21/2017 10:48 PM, Madhani, Himanshu wrote:
> From: Duane Grigsby 
> 
> Signed-off-by: Duane Grigsby 
> Signed-off-by: Darren Trapp 
> Signed-off-by: Anil Gurumurthy 
> Signed-off-by: Giridhar Malavali 
> Signed-off-by: Himanshu Madhani 
> Reviewed-by: Johannes Thumshirn 
> Reviewed-by: James Smart 
> ---
>  drivers/scsi/qla2xxx/qla_gs.c | 16 
>  1 file changed, 16 insertions(+)

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 4/6] qla2xxx: Send FC4 type NVMe to the management server

[Hannes Reinecke]

On 06/21/2017 10:48 PM, Madhani, Himanshu wrote:
> From: Duane Grigsby 
> 
> This patch adds switch command support for FC-4 type of FC-NVMe (0x28)
> for resgistering HBA port to the management server. RFT_ID command is
> used to register FC-4 type of 0x28 and RFF_ID is used to register
> FC-4 features bits for FC-NVMe port.
> 
> Signed-off-by: Darren Trapp 
> Signed-off-by: Duane Grigsby 
> Signed-off-by: Anil Gurumurthy 
> Signed-off-by: Giridhar Malavali 
> Signed-off-by: Himanshu Madhani 
> Reviewed-By: James Smart 
> ---
>  drivers/scsi/qla2xxx/qla_def.h  |   1 +
>  drivers/scsi/qla2xxx/qla_gbl.h  |   6 +-
>  drivers/scsi/qla2xxx/qla_gs.c   | 118 
> +++-
>  drivers/scsi/qla2xxx/qla_init.c |  11 +++-
>  4 files changed, 131 insertions(+), 5 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 3/6] qla2xxx: Add FC-NVMe F/W initialization and transport registration

[Hannes Reinecke]

On 06/21/2017 10:48 PM, Madhani, Himanshu wrote:
> From: Duane Grigsby 
> 
> 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 
> Signed-off-by: Duane Grigsby 
> Signed-off-by: Anil Gurumurthy 
> Signed-off-by: Giridhar Malavali 
> Signed-off-by: Himanshu Madhani 
> ---
>  drivers/scsi/qla2xxx/Makefile   |   2 +-
>  drivers/scsi/qla2xxx/qla_dbg.c  |   2 +-
>  drivers/scsi/qla2xxx/qla_def.h  |   6 +
>  drivers/scsi/qla2xxx/qla_gbl.h  |  11 +
>  drivers/scsi/qla2xxx/qla_init.c |   8 +
>  drivers/scsi/qla2xxx/qla_iocb.c |  36 ++
>  drivers/scsi/qla2xxx/qla_isr.c  |  19 +
>  drivers/scsi/qla2xxx/qla_mbx.c  |  21 ++
>  drivers/scsi/qla2xxx/qla_nvme.c | 756 
> 
>  drivers/scsi/qla2xxx/qla_nvme.h | 132 +++
>  drivers/scsi/qla2xxx/qla_os.c   |  40 ++-
>  11 files changed, 1024 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/scsi/qla2xxx/qla_nvme.c
>  create mode 100644 drivers/scsi/qla2xxx/qla_nvme.h
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 2/6] qla2xxx: Add FC-NVMe command handling

[Hannes Reinecke]

On 06/21/2017 10:48 PM, Madhani, Himanshu wrote:
> From: Duane Grigsby 
> 
> This patch adds logic to  handle the completion of
> FC-NVMe commands and creates a sub-command in the SRB
> command structure to manage NVMe commands.
> 
> Signed-off-by: Darren Trapp 
> Signed-off-by: Duane Grigsby 
> Signed-off-by: Anil Gurumurthy 
> Signed-off-by: Giridhar Malavali 
> Signed-off-by: Himanshu Madhani 
> Reviewed-by: Johannes Thumshirn 
> ---
>  drivers/scsi/qla2xxx/qla_def.h | 17 +
>  drivers/scsi/qla2xxx/qla_fw.h  | 22 ++--
>  drivers/scsi/qla2xxx/qla_isr.c | 79 
> ++
>  drivers/scsi/qla2xxx/qla_os.c  | 18 --
>  4 files changed, 131 insertions(+), 5 deletions(-)
> Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 1/6] qla2xxx: Add FC-NVMe port discovery and PRLI handling

[Hannes Reinecke]

On 06/21/2017 10:48 PM, Madhani, Himanshu wrote:
> From: Duane Grigsby 
> 
> Added logic to change the login process into an optional PRIL
> step for FC-NVMe ports as a separate operation, such that we can
> change type to 0x28 (NVMe).
> 
> Currently, the driver performs the PLOGI/PRLI together as one
> operation, but if the discovered port is an NVMe port then we
> first issue the PLOGI and then we issue the PRLI. Also, the
> fabric discovery logic was changed to mark each discovered FC
> NVMe port, so that we can register them with the FC-NVMe transport
> later.
> 
> Signed-off-by: Darren Trapp 
> Signed-off-by: Duane Grigsby 
> Signed-off-by: Anil Gurumurthy 
> Signed-off-by: Giridhar Malavali 
> Signed-off-by: Himanshu Madhani 
> Reviewed-by: Johannes Thumshirn 
> ---
>  drivers/scsi/qla2xxx/qla_dbg.c|   9 +-
>  drivers/scsi/qla2xxx/qla_def.h|  30 ++-
>  drivers/scsi/qla2xxx/qla_fw.h |  13 ++-
>  drivers/scsi/qla2xxx/qla_gbl.h|   1 +
>  drivers/scsi/qla2xxx/qla_init.c   | 168 
> --
>  drivers/scsi/qla2xxx/qla_iocb.c   |  21 +
>  drivers/scsi/qla2xxx/qla_mbx.c|  33 +---
>  drivers/scsi/qla2xxx/qla_os.c |   4 +
>  drivers/scsi/qla2xxx/qla_target.c |   4 +-
>  9 files changed, 256 insertions(+), 27 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)