This is small patch set with obvious prep patches for the SCSI results
rework discussed at LFS/MM and other occasions on the list.
Changes since v2:
- Dropped already applied patch 1
- Incorporated Bart's and Christoph's feedback
- Added Reviewed-bys
Johannes Thumshirn (2):
scsi: check for equality of result byte values
scsi: don't add scsi command result bytes
drivers/scsi/ch.c | 2 +-
drivers/scsi/dc395x.c | 5 ++---
drivers/scsi/imm.c | 2 +-
drivers/scsi/mesh.c | 4 ++--
drivers/scsi/scsi.c | 2 +-
drivers/scsi/scsi_ioctl.c | 4 ++--
drivers/scsi/scsi_lib.c | 4 ++--
drivers/scsi/scsi_scan.c | 2 +-
drivers/scsi/scsi_transport_spi.c | 2 +-
drivers/scsi/sd.c | 14 +++++++-------
drivers/scsi/sym53c8xx_2/sym_glue.c | 2 +-
drivers/scsi/sym53c8xx_2/sym_glue.h | 2 +-
drivers/scsi/ufs/ufshcd.c | 2 +-
13 files changed, 23 insertions(+), 24 deletions(-)
--
2.16.4
Some drivers are ADDing the scsi command's result bytes instead of
ORing them.
While this can produce correct results it has unexpected side effects.
Signed-off-by: Johannes Thumshirn <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
---
Changes since v2:
- Re-added braces around shift operators (Bart, Christoph)
- Add Reviewed-bys
Changes since v1:
- Fix kbuild robot warnings
---
drivers/scsi/imm.c | 2 +-
drivers/scsi/mesh.c | 4 ++--
drivers/scsi/sym53c8xx_2/sym_glue.c | 2 +-
drivers/scsi/sym53c8xx_2/sym_glue.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/imm.c b/drivers/scsi/imm.c
index 87c94191033b..8c6627bc8a39 100644
--- a/drivers/scsi/imm.c
+++ b/drivers/scsi/imm.c
@@ -892,7 +892,7 @@ static int imm_engine(imm_struct *dev, struct scsi_cmnd *cmd)
/* Check for optional message byte */
if (imm_wait(dev) == (unsigned char) 0xb8)
imm_in(dev, &h, 1);
- cmd->result = (DID_OK << 16) + (l & STATUS_MASK);
+ cmd->result = (DID_OK << 16) | (l & STATUS_MASK);
}
if ((dev->mode == IMM_NIBBLE) || (dev->mode == IMM_PS2)) {
w_ctr(ppb, 0x4);
diff --git a/drivers/scsi/mesh.c b/drivers/scsi/mesh.c
index 1753e42826dd..82e01dbe90af 100644
--- a/drivers/scsi/mesh.c
+++ b/drivers/scsi/mesh.c
@@ -594,9 +594,9 @@ static void mesh_done(struct mesh_state *ms, int start_next)
ms->current_req = NULL;
tp->current_req = NULL;
if (cmd) {
- cmd->result = (ms->stat << 16) + cmd->SCp.Status;
+ cmd->result = (ms->stat << 16) | cmd->SCp.Status;
if (ms->stat == DID_OK)
- cmd->result += (cmd->SCp.Message << 8);
+ cmd->result |= cmd->SCp.Message << 8;
if (DEBUG_TARGET(cmd)) {
printk(KERN_DEBUG "mesh_done: result = %x, data_ptr=%d, buflen=%d\n",
cmd->result, ms->data_ptr, scsi_bufflen(cmd));
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 7320d5fe4cbc..5f10aa9bad9b 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -252,7 +252,7 @@ void sym_set_cam_result_error(struct sym_hcb *np, struct sym_ccb *cp, int resid)
cam_status = sym_xerr_cam_status(DID_ERROR, cp->xerr_status);
}
scsi_set_resid(cmd, resid);
- cmd->result = (drv_status << 24) + (cam_status << 16) + scsi_status;
+ cmd->result = (drv_status << 24) | (cam_status << 16) | scsi_status;
}
static int sym_scatter(struct sym_hcb *np, struct sym_ccb *cp, struct scsi_cmnd *cmd)
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.h b/drivers/scsi/sym53c8xx_2/sym_glue.h
index 805369521df8..e34801ae5d69 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.h
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.h
@@ -256,7 +256,7 @@ sym_get_cam_status(struct scsi_cmnd *cmd)
static inline void sym_set_cam_result_ok(struct sym_ccb *cp, struct scsi_cmnd *cmd, int resid)
{
scsi_set_resid(cmd, resid);
- cmd->result = (((DID_OK) << 16) + ((cp->ssss_status) & 0x7f));
+ cmd->result = (DID_OK << 16) | (cp->ssss_status & 0x7f);
}
void sym_set_cam_result_error(struct sym_hcb *np, struct sym_ccb *cp, int resid);
--
2.16.4
When evaluating a SCSI command's result using the field access macros,
check for equality of the fields and not if a specific bit is set.
This is a preparation patch, for reworking the results field in the
SCSI command.
Signed-off-by: Johannes Thumshirn <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
---
Changes since v2:
- Fixed logical error in sd_spinup_disk() (Bart)
Changes since v1:
- Reworked braces (Bart)
---
drivers/scsi/ch.c | 2 +-
drivers/scsi/dc395x.c | 5 ++---
drivers/scsi/scsi.c | 2 +-
drivers/scsi/scsi_ioctl.c | 4 ++--
drivers/scsi/scsi_lib.c | 4 ++--
drivers/scsi/scsi_scan.c | 2 +-
drivers/scsi/scsi_transport_spi.c | 2 +-
drivers/scsi/sd.c | 14 +++++++-------
drivers/scsi/ufs/ufshcd.c | 2 +-
9 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index c535c52e72e5..1c5051b1c125 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -199,7 +199,7 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd, int cmd_len,
buflength, &sshdr, timeout * HZ,
MAX_RETRIES, NULL);
- if (driver_byte(result) & DRIVER_SENSE) {
+ if (driver_byte(result) == DRIVER_SENSE) {
if (debug)
scsi_print_sense_hdr(ch->device, ch->name, &sshdr);
errno = ch_find_errno(&sshdr);
diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index 60ef8df42b95..1ed2cd82129d 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -3473,9 +3473,8 @@ static void srb_done(struct AdapterCtlBlk *acb, struct DeviceCtlBlk *dcb,
/*if( srb->cmd->cmnd[0] == INQUIRY && */
/* (host_byte(cmd->result) == DID_OK || status_byte(cmd->result) & CHECK_CONDITION) ) */
- if ((cmd->result == (DID_OK << 16)
- || status_byte(cmd->result) &
- CHECK_CONDITION)) {
+ if ((cmd->result == (DID_OK << 16) ||
+ status_byte(cmd->result) == CHECK_CONDITION)) {
if (!dcb->init_tcq_flag) {
add_dev(acb, dcb, ptr);
dcb->init_tcq_flag = 1;
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 4c60c260c5da..70ef3c39061d 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -162,7 +162,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)
(level > 1)) {
scsi_print_result(cmd, "Done", disposition);
scsi_print_command(cmd);
- if (status_byte(cmd->result) & CHECK_CONDITION)
+ if (status_byte(cmd->result) == CHECK_CONDITION)
scsi_print_sense(cmd);
if (level > 3)
scmd_printk(KERN_INFO, cmd,
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index 0a875491f5a7..cc30fccc1a2e 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -100,8 +100,8 @@ static int ioctl_internal_command(struct scsi_device *sdev, char *cmd,
SCSI_LOG_IOCTL(2, sdev_printk(KERN_INFO, sdev,
"Ioctl returned 0x%x\n", result));
- if ((driver_byte(result) & DRIVER_SENSE) &&
- (scsi_sense_valid(&sshdr))) {
+ if (driver_byte(result) == DRIVER_SENSE &&
+ scsi_sense_valid(&sshdr)) {
switch (sshdr.sense_key) {
case ILLEGAL_REQUEST:
if (cmd[0] == ALLOW_MEDIUM_REMOVAL)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 41e9ac9fc138..d53eda6f1fe0 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1031,7 +1031,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
*/
if (!level && __ratelimit(&_rs)) {
scsi_print_result(cmd, NULL, FAILED);
- if (driver_byte(result) & DRIVER_SENSE)
+ if (driver_byte(result) == DRIVER_SENSE)
scsi_print_sense(cmd);
scsi_print_command(cmd);
}
@@ -2555,7 +2555,7 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
* ILLEGAL REQUEST if the code page isn't supported */
if (use_10_for_ms && !scsi_status_is_good(result) &&
- (driver_byte(result) & DRIVER_SENSE)) {
+ driver_byte(result) == DRIVER_SENSE) {
if (scsi_sense_valid(sshdr)) {
if ((sshdr->sense_key == ILLEGAL_REQUEST) &&
(sshdr->asc == 0x20) && (sshdr->ascq == 0)) {
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 0880d975eed3..78ca63dfba4a 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -614,7 +614,7 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
* INQUIRY should not yield UNIT_ATTENTION
* but many buggy devices do so anyway.
*/
- if ((driver_byte(result) & DRIVER_SENSE) &&
+ if (driver_byte(result) == DRIVER_SENSE &&
scsi_sense_valid(&sshdr)) {
if ((sshdr.sense_key == UNIT_ATTENTION) &&
((sshdr.asc == 0x28) ||
diff --git a/drivers/scsi/scsi_transport_spi.c b/drivers/scsi/scsi_transport_spi.c
index 2ca150b16764..40b85b752b79 100644
--- a/drivers/scsi/scsi_transport_spi.c
+++ b/drivers/scsi/scsi_transport_spi.c
@@ -136,7 +136,7 @@ static int spi_execute(struct scsi_device *sdev, const void *cmd,
REQ_FAILFAST_TRANSPORT |
REQ_FAILFAST_DRIVER,
0, NULL);
- if (!(driver_byte(result) & DRIVER_SENSE) ||
+ if (driver_byte(result) != DRIVER_SENSE ||
sshdr->sense_key != UNIT_ATTENTION)
break;
}
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 9421d9877730..612aa14c1b26 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1635,7 +1635,7 @@ static int sd_sync_cache(struct scsi_disk *sdkp, struct scsi_sense_hdr *sshdr)
if (res) {
sd_print_result(sdkp, "Synchronize Cache(10) failed", res);
- if (driver_byte(res) & DRIVER_SENSE)
+ if (driver_byte(res) == DRIVER_SENSE)
sd_print_sense_hdr(sdkp, sshdr);
/* we need to evaluate the error return */
@@ -1737,8 +1737,8 @@ static int sd_pr_command(struct block_device *bdev, u8 sa,
result = scsi_execute_req(sdev, cmd, DMA_TO_DEVICE, &data, sizeof(data),
&sshdr, SD_TIMEOUT, SD_MAX_RETRIES, NULL);
- if ((driver_byte(result) & DRIVER_SENSE) &&
- (scsi_sense_valid(&sshdr))) {
+ if (driver_byte(result) == DRIVER_SENSE &&
+ scsi_sense_valid(&sshdr)) {
sdev_printk(KERN_INFO, sdev, "PR command failed: %d\n", result);
scsi_print_sense_hdr(sdev, NULL, &sshdr);
}
@@ -2095,10 +2095,10 @@ sd_spinup_disk(struct scsi_disk *sdkp)
retries++;
} while (retries < 3 &&
(!scsi_status_is_good(the_result) ||
- ((driver_byte(the_result) & DRIVER_SENSE) &&
+ ((driver_byte(the_result) == DRIVER_SENSE) &&
sense_valid && sshdr.sense_key == UNIT_ATTENTION)));
- if ((driver_byte(the_result) & DRIVER_SENSE) == 0) {
+ if (driver_byte(the_result) != DRIVER_SENSE) {
/* no sense, TUR either succeeded or failed
* with a status error */
if(!spintime && !scsi_status_is_good(the_result)) {
@@ -2224,7 +2224,7 @@ static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp,
struct scsi_sense_hdr *sshdr, int sense_valid,
int the_result)
{
- if (driver_byte(the_result) & DRIVER_SENSE)
+ if (driver_byte(the_result) == DRIVER_SENSE)
sd_print_sense_hdr(sdkp, sshdr);
else
sd_printk(KERN_NOTICE, sdkp, "Sense not available.\n");
@@ -3490,7 +3490,7 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start)
SD_TIMEOUT, SD_MAX_RETRIES, 0, RQF_PM, NULL);
if (res) {
sd_print_result(sdkp, "Start/Stop Unit failed", res);
- if (driver_byte(res) & DRIVER_SENSE)
+ if (driver_byte(res) == DRIVER_SENSE)
sd_print_sense_hdr(sdkp, &sshdr);
if (scsi_sense_valid(&sshdr) &&
/* 0x3a is medium not present */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 350f859625f6..3560185002da 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7303,7 +7303,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
sdev_printk(KERN_WARNING, sdp,
"START_STOP failed for power mode: %d, result %x\n",
pwr_mode, ret);
- if (driver_byte(ret) & DRIVER_SENSE)
+ if (driver_byte(ret) == DRIVER_SENSE)
scsi_print_sense_hdr(sdp, NULL, &sshdr);
}
--
2.16.4
On Mon, 2018-06-25 at 13:20 +0200, Johannes Thumshirn wrote:
> When evaluating a SCSI command's result using the field access macros,
> check for equality of the fields and not if a specific bit is set.
>
> This is a preparation patch, for reworking the results field in the
> SCSI command.
Reviewed-by: Bart Van Assche <[email protected]>
Johannes,
> This is small patch set with obvious prep patches for the SCSI results
> rework discussed at LFS/MM and other occasions on the list.
Applied to 4.19/scsi-queue. Thank you!
--
Martin K. Petersen Oracle Linux Engineering