[PATCH 0/2] ufs: Add support for setting bRefClkFreq attribute

[PATCH 0/2] ufs: Add support for setting bRefClkFreq attribute

[Jared McArthur]

Currently, U-Boot doesn't set the bRefClkFreq attribute before
changing power mode of the UFS device. If there is a difference
between the UFS device and the host controller, all commands will fail
after switching to high speed.

This behavior is rarely observed, because Linux sets the bRefClkFreq
attribute on probe, and the bRefClkFreq is a persistent attribute. In
other words, once Linux has booted once, the issue will never be seen.

If trying to provision and boot from an unprovisioned UFS device,
U-Boot will fail. Fix this by adding support for setting the
bRefClkFreq attribute.

Jared McArthur (2):
  ufs: Add support for sending UFS attribute requests
  ufs: Add bRefClkFreq attribute setting

 drivers/ufs/ufs.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/ufs/ufs.h |  10 +++
 2 files changed, 194 insertions(+)

-- 
2.34.1

[PATCH 1/2] ufs: Add support for sending UFS attribute requests

[Jared McArthur]

Some UFS attributes must be set before a UFS device is initialized.
Add ufshcd_query_attr and ufshcd_query_attr_retry to send UFS
attribute requests.

Taken from Linux Kernel v6.17 (drivers/ufs/core/ufshcd.c) and ported
to U-Boot.

Signed-off-by: Jared McArthur <j-mcarthur@ti.com>
---
 drivers/ufs/ufs.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index 91f6ad3bfef..9e67dd86232 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -1107,6 +1107,101 @@ static int ufshcd_query_flag_retry(struct ufs_hba *hba,
 	return ret;
 }
 
