On 10/21/19 11:53 AM, Hannes Reinecke wrote:
Introduce scsi_build_sense() as a wrapper around
scsi_build_sense_buffer() to format the buffer and set
the correct SCSI status.
Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
---
drivers/s390/scsi/zfcp_scsi.c | 5 +--
16 files changed, 60 insertions(+), 128 deletions(-)
diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index e9ded2befa0d..da52d7649f4d 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -834,10 +834,7 @@ void zfcp_scsi_set_prot(struct zfcp_adapter *adapter)
*/
void zfcp_scsi_dif_sense_error(struct scsi_cmnd *scmd, int ascq)
{
- scsi_build_sense_buffer(1, scmd->sense_buffer,
- ILLEGAL_REQUEST, 0x10, ascq);
- set_driver_byte(scmd, DRIVER_SENSE);
- scmd->result |= SAM_STAT_CHECK_CONDITION;
+ scsi_build_sense(scmd, 1, ILLEGAL_REQUEST, 0x10, ascq);
set_host_byte(scmd, DID_SOFT_ERROR);
}
looks like a non-functional change for zfcp, so for this part:
Acked-by: Steffen Maier <maier@xxxxxxxxxxxxx> # for zfcp
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a0db8d8766a8..2babf6df8066 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3117,3 +3117,21 @@ int scsi_vpd_tpg_id(struct scsi_device *sdev, int *rel_id)
return group_id;
}
EXPORT_SYMBOL(scsi_vpd_tpg_id);
+
+/**
+ * scsi_build_sense - build sense data for a command
minor: I suppose kdoc&sphnix parse and render it correctly? Because
Documentation/doc-guide/kernel-doc.rst says the format for function kdoc has
"()" as function name suffix:
+ * scsi_build_sense() - build sense data for a command
+ * @scmd: scsi command for which the sense should be formatted
+ * @desc: Sense format (non-zero == descriptor format,
+ * 0 == fixed format)
Looks like this has already been like that. Not sure if this patch set touches
every user of scsi_build_sense{_buffer}(). It would be nice to have meaningful
identifiers for values passed to @desc, e.g. something like the following
instead of "magic" zero and non-zero:
enum scsi_sense_format {
SCSI_SENSE_FIXED = 0,
SCSI_SENSE_DESCRIPTOR
};
+ * @key: Sense key
+ * @asc: Additional sense code
+ * @ascq: Additional sense code qualifier
+ *
+ **/
minor:
+ */
[no double star at kdoc end?]
+void scsi_build_sense(struct scsi_cmnd *scmd, int desc, u8 key, u8 asc, u8 ascq)
+{
+ scsi_build_sense_buffer(desc, scmd->sense_buffer, key, asc, ascq);
+ scmd->result = (DRIVER_SENSE << 24) | (DID_OK << 16) |
+ SAM_STAT_CHECK_CONDITION;
While this is scsi_lib and thus "internal" helper code, I wonder if this should
nonetheless use the helper functions to access and build scmd->result in order
to have the error-prone bit shifts in only one central place?:
scmd->result = SAM_STAT_CHECK_CONDITION;
set_driver_byte(scmd, DRIVER_SENSE);
set_host_byte(scmd, DID_OK);
+}
+EXPORT_SYMBOL_GPL(scsi_build_sense);
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 6932d91472d5..9b9ca629097d 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -338,4 +338,7 @@ static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd)
return xfer_len;
}
+extern void scsi_build_sense(struct scsi_cmnd *scmd, int desc,
+ u8 key, u8 asc, u8 ascq);
+
#endif /* _SCSI_SCSI_CMND_H */
--
Mit freundlichen Gruessen / Kind regards
Steffen Maier
Linux on IBM Z Development
https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
