[PATCH v2 0/8] Support Multi-frequency scale for UFS

[PATCH v2 0/8] Support Multi-frequency scale for UFS

[Ziqi Chen]

With OPP V2 enabled, devfreq can scale clocks amongst multiple frequency
plans. However, the gear speed is only toggled between min and max during
clock scaling. Enable multi-level gear scaling by mapping clock frequencies
to gear speeds, so that when devfreq scales clock frequencies we can put
the UFS link at the appropraite gear speeds accordingly.

This series has been tested on below platforms -
sm8550 mtp + UFS3.1
SM8650 MTP + UFS3.1
SM8750 MTP + UFS4.0

v1 -> v2:
1. Withdraw old patch 8/8 "ARM: dts: msm: Use Operation Points V2 for UFS on SM8650"
2. Add new patch 8/8 "ABI: sysfs-driver-ufs: Add missing UFS sysfs addributes"
3. Modify commit message for  "scsi: ufs: core: Pass target_freq to clk_scale_notify() vops" and "scsi: ufs: qcom: Pass target_freq to clk scale pre and post change"
4. In "scsi: ufs: qcom: Pass target_freq to clk scale pre and post change", use common Macro HZ_PER_MHZ in function ufs_qcom_set_core_clk_ctrl()
5. In "scsi: ufs: qcom: Implement the freq_to_gear_speed() vops", print out freq and gear info as debugging message
6. In "scsi: ufs: core: Enable multi-level gear scaling", rename the lable "do_pmc" to "config_pwr_mode"
7. In "scsi: ufs: core: Toggle Write Booster during clock", initialize the local variables "wb_en" as "false"

Can Guo (6):
  scsi: ufs: core: Pass target_freq to clk_scale_notify() vops
  scsi: ufs: qcom: Pass target_freq to clk scale pre and post change
  scsi: ufs: core: Add a vops to map clock frequency to gear speed
  scsi: ufs: qcom: Implement the freq_to_gear_speed() vops
  scsi: ufs: core: Enable multi-level gear scaling
  scsi: ufs: core: Toggle Write Booster during clock scaling base on
    gear speed

Ziqi Chen (2):
  scsi: ufs: core: Check if scaling up is required when disable clkscale
  ABI: sysfs-driver-ufs: Add missing UFS sysfs addributes

 Documentation/ABI/testing/sysfs-driver-ufs | 31 ++++++++++
 drivers/ufs/core/ufshcd-priv.h             | 17 +++++-
 drivers/ufs/core/ufshcd.c                  | 71 ++++++++++++++++------
 drivers/ufs/host/ufs-mediatek.c            |  1 +
 drivers/ufs/host/ufs-qcom.c                | 66 +++++++++++++++-----
 include/ufs/ufshcd.h                       |  8 ++-
 6 files changed, 159 insertions(+), 35 deletions(-)

-- 
2.34.1

[PATCH v2 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops

[Ziqi Chen]

From: Can Guo <quic_cang@quicinc.com>

Instead of only two frequencies, If OPP V2 is used, the UFS devfreq clock
scaling may scale the clock among multiple frequencies, so just passing
up/down to vops clk_scale_notify() is not enough to cover the intermediate
clock freqs between the min and max freqs. Hence pass the target_freq ,
which will be used in successive commits, to clk_scale_notify() to allow
the vops to perform corresponding configurations with regard to the clock
freqs.

Signed-off-by: Can Guo <quic_cang@quicinc.com>
Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
---

v1 -> v2:
Modify commit message to make it more clear.
---
 drivers/ufs/core/ufshcd-priv.h  | 7 ++++---
 drivers/ufs/core/ufshcd.c       | 4 ++--
 drivers/ufs/host/ufs-mediatek.c | 1 +
 drivers/ufs/host/ufs-qcom.c     | 5 +++--
 include/ufs/ufshcd.h            | 2 +-
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 9ffd94ddf8c7..0549b65f71ed 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -117,11 +117,12 @@ static inline u32 ufshcd_vops_get_ufs_hci_version(struct ufs_hba *hba)
 	return ufshcd_readl(hba, REG_UFS_VERSION);
 }
 
-static inline int ufshcd_vops_clk_scale_notify(struct ufs_hba *hba,
-			bool up, enum ufs_notify_change_status status)
+static inline int ufshcd_vops_clk_scale_notify(struct ufs_hba *hba, bool up,
+					       unsigned long target_freq,
+					       enum ufs_notify_change_status status)
 {
 	if (hba->vops && hba->vops->clk_scale_notify)
-		return hba->vops->clk_scale_notify(hba, up, status);
+		return hba->vops->clk_scale_notify(hba, up, target_freq, status);
 	return 0;
 }
 
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index acc3607bbd9c..8d295cc827cc 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1157,7 +1157,7 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, unsigned long freq,
 	int ret = 0;
 	ktime_t start = ktime_get();
 
-	ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE);
+	ret = ufshcd_vops_clk_scale_notify(hba, scale_up, freq, PRE_CHANGE);
 	if (ret)
 		goto out;
 
@@ -1168,7 +1168,7 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, unsigned long freq,
 	if (ret)
 		goto out;
 
-	ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE);
+	ret = ufshcd_vops_clk_scale_notify(hba, scale_up, freq, POST_CHANGE);
 	if (ret) {
 		if (hba->use_pm_opp)
 			ufshcd_opp_set_rate(hba,
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 135cd78109e2..977dd0caaef6 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -1643,6 +1643,7 @@ static void ufs_mtk_clk_scale(struct ufs_hba *hba, bool scale_up)
 }
 
 static int ufs_mtk_clk_scale_notify(struct ufs_hba *hba, bool scale_up,
+				    unsigned long target_freq,
 				    enum ufs_notify_change_status status)
 {
 	if (!ufshcd_is_clkscaling_supported(hba))
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 68040b2ab5f8..b6eef975dc46 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1333,8 +1333,9 @@ static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba)
 	return ufs_qcom_set_core_clk_ctrl(hba, false);
 }
 
