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

From: Bart Van Assche <bvanassche@acm.org>
To: Avri Altman <Avri.Altman@wdc.com>,
	"Martin K . Petersen" <martin.petersen@oracle.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	Bean Huo <beanhuo@micron.com>,
	Jinyoung Choi <j-young.choi@samsung.com>
Subject: Re: [PATCH 5/8] scsi: ufs: Try harder to change the power mode
Date: Tue, 27 Sep 2022 09:44:17 -0700	[thread overview]
Message-ID: <8da1c1db-1ef4-d2c7-2bed-8c475d008b3c@acm.org> (raw)
In-Reply-To: <BL0PR04MB65646BEA3CADD907E21C64B5FC559@BL0PR04MB6564.namprd04.prod.outlook.com>

On 9/27/22 03:41, Avri Altman 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;
>
> continue?

Hi Avri,

Thanks for having taken a look. I chose "break" on purpose since the 
only case for which I expect scsi_execute() to return a negative value 
is request queue shutdown. If the request queue is being shutdown I 
think we should break out of the loop.

Thanks,

Bart.