Re: [PATCH 1/2] scsi: ufs: core: Introduce function ufshcd_query_attr_qword()

Re: [PATCH 1/2] scsi: ufs: core: Introduce function ufshcd_query_attr_qword()

[hoyoung seo]

Hi,

How about you to add EXPORT_SYMBOL at ufshcd_query_attr_qword() function. 
In the case of ufshcd_query_attr(), there is export_symbol so it can be used in vendor driver.
Likewise, if export_symbol is registered in ufshcd_query_attr_qword(), it can be used in the vendor driver and the pair will be correct.

Thanks.

SEO.

Re: [PATCH 1/2] scsi: ufs: core: Introduce function ufshcd_query_attr_qword()

[Can Guo]

Hi,

On 4/21/2026 7:01 PM, hoyoung seo wrote:
> Hi,
>
> How about you to add EXPORT_SYMBOL at ufshcd_query_attr_qword() function.
> In the case of ufshcd_query_attr(), there is export_symbol so it can be used in vendor driver.
> Likewise, if export_symbol is registered in ufshcd_query_attr_qword(), it can be used in the vendor driver and the pair will be correct.
One should export a symbol only if there is a module *really* using it...

Thanks,
Can Guo.
>
> Thanks.
>
> SEO.
>
>

[PATCH 0/2] scsi: ufs: Add persistent TX Equalization settings support

[Can Guo]

Hi,

This series is a follow-up to the earlier TX Equalization enablement series:

https://lore.kernel.org/all/20260325152154.1604082-1-can.guo@oss.qualcomm.com

In that cover letter, the "Next" section mentioned adding support for
UFS v5.0 Attributes qTxEQGnSettings and wTxEQGnSettingsExt, and enabling
persistent storage/retrieval of optimal TX Equalization settings. This
2-patch series implements that part.

Motivation
==========

TX EQTR procedure is required to find the optimal TX Equalization settings
for HS Gears (4-6) before changing Power Mode to the target HS Gears.
However, TX EQTR procedure introduces latencies to the first Power Mode
change.

With optimal TX Equalization settings stored in UFS v5.0 Attributes
qTxEQGnSettings and wTxEQGnSettingsExt, host software can reuse known-good
settings and avoid going through the TX EQTR procedure.

Array Attribute Model
=====================

qTxEQGnSettings and wTxEQGnSettingsExt are array-type Attributes. Each
element in an array-type Attribute is selected by an (Index, Selector) pair.

For these two attributes:
- Valid Index range: [0, Max HS Gear - 1]
- Valid Selector range: [0, 1]

This effectively forms a 2-dimensional array. For HS-Gear n, its TX
Equalization settings are stored/retrieved at Index (n - 1). Selector is
configurable via a module parameter so that platforms can choose the
Selector policy that matches their use.

Implementation Overview
=======================

1. Introduce a generic helper for 64-bit query attributes:
   ufshcd_query_attr_qword().

2. Add TX EQ settings persistence flow:
   - Read stored settings from qTxEQGnSettings & wTxEQGnSettingsExt.
   - Decode and populate per-gear TX EQ parameters.
   - Use Bit[15] in wTxEQGnSettingsExt as validity indication.
   - Store trained settings back to these attributes for future reuse.

3. Integrate with existing lifecycle:
   - Retrieve settings during device parameter initialization.
   - Store settings during shutdown.

New Module Parameters
=====================

Three module parameters are added for TX EQ settings persistence control:
- txeq_setting_sel (default: 0, range: 0..1)
    Selects which selector value is used when reading/writing
    qTxEQGnSettings and wTxEQGnSettingsExt.
- retrieve_txeq_setting (default: true)
    Enables/disables retrieving stored TX EQ settings from device attributes
    during initialization.
- store_txeq_setting (default: true)
    Enables/disables storing last trained TX EQ settings into device attributes
    during shutdown.

Testing
=======

Tested on a UFS v5.0 platform:
- TX Equalization setting store path, settings were correctly encoded and stored.
- TX Equalization setting retrieval path, settings were correctly extracted and reused.
- Full TX EQTR procedure was skipped for a given HS Gear when valid TX EQ settings
  were provided in qTxEQGnSettings & wTxEQGnSettingsExt for the given HS-Gear.


