2020-05-09 09:39:29

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v2 0/4] scsi: ufs: allow customizable WriteBooster flush policy

Hi,

This patch set tries to allow vendors to modify the WriteBooster flush policy.

In the same time, collect all customizable parameters to an unified structure to make UFS driver more clean.

v1 -> v2:
- Squash patch [3] and [4]
- Remove a dummy "new line" in patch [3]
- Fix commit message in patch [3]

Stanley Chu (4):
scsi: ufs: introduce ufs_hba_variant_params to collect customizable
parameters
scsi: ufs-mediatek: change the way to use customizable parameters
scsi: ufs: customize flush threshold for WriteBooster
scsi: ufs-mediatek: customize WriteBooster flush policy

drivers/scsi/ufs/ufs-mediatek.c | 5 ++--
drivers/scsi/ufs/ufs.h | 5 +---
drivers/scsi/ufs/ufshcd.c | 45 ++++++++++++++-------------------
drivers/scsi/ufs/ufshcd.h | 9 ++++++-
4 files changed, 31 insertions(+), 33 deletions(-)

--
2.18.0


2020-05-09 09:39:32

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v2 2/4] scsi: ufs-mediatek: change the way to use customizable parameters

Now all customizable parameters have been moved to hba->vps,
thus modify the way to use them.

Signed-off-by: Stanley Chu <[email protected]>
---
drivers/scsi/ufs/ufs-mediatek.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index c543142554d3..56620f7d88ce 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -79,9 +79,9 @@ static int ufs_mtk_hce_enable_notify(struct ufs_hba *hba,

if (status == PRE_CHANGE) {
if (host->unipro_lpm)
- hba->hba_enable_delay_us = 0;
+ hba->vps->hba_enable_delay_us = 0;
else
- hba->hba_enable_delay_us = 600;
+ hba->vps->hba_enable_delay_us = 600;
}

return 0;
--
2.18.0

2020-05-09 09:40:25

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v2 4/4] scsi: ufs-mediatek: customize WriteBooster flush policy

Change the WriteBooster policy to keep VCC on during
runtime suspend if available WriteBooster buffer is less
than 80%.

Signed-off-by: Stanley Chu <[email protected]>
---
drivers/scsi/ufs/ufs-mediatek.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 56620f7d88ce..94e97701f456 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -271,6 +271,7 @@ static int ufs_mtk_init(struct ufs_hba *hba)

/* Enable WriteBooster */
hba->caps |= UFSHCD_CAP_WB_EN;
+ hba->vps->wb_flush_threshold = UFS_WB_BUF_REMAIN_PERCENT(80);

