linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/9] qla2xxx patches for kernel v5.9
@ 2020-06-29 22:54 Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 1/9] qla2xxx: Check the size of struct fcp_hdr at compile time Bart Van Assche
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-06-29 22:54 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley; +Cc: linux-scsi, Bart Van Assche

Hi Martin,

This patch series includes cleanup patches and also patches that address
complaints from several static source code analyzers. Please consider these
patches for kernel v5.9.

Thanks,

Bart.

Changes compared to v1:
- Rewrote patch "Fix a Coverity complaint in qla2100_fw_dump()"

Bart Van Assche (9):
  qla2xxx: Check the size of struct fcp_hdr at compile time
  qla2xxx: Remove the __packed annotation from struct fcp_hdr and
    fcp_hdr_le
  qla2xxx: Make qla82xx_flash_wait_write_finish() easier to read
  qla2xxx: Initialize 'n' before using it
  qla2xxx: Remove a superfluous cast
  qla2xxx: Make __qla2x00_alloc_iocbs() initialize 32 bits of
    request_t.handle
  qla2xxx: Fix a Coverity complaint in qla2100_fw_dump()
  qla2xxx: Make qla2x00_restart_isp() easier to read
  qla2xxx: Introduce a function for computing the debug message prefix

 drivers/scsi/qla2xxx/qla_bsg.c     |  3 +-
 drivers/scsi/qla2xxx/qla_dbg.c     | 98 ++++++++++--------------------
 drivers/scsi/qla2xxx/qla_dbg.h     |  1 +
 drivers/scsi/qla2xxx/qla_init.c    | 39 ++++++------
 drivers/scsi/qla2xxx/qla_iocb.c    |  4 +-
 drivers/scsi/qla2xxx/qla_nx.c      | 20 +++---
 drivers/scsi/qla2xxx/qla_target.h  |  4 +-
 drivers/scsi/qla2xxx/tcm_qla2xxx.c |  1 +
 8 files changed, 70 insertions(+), 100 deletions(-)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/9] qla2xxx: Check the size of struct fcp_hdr at compile time
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
@ 2020-06-29 22:54 ` Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 2/9] qla2xxx: Remove the __packed annotation from struct fcp_hdr and fcp_hdr_le Bart Van Assche
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-06-29 22:54 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Nilesh Javali,
	Quinn Tran, Himanshu Madhani, Martin Wilck, Roman Bolshakov

Since struct fcp_hdr is used to exchange data with the firmware, check its
size at compile time.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/tcm_qla2xxx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 188aa5f02c01..f7e9b5bc0b26 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -1971,6 +1971,7 @@ static int __init tcm_qla2xxx_init(void)
 	BUILD_BUG_ON(sizeof(struct ctio_crc2_to_fw) != 64);
 	BUILD_BUG_ON(sizeof(struct ctio_crc_from_fw) != 64);
 	BUILD_BUG_ON(sizeof(struct ctio_to_2xxx) != 64);
+	BUILD_BUG_ON(sizeof(struct fcp_hdr) != 24);
 	BUILD_BUG_ON(sizeof(struct fcp_hdr_le) != 24);
 	BUILD_BUG_ON(sizeof(struct nack_to_isp) != 64);
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/9] qla2xxx: Remove the __packed annotation from struct fcp_hdr and fcp_hdr_le
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 1/9] qla2xxx: Check the size of struct fcp_hdr at compile time Bart Van Assche
@ 2020-06-29 22:54 ` Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 3/9] qla2xxx: Make qla82xx_flash_wait_write_finish() easier to read Bart Van Assche
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-06-29 22:54 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Nilesh Javali,
	Quinn Tran, Himanshu Madhani, Martin Wilck, Roman Bolshakov

