The parameter name of ufshcd_wb_toggle_flush_during_h8() has been changed
in the same as other toggle functions.
Function names were ambiguous. So changed to suit the meaning.
Reviewed-by: Avri Altman <[email protected]>
Signed-off-by: Jinyoung Choi <[email protected]>
---
drivers/ufs/core/ufshcd.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 48ba109e29f7..52377fedae49 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -265,8 +265,9 @@ static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
struct ufs_vreg *vreg);
static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
-static void ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
-static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable);
+static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
+ bool enable);
+static void ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable);
static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
@@ -286,16 +287,16 @@ static inline void ufshcd_disable_irq(struct ufs_hba *hba)
}
}
-static inline void ufshcd_wb_config(struct ufs_hba *hba)
+static void ufshcd_wb_set_default_flags(struct ufs_hba *hba)
{
if (!ufshcd_is_wb_allowed(hba))
return;
ufshcd_wb_toggle(hba, true);
- ufshcd_wb_toggle_flush_during_h8(hba, true);
+ ufshcd_wb_toggle_buf_flush_during_h8(hba, true);
if (!(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL))
- ufshcd_wb_toggle_flush(hba, true);
+ ufshcd_wb_toggle_buf_flush(hba, true);
}
static void ufshcd_scsi_unblock_requests(struct ufs_hba *hba)
@@ -5750,22 +5751,23 @@ int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable)
return ret;
}
-static void ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set)
+static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
+ bool enable)
{
int ret;
- ret = __ufshcd_wb_toggle(hba, set,
+ ret = __ufshcd_wb_toggle(hba, enable,
QUERY_FLAG_IDN_WB_BUFF_FLUSH_DURING_HIBERN8);
if (ret) {
dev_err(hba->dev, "%s: WB-Buf Flush during H8 %s failed: %d\n",
- __func__, set ? "enable" : "disable", ret);
+ __func__, enable ? "enable" : "disable", ret);
return;
}
dev_dbg(hba->dev, "%s WB-Buf Flush during H8 %s\n",
- __func__, set ? "enabled" : "disabled");
+ __func__, enable ? "enabled" : "disabled");
}
-static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable)
+static void ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable)
{
int ret;
@@ -5814,9 +5816,9 @@ static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba,
static void ufshcd_wb_force_disable(struct ufs_hba *hba)
{
if (!(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL))
- ufshcd_wb_toggle_flush(hba, false);
+ ufshcd_wb_toggle_buf_flush(hba, false);
- ufshcd_wb_toggle_flush_during_h8(hba, false);
+ ufshcd_wb_toggle_buf_flush_during_h8(hba, false);
ufshcd_wb_toggle(hba, false);
hba->caps &= ~UFSHCD_CAP_WB_EN;
@@ -8213,7 +8215,9 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params)
*/
ufshcd_set_active_icc_lvl(hba);
- ufshcd_wb_config(hba);
+ /* Enable UFS Write Booster if supported */
+ ufshcd_wb_set_default_flags(hba);
+
if (hba->ee_usr_mask)
ufshcd_write_ee_control(hba);
/* Enable Auto-Hibernate if configured */
--
2.25.1
There is the following quirk to bypass "WB Flush" in Write Booster.
- UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
If this quirk is not set, there is no knob that can control "WB Flush".
There are three flags that control Write Booster Feature.
1. WB ON/OFF
2. WB Hibern Flush ON/OFF (implicitly)
3. WB Flush ON/OFF (explicit)
The sysfs attribute that controls the WB was implemented. (1)
In the case of "Hibern Flush", it is always good to turn on.
Control may not be required. (2)
Finally, "Flush" may be necessary because the Auto-Hibern8 is not
supported in a specific environment.
So the sysfs attribute that controls this is necessary. (3)
Reviewed-by: Avri Altman <[email protected]>
Signed-off-by: Jinyoung Choi <[email protected]>
---
drivers/ufs/core/ufs-sysfs.c | 45 ++++++++++++++++++++++++++++++++++++
drivers/ufs/core/ufshcd.c | 9 ++++----
include/ufs/ufshcd.h | 1 +
3 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 0a088b47d557..e7800e49998a 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -254,6 +254,49 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr,
return res < 0 ? res : count;
}
+static ssize_t wb_buf_flush_en_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ufs_hba *hba = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", hba->dev_info.wb_buf_flush_enabled);
+}
+
+static ssize_t wb_buf_flush_en_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ufs_hba *hba = dev_get_drvdata(dev);
+ unsigned int wb_buf_flush_en;
+ ssize_t res;
+
+ if (ufshcd_is_wb_allowed(hba) &&
+ !(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL)) {
+ dev_warn(dev, "It is not allowed to configure WB buf flush!\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (kstrtouint(buf, 0, &wb_buf_flush_en))
+ return -EINVAL;
+
+ if (wb_buf_flush_en != 0 && wb_buf_flush_en != 1)
+ return -EINVAL;
+
+ down(&hba->host_sem);
+ if (!ufshcd_is_user_access_allowed(hba)) {
+ res = -EBUSY;
+ goto out;
+ }
+
+ ufshcd_rpm_get_sync(hba);
+ res = ufshcd_wb_toggle_buf_flush(hba, wb_buf_flush_en);
+ ufshcd_rpm_put_sync(hba);
+out:
+ up(&hba->host_sem);
+ return res < 0 ? res : count;
+}
+
static DEVICE_ATTR_RW(rpm_lvl);
static DEVICE_ATTR_RO(rpm_target_dev_state);
static DEVICE_ATTR_RO(rpm_target_link_state);
@@ -262,6 +305,7 @@ static DEVICE_ATTR_RO(spm_target_dev_state);
static DEVICE_ATTR_RO(spm_target_link_state);
static DEVICE_ATTR_RW(auto_hibern8);
static DEVICE_ATTR_RW(wb_on);
+static DEVICE_ATTR_RW(wb_buf_flush_en);
static struct attribute *ufs_sysfs_ufshcd_attrs[] = {
&dev_attr_rpm_lvl.attr,
@@ -272,6 +316,7 @@ static struct attribute *ufs_sysfs_ufshcd_attrs[] = {
&dev_attr_spm_target_link_state.attr,
&dev_attr_auto_hibern8.attr,
&dev_attr_wb_on.attr,
+ &dev_attr_wb_buf_flush_en.attr,
NULL
};
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 52377fedae49..a8a797e0033d 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -267,7 +267,6 @@ static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag);
static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
bool enable);
-static void ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable);
static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
@@ -5767,24 +5766,26 @@ static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba,
__func__, enable ? "enabled" : "disabled");
}
-static void ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable)
+int ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable)
{
int ret;
if (hba->dev_info.wb_buf_flush_enabled == enable)
- return;
+ return 0;
ret = __ufshcd_wb_toggle(hba, enable, QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN);
if (ret) {
dev_err(hba->dev, "%s WB-Buf Flush %s failed %d\n", __func__,
enable ? "enable" : "disable", ret);
- return;
+ return ret;
}
hba->dev_info.wb_buf_flush_enabled = enable;
dev_dbg(hba->dev, "%s WB-Buf Flush %s\n",
__func__, enable ? "enabled" : "disabled");
+
+ return ret;
}
static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba,
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 7fe1a926cd99..94bcfec98fb8 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1211,6 +1211,7 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
enum query_opcode desc_op);
int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable);
+int ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable);
int ufshcd_suspend_prepare(struct device *dev);
int __ufshcd_suspend_prepare(struct device *dev, bool rpm_ok_for_spm);
void ufshcd_resume_complete(struct device *dev);
--
2.25.1
The explicit flushing should check the following.
- UFSHCD_CAP_WB_EN
- UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
Changed to improve readability.
Reviewed-by: Avri Altman <[email protected]>
Signed-off-by: Jinyoung Choi <[email protected]>
---
drivers/ufs/core/ufs-sysfs.c | 3 +--
drivers/ufs/core/ufshcd.c | 5 +++--
include/ufs/ufshcd.h | 6 ++++++
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index e7800e49998a..63b02b2541c8 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -271,8 +271,7 @@ static ssize_t wb_buf_flush_en_store(struct device *dev,
unsigned int wb_buf_flush_en;
ssize_t res;
- if (ufshcd_is_wb_allowed(hba) &&
- !(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL)) {
+ if (!ufshcd_is_wb_buf_flush_allowed(hba)) {
dev_warn(dev, "It is not allowed to configure WB buf flush!\n");
return -EOPNOTSUPP;
}
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index a8a797e0033d..17a14b046c44 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -294,7 +294,8 @@ static void ufshcd_wb_set_default_flags(struct ufs_hba *hba)
ufshcd_wb_toggle(hba, true);
ufshcd_wb_toggle_buf_flush_during_h8(hba, true);
- if (!(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL))
+
+ if (ufshcd_is_wb_buf_flush_allowed(hba))
ufshcd_wb_toggle_buf_flush(hba, true);
}
@@ -5816,7 +5817,7 @@ static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba,
static void ufshcd_wb_force_disable(struct ufs_hba *hba)
{
- if (!(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL))
+ if (ufshcd_is_wb_buf_flush_allowed(hba))
ufshcd_wb_toggle_buf_flush(hba, false);
ufshcd_wb_toggle_buf_flush_during_h8(hba, false);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 94bcfec98fb8..78adc556444a 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1017,6 +1017,12 @@ static inline bool ufshcd_is_wb_allowed(struct ufs_hba *hba)
return hba->caps & UFSHCD_CAP_WB_EN;
}
+static inline bool ufshcd_is_wb_buf_flush_allowed(struct ufs_hba *hba)
+{
+ return ufshcd_is_wb_allowed(hba) &&
+ !(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL);
+}
+
#define ufshcd_writel(hba, val, reg) \
writel((val), (hba)->mmio_base + (reg))
#define ufshcd_readl(hba, reg) \
--
2.25.1
On Wed, 2022-07-27 at 16:08 +0900, Jinyoung CHOI wrote:
> There is the following quirk to bypass "WB Flush" in Write Booster.
>
> - UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
>
> If this quirk is not set, there is no knob that can control "WB
> Flush".
>
> There are three flags that control Write Booster Feature.
> 1. WB ON/OFF
> 2. WB Hibern Flush ON/OFF (implicitly)
> 3. WB Flush ON/OFF (explicit)
>
> The sysfs attribute that controls the WB was implemented. (1)
>
> In the case of "Hibern Flush", it is always good to turn on.
> Control may not be required. (2)
>
> Finally, "Flush" may be necessary because the Auto-Hibern8 is not
> supported in a specific environment.
> So the sysfs attribute that controls this is necessary. (3)
>
> Reviewed-by: Avri Altman <[email protected]>
> Signed-off-by: Jinyoung Choi <[email protected]>
> ---
...
>
> +static ssize_t wb_buf_flush_en_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + unsigned int wb_buf_flush_en;
> + ssize_t res;
> +
> + if (ufshcd_is_wb_allowed(hba) &&
> + !(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL)) {
> + dev_warn(dev, "It is not allowed to configure WB buf
> flush!\n");
> + return -EOPNOTSUPP;
> + }
> +
Hi J-young,
I don't understand here, if UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL is
not set (manual flush is not disable), so we cannot manually flush
buffer? or should we check if Auto-Hibern8 is supported?
Kind regards,
Bean
On Wed, 2022-07-27 at 16:10 +0900, Jinyoung CHOI wrote:
> The explicit flushing should check the following.
> - UFSHCD_CAP_WB_EN
> - UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
>
> Changed to improve readability.
>
> Reviewed-by: Avri Altman <[email protected]>
> Signed-off-by: Jinyoung Choi <[email protected]>
Acked-by: Bean Huo <[email protected]>
On 7/27/22 00:10, Jinyoung CHOI wrote:
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 94bcfec98fb8..78adc556444a 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -1017,6 +1017,12 @@ static inline bool ufshcd_is_wb_allowed(struct ufs_hba *hba)
> return hba->caps & UFSHCD_CAP_WB_EN;
> }
>
> +static inline bool ufshcd_is_wb_buf_flush_allowed(struct ufs_hba *hba)
> +{
> + return ufshcd_is_wb_allowed(hba) &&
> + !(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL);
> +}
Since this function is only used inside the UFS driver core it should be
added in drivers/ufs/core/ufshcd-priv.h instead of include/ufs/ufshcd.h.
Thanks,
Bart.
>> There is the following quirk to bypass "WB Flush" in Write Booster.
>>
>> - UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL
>>
>> If this quirk is not set, there is no knob that can control "WB
>> Flush".
>
>>
>> There are three flags that control Write Booster Feature.
>> 1. WB ON/OFF
>> 2. WB Hibern Flush ON/OFF (implicitly)
>> 3. WB Flush ON/OFF (explicit)
>>
>> The sysfs attribute that controls the WB was implemented. (1)
>>
>> In the case of "Hibern Flush", it is always good to turn on.
>> Control may not be required. (2)
>>
>> Finally, "Flush" may be necessary because the Auto-Hibern8 is not
>> supported in a specific environment.
>> So the sysfs attribute that controls this is necessary. (3)
>>
>> Reviewed-by: Avri Altman <[email protected]>
>> Signed-off-by: Jinyoung Choi <[email protected]>
>> ---
>...
>>
>> +static ssize_t wb_buf_flush_en_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct ufs_hba *hba = dev_get_drvdata(dev);
>> + unsigned int wb_buf_flush_en;
>> + ssize_t res;
>> +
>> + if (ufshcd_is_wb_allowed(hba) &&
>> + !(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL)) {
>> + dev_warn(dev, "It is not allowed to configure WB buf
>> flush!\n");
>> + return -EOPNOTSUPP;
>> + }
>> +
>Hi J-young,
>
>I don't understand here, if UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL is
>not set (manual flush is not disable), so we cannot manually flush
>buffer? or should we check if Auto-Hibern8 is supported?
>
>Kind regards,
>Bean
Hi Bean,
As the patch was separated, the conditional sentence went wrong.
Thank you for checking. I will modify it quickly.
Thanks,
Jinyoung.
>On 7/27/22 00:10, Jinyoung CHOI wrote:
>> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
>> index 94bcfec98fb8..78adc556444a 100644
>> --- a/include/ufs/ufshcd.h
>> +++ b/include/ufs/ufshcd.h
>> @@ -1017,6 +1017,12 @@ static inline bool ufshcd_is_wb_allowed(struct ufs_hba *hba)
>> return hba->caps & UFSHCD_CAP_WB_EN;
>> }
>>
>> +static inline bool ufshcd_is_wb_buf_flush_allowed(struct ufs_hba *hba)
>> +{
>> + return ufshcd_is_wb_allowed(hba) &&
>> + !(hba->quirks & UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL);
>> +}
>
>Since this function is only used inside the UFS driver core it should be
>added in drivers/ufs/core/ufshcd-priv.h instead of include/ufs/ufshcd.h.
>
>Thanks,
>
>Bart.
OK, I will move it.
I didn't know the exact purpose of ufshcd-priv.h.
Thank you for telling me.
Thanks, Jinyoung.