On 19/08/21 9:14 pm, Bart Van Assche wrote:

On 8/19/21 2:35 AM, Adrian Hunter wrote:

quoted

       * From SPC-6: the REQUEST SENSE command with any allocation length
-     * clears the sense data.
+     * clears the sense data, but not all UFS devices behave that way.
       */

How about removing the comment entirely? Comprehending the above comment is not possible without reviewing the git history so I think it's better to remove it.

Perhaps a comment might stop someone tempted to remove the sense size in the future.  What about:

	/*
	 * Some UFS devices clear unit attention condition only if the sense
	 * size used (UFS_SENSE_SIZE in this case) is non-zero.
	 */

quoted

-    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);

Why has the flags argument been changed from 0 into BLK_MQ_REQ_PM? MODE SENSE is not a power management command.

It is used in a PM path, it is consistent with RQF_PM also used by ufshcd_request_sense_async(), it is what __scsi_execute() does with RQF_PM, so it is what was used before "scsi: ufs: Request sense data asynchronously".