[PATCH 00/20] SCSI patches for kernel v5.13

[PATCH 00/20] SCSI patches for kernel v5.13

[Bart Van Assche]

Hi Martin,

This patch series includes the following changes:
- Modify a few source code comments.
- Rename scsi_softirq_done().
- Introduce enum scsi_disposition.
- Address CC=clang W=1 warnings.

Please consider these patches for Linux kernel v5.13.

Thanks,

Bart.

Bart Van Assche (20):
  Make the scsi_alloc_sgtables() documentation more accurate
  Remove an incorrect comment
  Rename scsi_softirq_done() into scsi_complete()
  Modify the scsi_send_eh_cmnd() return value for the SDEV_BLOCK case
  Introduce enum scsi_disposition
  aacraid: Remove an unused function
  libfc: Fix a format specifier
  fcoe: Suppress a compiler warning
  iscsi: Suppress two clang format mismatch warnings
  mpt3sas: Fix two kernel-doc headers
  myrb: Remove unused functions
  myrs: Remove unused functions
  qla4xxx: Remove an unused function
  smartpqi: Remove unused functions
  53c700: Open-code status_byte(u8) calls
  dc395x: Open-code status_byte(u8) calls
  sd: Introduce a new local variable in sd_check_events()
  target: Compare explicitly with SAM_STAT_GOOD
  target: Fix several format specifiers
  target/tcm_fc: Fix a kernel-doc header

 drivers/ata/libata-eh.c                    |  2 +-
 drivers/scsi/53c700.c                      |  4 +-
 drivers/scsi/aacraid/aachba.c              |  5 --
 drivers/scsi/dc395x.c                      |  4 +-
 drivers/scsi/device_handler/scsi_dh_alua.c |  4 +-
 drivers/scsi/device_handler/scsi_dh_emc.c  |  4 +-
 drivers/scsi/device_handler/scsi_dh_rdac.c |  4 +-
 drivers/scsi/fcoe/fcoe_transport.c         |  2 +-
 drivers/scsi/libfc/fc_lport.c              |  2 +-
 drivers/scsi/libiscsi.c                    |  5 +-
 drivers/scsi/mpt3sas/mpt3sas_base.c        |  4 +-
 drivers/scsi/myrb.c                        | 71 ----------------
 drivers/scsi/myrs.c                        | 99 ----------------------
 drivers/scsi/qla4xxx/ql4_nx.c              |  6 --
 drivers/scsi/scsi_error.c                  | 66 ++++++++-------
 drivers/scsi/scsi_lib.c                    | 19 +++--
 drivers/scsi/scsi_priv.h                   |  2 +-
 drivers/scsi/sd.c                          |  5 +-
 drivers/scsi/smartpqi/smartpqi_init.c      | 10 ---
 drivers/target/target_core_configfs.c      |  6 +-
 drivers/target/target_core_pr.c            |  6 +-
 drivers/target/target_core_pscsi.c         |  2 +-
 drivers/target/tcm_fc/tfc_sess.c           |  2 +-
 include/scsi/scsi.h                        | 21 ++---
 include/scsi/scsi_device.h                 |  2 +-
 include/scsi/scsi_dh.h                     |  3 +-
 include/scsi/scsi_eh.h                     |  2 +-
 27 files changed, 92 insertions(+), 270 deletions(-)

[PATCH 01/20] Make the scsi_alloc_sgtables() documentation more accurate

[Bart Van Assche]

The current scsi_alloc_sgtables() documentation does not accurately explain
what this function does. Hence improve the documentation of this function.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_lib.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 66e670aedd4c..985ed427445f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -999,8 +999,11 @@ static inline bool scsi_cmd_needs_dma_drain(struct scsi_device *sdev,
 }
 
 /**
- * scsi_alloc_sgtables - allocate S/G tables for a command
- * @cmd:  command descriptor we wish to initialize
+ * scsi_alloc_sgtables - Allocate and initialize data and integrity scatterlists
+ * @cmd: SCSI command data structure to initialize.
+ *
+ * Initializes @cmd->sdb and also @cmd->prot_sdb if data integrity is enabled
+ * for @cmd.
  *
  * Returns:
  * * BLK_STS_OK       - on success

[PATCH 02/20] Remove an incorrect comment

[Bart Van Assche]

scsi_device.sdev_target is used in more code than the single_lun code,
hence remove the comment next to the definition of the sdev_target member.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/scsi/scsi_device.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 05c7c320ef32..ac6ab16abee7 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -146,7 +146,7 @@ struct scsi_device {
 	struct scsi_vpd __rcu *vpd_pg80;
 	struct scsi_vpd __rcu *vpd_pg89;
 	unsigned char current_tag;	/* current tag */
