* [PATCH v3 0/2] ufs: allow host driver disable wb toggle druing clock scaling
@ 2022-08-02 14:32 peter.wang
2022-08-02 14:32 ` [PATCH v3 1/2] ufs: core: introduce a choice of wb toggle during " peter.wang
2022-08-02 14:32 ` [PATCH v3 2/2] ufs: host: support wb toggle with " peter.wang
0 siblings, 2 replies; 7+ messages in thread
From: peter.wang @ 2022-08-02 14:32 UTC (permalink / raw)
To: stanley.chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: wsd_upstream, linux-mediatek, peter.wang, chun-hung.wu,
alice.chao, cc.chou, chaotian.jing, jiajie.hao, powen.kao,
qilin.tan, lin.gui
From: Peter Wang <peter.wang@mediatek.com>
Mediatek ufs do not want to toggle write booster during clock scaling.
This patch set allow host driver disable wb toggle during clock scaling.
Peter Wang (2):
ufs: core: introduce a choice of wb toggle during clock scaling
ufs: host: support wb toggle with clock scaling
drivers/ufs/core/ufs-sysfs.c | 3 ++-
drivers/ufs/core/ufshcd.c | 8 +++++---
drivers/ufs/host/ufs-qcom.c | 2 +-
include/ufs/ufshcd.h | 7 +++++++
4 files changed, 15 insertions(+), 5 deletions(-)
--
2.18.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/2] ufs: core: introduce a choice of wb toggle during clock scaling
2022-08-02 14:32 [PATCH v3 0/2] ufs: allow host driver disable wb toggle druing clock scaling peter.wang
@ 2022-08-02 14:32 ` peter.wang
2022-08-02 18:44 ` Bart Van Assche
2022-08-02 14:32 ` [PATCH v3 2/2] ufs: host: support wb toggle with " peter.wang
1 sibling, 1 reply; 7+ messages in thread
From: peter.wang @ 2022-08-02 14:32 UTC (permalink / raw)
To: stanley.chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: wsd_upstream, linux-mediatek, peter.wang, chun-hung.wu,
alice.chao, cc.chou, chaotian.jing, jiajie.hao, powen.kao,
qilin.tan, lin.gui
From: Peter Wang <peter.wang@mediatek.com>
Host driver may want to disable/enable write booster during clock scaling.
Introduce a flag UFSHCD_CAP_WB_WITH_CLK_SCALING to decouple WB and
clock scaling.
UFSHCD_CAP_WB_WITH_CLK_SCALING only valid when UFSHCD_CAP_CLK_SCALING
is set. Just like UFSHCD_CAP_HIBERN8_WITH_CLK_GATING is valid only when
UFSHCD_CAP_CLK_GATING set.
Signed-off-by: Peter Wang <peter.wang@mediatek.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/ufs/core/ufs-sysfs.c | 3 ++-
drivers/ufs/core/ufshcd.c | 8 +++++---
include/ufs/ufshcd.h | 7 +++++++
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 0a088b47d557..8fa1679ffd1a 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -225,7 +225,8 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr,
unsigned int wb_enable;
ssize_t res;
- if (!ufshcd_is_wb_allowed(hba) || ufshcd_is_clkscaling_supported(hba)) {
+ if (!ufshcd_is_wb_allowed(hba) || (ufshcd_is_clkscaling_supported(hba)
+ && ufshcd_can_wb_during_scaling(hba))) {
/*
* If the platform supports UFSHCD_CAP_CLK_SCALING, turn WB
* on/off will be done while clock scaling up/down.
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 3d367be71728..d86826e958e6 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1301,9 +1301,11 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, bool scale_up)
}
/* Enable Write Booster if we have scaled up else disable it */
- downgrade_write(&hba->clk_scaling_lock);
- is_writelock = false;
- ufshcd_wb_toggle(hba, scale_up);
+ if (ufshcd_can_wb_during_scaling(hba)) {
+ downgrade_write(&hba->clk_scaling_lock);
+ is_writelock = false;
+ ufshcd_wb_toggle(hba, scale_up);
+ }
out_unprepare:
ufshcd_clock_scaling_unprepare(hba, is_writelock);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index a92271421718..4bd35d68acde 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -648,6 +648,9 @@ enum ufshcd_caps {
* notification if it is supported by the UFS device.
*/
UFSHCD_CAP_TEMP_NOTIF = 1 << 11,
+
+ /* Allow WB with clk scaling */
+ UFSHCD_CAP_WB_WITH_CLK_SCALING = 1 << 12,
};
struct ufs_hba_variant_params {
@@ -1004,6 +1007,10 @@ static inline bool ufshcd_is_wb_allowed(struct ufs_hba *hba)
{
return hba->caps & UFSHCD_CAP_WB_EN;
}
+static inline bool ufshcd_can_wb_during_scaling(struct ufs_hba *hba)
+{
+ return hba->caps & UFSHCD_CAP_WB_WITH_CLK_SCALING;
+}
#define ufshcd_writel(hba, val, reg) \
writel((val), (hba)->mmio_base + (reg))
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/2] ufs: host: support wb toggle with clock scaling
2022-08-02 14:32 [PATCH v3 0/2] ufs: allow host driver disable wb toggle druing clock scaling peter.wang
2022-08-02 14:32 ` [PATCH v3 1/2] ufs: core: introduce a choice of wb toggle during " peter.wang
@ 2022-08-02 14:32 ` peter.wang
2022-08-02 18:46 ` Bart Van Assche
1 sibling, 1 reply; 7+ messages in thread
From: peter.wang @ 2022-08-02 14:32 UTC (permalink / raw)
To: stanley.chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: wsd_upstream, linux-mediatek, peter.wang, chun-hung.wu,
alice.chao, cc.chou, chaotian.jing, jiajie.hao, powen.kao,
qilin.tan, lin.gui
From: Peter Wang <peter.wang@mediatek.com>
Set UFSHCD_CAP_WB_WITH_CLK_SCALING for qcom to compatible legacy design.
Signed-off-by: Peter Wang <peter.wang@mediatek.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/ufs/host/ufs-qcom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index f10d4668814c..f8c9a78e7776 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -869,7 +869,7 @@ static void ufs_qcom_set_caps(struct ufs_hba *hba)
struct ufs_qcom_host *host = ufshcd_get_variant(hba);
hba->caps |= UFSHCD_CAP_CLK_GATING | UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
- hba->caps |= UFSHCD_CAP_CLK_SCALING;
+ hba->caps |= UFSHCD_CAP_CLK_SCALING | UFSHCD_CAP_WB_WITH_CLK_SCALING;
hba->caps |= UFSHCD_CAP_AUTO_BKOPS_SUSPEND;
hba->caps |= UFSHCD_CAP_WB_EN;
hba->caps |= UFSHCD_CAP_CRYPTO;
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] ufs: core: introduce a choice of wb toggle during clock scaling
2022-08-02 14:32 ` [PATCH v3 1/2] ufs: core: introduce a choice of wb toggle during " peter.wang
@ 2022-08-02 18:44 ` Bart Van Assche
2022-08-03 2:44 ` Peter Wang
0 siblings, 1 reply; 7+ messages in thread
From: Bart Van Assche @ 2022-08-02 18:44 UTC (permalink / raw)
To: peter.wang, stanley.chu, linux-scsi, martin.petersen,
avri.altman, alim.akhtar, jejb
Cc: wsd_upstream, linux-mediatek, chun-hung.wu, alice.chao, cc.chou,
chaotian.jing, jiajie.hao, powen.kao, qilin.tan, lin.gui
On 8/2/22 07:32, peter.wang@mediatek.com wrote:
> + /* Allow WB with clk scaling */
This comment is misleading. Consider changing this comment into
something like "Enable WB when scaling up the clock and disable WB when
scaling the clock down".
> + UFSHCD_CAP_WB_WITH_CLK_SCALING = 1 << 12,
Whether or not the WriteBooster is toggled when the clock is scaled is
not a host controller capability. It is a policy that is tied by this
patch to the host controller. So I think that using the "UFSHCD_CAP_"
prefix is misleading. Consider using e.g. the prefix UFSHCD_POLICY_.
> +static inline bool ufshcd_can_wb_during_scaling(struct ufs_hba *hba)
> +{
> + return hba->caps & UFSHCD_CAP_WB_WITH_CLK_SCALING;
> +}
The name of this function is misleading. Consider renaming this function
into e.g. ufshcd_enable_wb_if_scaling_up().
Thanks,
Bart.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] ufs: host: support wb toggle with clock scaling
2022-08-02 14:32 ` [PATCH v3 2/2] ufs: host: support wb toggle with " peter.wang
@ 2022-08-02 18:46 ` Bart Van Assche
2022-08-03 2:42 ` Peter Wang
0 siblings, 1 reply; 7+ messages in thread
From: Bart Van Assche @ 2022-08-02 18:46 UTC (permalink / raw)
To: peter.wang, stanley.chu, linux-scsi, martin.petersen,
avri.altman, alim.akhtar, jejb
Cc: wsd_upstream, linux-mediatek, chun-hung.wu, alice.chao, cc.chou,
chaotian.jing, jiajie.hao, powen.kao, qilin.tan, lin.gui
On 8/2/22 07:32, peter.wang@mediatek.com wrote:
> From: Peter Wang <peter.wang@mediatek.com>
>
> Set UFSHCD_CAP_WB_WITH_CLK_SCALING for qcom to compatible legacy design.
>
> Signed-off-by: Peter Wang <peter.wang@mediatek.com>
> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
> drivers/ufs/host/ufs-qcom.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index f10d4668814c..f8c9a78e7776 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -869,7 +869,7 @@ static void ufs_qcom_set_caps(struct ufs_hba *hba)
> struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>
> hba->caps |= UFSHCD_CAP_CLK_GATING | UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
> - hba->caps |= UFSHCD_CAP_CLK_SCALING;
> + hba->caps |= UFSHCD_CAP_CLK_SCALING | UFSHCD_CAP_WB_WITH_CLK_SCALING;
> hba->caps |= UFSHCD_CAP_AUTO_BKOPS_SUSPEND;
> hba->caps |= UFSHCD_CAP_WB_EN;
> hba->caps |= UFSHCD_CAP_CRYPTO;
A patch series should be bisectable. Without this patch the previous
patch in this series introduces a regression for Qualcomm controllers.
So I think that the two patches should be combined into a single patch.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] ufs: host: support wb toggle with clock scaling
2022-08-02 18:46 ` Bart Van Assche
@ 2022-08-03 2:42 ` Peter Wang
0 siblings, 0 replies; 7+ messages in thread
From: Peter Wang @ 2022-08-03 2:42 UTC (permalink / raw)
To: Bart Van Assche, stanley.chu, linux-scsi, martin.petersen,
avri.altman, alim.akhtar, jejb
Cc: wsd_upstream, linux-mediatek, chun-hung.wu, alice.chao, cc.chou,
chaotian.jing, jiajie.hao, powen.kao, qilin.tan, lin.gui
On 8/3/22 2:46 AM, Bart Van Assche wrote:
> On 8/2/22 07:32, peter.wang@mediatek.com wrote:
>> From: Peter Wang <peter.wang@mediatek.com>
>>
>> Set UFSHCD_CAP_WB_WITH_CLK_SCALING for qcom to compatible legacy design.
>>
>> Signed-off-by: Peter Wang <peter.wang@mediatek.com>
>> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
>> ---
>> drivers/ufs/host/ufs-qcom.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index f10d4668814c..f8c9a78e7776 100644
>> --- a/drivers/ufs/host/ufs-qcom.c
>> +++ b/drivers/ufs/host/ufs-qcom.c
>> @@ -869,7 +869,7 @@ static void ufs_qcom_set_caps(struct ufs_hba *hba)
>> struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>> hba->caps |= UFSHCD_CAP_CLK_GATING |
>> UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
>> - hba->caps |= UFSHCD_CAP_CLK_SCALING;
>> + hba->caps |= UFSHCD_CAP_CLK_SCALING |
>> UFSHCD_CAP_WB_WITH_CLK_SCALING;
>> hba->caps |= UFSHCD_CAP_AUTO_BKOPS_SUSPEND;
>> hba->caps |= UFSHCD_CAP_WB_EN;
>> hba->caps |= UFSHCD_CAP_CRYPTO;
>
> A patch series should be bisectable. Without this patch the previous
> patch in this series introduces a regression for Qualcomm controllers.
> So I think that the two patches should be combined into a single patch.
>
> Thanks,
>
> Bart.
Hi Bart,
Will combine into a single patch next version.
Thanks.
Peter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] ufs: core: introduce a choice of wb toggle during clock scaling
2022-08-02 18:44 ` Bart Van Assche
@ 2022-08-03 2:44 ` Peter Wang
0 siblings, 0 replies; 7+ messages in thread
From: Peter Wang @ 2022-08-03 2:44 UTC (permalink / raw)
To: Bart Van Assche, stanley.chu, linux-scsi, martin.petersen,
avri.altman, alim.akhtar, jejb
Cc: wsd_upstream, linux-mediatek, chun-hung.wu, alice.chao, cc.chou,
chaotian.jing, jiajie.hao, powen.kao, qilin.tan, lin.gui
Hi Bart,
On 8/3/22 2:44 AM, Bart Van Assche wrote:
> On 8/2/22 07:32, peter.wang@mediatek.com wrote:
>> + /* Allow WB with clk scaling */
>
> This comment is misleading. Consider changing this comment into
> something like "Enable WB when scaling up the clock and disable WB
> when scaling the clock down".
Will change comment next version, thanks.
>
>> + UFSHCD_CAP_WB_WITH_CLK_SCALING = 1 << 12,
>
> Whether or not the WriteBooster is toggled when the clock is scaled is
> not a host controller capability. It is a policy that is tied by this
> patch to the host controller. So I think that using the "UFSHCD_CAP_"
> prefix is misleading. Consider using e.g. the prefix UFSHCD_POLICY_.
>
The prefix UFSHCD_CAP_ is used in ufshcd_caps and not all of them is
host capability.
Ex. UFSHCD_CAP_HIBERN8_WITH_CLK_GATING is a policy host send hiberb8
with clk gating or not.
Ex. UFSHCD_CAP_WB_EN is a host policy to turn-on WriteBooster or not.
So, I think it is not suitable to break the naming rule in ufshcd_caps now.
> > +static inline bool ufshcd_can_wb_during_scaling(struct ufs_hba *hba)
> > +{
> > + return hba->caps & UFSHCD_CAP_WB_WITH_CLK_SCALING;
> > +}
>
> The name of this function is misleading. Consider renaming this
> function into e.g. ufshcd_enable_wb_if_scaling_up().
>
> Thanks,
>
> Bart.
Will change function name next version.
Thanks.
Peter
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-03 2:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-02 14:32 [PATCH v3 0/2] ufs: allow host driver disable wb toggle druing clock scaling peter.wang
2022-08-02 14:32 ` [PATCH v3 1/2] ufs: core: introduce a choice of wb toggle during " peter.wang
2022-08-02 18:44 ` Bart Van Assche
2022-08-03 2:44 ` Peter Wang
2022-08-02 14:32 ` [PATCH v3 2/2] ufs: host: support wb toggle with " peter.wang
2022-08-02 18:46 ` Bart Van Assche
2022-08-03 2:42 ` Peter Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.