Re: [PATCH v1 4/4] scsi: ufs: Fix WriteBooster flush during runtime suspend
On 5/14/2020 7:49 AM, Stanley Chu wrote:
Hi Asutosh,
On Thu, 2020-05-14 at 10:23 +0800, Stanley Chu wrote:
Hi Asutosh,
On Wed, 2020-05-13 at 12:31 -0700, Asutosh Das (asd) wrote:
On 5/12/2020 3:47 AM, Stanley Chu wrote:
Currently UFS host driver promises VCC supply if UFS device
needs to do WriteBooster flush during runtime suspend.
However the UFS specification mentions,
"While the flushing operation is in progress, the device is
in Active power mode."
Therefore UFS host driver needs to promise more: Keep UFS
device as "Active power mode", otherwise UFS device shall not
do any flush if device enters Sleep or PowerDown power mode.
Fix this by not changing device power mode if WriteBooster
flush is required in ufshcd_suspend().
Signed-off-by: Stanley Chu
---
drivers/scsi/ufs/ufs.h| 1 -
drivers/scsi/ufs/ufshcd.c | 39 +++
2 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index b3135344ab3f..9e4bc2e97ada 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -577,7 +577,6 @@ struct ufs_dev_info {
u32 d_ext_ufs_feature_sup;
u8 b_wb_buffer_type;
u32 d_wb_alloc_units;
- bool keep_vcc_on;
u8 b_presrv_uspc_en;
};
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 169a3379e468..2d0aff8ac260 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8101,8 +8101,7 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
!hba->dev_info.is_lu_power_on_wp) {
ufshcd_setup_vreg(hba, false);
} else if (!ufshcd_is_ufs_dev_active(hba)) {
- if (!hba->dev_info.keep_vcc_on)
- ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
+ ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
if (!ufshcd_is_link_active(hba)) {
ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq);
ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq2);
@@ -8172,6 +8171,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
ufs_pm_op pm_op)
enum ufs_pm_level pm_lvl;
enum ufs_dev_pwr_mode req_dev_pwr_mode;
enum uic_link_state req_link_state;
+ bool keep_curr_dev_pwr_mode = false;
hba->pm_op_in_progress = 1;
if (!ufshcd_is_shutdown_pm(pm_op)) {
@@ -8226,28 +8226,27 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
ufs_pm_op pm_op)
/* make sure that auto bkops is disabled */
ufshcd_disable_auto_bkops(hba);
}
+
Unnecessary newline, perhaps?
Yap, I will remove it in next version.
/*
-* With wb enabled, if the bkops is enabled or if the
-* configured WB type is 70% full, keep vcc ON
-* for the device to flush the wb buffer
+* If device needs to do BKOP or WB buffer flush, keep device
+* power mode as "active power mode" and its VCC supply.
*/
- if ((hba->auto_bkops_enabled && ufshcd_is_wb_allowed(hba)) ||
- ufshcd_wb_keep_vcc_on(hba))
- hba->dev_info.keep_vcc_on = true;
- else
- hba->dev_info.keep_vcc_on = false;
- } else {
- hba->dev_info.keep_vcc_on = false;
+ keep_curr_dev_pwr_mode = hba->auto_bkops_enabled ||
+ ufshcd_wb_keep_vcc_on(hba);
Should the device be in UFS_ACTIVE_PWR_MODE to perform auto-bkops?
Also, is it needed to keep the device in UFS_ACTIVE_PWR_MODE , if flush
on hibern8 is enabled and the link is being put to hibern8 mode during
runtime-suspend? Perhaps that should also be factored in here?
Both auto-bkops and WriteBooster flush during Hibern8 need device power
mode to be "Active Power Mode".
For auto-bkops, the spec mentions,
"If the background operations enable bit is set and the device is in
Active power mode or Idle power mode, then the device is allowed to
execute any internal operations."
For WriteBooster flush during Hibern8, the spec mentions,
"While the flushing operation is in progress, the device is in Active
power mode."
Therefore here we can use an unified "keep_curr_dev_pwr_mode" to
indicate the same requirements of above both features.
Besides, both operations may access flash array inside UFS device thus
VCC supply shall be also kept.
Before this patch, the original code will keep device power mode (stay
in Active Power Mode) if hba->auto_bkops_enabled is set as true during
runtime-suspend with UFSHCD_CAP_AUTO_BKOPS_SUSPEND capability is
enabled. This patch will not change this decision, just add
"WriteBooster flush during Hibern8" feature as another condition to do
so.
Thank you so much to remind me that "Link shall be put in Hibern8" is a
necessary condition for "WriteBoost
Re: [PATCH v1 4/4] scsi: ufs: Fix WriteBooster flush during runtime suspend
Hi Asutosh,
On Thu, 2020-05-14 at 10:23 +0800, Stanley Chu wrote:
> Hi Asutosh,
>
> On Wed, 2020-05-13 at 12:31 -0700, Asutosh Das (asd) wrote:
> > On 5/12/2020 3:47 AM, Stanley Chu wrote:
> > > Currently UFS host driver promises VCC supply if UFS device
> > > needs to do WriteBooster flush during runtime suspend.
> > >
> > > However the UFS specification mentions,
> > >
> > > "While the flushing operation is in progress, the device is
> > > in Active power mode."
> > >
> > > Therefore UFS host driver needs to promise more: Keep UFS
> > > device as "Active power mode", otherwise UFS device shall not
> > > do any flush if device enters Sleep or PowerDown power mode.
> > >
> > > Fix this by not changing device power mode if WriteBooster
> > > flush is required in ufshcd_suspend().
> > >
> > > Signed-off-by: Stanley Chu
> > > ---
> > > drivers/scsi/ufs/ufs.h| 1 -
> > > drivers/scsi/ufs/ufshcd.c | 39 +++
> > > 2 files changed, 19 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> > > index b3135344ab3f..9e4bc2e97ada 100644
> > > --- a/drivers/scsi/ufs/ufs.h
> > > +++ b/drivers/scsi/ufs/ufs.h
> > > @@ -577,7 +577,6 @@ struct ufs_dev_info {
> > > u32 d_ext_ufs_feature_sup;
> > > u8 b_wb_buffer_type;
> > > u32 d_wb_alloc_units;
> > > - bool keep_vcc_on;
> > > u8 b_presrv_uspc_en;
> > > };
> > >
> > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > > index 169a3379e468..2d0aff8ac260 100644
> > > --- a/drivers/scsi/ufs/ufshcd.c
> > > +++ b/drivers/scsi/ufs/ufshcd.c
> > > @@ -8101,8 +8101,7 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
> > > !hba->dev_info.is_lu_power_on_wp) {
> > > ufshcd_setup_vreg(hba, false);
> > > } else if (!ufshcd_is_ufs_dev_active(hba)) {
> > > - if (!hba->dev_info.keep_vcc_on)
> > > - ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
> > > + ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
> > > if (!ufshcd_is_link_active(hba)) {
> > > ufshcd_config_vreg_lpm(hba,
> > > hba->vreg_info.vccq);
> > > ufshcd_config_vreg_lpm(hba,
> > > hba->vreg_info.vccq2);
> > > @@ -8172,6 +8171,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
> > > ufs_pm_op pm_op)
> > > enum ufs_pm_level pm_lvl;
> > > enum ufs_dev_pwr_mode req_dev_pwr_mode;
> > > enum uic_link_state req_link_state;
> > > + bool keep_curr_dev_pwr_mode = false;
> > >
> > > hba->pm_op_in_progress = 1;
> > > if (!ufshcd_is_shutdown_pm(pm_op)) {
> > > @@ -8226,28 +8226,27 @@ static int ufshcd_suspend(struct ufs_hba *hba,
> > > enum ufs_pm_op pm_op)
> > > /* make sure that auto bkops is disabled */
> > > ufshcd_disable_auto_bkops(hba);
> > > }
> > > +
> > Unnecessary newline, perhaps?
>
> Yap, I will remove it in next version.
>
> > > /*
> > > - * With wb enabled, if the bkops is enabled or if the
> > > - * configured WB type is 70% full, keep vcc ON
> > > - * for the device to flush the wb buffer
> > > + * If device needs to do BKOP or WB buffer flush, keep device
> > > + * power mode as "active power mode" and its VCC supply.
> > >*/
> > > - if ((hba->auto_bkops_enabled && ufshcd_is_wb_allowed(hba)) ||
> > > - ufshcd_wb_keep_vcc_on(hba))
> > > - hba->dev_info.keep_vcc_on = true;
> > > - else
> > > - hba->dev_info.keep_vcc_on = false;
> > > - } else {
> > > - hba->dev_info.keep_vcc_on = false;
> > > + keep_curr_dev_pwr_mode = hba->auto_bkops_enabled ||
> > > + ufshcd_wb_keep_vcc_on(hba);
> > Should the device be in UFS_ACTIVE_PWR_MODE to perform auto-bkops?
> >
> > Also, is it needed to keep the device in UFS_ACTIVE_PWR_MODE , if flush
> > on hibern8 is enabled and the link is being put to hibern8 mode during
> > runtime-suspend? Perhaps that should also be factored in here?
>
> Both auto-bkops and WriteBooster flush during Hibern8 need device power
> mode to be "Active Power Mode".
>
> For auto-bkops, the spec mentions,
>
> "If the background operations enable bit is set and the device is in
> Active power mode or Idle power mode, then the device is allowed to
> execute any internal operations."
>
> For WriteBooster flush during Hibern8, the spec mentions,
>
> "While the flushing operation is in progress, the device is in Active
> power mode."
>
> Therefore here we can use an unified "keep_curr_dev_pwr_mode" to
> indicate the same requirements of above both features.
>
> Besides, both operations may access flash array inside UFS device thus
> VCC supply shall be also kept.
>
> Before this
Re: [PATCH v1 4/4] scsi: ufs: Fix WriteBooster flush during runtime suspend
Hi Asutosh,
On Wed, 2020-05-13 at 12:31 -0700, Asutosh Das (asd) wrote:
> On 5/12/2020 3:47 AM, Stanley Chu wrote:
> > Currently UFS host driver promises VCC supply if UFS device
> > needs to do WriteBooster flush during runtime suspend.
> >
> > However the UFS specification mentions,
> >
> > "While the flushing operation is in progress, the device is
> > in Active power mode."
> >
> > Therefore UFS host driver needs to promise more: Keep UFS
> > device as "Active power mode", otherwise UFS device shall not
> > do any flush if device enters Sleep or PowerDown power mode.
> >
> > Fix this by not changing device power mode if WriteBooster
> > flush is required in ufshcd_suspend().
> >
> > Signed-off-by: Stanley Chu
> > ---
> > drivers/scsi/ufs/ufs.h| 1 -
> > drivers/scsi/ufs/ufshcd.c | 39 +++
> > 2 files changed, 19 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> > index b3135344ab3f..9e4bc2e97ada 100644
> > --- a/drivers/scsi/ufs/ufs.h
> > +++ b/drivers/scsi/ufs/ufs.h
> > @@ -577,7 +577,6 @@ struct ufs_dev_info {
> > u32 d_ext_ufs_feature_sup;
> > u8 b_wb_buffer_type;
> > u32 d_wb_alloc_units;
> > - bool keep_vcc_on;
> > u8 b_presrv_uspc_en;
> > };
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 169a3379e468..2d0aff8ac260 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -8101,8 +8101,7 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
> > !hba->dev_info.is_lu_power_on_wp) {
> > ufshcd_setup_vreg(hba, false);
> > } else if (!ufshcd_is_ufs_dev_active(hba)) {
> > - if (!hba->dev_info.keep_vcc_on)
> > - ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
> > + ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
> > if (!ufshcd_is_link_active(hba)) {
> > ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq);
> > ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq2);
> > @@ -8172,6 +8171,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
> > ufs_pm_op pm_op)
> > enum ufs_pm_level pm_lvl;
> > enum ufs_dev_pwr_mode req_dev_pwr_mode;
> > enum uic_link_state req_link_state;
> > + bool keep_curr_dev_pwr_mode = false;
> >
> > hba->pm_op_in_progress = 1;
> > if (!ufshcd_is_shutdown_pm(pm_op)) {
> > @@ -8226,28 +8226,27 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
> > ufs_pm_op pm_op)
> > /* make sure that auto bkops is disabled */
> > ufshcd_disable_auto_bkops(hba);
> > }
> > +
> Unnecessary newline, perhaps?
Yap, I will remove it in next version.
> > /*
> > -* With wb enabled, if the bkops is enabled or if the
> > -* configured WB type is 70% full, keep vcc ON
> > -* for the device to flush the wb buffer
> > +* If device needs to do BKOP or WB buffer flush, keep device
> > +* power mode as "active power mode" and its VCC supply.
> > */
> > - if ((hba->auto_bkops_enabled && ufshcd_is_wb_allowed(hba)) ||
> > - ufshcd_wb_keep_vcc_on(hba))
> > - hba->dev_info.keep_vcc_on = true;
> > - else
> > - hba->dev_info.keep_vcc_on = false;
> > - } else {
> > - hba->dev_info.keep_vcc_on = false;
> > + keep_curr_dev_pwr_mode = hba->auto_bkops_enabled ||
> > + ufshcd_wb_keep_vcc_on(hba);
> Should the device be in UFS_ACTIVE_PWR_MODE to perform auto-bkops?
>
> Also, is it needed to keep the device in UFS_ACTIVE_PWR_MODE , if flush
> on hibern8 is enabled and the link is being put to hibern8 mode during
> runtime-suspend? Perhaps that should also be factored in here?
Both auto-bkops and WriteBooster flush during Hibern8 need device power
mode to be "Active Power Mode".
For auto-bkops, the spec mentions,
"If the background operations enable bit is set and the device is in
Active power mode or Idle power mode, then the device is allowed to
execute any internal operations."
For WriteBooster flush during Hibern8, the spec mentions,
"While the flushing operation is in progress, the device is in Active
power mode."
Therefore here we can use an unified "keep_curr_dev_pwr_mode" to
indicate the same requirements of above both features.
Besides, both operations may access flash array inside UFS device thus
VCC supply shall be also kept.
Before this patch, the original code will keep device power mode (stay
in Active Power Mode) if hba->auto_bkops_enabled is set as true during
runtime-suspend with UFSHCD_CAP_AUTO_BKOPS_SUSPEND capability is
enabled. This patch will not change this decision, just add
"WriteBooster flush during Hibern8" feature as another condition to do
so.
Thank you so much to remind me that "Link shall be put in
Re: [PATCH v1 4/4] scsi: ufs: Fix WriteBooster flush during runtime suspend
On 5/12/2020 3:47 AM, Stanley Chu wrote:
Currently UFS host driver promises VCC supply if UFS device
needs to do WriteBooster flush during runtime suspend.
However the UFS specification mentions,
"While the flushing operation is in progress, the device is
in Active power mode."
Therefore UFS host driver needs to promise more: Keep UFS
device as "Active power mode", otherwise UFS device shall not
do any flush if device enters Sleep or PowerDown power mode.
Fix this by not changing device power mode if WriteBooster
flush is required in ufshcd_suspend().
Signed-off-by: Stanley Chu
---
drivers/scsi/ufs/ufs.h| 1 -
drivers/scsi/ufs/ufshcd.c | 39 +++
2 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index b3135344ab3f..9e4bc2e97ada 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -577,7 +577,6 @@ struct ufs_dev_info {
u32 d_ext_ufs_feature_sup;
u8 b_wb_buffer_type;
u32 d_wb_alloc_units;
- bool keep_vcc_on;
u8 b_presrv_uspc_en;
};
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 169a3379e468..2d0aff8ac260 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8101,8 +8101,7 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
!hba->dev_info.is_lu_power_on_wp) {
ufshcd_setup_vreg(hba, false);
} else if (!ufshcd_is_ufs_dev_active(hba)) {
- if (!hba->dev_info.keep_vcc_on)
- ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
+ ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
if (!ufshcd_is_link_active(hba)) {
ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq);
ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq2);
@@ -8172,6 +8171,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
ufs_pm_op pm_op)
enum ufs_pm_level pm_lvl;
enum ufs_dev_pwr_mode req_dev_pwr_mode;
enum uic_link_state req_link_state;
+ bool keep_curr_dev_pwr_mode = false;
hba->pm_op_in_progress = 1;
if (!ufshcd_is_shutdown_pm(pm_op)) {
@@ -8226,28 +8226,27 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
ufs_pm_op pm_op)
/* make sure that auto bkops is disabled */
ufshcd_disable_auto_bkops(hba);
}
+
Unnecessary newline, perhaps?
/*
-* With wb enabled, if the bkops is enabled or if the
-* configured WB type is 70% full, keep vcc ON
-* for the device to flush the wb buffer
+* If device needs to do BKOP or WB buffer flush, keep device
+* power mode as "active power mode" and its VCC supply.
*/
- if ((hba->auto_bkops_enabled && ufshcd_is_wb_allowed(hba)) ||
- ufshcd_wb_keep_vcc_on(hba))
- hba->dev_info.keep_vcc_on = true;
- else
- hba->dev_info.keep_vcc_on = false;
- } else {
- hba->dev_info.keep_vcc_on = false;
+ keep_curr_dev_pwr_mode = hba->auto_bkops_enabled ||
+ ufshcd_wb_keep_vcc_on(hba);
Should the device be in UFS_ACTIVE_PWR_MODE to perform auto-bkops?
Also, is it needed to keep the device in UFS_ACTIVE_PWR_MODE , if flush
on hibern8 is enabled and the link is being put to hibern8 mode during
runtime-suspend? Perhaps that should also be factored in here?
}
- if ((req_dev_pwr_mode != hba->curr_dev_pwr_mode) &&
- ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) ||
- !ufshcd_is_runtime_pm(pm_op))) {
- /* ensure that bkops is disabled */
- ufshcd_disable_auto_bkops(hba);
- ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
- if (ret)
- goto enable_gating;
+ if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) {
+ if ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) ||
+ !ufshcd_is_runtime_pm(pm_op)) {
+ /* ensure that bkops is disabled */
+ ufshcd_disable_auto_bkops(hba);
+ }
+
+ if (!keep_curr_dev_pwr_mode) {
+ ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
Now, when the WB buffer is completely flushed out, the device should be
put back into UFS_SLEEP_PWR_MODE or UFS_POWERDOWN_PWR_MODE. Say, the
device buffer has to be flushed and during runtime-suspend, the device
is put to UFS_ACTIVE_PWR_MODE and Vcc is kept ON; the device doesn't
resume nor does the system enters suspend for a very long time, and with
AH8 and hibern8 disabled, there will be an unnecessary power drain for
that much time.
How about a periodic interval che
Re: [PATCH v1 4/4] scsi: ufs: Fix WriteBooster flush during runtime suspend
Hi Asutosh,
On Tue, 2020-05-12 at 10:04 -0700, Asutosh Das (asd) wrote:
> Hi Stanley,
>
> On 5/12/2020 3:47 AM, Stanley Chu wrote:
> > Currently UFS host driver promises VCC supply if UFS device
> > needs to do WriteBooster flush during runtime suspend.
> >
> > However the UFS specification mentions,
> >
> > "While the flushing operation is in progress, the device is
> > in Active power mode."
> >
> > Therefore UFS host driver needs to promise more: Keep UFS
> > device as "Active power mode", otherwise UFS device shall not
> > do any flush if device enters Sleep or PowerDown power mode.
> >
> > Fix this by not changing device power mode if WriteBooster
> > flush is required in ufshcd_suspend().
> >
> > Signed-off-by: Stanley Chu
> > ---
> > drivers/scsi/ufs/ufs.h| 1 -
> > drivers/scsi/ufs/ufshcd.c | 39 +++
> > 2 files changed, 19 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> > index b3135344ab3f..9e4bc2e97ada 100644
> > --- a/drivers/scsi/ufs/ufs.h
> > +++ b/drivers/scsi/ufs/ufs.h
> > @@ -577,7 +577,6 @@ struct ufs_dev_info {
> > u32 d_ext_ufs_feature_sup;
> > u8 b_wb_buffer_type;
> > u32 d_wb_alloc_units;
> > - bool keep_vcc_on;
> > u8 b_presrv_uspc_en;
> > };
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 169a3379e468..2d0aff8ac260 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -8101,8 +8101,7 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
> > !hba->dev_info.is_lu_power_on_wp) {
> > ufshcd_setup_vreg(hba, false);
> > } else if (!ufshcd_is_ufs_dev_active(hba)) {
> > - if (!hba->dev_info.keep_vcc_on)
> > - ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
> > + ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
> > if (!ufshcd_is_link_active(hba)) {
> > ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq);
> > ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq2);
> > @@ -8172,6 +8171,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
> > ufs_pm_op pm_op)
> > enum ufs_pm_level pm_lvl;
> > enum ufs_dev_pwr_mode req_dev_pwr_mode;
> > enum uic_link_state req_link_state;
> > + bool keep_curr_dev_pwr_mode = false;
> >
> > hba->pm_op_in_progress = 1;
> > if (!ufshcd_is_shutdown_pm(pm_op)) {
> > @@ -8226,28 +8226,27 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
> > ufs_pm_op pm_op)
> > /* make sure that auto bkops is disabled */
> > ufshcd_disable_auto_bkops(hba);
> > }
> > +
> > /*
> > -* With wb enabled, if the bkops is enabled or if the
> > -* configured WB type is 70% full, keep vcc ON
> > -* for the device to flush the wb buffer
> > +* If device needs to do BKOP or WB buffer flush, keep device
> > +* power mode as "active power mode" and its VCC supply.
> > */
> > - if ((hba->auto_bkops_enabled && ufshcd_is_wb_allowed(hba)) ||
> > - ufshcd_wb_keep_vcc_on(hba))
> > - hba->dev_info.keep_vcc_on = true;
> > - else
> > - hba->dev_info.keep_vcc_on = false;
> > - } else {
> > - hba->dev_info.keep_vcc_on = false;
> > + keep_curr_dev_pwr_mode = hba->auto_bkops_enabled ||
> > + ufshcd_wb_keep_vcc_on(hba);
> > }
> >
> > - if ((req_dev_pwr_mode != hba->curr_dev_pwr_mode) &&
> > - ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) ||
> > - !ufshcd_is_runtime_pm(pm_op))) {
> > - /* ensure that bkops is disabled */
> > - ufshcd_disable_auto_bkops(hba);
> > - ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
> > - if (ret)
> > - goto enable_gating;
> > + if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) {
> > + if ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) ||
> > + !ufshcd_is_runtime_pm(pm_op)) {
> > + /* ensure that bkops is disabled */
> > + ufshcd_disable_auto_bkops(hba);
> > + }
> > +
> > + if (!keep_curr_dev_pwr_mode) {
> > + ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
> > + if (ret)
> > + goto enable_gating;
> > + }
> > }
> >
> > flush_work(&hba->eeh_work);
> >
>
> Can you please confirm that you've tested and found that with the
> previous code, the flush operation in the device was not happening.
>
> If so, please can you let me know the test-case that you ran to figure
> this out.
>
> I'd like to verify this at my end.
Sorry currently I have no easy test cases or scripts available.
To precisely confirm the flush beh
Re: [PATCH v1 4/4] scsi: ufs: Fix WriteBooster flush during runtime suspend
Hi Stanley,
On 5/12/2020 3:47 AM, Stanley Chu wrote:
Currently UFS host driver promises VCC supply if UFS device
needs to do WriteBooster flush during runtime suspend.
However the UFS specification mentions,
"While the flushing operation is in progress, the device is
in Active power mode."
Therefore UFS host driver needs to promise more: Keep UFS
device as "Active power mode", otherwise UFS device shall not
do any flush if device enters Sleep or PowerDown power mode.
Fix this by not changing device power mode if WriteBooster
flush is required in ufshcd_suspend().
Signed-off-by: Stanley Chu
---
drivers/scsi/ufs/ufs.h| 1 -
drivers/scsi/ufs/ufshcd.c | 39 +++
2 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index b3135344ab3f..9e4bc2e97ada 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -577,7 +577,6 @@ struct ufs_dev_info {
u32 d_ext_ufs_feature_sup;
u8 b_wb_buffer_type;
u32 d_wb_alloc_units;
- bool keep_vcc_on;
u8 b_presrv_uspc_en;
};
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 169a3379e468..2d0aff8ac260 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8101,8 +8101,7 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
!hba->dev_info.is_lu_power_on_wp) {
ufshcd_setup_vreg(hba, false);
} else if (!ufshcd_is_ufs_dev_active(hba)) {
- if (!hba->dev_info.keep_vcc_on)
- ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
+ ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
if (!ufshcd_is_link_active(hba)) {
ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq);
ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq2);
@@ -8172,6 +8171,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
ufs_pm_op pm_op)
enum ufs_pm_level pm_lvl;
enum ufs_dev_pwr_mode req_dev_pwr_mode;
enum uic_link_state req_link_state;
+ bool keep_curr_dev_pwr_mode = false;
hba->pm_op_in_progress = 1;
if (!ufshcd_is_shutdown_pm(pm_op)) {
@@ -8226,28 +8226,27 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
ufs_pm_op pm_op)
/* make sure that auto bkops is disabled */
ufshcd_disable_auto_bkops(hba);
}
+
/*
-* With wb enabled, if the bkops is enabled or if the
-* configured WB type is 70% full, keep vcc ON
-* for the device to flush the wb buffer
+* If device needs to do BKOP or WB buffer flush, keep device
+* power mode as "active power mode" and its VCC supply.
*/
- if ((hba->auto_bkops_enabled && ufshcd_is_wb_allowed(hba)) ||
- ufshcd_wb_keep_vcc_on(hba))
- hba->dev_info.keep_vcc_on = true;
- else
- hba->dev_info.keep_vcc_on = false;
- } else {
- hba->dev_info.keep_vcc_on = false;
+ keep_curr_dev_pwr_mode = hba->auto_bkops_enabled ||
+ ufshcd_wb_keep_vcc_on(hba);
}
- if ((req_dev_pwr_mode != hba->curr_dev_pwr_mode) &&
- ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) ||
- !ufshcd_is_runtime_pm(pm_op))) {
- /* ensure that bkops is disabled */
- ufshcd_disable_auto_bkops(hba);
- ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
- if (ret)
- goto enable_gating;
+ if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) {
+ if ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) ||
+ !ufshcd_is_runtime_pm(pm_op)) {
+ /* ensure that bkops is disabled */
+ ufshcd_disable_auto_bkops(hba);
+ }
+
+ if (!keep_curr_dev_pwr_mode) {
+ ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
+ if (ret)
+ goto enable_gating;
+ }
}
flush_work(&hba->eeh_work);
Can you please confirm that you've tested and found that with the
previous code, the flush operation in the device was not happening.
If so, please can you let me know the test-case that you ran to figure
this out.
I'd like to verify this at my end.
--
Thanks,
-asd
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project
[PATCH v1 4/4] scsi: ufs: Fix WriteBooster flush during runtime suspend
Currently UFS host driver promises VCC supply if UFS device
needs to do WriteBooster flush during runtime suspend.
However the UFS specification mentions,
"While the flushing operation is in progress, the device is
in Active power mode."
Therefore UFS host driver needs to promise more: Keep UFS
device as "Active power mode", otherwise UFS device shall not
do any flush if device enters Sleep or PowerDown power mode.
Fix this by not changing device power mode if WriteBooster
flush is required in ufshcd_suspend().
Signed-off-by: Stanley Chu
---
drivers/scsi/ufs/ufs.h| 1 -
drivers/scsi/ufs/ufshcd.c | 39 +++
2 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index b3135344ab3f..9e4bc2e97ada 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -577,7 +577,6 @@ struct ufs_dev_info {
u32 d_ext_ufs_feature_sup;
u8 b_wb_buffer_type;
u32 d_wb_alloc_units;
- bool keep_vcc_on;
u8 b_presrv_uspc_en;
};
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 169a3379e468..2d0aff8ac260 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8101,8 +8101,7 @@ static void ufshcd_vreg_set_lpm(struct ufs_hba *hba)
!hba->dev_info.is_lu_power_on_wp) {
ufshcd_setup_vreg(hba, false);
} else if (!ufshcd_is_ufs_dev_active(hba)) {
- if (!hba->dev_info.keep_vcc_on)
- ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
+ ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
if (!ufshcd_is_link_active(hba)) {
ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq);
ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq2);
@@ -8172,6 +8171,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
ufs_pm_op pm_op)
enum ufs_pm_level pm_lvl;
enum ufs_dev_pwr_mode req_dev_pwr_mode;
enum uic_link_state req_link_state;
+ bool keep_curr_dev_pwr_mode = false;
hba->pm_op_in_progress = 1;
if (!ufshcd_is_shutdown_pm(pm_op)) {
@@ -8226,28 +8226,27 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum
ufs_pm_op pm_op)
/* make sure that auto bkops is disabled */
ufshcd_disable_auto_bkops(hba);
}
+
/*
-* With wb enabled, if the bkops is enabled or if the
-* configured WB type is 70% full, keep vcc ON
-* for the device to flush the wb buffer
+* If device needs to do BKOP or WB buffer flush, keep device
+* power mode as "active power mode" and its VCC supply.
*/
- if ((hba->auto_bkops_enabled && ufshcd_is_wb_allowed(hba)) ||
- ufshcd_wb_keep_vcc_on(hba))
- hba->dev_info.keep_vcc_on = true;
- else
- hba->dev_info.keep_vcc_on = false;
- } else {
- hba->dev_info.keep_vcc_on = false;
+ keep_curr_dev_pwr_mode = hba->auto_bkops_enabled ||
+ ufshcd_wb_keep_vcc_on(hba);
}
- if ((req_dev_pwr_mode != hba->curr_dev_pwr_mode) &&
- ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) ||
- !ufshcd_is_runtime_pm(pm_op))) {
- /* ensure that bkops is disabled */
- ufshcd_disable_auto_bkops(hba);
- ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
- if (ret)
- goto enable_gating;
+ if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) {
+ if ((ufshcd_is_runtime_pm(pm_op) && !hba->auto_bkops_enabled) ||
+ !ufshcd_is_runtime_pm(pm_op)) {
+ /* ensure that bkops is disabled */
+ ufshcd_disable_auto_bkops(hba);
+ }
+
+ if (!keep_curr_dev_pwr_mode) {
+ ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
+ if (ret)
+ goto enable_gating;
+ }
}
flush_work(&hba->eeh_work);
--
2.18.0