-static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
-		bool scale_up, enum ufs_notify_change_status status)
+static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, bool scale_up,
+				     unsigned long target_freq,
+				     enum ufs_notify_change_status status)
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
 	int err;
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index d7aca9e61684..a4dac897a169 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -344,7 +344,7 @@ struct ufs_hba_variant_ops {
 	void    (*exit)(struct ufs_hba *);
 	u32	(*get_ufs_hci_version)(struct ufs_hba *);
 	int	(*set_dma_mask)(struct ufs_hba *);
-	int	(*clk_scale_notify)(struct ufs_hba *, bool,
+	int (*clk_scale_notify)(struct ufs_hba *, bool, unsigned long,
 				    enum ufs_notify_change_status);
 	int	(*setup_clocks)(struct ufs_hba *, bool,
 				enum ufs_notify_change_status);
-- 
2.34.1

[PATCH v2 2/8] scsi: ufs: qcom: Pass target_freq to clk scale pre and post change

[Ziqi Chen]

From: Can Guo <quic_cang@quicinc.com>

Instead of only two frequencies, If OPP V2 is used, the UFS devfreq clock
scaling may scale the clock among multiple frequencies. In the case of
scaling up, the devfreq may decide to scale the clock to an intermidiate
freq base on load, but the clock scale up pre change operation uses
settings for the max clock freq unconditionally. Fix it by passing the
target_freq to clock scale up pre change so that the correct settings for
the target_freq can be used.

In the case of scaling down, the clock scale down post change operation
is doing fine, because it reads the actual clock rate to tell freq, but to
keep symmetry with clock scale up pre change operation, just use the
target_freq instead of reading clock rate.

Signed-off-by: Can Guo <quic_cang@quicinc.com>
Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
---

v1 -> v2:
1. Modify commit message to make it more clear.
2. Use common Macro HZ_PER_MHZ in function ufs_qcom_set_core_clk_ctrl().
---
 drivers/ufs/host/ufs-qcom.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index b6eef975dc46..a1eb3cab45e4 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -15,6 +15,7 @@
 #include <linux/platform_device.h>
 #include <linux/reset-controller.h>
 #include <linux/time.h>
+#include <linux/units.h>
 
 #include <soc/qcom/ice.h>
 
@@ -97,7 +98,7 @@ static const struct __ufs_qcom_bw_table {
 };
 
 static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host);
-static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, bool is_scale_up);
+static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, unsigned long freq);
 
 static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd)
 {
@@ -524,7 +525,7 @@ static int ufs_qcom_link_startup_notify(struct ufs_hba *hba,
 			return -EINVAL;
 		}
 
-		err = ufs_qcom_set_core_clk_ctrl(hba, true);
+		err = ufs_qcom_set_core_clk_ctrl(hba, ULONG_MAX);
 		if (err)
 			dev_err(hba->dev, "cfg core clk ctrl failed\n");
 		/*
@@ -1231,7 +1232,7 @@ static int ufs_qcom_set_clk_40ns_cycles(struct ufs_hba *hba,
 	return ufshcd_dme_set(hba, UIC_ARG_MIB(PA_VS_CORE_CLK_40NS_CYCLES), reg);
 }
 
-static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, bool is_scale_up)
+static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, unsigned long freq)
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
 	struct list_head *head = &hba->clk_list_head;
@@ -1245,10 +1246,11 @@ static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, bool is_scale_up)
 		    !strcmp(clki->name, "core_clk_unipro")) {
 			if (!clki->max_freq)
 				cycles_in_1us = 150; /* default for backwards compatibility */
-			else if (is_scale_up)
-				cycles_in_1us = ceil(clki->max_freq, (1000 * 1000));
+			else if (freq == ULONG_MAX)
+				cycles_in_1us = ceil(clki->max_freq, HZ_PER_MHZ);
 			else
-				cycles_in_1us = ceil(clk_get_rate(clki->clk), (1000 * 1000));
+				cycles_in_1us = ceil(freq, HZ_PER_MHZ);
+
 			break;
 		}
 	}
@@ -1285,7 +1287,7 @@ static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, bool is_scale_up)
 	return ufs_qcom_set_clk_40ns_cycles(hba, cycles_in_1us);
 }
 
-static int ufs_qcom_clk_scale_up_pre_change(struct ufs_hba *hba)
+static int ufs_qcom_clk_scale_up_pre_change(struct ufs_hba *hba, unsigned long freq)
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
 	struct ufs_pa_layer_attr *attr = &host->dev_req_params;
@@ -1298,7 +1300,7 @@ static int ufs_qcom_clk_scale_up_pre_change(struct ufs_hba *hba)
 		return ret;
 	}
 	/* set unipro core clock attributes and clear clock divider */
-	return ufs_qcom_set_core_clk_ctrl(hba, true);
+	return ufs_qcom_set_core_clk_ctrl(hba, freq);
 }
 
 static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba)
@@ -1327,10 +1329,10 @@ static int ufs_qcom_clk_scale_down_pre_change(struct ufs_hba *hba)
 	return err;
 }
 
-static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba)
+static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba, unsigned long freq)
 {
 	/* set unipro core clock attributes and clear clock divider */
-	return ufs_qcom_set_core_clk_ctrl(hba, false);
+	return ufs_qcom_set_core_clk_ctrl(hba, freq);
 }
 
 static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, bool scale_up,
@@ -1349,7 +1351,7 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, bool scale_up,
 		if (err)
 			return err;
 		if (scale_up)
-			err = ufs_qcom_clk_scale_up_pre_change(hba);
+			err = ufs_qcom_clk_scale_up_pre_change(hba, target_freq);
 		else
 			err = ufs_qcom_clk_scale_down_pre_change(hba);
 
@@ -1361,7 +1363,7 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, bool scale_up,
 		if (scale_up)
 			err = ufs_qcom_clk_scale_up_post_change(hba);
 		else
-			err = ufs_qcom_clk_scale_down_post_change(hba);
+			err = ufs_qcom_clk_scale_down_post_change(hba, target_freq);
 
 
 		if (err) {
-- 
2.34.1

[PATCH v2 3/8] scsi: ufs: core: Add a vops to map clock frequency to gear speed

[Ziqi Chen]

From: Can Guo <quic_cang@quicinc.com>

Add a vops to map UFS host controller clock frequencies to the maximum
supported UFS high speed gear speeds. During clock scaling, we can map the
target clock frequency, demanded by devfreq, to the maximum supported gear
speed, so that devfreq can scale the gear to the highest gear speed
supported at the target clock frequency, instead of just scaling up/down
the gear between the min and max gear speeds.

Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
Signed-off-by: Can Guo <quic_cang@quicinc.com>
---
 drivers/ufs/core/ufshcd-priv.h | 10 ++++++++++
 include/ufs/ufshcd.h           |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 0549b65f71ed..a8f05fc6e830 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -277,6 +277,16 @@ static inline int ufshcd_mcq_vops_config_esi(struct ufs_hba *hba)
 	return -EOPNOTSUPP;
 }
 
+static inline int ufshcd_vops_freq_to_gear_speed(struct ufs_hba *hba,
+						 unsigned long freq,
+						 u32 *gear)
+{
+	if (hba->vops && hba->vops->freq_to_gear_speed)
+		return hba->vops->freq_to_gear_speed(hba, freq, gear);
+
+	return -EOPNOTSUPP;
+}
+
 extern const struct ufs_pm_lvl_states ufs_pm_lvl_states[];
 
 /**
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index a4dac897a169..8c7c497d63d3 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -336,6 +336,7 @@ struct ufs_pwr_mode_info {
  * @get_outstanding_cqs: called to get outstanding completion queues
  * @config_esi: called to config Event Specific Interrupt
  * @config_scsi_dev: called to configure SCSI device parameters
+ * @freq_to_gear_speed: called to map clock frequency to the max supported gear speed
  */
 struct ufs_hba_variant_ops {
 	const char *name;
@@ -387,6 +388,8 @@ struct ufs_hba_variant_ops {
 				       unsigned long *ocqs);
 	int	(*config_esi)(struct ufs_hba *hba);
 	void	(*config_scsi_dev)(struct scsi_device *sdev);
+	int (*freq_to_gear_speed)(struct ufs_hba *hba, unsigned long freq,
+					u32 *gear);
 };
 
 /* clock gating state  */
-- 
2.34.1

[PATCH v2 4/8] scsi: ufs: qcom: Implement the freq_to_gear_speed() vops

[Ziqi Chen]

From: Can Guo <quic_cang@quicinc.com>

Implement the freq_to_gear_speed() vops to map the unipro core clock
frequency to the corresponding maximum supported gear speed.

Signed-off-by: Can Guo <quic_cang@quicinc.com>
Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
---

v1 -> v2:
Print out freq and gear info as debugging message.
---
 drivers/ufs/host/ufs-qcom.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index a1eb3cab45e4..77cc1b8019a9 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1804,6 +1804,40 @@ static int ufs_qcom_config_esi(struct ufs_hba *hba)
 	return ret;
 }
 
