Commit 8793613d authored by Hannes Reinecke's avatar Hannes Reinecke Committed by Martin K. Petersen

scsi: core: Fixup calling convention for scsi_mode_sense()

The description for scsi_mode_sense() claims to return the number of valid
bytes on success, which is not what the code does.  Additionally there is
no gain in returning the SCSI status, as everything the callers do is to
check against scsi_result_is_good(), which is what scsi_mode_sense() does
already.  So change the calling convention to return a standard error code
on failure, and 0 on success, and adapt the description and all callers.

Link: https://lore.kernel.org/r/20210427083046.31620-4-hare@suse.deReviewed-by: default avatarBart Van Assche <bvanassche@acm.org>
Signed-off-by: default avatarHannes Reinecke <hare@suse.de>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 21eccf30
...@@ -2093,9 +2093,7 @@ EXPORT_SYMBOL_GPL(scsi_mode_select); ...@@ -2093,9 +2093,7 @@ EXPORT_SYMBOL_GPL(scsi_mode_select);
* @sshdr: place to put sense data (or NULL if no sense to be collected). * @sshdr: place to put sense data (or NULL if no sense to be collected).
* must be SCSI_SENSE_BUFFERSIZE big. * must be SCSI_SENSE_BUFFERSIZE big.
* *
* Returns zero if unsuccessful, or the header offset (either 4 * Returns zero if successful, or a negative error number on failure
* or 8 depending on whether a six or ten byte command was
* issued) if successful.
*/ */
int int
scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage, scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
...@@ -2142,6 +2140,8 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage, ...@@ -2142,6 +2140,8 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
result = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buffer, len, result = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buffer, len,
sshdr, timeout, retries, NULL); sshdr, timeout, retries, NULL);
if (result < 0)
return result;
/* This code looks awful: what it's doing is making sure an /* This code looks awful: what it's doing is making sure an
* ILLEGAL REQUEST sense return identifies the actual command * ILLEGAL REQUEST sense return identifies the actual command
...@@ -2186,13 +2186,15 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage, ...@@ -2186,13 +2186,15 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
data->block_descriptor_length = buffer[3]; data->block_descriptor_length = buffer[3];
} }
data->header_length = header_length; data->header_length = header_length;
result = 0;
} else if ((status_byte(result) == CHECK_CONDITION) && } else if ((status_byte(result) == CHECK_CONDITION) &&
scsi_sense_valid(sshdr) && scsi_sense_valid(sshdr) &&
sshdr->sense_key == UNIT_ATTENTION && retry_count) { sshdr->sense_key == UNIT_ATTENTION && retry_count) {
retry_count--; retry_count--;
goto retry; goto retry;
} }
if (result > 0)
result = -EIO;
return result; return result;
} }
EXPORT_SYMBOL(scsi_mode_sense); EXPORT_SYMBOL(scsi_mode_sense);
......
...@@ -1229,16 +1229,15 @@ int sas_read_port_mode_page(struct scsi_device *sdev) ...@@ -1229,16 +1229,15 @@ int sas_read_port_mode_page(struct scsi_device *sdev)
char *buffer = kzalloc(BUF_SIZE, GFP_KERNEL), *msdata; char *buffer = kzalloc(BUF_SIZE, GFP_KERNEL), *msdata;
struct sas_end_device *rdev = sas_sdev_to_rdev(sdev); struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
struct scsi_mode_data mode_data; struct scsi_mode_data mode_data;
int res, error; int error;
if (!buffer) if (!buffer)
return -ENOMEM; return -ENOMEM;
res = scsi_mode_sense(sdev, 1, 0x19, buffer, BUF_SIZE, 30*HZ, 3, error = scsi_mode_sense(sdev, 1, 0x19, buffer, BUF_SIZE, 30*HZ, 3,
&mode_data, NULL); &mode_data, NULL);
error = -EINVAL; if (error)
if (!scsi_status_is_good(res))
goto out; goto out;
msdata = buffer + mode_data.header_length + msdata = buffer + mode_data.header_length +
......
...@@ -2670,18 +2670,18 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer) ...@@ -2670,18 +2670,18 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
* 5: Illegal Request, Sense Code 24: Invalid field in * 5: Illegal Request, Sense Code 24: Invalid field in
* CDB. * CDB.
*/ */
if (!scsi_status_is_good(res)) if (res < 0)
res = sd_do_mode_sense(sdkp, 0, 0, buffer, 4, &data, NULL); res = sd_do_mode_sense(sdkp, 0, 0, buffer, 4, &data, NULL);
/* /*
* Third attempt: ask 255 bytes, as we did earlier. * Third attempt: ask 255 bytes, as we did earlier.
*/ */
if (!scsi_status_is_good(res)) if (res < 0)
res = sd_do_mode_sense(sdkp, 0, 0x3F, buffer, 255, res = sd_do_mode_sense(sdkp, 0, 0x3F, buffer, 255,
&data, NULL); &data, NULL);
} }
if (!scsi_status_is_good(res)) { if (res < 0) {
sd_first_printk(KERN_WARNING, sdkp, sd_first_printk(KERN_WARNING, sdkp,
"Test WP failed, assume Write Enabled\n"); "Test WP failed, assume Write Enabled\n");
} else { } else {
...@@ -2742,7 +2742,7 @@ sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer) ...@@ -2742,7 +2742,7 @@ sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer)
res = sd_do_mode_sense(sdkp, dbd, modepage, buffer, first_len, res = sd_do_mode_sense(sdkp, dbd, modepage, buffer, first_len,
&data, &sshdr); &data, &sshdr);
if (!scsi_status_is_good(res)) if (res < 0)
goto bad_sense; goto bad_sense;
if (!data.header_length) { if (!data.header_length) {
...@@ -2774,7 +2774,7 @@ sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer) ...@@ -2774,7 +2774,7 @@ sd_read_cache_type(struct scsi_disk *sdkp, unsigned char *buffer)
res = sd_do_mode_sense(sdkp, dbd, modepage, buffer, len, res = sd_do_mode_sense(sdkp, dbd, modepage, buffer, len,
&data, &sshdr); &data, &sshdr);
if (scsi_status_is_good(res)) { if (!res) {
int offset = data.header_length + data.block_descriptor_length; int offset = data.header_length + data.block_descriptor_length;
while (offset < len) { while (offset < len) {
...@@ -2892,7 +2892,7 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer) ...@@ -2892,7 +2892,7 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer)
res = scsi_mode_sense(sdp, 1, 0x0a, buffer, 36, SD_TIMEOUT, res = scsi_mode_sense(sdp, 1, 0x0a, buffer, 36, SD_TIMEOUT,
sdkp->max_retries, &data, &sshdr); sdkp->max_retries, &data, &sshdr);
if (!scsi_status_is_good(res) || !data.header_length || if (res < 0 || !data.header_length ||
data.length < 6) { data.length < 6) {
sd_first_printk(KERN_WARNING, sdkp, sd_first_printk(KERN_WARNING, sdkp,
"getting Control mode page failed, assume no ATO\n"); "getting Control mode page failed, assume no ATO\n");
......
...@@ -911,7 +911,7 @@ static void get_capabilities(struct scsi_cd *cd) ...@@ -911,7 +911,7 @@ static void get_capabilities(struct scsi_cd *cd)
rc = scsi_mode_sense(cd->device, 0, 0x2a, buffer, ms_len, rc = scsi_mode_sense(cd->device, 0, 0x2a, buffer, ms_len,
SR_TIMEOUT, 3, &data, NULL); SR_TIMEOUT, 3, &data, NULL);
if (!scsi_status_is_good(rc) || data.length > ms_len || if (rc < 0 || data.length > ms_len ||
data.header_length + data.block_descriptor_length > data.length) { data.header_length + data.block_descriptor_length > data.length) {
/* failed, drive doesn't have capabilities mode page */ /* failed, drive doesn't have capabilities mode page */
cd->cdi.speed = 1; cd->cdi.speed = 1;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment