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

[Bean Huo <beanhuo@iokpp.de>]

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