+static int ufs_qcom_freq_to_gear_speed(struct ufs_hba *hba, unsigned long freq, u32 *gear)
+{
+	int ret = 0;
+
+	switch (freq) {
+	case 403000000:
+		*gear = UFS_HS_G5;
+		break;
+	case 300000000:
+		*gear = UFS_HS_G4;
+		break;
+	case 201500000:
+		*gear = UFS_HS_G3;
+		break;
+	case 150000000:
+	case 100000000:
+		*gear = UFS_HS_G2;
+		break;
+	case 75000000:
+	case 37500000:
+		*gear = UFS_HS_G1;
+		break;
+	default:
+		ret = -EINVAL;
+		dev_err(hba->dev, "%s: Unsupported clock freq : %lu\n", __func__, freq);
+		break;
+	}
+
+	if (!ret)
+		dev_dbg(hba->dev, "%s: Freq %lu to Gear %u\n", __func__, freq, *gear);
+
+	return ret;
+}
+
 /*
  * struct ufs_hba_qcom_vops - UFS QCOM specific variant operations
  *
@@ -1834,6 +1868,7 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
 	.op_runtime_config	= ufs_qcom_op_runtime_config,
 	.get_outstanding_cqs	= ufs_qcom_get_outstanding_cqs,
 	.config_esi		= ufs_qcom_config_esi,
+	.freq_to_gear_speed	= ufs_qcom_freq_to_gear_speed,
 };
 
 /**
-- 
2.34.1

[PATCH v2 5/8] scsi: ufs: core: Enable multi-level gear scaling

[Ziqi Chen]

From: Can Guo <quic_cang@quicinc.com>

With OPP V2 enabled, devfreq can scale clocks amongst multiple frequency
plans. However, the gear speed is only toggled between min and max during
clock scaling. Enable multi-level gear scaling by mapping clock frequencies
to gear speeds, so that when devfreq scales clock frequencies we can put
the UFS link at the appropraite gear speeds accordingly.

Signed-off-by: Can Guo <quic_cang@quicinc.com>
Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
---

v1 -> v2:
Rename the lable "do_pmc" to "config_pwr_mode".
---
 drivers/ufs/core/ufshcd.c | 46 ++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 8d295cc827cc..e0fc198328a5 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1308,16 +1308,28 @@ static int ufshcd_wait_for_doorbell_clr(struct ufs_hba *hba,
 /**
  * ufshcd_scale_gear - scale up/down UFS gear
  * @hba: per adapter instance
+ * @target_gear: target gear to scale to
  * @scale_up: True for scaling up gear and false for scaling down
  *
  * Return: 0 for success; -EBUSY if scaling can't happen at this time;
  * non-zero for any other errors.
  */
-static int ufshcd_scale_gear(struct ufs_hba *hba, bool scale_up)
+static int ufshcd_scale_gear(struct ufs_hba *hba, u32 target_gear, bool scale_up)
 {
 	int ret = 0;
 	struct ufs_pa_layer_attr new_pwr_info;
 
+	if (target_gear) {
+		memcpy(&new_pwr_info, &hba->pwr_info,
+		       sizeof(struct ufs_pa_layer_attr));
+
+		new_pwr_info.gear_tx = target_gear;
+		new_pwr_info.gear_rx = target_gear;
+
+		goto config_pwr_mode;
+	}
+
+	/* Legacy gear scaling, in case vops_freq_to_gear_speed() is not implemented */
 	if (scale_up) {
 		memcpy(&new_pwr_info, &hba->clk_scaling.saved_pwr_info,
 		       sizeof(struct ufs_pa_layer_attr));
@@ -1338,6 +1350,7 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, bool scale_up)
 		}
 	}
 
+config_pwr_mode:
 	/* check if the power mode needs to be changed or not? */
 	ret = ufshcd_config_pwr_mode(hba, &new_pwr_info);
 	if (ret)
@@ -1408,15 +1421,19 @@ static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, int err, bool sc
 static int ufshcd_devfreq_scale(struct ufs_hba *hba, unsigned long freq,
 				bool scale_up)
 {
+	u32 old_gear = hba->pwr_info.gear_rx;
+	u32 new_gear = 0;
 	int ret = 0;
 
+	ufshcd_vops_freq_to_gear_speed(hba, freq, &new_gear);
+
 	ret = ufshcd_clock_scaling_prepare(hba, 1 * USEC_PER_SEC);
 	if (ret)
 		return ret;
 
 	/* scale down the gear before scaling down clocks */
 	if (!scale_up) {
-		ret = ufshcd_scale_gear(hba, false);
+		ret = ufshcd_scale_gear(hba, new_gear, false);
 		if (ret)
 			goto out_unprepare;
 	}
@@ -1424,13 +1441,13 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, unsigned long freq,
 	ret = ufshcd_scale_clks(hba, freq, scale_up);
 	if (ret) {
 		if (!scale_up)
-			ufshcd_scale_gear(hba, true);
+			ufshcd_scale_gear(hba, old_gear, true);
 		goto out_unprepare;
 	}
 
 	/* scale up the gear after scaling up clocks */
 	if (scale_up) {
-		ret = ufshcd_scale_gear(hba, true);
+		ret = ufshcd_scale_gear(hba, new_gear, true);
 		if (ret) {
 			ufshcd_scale_clks(hba, hba->devfreq->previous_freq,
 					  false);
@@ -1723,6 +1740,8 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct ufs_hba *hba = dev_get_drvdata(dev);
+	struct ufs_clk_info *clki;
+	unsigned long freq;
 	u32 value;
 	int err = 0;
 
@@ -1746,14 +1765,21 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
 
 	if (value) {
 		ufshcd_resume_clkscaling(hba);
-	} else {
-		ufshcd_suspend_clkscaling(hba);
-		err = ufshcd_devfreq_scale(hba, ULONG_MAX, true);
-		if (err)
-			dev_err(hba->dev, "%s: failed to scale clocks up %d\n",
-					__func__, err);
+		goto out_rel;
 	}
 
+	clki = list_first_entry(&hba->clk_list_head, struct ufs_clk_info, list);
+	freq = clki->max_freq;
+
+	ufshcd_suspend_clkscaling(hba);
+	err = ufshcd_devfreq_scale(hba, freq, true);
+	if (err)
+		dev_err(hba->dev, "%s: failed to scale clocks up %d\n",
+				__func__, err);
+	else
+		hba->clk_scaling.target_freq = freq;
+
+out_rel:
 	ufshcd_release(hba);
 	ufshcd_rpm_put_sync(hba);
 out:
-- 
2.34.1

[PATCH v2 6/8] scsi: ufs: core: Check if scaling up is required when disable clkscale

[Ziqi Chen]

When disabling clkscale via the clkscale_enable sysfs entry, UFS driver
shall perform scaling up once regardless. Check if scaling up is required
or not first to avoid repetitive work.

Co-developed-by: Can Guo <quic_cang@quicinc.com>
Signed-off-by: Can Guo <quic_cang@quicinc.com>
Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
---
 drivers/ufs/core/ufshcd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index e0fc198328a5..741ddf3a0cb5 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1772,6 +1772,10 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
 	freq = clki->max_freq;
 
 	ufshcd_suspend_clkscaling(hba);
+
+	if (!ufshcd_is_devfreq_scaling_required(hba, freq, true))
+		goto out_rel;
+
 	err = ufshcd_devfreq_scale(hba, freq, true);
 	if (err)
 		dev_err(hba->dev, "%s: failed to scale clocks up %d\n",
-- 
2.34.1

[PATCH v2 7/8] scsi: ufs: core: Toggle Write Booster during clock scaling base on gear speed

[Ziqi Chen]

From: Can Guo <quic_cang@quicinc.com>

During clock scaling, Write Booster is toggled on or off based on
whether the clock is scaled up or down. However, with OPP V2 powered
multi-level gear scaling, the gear can be scaled amongst multiple gear
speeds, e.g., it may scale down from G5 to G4, or from G4 to G2. To provide
flexibilities, add a new field for clock scaling such that during clock
scaling Write Booster can be enabled or disabled based on gear speeds but
not based on scaling up or down.

Signed-off-by: Can Guo <quic_cang@quicinc.com>
Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
---

v1 - > v2:
Initialize the local variables "wb_en" as "false".
---
 drivers/ufs/core/ufshcd.c | 17 ++++++++++++-----
 include/ufs/ufshcd.h      |  3 +++
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 741ddf3a0cb5..ae825da00791 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1395,13 +1395,17 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us)
 	return ret;
 }
 
-static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, int err, bool scale_up)
+static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, int err)
 {
 	up_write(&hba->clk_scaling_lock);
 
-	/* Enable Write Booster if we have scaled up else disable it */
-	if (ufshcd_enable_wb_if_scaling_up(hba) && !err)
-		ufshcd_wb_toggle(hba, scale_up);
+	/* Enable Write Booster if current gear requires it else disable it */
+	if (ufshcd_enable_wb_if_scaling_up(hba) && !err) {
+		bool wb_en = false;
+
+		wb_en = hba->pwr_info.gear_rx >= hba->clk_scaling.wb_gear ? true : false;
+		ufshcd_wb_toggle(hba, wb_en);
+	}
 
 	mutex_unlock(&hba->wb_mutex);
 
@@ -1456,7 +1460,7 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, unsigned long freq,
 	}
 
 out_unprepare:
