Re: [PATCH v3 6/8] scsi: ufs: Try harder to change the power mode

[Bart Van Assche <bvanassche@acm.org>]

On 10/2/22 23:10, Adrian Hunter wrote:
> On 30/09/22 01:00, Bart Van Assche wrote:
>> Instead of only retrying the START STOP UNIT command if a unit attention
>> is reported, repeat it if any SCSI error is reported by the device or if
>> the command timed out.
>>
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> ---
>>   drivers/ufs/core/ufshcd.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 02e73208b921..e8c0504e9e83 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -8784,9 +8784,9 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
>>   	for (retries = 3; retries > 0; --retries) {
>>   		ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
>>   				START_STOP_TIMEOUT, 0, 0, RQF_PM, NULL);
>> -		if (!scsi_status_is_check_condition(ret) ||
>> -				!scsi_sense_valid(&sshdr) ||
>> -				sshdr.sense_key != UNIT_ATTENTION)
>> +		if (ret < 0)
>> +			break;
> 
> Could use a comment to answer: why not retry if ret < 0 ?

OK, I will add a comment above that if-test.

>> +		if (host_byte(ret) == 0 && scsi_status_is_good(ret))
>>   			break;
> 
> Also could use a comment: why not just "if (!ret)" ?

The UFS spec says "CONDITION MET - Not used for UFS" so changing the 
above test into "if (!ret)" would be valid. If nobody objects I will 
change the above test into "if (ret == 0)" instead of adding a comment.

Thanks,

Bart.