Remove the __packed annotation from struct fcp_hdr* because that annotation
is not necessary for these data structures.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_target.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index 010f12523b2a..1cff7c69d448 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -258,7 +258,7 @@ struct fcp_hdr {
 	__be16   ox_id;
 	uint16_t rx_id;
 	__le32	parameter;
-} __packed;
+};
 
 struct fcp_hdr_le {
 	le_id_t  d_id;
@@ -273,7 +273,7 @@ struct fcp_hdr_le {
 	__le16	rx_id;
 	__le16	ox_id;
 	__le32	parameter;
-} __packed;
+};
 
 #define F_CTL_EXCH_CONTEXT_RESP	BIT_23
 #define F_CTL_SEQ_CONTEXT_RESIP	BIT_22

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/9] qla2xxx: Make qla82xx_flash_wait_write_finish() easier to read
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 1/9] qla2xxx: Check the size of struct fcp_hdr at compile time Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 2/9] qla2xxx: Remove the __packed annotation from struct fcp_hdr and fcp_hdr_le Bart Van Assche
@ 2020-06-29 22:54 ` Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 4/9] qla2xxx: Initialize 'n' before using it Bart Van Assche
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-06-29 22:54 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Nilesh Javali,
	Quinn Tran, Himanshu Madhani, Martin Wilck, Roman Bolshakov

Return early instead of having a single return statement at the end of this
function. This patch fixes the following sparse warning:

qla_nx.c:1018: qla82xx_flash_wait_write_finish() error: uninitialized symbol 'val'.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_nx.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_nx.c b/drivers/scsi/qla2xxx/qla_nx.c
index 0baf55b7e88f..ff365b434a02 100644
--- a/drivers/scsi/qla2xxx/qla_nx.c
+++ b/drivers/scsi/qla2xxx/qla_nx.c
@@ -966,26 +966,21 @@ qla82xx_read_status_reg(struct qla_hw_data *ha, uint32_t *val)
 static int
 qla82xx_flash_wait_write_finish(struct qla_hw_data *ha)
 {
-	long timeout = 0;
-	uint32_t done = 1 ;
 	uint32_t val;
-	int ret = 0;
+	int i, ret;
 	scsi_qla_host_t *vha = pci_get_drvdata(ha->pdev);
 
 	qla82xx_wr_32(ha, QLA82XX_ROMUSB_ROM_ABYTE_CNT, 0);
-	while ((done != 0) && (ret == 0)) {
+	for (i = 0; i < 50000; i++) {
 		ret = qla82xx_read_status_reg(ha, &val);
-		done = val & 1;
-		timeout++;
+		if (ret < 0 || (val & 1) == 0)
+			return ret;
 		udelay(10);
 		cond_resched();
-		if (timeout >= 50000) {
-			ql_log(ql_log_warn, vha, 0xb00d,
-			    "Timeout reached waiting for write finish.\n");
-			return -1;
-		}
 	}
-	return ret;
+	ql_log(ql_log_warn, vha, 0xb00d,
+	       "Timeout reached waiting for write finish.\n");
+	return -1;
 }
 
 static int

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/9] qla2xxx: Initialize 'n' before using it
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
                   ` (2 preceding siblings ...)
  2020-06-29 22:54 ` [PATCH v2 3/9] qla2xxx: Make qla82xx_flash_wait_write_finish() easier to read Bart Van Assche
@ 2020-06-29 22:54 ` Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 5/9] qla2xxx: Remove a superfluous cast Bart Van Assche
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-06-29 22:54 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Shyam Sundar,
	Nilesh Javali, Quinn Tran, Himanshu Madhani, Martin Wilck,
	Roman Bolshakov

The following code:
	qla82xx_rom_fast_read(ha, 0, &n)
only initializes 'n' if it succeeds. Since 'n' may be reported in a debug
message even if no ROM reads succeeded, initialize 'n' to zero.

This patch fixes the following sparse warning:

qla_nx.c:1218: qla82xx_pinit_from_rom() error: uninitialized symbol 'n'.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Shyam Sundar <ssundar@marvell.com>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_nx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/qla2xxx/qla_nx.c b/drivers/scsi/qla2xxx/qla_nx.c
index ff365b434a02..71273eb634d3 100644
--- a/drivers/scsi/qla2xxx/qla_nx.c
+++ b/drivers/scsi/qla2xxx/qla_nx.c
@@ -1167,6 +1167,7 @@ qla82xx_pinit_from_rom(scsi_qla_host_t *vha)
 	 * Offset 4: Offset and number of addr/value pairs
 	 * that present in CRB initialize sequence
 	 */