-	struct scsi_target      *sdev_target;   /* used only for single_lun */
+	struct scsi_target      *sdev_target;
 
 	blist_flags_t		sdev_bflags; /* black/white flags as also found in
 				 * scsi_devinfo.[hc]. For now used only to

[PATCH 03/20] Rename scsi_softirq_done() into scsi_complete()

[Bart Van Assche]

Commit 320ae51feed5 ("blk-mq: new multi-queue block IO queueing mechanism";
v3.13) introduced a code path that calls the blk-mq completion function from
interrupt context. scsi-mq was introduced by commit d285203cf647 ("scsi:
add support for a blk-mq based I/O path."; v3.17). Since the introduction of
scsi-mq scsi_softirq_done() can be called from interrupt context. Because
that made the name of that function misleading, rename it into
scsi_complete().

Cc: Christoph Hellwig <hch@lst.de>
Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_lib.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 985ed427445f..c4d6157e8051 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1434,7 +1434,11 @@ static bool scsi_mq_lld_busy(struct request_queue *q)
 	return false;
 }
 
-static void scsi_softirq_done(struct request *rq)
+/*
+ * Block layer request completion callback. May be called from interrupt
+ * context.
+ */
+static void scsi_complete(struct request *rq)
 {
 	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
 	int disposition;
@@ -1889,7 +1893,7 @@ static const struct blk_mq_ops scsi_mq_ops_no_commit = {
 	.get_budget	= scsi_mq_get_budget,
 	.put_budget	= scsi_mq_put_budget,
 	.queue_rq	= scsi_queue_rq,
-	.complete	= scsi_softirq_done,
+	.complete	= scsi_complete,
 	.timeout	= scsi_timeout,
 #ifdef CONFIG_BLK_DEBUG_FS
 	.show_rq	= scsi_show_rq,
@@ -1919,7 +1923,7 @@ static const struct blk_mq_ops scsi_mq_ops = {
 	.put_budget	= scsi_mq_put_budget,
 	.queue_rq	= scsi_queue_rq,
 	.commit_rqs	= scsi_commit_rqs,
-	.complete	= scsi_softirq_done,
+	.complete	= scsi_complete,
 	.timeout	= scsi_timeout,
 #ifdef CONFIG_BLK_DEBUG_FS
 	.show_rq	= scsi_show_rq,

[PATCH 04/20] Modify the scsi_send_eh_cmnd() return value for the SDEV_BLOCK case

[Bart Van Assche]

The comment above scsi_send_eh_cmnd() says: "Returns SUCCESS or FAILED or
NEEDS_RETRY". This patch makes all values returned by scsi_send_eh_cmnd()
match the documentation of this function. This change does not affect the
behavior of scsi_eh_tur() nor of scsi_eh_try_stu() nor of the
scsi_request_sense() callers.

See also commit bbe9fb0d04b9 ("scsi: Avoid that .queuecommand() gets called
for a blocked SCSI device"; v5.3).

Cc: Christoph Hellwig <hch@lst.de>
Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c0a3497fc474..28b287e9f50a 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1112,7 +1112,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
 	if (sdev->sdev_state != SDEV_BLOCK)
 		rtn = shost->hostt->queuecommand(shost, scmd);
 	else
-		rtn = SCSI_MLQUEUE_DEVICE_BUSY;
+		rtn = FAILED;
 	mutex_unlock(&sdev->state_mutex);
 
 	if (rtn) {

[PATCH 05/20] Introduce enum scsi_disposition

[Bart Van Assche]

Improve readability of the code in the SCSI core by introducing an
enumeration type for the values used internally that decide how to
continue processing a SCSI command. The eh_*_handler return values have
not been changed because that would involve modifying all SCSI drivers.

The output of the following command has been inspected to verify that
no out-of-range values are assigned to a variable of type enum
scsi_disposition:

KCFLAGS=-Wassign-enum make CC=clang W=1 drivers/scsi/

Cc: Christoph Hellwig <hch@lst.de>
Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ata/libata-eh.c                    |  2 +-
 drivers/scsi/device_handler/scsi_dh_alua.c |  4 +-
 drivers/scsi/device_handler/scsi_dh_emc.c  |  4 +-
 drivers/scsi/device_handler/scsi_dh_rdac.c |  4 +-
 drivers/scsi/scsi_error.c                  | 64 ++++++++++++----------
 drivers/scsi/scsi_lib.c                    |  2 +-
 drivers/scsi/scsi_priv.h                   |  2 +-
 include/scsi/scsi.h                        | 21 +++----
 include/scsi/scsi_dh.h                     |  3 +-
 include/scsi/scsi_eh.h                     |  2 +-
 10 files changed, 57 insertions(+), 51 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index b6f92050e60c..5352be2b447d 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1599,7 +1599,7 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc,
 	}
 
 	if (qc->flags & ATA_QCFLAG_SENSE_VALID) {
-		int ret = scsi_check_sense(qc->scsicmd);
+		enum scsi_disposition ret = scsi_check_sense(qc->scsicmd);
 		/*
 		 * SUCCESS here means that the sense code could be
 		 * evaluated and should be passed to the upper layers
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index e6fde27d4565..efa8c0381476 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -405,8 +405,8 @@ static char print_alua_state(unsigned char state)
 	}
 }
 
-static int alua_check_sense(struct scsi_device *sdev,
-			    struct scsi_sense_hdr *sense_hdr)
+static enum scsi_disposition alua_check_sense(struct scsi_device *sdev,
+					      struct scsi_sense_hdr *sense_hdr)
 {
 	struct alua_dh_data *h = sdev->handler_data;
 	struct alua_port_group *pg;
diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
index caa685cfe3d4..bd28ec6cfb72 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -280,8 +280,8 @@ static int send_trespass_cmd(struct scsi_device *sdev,
 	return res;
 }
 
-static int clariion_check_sense(struct scsi_device *sdev,
-				struct scsi_sense_hdr *sense_hdr)
+static enum scsi_disposition clariion_check_sense(struct scsi_device *sdev,
+					struct scsi_sense_hdr *sense_hdr)
 {
 	switch (sense_hdr->sense_key) {
 	case NOT_READY:
diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c
index 5efc959493ec..25f6e1ac9e7b 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -656,8 +656,8 @@ static blk_status_t rdac_prep_fn(struct scsi_device *sdev, struct request *req)
 	return BLK_STS_OK;
 }
 
-static int rdac_check_sense(struct scsi_device *sdev,
-				struct scsi_sense_hdr *sense_hdr)
+static enum scsi_disposition rdac_check_sense(struct scsi_device *sdev,
+					      struct scsi_sense_hdr *sense_hdr)
 {
 	struct rdac_dh_data *h = sdev->handler_data;
 
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 28b287e9f50a..9afd65eb2c8b 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -60,8 +60,8 @@ static void scsi_eh_done(struct scsi_cmnd *scmd);
 #define HOST_RESET_SETTLE_TIME  (10)
 
 static int scsi_eh_try_stu(struct scsi_cmnd *scmd);
-static int scsi_try_to_abort_cmd(struct scsi_host_template *,
-				 struct scsi_cmnd *);
+static enum scsi_disposition scsi_try_to_abort_cmd(struct scsi_host_template *,
+						   struct scsi_cmnd *);
 
 void scsi_eh_wakeup(struct Scsi_Host *shost)
 {
@@ -151,7 +151,7 @@ scmd_eh_abort_handler(struct work_struct *work)
 	struct scsi_cmnd *scmd =
 		container_of(work, struct scsi_cmnd, abort_work.work);
 	struct scsi_device *sdev = scmd->device;
-	int rtn;
+	enum scsi_disposition rtn;
 
 	if (scsi_host_eh_past_deadline(sdev->host)) {
 		SCSI_LOG_ERROR_RECOVERY(3,
@@ -498,7 +498,7 @@ static void scsi_report_sense(struct scsi_device *sdev,
  *	When a deferred error is detected the current command has
  *	not been executed and needs retrying.
  */
-int scsi_check_sense(struct scsi_cmnd *scmd)
+enum scsi_disposition scsi_check_sense(struct scsi_cmnd *scmd)
 {
 	struct scsi_device *sdev = scmd->device;
 	struct scsi_sense_hdr sshdr;
@@ -512,7 +512,7 @@ int scsi_check_sense(struct scsi_cmnd *scmd)
 		return NEEDS_RETRY;
 
 	if (sdev->handler && sdev->handler->check_sense) {
-		int rc;
+		enum scsi_disposition rc;
 
 		rc = sdev->handler->check_sense(sdev, &sshdr);
 		if (rc != SCSI_RETURN_NOT_HANDLED)
@@ -726,7 +726,7 @@ static void scsi_handle_queue_full(struct scsi_device *sdev)
  *    don't allow for the possibility of retries here, and we are a lot
  *    more restrictive about what we consider acceptable.
  */
-static int scsi_eh_completed_normally(struct scsi_cmnd *scmd)
+static enum scsi_disposition scsi_eh_completed_normally(struct scsi_cmnd *scmd)
 {
 	/*
 	 * first check the host byte, to see if there is anything in there
@@ -807,10 +807,10 @@ static void scsi_eh_done(struct scsi_cmnd *scmd)
  * scsi_try_host_reset - ask host adapter to reset itself
  * @scmd:	SCSI cmd to send host reset.
  */
-static int scsi_try_host_reset(struct scsi_cmnd *scmd)
+static enum scsi_disposition scsi_try_host_reset(struct scsi_cmnd *scmd)
 {
 	unsigned long flags;
-	int rtn;
+	enum scsi_disposition rtn;
 	struct Scsi_Host *host = scmd->device->host;
 	struct scsi_host_template *hostt = host->hostt;
 
@@ -837,10 +837,10 @@ static int scsi_try_host_reset(struct scsi_cmnd *scmd)
  * scsi_try_bus_reset - ask host to perform a bus reset
  * @scmd:	SCSI cmd to send bus reset.
  */
-static int scsi_try_bus_reset(struct scsi_cmnd *scmd)
+static enum scsi_disposition scsi_try_bus_reset(struct scsi_cmnd *scmd)
 {
 	unsigned long flags;
-	int rtn;
+	enum scsi_disposition rtn;
 	struct Scsi_Host *host = scmd->device->host;
 	struct scsi_host_template *hostt = host->hostt;
 
@@ -879,10 +879,10 @@ static void __scsi_report_device_reset(struct scsi_device *sdev, void *data)
  *    timer on it, and set the host back to a consistent state prior to
  *    returning.
  */
-static int scsi_try_target_reset(struct scsi_cmnd *scmd)
+static enum scsi_disposition scsi_try_target_reset(struct scsi_cmnd *scmd)
 {
 	unsigned long flags;
-	int rtn;
+	enum scsi_disposition rtn;
 	struct Scsi_Host *host = scmd->device->host;
 	struct scsi_host_template *hostt = host->hostt;
 
@@ -910,9 +910,9 @@ static int scsi_try_target_reset(struct scsi_cmnd *scmd)
  *    timer on it, and set the host back to a consistent state prior to
  *    returning.
  */
-static int scsi_try_bus_device_reset(struct scsi_cmnd *scmd)
+static enum scsi_disposition scsi_try_bus_device_reset(struct scsi_cmnd *scmd)
 {
-	int rtn;
+	enum scsi_disposition rtn;
 	struct scsi_host_template *hostt = scmd->device->host->hostt;
 
 	if (!hostt->eh_device_reset_handler)
@@ -941,8 +941,8 @@ static int scsi_try_bus_device_reset(struct scsi_cmnd *scmd)
  *    if the device is temporarily unavailable (eg due to a
  *    link down on FibreChannel)
  */
-static int scsi_try_to_abort_cmd(struct scsi_host_template *hostt,
-				 struct scsi_cmnd *scmd)
+static enum scsi_disposition
+scsi_try_to_abort_cmd(struct scsi_host_template *hostt, struct scsi_cmnd *scmd)
 {
 	if (!hostt->eh_abort_handler)
 		return FAILED;
@@ -1075,8 +1075,8 @@ EXPORT_SYMBOL(scsi_eh_restore_cmnd);
  * Return value:
  *    SUCCESS or FAILED or NEEDS_RETRY
  */
-static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
-			     int cmnd_size, int timeout, unsigned sense_bytes)
+static enum scsi_disposition scsi_send_eh_cmnd(struct scsi_cmnd *scmd,
+	unsigned char *cmnd, int cmnd_size, int timeout, unsigned sense_bytes)
 {
 	struct scsi_device *sdev = scmd->device;
 	struct Scsi_Host *shost = sdev->host;
@@ -1183,12 +1183,13 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, unsigned char *cmnd,
  *    that we obtain it on our own. This function will *not* return until
  *    the command either times out, or it completes.
  */
-static int scsi_request_sense(struct scsi_cmnd *scmd)
+static enum scsi_disposition scsi_request_sense(struct scsi_cmnd *scmd)
 {
 	return scsi_send_eh_cmnd(scmd, NULL, 0, scmd->device->eh_timeout, ~0);
 }
 
-static int scsi_eh_action(struct scsi_cmnd *scmd, int rtn)
+static enum scsi_disposition
+scsi_eh_action(struct scsi_cmnd *scmd, enum scsi_disposition rtn)
 {
 	if (!blk_rq_is_passthrough(scmd->request)) {
 		struct scsi_driver *sdrv = scsi_cmd_to_driver(scmd);
@@ -1241,7 +1242,7 @@ int scsi_eh_get_sense(struct list_head *work_q,
 {
 	struct scsi_cmnd *scmd, *next;
 	struct Scsi_Host *shost;
-	int rtn;
+	enum scsi_disposition rtn;
 
 	/*
 	 * If SCSI_EH_ABORT_SCHEDULED has been set, it is timeout IO,
@@ -1319,7 +1320,8 @@ EXPORT_SYMBOL_GPL(scsi_eh_get_sense);
 static int scsi_eh_tur(struct scsi_cmnd *scmd)
 {
 	static unsigned char tur_command[6] = {TEST_UNIT_READY, 0, 0, 0, 0, 0};
-	int retry_cnt = 1, rtn;
+	int retry_cnt = 1;
+	enum scsi_disposition rtn;
 
 retry_tur:
 	rtn = scsi_send_eh_cmnd(scmd, tur_command, 6,
@@ -1407,7 +1409,8 @@ static int scsi_eh_try_stu(struct scsi_cmnd *scmd)
 	static unsigned char stu_command[6] = {START_STOP, 0, 0, 0, 1, 0};
 
 	if (scmd->device->allow_restart) {
-		int i, rtn = NEEDS_RETRY;
+		int i;
+		enum scsi_disposition rtn = NEEDS_RETRY;
 
 		for (i = 0; rtn == NEEDS_RETRY && i < 2; i++)
 			rtn = scsi_send_eh_cmnd(scmd, stu_command, 6, scmd->device->request_queue->rq_timeout, 0);
@@ -1501,7 +1504,7 @@ static int scsi_eh_bus_device_reset(struct Scsi_Host *shost,
 {
 	struct scsi_cmnd *scmd, *bdr_scmd, *next;
 	struct scsi_device *sdev;
-	int rtn;
+	enum scsi_disposition rtn;
 
 	shost_for_each_device(sdev, shost) {
 		if (scsi_host_eh_past_deadline(shost)) {
@@ -1568,7 +1571,7 @@ static int scsi_eh_target_reset(struct Scsi_Host *shost,
 
 	while (!list_empty(&tmp_list)) {
 		struct scsi_cmnd *next, *scmd;
-		int rtn;
+		enum scsi_disposition rtn;
 		unsigned int id;
 
 		if (scsi_host_eh_past_deadline(shost)) {
@@ -1626,7 +1629,7 @@ static int scsi_eh_bus_reset(struct Scsi_Host *shost,
 	struct scsi_cmnd *scmd, *chan_scmd, *next;
 	LIST_HEAD(check_list);
 	unsigned int channel;
-	int rtn;
+	enum scsi_disposition rtn;
 
 	/*
 	 * we really want to loop over the various channels, and do this on
@@ -1697,7 +1700,7 @@ static int scsi_eh_host_reset(struct Scsi_Host *shost,
 {
 	struct scsi_cmnd *scmd, *next;
 	LIST_HEAD(check_list);
-	int rtn;
+	enum scsi_disposition rtn;
 
 	if (!list_empty(work_q)) {
 		scmd = list_entry(work_q->next,
@@ -1803,9 +1806,9 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd)
  *    doesn't require the error handler read (i.e. we don't need to
  *    abort/reset), this function should return SUCCESS.
  */
-int scsi_decide_disposition(struct scsi_cmnd *scmd)
+enum scsi_disposition scsi_decide_disposition(struct scsi_cmnd *scmd)
 {
-	int rtn;
+	enum scsi_disposition rtn;
 
 	/*
 	 * if the device is offline, then we clearly just pass the result back
@@ -2368,7 +2371,8 @@ scsi_ioctl_reset(struct scsi_device *dev, int __user *arg)
 	struct Scsi_Host *shost = dev->host;
 	struct request *rq;
 	unsigned long flags;
-	int error = 0, rtn, val;
+	int error = 0, val;
+	enum scsi_disposition rtn;
 
 	if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
 		return -EACCES;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c4d6157e8051..a979a9457dff 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1441,7 +1441,7 @@ static bool scsi_mq_lld_busy(struct request_queue *q)
 static void scsi_complete(struct request *rq)
 {
 	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
-	int disposition;
+	enum scsi_disposition disposition;
 
 	INIT_LIST_HEAD(&cmd->eh_entry);
 
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index ed240f006c04..75d6f23e4fff 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -74,7 +74,7 @@ extern void scsi_exit_devinfo(void);
 extern void scmd_eh_abort_handler(struct work_struct *work);
 extern enum blk_eh_timer_return scsi_times_out(struct request *req);
 extern int scsi_error_handler(void *host);
-extern int scsi_decide_disposition(struct scsi_cmnd *cmd);
+extern enum scsi_disposition scsi_decide_disposition(struct scsi_cmnd *cmd);
 extern void scsi_eh_wakeup(struct Scsi_Host *shost);
 extern void scsi_eh_scmd_add(struct scsi_cmnd *);
 void scsi_eh_ready_devs(struct Scsi_Host *shost,
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index e75cca25338a..246ced401683 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -180,16 +180,17 @@ static inline int scsi_is_wlun(u64 lun)
 /*
  * Internal return values.
  */
-
-#define NEEDS_RETRY     0x2001
-#define SUCCESS         0x2002
-#define FAILED          0x2003
-#define QUEUED          0x2004
-#define SOFT_ERROR      0x2005
-#define ADD_TO_MLQUEUE  0x2006
-#define TIMEOUT_ERROR   0x2007
-#define SCSI_RETURN_NOT_HANDLED   0x2008
-#define FAST_IO_FAIL	0x2009
+enum scsi_disposition {
+	NEEDS_RETRY		= 0x2001,
+	SUCCESS			= 0x2002,
+	FAILED			= 0x2003,
+	QUEUED			= 0x2004,
+	SOFT_ERROR		= 0x2005,
+	ADD_TO_MLQUEUE		= 0x2006,
+	TIMEOUT_ERROR		= 0x2007,
+	SCSI_RETURN_NOT_HANDLED	= 0x2008,
+	FAST_IO_FAIL		= 0x2009,
+};
 
 /*
  * Midlevel queue return values.
diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
index a9f782fe732a..4df943c1b90b 100644
--- a/include/scsi/scsi_dh.h
+++ b/include/scsi/scsi_dh.h
@@ -52,7 +52,8 @@ struct scsi_device_handler {
 	/* Filled by the hardware handler */
 	struct module *module;
 	const char *name;
-	int (*check_sense)(struct scsi_device *, struct scsi_sense_hdr *);
+	enum scsi_disposition (*check_sense)(struct scsi_device *,
+					     struct scsi_sense_hdr *);
 	int (*attach)(struct scsi_device *);
 	void (*detach)(struct scsi_device *);
 	int (*activate)(struct scsi_device *, activate_complete, void *);
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 6bd5ed695a5e..468094254b3c 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -17,7 +17,7 @@ extern void scsi_report_device_reset(struct Scsi_Host *, int, int);
 extern int scsi_block_when_processing_errors(struct scsi_device *);
 extern bool scsi_command_normalize_sense(const struct scsi_cmnd *cmd,
 					 struct scsi_sense_hdr *sshdr);
-extern int scsi_check_sense(struct scsi_cmnd *);
+extern enum scsi_disposition scsi_check_sense(struct scsi_cmnd *);
 
 static inline bool scsi_sense_is_deferred(const struct scsi_sense_hdr *sshdr)
 {

[PATCH 06/20] aacraid: Remove an unused function

[Bart Van Assche]

This was detected by building the kernel with clang and W=1.

Cc: aacraid@microsemi.com
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/aacraid/aachba.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 8e06604370c4..f1f62b5da8b7 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -1881,11 +1881,6 @@ static inline u32 aac_get_safw_phys_nexus(struct aac_dev *dev, int lun)
 	return *((u32 *)&dev->safw_phys_luns->lun[lun].node_ident[12]);
 }
 
-static inline u32 aac_get_safw_phys_device_type(struct aac_dev *dev, int lun)
-{
-	return dev->safw_phys_luns->lun[lun].node_ident[8];
-}
-
 static inline void aac_free_safw_identify_resp(struct aac_dev *dev,
 						int bus, int target)
 {

[PATCH 07/20] libfc: Fix a format specifier

[Bart Van Assche]

Since the 'mfs' member has been declared as 'u32' in include/scsi/libfc.h,
use the %u format specifier instead of %hu. This patch fixes the following
clang compiler warning:

warning: format specifies type
      'unsigned short' but the argument has type 'u32' (aka 'unsigned int')
      [-Wformat]
                             "lport->mfs:%hu\n", mfs, lport->mfs);
                                         ~~~          ^~~~~~~~~~
                                         %u

Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/libfc/fc_lport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index 78bd317f0553..cf36c8cb5493 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -1731,7 +1731,7 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp,
 
 	if (mfs < FC_SP_MIN_MAX_PAYLOAD || mfs > FC_SP_MAX_MAX_PAYLOAD) {
 		FC_LPORT_DBG(lport, "FLOGI bad mfs:%hu response, "
-			     "lport->mfs:%hu\n", mfs, lport->mfs);
+			     "lport->mfs:%u\n", mfs, lport->mfs);
 		fc_lport_error(lport, fp);
 		goto out;
 	}

[PATCH 08/20] fcoe: Suppress a compiler warning

[Bart Van Assche]

Suppress the following compiler warning:

warning: cast to smaller integer type
      'enum fip_mode' from 'void *' [-Wvoid-pointer-to-enum-cast]
        enum fip_mode fip_mode = (enum fip_mode)kp->arg;
                                 ^~~~~~~~~~~~~~~~~~~~~~

Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/fcoe/fcoe_transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c
index b927b3d84523..4d0e19e7c84b 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -863,7 +863,7 @@ static int fcoe_transport_create(const char *buffer,
 	int rc = -ENODEV;
 	struct net_device *netdev = NULL;
 	struct fcoe_transport *ft = NULL;
-	enum fip_mode fip_mode = (enum fip_mode)kp->arg;
+	enum fip_mode fip_mode = (enum fip_mode)(uintptr_t)kp->arg;
 
 	mutex_lock(&ft_mutex);
 

[PATCH 09/20] iscsi: Suppress two clang format mismatch warnings

[Bart Van Assche]

Suppress two instances of the following clang compiler warning:

warning: format specifies type 'unsigned short'
      but the argument has type 'int' [-Wformat]

Cc: Lee Duncan <lduncan@suse.com>
Cc: Chris Leech <cleech@redhat.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/libiscsi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 7ad11e42306d..0c3082d09712 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -3587,10 +3587,11 @@ int iscsi_conn_get_addr_param(struct sockaddr_storage *addr,
 	case ISCSI_PARAM_CONN_PORT:
 	case ISCSI_PARAM_LOCAL_PORT:
 		if (sin)
-			len = sprintf(buf, "%hu\n", be16_to_cpu(sin->sin_port));
+			len = sprintf(buf, "%hu\n",
+				      (u16)be16_to_cpu(sin->sin_port));
 		else
 			len = sprintf(buf, "%hu\n",
-				      be16_to_cpu(sin6->sin6_port));
+				      (u16)be16_to_cpu(sin6->sin6_port));
 		break;
 	default:
 		return -EINVAL;

[PATCH 10/20] mpt3sas: Fix two kernel-doc headers

[Bart Van Assche]

Fix the following warnings:

drivers/scsi/mpt3sas/mpt3sas_base.c:5430: warning: Excess function parameter 'ct' description in '_base_allocate_pcie_sgl_pool'
drivers/scsi/mpt3sas/mpt3sas_base.c:5493: warning: Excess function parameter 'ctr' description in '_base_allocate_chain_dma_pool'

Cc: Sathya Prakash <sathya.prakash@broadcom.com>
Cc: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
Cc: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>
Fixes: d6adc251dd2f ("scsi: mpt3sas: Force PCIe scatterlist allocations to be within same 4 GB region")
Fixes: 7dd847dae1c4 ("scsi: mpt3sas: Force chain buffer allocations to be within same 4 GB region")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 84c507587166..5779f313f6f8 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -5421,7 +5421,7 @@ _base_reduce_hba_queue_depth(struct MPT3SAS_ADAPTER *ioc)
  *			for pcie sgl pools.
  * @ioc: Adapter object
  * @sz: DMA Pool size
- * @ct: Chain tracker
+ *
  * Return: 0 for success, non-zero for failure.
  */
 
@@ -5485,7 +5485,7 @@ _base_allocate_pcie_sgl_pool(struct MPT3SAS_ADAPTER *ioc, u32 sz)
  *			for chain dma pool.
  * @ioc: Adapter object
  * @sz: DMA Pool size
- * @ctr: Chain tracker
+ *
  * Return: 0 for success, non-zero for failure.
  */
 static int

[PATCH 11/20] myrb: Remove unused functions

[Bart Van Assche]

This was detected by building the kernel with clang and W=1.

Cc: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/myrb.c | 71 ---------------------------------------------
 1 file changed, 71 deletions(-)

diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
index 56767f8610d4..d9c82e211ae7 100644
--- a/drivers/scsi/myrb.c
+++ b/drivers/scsi/myrb.c
@@ -2552,11 +2552,6 @@ static inline void DAC960_LA_ack_hw_mbox_status(void __iomem *base)
 	writeb(DAC960_LA_IDB_HWMBOX_ACK_STS, base + DAC960_LA_IDB_OFFSET);
 }
 
-static inline void DAC960_LA_gen_intr(void __iomem *base)
-{
-	writeb(DAC960_LA_IDB_GEN_IRQ, base + DAC960_LA_IDB_OFFSET);
-}
-
 static inline void DAC960_LA_reset_ctrl(void __iomem *base)
 {
 	writeb(DAC960_LA_IDB_CTRL_RESET, base + DAC960_LA_IDB_OFFSET);
@@ -2586,11 +2581,6 @@ static inline void DAC960_LA_ack_hw_mbox_intr(void __iomem *base)
 	writeb(DAC960_LA_ODB_HWMBOX_ACK_IRQ, base + DAC960_LA_ODB_OFFSET);
 }
 
-static inline void DAC960_LA_ack_mem_mbox_intr(void __iomem *base)
-{
-	writeb(DAC960_LA_ODB_MMBOX_ACK_IRQ, base + DAC960_LA_ODB_OFFSET);
-}
-
 static inline void DAC960_LA_ack_intr(void __iomem *base)
 {
 	writeb(DAC960_LA_ODB_HWMBOX_ACK_IRQ | DAC960_LA_ODB_MMBOX_ACK_IRQ,
@@ -2604,13 +2594,6 @@ static inline bool DAC960_LA_hw_mbox_status_available(void __iomem *base)
 	return odb & DAC960_LA_ODB_HWMBOX_STS_AVAIL;
 }
 
-static inline bool DAC960_LA_mem_mbox_status_available(void __iomem *base)
-{
-	unsigned char odb = readb(base + DAC960_LA_ODB_OFFSET);
-
-	return odb & DAC960_LA_ODB_MMBOX_STS_AVAIL;
-}
-
 static inline void DAC960_LA_enable_intr(void __iomem *base)
 {
 	unsigned char odb = 0xFF;
@@ -2627,13 +2610,6 @@ static inline void DAC960_LA_disable_intr(void __iomem *base)
 	writeb(odb, base + DAC960_LA_IRQMASK_OFFSET);
 }
 
-static inline bool DAC960_LA_intr_enabled(void __iomem *base)
-{
-	unsigned char imask = readb(base + DAC960_LA_IRQMASK_OFFSET);
-
-	return !(imask & DAC960_LA_IRQMASK_DISABLE_IRQ);
-}
-
 static inline void DAC960_LA_write_cmd_mbox(union myrb_cmd_mbox *mem_mbox,
 		union myrb_cmd_mbox *mbox)
 {
@@ -2656,11 +2632,6 @@ static inline void DAC960_LA_write_hw_mbox(void __iomem *base,
 	writeb(mbox->bytes[12], base + DAC960_LA_MBOX12_OFFSET);
 }
 
-static inline unsigned char DAC960_LA_read_status_cmd_ident(void __iomem *base)
-{
-	return readb(base + DAC960_LA_STSID_OFFSET);
-}
-
 static inline unsigned short DAC960_LA_read_status(void __iomem *base)
 {
 	return readw(base + DAC960_LA_STS_OFFSET);
@@ -2828,11 +2799,6 @@ static inline void DAC960_PG_ack_hw_mbox_status(void __iomem *base)
 	writel(DAC960_PG_IDB_HWMBOX_ACK_STS, base + DAC960_PG_IDB_OFFSET);
 }
 
-static inline void DAC960_PG_gen_intr(void __iomem *base)
-{
-	writel(DAC960_PG_IDB_GEN_IRQ, base + DAC960_PG_IDB_OFFSET);
-}
-
 static inline void DAC960_PG_reset_ctrl(void __iomem *base)
 {
 	writel(DAC960_PG_IDB_CTRL_RESET, base + DAC960_PG_IDB_OFFSET);
@@ -2862,11 +2828,6 @@ static inline void DAC960_PG_ack_hw_mbox_intr(void __iomem *base)
 	writel(DAC960_PG_ODB_HWMBOX_ACK_IRQ, base + DAC960_PG_ODB_OFFSET);
 }
 
-static inline void DAC960_PG_ack_mem_mbox_intr(void __iomem *base)
-{
-	writel(DAC960_PG_ODB_MMBOX_ACK_IRQ, base + DAC960_PG_ODB_OFFSET);
-}
-
 static inline void DAC960_PG_ack_intr(void __iomem *base)
 {
 	writel(DAC960_PG_ODB_HWMBOX_ACK_IRQ | DAC960_PG_ODB_MMBOX_ACK_IRQ,
@@ -2880,13 +2841,6 @@ static inline bool DAC960_PG_hw_mbox_status_available(void __iomem *base)
 	return odb & DAC960_PG_ODB_HWMBOX_STS_AVAIL;
 }
 
-static inline bool DAC960_PG_mem_mbox_status_available(void __iomem *base)
-{
-	unsigned char odb = readl(base + DAC960_PG_ODB_OFFSET);
-
-	return odb & DAC960_PG_ODB_MMBOX_STS_AVAIL;
-}
-
 static inline void DAC960_PG_enable_intr(void __iomem *base)
 {
 	unsigned int imask = (unsigned int)-1;
@@ -2902,13 +2856,6 @@ static inline void DAC960_PG_disable_intr(void __iomem *base)
 	writel(imask, base + DAC960_PG_IRQMASK_OFFSET);
 }
 
-static inline bool DAC960_PG_intr_enabled(void __iomem *base)
-{
-	unsigned int imask = readl(base + DAC960_PG_IRQMASK_OFFSET);
-
-	return !(imask & DAC960_PG_IRQMASK_DISABLE_IRQ);
-}
-
 static inline void DAC960_PG_write_cmd_mbox(union myrb_cmd_mbox *mem_mbox,
 		union myrb_cmd_mbox *mbox)
 {
@@ -2931,12 +2878,6 @@ static inline void DAC960_PG_write_hw_mbox(void __iomem *base,
 	writeb(mbox->bytes[12], base + DAC960_PG_MBOX12_OFFSET);
 }
 
-static inline unsigned char
-DAC960_PG_read_status_cmd_ident(void __iomem *base)
-{
-	return readb(base + DAC960_PG_STSID_OFFSET);
-}
-
 static inline unsigned short
 DAC960_PG_read_status(void __iomem *base)
 {
@@ -3106,11 +3047,6 @@ static inline void DAC960_PD_ack_hw_mbox_status(void __iomem *base)
 	writeb(DAC960_PD_IDB_HWMBOX_ACK_STS, base + DAC960_PD_IDB_OFFSET);
 }
 
-static inline void DAC960_PD_gen_intr(void __iomem *base)
-{
-	writeb(DAC960_PD_IDB_GEN_IRQ, base + DAC960_PD_IDB_OFFSET);
-}
-
 static inline void DAC960_PD_reset_ctrl(void __iomem *base)
 {
 	writeb(DAC960_PD_IDB_CTRL_RESET, base + DAC960_PD_IDB_OFFSET);
@@ -3152,13 +3088,6 @@ static inline void DAC960_PD_disable_intr(void __iomem *base)
 	writeb(0, base + DAC960_PD_IRQEN_OFFSET);
 }
 
-static inline bool DAC960_PD_intr_enabled(void __iomem *base)
-{
-	unsigned char imask = readb(base + DAC960_PD_IRQEN_OFFSET);
-
-	return imask & DAC960_PD_IRQMASK_ENABLE_IRQ;
-}
-
 static inline void DAC960_PD_write_cmd_mbox(void __iomem *base,
 		union myrb_cmd_mbox *mbox)
 {

[PATCH 12/20] myrs: Remove unused functions

[Bart Van Assche]

This was detected by building the kernel with clang and W=1.

Cc: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/myrs.c | 99 ---------------------------------------------
 1 file changed, 99 deletions(-)

diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c
index d5ec1cdea0e1..3b68c68d1716 100644
--- a/drivers/scsi/myrs.c
+++ b/drivers/scsi/myrs.c
@@ -2410,13 +2410,6 @@ static inline void DAC960_GEM_ack_hw_mbox_status(void __iomem *base)
 	writel(val, base + DAC960_GEM_IDB_CLEAR_OFFSET);
 }
 
-static inline void DAC960_GEM_gen_intr(void __iomem *base)
-{
-	__le32 val = cpu_to_le32(DAC960_GEM_IDB_GEN_IRQ << 24);
-
-	writel(val, base + DAC960_GEM_IDB_READ_OFFSET);
-}
-
 static inline void DAC960_GEM_reset_ctrl(void __iomem *base)
 {
 	__le32 val = cpu_to_le32(DAC960_GEM_IDB_CTRL_RESET << 24);
@@ -2454,13 +2447,6 @@ static inline void DAC960_GEM_ack_hw_mbox_intr(void __iomem *base)
 	writel(val, base + DAC960_GEM_ODB_CLEAR_OFFSET);
 }
 
-static inline void DAC960_GEM_ack_mem_mbox_intr(void __iomem *base)
-{
-	__le32 val = cpu_to_le32(DAC960_GEM_ODB_MMBOX_ACK_IRQ << 24);
-
-	writel(val, base + DAC960_GEM_ODB_CLEAR_OFFSET);
-}
-
 static inline void DAC960_GEM_ack_intr(void __iomem *base)
 {
 	__le32 val = cpu_to_le32((DAC960_GEM_ODB_HWMBOX_ACK_IRQ |
@@ -2477,14 +2463,6 @@ static inline bool DAC960_GEM_hw_mbox_status_available(void __iomem *base)
 	return (le32_to_cpu(val) >> 24) & DAC960_GEM_ODB_HWMBOX_STS_AVAIL;
 }
 
-static inline bool DAC960_GEM_mem_mbox_status_available(void __iomem *base)
-{
-	__le32 val;
-
-	val = readl(base + DAC960_GEM_ODB_READ_OFFSET);
-	return (le32_to_cpu(val) >> 24) & DAC960_GEM_ODB_MMBOX_STS_AVAIL;
-}
-
 static inline void DAC960_GEM_enable_intr(void __iomem *base)
 {
 	__le32 val = cpu_to_le32((DAC960_GEM_IRQMASK_HWMBOX_IRQ |
@@ -2499,16 +2477,6 @@ static inline void DAC960_GEM_disable_intr(void __iomem *base)
 	writel(val, base + DAC960_GEM_IRQMASK_READ_OFFSET);
 }
 
-static inline bool DAC960_GEM_intr_enabled(void __iomem *base)
-{
-	__le32 val;
-
-	val = readl(base + DAC960_GEM_IRQMASK_READ_OFFSET);
-	return !((le32_to_cpu(val) >> 24) &
-		 (DAC960_GEM_IRQMASK_HWMBOX_IRQ |
-		  DAC960_GEM_IRQMASK_MMBOX_IRQ));
-}
-
 static inline void DAC960_GEM_write_cmd_mbox(union myrs_cmd_mbox *mem_mbox,
 		union myrs_cmd_mbox *mbox)
 {
@@ -2527,11 +2495,6 @@ static inline void DAC960_GEM_write_hw_mbox(void __iomem *base,
 	dma_addr_writeql(cmd_mbox_addr, base + DAC960_GEM_CMDMBX_OFFSET);
 }
 
-static inline unsigned short DAC960_GEM_read_cmd_ident(void __iomem *base)
-{
-	return readw(base + DAC960_GEM_CMDSTS_OFFSET);
-}
-
 static inline unsigned char DAC960_GEM_read_cmd_status(void __iomem *base)
 {
 	return readw(base + DAC960_GEM_CMDSTS_OFFSET + 2);
@@ -2676,11 +2639,6 @@ static inline void DAC960_BA_ack_hw_mbox_status(void __iomem *base)
 	writeb(DAC960_BA_IDB_HWMBOX_ACK_STS, base + DAC960_BA_IDB_OFFSET);
 }
 
-static inline void DAC960_BA_gen_intr(void __iomem *base)
-{
-	writeb(DAC960_BA_IDB_GEN_IRQ, base + DAC960_BA_IDB_OFFSET);
-}
-
 static inline void DAC960_BA_reset_ctrl(void __iomem *base)
 {
 	writeb(DAC960_BA_IDB_CTRL_RESET, base + DAC960_BA_IDB_OFFSET);
@@ -2712,11 +2670,6 @@ static inline void DAC960_BA_ack_hw_mbox_intr(void __iomem *base)
 	writeb(DAC960_BA_ODB_HWMBOX_ACK_IRQ, base + DAC960_BA_ODB_OFFSET);
 }
 
-static inline void DAC960_BA_ack_mem_mbox_intr(void __iomem *base)
-{
-	writeb(DAC960_BA_ODB_MMBOX_ACK_IRQ, base + DAC960_BA_ODB_OFFSET);
-}
-
 static inline void DAC960_BA_ack_intr(void __iomem *base)
 {
 	writeb(DAC960_BA_ODB_HWMBOX_ACK_IRQ | DAC960_BA_ODB_MMBOX_ACK_IRQ,
@@ -2731,14 +2684,6 @@ static inline bool DAC960_BA_hw_mbox_status_available(void __iomem *base)
 	return val & DAC960_BA_ODB_HWMBOX_STS_AVAIL;
 }
 
-static inline bool DAC960_BA_mem_mbox_status_available(void __iomem *base)
-{
-	u8 val;
-
-	val = readb(base + DAC960_BA_ODB_OFFSET);
-	return val & DAC960_BA_ODB_MMBOX_STS_AVAIL;
-}
-
 static inline void DAC960_BA_enable_intr(void __iomem *base)
 {
 	writeb(~DAC960_BA_IRQMASK_DISABLE_IRQ, base + DAC960_BA_IRQMASK_OFFSET);
@@ -2749,14 +2694,6 @@ static inline void DAC960_BA_disable_intr(void __iomem *base)
 	writeb(0xFF, base + DAC960_BA_IRQMASK_OFFSET);
 }
 
-static inline bool DAC960_BA_intr_enabled(void __iomem *base)
-{
-	u8 val;
-
-	val = readb(base + DAC960_BA_IRQMASK_OFFSET);
-	return !(val & DAC960_BA_IRQMASK_DISABLE_IRQ);
-}
-
 static inline void DAC960_BA_write_cmd_mbox(union myrs_cmd_mbox *mem_mbox,
 		union myrs_cmd_mbox *mbox)
 {
@@ -2776,11 +2713,6 @@ static inline void DAC960_BA_write_hw_mbox(void __iomem *base,
 	dma_addr_writeql(cmd_mbox_addr, base + DAC960_BA_CMDMBX_OFFSET);
 }
 
-static inline unsigned short DAC960_BA_read_cmd_ident(void __iomem *base)
-{
-	return readw(base + DAC960_BA_CMDSTS_OFFSET);
-}
-
 static inline unsigned char DAC960_BA_read_cmd_status(void __iomem *base)
 {
 	return readw(base + DAC960_BA_CMDSTS_OFFSET + 2);
@@ -2926,11 +2858,6 @@ static inline void DAC960_LP_ack_hw_mbox_status(void __iomem *base)
 	writeb(DAC960_LP_IDB_HWMBOX_ACK_STS, base + DAC960_LP_IDB_OFFSET);
 }
 
-static inline void DAC960_LP_gen_intr(void __iomem *base)
-{
-	writeb(DAC960_LP_IDB_GEN_IRQ, base + DAC960_LP_IDB_OFFSET);
-}
-
 static inline void DAC960_LP_reset_ctrl(void __iomem *base)
 {
 	writeb(DAC960_LP_IDB_CTRL_RESET, base + DAC960_LP_IDB_OFFSET);
@@ -2962,11 +2889,6 @@ static inline void DAC960_LP_ack_hw_mbox_intr(void __iomem *base)
 	writeb(DAC960_LP_ODB_HWMBOX_ACK_IRQ, base + DAC960_LP_ODB_OFFSET);
 }
 
-static inline void DAC960_LP_ack_mem_mbox_intr(void __iomem *base)
-{
-	writeb(DAC960_LP_ODB_MMBOX_ACK_IRQ, base + DAC960_LP_ODB_OFFSET);
-}
-
 static inline void DAC960_LP_ack_intr(void __iomem *base)
 {
 	writeb(DAC960_LP_ODB_HWMBOX_ACK_IRQ | DAC960_LP_ODB_MMBOX_ACK_IRQ,
@@ -2981,14 +2903,6 @@ static inline bool DAC960_LP_hw_mbox_status_available(void __iomem *base)
 	return val & DAC960_LP_ODB_HWMBOX_STS_AVAIL;
 }
 
-static inline bool DAC960_LP_mem_mbox_status_available(void __iomem *base)
-{
-	u8 val;
-
-	val = readb(base + DAC960_LP_ODB_OFFSET);
-	return val & DAC960_LP_ODB_MMBOX_STS_AVAIL;
-}
-
 static inline void DAC960_LP_enable_intr(void __iomem *base)
 {
 	writeb(~DAC960_LP_IRQMASK_DISABLE_IRQ, base + DAC960_LP_IRQMASK_OFFSET);
@@ -2999,14 +2913,6 @@ static inline void DAC960_LP_disable_intr(void __iomem *base)
 	writeb(0xFF, base + DAC960_LP_IRQMASK_OFFSET);
 }
 
-static inline bool DAC960_LP_intr_enabled(void __iomem *base)
-{
-	u8 val;
-
-	val = readb(base + DAC960_LP_IRQMASK_OFFSET);
-	return !(val & DAC960_LP_IRQMASK_DISABLE_IRQ);
-}
-
 static inline void DAC960_LP_write_cmd_mbox(union myrs_cmd_mbox *mem_mbox,
 		union myrs_cmd_mbox *mbox)
 {
@@ -3025,11 +2931,6 @@ static inline void DAC960_LP_write_hw_mbox(void __iomem *base,
 	dma_addr_writeql(cmd_mbox_addr, base + DAC960_LP_CMDMBX_OFFSET);
 }
 
-static inline unsigned short DAC960_LP_read_cmd_ident(void __iomem *base)
-{
-	return readw(base + DAC960_LP_CMDSTS_OFFSET);
-}
-
 static inline unsigned char DAC960_LP_read_cmd_status(void __iomem *base)
 {
 	return readw(base + DAC960_LP_CMDSTS_OFFSET + 2);

[PATCH 13/20] qla4xxx: Remove an unused function

[Bart Van Assche]

This was detected by building the kernel with clang and W=1.

Cc: Nilesh Javali <njavali@marvell.com>
Cc: Manish Rangankar <mrangankar@marvell.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla4xxx/ql4_nx.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c
index e6e35e6958f6..66a487795c53 100644
--- a/drivers/scsi/qla4xxx/ql4_nx.c
+++ b/drivers/scsi/qla4xxx/ql4_nx.c
@@ -3634,12 +3634,6 @@ flash_conf_addr(struct ql82xx_hw_data *hw, uint32_t faddr)
 	return hw->flash_conf_off | faddr;
 }
 
-static inline uint32_t
-flash_data_addr(struct ql82xx_hw_data *hw, uint32_t faddr)
-{
-	return hw->flash_data_off | faddr;
-}
-
 static uint32_t *
 qla4_82xx_read_flash_data(struct scsi_qla_host *ha, uint32_t *dwptr,
     uint32_t faddr, uint32_t length)

[PATCH 14/20] smartpqi: Remove unused functions

[Bart Van Assche]

This was detected by building the kernel with clang and W=1.

Cc: Don Brace <don.brace@microchip.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/smartpqi/smartpqi_init.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index 3b0f281daa2b..80d5c00379ee 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -443,11 +443,6 @@ static inline void pqi_cancel_rescan_worker(struct pqi_ctrl_info *ctrl_info)
 	cancel_delayed_work_sync(&ctrl_info->rescan_work);
 }
 
-static inline void pqi_cancel_event_worker(struct pqi_ctrl_info *ctrl_info)
-{
-	cancel_work_sync(&ctrl_info->event_work);
-}
-
 static inline u32 pqi_read_heartbeat_counter(struct pqi_ctrl_info *ctrl_info)
 {
 	if (!ctrl_info->heartbeat_counter)
@@ -4828,11 +4823,6 @@ static inline int pqi_enable_events(struct pqi_ctrl_info *ctrl_info)
 	return pqi_configure_events(ctrl_info, true);
 }
 
-static inline int pqi_disable_events(struct pqi_ctrl_info *ctrl_info)
-{
-	return pqi_configure_events(ctrl_info, false);
-}
-
 static void pqi_free_all_io_requests(struct pqi_ctrl_info *ctrl_info)
 {
 	unsigned int i;

[PATCH 15/20] 53c700: Open-code status_byte(u8) calls

[Bart Van Assche]

The 53c700 driver is one of the two drivers that passes an u8 argument
to status_byte() instead of an s32 argument. Open-code status_byte in
preparation of changing SCSI status values into a structure.

Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/53c700.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index 4fd91f81244d..ab42feab233f 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -981,8 +981,8 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp,
 						  NCR_700_FINISHED_TAG_NEGOTIATION);
 			
 		/* check for contingent allegiance conditions */
-		if(status_byte(hostdata->status[0]) == CHECK_CONDITION ||
-		   status_byte(hostdata->status[0]) == COMMAND_TERMINATED) {
+		if (hostdata->status[0] >> 1 == CHECK_CONDITION ||
+		    hostdata->status[0] >> 1 == COMMAND_TERMINATED) {
 			struct NCR_700_command_slot *slot =
 				(struct NCR_700_command_slot *)SCp->host_scribble;
 			if(slot->flags == NCR_700_FLAG_AUTOSENSE) {

[PATCH 16/20] dc395x: Open-code status_byte(u8) calls

[Bart Van Assche]

The dc395x driver is one of the two drivers that passes an u8 argument
to status_byte() instead of an s32 argument. Open-code status_byte() in
preparation of changing SCSI status values into a structure.

Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/dc395x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index 1e9ec4d8c605..be87d5a7583d 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -3258,10 +3258,10 @@ static void srb_done(struct AdapterCtlBlk *acb, struct DeviceCtlBlk *dcb,
 		/*
 		 * target status..........................
 		 */
-		if (status_byte(status) == CHECK_CONDITION) {
+		if (status >> 1 == CHECK_CONDITION) {
 			request_sense(acb, dcb, srb);
 			return;
-		} else if (status_byte(status) == QUEUE_FULL) {
+		} else if (status >> 1 == QUEUE_FULL) {
 			tempcnt = (u8)list_size(&dcb->srb_going_list);
 			dprintkl(KERN_INFO, "QUEUE_FULL for dev <%02i-%i> with %i cmnds\n",
 			     dcb->target_id, dcb->target_lun, tempcnt);

[PATCH 17/20] sd: Introduce a new local variable in sd_check_events()

[Bart Van Assche]

Instead of using 'retval' to represent first a SCSI status and later
whether or not a disk change event occurred, introduce a new variable for
the latter purpose.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/sd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 91c34ee972c7..cb3c37d1e009 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1623,6 +1623,7 @@ static unsigned int sd_check_events(struct gendisk *disk, unsigned int clearing)
 	struct scsi_disk *sdkp = scsi_disk_get(disk);
 	struct scsi_device *sdp;
 	int retval;
+	bool disk_changed;
 
 	if (!sdkp)
 		return 0;
@@ -1680,10 +1681,10 @@ static unsigned int sd_check_events(struct gendisk *disk, unsigned int clearing)
 	 *	Medium present state has changed in either direction.
 	 *	Device has indicated UNIT_ATTENTION.
 	 */
-	retval = sdp->changed ? DISK_EVENT_MEDIA_CHANGE : 0;
+	disk_changed = sdp->changed;
 	sdp->changed = 0;
 	scsi_disk_put(sdkp);
-	return retval;
+	return disk_changed ? DISK_EVENT_MEDIA_CHANGE : 0;
 }
 
 static int sd_sync_cache(struct scsi_disk *sdkp, struct scsi_sense_hdr *sshdr)

[PATCH 18/20] target: Compare explicitly with SAM_STAT_GOOD

[Bart Van Assche]

Instead of leaving it implicit that SAM_STAT_GOOD == 0, compare explicitly
with SAM_STAT_GOOD.

Cc: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/target/target_core_pscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 1c9aeab93477..dac44caf77a3 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -1046,7 +1046,7 @@ static void pscsi_req_done(struct request *req, blk_status_t status)
 	int result = scsi_req(req)->result;
 	u8 scsi_status = status_byte(result) << 1;
 
-	if (scsi_status) {
+	if (scsi_status != SAM_STAT_GOOD) {
 		pr_debug("PSCSI Status Byte exception at cmd: %p CDB:"
 			" 0x%02x Result: 0x%08x\n", cmd, pt->pscsi_cdb[0],
 			result);

[PATCH 19/20] target: Fix several format specifiers

[Bart Van Assche]

Use format specifier '%u' to format the u32 and int data types instead of
'%hu'.

Cc: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/target/target_core_configfs.c | 6 +++---
 drivers/target/target_core_pr.c       | 6 ++----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 9cb1ca8421c8..01005a9e5128 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -2746,7 +2746,7 @@ static ssize_t target_tg_pt_gp_alua_access_state_store(struct config_item *item,
 
 	if (!tg_pt_gp->tg_pt_gp_valid_id) {
 		pr_err("Unable to do implicit ALUA on non valid"
-			" tg_pt_gp ID: %hu\n", tg_pt_gp->tg_pt_gp_valid_id);
+			" tg_pt_gp ID: %u\n", tg_pt_gp->tg_pt_gp_valid_id);
 		return -EINVAL;
 	}
 	if (!target_dev_configured(dev)) {
@@ -2798,7 +2798,7 @@ static ssize_t target_tg_pt_gp_alua_access_status_store(
 
 	if (!tg_pt_gp->tg_pt_gp_valid_id) {
 		pr_err("Unable to do set ALUA access status on non"
-			" valid tg_pt_gp ID: %hu\n",
+			" valid tg_pt_gp ID: %u\n",
 			tg_pt_gp->tg_pt_gp_valid_id);
 		return -EINVAL;
 	}
@@ -2853,7 +2853,7 @@ static ssize_t target_tg_pt_gp_alua_support_##_name##_store(		\
 									\
 	if (!t->tg_pt_gp_valid_id) {					\
 		pr_err("Unable to do set " #_name " ALUA state on non"	\
-		       " valid tg_pt_gp ID: %hu\n",			\
+		       " valid tg_pt_gp ID: %u\n",			\
 		       t->tg_pt_gp_valid_id);				\
 		return -EINVAL;						\
 	}								\
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index d61dc166bc5f..6fd5fec95539 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -1637,8 +1637,7 @@ core_scsi3_decode_spec_i_port(
 			}
 
 			dest_tpg = tmp_tpg;
-			pr_debug("SPC-3 PR SPEC_I_PT: Located %s Node:"
-				" %s Port RTPI: %hu\n",
+			pr_debug("SPC-3 PR SPEC_I_PT: Located %s Node: %s Port RTPI: %u\n",
 				dest_tpg->se_tpg_tfo->fabric_name,
 				dest_node_acl->initiatorname, dest_rtpi);
 
@@ -1675,8 +1674,7 @@ core_scsi3_decode_spec_i_port(
 		dest_se_deve = core_get_se_deve_from_rtpi(dest_node_acl,
 					dest_rtpi);
 		if (!dest_se_deve) {
-			pr_err("Unable to locate %s dest_se_deve"
-				" from destination RTPI: %hu\n",
+			pr_err("Unable to locate %s dest_se_deve from destination RTPI: %u\n",
 				dest_tpg->se_tpg_tfo->fabric_name,
 				dest_rtpi);
 

[PATCH 20/20] target/tcm_fc: Fix a kernel-doc header

[Bart Van Assche]

Fix the function name in the kernel-doc header above ft_prli().

Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/target/tcm_fc/tfc_sess.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c
index 23ce506d5402..593540da9346 100644
--- a/drivers/target/tcm_fc/tfc_sess.c
+++ b/drivers/target/tcm_fc/tfc_sess.c
@@ -410,7 +410,7 @@ static int ft_prli_locked(struct fc_rport_priv *rdata, u32 spp_len,
 }
 
 /**
- * tcm_fcp_prli() - Handle incoming or outgoing PRLI for the FCP target
+ * ft_prli() - Handle incoming or outgoing PRLI for the FCP target
  * @rdata: remote port private
  * @spp_len: service parameter page length
  * @rspp: received service parameter page (NULL for outgoing PRLI)

Re: [PATCH 18/20] target: Compare explicitly with SAM_STAT_GOOD

[Mike Christie]

On 4/13/21 12:07 PM, Bart Van Assche wrote:
> Instead of leaving it implicit that SAM_STAT_GOOD == 0, compare explicitly
> with SAM_STAT_GOOD.
> 
> Cc: Mike Christie <michael.christie@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/target/target_core_pscsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
> index 1c9aeab93477..dac44caf77a3 100644
> --- a/drivers/target/target_core_pscsi.c
> +++ b/drivers/target/target_core_pscsi.c
> @@ -1046,7 +1046,7 @@ static void pscsi_req_done(struct request *req, blk_status_t status)
>  	int result = scsi_req(req)->result;
>  	u8 scsi_status = status_byte(result) << 1;
>  
> -	if (scsi_status) {
> +	if (scsi_status != SAM_STAT_GOOD) {
>  		pr_debug("PSCSI Status Byte exception at cmd: %p CDB:"
>  			" 0x%02x Result: 0x%08x\n", cmd, pt->pscsi_cdb[0],
>  			result);
> 

Reviewed-by: Mike Christie <michael.christie@oracle.com>

Re: [PATCH 19/20] target: Fix several format specifiers

[Mike Christie]

On 4/13/21 12:07 PM, Bart Van Assche wrote:
> Use format specifier '%u' to format the u32 and int data types instead of
> '%hu'.
> 
> Cc: Mike Christie <michael.christie@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/target/target_core_configfs.c | 6 +++---
>  drivers/target/target_core_pr.c       | 6 ++----
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index 9cb1ca8421c8..01005a9e5128 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -2746,7 +2746,7 @@ static ssize_t target_tg_pt_gp_alua_access_state_store(struct config_item *item,
>  
>  	if (!tg_pt_gp->tg_pt_gp_valid_id) {
>  		pr_err("Unable to do implicit ALUA on non valid"
> -			" tg_pt_gp ID: %hu\n", tg_pt_gp->tg_pt_gp_valid_id);
> +			" tg_pt_gp ID: %u\n", tg_pt_gp->tg_pt_gp_valid_id);
>  		return -EINVAL;
>  	}
>  	if (!target_dev_configured(dev)) {
> @@ -2798,7 +2798,7 @@ static ssize_t target_tg_pt_gp_alua_access_status_store(
>  
>  	if (!tg_pt_gp->tg_pt_gp_valid_id) {
>  		pr_err("Unable to do set ALUA access status on non"
> -			" valid tg_pt_gp ID: %hu\n",
> +			" valid tg_pt_gp ID: %u\n",
>  			tg_pt_gp->tg_pt_gp_valid_id);
>  		return -EINVAL;
>  	}
> @@ -2853,7 +2853,7 @@ static ssize_t target_tg_pt_gp_alua_support_##_name##_store(		\
>  									\
>  	if (!t->tg_pt_gp_valid_id) {					\
>  		pr_err("Unable to do set " #_name " ALUA state on non"	\
> -		       " valid tg_pt_gp ID: %hu\n",			\
> +		       " valid tg_pt_gp ID: %u\n",			\

Did you just want to drop the tg_pt_gp_valid_id from the messages above?
Instead make the messages stop at non valid tg_pt_gp. Like for the first one:

Unable to do implicit ALUA on non valid tg_pt_gp ID.

It looks like we might have used to print the actual id. That's why we did
the hu and the message looks like we are printing the id "tg_pt_gp ID:".
We are now printing just 0 or 1 but it looks like an id value in the message.

Re: [PATCH 09/20] iscsi: Suppress two clang format mismatch warnings

[James Bottomley]

On Tue, 2021-04-13 at 10:07 -0700, Bart Van Assche wrote:
> Suppress two instances of the following clang compiler warning:
> 
> warning: format specifies type 'unsigned short'
>       but the argument has type 'int' [-Wformat]
> 
> Cc: Lee Duncan <lduncan@suse.com>
> Cc: Chris Leech <cleech@redhat.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/libiscsi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index 7ad11e42306d..0c3082d09712 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -3587,10 +3587,11 @@ int iscsi_conn_get_addr_param(struct
> sockaddr_storage *addr,
>  	case ISCSI_PARAM_CONN_PORT:
>  	case ISCSI_PARAM_LOCAL_PORT:
>  		if (sin)
> -			len = sprintf(buf, "%hu\n", be16_to_cpu(sin-
> >sin_port));
> +			len = sprintf(buf, "%hu\n",
> +				      (u16)be16_to_cpu(sin->sin_port));
>  		else
>  			len = sprintf(buf, "%hu\n",
> -				      be16_to_cpu(sin6->sin6_port));
> +				      (u16)be16_to_cpu(sin6-
> 

This looks odd: the generic definition of be16_to_cpu on le is

#define __be16_to_cpu(x) __swab16((__force __u16)(__be16)(x))

and __swab16 is

#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))

So why doesn't clang see the existing __u16 as short?  This smells like
a problem in the compiler file.

James

Re: [PATCH 19/20] target: Fix several format specifiers

[Bart Van Assche]

On 4/13/21 10:47 AM, Mike Christie wrote:
> On 4/13/21 12:07 PM, Bart Van Assche wrote:
>> Use format specifier '%u' to format the u32 and int data types instead of
>> '%hu'.
>>
>> Cc: Mike Christie <michael.christie@oracle.com>
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> ---
>>  drivers/target/target_core_configfs.c | 6 +++---
>>  drivers/target/target_core_pr.c       | 6 ++----
>>  2 files changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
>> index 9cb1ca8421c8..01005a9e5128 100644
>> --- a/drivers/target/target_core_configfs.c
>> +++ b/drivers/target/target_core_configfs.c
>> @@ -2746,7 +2746,7 @@ static ssize_t target_tg_pt_gp_alua_access_state_store(struct config_item *item,
>>  
>>  	if (!tg_pt_gp->tg_pt_gp_valid_id) {
>>  		pr_err("Unable to do implicit ALUA on non valid"
>> -			" tg_pt_gp ID: %hu\n", tg_pt_gp->tg_pt_gp_valid_id);
>> +			" tg_pt_gp ID: %u\n", tg_pt_gp->tg_pt_gp_valid_id);
>>  		return -EINVAL;
>>  	}
>>  	if (!target_dev_configured(dev)) {
>> @@ -2798,7 +2798,7 @@ static ssize_t target_tg_pt_gp_alua_access_status_store(
>>  
>>  	if (!tg_pt_gp->tg_pt_gp_valid_id) {
>>  		pr_err("Unable to do set ALUA access status on non"
>> -			" valid tg_pt_gp ID: %hu\n",
>> +			" valid tg_pt_gp ID: %u\n",
>>  			tg_pt_gp->tg_pt_gp_valid_id);
>>  		return -EINVAL;
>>  	}
>> @@ -2853,7 +2853,7 @@ static ssize_t target_tg_pt_gp_alua_support_##_name##_store(		\
>>  									\
>>  	if (!t->tg_pt_gp_valid_id) {					\
>>  		pr_err("Unable to do set " #_name " ALUA state on non"	\
>> -		       " valid tg_pt_gp ID: %hu\n",			\
>> +		       " valid tg_pt_gp ID: %u\n",			\
> 
> Did you just want to drop the tg_pt_gp_valid_id from the messages above?
> Instead make the messages stop at non valid tg_pt_gp. Like for the first one:
> 
> Unable to do implicit ALUA on non valid tg_pt_gp ID.
> 
> It looks like we might have used to print the actual id. That's why we did
> the hu and the message looks like we are printing the id "tg_pt_gp ID:".
> We are now printing just 0 or 1 but it looks like an id value in the message.

Right, printing tg_pt_gp_valid_id if we already know that it is zero is
not very useful. I will remove tg_pt_gp_valid_id from the above pr_err()
statements.

Bart.

Re: [PATCH 09/20] iscsi: Suppress two clang format mismatch warnings

[Bart Van Assche]

On 4/13/21 10:59 AM, James Bottomley wrote:
> On Tue, 2021-04-13 at 10:07 -0700, Bart Van Assche wrote:
>> Suppress two instances of the following clang compiler warning:
>>
>> warning: format specifies type 'unsigned short'
>>       but the argument has type 'int' [-Wformat]
>>
>> Cc: Lee Duncan <lduncan@suse.com>
>> Cc: Chris Leech <cleech@redhat.com>
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> ---
>>  drivers/scsi/libiscsi.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
>> index 7ad11e42306d..0c3082d09712 100644
>> --- a/drivers/scsi/libiscsi.c
>> +++ b/drivers/scsi/libiscsi.c
>> @@ -3587,10 +3587,11 @@ int iscsi_conn_get_addr_param(struct
>> sockaddr_storage *addr,
>>  	case ISCSI_PARAM_CONN_PORT:
>>  	case ISCSI_PARAM_LOCAL_PORT:
>>  		if (sin)
>> -			len = sprintf(buf, "%hu\n", be16_to_cpu(sin-
>>> sin_port));
>> +			len = sprintf(buf, "%hu\n",
>> +				      (u16)be16_to_cpu(sin->sin_port));
>>  		else
>>  			len = sprintf(buf, "%hu\n",
>> -				      be16_to_cpu(sin6->sin6_port));
>> +				      (u16)be16_to_cpu(sin6-
>>
> 
> This looks odd: the generic definition of be16_to_cpu on le is
> 
> #define __be16_to_cpu(x) __swab16((__force __u16)(__be16)(x))
> 
> and __swab16 is
> 
> #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
> 
> So why doesn't clang see the existing __u16 as short?  This smells like
> a problem in the compiler file.

Hi James,

To me this also seems to be a compiler issue. I will drop this patch
since I prefer not to insert casts if an expression already has the
proper type.

Thanks,

Bart.