/*
* ufshcd_vops_init() is invoked after
--
2.18.0

2020-05-09 09:41:11

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v2 1/4] scsi: ufs: introduce ufs_hba_variant_params to collect customizable parameters

There are more and more customizable parameters showed up
in UFS driver. Let's collect them into a unified place to make
the driver more clean.

Signed-off-by: Stanley Chu <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 38 +++++++++++++++-----------------------
drivers/scsi/ufs/ufshcd.h | 8 +++++++-
2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 426073a518ef..cdacbe6378a1 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1353,23 +1353,6 @@ static int ufshcd_devfreq_get_dev_status(struct device *dev,
return 0;
}

-static struct devfreq_dev_profile ufs_devfreq_profile = {
- .polling_ms = 100,
- .target = ufshcd_devfreq_target,
- .get_dev_status = ufshcd_devfreq_get_dev_status,
-};
-
-#if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)
-static struct devfreq_simple_ondemand_data ufs_ondemand_data = {
- .upthreshold = 70,
- .downdifferential = 5,
-};
-
-static void *gov_data = &ufs_ondemand_data;
-#else
-static void *gov_data; /* NULL */
-#endif
-
static int ufshcd_devfreq_init(struct ufs_hba *hba)
{
struct list_head *clk_list = &hba->clk_list_head;
@@ -1385,12 +1368,12 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
dev_pm_opp_add(hba->dev, clki->min_freq, 0);
dev_pm_opp_add(hba->dev, clki->max_freq, 0);

- ufshcd_vops_config_scaling_param(hba, &ufs_devfreq_profile,
- gov_data);
+ ufshcd_vops_config_scaling_param(hba, &hba->vps->devfreq_profile,
+ &hba->vps->ondemand_data);
devfreq = devfreq_add_device(hba->dev,
- &ufs_devfreq_profile,
+ &hba->vps->devfreq_profile,
DEVFREQ_GOV_SIMPLE_ONDEMAND,
- gov_data);
+ &hba->vps->ondemand_data);
if (IS_ERR(devfreq)) {
ret = PTR_ERR(devfreq);
dev_err(hba->dev, "Unable to register with devfreq %d\n", ret);
@@ -4314,7 +4297,7 @@ int ufshcd_hba_enable(struct ufs_hba *hba)
* instruction might be read back.
* This delay can be changed based on the controller.
*/
- ufshcd_delay_us(hba->hba_enable_delay_us, 100);
+ ufshcd_delay_us(hba->vps->hba_enable_delay_us, 100);

/* wait for the host controller to complete initialization */
retry = 50;
@@ -7477,6 +7460,15 @@ static const struct attribute_group *ufshcd_driver_groups[] = {
NULL,
};

+static struct ufs_hba_variant_params ufs_hba_vps = {
+ .hba_enable_delay_us = 1000,
+ .devfreq_profile.polling_ms = 100,
+ .devfreq_profile.target = ufshcd_devfreq_target,
+ .devfreq_profile.get_dev_status = ufshcd_devfreq_get_dev_status,
+ .ondemand_data.upthreshold = 70,
+ .ondemand_data.downdifferential = 5,
+};
+
static struct scsi_host_template ufshcd_driver_template = {
.module = THIS_MODULE,
.name = UFSHCD,
@@ -8724,7 +8716,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)

hba->mmio_base = mmio_base;
hba->irq = irq;
- hba->hba_enable_delay_us = 1000;
+ hba->vps = &ufs_hba_vps;

err = ufshcd_hba_init(hba);
if (err)
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 23a434c03c2a..f7bdf52ba8b0 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -566,6 +566,12 @@ enum ufshcd_caps {
UFSHCD_CAP_WB_EN = 1 << 7,
};

+struct ufs_hba_variant_params {
+ struct devfreq_dev_profile devfreq_profile;
+ struct devfreq_simple_ondemand_data ondemand_data;
+ u16 hba_enable_delay_us;
+};
+
/**
* struct ufs_hba - per adapter private structure
* @mmio_base: UFSHCI base register address
@@ -663,6 +669,7 @@ struct ufs_hba {
int nutmrs;
u32 ufs_version;
const struct ufs_hba_variant_ops *vops;
+ struct ufs_hba_variant_params *vps;
void *priv;
unsigned int irq;
bool is_irq_enabled;
@@ -684,7 +691,6 @@ struct ufs_hba {
u32 eh_flags;
u32 intr_mask;
u16 ee_ctrl_mask;
- u16 hba_enable_delay_us;
bool is_powered;

/* Work Queues */
--
2.18.0

2020-05-12 02:24:43

by Asutosh Das (asd)

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] scsi: ufs-mediatek: customize WriteBooster flush policy

On 5/9/2020 2:37 AM, Stanley Chu wrote:
> Change the WriteBooster policy to keep VCC on during
> runtime suspend if available WriteBooster buffer is less
> than 80%.
>
> Signed-off-by: Stanley Chu <[email protected]>
> ---
> drivers/scsi/ufs/ufs-mediatek.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
> index 56620f7d88ce..94e97701f456 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.c
> +++ b/drivers/scsi/ufs/ufs-mediatek.c
> @@ -271,6 +271,7 @@ static int ufs_mtk_init(struct ufs_hba *hba)
>
> /* Enable WriteBooster */
> hba->caps |= UFSHCD_CAP_WB_EN;
> + hba->vps->wb_flush_threshold = UFS_WB_BUF_REMAIN_PERCENT(80);
>
> /*
> * ufshcd_vops_init() is invoked after
>

Patchset looks good to me.

Reviewed-by: Asutosh Das <[email protected]>


--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project

2020-05-12 16:24:41

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] scsi: ufs-mediatek: customize WriteBooster flush policy


Hi Asutosh!

> Patchset looks good to me.
>
> Reviewed-by: Asutosh Das <[email protected]>

When you want to approve an entire series, please respond to the cover
letter email. Otherwise the kernel.org tooling will only record the tag
for the individual patch you are responding to. In this case only patch
4 got tagged as reviewed in patchwork.

--
Martin K. Petersen Oracle Linux Engineering

2020-05-12 18:13:24

by Asutosh Das (asd)

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] scsi: ufs-mediatek: customize WriteBooster flush policy

On 5/12/2020 9:21 AM, Martin K. Petersen wrote:
>
> Hi Asutosh!
>
>> Patchset looks good to me.
>>
>> Reviewed-by: Asutosh Das <[email protected]>
>
> When you want to approve an entire series, please respond to the cover
> letter email. Otherwise the kernel.org tooling will only record the tag
> for the individual patch you are responding to. In this case only patch
> 4 got tagged as reviewed in patchwork.
>

Hi Martin
Sure - I'll keep this in mind.

Thanks,
Asutosh

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project

2020-05-15 01:16:11

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] scsi: ufs: allow customizable WriteBooster flush policy

On Sat, 9 May 2020 17:37:12 +0800, Stanley Chu wrote:

> This patch set tries to allow vendors to modify the WriteBooster flush policy.
>
> In the same time, collect all customizable parameters to an unified structure to make UFS driver more clean.
>
> v1 -> v2:
> - Squash patch [3] and [4]
> - Remove a dummy "new line" in patch [3]
> - Fix commit message in patch [3]
>
> [...]

Applied to 5.8/scsi-queue, thanks!

I had to combine patches 1 and 2. Otherwise you'd get compile
failures due to the fields moving inside the struct.

[1/4] scsi: ufs: Introduce ufs_hba_variant_params to group customizable parameters
https://git.kernel.org/mkp/scsi/c/90b8491c0033
[3/4] scsi: ufs: Customize flush threshold for WriteBooster
https://git.kernel.org/mkp/scsi/c/d14734ae3ae7
[4/4] scsi: ufs-mediatek: Customize WriteBooster flush policy
https://git.kernel.org/mkp/scsi/c/f48b285ae658

--
Martin K. Petersen Oracle Linux Engineering

2020-05-15 01:18:24

by Stanley Chu

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] scsi: ufs: allow customizable WriteBooster flush policy

Hi Martin,

On Thu, 2020-05-14 at 21:10 -0400, Martin K. Petersen wrote:
> On Sat, 9 May 2020 17:37:12 +0800, Stanley Chu wrote:
>
> > This patch set tries to allow vendors to modify the WriteBooster flush policy.
> >
> > In the same time, collect all customizable parameters to an unified structure to make UFS driver more clean.
> >
> > v1 -> v2:
> > - Squash patch [3] and [4]
> > - Remove a dummy "new line" in patch [3]
> > - Fix commit message in patch [3]
> >
> > [...]
>
> Applied to 5.8/scsi-queue, thanks!
>
> I had to combine patches 1 and 2. Otherwise you'd get compile
> failures due to the fields moving inside the struct.
>
> [1/4] scsi: ufs: Introduce ufs_hba_variant_params to group customizable parameters
> https://git.kernel.org/mkp/scsi/c/90b8491c0033
> [3/4] scsi: ufs: Customize flush threshold for WriteBooster
> https://git.kernel.org/mkp/scsi/c/d14734ae3ae7
> [4/4] scsi: ufs-mediatek: Customize WriteBooster flush policy
> https://git.kernel.org/mkp/scsi/c/f48b285ae658
>

Thanks so much for helping the patch squash.

Thanks,
Stanley Chu