+	n = 0;
 	if (qla82xx_rom_fast_read(ha, 0, &n) != 0 || n != 0xcafecafeUL ||
 	    qla82xx_rom_fast_read(ha, 4, &n) != 0) {
 		ql_log(ql_log_fatal, vha, 0x006e,

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/9] qla2xxx: Remove a superfluous cast
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
                   ` (3 preceding siblings ...)
  2020-06-29 22:54 ` [PATCH v2 4/9] qla2xxx: Initialize 'n' before using it Bart Van Assche
@ 2020-06-29 22:54 ` Bart Van Assche
  2020-06-30 17:13   ` Shyam Sundar
  2020-06-29 22:54 ` [PATCH v2 6/9] qla2xxx: Make __qla2x00_alloc_iocbs() initialize 32 bits of request_t.handle Bart Van Assche
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-06-29 22:54 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Nilesh Javali,
	Quinn Tran, Himanshu Madhani, Martin Wilck, Roman Bolshakov

Remove an unnecessary cast because it prevents the compiler to perform
type checking.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_bsg.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index 88c0338a2ec7..67efde1d4b8e 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -223,8 +223,7 @@ qla24xx_proc_fcp_prio_cfg_cmd(struct bsg_job *bsg_job)
 
 		/* validate fcp priority data */
 
-		if (!qla24xx_fcp_prio_cfg_valid(vha,
-		    (struct qla_fcp_prio_cfg *) ha->fcp_prio_cfg, 1)) {
+		if (!qla24xx_fcp_prio_cfg_valid(vha, ha->fcp_prio_cfg, 1)) {
 			bsg_reply->result = (DID_ERROR << 16);
 			ret = -EINVAL;
 			/* If buffer was invalidatic int

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 6/9] qla2xxx: Make __qla2x00_alloc_iocbs() initialize 32 bits of request_t.handle
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
                   ` (4 preceding siblings ...)
  2020-06-29 22:54 ` [PATCH v2 5/9] qla2xxx: Remove a superfluous cast Bart Van Assche
@ 2020-06-29 22:54 ` Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 7/9] qla2xxx: Fix a Coverity complaint in qla2100_fw_dump() Bart Van Assche
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-06-29 22:54 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Nilesh Javali,
	Quinn Tran, Himanshu Madhani, Martin Wilck, Roman Bolshakov

The request_t 'handle' member is 32-bits wide, hence use wrt_reg_dword().
Change the cast in the wrt_reg_byte() call to make it clear that a
regular pointer is casted to an __iomem pointer.

Note: 'pkt' points to I/O memory for the qlafx00 adapter family and to
coherent memory for all other adapter families.

This patch fixes the following Coverity complaint:

