On 4/24/21 11:20 AM, Finn Thain wrote:
> On Fri, 23 Apr 2021, Hannes Reinecke wrote:
>
>> Instead of setting the message byte translate it to the appropriate
>> host byte. As error recovery would return DID_ERROR for any non-zero
>> message byte the translation doesn't change the error handling.
>>
>> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
>> ---
>> drivers/scsi/wd33c93.c | 46 ++++++++++++++++++++++++------------------
>> 1 file changed, 26 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/scsi/wd33c93.c b/drivers/scsi/wd33c93.c
>> index a23277bb870e..187b363db00e 100644
>> --- a/drivers/scsi/wd33c93.c
>> +++ b/drivers/scsi/wd33c93.c
>> @@ -1176,13 +1176,14 @@ wd33c93_intr(struct Scsi_Host *instance)
>> if (cmd->SCp.Status == ILLEGAL_STATUS_BYTE)
>> cmd->SCp.Status = lun;
>> if (cmd->cmnd[0] == REQUEST_SENSE
>> - && cmd->SCp.Status != SAM_STAT_GOOD)
>> - cmd->result =
>> - (cmd->
>> - result & 0x00ffff) | (DID_ERROR << 16);
>> - else
>> - cmd->result =
>> - cmd->SCp.Status | (cmd->SCp.Message << 8);
>> + && cmd->SCp.Status != SAM_STAT_GOOD) {
>> + set_host_byte(cmd, DID_ERROR);
>> + set_status_byte(cmd, SAM_STAT_GOOD);
>> + } else {
>> + set_host_byte(cmd, DID_OK);
>> + translate_msg_byte(cmd, cmd->SCp.Message);
>> + set_status_byte(cmd, cmd->SCp.Status);
>> + }
>> cmd->scsi_done(cmd);
>>
>> /* We are no longer connected to a target - check to see if
>> @@ -1262,11 +1263,15 @@ wd33c93_intr(struct Scsi_Host *instance)
>> hostdata->connected = NULL;
>> hostdata->busy[cmd->device->id] &= ~(1 << (cmd->device->lun & 0xff));
>> hostdata->state = S_UNCONNECTED;
>> - if (cmd->cmnd[0] == REQUEST_SENSE && cmd->SCp.Status != SAM_STAT_GOOD)
>> - cmd->result =
>> - (cmd->result & 0x00ffff) | (DID_ERROR << 16);
>> - else
>> - cmd->result = cmd->SCp.Status | (cmd->SCp.Message << 8);
>> + if (cmd->cmnd[0] == REQUEST_SENSE &&
>> + cmd->SCp.Status != SAM_STAT_GOOD) {
>> + set_host_byte(cmd, DID_ERROR);
>> + set_status_byte(cmd, SAM_STAT_GOOD);
>> + } else {
>> + set_host_byte(cmd, DID_OK);
>> + translate_msg_byte(cmd, cmd->SCp.Message);
>> + set_status_byte(cmd, cmd->SCp.Status);
>> + }
>> cmd->scsi_done(cmd);
>>
>> /* We are no longer connected to a target - check to see if
>> @@ -1295,14 +1300,15 @@ wd33c93_intr(struct Scsi_Host *instance)
>> hostdata->busy[cmd->device->id] &= ~(1 << (cmd->device->lun & 0xff));
>> hostdata->state = S_UNCONNECTED;
>> DB(DB_INTR, printk(":%d", cmd->SCp.Status))
>> - if (cmd->cmnd[0] == REQUEST_SENSE
>> - && cmd->SCp.Status != SAM_STAT_GOOD)
>> - cmd->result =
>> - (cmd->
>> - result & 0x00ffff) | (DID_ERROR << 16);
>> - else
>> - cmd->result =
>> - cmd->SCp.Status | (cmd->SCp.Message << 8);
>> + if (cmd->cmnd[0] == REQUEST_SENSE
>> + && cmd->SCp.Status != SAM_STAT_GOOD) {
>> + set_host_byte(cmd, DID_ERROR);
>> + set_status_byte(cmd, SAM_STAT_GOOD);
>> + } else {
>> + set_host_byte(cmd, DID_OK);
>> + translate_msg_byte(cmd, cmd->SCp.Message);
>> + set_status_byte(cmd, cmd->SCp.Status);
>> + }
>> cmd->scsi_done(cmd);
>> break;
>> case S_PRE_TMP_DISC:
>>
>
> I think these three hunks all have the same mistake, which would force
> SAM_STAT_GOOD.
>
Which mistake was that again?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)
