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

[Can Guo <can.guo@oss.qualcomm.com>]

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