On 7/17/23 12:05 PM, Bart Van Assche wrote:
> On 7/14/23 14:34, Mike Christie wrote:
>> This has scsi-ml retry errors instead of driving them itself.
>>
>> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
>> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
>> ---
>> drivers/ufs/core/ufshcd.c | 19 +++++++++----------
>> 1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 983fae84d9e8..267c24c57396 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -9291,7 +9291,14 @@ static int ufshcd_execute_start_stop(struct scsi_device *sdev,
>> struct scsi_sense_hdr *sshdr)
>> {
>> const unsigned char cdb[6] = { START_STOP, 0, 0, 0, pwr_mode << 4, 0 };
>> + struct scsi_failure failures[] = {
>> + {
>> + .allowed = 2,
>> + .result = SCMD_FAILURE_RESULT_ANY,
>> + },
>> + };
>> const struct scsi_exec_args args = {
>> + .failures = failures,
>> .sshdr = sshdr,
>> .req_flags = BLK_MQ_REQ_PM,
>> .scmd_flags = SCMD_FAIL_IF_RECOVERING,
>> @@ -9317,7 +9324,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
>> struct scsi_sense_hdr sshdr;
>> struct scsi_device *sdp;
>> unsigned long flags;
>> - int ret, retries;
>> + int ret;
>> spin_lock_irqsave(hba->host->host_lock, flags);
>> sdp = hba->ufs_device_wlun;
>> @@ -9343,15 +9350,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
>> * callbacks hence set the RQF_PM flag so that it doesn't resume the
>> * already suspended childs.
>> */
>> - for (retries = 3; retries > 0; --retries) {
>> - ret = ufshcd_execute_start_stop(sdp, pwr_mode, &sshdr);
>> - /*
>> - * scsi_execute() only returns a negative value if the request
>> - * queue is dying.
>> - */
>> - if (ret <= 0)
>> - break;
>> - }
>> + ret = ufshcd_execute_start_stop(sdp, pwr_mode, &sshdr);
>> if (ret) {
>> sdev_printk(KERN_WARNING, sdp,
>> "START_STOP failed for power mode: %d, result %x\n",
>
> The original code only retries if ->result > 0. Is my understanding correct that the new code retries SCSI command execution whether ->result is < 0 or > 0? If so, I think this patch introduces an unintended behavior change.
The new code does not retry when result is < 0.
SCMD_FAILURE_RESULT_ANY is for cases where result > 0.
