Re: [PATCH v4 5/8] scsi: ufs: core: Enable multi-level gear scaling
From: Ziqi Chen
Date: Tue Feb 11 2025 - 05:29:11 EST
On 2/11/2025 5:28 PM, Peter Wang (王信友) wrote:
On Mon, 2025-02-10 at 18:02 +0800, Ziqi Chen wrote:
External email : Please do not click links or open attachments until
you have verified the sender or the content.
From: Can Guo <quic_cang@xxxxxxxxxxx>
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 appropriate gear speeds accordingly.
Signed-off-by: Can Guo <quic_cang@xxxxxxxxxxx>
Co-developed-by: Ziqi Chen <quic_ziqichen@xxxxxxxxxxx>
Signed-off-by: Ziqi Chen <quic_ziqichen@xxxxxxxxxxx>
Reviewed-by: Bean Huo <beanhuo@xxxxxxxxxx>
Reviewed-by: Bart Van Assche <bvanassche@xxxxxxx>
Tested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
---
v1 -> v2:
Rename the lable "do_pmc" to "config_pwr_mode".
v2 -> v3:
Use assignment instead memcpy() in function ufshcd_scale_gear().
v3 -> v4:
Typo fixed for commit message.
---
drivers/ufs/core/ufshcd.c | 51 +++++++++++++++++++++++++++++++------
--
1 file changed, 41 insertions(+), 10 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 8d295cc827cc..ebab897080a6 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1308,16 +1308,26 @@ 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) {
+ new_pwr_info = hba->pwr_info;
+ 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 +1348,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 +1419,26 @@ 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;
+ int new_gear = 0;
int ret = 0;
+ new_gear = ufshcd_vops_freq_to_gear_speed(hba, freq);
+ if (new_gear < 0)
+ /*
+ * return negative value means that the
vops_freq_to_gear_speed() is not
+ * implemented or didn't find matched gear speed,
assign '0' to new_gear
+ * to switch to legacy gear scaling sequence in
ufshcd_scale_gear().
+ */
+ new_gear = 0;
+
Hi Ziqi,
I think remove help function is better.
No need change new_gear type when use.
The readability is higher, and no need add that large amount comments.
u32_new_gear = 0;
if (hba->vops && hba->vops->freq_to_gear_speed)
new_gear = hba->vops->freq_to_gear_speed(hba, freq);
Thanks.
Peter
Hi Peter,
Thanks, Peter.
Frankly, I also think this way has low readability. However, keep the
u32 type for new_gear is OK to me. But this vop would lose the ability
to indicate the error types. All types of error can only return "0".
However, we don't need to deal with various types of errors up to now, I
can submit a new version to change back the new_gear and vop return
value type to u32 and make correspondingly change in patch 3/8 and 4/8.
-Ziqi
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, (u32)new_gear, false);
if (ret)
goto out_unprepare;
}
@@ -1424,13 +1446,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, (u32)new_gear, true);
if (ret) {
ufshcd_scale_clks(hba, hba->devfreq-
previous_freq,
false);
@@ -1723,6 +1745,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 +1770,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