Can Guo (2):
  scsi: ufs: core: Introduce function ufshcd_query_attr_qword()
  scsi: ufs: core: Add support to retrieve and store TX Equalization
    settings

 drivers/ufs/core/ufs-sysfs.c   |  30 +++-
 drivers/ufs/core/ufs-txeq.c    | 241 +++++++++++++++++++++++++++++++++
 drivers/ufs/core/ufshcd-priv.h |   5 +-
 drivers/ufs/core/ufshcd.c      | 131 ++++++++++--------
 include/ufs/ufs.h              |   2 +
 include/ufs/ufshcd.h           |   2 +
 6 files changed, 346 insertions(+), 65 deletions(-)

-- 
2.34.1

[PATCH 1/2] scsi: ufs: core: Introduce function ufshcd_query_attr_qword()

[Can Guo]

Introduce a new generic function ufshcd_query_attr_qword() to handle
quad-word (64-bit) UFS attribute operations. This consolidates the
handling of 64-bit attributes which was previously scattered across
multiple specialized functions.

Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
---
 drivers/ufs/core/ufs-sysfs.c   |  30 ++++++--
 drivers/ufs/core/ufshcd-priv.h |   3 +-
 drivers/ufs/core/ufshcd.c      | 126 +++++++++++++++++----------------
 3 files changed, 94 insertions(+), 65 deletions(-)

diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 99af3c73f1af..61052df2915c 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -594,8 +594,13 @@ static ssize_t device_lvl_exception_id_show(struct device *dev,
 	u64 exception_id;
 	int err;
 
+	if (hba->dev_info.wspecversion < 0x410)
+		return -EOPNOTSUPP;
+
 	ufshcd_rpm_get_sync(hba);
-	err = ufshcd_read_device_lvl_exception_id(hba, &exception_id);
+	err = ufshcd_query_attr_qword(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+				      QUERY_ATTR_IDN_DEV_LVL_EXCEPTION_ID,
+				      0, 0, &exception_id);
 	ufshcd_rpm_put_sync(hba);
 
 	if (err)
@@ -1670,6 +1675,12 @@ static inline bool ufshcd_is_wb_attrs(enum attr_idn idn)
 		idn <= QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE;
 }
 
