| Message ID | 20210423113944.42672-8-hare@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SCSI result cleanup, part 2 | expand |
On 4/23/21 4:39 AM, Hannes Reinecke wrote: > Add a helper function scsi_status_is_check_condtion() to ^^^^^^^^ typo > encapsulate the frequent checks for SAM_STAT_CHECK_CONDITION. [ ... ] > +/** scsi_status_is_check_condition - check the status return. > + * > + * @status: the status passed up from the driver (including host and > + * driver components) > + * > + * This returns true if the status code is SAM_STAT_CHECK_CONDITION. > + */ Shouldn't the function name and the description appear on the second line of the kernel-doc header? > +static inline int scsi_status_is_check_condition(int status) > +{ > + if (status < 0) > + return false; > + status &= 0xfe; > + return (status == SAM_STAT_CHECK_CONDITION); > +} No parentheses around the expression in a return statement please. Thanks, Bart.
On 4/26/21 5:34 AM, Bart Van Assche wrote: > On 4/23/21 4:39 AM, Hannes Reinecke wrote: >> Add a helper function scsi_status_is_check_condtion() to > ^^^^^^^^ typo >> encapsulate the frequent checks for SAM_STAT_CHECK_CONDITION. > > [ ... ] > >> +/** scsi_status_is_check_condition - check the status return. >> + * >> + * @status: the status passed up from the driver (including host and >> + * driver components) >> + * >> + * This returns true if the status code is SAM_STAT_CHECK_CONDITION. >> + */ > > Shouldn't the function name and the description appear on the second > line of the kernel-doc header? > Yeah, you're right. I used copy&paste from the function above, looked kinda fishy even then ... Will be fixing it up. >> +static inline int scsi_status_is_check_condition(int status) >> +{ >> + if (status < 0) >> + return false; >> + status &= 0xfe; >> + return (status == SAM_STAT_CHECK_CONDITION); >> +} > > No parentheses around the expression in a return statement please. > Will be fixing it up. Cheers, Hannes
Modulo the little style issue:
Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index f33f56680c59..9e229a1a4965 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -1005,7 +1005,7 @@ static void handle_cmd_rsp(struct srp_event_struct *evt_struct) if (cmnd) { cmnd->result |= rsp->status; - if (((cmnd->result >> 1) & 0x1f) == CHECK_CONDITION) + if (scsi_status_is_check_condition(cmnd->result)) memcpy(cmnd->sense_buffer, rsp->data, be32_to_cpu(rsp->sense_data_len)); diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 99dc6ec0b6e5..1ce46e6e6483 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -144,7 +144,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 (scsi_status_is_check_condition(cmd->result)) scsi_print_sense(cmd); if (level > 3) scmd_printk(KERN_INFO, cmd, diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index d8fafe77dbbe..0967021cc06e 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -1258,7 +1258,7 @@ int scsi_eh_get_sense(struct list_head *work_q, current->comm)); break; } - if (status_byte(scmd->result) != CHECK_CONDITION) + if (!scsi_status_is_check_condition(scmd->result)) /* * don't request sense if there's no check condition * status because the error we're processing isn't one @@ -1774,7 +1774,7 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd) return (scmd->request->cmd_flags & REQ_FAILFAST_DRIVER); } - if (status_byte(scmd->result) != CHECK_CONDITION) + if (!scsi_status_is_check_condition(scmd->result)) return 0; check_type: diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index d72f15f6c225..488bc49afa76 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2232,7 +2232,7 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage, } data->header_length = header_length; result = 0; - } else if ((status_byte(result) == CHECK_CONDITION) && + } else if (scsi_status_is_check_condition(result) && scsi_sense_valid(sshdr) && sshdr->sense_key == UNIT_ATTENTION && retry_count) { retry_count--; diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h index d0a24e55ad63..4ff4b45c19f3 100644 --- a/include/scsi/scsi.h +++ b/include/scsi/scsi.h @@ -58,6 +58,20 @@ static inline int scsi_status_is_good(int status) (status == SAM_STAT_COMMAND_TERMINATED)); } +/** scsi_status_is_check_condition - check the status return. + * + * @status: the status passed up from the driver (including host and + * driver components) + * + * This returns true if the status code is SAM_STAT_CHECK_CONDITION. + */ +static inline int scsi_status_is_check_condition(int status) +{ + if (status < 0) + return false; + status &= 0xfe; + return (status == SAM_STAT_CHECK_CONDITION); +} /* * standard mode-select header prepended to all mode-select commands
Add a helper function scsi_status_is_check_condtion() to encapsulate the frequent checks for SAM_STAT_CHECK_CONDITION. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +- drivers/scsi/scsi.c | 2 +- drivers/scsi/scsi_error.c | 4 ++-- drivers/scsi/scsi_lib.c | 2 +- include/scsi/scsi.h | 14 ++++++++++++++ 5 files changed, 19 insertions(+), 5 deletions(-)