+/**
+ * ufshcd_query_attr - API function for sending attribute requests
+ * @hba: per-adapter instance
+ * @opcode: attribute opcode
+ * @idn: attribute idn to access
+ * @index: index field
+ * @selector: 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(struct ufs_hba *hba, enum query_opcode opcode,
+		      enum attr_idn idn, u8 index, u8 selector, u32 *attr_val)
+{
+	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_init_query(hba, &request, &response, opcode, idn, index, selector);
+
+	switch (opcode) {
+	case UPIU_QUERY_OPCODE_WRITE_ATTR:
+		request->query_func = UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST;
+		request->upiu_req.value = cpu_to_be32(*attr_val);
+		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;
+	}
+
+	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, QUERY_REQ_TIMEOUT);
+
+	if (err) {
+		dev_err(hba->dev,
+			"%s: opcode 0x%.2x for idn %d failed, index %d, err = %d\n",
+			__func__, opcode, idn, index, err);
+		goto out;
+	}
+
+	*attr_val = be32_to_cpu(response->upiu_res.value);
+
+out:
+	return err;
+}
+
+/**
+ * ufshcd_query_attr_retry() - API function for sending query
+ * attribute with retries
+ * @hba: per-adapter instance
+ * @opcode: attribute opcode
+ * @idn: attribute idn to access
+ * @index: index field
+ * @selector: 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_retry(struct ufs_hba *hba, enum query_opcode opcode,
+			    enum attr_idn idn, u8 index, u8 selector,
+			    u32 *attr_val)
+{
+	int ret = 0;
+	u32 retries;
+
+	for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
+		ret = ufshcd_query_attr(hba, opcode, idn, index, selector, attr_val);
+		if (ret)
+			dev_dbg(hba->dev,
+				"%s: failed with error %d, retries %d\n",
+				__func__, ret, retries);
+		else
+			break;
+	}
+
+	if (ret)
+		dev_err(hba->dev,
+			"%s: query attribute, idn %d, failed with error %d after %d retries\n",
+			__func__, idn, ret, QUERY_REQ_RETRIES);
+	return ret;
+}
+
 static int __ufshcd_query_descriptor(struct ufs_hba *hba,
 				     enum query_opcode opcode,
 				     enum desc_idn idn, u8 index, u8 selector,
-- 
2.34.1

[PATCH 2/2] ufs: Add bRefClkFreq attribute setting

[Jared McArthur]

A UFS device needs its bRefClkFreq attribute set to the correct value
before switching to high speed. If bRefClkFreq is set to the wrong
value, all transactions after the power mode change will fail.

The bRefClkFreq depends on the host controller and the device.
Query the device's current bRefClkFreq and compare with the ref_clk
specified in the device-tree. If the two differ, set the bRefClkFreq
to the device-tree's ref_clk frequency.

Taken from Linux kernel v6.17 (drivers/ufs/core/ufshcd.c and
include/ufs/ufs.h) and ported to U-Boot.

This patch depends on the previous patch: "ufs: Add support for
sending UFS attribute requests"

Signed-off-by: Jared McArthur <j-mcarthur@ti.com>
---
 drivers/ufs/ufs.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/ufs/ufs.h | 10 ++++++
 2 files changed, 99 insertions(+)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index 9e67dd86232..4bffbf87749 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -10,6 +10,7 @@
 
 #include <bouncebuf.h>
 #include <charset.h>
+#include <clk.h>
 #include <dm.h>
 #include <log.h>
 #include <dm/device_compat.h>
@@ -1773,6 +1774,92 @@ out:
 	return err;
 }
 
+struct ufs_ref_clk {
+	unsigned long freq_hz;
+	enum ufs_ref_clk_freq val;
+};
+
+static const struct ufs_ref_clk ufs_ref_clk_freqs[] = {
+	{19200000, REF_CLK_FREQ_19_2_MHZ},
+	{26000000, REF_CLK_FREQ_26_MHZ},
+	{38400000, REF_CLK_FREQ_38_4_MHZ},
+	{52000000, REF_CLK_FREQ_52_MHZ},
+	{0, REF_CLK_FREQ_INVAL},
+};
+
+static enum ufs_ref_clk_freq
+ufs_get_bref_clk_from_hz(unsigned long freq)
+{
+	int i;
+
+	for (i = 0; ufs_ref_clk_freqs[i].freq_hz; i++)
+		if (ufs_ref_clk_freqs[i].freq_hz == freq)
+			return ufs_ref_clk_freqs[i].val;
+
+	return REF_CLK_FREQ_INVAL;
+}
+
+void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba, struct clk *refclk)
+{
+	unsigned long freq;
+
+	freq = clk_get_rate(refclk);
+
+	hba->dev_ref_clk_freq =
+		ufs_get_bref_clk_from_hz(freq);
+
+	if (hba->dev_ref_clk_freq == REF_CLK_FREQ_INVAL)
+		dev_err(hba->dev,
+			"invalid ref_clk setting = %ld\n", freq);
+}
+
+static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
+{
+	int err;
+	struct clk *ref_clk;
+	u32 host_ref_clk_freq;
+	u32 dev_ref_clk_freq;
+
+	/* get ref_clk */
+	ref_clk = devm_clk_get(hba->dev, "ref_clk");
+	if (IS_ERR((const void *)ref_clk)) {
+		err = PTR_ERR(ref_clk);
+		goto out;
+	}
+
+	ufshcd_parse_dev_ref_clk_freq(hba, ref_clk);
+	host_ref_clk_freq = hba->dev_ref_clk_freq;
+
+	if (host_ref_clk_freq == REF_CLK_FREQ_INVAL)
+		goto out;
+
+	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+				      QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0, &dev_ref_clk_freq);
+
+	if (err) {
+		dev_err(hba->dev, "failed reading bRefClkFreq. err = %d\n", err);
+		goto out;
+	}
+
+	if (dev_ref_clk_freq == host_ref_clk_freq)
+		goto out; /* nothing to update */
+
+	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
+				      QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0, &host_ref_clk_freq);
+
+	if (err) {
+		dev_err(hba->dev, "bRefClkFreq setting to %lu Hz failed\n",
+			ufs_ref_clk_freqs[host_ref_clk_freq].freq_hz);
+		goto out;
+	}
+
+	dev_dbg(hba->dev, "bRefClkFreq setting to %lu Hz succeeded\n",
+		ufs_ref_clk_freqs[host_ref_clk_freq].freq_hz);
+
+out:
+	return err;
+}
+
 /**
  * ufshcd_get_max_pwr_mode - reads the max power mode negotiated with device
  */
@@ -2000,6 +2087,8 @@ static int ufs_start(struct ufs_hba *hba)
 		return ret;
 	}
 
