[PATCH 0/2] scsi: ufs: core: Handle devices declining optional Query IDNs

[PATCH 0/2] scsi: ufs: core: Handle devices declining optional Query IDNs

[Stanley Jhu]

JEDEC JESD220F specifies that a device which does not support an
optional Query IDN returns QUERY_RESULT_INVALID_IDN (0xFD) in the
Query Response UPIU.

The driver maps every non-zero Query response to -EINVAL and logs
dev_err(). A device declining an optional IDN is spec-compliant, so
this reports a failure that did not happen and keeps the retry loops
resending a request the device will never accept. On UFS 4.0 parts
that omit the Timestamp attribute it repeats on every boot and every
resume.

A third model was recently proposed for
UFS_DEVICE_QUIRK_NO_TIMESTAMP_SUPPORT; recognising the rejection at
runtime avoids growing that table further.

Patch 1 decodes 0xFD as -EOPNOTSUPP across the query layer. Patch 2
uses that to stop reissuing the Timestamp write on devices that
reject it.

Compile tested on arm64 (defconfig + SCSI_UFSHCD).

Stanley Jhu (2):
  scsi: ufs: core: Fast-abort unsupported Query IDNs
  scsi: ufs: core: Dynamically disable timestamp on unsupported devices

 drivers/ufs/core/ufshcd.c | 39 ++++++++++++++++++++++++++++++---------
 include/ufs/ufs.h         |  1 +
 2 files changed, 31 insertions(+), 9 deletions(-)

-- 
2.55.0.1007.g17ff1f9808-goog

[PATCH 1/2] scsi: ufs: core: Fast-abort unsupported Query IDNs

[Stanley Jhu]

JEDEC JESD220F specifies that devices not supporting an optional
Query IDN return QUERY_RESULT_INVALID_IDN (0xFD) in the Query Response
UPIU.

ufshcd_dev_cmd_completion() maps all non-zero Query responses to
-EINVAL and logs dev_err(). A device declining an optional IDN is
spec-compliant, so this reports an error that did not occur and leaves
the retry loops resending a request the device will never accept.

Decode 0xFD as -EOPNOTSUPP, terminate the query retry loops on it, and
drop the dev_err() in the query wrappers and descriptor readers for
that case. Other errors are logged and retried as before.

