* [PATCH 0/8] Support Multi-frequency scale for UFS
@ 2025-01-16 9:11 Ziqi Chen
2025-01-16 9:11 ` [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops Ziqi Chen
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Ziqi Chen @ 2025-01-16 9:11 UTC (permalink / raw)
To: quic_cang, bvanassche, mani, beanhuo, avri.altman, junwoo80.lee,
martin.petersen, quic_ziqichen, quic_nguyenb, quic_nitirawa,
quic_rampraka
Cc: linux-scsi, Matthias Brugger, AngeloGioacchino Del Regno,
open list:ARM/Mediatek SoC support:Keyword:mediatek,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
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 -
SM8650 + UFS3.1
SM8750 + UFS4.0
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
ARM: dts: msm: Use Operation Points V2 for UFS on SM8650
arch/arm64/boot/dts/qcom/sm8650.dtsi | 51 ++++++++++++++++----
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 | 60 ++++++++++++++++++-----
include/ufs/ufshcd.h | 8 +++-
6 files changed, 166 insertions(+), 42 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops
2025-01-16 9:11 [PATCH 0/8] Support Multi-frequency scale for UFS Ziqi Chen
@ 2025-01-16 9:11 ` Ziqi Chen
2025-01-19 7:11 ` Manivannan Sadhasivam
2025-01-16 9:28 ` [PATCH 0/8] Support Multi-frequency scale for UFS Neil Armstrong
2025-01-19 7:57 ` Manivannan Sadhasivam
2 siblings, 1 reply; 11+ messages in thread
From: Ziqi Chen @ 2025-01-16 9:11 UTC (permalink / raw)
To: quic_cang, bvanassche, mani, beanhuo, avri.altman, junwoo80.lee,
martin.petersen, quic_ziqichen, quic_nguyenb, quic_nitirawa,
quic_rampraka
Cc: linux-scsi, Alim Akhtar, James E.J. Bottomley, Peter Wang,
Stanley Jhu, Manivannan Sadhasivam, Matthias Brugger,
AngeloGioacchino Del Regno, Andrew Halaney, Maramaina Naresh,
Eric Biggers, Minwoo Im, open list,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
open list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
From: Can Guo <quic_cang@quicinc.com>
If OPP V2 is used, devfreq clock scaling may scale clock amongst more than
two freqs, 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 to clk_scale_notify() to allow the vops to perform
corresponding configurations with regard to the clock freqs.
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 | 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/8] Support Multi-frequency scale for UFS
2025-01-16 9:11 [PATCH 0/8] Support Multi-frequency scale for UFS Ziqi Chen
2025-01-16 9:11 ` [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops Ziqi Chen
@ 2025-01-16 9:28 ` Neil Armstrong
2025-01-20 11:56 ` Ziqi Chen
2025-01-19 7:57 ` Manivannan Sadhasivam
2 siblings, 1 reply; 11+ messages in thread
From: Neil Armstrong @ 2025-01-16 9:28 UTC (permalink / raw)
To: Ziqi Chen, quic_cang, bvanassche, mani, beanhuo, avri.altman,
junwoo80.lee, martin.petersen, quic_nguyenb, quic_nitirawa,
quic_rampraka
Cc: linux-scsi, Matthias Brugger, AngeloGioacchino Del Regno,
open list:ARM/Mediatek SoC support:Keyword:mediatek,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
Hi,
[+linux-arm-msm@vger.kernel.org]
On 16/01/2025 10:11, 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 -
> SM8650 + UFS3.1
Which board did you use ? the MTP ?
> SM8750 + UFS4.0
Did you alse test it on SM8550 ? this platform is also concerned.
And perhaps SM8450 should be also converted to the OPP table & tested.
Please Cc linux-arm-msm on all patches since we're directly concerned by
the whole changeset.
Thanks,
Neil
>
>
> 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
> ARM: dts: msm: Use Operation Points V2 for UFS on SM8650
>
> arch/arm64/boot/dts/qcom/sm8650.dtsi | 51 ++++++++++++++++----
> 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 | 60 ++++++++++++++++++-----
> include/ufs/ufshcd.h | 8 +++-
> 6 files changed, 166 insertions(+), 42 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops
2025-01-16 9:11 ` [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops Ziqi Chen
@ 2025-01-19 7:11 ` Manivannan Sadhasivam
2025-01-20 12:01 ` Ziqi Chen
0 siblings, 1 reply; 11+ messages in thread
From: Manivannan Sadhasivam @ 2025-01-19 7:11 UTC (permalink / raw)
To: Ziqi Chen
Cc: quic_cang, bvanassche, beanhuo, avri.altman, junwoo80.lee,
martin.petersen, quic_nguyenb, quic_nitirawa, quic_rampraka,
linux-scsi, Alim Akhtar, James E.J. Bottomley, Peter Wang,
Stanley Jhu, Manivannan Sadhasivam, Matthias Brugger,
AngeloGioacchino Del Regno, Andrew Halaney, Maramaina Naresh,
Eric Biggers, Minwoo Im, open list,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
open list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
On Thu, Jan 16, 2025 at 05:11:42PM +0800, Ziqi Chen wrote:
> From: Can Guo <quic_cang@quicinc.com>
>
> If OPP V2 is used, devfreq clock scaling may scale clock amongst more than
> two freqs,
'amongst more than two freqs': I couldn't parse this.
> 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 to clk_scale_notify() to allow the vops to perform
> corresponding configurations with regard to the clock freqs.
>
Add a note that the 'target_freq' is not used in this commit.
> 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>
Signed-off-by tag order is not correct here. This implies that Ziqi originally
worked on it, then Can took over and submitted. But it seems the reverse.
- Mani
> ---
> 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
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/8] Support Multi-frequency scale for UFS
2025-01-16 9:11 [PATCH 0/8] Support Multi-frequency scale for UFS Ziqi Chen
2025-01-16 9:11 ` [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops Ziqi Chen
2025-01-16 9:28 ` [PATCH 0/8] Support Multi-frequency scale for UFS Neil Armstrong
@ 2025-01-19 7:57 ` Manivannan Sadhasivam
2025-01-20 11:58 ` Ziqi Chen
2 siblings, 1 reply; 11+ messages in thread
From: Manivannan Sadhasivam @ 2025-01-19 7:57 UTC (permalink / raw)
To: Ziqi Chen
Cc: quic_cang, bvanassche, mani, beanhuo, avri.altman, junwoo80.lee,
martin.petersen, quic_nguyenb, quic_nitirawa, quic_rampraka,
linux-scsi, Matthias Brugger, AngeloGioacchino Del Regno,
open list:ARM/Mediatek SoC support:Keyword:mediatek,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
On Thu, Jan 16, 2025 at 05:11:41PM +0800, Ziqi Chen wrote:
You missed CCing linux-arm-msm mailing list to the cover letter.
> 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.
>
But the UFSHC PHY settings are not updated for each gear speed, isn't it? Then
I'm wondering how much we get out of this 'multi-level gear scaling'.
- Mani
> This series has been tested on below platforms -
> SM8650 + UFS3.1
> SM8750 + UFS4.0
>
>
> 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
> ARM: dts: msm: Use Operation Points V2 for UFS on SM8650
>
> arch/arm64/boot/dts/qcom/sm8650.dtsi | 51 ++++++++++++++++----
> 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 | 60 ++++++++++++++++++-----
> include/ufs/ufshcd.h | 8 +++-
> 6 files changed, 166 insertions(+), 42 deletions(-)
>
> --
> 2.34.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/8] Support Multi-frequency scale for UFS
2025-01-16 9:28 ` [PATCH 0/8] Support Multi-frequency scale for UFS Neil Armstrong
@ 2025-01-20 11:56 ` Ziqi Chen
0 siblings, 0 replies; 11+ messages in thread
From: Ziqi Chen @ 2025-01-20 11:56 UTC (permalink / raw)
To: neil.armstrong, quic_cang, bvanassche, mani, beanhuo, avri.altman,
junwoo80.lee, martin.petersen, quic_nguyenb, quic_nitirawa,
quic_rampraka
Cc: linux-scsi, Matthias Brugger, AngeloGioacchino Del Regno,
open list:ARM/Mediatek SoC support:Keyword:mediatek,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
Hi Neil,
Thanks for your comment.
On 1/16/2025 5:28 PM, Neil Armstrong wrote:
> Hi,
>
> [+linux-arm-msm@vger.kernel.org]
>
> On 16/01/2025 10:11, 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 -
>> SM8650 + UFS3.1
>
> Which board did you use ? the MTP ? >
We tested on MTP.
>> SM8750 + UFS4.0
>
> Did you alse test it on SM8550 ? this platform is also concerned.
> And perhaps SM8450 should be also converted to the OPP table & tested.
>
We didn't test in on SM8550 before, but now we already tested it on
SM8550 once see you this comment. It works fine on SM8550 as well.
I will update this information in next version.
> Please Cc linux-arm-msm on all patches since we're directly concerned by
> the whole changeset.
Sure , Thank you for reminder, I will CC this group from next version.
>
> Thanks,
> Neil
>
-Ziqi
>>
>>
>> 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
>> ARM: dts: msm: Use Operation Points V2 for UFS on SM8650
>>
>> arch/arm64/boot/dts/qcom/sm8650.dtsi | 51 ++++++++++++++++----
>> 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 | 60 ++++++++++++++++++-----
>> include/ufs/ufshcd.h | 8 +++-
>> 6 files changed, 166 insertions(+), 42 deletions(-)
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/8] Support Multi-frequency scale for UFS
2025-01-19 7:57 ` Manivannan Sadhasivam
@ 2025-01-20 11:58 ` Ziqi Chen
0 siblings, 0 replies; 11+ messages in thread
From: Ziqi Chen @ 2025-01-20 11:58 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: quic_cang, bvanassche, beanhuo, avri.altman, junwoo80.lee,
martin.petersen, quic_nguyenb, quic_nitirawa, quic_rampraka,
linux-scsi, Matthias Brugger, AngeloGioacchino Del Regno,
open list:ARM/Mediatek SoC support:Keyword:mediatek,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
Hi Mani,
Thanks for you review~
On 1/19/2025 3:57 PM, Manivannan Sadhasivam wrote:
> On Thu, Jan 16, 2025 at 05:11:41PM +0800, Ziqi Chen wrote:
>
> You missed CCing linux-arm-msm mailing list to the cover letter.
>
Thank you for reminder, I will cc this group in next patch version.
>> 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.
>>
>
> But the UFSHC PHY settings are not updated for each gear speed, isn't it? Then
> I'm wondering how much we get out of this 'multi-level gear scaling'.
Per design, we don't need to update any PHY setting for each gear speed
mode.
>
> - Mani
>
-Ziqi
>> This series has been tested on below platforms -
>> SM8650 + UFS3.1
>> SM8750 + UFS4.0
>>
>>
>> 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
>> ARM: dts: msm: Use Operation Points V2 for UFS on SM8650
>>
>> arch/arm64/boot/dts/qcom/sm8650.dtsi | 51 ++++++++++++++++----
>> 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 | 60 ++++++++++++++++++-----
>> include/ufs/ufshcd.h | 8 +++-
>> 6 files changed, 166 insertions(+), 42 deletions(-)
>>
>> --
>> 2.34.1
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops
2025-01-19 7:11 ` Manivannan Sadhasivam
@ 2025-01-20 12:01 ` Ziqi Chen
2025-01-20 15:36 ` Manivannan Sadhasivam
2025-01-20 15:38 ` Manivannan Sadhasivam
0 siblings, 2 replies; 11+ messages in thread
From: Ziqi Chen @ 2025-01-20 12:01 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: quic_cang, bvanassche, beanhuo, avri.altman, junwoo80.lee,
martin.petersen, quic_nguyenb, quic_nitirawa, quic_rampraka,
linux-scsi, Alim Akhtar, James E.J. Bottomley, Peter Wang,
Stanley Jhu, Manivannan Sadhasivam, Matthias Brugger,
AngeloGioacchino Del Regno, Andrew Halaney, Maramaina Naresh,
Eric Biggers, Minwoo Im, open list,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
open list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
Hi Mani,
Thanks for you review~
On 1/19/2025 3:11 PM, Manivannan Sadhasivam wrote:
> On Thu, Jan 16, 2025 at 05:11:42PM +0800, Ziqi Chen wrote:
>> From: Can Guo <quic_cang@quicinc.com>
>>
>> If OPP V2 is used, devfreq clock scaling may scale clock amongst more than
>> two freqs,
>
> 'amongst more than two freqs': I couldn't parse this.
>
It means that the devfreq framework will tell UFS core driver the
devfreq freq, then UFS core driver will find the recommended freq from
our freq table based on the devfreq freq. For legacy mode , we can only
have 2 frequencies in the table. But if the OPP V2 is used, we can have
3 , 4 or more freq tables. You can refer to my PATCH 8/8.
>> 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 to clk_scale_notify() to allow the vops to perform
>> corresponding configurations with regard to the clock freqs.
>>
>
> Add a note that the 'target_freq' is not used in this commit.
>
Sorry, I don't very understand this comment, I mentioned the
"target_freq" in the commit message, Could you let me know what note
you want me do add?
>> 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>
>
> Signed-off-by tag order is not correct here. This implies that Ziqi originally
> worked on it, then Can took over and submitted. But it seems the reverse.
Thanks for your reminder. Is below tag order OK ?
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>
>
> - Mani
-Ziqi
>
>> ---
>> 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
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops
2025-01-20 12:01 ` Ziqi Chen
@ 2025-01-20 15:36 ` Manivannan Sadhasivam
2025-01-21 3:51 ` Ziqi Chen
2025-01-20 15:38 ` Manivannan Sadhasivam
1 sibling, 1 reply; 11+ messages in thread
From: Manivannan Sadhasivam @ 2025-01-20 15:36 UTC (permalink / raw)
To: Ziqi Chen
Cc: Manivannan Sadhasivam, quic_cang, bvanassche, beanhuo,
avri.altman, junwoo80.lee, martin.petersen, quic_nguyenb,
quic_nitirawa, quic_rampraka, linux-scsi, Alim Akhtar,
James E.J. Bottomley, Peter Wang, Stanley Jhu, Matthias Brugger,
AngeloGioacchino Del Regno, Andrew Halaney, Maramaina Naresh,
Eric Biggers, Minwoo Im, open list,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
open list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
On Mon, Jan 20, 2025 at 08:01:03PM +0800, Ziqi Chen wrote:
> Hi Mani,
>
> Thanks for you review~
>
> On 1/19/2025 3:11 PM, Manivannan Sadhasivam wrote:
> > On Thu, Jan 16, 2025 at 05:11:42PM +0800, Ziqi Chen wrote:
> > > From: Can Guo <quic_cang@quicinc.com>
> > >
> > > If OPP V2 is used, devfreq clock scaling may scale clock amongst more than
> > > two freqs,
> >
> > 'amongst more than two freqs': I couldn't parse this.
> >
>
> It means that the devfreq framework will tell UFS core driver the devfreq
> freq, then UFS core driver will find the recommended freq from our freq
> table based on the devfreq freq. For legacy mode , we can only have 2
> frequencies in the table. But if the OPP V2 is used, we can have 3 , 4 or
> more freq tables. You can refer to my PATCH 8/8.
>
I got the motive, but the wording is not correct.
> > > 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 to clk_scale_notify() to allow the vops to perform
> > > corresponding configurations with regard to the clock freqs.
> > >
> >
> > Add a note that the 'target_freq' is not used in this commit.
> >
>
> Sorry, I don't very understand this comment, I mentioned the "target_freq"
> in the commit message, Could you let me know what note you want me do add?
>
This patch is introducing 'target_freq' as a parameter, but it is not used. I
was asking you to mention that it will be used in successive commits.
> > > 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>
> >
> > Signed-off-by tag order is not correct here. This implies that Ziqi originally
> > worked on it, then Can took over and submitted. But it seems the reverse.
>
> Thanks for your reminder. Is below tag order OK ?
> 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>
> >
> > - Mani
>
> -Ziqi
>
> >
> > > ---
> > > 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
> > >
> >
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops
2025-01-20 12:01 ` Ziqi Chen
2025-01-20 15:36 ` Manivannan Sadhasivam
@ 2025-01-20 15:38 ` Manivannan Sadhasivam
1 sibling, 0 replies; 11+ messages in thread
From: Manivannan Sadhasivam @ 2025-01-20 15:38 UTC (permalink / raw)
To: Ziqi Chen
Cc: quic_cang, bvanassche, beanhuo, avri.altman, junwoo80.lee,
martin.petersen, quic_nguyenb, quic_nitirawa, quic_rampraka,
linux-scsi, Alim Akhtar, James E.J. Bottomley, Peter Wang,
Stanley Jhu, Manivannan Sadhasivam, Matthias Brugger,
AngeloGioacchino Del Regno, Andrew Halaney, Maramaina Naresh,
Eric Biggers, Minwoo Im, open list,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
open list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
On Mon, Jan 20, 2025 at 08:01:03PM +0800, Ziqi Chen wrote:
> Hi Mani,
>
> Thanks for you review~
>
> On 1/19/2025 3:11 PM, Manivannan Sadhasivam wrote:
> > On Thu, Jan 16, 2025 at 05:11:42PM +0800, Ziqi Chen wrote:
> > > From: Can Guo <quic_cang@quicinc.com>
> > >
> > > If OPP V2 is used, devfreq clock scaling may scale clock amongst more than
> > > two freqs,
> >
> > 'amongst more than two freqs': I couldn't parse this.
> >
>
> It means that the devfreq framework will tell UFS core driver the devfreq
> freq, then UFS core driver will find the recommended freq from our freq
> table based on the devfreq freq. For legacy mode , we can only have 2
> frequencies in the table. But if the OPP V2 is used, we can have 3 , 4 or
> more freq tables. You can refer to my PATCH 8/8.
>
> > > 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 to clk_scale_notify() to allow the vops to perform
> > > corresponding configurations with regard to the clock freqs.
> > >
> >
> > Add a note that the 'target_freq' is not used in this commit.
> >
>
> Sorry, I don't very understand this comment, I mentioned the "target_freq"
> in the commit message, Could you let me know what note you want me do add?
>
> > > 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>
> >
> > Signed-off-by tag order is not correct here. This implies that Ziqi originally
> > worked on it, then Can took over and submitted. But it seems the reverse.
>
> Thanks for your reminder. Is below tag order OK ?
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> Co-developed-by: Ziqi Chen <quic_ziqichen@quicinc.com>
'Co-developed-by' is not needed unless you also worked on the patch. I guess you
are just sending the patch authored by Can, so you can drop this and keep your
'Signed-off-by' tag.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops
2025-01-20 15:36 ` Manivannan Sadhasivam
@ 2025-01-21 3:51 ` Ziqi Chen
0 siblings, 0 replies; 11+ messages in thread
From: Ziqi Chen @ 2025-01-21 3:51 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Manivannan Sadhasivam, quic_cang, bvanassche, beanhuo,
avri.altman, junwoo80.lee, martin.petersen, quic_nguyenb,
quic_nitirawa, quic_rampraka, linux-scsi, Alim Akhtar,
James E.J. Bottomley, Peter Wang, Stanley Jhu, Matthias Brugger,
AngeloGioacchino Del Regno, Andrew Halaney, Maramaina Naresh,
Eric Biggers, Minwoo Im, open list,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
open list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...,
moderated list:ARM/Mediatek SoC support:Keyword:mediatek
On 1/20/2025 11:36 PM, Manivannan Sadhasivam wrote:
> On Mon, Jan 20, 2025 at 08:01:03PM +0800, Ziqi Chen wrote:
>> Hi Mani,
>>
>> Thanks for you review~
>>
>> On 1/19/2025 3:11 PM, Manivannan Sadhasivam wrote:
>>> On Thu, Jan 16, 2025 at 05:11:42PM +0800, Ziqi Chen wrote:
>>>> From: Can Guo <quic_cang@quicinc.com>
>>>>
>>>> If OPP V2 is used, devfreq clock scaling may scale clock amongst more than
>>>> two freqs,
>>>
>>> 'amongst more than two freqs': I couldn't parse this.
>>>
>>
>> It means that the devfreq framework will tell UFS core driver the devfreq
>> freq, then UFS core driver will find the recommended freq from our freq
>> table based on the devfreq freq. For legacy mode , we can only have 2
>> frequencies in the table. But if the OPP V2 is used, we can have 3 , 4 or
>> more freq tables. You can refer to my PATCH 8/8.
>>
>
> I got the motive, but the wording is not correct.
>
OK , let me express it in another way:
"Instead of only two frequencies, If OPP V2 is used, the UFS devfreq
clock scaling may scale the clock among multiple frequencies."
How about this?
>>>> 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 to clk_scale_notify() to allow the vops to perform
>>>> corresponding configurations with regard to the clock freqs.
>>>>
>>>
>>> Add a note that the 'target_freq' is not used in this commit.
>>>
>>
>> Sorry, I don't very understand this comment, I mentioned the "target_freq"
>> in the commit message, Could you let me know what note you want me do add?
>>
>
> This patch is introducing 'target_freq' as a parameter, but it is not used. I
> was asking you to mention that it will be used in successive commits.
>
OK , thank you, I will add this note in next version.
-Ziqi
>>>> 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>
>>>
>>> Signed-off-by tag order is not correct here. This implies that Ziqi originally
>>> worked on it, then Can took over and submitted. But it seems the reverse.
>>
>> Thanks for your reminder. Is below tag order OK ?
>> 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>
>>>
>>> - Mani
>>
>> -Ziqi
>>
>>>
>>>> ---
>>>> 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
>>>>
>>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-01-21 5:05 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-16 9:11 [PATCH 0/8] Support Multi-frequency scale for UFS Ziqi Chen
2025-01-16 9:11 ` [PATCH 1/8] scsi: ufs: core: Pass target_freq to clk_scale_notify() vops Ziqi Chen
2025-01-19 7:11 ` Manivannan Sadhasivam
2025-01-20 12:01 ` Ziqi Chen
2025-01-20 15:36 ` Manivannan Sadhasivam
2025-01-21 3:51 ` Ziqi Chen
2025-01-20 15:38 ` Manivannan Sadhasivam
2025-01-16 9:28 ` [PATCH 0/8] Support Multi-frequency scale for UFS Neil Armstrong
2025-01-20 11:56 ` Ziqi Chen
2025-01-19 7:57 ` Manivannan Sadhasivam
2025-01-20 11:58 ` Ziqi Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).