CID 358864 (#1 of 1): Reliance on integer endianness (INCOMPATIBLE_CAST)
incompatible_cast: Pointer &pkt->handle points to an object whose effective
type is unsigned int (32 bits, unsigned) but is dereferenced as a narrower
unsigned short (16 bits, unsigned). This may lead to unexpected results
depending on machine endianness.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Fixes: 8ae6d9c7eb10 ("[SCSI] qla2xxx: Enhancements to support ISPFx00.")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_iocb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 1d3c58c5f0e2..e3d2dea0b057 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2305,8 +2305,8 @@ __qla2x00_alloc_iocbs(struct qla_qpair *qpair, srb_t *sp)
 	pkt = req->ring_ptr;
 	memset(pkt, 0, REQUEST_ENTRY_SIZE);
 	if (IS_QLAFX00(ha)) {
-		wrt_reg_byte((void __iomem *)&pkt->entry_count, req_cnt);
-		wrt_reg_word((void __iomem *)&pkt->handle, handle);
+		wrt_reg_byte((u8 __force __iomem *)&pkt->entry_count, req_cnt);
+		wrt_reg_dword((__le32 __force __iomem *)&pkt->handle, handle);
 	} else {
 		pkt->entry_count = req_cnt;
 		pkt->handle = handle;

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 7/9] qla2xxx: Fix a Coverity complaint in qla2100_fw_dump()
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
                   ` (5 preceding siblings ...)
  2020-06-29 22:54 ` [PATCH v2 6/9] qla2xxx: Make __qla2x00_alloc_iocbs() initialize 32 bits of request_t.handle Bart Van Assche
@ 2020-06-29 22:54 ` Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 8/9] qla2xxx: Make qla2x00_restart_isp() easier to read Bart Van Assche
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-06-29 22:54 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Nilesh Javali,
	Quinn Tran, Himanshu Madhani, Martin Wilck, Roman Bolshakov

'cnt' can exceed the size of the risc_ram[] array. Prevent that Coverity
complains by rewriting an address calculation expression. This patch fixes
the following Coverity complaint:

CID 337803 (#1 of 1): Out-of-bounds read (OVERRUN)
109. overrun-local: Overrunning array of 122880 bytes at byte offset 122880
by dereferencing pointer &fw->risc_ram[cnt].

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_dbg.c | 2 +-
 drivers/scsi/qla2xxx/qla_dbg.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index 19005710f7f6..41493bd53fc0 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -1063,7 +1063,7 @@ qla2100_fw_dump(scsi_qla_host_t *vha)
 	}
 
 	if (rval == QLA_SUCCESS)
-		qla2xxx_copy_queues(ha, &fw->risc_ram[cnt]);
+		qla2xxx_copy_queues(ha, &fw->queue_dump[0]);
 
 	qla2xxx_dump_post_process(base_vha, rval);
 }
diff --git a/drivers/scsi/qla2xxx/qla_dbg.h b/drivers/scsi/qla2xxx/qla_dbg.h
index 54ed020e6f75..91eb6901815c 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.h
+++ b/drivers/scsi/qla2xxx/qla_dbg.h
@@ -53,6 +53,7 @@ struct qla2100_fw_dump {
 	__be16 fpm_b0_reg[64];
 	__be16 fpm_b1_reg[64];
 	__be16 risc_ram[0xf000];
+	u8	queue_dump[];
 };
 
 struct qla24xx_fw_dump {

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 8/9] qla2xxx: Make qla2x00_restart_isp() easier to read
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
                   ` (6 preceding siblings ...)
  2020-06-29 22:54 ` [PATCH v2 7/9] qla2xxx: Fix a Coverity complaint in qla2100_fw_dump() Bart Van Assche
@ 2020-06-29 22:54 ` Bart Van Assche
  2020-06-29 22:54 ` [PATCH v2 9/9] qla2xxx: Introduce a function for computing the debug message prefix Bart Van Assche
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-06-29 22:54 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Nilesh Javali,
	Quinn Tran, Himanshu Madhani, Martin Wilck, Roman Bolshakov

Instead of using complicated control flow to only have one return statement
at the end of qla2x00_restart_isp(), return an error status as soon as it is
known that this function will fail.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_init.c | 39 +++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index d2717e7cf22a..8c739abf5589 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -6996,36 +6996,41 @@ qla2x00_abort_isp(scsi_qla_host_t *vha)
 static int
 qla2x00_restart_isp(scsi_qla_host_t *vha)
 {
-	int status = 0;
+	int status;
 	struct qla_hw_data *ha = vha->hw;
 
 	/* If firmware needs to be loaded */
 	if (qla2x00_isp_firmware(vha)) {
 		vha->flags.online = 0;
 		status = ha->isp_ops->chip_diag(vha);
-		if (!status)
-			status = qla2x00_setup_chip(vha);
+		if (status)
+			return status;
+		status = qla2x00_setup_chip(vha);
+		if (status)
+			return status;
 	}
 
-	if (!status && !(status = qla2x00_init_rings(vha))) {
-		clear_bit(RESET_MARKER_NEEDED, &vha->dpc_flags);
-		ha->flags.chip_reset_done = 1;
+	status = qla2x00_init_rings(vha);
+	if (status)
+		return status;
 
-		/* Initialize the queues in use */
-		qla25xx_init_queues(ha);
+	clear_bit(RESET_MARKER_NEEDED, &vha->dpc_flags);
+	ha->flags.chip_reset_done = 1;
 
-		status = qla2x00_fw_ready(vha);
-		if (!status) {
-			/* Issue a marker after FW becomes ready. */
-			qla2x00_marker(vha, ha->base_qpair, 0, 0, MK_SYNC_ALL);
-			set_bit(LOOP_RESYNC_NEEDED, &vha->dpc_flags);
-		}
+	/* Initialize the queues in use */
+	qla25xx_init_queues(ha);
 
+	status = qla2x00_fw_ready(vha);
+	if (status) {
 		/* if no cable then assume it's good */
-		if ((vha->device_flags & DFLG_NO_CABLE))
-			status = 0;
+		return vha->device_flags & DFLG_NO_CABLE ? 0 : status;
 	}
-	return (status);
+
+	/* Issue a marker after FW becomes ready. */
+	qla2x00_marker(vha, ha->base_qpair, 0, 0, MK_SYNC_ALL);
+	set_bit(LOOP_RESYNC_NEEDED, &vha->dpc_flags);
+
+	return 0;
 }
 
 static int

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 9/9] qla2xxx: Introduce a function for computing the debug message prefix
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
                   ` (7 preceding siblings ...)
  2020-06-29 22:54 ` [PATCH v2 8/9] qla2xxx: Make qla2x00_restart_isp() easier to read Bart Van Assche
@ 2020-06-29 22:54 ` Bart Van Assche
  2020-06-29 23:26 ` [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Himanshu Madhani
  2020-07-01  3:23 ` Martin K. Petersen
  10 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-06-29 22:54 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: linux-scsi, Bart Van Assche, Daniel Wagner, Nilesh Javali,
	Quinn Tran, Himanshu Madhani, Martin Wilck, Roman Bolshakov

Instead of repeating the code for generating a debug message prefix
six times, introduce a function for computing the debug message prefix.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_dbg.c | 96 ++++++++++++----------------------
 1 file changed, 32 insertions(+), 64 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index 41493bd53fc0..911c7852e660 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -2447,6 +2447,23 @@ qla83xx_fw_dump(scsi_qla_host_t *vha)
 /*                         Driver Debug Functions.                          */
 /****************************************************************************/
 
+/* Write the debug message prefix into @pbuf. */
+static void ql_dbg_prefix(char *pbuf, int pbuf_size,
+			  const scsi_qla_host_t *vha, uint msg_id)
+{
+	if (vha) {
+		const struct pci_dev *pdev = vha->hw->pdev;
+
+		/* <module-name> [<dev-name>]-<msg-id>:<host>: */
+		snprintf(pbuf, pbuf_size, "%s [%s]-%04x:%ld: ", QL_MSGHDR,
+			 dev_name(&(pdev->dev)), msg_id, vha->host_no);
+	} else {
+		/* <module-name> [<dev-name>]-<msg-id>: : */
+		snprintf(pbuf, pbuf_size, "%s [%s]-%04x: : ", QL_MSGHDR,
+			 "0000:00:00.0", msg_id);
+	}
+}
+
 /*
  * This function is for formatting and logging debug information.
  * It is to be used when vha is available. It formats the message
@@ -2465,41 +2482,19 @@ ql_dbg(uint level, scsi_qla_host_t *vha, uint id, const char *fmt, ...)
 {
 	va_list va;
 	struct va_format vaf;
+	char pbuf[64];
 
 	va_start(va, fmt);
 
 	vaf.fmt = fmt;
 	vaf.va = &va;
 
-	if (!ql_mask_match(level)) {
-		char pbuf[64];
+	ql_dbg_prefix(pbuf, ARRAY_SIZE(pbuf), vha, id);
 
-		if (vha != NULL) {
-			const struct pci_dev *pdev = vha->hw->pdev;
-			/* <module-name> <msg-id>:<host> Message */
-			snprintf(pbuf, sizeof(pbuf), "%s [%s]-%04x:%ld: ",
-			    QL_MSGHDR, dev_name(&(pdev->dev)), id,
-			    vha->host_no);
-		} else {
-			snprintf(pbuf, sizeof(pbuf), "%s [%s]-%04x: : ",
-			    QL_MSGHDR, "0000:00:00.0", id);
-		}
-		pbuf[sizeof(pbuf) - 1] = 0;
+	if (!ql_mask_match(level))
 		trace_ql_dbg_log(pbuf, &vaf);