The suppressed dev_err() calls carried the opcode and IDN; log them
with dev_dbg() where the response is decoded.

Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 1ca731675ad1..f01703c3825f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3284,6 +3284,11 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 
 		if (response == 0) {
 			err = ufshcd_copy_query_response(hba, lrbp);
+		} else if (response == QUERY_RESULT_INVALID_IDN) {
+			err = -EOPNOTSUPP;
+			dev_dbg(hba->dev, "%s: unsupported opcode 0x%x idn 0x%x\n",
+				__func__, lrbp->ucd_req_ptr->qr.opcode,
+				lrbp->ucd_req_ptr->qr.idn);
 		} else {
 			err = -EINVAL;
 			dev_err(hba->dev, "%s: unexpected response in Query RSP: %x\n",
@@ -3446,6 +3451,8 @@ static int ufshcd_query_flag_retry(struct ufs_hba *hba,
 
 	for (retries = 0; retries < QUERY_REQ_RETRIES; retries++) {
 		ret = ufshcd_query_flag(hba, opcode, idn, index, flag_res);
+		if (ret == -EOPNOTSUPP)
+			return ret;
 		if (ret)
 			dev_dbg(hba->dev,
 				"%s: failed with error %d, retries %d\n",
@@ -3512,7 +3519,8 @@ int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
 	}
 
 	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, timeout);
-
+	if (err == -EOPNOTSUPP)
+		goto out_unlock;
 	if (err) {
 		dev_err(hba->dev,
 			"%s: Sending flag query for idn %d failed, err = %d\n",
@@ -3577,7 +3585,8 @@ int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
 	}
 
 	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
-
+	if (err == -EOPNOTSUPP)
+		goto out_unlock;
 	if (err) {
 		dev_err(hba->dev, "%s: opcode 0x%.2x for idn %d failed, index %d, err = %d\n",
 				__func__, opcode, idn, index, err);
@@ -3615,6 +3624,8 @@ int ufshcd_query_attr_retry(struct ufs_hba *hba,
 	for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
 		ret = ufshcd_query_attr(hba, opcode, idn, index,
 						selector, attr_val);
+		if (ret == -EOPNOTSUPP)
+			return ret;
 		if (ret)
 			dev_dbg(hba->dev, "%s: failed with error %d, retries %d\n",
 				__func__, ret, retries);
@@ -3676,6 +3687,8 @@ int ufshcd_query_attr_qword(struct ufs_hba *hba, enum query_opcode opcode,
 	}
 
 	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
+	if (err == -EOPNOTSUPP)
+		goto out_unlock;
 	if (err) {
 		dev_err(hba->dev, "%s: opcode 0x%.2x for idn %d failed, index %d, selector %d, err = %d\n",
 			__func__, opcode, idn, index, sel, err);
@@ -3739,7 +3752,8 @@ static int __ufshcd_query_descriptor(struct ufs_hba *hba,
 	}
 
 	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
-
+	if (err == -EOPNOTSUPP)
+		goto out_unlock;
 	if (err) {
 		dev_err(hba->dev, "%s: opcode 0x%.2x for idn %d failed, index %d, err = %d\n",
 				__func__, opcode, idn, index, err);
@@ -3782,7 +3796,7 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
 	for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
 		err = __ufshcd_query_descriptor(hba, opcode, idn, index,
 						selector, desc_buf, buf_len);
-		if (!err || err == -EINVAL)
+		if (!err || err == -EINVAL || err == -EOPNOTSUPP)
 			break;
 	}
 
@@ -3831,6 +3845,8 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
 	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
 					    desc_id, desc_index, 0,
 					    desc_buf, &buff_len);
+	if (ret == -EOPNOTSUPP)
+		goto out;
 	if (ret) {
 		dev_err(hba->dev, "%s: Failed reading descriptor. desc_id %d, desc_index %d, param_offset %d, ret %d\n",
 			__func__, desc_id, desc_index, param_offset, ret);
@@ -3917,8 +3933,9 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, u8 desc_index, u8 **buf, enum u
 	ret = ufshcd_read_desc_param(hba, QUERY_DESC_IDN_STRING, desc_index, 0,
 				     (u8 *)uc_str, QUERY_DESC_MAX_SIZE);
 	if (ret < 0) {
-		dev_err(hba->dev, "Reading String Desc failed after %d retries. err = %d\n",
-			QUERY_REQ_RETRIES, ret);
+		if (ret != -EOPNOTSUPP)
+			dev_err(hba->dev, "Reading String Desc failed after %d retries. err = %d\n",
+				QUERY_REQ_RETRIES, ret);
 		str = NULL;
 		goto out;
 	}
-- 
2.55.0.1007.g17ff1f9808-goog

[PATCH 2/2] scsi: ufs: core: Dynamically disable timestamp on unsupported devices

[Stanley Jhu]

JEDEC JESD220F specifies that the Timestamp attribute (qTimestamp) is
an optional feature for UFS devices. Some compliant UFS 4.0 devices do
not implement it and return QUERY_RESULT_INVALID_IDN (0xFD).

ufshcd_set_timestamp_attr() gates the write on wspecversion >= 0x400
and the absence of UFS_DEVICE_QUIRK_NO_TIMESTAMP_SUPPORT. Neither
covers a device that advertises UFS 4.0 but declines the attribute, so
the write is reissued on every boot and every resume.

Record the capability in struct ufs_dev_info and clear it when the
device answers -EOPNOTSUPP, so the write is not retried. -ETIMEDOUT
and -EIO still reach dev_err(); they are transient and must not
disable the attribute.

Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 10 +++++++---
 include/ufs/ufs.h         |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index f01703c3825f..1964d0a23c17 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8962,6 +8962,9 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
 
 	ufs_fixup_device_setup(hba);
 
+	dev_info->timestamp_sup = dev_info->wspecversion >= 0x400 &&
+		!(hba->dev_quirks & UFS_DEVICE_QUIRK_NO_TIMESTAMP_SUPPORT);
+
 	ufshcd_wb_probe(hba, desc_buf);
 
 	ufshcd_temp_notif_probe(hba, desc_buf);
@@ -9262,14 +9265,15 @@ static void ufshcd_set_timestamp_attr(struct ufs_hba *hba)
 	u64 ts_ns;
 	int err;
 
-	if (dev_info->wspecversion < 0x400 ||
-	    hba->dev_quirks & UFS_DEVICE_QUIRK_NO_TIMESTAMP_SUPPORT)
+	if (!dev_info->timestamp_sup)
 		return;
 
 	ts_ns = ktime_get_real_ns();
 	err = ufshcd_query_attr_qword(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
 				      QUERY_ATTR_IDN_TIMESTAMP, 0, 0, &ts_ns);
-	if (err)
+	if (err == -EOPNOTSUPP)
+		dev_info->timestamp_sup = false;
+	else if (err)
 		dev_err(hba->dev, "%s: failed to set timestamp %d\n",
 			__func__, err);
 }
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index afbb32654fab..b0207e694911 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -662,6 +662,7 @@ struct ufs_dev_info {
 	u8 rtt_cap; /* bDeviceRTTCap */
 
 	bool hid_sup;
+	bool timestamp_sup;
 
 	/* Unique device ID string (manufacturer+model+serial+version+date) */
 	char *device_id;
-- 
2.55.0.1007.g17ff1f9808-goog