+	ufshcd_set_dev_ref_clk(hba);
+
 	if (ufshcd_get_max_pwr_mode(hba)) {
 		dev_err(hba->dev,
 			"%s: Failed getting max supported power mode\n",
diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
index 53137fae3a8..def39d4ad24 100644
--- a/drivers/ufs/ufs.h
+++ b/drivers/ufs/ufs.h
@@ -172,6 +172,15 @@ enum query_opcode {
 	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
 };
 
+/* bRefClkFreq attribute values */
+enum ufs_ref_clk_freq {
+	REF_CLK_FREQ_19_2_MHZ	= 0,
+	REF_CLK_FREQ_26_MHZ	= 1,
+	REF_CLK_FREQ_38_4_MHZ	= 2,
+	REF_CLK_FREQ_52_MHZ	= 3,
+	REF_CLK_FREQ_INVAL	= -1,
+};
+
 /* Query response result code */
 enum {
 	QUERY_RESULT_SUCCESS                    = 0x00,
@@ -684,6 +693,7 @@ struct ufs_hba {
 	u32			capabilities;
 	u32			version;
 	u32			intr_mask;
+	enum ufs_ref_clk_freq dev_ref_clk_freq;
 	enum ufshcd_quirks	quirks;
 
 	/* Virtual memory reference */
-- 
2.34.1

Re: [PATCH 0/2] ufs: Add support for setting bRefClkFreq attribute

[Bryan Brattlof]

On August 27, 2025 thus sayeth Jared McArthur:
> Currently, U-Boot doesn't set the bRefClkFreq attribute before
> changing power mode of the UFS device. If there is a difference
> between the UFS device and the host controller, all commands will fail
> after switching to high speed.
> 
> This behavior is rarely observed, because Linux sets the bRefClkFreq
> attribute on probe, and the bRefClkFreq is a persistent attribute. In
> other words, once Linux has booted once, the issue will never be seen.
> 
> If trying to provision and boot from an unprovisioned UFS device,
> U-Boot will fail. Fix this by adding support for setting the
> bRefClkFreq attribute.
> 
> Jared McArthur (2):
>   ufs: Add support for sending UFS attribute requests
>   ufs: Add bRefClkFreq attribute setting
> 
>  drivers/ufs/ufs.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/ufs/ufs.h |  10 +++
>  2 files changed, 194 insertions(+)

Reviewed-by: Bryan Brattlof <bb@ti.com>

~Bryan

Re: [PATCH 1/2] ufs: Add support for sending UFS attribute requests

[Neil Armstrong]

On 27/08/2025 20:50, Jared McArthur wrote:
> Some UFS attributes must be set before a UFS device is initialized.
> Add ufshcd_query_attr and ufshcd_query_attr_retry to send UFS
> attribute requests.
> 
> Taken from Linux Kernel v6.17 (drivers/ufs/core/ufshcd.c) and ported
> to U-Boot.
> 
> Signed-off-by: Jared McArthur <j-mcarthur@ti.com>
> ---
>   drivers/ufs/ufs.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 95 insertions(+)
> 
> diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
> index 91f6ad3bfef..9e67dd86232 100644
> --- a/drivers/ufs/ufs.c
> +++ b/drivers/ufs/ufs.c
> @@ -1107,6 +1107,101 @@ static int ufshcd_query_flag_retry(struct ufs_hba *hba,
>   	return ret;
>   }
>   
> +/**
> + * ufshcd_query_attr - API function for sending attribute requests
> + * @hba: per-adapter instance
> + * @opcode: attribute opcode
> + * @idn: attribute idn to access
> + * @index: index field
> + * @selector: 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(struct ufs_hba *hba, enum query_opcode opcode,
> +		      enum attr_idn idn, u8 index, u8 selector, u32 *attr_val)
> +{
> +	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_init_query(hba, &request, &response, opcode, idn, index, selector);
> +
> +	switch (opcode) {
> +	case UPIU_QUERY_OPCODE_WRITE_ATTR:
> +		request->query_func = UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST;
> +		request->upiu_req.value = cpu_to_be32(*attr_val);
> +		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;
> +	}
> +
> +	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, QUERY_REQ_TIMEOUT);
> +
> +	if (err) {
> +		dev_err(hba->dev,
> +			"%s: opcode 0x%.2x for idn %d failed, index %d, err = %d\n",
> +			__func__, opcode, idn, index, err);
> +		goto out;
> +	}
> +
> +	*attr_val = be32_to_cpu(response->upiu_res.value);
> +
> +out:
> +	return err;
> +}
> +
> +/**
> + * ufshcd_query_attr_retry() - API function for sending query
> + * attribute with retries
> + * @hba: per-adapter instance
> + * @opcode: attribute opcode
> + * @idn: attribute idn to access
> + * @index: index field
> + * @selector: 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_retry(struct ufs_hba *hba, enum query_opcode opcode,
> +			    enum attr_idn idn, u8 index, u8 selector,
> +			    u32 *attr_val)
> +{
> +	int ret = 0;
> +	u32 retries;
> +
> +	for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
> +		ret = ufshcd_query_attr(hba, opcode, idn, index, selector, attr_val);
> +		if (ret)
> +			dev_dbg(hba->dev,
> +				"%s: failed with error %d, retries %d\n",
> +				__func__, ret, retries);
> +		else
> +			break;
> +	}
> +
> +	if (ret)
> +		dev_err(hba->dev,
> +			"%s: query attribute, idn %d, failed with error %d after %d retries\n",
> +			__func__, idn, ret, QUERY_REQ_RETRIES);
> +	return ret;
> +}
> +
>   static int __ufshcd_query_descriptor(struct ufs_hba *hba,
>   				     enum query_opcode opcode,
>   				     enum desc_idn idn, u8 index, u8 selector,

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>