-		va_end(va);
-		return;
-	}
-
-	if (vha != NULL) {
-		const struct pci_dev *pdev = vha->hw->pdev;
-		/* <module-name> <pci-name> <msg-id>:<host> Message */
-		pr_warn("%s [%s]-%04x:%ld: %pV",
-			QL_MSGHDR, dev_name(&(pdev->dev)), id + ql_dbg_offset,
-			vha->host_no, &vaf);
-	} else {
-		pr_warn("%s [%s]-%04x: : %pV",
-			QL_MSGHDR, "0000:00:00.0", id + ql_dbg_offset, &vaf);
-	}
+	else
+		pr_warn("%s%pV", pbuf, &vaf);
 
 	va_end(va);
 
@@ -2524,6 +2519,7 @@ ql_dbg_pci(uint level, struct pci_dev *pdev, uint id, const char *fmt, ...)
 {
 	va_list va;
 	struct va_format vaf;
+	char pbuf[128];
 
 	if (pdev == NULL)
 		return;
@@ -2535,9 +2531,8 @@ ql_dbg_pci(uint level, struct pci_dev *pdev, uint id, const char *fmt, ...)
 	vaf.fmt = fmt;
 	vaf.va = &va;
 
-	/* <module-name> <dev-name>:<msg-id> Message */
-	pr_warn("%s [%s]-%04x: : %pV",
-		QL_MSGHDR, dev_name(&(pdev->dev)), id + ql_dbg_offset, &vaf);
+	ql_dbg_prefix(pbuf, ARRAY_SIZE(pbuf), NULL, id + ql_dbg_offset);
+	pr_warn("%s%pV", pbuf, &vaf);
 
 	va_end(va);
 }