-	ufshcd_clock_scaling_unprepare(hba, ret, scale_up);
+	ufshcd_clock_scaling_unprepare(hba, ret);
 	return ret;
 }
 
@@ -1816,6 +1820,9 @@ static void ufshcd_init_clk_scaling(struct ufs_hba *hba)
 	if (!hba->clk_scaling.min_gear)
 		hba->clk_scaling.min_gear = UFS_HS_G1;
 
+	if (!hba->clk_scaling.wb_gear)
+		hba->clk_scaling.wb_gear = UFS_HS_G3;
+
 	INIT_WORK(&hba->clk_scaling.suspend_work,
 		  ufshcd_clk_scaling_suspend_work);
 	INIT_WORK(&hba->clk_scaling.resume_work,
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 8c7c497d63d3..8e6c2eb68011 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -448,6 +448,8 @@ struct ufs_clk_gating {
  * @resume_work: worker to resume devfreq
  * @target_freq: frequency requested by devfreq framework
  * @min_gear: lowest HS gear to scale down to
+ * @wb_gear: enable Write Booster when HS gear scales above or equal to it, else
+ *		disable Write Booster
  * @is_enabled: tracks if scaling is currently enabled or not, controlled by
  *		clkscale_enable sysfs node
  * @is_allowed: tracks if scaling is currently allowed or not, used to block
@@ -468,6 +470,7 @@ struct ufs_clk_scaling {
 	struct work_struct resume_work;
 	unsigned long target_freq;
 	u32 min_gear;
+	u32 wb_gear;
 	bool is_enabled;
 	bool is_allowed;
 	bool is_initialized;
-- 
2.34.1

[PATCH v2 8/8] ABI: sysfs-driver-ufs: Add missing UFS sysfs addributes

[Ziqi Chen]

Add UFS driver sysfs addributes clkscale_enable, clkgate_enable and
clkgate_delay_ms to this doucment.

Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
---
v1 -> v2:
It is a new patch be added to this series since v2.
---
 Documentation/ABI/testing/sysfs-driver-ufs | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
index 5fa6655aee84..7f60821c29ca 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -1559,3 +1559,34 @@ Description:
 		Symbol - HCMID. This file shows the UFSHCD manufacturer id.
 		The Manufacturer ID is defined by JEDEC in JEDEC-JEP106.
 		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/clkscale_enable
+What:		/sys/bus/platform/devices/*.ufs/clkscale_enable
+Date:		January 2025
+Contact:	Ziqi Chen <quic_ziqichen@quicinc.com>
+Description:
+		This file shows the status of UFS clock scaling enablement
+		and it can be used to enable/disable clock scaling.
+
+		The file is read write.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/clkgate_enable
+What:		/sys/bus/platform/devices/*.ufs/clkgate_enable
+Date:		January 2025
+Contact:	Ziqi Chen <quic_ziqichen@quicinc.com>
+Description:
+		This file shows the status of UFS clock gating enablement
+		and it can be used to enable/disable clock gating.
+
+		The file is read write.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/clkgate_delay_ms
+What:		/sys/bus/platform/devices/*.ufs/clkgate_delay_ms
+Date:		January 2025
+Contact:	Ziqi Chen <quic_ziqichen@quicinc.com>
+Description:
+		This file shows and sets the number of milliseconds of idle
+		time before the UFS driver start to do clock gating. This can
+		prevent the UFS from frequently performing clock gate/ungate.
+
+		The file is read write.
-- 
2.34.1

Re: [PATCH v2 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops

[Bean Huo]

On Wed, 2025-01-22 at 18:02 +0800, Ziqi Chen wrote:
> From: Can Guo <quic_cang@quicinc.com>
> 
> Instead of only two frequencies, If OPP V2 is used, the UFS devfreq
> clock
> scaling may scale the clock among multiple frequencies, so just
> passing
> up/down to vops clk_scale_notify() is not enough to cover the
> intermediate
> clock freqs between the min and max freqs. Hence pass the target_freq
> ,
> which will be used in successive commits, to clk_scale_notify() to
> allow
> the vops to perform corresponding configurations with regard to the
> clock
> freqs.
> 
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>

Reviewed-by: Bean Huo <beanhuo@micron.com>

Re: [PATCH v2 2/8] scsi: ufs: qcom: Pass target_freq to clk scale pre and post change

[Bean Huo]

On Wed, 2025-01-22 at 18:02 +0800, Ziqi Chen wrote:
> From: Can Guo <quic_cang@quicinc.com>
> 
> Instead of only two frequencies, If OPP V2 is used, the UFS devfreq
> clock
> scaling may scale the clock among multiple frequencies. In the case
> of
> scaling up, the devfreq may decide to scale the clock to an
> intermidiate
> freq base on load, but the clock scale up pre change operation uses
> settings for the max clock freq unconditionally. Fix it by passing
> the
> target_freq to clock scale up pre change so that the correct settings
> for
> the target_freq can be used.
> 
> In the case of scaling down, the clock scale down post change
> operation
> is doing fine, because it reads the actual clock rate to tell freq,
> but to
> keep symmetry with clock scale up pre change operation, just use the
> target_freq instead of reading clock rate.
> 
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>

Reviewed-by: Bean Huo <beanhuo@micron.com>

Re: [PATCH v2 3/8] scsi: ufs: core: Add a vops to map clock frequency to gear speed

[Bean Huo]

On Wed, 2025-01-22 at 18:02 +0800, Ziqi Chen wrote:
> From: Can Guo <quic_cang@quicinc.com>
> 
> Add a vops to map UFS host controller clock frequencies to the
> maximum
> supported UFS high speed gear speeds. During clock scaling, we can
> map the
> target clock frequency, demanded by devfreq, to the maximum supported
> gear
> speed, so that devfreq can scale the gear to the highest gear speed
> supported at the target clock frequency, instead of just scaling
> up/down
> the gear between the min and max gear speeds.
> 
> Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
> Signed-off-by: Can Guo <quic_cang@quicinc.com>

Reviewed-by: Bean Huo <beanhuo@micron.com>

Re: [PATCH v2 4/8] scsi: ufs: qcom: Implement the freq_to_gear_speed() vops

[Bean Huo]

On Wed, 2025-01-22 at 18:02 +0800, Ziqi Chen wrote:
> From: Can Guo <quic_cang@quicinc.com>
> 
> Implement the freq_to_gear_speed() vops to map the unipro core clock
> frequency to the corresponding maximum supported gear speed.
> 
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>


Reviewed-by: Bean Huo <beanhuo@micron.com>

Re: [PATCH v2 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops

[Bart Van Assche]

On 1/22/25 2:02 AM, Ziqi Chen wrote:
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index d7aca9e61684..a4dac897a169 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -344,7 +344,7 @@ struct ufs_hba_variant_ops {
>   	void    (*exit)(struct ufs_hba *);
>   	u32	(*get_ufs_hci_version)(struct ufs_hba *);
>   	int	(*set_dma_mask)(struct ufs_hba *);
> -	int	(*clk_scale_notify)(struct ufs_hba *, bool,
> +	int (*clk_scale_notify)(struct ufs_hba *, bool, unsigned long,
>   				    enum ufs_notify_change_status);
>   	int	(*setup_clocks)(struct ufs_hba *, bool,
>   				enum ufs_notify_change_status);

Please keep the indentation consistent.

Thanks,

Bart.

Re: [PATCH v2 2/8] scsi: ufs: qcom: Pass target_freq to clk scale pre and post change

[Bart Van Assche]

On 1/22/25 2:02 AM, Ziqi Chen wrote:
> scaling up, the devfreq may decide to scale the clock to an intermidiate

intermidiate -> intermediate

Thanks,

Bart.

Re: [PATCH v2 4/8] scsi: ufs: qcom: Implement the freq_to_gear_speed() vops

[Eric Biggers]

On Wed, Jan 22, 2025 at 06:02:10PM +0800, Ziqi Chen wrote:
> From: Can Guo <quic_cang@quicinc.com>
> 
> Implement the freq_to_gear_speed() vops to map the unipro core clock
> frequency to the corresponding maximum supported gear speed.
> 
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>

vop, not vops.  It's one operation.

- Eric

Re: [PATCH v2 3/8] scsi: ufs: core: Add a vops to map clock frequency to gear speed

[Bart Van Assche]

On 1/22/25 2:02 AM, Ziqi Chen wrote:
> From: Can Guo <quic_cang@quicinc.com>
> 
> Add a vops to map UFS host controller clock frequencies to the maximum

Above and in the subject, please change "vops" into "vop" (variant
operation).

>   struct ufs_hba_variant_ops {
>   	const char *name;
> @@ -387,6 +388,8 @@ struct ufs_hba_variant_ops {
>   				       unsigned long *ocqs);
>   	int	(*config_esi)(struct ufs_hba *hba);
>   	void	(*config_scsi_dev)(struct scsi_device *sdev);
> +	int (*freq_to_gear_speed)(struct ufs_hba *hba, unsigned long freq,
> +					u32 *gear);
>   };

Please keep the indentation consistent.

Thanks,

Bart.

Re: [PATCH v2 4/8] scsi: ufs: qcom: Implement the freq_to_gear_speed() vops

[Bart Van Assche]

On 1/22/25 2:02 AM, Ziqi Chen wrote:
> +static int ufs_qcom_freq_to_gear_speed(struct ufs_hba *hba, unsigned long freq, u32 *gear)
> +{
> +	int ret = 0;
> +
> +	switch (freq) {
> +	case 403000000:
> +		*gear = UFS_HS_G5;
> +		break;
> +	case 300000000:
> +		*gear = UFS_HS_G4;
> +		break;
> +	case 201500000:
> +		*gear = UFS_HS_G3;
> +		break;
> +	case 150000000:
> +	case 100000000:
> +		*gear = UFS_HS_G2;
> +		break;
> +	case 75000000:
> +	case 37500000:
> +		*gear = UFS_HS_G1;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		dev_err(hba->dev, "%s: Unsupported clock freq : %lu\n", __func__, freq);
> +		break;
> +	}
> +
> +	if (!ret)
> +		dev_dbg(hba->dev, "%s: Freq %lu to Gear %u\n", __func__, freq, *gear);
> +
> +	return ret;
> +}

Please simplify the above function by returning early in case of an
unsupported clock frequency and by removing the 'ret' variable.

Thanks,

Bart.

Re: [PATCH v2 3/8] scsi: ufs: core: Add a vops to map clock frequency to gear speed

[Bart Van Assche]

On 1/22/25 2:02 AM, Ziqi Chen wrote:
> +static inline int ufshcd_vops_freq_to_gear_speed(struct ufs_hba *hba,
> +						 unsigned long freq,
> +						 u32 *gear)
> +{
> +	if (hba->vops && hba->vops->freq_to_gear_speed)
> +		return hba->vops->freq_to_gear_speed(hba, freq, gear);
> +
> +	return -EOPNOTSUPP;
> +}

Please remove "vops_" from the function name. I don't think this part of 
the function name is useful. Additionally, please return the gear value 
as the function result and remove the "u32 *gear" argument.

Thanks,

Bart.

Re: [PATCH v2 5/8] scsi: ufs: core: Enable multi-level gear scaling

[Bart Van Assche]

On 1/22/25 2:02 AM, Ziqi Chen wrote:
> +	if (target_gear) {
> +		memcpy(&new_pwr_info, &hba->pwr_info,
> +		       sizeof(struct ufs_pa_layer_attr));

Why memcpy() instead of an assignment? The advantage of an assignment is 
that the compiler can perform type checking.

Thanks,

Bart.

Re: [PATCH v2 8/8] ABI: sysfs-driver-ufs: Add missing UFS sysfs addributes

[Bart Van Assche]

On 1/22/25 2:02 AM, Ziqi Chen wrote:
> +What:		/sys/bus/platform/drivers/ufshcd/*/clkscale_enable
> +What:		/sys/bus/platform/devices/*.ufs/clkscale_enable
> +Date:		January 2025
> +Contact:	Ziqi Chen <quic_ziqichen@quicinc.com>
> +Description:
> +		This file shows the status of UFS clock scaling enablement
> +		and it can be used to enable/disable clock scaling.
> +
> +		The file is read write.

Please improve the grammar of this and other descriptions, e.g. by 
changing the above description into the following: "Whether or not
clock scaling is enabled. This attribute is read/write."

Thanks,

Bart.

Re: [PATCH v2 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops

[Ziqi Chen]

On 1/23/2025 2:19 AM, Bart Van Assche wrote:
> On 1/22/25 2:02 AM, Ziqi Chen wrote:
>> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
>> index d7aca9e61684..a4dac897a169 100644
>> --- a/include/ufs/ufshcd.h
>> +++ b/include/ufs/ufshcd.h
>> @@ -344,7 +344,7 @@ struct ufs_hba_variant_ops {
>>       void    (*exit)(struct ufs_hba *);
>>       u32    (*get_ufs_hci_version)(struct ufs_hba *);
>>       int    (*set_dma_mask)(struct ufs_hba *);
>> -    int    (*clk_scale_notify)(struct ufs_hba *, bool,
>> +    int (*clk_scale_notify)(struct ufs_hba *, bool, unsigned long,
>>                       enum ufs_notify_change_status);
>>       int    (*setup_clocks)(struct ufs_hba *, bool,
>>                   enum ufs_notify_change_status);
> 
> Please keep the indentation consistent.
> 
> Thanks,
> 
> Bart.

Thanks, Bart, I will improve it.

-Ziqi

Re: [PATCH v2 2/8] scsi: ufs: qcom: Pass target_freq to clk scale pre and post change

[Ziqi Chen]

On 1/23/2025 2:20 AM, Bart Van Assche wrote:
> On 1/22/25 2:02 AM, Ziqi Chen wrote:
>> scaling up, the devfreq may decide to scale the clock to an intermidiate
> 
> intermidiate -> intermediate
> 
> Thanks,
> 
> Bart.

Sharp insight! I will fix this typo in next version.

-Ziqi

Re: [PATCH v2 4/8] scsi: ufs: qcom: Implement the freq_to_gear_speed() vops

[Ziqi Chen]

On 1/23/2025 2:21 AM, Eric Biggers wrote:
> On Wed, Jan 22, 2025 at 06:02:10PM +0800, Ziqi Chen wrote:
>> From: Can Guo <quic_cang@quicinc.com>
>>
>> Implement the freq_to_gear_speed() vops to map the unipro core clock
>> frequency to the corresponding maximum supported gear speed.
>>
>> Signed-off-by: Can Guo <quic_cang@quicinc.com>
>> Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
>> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
> 
> vop, not vops.  It's one operation.
> 
> - Eric

Sure, Eric , I will modify it. Thanks for your review.

- Ziqi

Re: [PATCH v2 3/8] scsi: ufs: core: Add a vops to map clock frequency to gear speed

[Ziqi Chen]

Hi Bart,

Thanks for you review~

On 1/23/2025 2:22 AM, Bart Van Assche wrote:
> On 1/22/25 2:02 AM, Ziqi Chen wrote:
>> From: Can Guo <quic_cang@quicinc.com>
>>
>> Add a vops to map UFS host controller clock frequencies to the maximum
> 
> Above and in the subject, please change "vops" into "vop" (variant
> operation).
> 

OK , will update it in next version.

>>   struct ufs_hba_variant_ops {
>>       const char *name;
>> @@ -387,6 +388,8 @@ struct ufs_hba_variant_ops {
>>                          unsigned long *ocqs);
>>       int    (*config_esi)(struct ufs_hba *hba);
>>       void    (*config_scsi_dev)(struct scsi_device *sdev);
>> +    int (*freq_to_gear_speed)(struct ufs_hba *hba, unsigned long freq,
>> +                    u32 *gear);
>>   };
> 
> Please keep the indentation consistent.

Sure, thanks~

> 
> Thanks,
> 
> Bart.
> 

Re: [PATCH v2 4/8] scsi: ufs: qcom: Implement the freq_to_gear_speed() vops

[Ziqi Chen]

On 1/23/2025 2:23 AM, Bart Van Assche wrote:
> On 1/22/25 2:02 AM, Ziqi Chen wrote:
>> +static int ufs_qcom_freq_to_gear_speed(struct ufs_hba *hba, unsigned 
>> long freq, u32 *gear)
>> +{
>> +    int ret = 0;
>> +
>> +    switch (freq) {
>> +    case 403000000:
>> +        *gear = UFS_HS_G5;
>> +        break;
>> +    case 300000000:
>> +        *gear = UFS_HS_G4;
>> +        break;
>> +    case 201500000:
>> +        *gear = UFS_HS_G3;
>> +        break;
>> +    case 150000000:
>> +    case 100000000:
>> +        *gear = UFS_HS_G2;
>> +        break;
>> +    case 75000000:
>> +    case 37500000:
>> +        *gear = UFS_HS_G1;
>> +        break;
>> +    default:
>> +        ret = -EINVAL;
>> +        dev_err(hba->dev, "%s: Unsupported clock freq : %lu\n", 
>> __func__, freq);
>> +        break;
>> +    }
>> +
>> +    if (!ret)
>> +        dev_dbg(hba->dev, "%s: Freq %lu to Gear %u\n", __func__, 
>> freq, *gear);
>> +
>> +    return ret;
>> +}
> 
> Please simplify the above function by returning early in case of an
> unsupported clock frequency and by removing the 'ret' variable.
> 
> Thanks,
> 
> Bart.
> 
Hi Bart,

looks like a good way , thanks~

-Ziqi

Re: [PATCH v2 3/8] scsi: ufs: core: Add a vops to map clock frequency to gear speed

[Ziqi Chen]

On 1/23/2025 2:30 AM, Bart Van Assche wrote:
> On 1/22/25 2:02 AM, Ziqi Chen wrote:
>> +static inline int ufshcd_vops_freq_to_gear_speed(struct ufs_hba *hba,
>> +                         unsigned long freq,
>> +                         u32 *gear)
>> +{
>> +    if (hba->vops && hba->vops->freq_to_gear_speed)
>> +        return hba->vops->freq_to_gear_speed(hba, freq, gear);
>> +
>> +    return -EOPNOTSUPP;
>> +}
> 
> Please remove "vops_" from the function name. I don't think this part of 
> the function name is useful. Additionally, please return the gear value 
> as the function result and remove the "u32 *gear" argument.
> 
> Thanks,
> 
> Bart.

Hi Bart,

Thanks for your review~

In ufshcd-priv.h , the function name of all vop wrapping APIs have the 
same prefix "ufshcd_vops", I need to use the same format as them.


As for return the gear value as the function result. In our original 
design, we also return gear result for this function, but finally we 
want to use return value to indicate the status , e.g,, if vendor 
doesn't implement this vop, we return -EOPNOTSUPP , if there is no 
matched gear to the freq , we return -EINVAL. Although we didn't check 
the return value in this series, we still want to preserve this 
extensibility in case this function be used to other where in the future.

-Ziqi

Re: [PATCH v2 5/8] scsi: ufs: core: Enable multi-level gear scaling

[Ziqi Chen]

On 1/23/2025 2:32 AM, Bart Van Assche wrote:
> On 1/22/25 2:02 AM, Ziqi Chen wrote:
>> +    if (target_gear) {
>> +        memcpy(&new_pwr_info, &hba->pwr_info,
>> +               sizeof(struct ufs_pa_layer_attr));
> 
> Why memcpy() instead of an assignment? The advantage of an assignment is 
> that the compiler can perform type checking.
> 
> Thanks,
> 
> Bart.
> 

Hi Bart,

We use memcpy() here is due to memcpy() can be faster than direct 
assignment. We don't worry about safety because they are same struct 
"ufs_pa_layer_attr" so that we can ensure the accuracy of number of 
bytes and member type.

-Ziqi

Re: [PATCH v2 8/8] ABI: sysfs-driver-ufs: Add missing UFS sysfs addributes

[Ziqi Chen]

On 1/23/2025 2:35 AM, Bart Van Assche wrote:
> On 1/22/25 2:02 AM, Ziqi Chen wrote:
>> +What:        /sys/bus/platform/drivers/ufshcd/*/clkscale_enable
>> +What:        /sys/bus/platform/devices/*.ufs/clkscale_enable
>> +Date:        January 2025
>> +Contact:    Ziqi Chen <quic_ziqichen@quicinc.com>
>> +Description:
>> +        This file shows the status of UFS clock scaling enablement
>> +        and it can be used to enable/disable clock scaling.
>> +
>> +        The file is read write.
> 
> Please improve the grammar of this and other descriptions, e.g. by 
> changing the above description into the following: "Whether or not
> clock scaling is enabled. This attribute is read/write."
> 
> Thanks,
> 
> Bart.

Hi Bart,

Thanks , I will improve it.

-Ziqi

Re: [PATCH v2 8/8] ABI: sysfs-driver-ufs: Add missing UFS sysfs addributes

[Bryan O'Donoghue]

On 22/01/2025 10:02, Ziqi Chen wrote:
> Add UFS driver sysfs addributes clkscale_enable, clkgate_enable and
> clkgate_delay_ms to this doucment.

I'm 99% sure you mean "attributes" not "addributes"

---
bod

Re: [PATCH v2 3/8] scsi: ufs: core: Add a vops to map clock frequency to gear speed

[Bart Van Assche]

On 1/22/25 11:40 PM, Ziqi Chen wrote:
> In ufshcd-priv.h , the function name of all vop wrapping APIs have the 
> same prefix "ufshcd_vops", I need to use the same format as them.

That sounds fair to me.

> As for return the gear value as the function result. In our original 
> design, we also return gear result for this function, but finally we 
> want to use return value to indicate the status , e.g,, if vendor 
> doesn't implement this vop, we return -EOPNOTSUPP , if there is no 
> matched gear to the freq , we return -EINVAL. Although we didn't check 
> the return value in this series, we still want to preserve this 
> extensibility in case this function be used to other where in the future.

There are many functions in the Linux kernel that either return a
negative error code or a positive value in case of success. Regarding
future extensibility, we can't know how this function will evolve in the
future. This is not an argument to keep the approach of separate error
codes (return value) and gear values (gear argument).

Thanks,

Bart.

Re: [PATCH v2 5/8] scsi: ufs: core: Enable multi-level gear scaling

[Bart Van Assche]

On 1/22/25 11:41 PM, Ziqi Chen wrote:
> We use memcpy() here is due to memcpy() can be faster than direct 
> assignment. We don't worry about safety because they are same struct 
> "ufs_pa_layer_attr" so that we can ensure the accuracy of number of 
> bytes and member type.

The memcpy() call we are discussing is not in the hot path so it doesn't
have to be hyper-optimized. Making the compiler perform type checking is
more important in this code path than micro-optimizing the code.

Additionally, please do not try to be smarter than the compiler. 
Compilers are able to convert struct assignments into a memcpy() call if
there are good reasons to assume that the memcpy() call will be faster.

Given the small size of struct ufs_pa_layer_attr (7 * 4 = 28 bytes),
memberwise assignment probably is faster than a memcpy() call. The trunk
version of gcc (ARM64) translates a memberwise assignment of struct 
ufs_pa_layer_attr into the following four assembler instructions (x0 and
x1 point to struct ufs_pa_layer_attr instances, q30 and q31 are 128 bit
registers):

         ldr     q30, [x1]
         ldr     q31, [x1, 12]
         str     q30, [x0]
         str     q31, [x0, 12]

Thanks,

Bart.

Re: [PATCH v2 0/8] Support Multi-frequency scale for UFS

[neil.armstrong]

Hi,

On 22/01/2025 11:02, Ziqi Chen wrote:
> With OPP V2 enabled, devfreq can scale clocks amongst multiple frequency
> plans. However, the gear speed is only toggled between min and max during
> clock scaling. Enable multi-level gear scaling by mapping clock frequencies
> to gear speeds, so that when devfreq scales clock frequencies we can put
> the UFS link at the appropraite gear speeds accordingly.
> 
> This series has been tested on below platforms -
> sm8550 mtp + UFS3.1
> SM8650 MTP + UFS3.1
> SM8750 MTP + UFS4.0

Thanks, could you be more precise on how you tested this feature ? how did you exercise the gear changes and check that is cales correctly ?

Thanks,
Neil

> 
> v1 -> v2:
> 1. Withdraw old patch 8/8 "ARM: dts: msm: Use Operation Points V2 for UFS on SM8650"
> 2. Add new patch 8/8 "ABI: sysfs-driver-ufs: Add missing UFS sysfs addributes"
> 3. Modify commit message for  "scsi: ufs: core: Pass target_freq to clk_scale_notify() vops" and "scsi: ufs: qcom: Pass target_freq to clk scale pre and post change"
> 4. In "scsi: ufs: qcom: Pass target_freq to clk scale pre and post change", use common Macro HZ_PER_MHZ in function ufs_qcom_set_core_clk_ctrl()
> 5. In "scsi: ufs: qcom: Implement the freq_to_gear_speed() vops", print out freq and gear info as debugging message
> 6. In "scsi: ufs: core: Enable multi-level gear scaling", rename the lable "do_pmc" to "config_pwr_mode"
> 7. In "scsi: ufs: core: Toggle Write Booster during clock", initialize the local variables "wb_en" as "false"
> 
> Can Guo (6):
>    scsi: ufs: core: Pass target_freq to clk_scale_notify() vops
>    scsi: ufs: qcom: Pass target_freq to clk scale pre and post change
>    scsi: ufs: core: Add a vops to map clock frequency to gear speed
>    scsi: ufs: qcom: Implement the freq_to_gear_speed() vops
>    scsi: ufs: core: Enable multi-level gear scaling
>    scsi: ufs: core: Toggle Write Booster during clock scaling base on
>      gear speed
> 
> Ziqi Chen (2):
>    scsi: ufs: core: Check if scaling up is required when disable clkscale
>    ABI: sysfs-driver-ufs: Add missing UFS sysfs addributes
> 
>   Documentation/ABI/testing/sysfs-driver-ufs | 31 ++++++++++
>   drivers/ufs/core/ufshcd-priv.h             | 17 +++++-
>   drivers/ufs/core/ufshcd.c                  | 71 ++++++++++++++++------
>   drivers/ufs/host/ufs-mediatek.c            |  1 +
>   drivers/ufs/host/ufs-qcom.c                | 66 +++++++++++++++-----
>   include/ufs/ufshcd.h                       |  8 ++-
>   6 files changed, 159 insertions(+), 35 deletions(-)
> 

Re: [PATCH v2 3/8] scsi: ufs: core: Add a vops to map clock frequency to gear speed

[Ziqi Chen]

On 1/24/2025 1:49 AM, Bart Van Assche wrote:
> On 1/22/25 11:40 PM, Ziqi Chen wrote:
>> In ufshcd-priv.h , the function name of all vop wrapping APIs have the 
>> same prefix "ufshcd_vops", I need to use the same format as them.
> 
> That sounds fair to me.
> 
>> As for return the gear value as the function result. In our original 
>> design, we also return gear result for this function, but finally we 
>> want to use return value to indicate the status , e.g,, if vendor 
>> doesn't implement this vop, we return -EOPNOTSUPP , if there is no 
>> matched gear to the freq , we return -EINVAL. Although we didn't check 
>> the return value in this series, we still want to preserve this 
>> extensibility in case this function be used to other where in the future.
> 
> There are many functions in the Linux kernel that either return a
> negative error code or a positive value in case of success. Regarding
> future extensibility, we can't know how this function will evolve in the
> future. This is not an argument to keep the approach of separate error
> codes (return value) and gear values (gear argument).
> 
> Thanks,
> 
> Bart.
> 
OK , Bart, let me improve it and please review again in my next version.

-Ziqi

Re: [PATCH v2 5/8] scsi: ufs: core: Enable multi-level gear scaling

[Ziqi Chen]

On 1/24/2025 2:02 AM, Bart Van Assche wrote:
> On 1/22/25 11:41 PM, Ziqi Chen wrote:
>> We use memcpy() here is due to memcpy() can be faster than direct 
>> assignment. We don't worry about safety because they are same struct 
>> "ufs_pa_layer_attr" so that we can ensure the accuracy of number of 
>> bytes and member type.
> 
> The memcpy() call we are discussing is not in the hot path so it doesn't
> have to be hyper-optimized. Making the compiler perform type checking is
> more important in this code path than micro-optimizing the code.
> 
> Additionally, please do not try to be smarter than the compiler. 
> Compilers are able to convert struct assignments into a memcpy() call if
> there are good reasons to assume that the memcpy() call will be faster.
> 
> Given the small size of struct ufs_pa_layer_attr (7 * 4 = 28 bytes),
> memberwise assignment probably is faster than a memcpy() call. The trunk
> version of gcc (ARM64) translates a memberwise assignment of struct 
> ufs_pa_layer_attr into the following four assembler instructions (x0 and
> x1 point to struct ufs_pa_layer_attr instances, q30 and q31 are 128 bit
> registers):
> 
>          ldr     q30, [x1]
>          ldr     q31, [x1, 12]
>          str     q30, [x0]
>          str     q31, [x0, 12]
> 
> Thanks,
> 
> Bart.
> 
Sure , Let me try and test it. If works fine , I will update in next 
version.

-Ziqi

Re: [PATCH v2 8/8] ABI: sysfs-driver-ufs: Add missing UFS sysfs addributes

[Ziqi Chen]

On 1/23/2025 7:36 PM, Bryan O'Donoghue wrote:
> On 22/01/2025 10:02, Ziqi Chen wrote:
>> Add UFS driver sysfs addributes clkscale_enable, clkgate_enable and
>> clkgate_delay_ms to this doucment.
> 
> I'm 99% sure you mean "attributes" not "addributes"
> 
> ---
> bod

Yes , thanks for point it out, bod.

-Ziqi

Re: [PATCH v2 0/8] Support Multi-frequency scale for UFS

[Ziqi Chen]

On 1/24/2025 4:12 AM, neil.armstrong@linaro.org wrote:
> Hi,
> 
> On 22/01/2025 11:02, Ziqi Chen wrote:
>> With OPP V2 enabled, devfreq can scale clocks amongst multiple frequency
>> plans. However, the gear speed is only toggled between min and max during
>> clock scaling. Enable multi-level gear scaling by mapping clock 
>> frequencies
>> to gear speeds, so that when devfreq scales clock frequencies we can put
>> the UFS link at the appropraite gear speeds accordingly.
>>
>> This series has been tested on below platforms -
>> sm8550 mtp + UFS3.1
>> SM8650 MTP + UFS3.1
>> SM8750 MTP + UFS4.0
> 
> Thanks, could you be more precise on how you tested this feature ? how 
> did you exercise the gear changes and check that is cales correctly ?
> 
> Thanks,
> Neil
> 

Hi Neil,

I made a Debugging patch that track the freq and gear in UFS devfreq and 
clkscale path. I do data transfer from userspace and print out devfreq 
target freq , clkscale recommended freq , final clkscale freq and gear 
value of before and after clkscale.

For example, we can get such logs on SM8550 as below:

[    5.424720] [DEBUG]ufshcd_devfreq_target: target freq = 75000000
[    5.509541] [DEBUG]ufshcd_devfreq_target: recommended freq = 75000000
[    5.525463] [DEBUG]ufshcd_devfreq_target: final freq = 75000000
[    5.525464] [DEBUG]ufshcd_devfreq_target: scaling DOWN from freq 
300000000 to freq 75000000
[    5.525468] [DEBUG]ufshcd_devfreq_scale: freq 75000000 mapped to gear 
1 , caller: ufshcd_devfreq_target+0x298/0x3e4
[    5.525531] [DEBUG]ufshcd_devfreq_scale: scaling DOWN gear from 4 -> 1
[    5.728018] [DEBUG]ufshcd_devfreq_target: updated target freq to 75000000


.. G1 -> G4...

[  261.068843] [DEBUG]ufshcd_devfreq_target: target freq = 300000000
[  261.075242] [DEBUG]ufshcd_devfreq_target: recommended freq = 300000000
[  261.082055] [DEBUG]ufshcd_devfreq_target: final freq = 300000000
[  261.088280] [DEBUG]ufshcd_devfreq_target: scaling UP from freq 
75000000 to freq 300000000
[  261.096743] ufshcd-qcom 1d84000.ufshc: 
ufshcd_is_devfreq_scaling_required: req_freq= 300000000, target_freq = 
75000000
[  261.107814] [DEBUG]ufshcd_devfreq_scale: freq 300000000 mapped to 
gear 4 , caller: ufshcd_devfreq_target+0x298/0x3e4
[  261.126922] [DEBUG]ufshcd_devfreq_scale: scaling UP gear from 1 -> 4
[  261.134810] [DEBUG]ufshcd_devfreq_target: updated target freq to 
300000000
[  261.196460] [DEBUG]ufshcd_devfreq_target: target freq = 244755397
[  261.210039] [DEBUG]ufshcd_devfreq_target: recommended freq = 300000000
[  261.216900] [DEBUG]ufshcd_devfreq_target: final freq = 300000000
[  261.223135] [DEBUG]ufshcd_devfreq_target: scaling DOWN from freq 
300000000 to freq 300000000
[  261.231872] ufshcd-qcom 1d84000.ufshc: 
ufshcd_is_devfreq_scaling_required: req_freq= 300000000, target_freq = 
300000000
[  261.243017] [DEBUG]ufshcd_devfreq_target: scaling DOWN from freq 
300000000 to freq 300000000 not required


.. G4 -> G2...

[  455.604414] [DEBUG]ufshcd_devfreq_target: target freq = 149736604
[  455.617895] [DEBUG]ufshcd_devfreq_target: recommended freq = 150000000
[  455.624702] [DEBUG]ufshcd_devfreq_target: final freq = 150000000
[  455.630936] [DEBUG]ufshcd_devfreq_target: scaling DOWN from freq 
300000000 to freq 150000000
[  455.639656] ufshcd-qcom 1d84000.ufshc: 
ufshcd_is_devfreq_scaling_required: req_freq= 150000000, target_freq = 
300000000
[  455.650838] [DEBUG]ufshcd_devfreq_scale: freq 150000000 mapped to 
gear 2 , caller: ufshcd_devfreq_target+0x298/0x3e4
[  455.661809] [DEBUG]ufshcd_devfreq_scale: scaling DOWN gear from 4 -> 2
[  455.670529] [DEBUG]ufshcd_devfreq_target: updated target freq to 
150000000


...G2 -> G1

[  548.484492] [DEBUG]ufshcd_devfreq_target: target freq = 75000000
[  548.497957] [DEBUG]ufshcd_devfreq_target: recommended freq = 75000000
[  548.504801] [DEBUG]ufshcd_devfreq_target: final freq = 75000000
[  548.510949] [DEBUG]ufshcd_devfreq_target: scaling DOWN from freq 
150000000 to freq 75000000
[  548.519590] ufshcd-qcom 1d84000.ufshc: 
ufshcd_is_devfreq_scaling_required: req_freq= 75000000, target_freq = 
150000000
[  548.530725] [DEBUG]ufshcd_devfreq_scale: freq 75000000 mapped to gear 
1 , caller: ufshcd_devfreq_target+0x298/0x3e4
[  548.552850] [DEBUG]ufshcd_devfreq_scale: scaling DOWN gear from 2 -> 1


-Ziqi

>>
>> v1 -> v2:
>> 1. Withdraw old patch 8/8 "ARM: dts: msm: Use Operation Points V2 for 
>> UFS on SM8650"
>> 2. Add new patch 8/8 "ABI: sysfs-driver-ufs: Add missing UFS sysfs 
>> addributes"
>> 3. Modify commit message for  "scsi: ufs: core: Pass target_freq to 
>> clk_scale_notify() vops" and "scsi: ufs: qcom: Pass target_freq to clk 
>> scale pre and post change"
>> 4. In "scsi: ufs: qcom: Pass target_freq to clk scale pre and post 
>> change", use common Macro HZ_PER_MHZ in function 
>> ufs_qcom_set_core_clk_ctrl()
>> 5. In "scsi: ufs: qcom: Implement the freq_to_gear_speed() vops", 
>> print out freq and gear info as debugging message
>> 6. In "scsi: ufs: core: Enable multi-level gear scaling", rename the 
>> lable "do_pmc" to "config_pwr_mode"
>> 7. In "scsi: ufs: core: Toggle Write Booster during clock", initialize 
>> the local variables "wb_en" as "false"
>>
>> Can Guo (6):
>>    scsi: ufs: core: Pass target_freq to clk_scale_notify() vops
>>    scsi: ufs: qcom: Pass target_freq to clk scale pre and post change
>>    scsi: ufs: core: Add a vops to map clock frequency to gear speed
>>    scsi: ufs: qcom: Implement the freq_to_gear_speed() vops
>>    scsi: ufs: core: Enable multi-level gear scaling
>>    scsi: ufs: core: Toggle Write Booster during clock scaling base on
>>      gear speed
>>
>> Ziqi Chen (2):
>>    scsi: ufs: core: Check if scaling up is required when disable clkscale
>>    ABI: sysfs-driver-ufs: Add missing UFS sysfs addributes
>>
>>   Documentation/ABI/testing/sysfs-driver-ufs | 31 ++++++++++
>>   drivers/ufs/core/ufshcd-priv.h             | 17 +++++-
>>   drivers/ufs/core/ufshcd.c                  | 71 ++++++++++++++++------
>>   drivers/ufs/host/ufs-mediatek.c            |  1 +
>>   drivers/ufs/host/ufs-qcom.c                | 66 +++++++++++++++-----
>>   include/ufs/ufshcd.h                       |  8 ++-
>>   6 files changed, 159 insertions(+), 35 deletions(-)
>>
>