+static inline bool ufshcd_is_qword_attrs(enum attr_idn idn)
+{
+	return idn == QUERY_ATTR_IDN_TIMESTAMP ||
+	       idn == QUERY_ATTR_IDN_DEV_LVL_EXCEPTION_ID;
+}
+
 static int wb_read_resize_attrs(struct ufs_hba *hba,
 			enum attr_idn idn, u32 *attr_val)
 {
@@ -1736,6 +1747,7 @@ static ssize_t _name##_show(struct device *dev,				\
 	struct device_attribute *attr, char *buf)			\
 {									\
 	struct ufs_hba *hba = dev_get_drvdata(dev);			\
+	u64 qword_value;							\
 	u32 value;							\
 	int ret;							\
 	u8 index = 0;							\
@@ -1748,14 +1760,24 @@ static ssize_t _name##_show(struct device *dev,				\
 	if (ufshcd_is_wb_attrs(QUERY_ATTR_IDN##_uname))			\
 		index = ufshcd_wb_get_query_index(hba);			\
 	ufshcd_rpm_get_sync(hba);					\
-	ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,	\
-		QUERY_ATTR_IDN##_uname, index, 0, &value);		\
+	if (ufshcd_is_qword_attrs(QUERY_ATTR_IDN##_uname))		\
+		ret = ufshcd_query_attr_qword(hba,			\
+			UPIU_QUERY_OPCODE_READ_ATTR,			\
+			QUERY_ATTR_IDN##_uname,				\
+			index, 0, &qword_value);				\
+	else								\
+		ret = ufshcd_query_attr(hba,				\
+			UPIU_QUERY_OPCODE_READ_ATTR,			\
+			QUERY_ATTR_IDN##_uname, index, 0, &value);	\
 	ufshcd_rpm_put_sync(hba);					\
 	if (ret) {							\
 		ret = -EINVAL;						\
 		goto out;						\
 	}								\
-	ret = sysfs_emit(buf, "0x%08X\n", value);			\
+	if (ufshcd_is_qword_attrs(QUERY_ATTR_IDN##_uname))               \
+		ret = sysfs_emit(buf, "0x%016llX\n", qword_value);		\
+	else								\
+		ret = sysfs_emit(buf, "0x%08X\n", value);		\
 out:									\
 	up(&hba->host_sem);						\
 	return ret;							\
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 0a72148cb053..ed1adeb22ec6 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -60,6 +60,8 @@ int ufshcd_query_attr_retry(struct ufs_hba *hba, enum query_opcode opcode,
 			    u32 *attr_val);
 int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
 		      enum attr_idn idn, u8 index, u8 selector, u32 *attr_val);
+int ufshcd_query_attr_qword(struct ufs_hba *hba, enum query_opcode opcode,
+			    enum attr_idn idn, u8 index, u8 sel, u64 *attr_val);
 int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
 	enum flag_idn idn, u8 index, bool *flag_res);
 void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit);
@@ -106,7 +108,6 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
 			     enum query_opcode desc_op);
 
 int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable);
-int ufshcd_read_device_lvl_exception_id(struct ufs_hba *hba, u64 *exception_id);
 
 int ufshcd_uic_tx_eqtr(struct ufs_hba *hba, int gear);
 void ufshcd_apply_valid_tx_eq_settings(struct ufs_hba *hba);
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 4805e40ed4d7..e5e22adcbbc3 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3611,6 +3611,67 @@ int ufshcd_query_attr_retry(struct ufs_hba *hba,
 	return ret;
 }
 
+/**
+ * ufshcd_query_attr_qword - API function of sending query requests for quad-word attributes
+ * @hba: per-adapter instance
+ * @opcode: attribute opcode
+ * @idn: attribute idn to access
+ * @index: index field
+ * @sel: selector field
+ * @attr_val: the attribute value after the query request completes
+ *
+ * Return: 0 for success, non-zero in case of failure.
+ */
+int ufshcd_query_attr_qword(struct ufs_hba *hba, enum query_opcode opcode,
+			    enum attr_idn idn, u8 index, u8 sel, u64 *attr_val)
+{
+	struct utp_upiu_query_v4_0 *upiu_req;
+	struct utp_upiu_query_v4_0 *upiu_resp;
+	struct ufs_query_req *request = NULL;
+	struct ufs_query_res *response = NULL;
+	int err;
+
+	if (!attr_val) {
+		dev_err(hba->dev, "%s: attribute value required for opcode 0x%x\n",
+			__func__, opcode);
+		return -EINVAL;
+	}
+
+	ufshcd_dev_man_lock(hba);
+
+	ufshcd_init_query(hba, &request, &response, opcode, idn, index, sel);
+
+	switch (opcode) {
+	case UPIU_QUERY_OPCODE_WRITE_ATTR:
+		request->query_func = UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST;
+		upiu_req = (struct utp_upiu_query_v4_0 *)&request->upiu_req;
+		put_unaligned_be64(*attr_val, &upiu_req->osf3);
+		break;
+	case UPIU_QUERY_OPCODE_READ_ATTR:
+		request->query_func = UPIU_QUERY_FUNC_STANDARD_READ_REQUEST;
+		break;
+	default:
+		dev_err(hba->dev, "%s: Expected query attr opcode but got = 0x%.2x\n",
+			__func__, opcode);
+		err = -EINVAL;
+		goto out_unlock;
+	}
+
+	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
+	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);
+		goto out_unlock;
+	}
+
+	upiu_resp = (struct utp_upiu_query_v4_0 *)response;
+	*attr_val = get_unaligned_be64(&upiu_resp->osf3);
+
+out_unlock:
+	ufshcd_dev_man_unlock(hba);
+	return err;
+}
+
 /*
  * Return: 0 upon success; > 0 in case the UFS device reported an OCS error;
  * < 0 if another error occurred.
@@ -6224,46 +6285,6 @@ static void ufshcd_bkops_exception_event_handler(struct ufs_hba *hba)
 				__func__, err);
 }
 
-/*
- * Return: 0 upon success; > 0 in case the UFS device reported an OCS error;
- * < 0 if another error occurred.
- */
-int ufshcd_read_device_lvl_exception_id(struct ufs_hba *hba, u64 *exception_id)
-{
-	struct utp_upiu_query_v4_0 *upiu_resp;
-	struct ufs_query_req *request = NULL;
-	struct ufs_query_res *response = NULL;
-	int err;
-
-	if (hba->dev_info.wspecversion < 0x410)
-		return -EOPNOTSUPP;
-
-	ufshcd_hold(hba);
-	mutex_lock(&hba->dev_cmd.lock);
-
-	ufshcd_init_query(hba, &request, &response,
-			  UPIU_QUERY_OPCODE_READ_ATTR,
-			  QUERY_ATTR_IDN_DEV_LVL_EXCEPTION_ID, 0, 0);
-
-	request->query_func = UPIU_QUERY_FUNC_STANDARD_READ_REQUEST;
-
-	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
-
-	if (err) {
-		dev_err(hba->dev, "%s: failed to read device level exception %d\n",
-			__func__, err);
-		goto out;
-	}
-
-	upiu_resp = (struct utp_upiu_query_v4_0 *)response;
-	*exception_id = get_unaligned_be64(&upiu_resp->osf3);
-out:
-	mutex_unlock(&hba->dev_cmd.lock);
-	ufshcd_release(hba);
-
-	return err;
-}
-
 static int __ufshcd_wb_toggle(struct ufs_hba *hba, bool set, enum flag_idn idn)
 {
 	u8 index;
@@ -9113,35 +9134,20 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
 
 static void ufshcd_set_timestamp_attr(struct ufs_hba *hba)
 {
-	int err;
-	struct ufs_query_req *request = NULL;
-	struct ufs_query_res *response = NULL;
 	struct ufs_dev_info *dev_info = &hba->dev_info;
-	struct utp_upiu_query_v4_0 *upiu_data;
+	u64 ts_ns;
+	int err;
 
 	if (dev_info->wspecversion < 0x400 ||
 	    hba->dev_quirks & UFS_DEVICE_QUIRK_NO_TIMESTAMP_SUPPORT)
 		return;
 
-	ufshcd_dev_man_lock(hba);
-
-	ufshcd_init_query(hba, &request, &response,
-			  UPIU_QUERY_OPCODE_WRITE_ATTR,
-			  QUERY_ATTR_IDN_TIMESTAMP, 0, 0);
-
-	request->query_func = UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST;
-
-	upiu_data = (struct utp_upiu_query_v4_0 *)&request->upiu_req;
-
-	put_unaligned_be64(ktime_get_real_ns(), &upiu_data->osf3);
-
-	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
-
+	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)
 		dev_err(hba->dev, "%s: failed to set timestamp %d\n",
 			__func__, err);
-
-	ufshcd_dev_man_unlock(hba);
 }
 
 /**
-- 
2.34.1

Re: [PATCH 1/2] scsi: ufs: core: Introduce function ufshcd_query_attr_qword()

[Peter Wang (王信友)]

On Sun, 2026-04-19 at 06:52 -0700, Can Guo wrote:
> Introduce a new generic function ufshcd_query_attr_qword() to handle
> quad-word (64-bit) UFS attribute operations. This consolidates the
> handling of 64-bit attributes which was previously scattered across
> multiple specialized functions.
> 
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
> ---

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 1/2] scsi: ufs: core: Introduce function ufshcd_query_attr_qword()

[Bart Van Assche]

On 4/19/26 6:52 AM, Can Guo wrote:
> +static inline bool ufshcd_is_qword_attrs(enum attr_idn idn)
> +{
> +	return idn == QUERY_ATTR_IDN_TIMESTAMP ||
> +	       idn == QUERY_ATTR_IDN_DEV_LVL_EXCEPTION_ID;
> +}

Please change "ufshcd_is_qword_attrs()" into "ufshcd_is_qword_attr()".

> +/**
> + * ufshcd_query_attr_qword - API function of sending query requests for quad-word attributes
> + * @hba: per-adapter instance
> + * @opcode: attribute opcode
> + * @idn: attribute idn to access
> + * @index: index field
> + * @sel: selector field
> + * @attr_val: the attribute value after the query request completes
> + *
> + * Return: 0 for success, non-zero in case of failure.
> + */

The word "API" is uncommon in the first line of kernel-doc headers.
Please remove it.

Otherwise this patch looks good to me.

Thanks,

Bart.

Re: [PATCH 1/2] scsi: ufs: core: Introduce function ufshcd_query_attr_qword()

[Can Guo]

On 4/21/2026 12:58 AM, Bart Van Assche wrote:
> On 4/19/26 6:52 AM, Can Guo wrote:
>> +static inline bool ufshcd_is_qword_attrs(enum attr_idn idn)
>> +{
>> +    return idn == QUERY_ATTR_IDN_TIMESTAMP ||
>> +           idn == QUERY_ATTR_IDN_DEV_LVL_EXCEPTION_ID;
>> +}
>
> Please change "ufshcd_is_qword_attrs()" into "ufshcd_is_qword_attr()".
Done.
>
>> +/**
>> + * ufshcd_query_attr_qword - API function of sending query requests 
>> for quad-word attributes
>> + * @hba: per-adapter instance
>> + * @opcode: attribute opcode
>> + * @idn: attribute idn to access
>> + * @index: index field
>> + * @sel: selector field
>> + * @attr_val: the attribute value after the query request completes
>> + *
>> + * Return: 0 for success, non-zero in case of failure.
>> + */
>
> The word "API" is uncommon in the first line of kernel-doc headers.
> Please remove it.
Done.

Thanks,
Can Guo.
>
> Otherwise this patch looks good to me.
>
> Thanks,
>
> Bart.

Re: [PATCH 1/2] scsi: ufs: core: Introduce function ufshcd_query_attr_qword()

[Bean Huo]

Can,


On Sun, 2026-04-19 at 06:52 -0700, Can Guo wrote:
>  static int wb_read_resize_attrs(struct ufs_hba *hba,
>                         enum attr_idn idn, u32 *attr_val)
>  {
> @@ -1736,6 +1747,7 @@ static ssize_t _name##_show(struct device
> *dev,                           \
>         struct device_attribute *attr, char *buf)                       \
>  {                                                                      \
>         struct ufs_hba *hba = dev_get_drvdata(dev);                     \
> +       u64
> qword_value;                                                        \

u64 qword_value __maybe_unused;


>         u32 value;                                                      \
>         int ret;                                                        \
>         u8 index = 0;                                                   \
> @@ -1748,14 +1760,24 @@ static ssize_t _name##_show(struct device
> *dev,                         \
>         if (ufshcd_is_wb_attrs(QUERY_ATTR_IDN##_uname))                 \
>                 index = ufshcd_wb_get_query_index(hba);                 \
>         ufshcd_rpm_get_sync(hba);                                       \
> -       ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,       \
> -               QUERY_ATTR_IDN##_uname, index, 0, &value);              \
> +       if (ufshcd_is_qword_attrs(QUERY_ATTR_IDN##_uname))              \
> +               ret = ufshcd_query_attr_qword(hba,                      \
> +                       UPIU_QUERY_OPCODE_READ_ATTR,                    \
> +                       QUERY_ATTR_IDN##_uname,                         \
> +                       index, 0,
> &qword_value);                                \
> +       else                                                            \
> +               ret = ufshcd_query_attr(hba,                            \
> +                       UPIU_QUERY_OPCODE_READ_ATTR,                    \
> +                       QUERY_ATTR_IDN##_uname, index, 0, &value);      \
>         ufshcd_rpm_put_sync(hba);                                       \
>         if (ret) {                                                      \
>                 ret = -EINVAL;                                          \
>                 goto out;                                               \
>         }                                                               \
> -       ret = sysfs_emit(buf, "0x%08X\n", value);                       \
> +       if (ufshcd_is_qword_attrs(QUERY_ATTR_IDN##_uname))               \
> +               ret = sysfs_emit(buf, "0x%016llX\n",
> qword_value);              \
> +       else                                                            \
> +               ret = sysfs_emit(buf, "0x%08X\n", value);               \
>  out:                                                                   \
>         up(&hba->host_sem);                                             \
>         return ret;                                                     \
> 

...


>  
> +/**
> + * ufshcd_query_attr_qword - API function of sending query requests for quad-
> word attributes
> + * @hba: per-adapter instance
> + * @opcode: attribute opcode
> + * @idn: attribute idn to access
> + * @index: index field
> + * @sel: selector field
> + * @attr_val: the attribute value after the query request completes
> + *
> + * Return: 0 for success, non-zero in case of failure.
> + */
> +int ufshcd_query_attr_qword(struct ufs_hba *hba, enum query_opcode opcode,
> +                           enum attr_idn idn, u8 index, u8 sel, u64
> *attr_val)
> +{
> +       struct utp_upiu_query_v4_0 *upiu_req;
> +       struct utp_upiu_query_v4_0 *upiu_resp;
> +       struct ufs_query_req *request = NULL;
> +       struct ufs_query_res *response = NULL;
> +       int err;
> +
> +       if (!attr_val) {
> +               dev_err(hba->dev, "%s: attribute value required for opcode
> 0x%x\n",
> +                       __func__, opcode);
> +               return -EINVAL;
> +       }
> +
> +       ufshcd_dev_man_lock(hba);
> +
> +       ufshcd_init_query(hba, &request, &response, opcode, idn, index, sel);
> +
> +       switch (opcode) {
> +       case UPIU_QUERY_OPCODE_WRITE_ATTR:
> +               request->query_func = UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST;
> +               upiu_req = (struct utp_upiu_query_v4_0 *)&request->upiu_req;
> +               put_unaligned_be64(*attr_val, &upiu_req->osf3);
> +               break;
> +       case UPIU_QUERY_OPCODE_READ_ATTR:
> +               request->query_func = UPIU_QUERY_FUNC_STANDARD_READ_REQUEST;
> +               break;
> +       default:
> +               dev_err(hba->dev, "%s: Expected query attr opcode but got =
> 0x%.2x\n",
> +                       __func__, opcode);
> +               err = -EINVAL;
> +               goto out_unlock;
> +       }
> +
> +       err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
> +       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);
> +               goto out_unlock;
> +       }
> +
> +       upiu_resp = (struct utp_upiu_query_v4_0 *)response;
> +       *attr_val = get_unaligned_be64(&upiu_resp->osf3);
> +
> +out_unlock:
> +       ufshcd_dev_man_unlock(hba);
> +       return err;
> +}
> +

this needs a wrapper for retry?  In ufshcd_extract_tx_eq_settings_attrs(), the
32-bit dTxEQGnSettingsExt read uses ufshcd_query_attr_retry(), but the 64-bit
qTxEQGnSettings read uses bare ufshcd_query_attr_qword() with no retries.

Kind regards,
Bean

Re: [PATCH 1/2] scsi: ufs: core: Introduce function ufshcd_query_attr_qword()

[Can Guo]

On 4/21/2026 6:04 AM, Bean Huo wrote:
> Can,
>
>
> On Sun, 2026-04-19 at 06:52 -0700, Can Guo wrote:
>>   static int wb_read_resize_attrs(struct ufs_hba *hba,
>>                          enum attr_idn idn, u32 *attr_val)
>>   {
>> @@ -1736,6 +1747,7 @@ static ssize_t _name##_show(struct device
>> *dev,                           \
>>          struct device_attribute *attr, char *buf)                       \
>>   {                                                                      \
>>          struct ufs_hba *hba = dev_get_drvdata(dev);                     \
>> +       u64
>> qword_value;                                                        \
> u64 qword_value __maybe_unused;
>
>
>>          u32 value;                                                      \
>>          int ret;                                                        \
>>          u8 index = 0;                                                   \
>> @@ -1748,14 +1760,24 @@ static ssize_t _name##_show(struct device
>> *dev,                         \
>>          if (ufshcd_is_wb_attrs(QUERY_ATTR_IDN##_uname))                 \
>>                  index = ufshcd_wb_get_query_index(hba);                 \
>>          ufshcd_rpm_get_sync(hba);                                       \
>> -       ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,       \
>> -               QUERY_ATTR_IDN##_uname, index, 0, &value);              \
>> +       if (ufshcd_is_qword_attrs(QUERY_ATTR_IDN##_uname))              \
>> +               ret = ufshcd_query_attr_qword(hba,                      \
>> +                       UPIU_QUERY_OPCODE_READ_ATTR,                    \
>> +                       QUERY_ATTR_IDN##_uname,                         \
>> +                       index, 0,
>> &qword_value);                                \
>> +       else                                                            \
>> +               ret = ufshcd_query_attr(hba,                            \
>> +                       UPIU_QUERY_OPCODE_READ_ATTR,                    \
>> +                       QUERY_ATTR_IDN##_uname, index, 0, &value);      \
>>          ufshcd_rpm_put_sync(hba);                                       \
>>          if (ret) {                                                      \
>>                  ret = -EINVAL;                                          \
>>                  goto out;                                               \
>>          }                                                               \
>> -       ret = sysfs_emit(buf, "0x%08X\n", value);                       \
>> +       if (ufshcd_is_qword_attrs(QUERY_ATTR_IDN##_uname))               \
>> +               ret = sysfs_emit(buf, "0x%016llX\n",
>> qword_value);              \
>> +       else                                                            \
>> +               ret = sysfs_emit(buf, "0x%08X\n", value);               \
>>   out:                                                                   \
>>          up(&hba->host_sem);                                             \
>>          return ret;                                                     \
>>
> ...
>
>
>>   
>> +/**
>> + * ufshcd_query_attr_qword - API function of sending query requests for quad-
>> word attributes
>> + * @hba: per-adapter instance
>> + * @opcode: attribute opcode
>> + * @idn: attribute idn to access
>> + * @index: index field
>> + * @sel: selector field
>> + * @attr_val: the attribute value after the query request completes
>> + *
>> + * Return: 0 for success, non-zero in case of failure.
>> + */
>> +int ufshcd_query_attr_qword(struct ufs_hba *hba, enum query_opcode opcode,
>> +                           enum attr_idn idn, u8 index, u8 sel, u64
>> *attr_val)
>> +{
>> +       struct utp_upiu_query_v4_0 *upiu_req;
>> +       struct utp_upiu_query_v4_0 *upiu_resp;
>> +       struct ufs_query_req *request = NULL;
>> +       struct ufs_query_res *response = NULL;
>> +       int err;
>> +
>> +       if (!attr_val) {
>> +               dev_err(hba->dev, "%s: attribute value required for opcode
>> 0x%x\n",
>> +                       __func__, opcode);
>> +               return -EINVAL;
>> +       }
>> +
>> +       ufshcd_dev_man_lock(hba);
>> +
>> +       ufshcd_init_query(hba, &request, &response, opcode, idn, index, sel);
>> +
>> +       switch (opcode) {
>> +       case UPIU_QUERY_OPCODE_WRITE_ATTR:
>> +               request->query_func = UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST;
>> +               upiu_req = (struct utp_upiu_query_v4_0 *)&request->upiu_req;
>> +               put_unaligned_be64(*attr_val, &upiu_req->osf3);
>> +               break;
>> +       case UPIU_QUERY_OPCODE_READ_ATTR:
>> +               request->query_func = UPIU_QUERY_FUNC_STANDARD_READ_REQUEST;
>> +               break;
>> +       default:
>> +               dev_err(hba->dev, "%s: Expected query attr opcode but got =
>> 0x%.2x\n",
>> +                       __func__, opcode);
>> +               err = -EINVAL;
>> +               goto out_unlock;
>> +       }
>> +
>> +       err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
>> +       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);
>> +               goto out_unlock;
>> +       }
>> +
>> +       upiu_resp = (struct utp_upiu_query_v4_0 *)response;
>> +       *attr_val = get_unaligned_be64(&upiu_resp->osf3);
>> +
>> +out_unlock:
>> +       ufshcd_dev_man_unlock(hba);
>> +       return err;
>> +}
>> +
> this needs a wrapper for retry?  In ufshcd_extract_tx_eq_settings_attrs(), the
> 32-bit dTxEQGnSettingsExt read uses ufshcd_query_attr_retry(), but the 64-bit
> qTxEQGnSettings read uses bare ufshcd_query_attr_qword() with no retries.
I will use the non-retry version in ufs_txeq.c to be symmetrical, 
because I don't really need
retry - retry never helped in real cases.

Thanks,
Can Guo.
>
> Kind regards,
> Bean