@@ -2565,16 +2560,7 @@ ql_log(uint level, scsi_qla_host_t *vha, uint id, const char *fmt, ...)
 	if (level > ql_errlev)
 		return;
 
-	if (vha != NULL) {
-		const struct pci_dev *pdev = vha->hw->pdev;
-		/* <module-name> <msg-id>:<host> Message */
-		snprintf(pbuf, sizeof(pbuf), "%s [%s]-%04x:%ld: ",
-			QL_MSGHDR, dev_name(&(pdev->dev)), id, vha->host_no);
-	} else {
-		snprintf(pbuf, sizeof(pbuf), "%s [%s]-%04x: : ",
-			QL_MSGHDR, "0000:00:00.0", id);
-	}
-	pbuf[sizeof(pbuf) - 1] = 0;
+	ql_dbg_prefix(pbuf, ARRAY_SIZE(pbuf), vha, id);
 
 	va_start(va, fmt);
 
@@ -2625,10 +2611,7 @@ ql_log_pci(uint level, struct pci_dev *pdev, uint id, const char *fmt, ...)
 	if (level > ql_errlev)
 		return;
 
-	/* <module-name> <dev-name>:<msg-id> Message */
-	snprintf(pbuf, sizeof(pbuf), "%s [%s]-%04x: : ",
-		 QL_MSGHDR, dev_name(&(pdev->dev)), id);
-	pbuf[sizeof(pbuf) - 1] = 0;
+	ql_dbg_prefix(pbuf, ARRAY_SIZE(pbuf), NULL, id);
 
 	va_start(va, fmt);
 
