[V2] scsi: ufs: Fix ufshcd_request_sense_async() for Samsung KLUFG8RHDA-B2D1

Commit Message

Adrian Hunter Aug. 23, 2021, 5:01 a.m. UTC

From: Adrian Hunter <[e-mail]>

Samsung KLUFG8RHDA-B2D1 does not clear the unit attention condition if the
length is zero. So go back to requesting all the sense data, as it was
before patch "scsi: ufs: Request sense data asynchronously". That is
simpler than creating and maintaining a quirk for affected devices.

Signed-off-by: Adrian Hunter <[e-mail]>
---


Changes in V2

	Alter comment to note the need for non-zero sense size.


 drivers/scsi/ufs/ufshcd.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

Comments

Bart Van Assche Aug. 24, 2021, 2:27 a.m. UTC | #1

On 8/22/21 10:01 PM, Adrian Hunter wrote:
> Samsung KLUFG8RHDA-B2D1 does not clear the unit attention condition if the
> length is zero. So go back to requesting all the sense data, as it was
> before patch "scsi: ufs: Request sense data asynchronously". That is
> simpler than creating and maintaining a quirk for affected devices.

Reviewed-by: Bart Van Assche <[e-mail]>

Martin K. Petersen Aug. 24, 2021, 3:34 a.m. UTC | #2

Adrian,

> Samsung KLUFG8RHDA-B2D1 does not clear the unit attention condition if
> the length is zero. So go back to requesting all the sense data, as it
> was before patch "scsi: ufs: Request sense data asynchronously". That
> is simpler than creating and maintaining a quirk for affected devices.

Applied to 5.15/scsi-staging, thanks!

Adrian Hunter Aug. 24, 2021, 11:34 a.m. UTC | #3

On 24/08/21 6:34 am, Martin K. Petersen wrote:
> 
> Adrian,
> 
>> Samsung KLUFG8RHDA-B2D1 does not clear the unit attention condition if
>> the length is zero. So go back to requesting all the sense data, as it
>> was before patch "scsi: ufs: Request sense data asynchronously". That
>> is simpler than creating and maintaining a quirk for affected devices.
> 
> Applied to 5.15/scsi-staging, thanks!
> 

Can you please drop V2 of this patch?  I will send a V3 with the buffer leak fixed.

Patch

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a3b419848f0a..9d6207f7cf6d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7937,7 +7937,8 @@  static int ufshcd_add_lus(struct ufs_hba *hba)
 static void ufshcd_request_sense_done(struct request *rq, blk_status_t error)
 {
 	if (error != BLK_STS_OK)
-		pr_err("%s: REQUEST SENSE failed (%d)", __func__, error);
+		pr_err("%s: REQUEST SENSE failed (%d)\n", __func__, error);
+	kfree(rq->end_io_data);
 	blk_put_request(rq);
 }
 
@@ -7945,23 +7946,39 @@  static int
 ufshcd_request_sense_async(struct ufs_hba *hba, struct scsi_device *sdev)
 {
 	/*
-	 * From SPC-6: the REQUEST SENSE command with any allocation length
-	 * clears the sense data.
+	 * Some UFS devices clear unit attention condition only if the sense
+	 * size used (UFS_SENSE_SIZE in this case) is non-zero.
 	 */
-	static const u8 cmd[6] = {REQUEST_SENSE, 0, 0, 0, 0, 0};
+	static const u8 cmd[6] = {REQUEST_SENSE, 0, 0, 0, UFS_SENSE_SIZE, 0};
 	struct scsi_request *rq;
 	struct request *req;
+	char *buffer;
+	int ret;
+
+	buffer = kzalloc(UFS_SENSE_SIZE, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
 
-	req = blk_get_request(sdev->request_queue, REQ_OP_DRV_IN, /*flags=*/0);
+	req = blk_get_request(sdev->request_queue, REQ_OP_DRV_IN,
+			      /*flags=*/BLK_MQ_REQ_PM);
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 
+	ret = blk_rq_map_kern(sdev->request_queue, req,
+			      buffer, UFS_SENSE_SIZE, GFP_NOIO);
+	if (ret) {
+		blk_put_request(req);
+		kfree(buffer);
+		return ret;
+	}
+
 	rq = scsi_req(req);
 	rq->cmd_len = ARRAY_SIZE(cmd);
 	memcpy(rq->cmd, cmd, rq->cmd_len);
 	rq->retries = 3;
 	req->timeout = 1 * HZ;
 	req->rq_flags |= RQF_PM | RQF_QUIET;
+	req->end_io_data = buffer;
 
 	blk_execute_rq_nowait(/*bd_disk=*/NULL, req, /*at_head=*/true,
 			      ufshcd_request_sense_done);