@@ -2724,16 +2707,7 @@ ql_log_qp(uint32_t level, struct qla_qpair *qpair, int32_t id,
 	if (level > ql_errlev)
 		return;
 
-	if (qpair != NULL) {
-		const struct pci_dev *pdev = qpair->pdev;
-		/* <module-name> <msg-id>:<host> Message */
-		snprintf(pbuf, sizeof(pbuf), "%s [%s]-%04x: ",
-			QL_MSGHDR, dev_name(&(pdev->dev)), id);
-	} else {
-		snprintf(pbuf, sizeof(pbuf), "%s [%s]-%04x: : ",
-			QL_MSGHDR, "0000:00:00.0", id);
-	}
-	pbuf[sizeof(pbuf) - 1] = 0;
+	ql_dbg_prefix(pbuf, ARRAY_SIZE(pbuf), qpair ? qpair->vha : NULL, id);
 
 	va_start(va, fmt);
 
@@ -2777,6 +2751,7 @@ ql_dbg_qp(uint32_t level, struct qla_qpair *qpair, int32_t id,
 {
 	va_list va;
 	struct va_format vaf;
+	char pbuf[128];
 
 	if (!ql_mask_match(level))
 		return;
@@ -2786,16 +2761,9 @@ ql_dbg_qp(uint32_t level, struct qla_qpair *qpair, int32_t id,
 	vaf.fmt = fmt;
 	vaf.va = &va;
 
-	if (qpair != NULL) {
-		const struct pci_dev *pdev = qpair->pdev;
-		/* <module-name> <pci-name> <msg-id>:<host> Message */
-		pr_warn("%s [%s]-%04x: %pV",
-		    QL_MSGHDR, dev_name(&(pdev->dev)), id + ql_dbg_offset,
-		    &vaf);
-	} else {
-		pr_warn("%s [%s]-%04x: : %pV",
-			QL_MSGHDR, "0000:00:00.0", id + ql_dbg_offset, &vaf);
-	}
+	ql_dbg_prefix(pbuf, ARRAY_SIZE(pbuf), qpair ? qpair->vha : NULL,
+		      id + ql_dbg_offset);
+	pr_warn("%s%pV", pbuf, &vaf);
 
 	va_end(va);
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/9] qla2xxx patches for kernel v5.9
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
                   ` (8 preceding siblings ...)
  2020-06-29 22:54 ` [PATCH v2 9/9] qla2xxx: Introduce a function for computing the debug message prefix Bart Van Assche
@ 2020-06-29 23:26 ` Himanshu Madhani
  2020-07-01  4:28   ` Bart Van Assche
  2020-07-01  3:23 ` Martin K. Petersen
  10 siblings, 1 reply; 14+ messages in thread
From: Himanshu Madhani @ 2020-06-29 23:26 UTC (permalink / raw)
  To: Bart Van Assche; +Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi

Hi Bart, 

> On Jun 29, 2020, at 5:54 PM, Bart Van Assche <bvanassche@acm.org> wrote:
> 
> Hi Martin,
> 
> This patch series includes cleanup patches and also patches that address
> complaints from several static source code analyzers. Please consider these
> patches for kernel v5.9.
> 
> Thanks,
> 
> Bart.
> 
> Changes compared to v1:
> - Rewrote patch "Fix a Coverity complaint in qla2100_fw_dump()"
> 
> Bart Van Assche (9):
>  qla2xxx: Check the size of struct fcp_hdr at compile time
>  qla2xxx: Remove the __packed annotation from struct fcp_hdr and
>    fcp_hdr_le
>  qla2xxx: Make qla82xx_flash_wait_write_finish() easier to read
>  qla2xxx: Initialize 'n' before using it
>  qla2xxx: Remove a superfluous cast
>  qla2xxx: Make __qla2x00_alloc_iocbs() initialize 32 bits of
>    request_t.handle
>  qla2xxx: Fix a Coverity complaint in qla2100_fw_dump()
>  qla2xxx: Make qla2x00_restart_isp() easier to read
>  qla2xxx: Introduce a function for computing the debug message prefix
> 
> drivers/scsi/qla2xxx/qla_bsg.c     |  3 +-
> drivers/scsi/qla2xxx/qla_dbg.c     | 98 ++++++++++--------------------
> drivers/scsi/qla2xxx/qla_dbg.h     |  1 +
> drivers/scsi/qla2xxx/qla_init.c    | 39 ++++++------
> drivers/scsi/qla2xxx/qla_iocb.c    |  4 +-
> drivers/scsi/qla2xxx/qla_nx.c      | 20 +++---
> drivers/scsi/qla2xxx/qla_target.h  |  4 +-
> drivers/scsi/qla2xxx/tcm_qla2xxx.c |  1 +
> 8 files changed, 70 insertions(+), 100 deletions(-)
> 

I had reviewed this series earlier and provided Reviewed-by tag. 

https://lore.kernel.org/linux-scsi/EAC19A51-7256-4D5D-9DBB-D30CEF8551E9@oracle.com/

--
Himanshu Madhani	Oracle Linux Engineering






^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/9] qla2xxx: Remove a superfluous cast
  2020-06-29 22:54 ` [PATCH v2 5/9] qla2xxx: Remove a superfluous cast Bart Van Assche
@ 2020-06-30 17:13   ` Shyam Sundar
  0 siblings, 0 replies; 14+ messages in thread
From: Shyam Sundar @ 2020-06-30 17:13 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
	Daniel Wagner, Nilesh Javali, Quinn Tran, Himanshu Madhani,
	Martin Wilck, Roman Bolshakov

Reviewed-by: Shyam Sundar <ssundar@marvell.com>

> On Jun 29, 2020, at 3:54 PM, Bart Van Assche <bvanassche@acm.org> wrote:
> 
> Remove an unnecessary cast because it prevents the compiler to perform
> type checking.
> 
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Cc: Nilesh Javali <njavali@marvell.com>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
> Cc: Martin Wilck <mwilck@suse.com>
> Cc: Roman Bolshakov <r.bolshakov@yadro.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/qla2xxx/qla_bsg.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> index 88c0338a2ec7..67efde1d4b8e 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> @@ -223,8 +223,7 @@ qla24xx_proc_fcp_prio_cfg_cmd(struct bsg_job *bsg_job)
> 
> 		/* validate fcp priority data */
> 
> -		if (!qla24xx_fcp_prio_cfg_valid(vha,
> -		    (struct qla_fcp_prio_cfg *) ha->fcp_prio_cfg, 1)) {
> +		if (!qla24xx_fcp_prio_cfg_valid(vha, ha->fcp_prio_cfg, 1)) {
> 			bsg_reply->result = (DID_ERROR << 16);
> 			ret = -EINVAL;
> 			/* If buffer was invalidatic int


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/9] qla2xxx patches for kernel v5.9
  2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
                   ` (9 preceding siblings ...)
  2020-06-29 23:26 ` [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Himanshu Madhani
@ 2020-07-01  3:23 ` Martin K. Petersen
  10 siblings, 0 replies; 14+ messages in thread
From: Martin K. Petersen @ 2020-07-01  3:23 UTC (permalink / raw)
  To: James E . J . Bottomley, Bart Van Assche; +Cc: Martin K . Petersen, linux-scsi

On Mon, 29 Jun 2020 15:54:45 -0700, Bart Van Assche wrote:

> This patch series includes cleanup patches and also patches that address
> complaints from several static source code analyzers. Please consider these
> patches for kernel v5.9.
> 
> Thanks,
> 
> Bart.
> 
> [...]

Applied to 5.9/scsi-queue, thanks!

[1/9] scsi: qla2xxx: Check the size of struct fcp_hdr at compile time
      https://git.kernel.org/mkp/scsi/c/a7f474542ea3
[2/9] scsi: qla2xxx: Remove the __packed annotation from struct fcp_hdr and fcp_hdr_le
      https://git.kernel.org/mkp/scsi/c/f1e12bee55e6
[3/9] scsi: qla2xxx: Make qla82xx_flash_wait_write_finish() easier to read
      https://git.kernel.org/mkp/scsi/c/2f91a0a03c2d
[4/9] scsi: qla2xxx: Initialize 'n' before using it
      https://git.kernel.org/mkp/scsi/c/67668b5b13c7
[5/9] scsi: qla2xxx: Remove a superfluous cast
      https://git.kernel.org/mkp/scsi/c/9bb013584a5e
[6/9] scsi: qla2xxx: Make __qla2x00_alloc_iocbs() initialize 32 bits of request_t.handle
      https://git.kernel.org/mkp/scsi/c/f8f12bda53ea
[7/9] scsi: qla2xxx: Fix a Coverity complaint in qla2100_fw_dump()
      https://git.kernel.org/mkp/scsi/c/57fec9f24e58
[8/9] scsi: qla2xxx: Make qla2x00_restart_isp() easier to read
      https://git.kernel.org/mkp/scsi/c/f85a299f5ec5
[9/9] scsi: qla2xxx: Introduce a function for computing the debug message prefix
      https://git.kernel.org/mkp/scsi/c/e7019c95c40d

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/9] qla2xxx patches for kernel v5.9
  2020-06-29 23:26 ` [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Himanshu Madhani
@ 2020-07-01  4:28   ` Bart Van Assche
  0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-07-01  4:28 UTC (permalink / raw)
  To: Himanshu Madhani; +Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi

On 2020-06-29 16:26, Himanshu Madhani wrote:
> I had reviewed this series earlier and provided Reviewed-by tag. 
> 
> https://lore.kernel.org/linux-scsi/EAC19A51-7256-4D5D-9DBB-D30CEF8551E9@oracle.com/

Thank you Himanshu for the reviews. I appreciate this. I promise to
check Reviewed-by tags more carefully in the future when reposting a
patch series.

Bart.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-07-01  4:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-29 22:54 [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Bart Van Assche
2020-06-29 22:54 ` [PATCH v2 1/9] qla2xxx: Check the size of struct fcp_hdr at compile time Bart Van Assche
2020-06-29 22:54 ` [PATCH v2 2/9] qla2xxx: Remove the __packed annotation from struct fcp_hdr and fcp_hdr_le Bart Van Assche
2020-06-29 22:54 ` [PATCH v2 3/9] qla2xxx: Make qla82xx_flash_wait_write_finish() easier to read Bart Van Assche
2020-06-29 22:54 ` [PATCH v2 4/9] qla2xxx: Initialize 'n' before using it Bart Van Assche
2020-06-29 22:54 ` [PATCH v2 5/9] qla2xxx: Remove a superfluous cast Bart Van Assche
2020-06-30 17:13   ` Shyam Sundar
2020-06-29 22:54 ` [PATCH v2 6/9] qla2xxx: Make __qla2x00_alloc_iocbs() initialize 32 bits of request_t.handle Bart Van Assche
2020-06-29 22:54 ` [PATCH v2 7/9] qla2xxx: Fix a Coverity complaint in qla2100_fw_dump() Bart Van Assche
2020-06-29 22:54 ` [PATCH v2 8/9] qla2xxx: Make qla2x00_restart_isp() easier to read Bart Van Assche
2020-06-29 22:54 ` [PATCH v2 9/9] qla2xxx: Introduce a function for computing the debug message prefix Bart Van Assche
2020-06-29 23:26 ` [PATCH v2 0/9] qla2xxx patches for kernel v5.9 Himanshu Madhani
2020-07-01  4:28   ` Bart Van Assche
2020-07-01  3:23 ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).