* [PATCH] scsi: ufs: Enable WB flush during suspend only if WB is enabled @ 2020-12-07 5:50 Stanley Chu 2020-12-07 10:59 ` Bean Huo 0 siblings, 1 reply; 3+ messages in thread From: Stanley Chu @ 2020-12-07 5:50 UTC (permalink / raw) To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb Cc: beanhuo, asutoshd, cang, matthias.bgg, bvanassche, linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang, chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao, alice.chao, Stanley Chu WriteBootser flush during suspend is not necessary to be enabled if WriteBooster feature is disabled. Simply adding a check to prevent unexpected power drain. Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> --- drivers/scsi/ufs/ufshcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 4879e87577e1..89fa8b9ac11d 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -5458,7 +5458,7 @@ static bool ufshcd_wb_need_flush(struct ufs_hba *hba) u32 avail_buf; u8 index; - if (!ufshcd_is_wb_allowed(hba)) + if (!ufshcd_is_wb_allowed(hba) || !hba->wb_enabled) return false; /* * The ufs device needs the vcc to be ON to flush. -- 2.18.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] scsi: ufs: Enable WB flush during suspend only if WB is enabled 2020-12-07 5:50 [PATCH] scsi: ufs: Enable WB flush during suspend only if WB is enabled Stanley Chu @ 2020-12-07 10:59 ` Bean Huo 2020-12-07 13:43 ` Stanley Chu 0 siblings, 1 reply; 3+ messages in thread From: Bean Huo @ 2020-12-07 10:59 UTC (permalink / raw) To: Stanley Chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb Cc: beanhuo, asutoshd, cang, matthias.bgg, bvanassche, linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang, chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao, alice.chao On Mon, 2020-12-07 at 13:50 +0800, Stanley Chu wrote: > WriteBootser flush during suspend is not necessary to be enabled if > WriteBooster feature is disabled. Simply adding a check to prevent > unexpected power drain. > > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> > --- > drivers/scsi/ufs/ufshcd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 4879e87577e1..89fa8b9ac11d 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -5458,7 +5458,7 @@ static bool ufshcd_wb_need_flush(struct ufs_hba > *hba) > u32 avail_buf; > u8 index; > > - if (!ufshcd_is_wb_allowed(hba)) > + if (!ufshcd_is_wb_allowed(hba) || !hba->wb_enabled) > return false; > /* > * The ufs device needs the vcc to be ON to flush. Hi Stanley In the 3.1 Spec: "If the fWriteBoosterEn flag is set to zero, data written to any logical unit is written in normal storage. If the fWriteBoosterEn flag is set to one and the device is configured in “shared buffer” mode, data written to any logical unit is written in the shared WriteBooster Buffer." so, IMO, fWriteBoosterEn is independant with WB buffer flush. as for the flush: "There are two methods for flushing data from the WriteBooster Buffer to the normal storage: one is using an explicit flush command, the other enabling the flushing during link hibernate state. If the fWriteBoosterBufferFlushEn flag is set to one, the device shall flush the data stored in the WriteBooster Buffer to the normal storage. If fWriteBoosterBufferFlushDuringHibernate is set to one, the device flushes the WriteBooster Buffer data automatically whenever the link enters the hibernate (HIBERN8) state." IMO, for the flush, it is controlled by fWriteBoosterBufferFlushEn and fWriteBoosterBufferFlushDuringHibernate. how do you understand the above two paragraphs from Spec? thanks, Bean ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] scsi: ufs: Enable WB flush during suspend only if WB is enabled 2020-12-07 10:59 ` Bean Huo @ 2020-12-07 13:43 ` Stanley Chu 0 siblings, 0 replies; 3+ messages in thread From: Stanley Chu @ 2020-12-07 13:43 UTC (permalink / raw) To: Bean Huo Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb, beanhuo, asutoshd, cang, matthias.bgg, bvanassche, linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang, chun-hung.wu, andy.teng, chaotian.jing, cc.chou, jiajie.hao, alice.chao Hi Bean, On Mon, 2020-12-07 at 11:59 +0100, Bean Huo wrote: > On Mon, 2020-12-07 at 13:50 +0800, Stanley Chu wrote: > > WriteBootser flush during suspend is not necessary to be enabled if > > WriteBooster feature is disabled. Simply adding a check to prevent > > unexpected power drain. > > > > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> > > --- > > drivers/scsi/ufs/ufshcd.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index 4879e87577e1..89fa8b9ac11d 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -5458,7 +5458,7 @@ static bool ufshcd_wb_need_flush(struct ufs_hba > > *hba) > > u32 avail_buf; > > u8 index; > > > > - if (!ufshcd_is_wb_allowed(hba)) > > + if (!ufshcd_is_wb_allowed(hba) || !hba->wb_enabled) > > return false; > > /* > > * The ufs device needs the vcc to be ON to flush. > > > Hi Stanley > > In the 3.1 Spec: > > "If the fWriteBoosterEn flag is set to zero, data written to any > logical unit is written in normal storage. > If the fWriteBoosterEn flag is set to one and the device is configured > in “shared buffer” mode, data written to any logical unit is written in > the shared WriteBooster Buffer." > > so, IMO, fWriteBoosterEn is independant with WB buffer flush. > > as for the flush: > > "There are two methods for flushing data from the WriteBooster Buffer > to the normal storage: one is using an explicit flush command, the > other enabling the flushing during link hibernate state. If the > fWriteBoosterBufferFlushEn flag is set to one, the device shall flush > the data stored in the WriteBooster Buffer to the normal storage. If > fWriteBoosterBufferFlushDuringHibernate is set to one, the device > flushes the WriteBooster Buffer data automatically whenever the link > enters the hibernate (HIBERN8) state." > > IMO, for the flush, it is controlled by fWriteBoosterBufferFlushEn and > fWriteBoosterBufferFlushDuringHibernate. > > how do you understand the above two paragraphs from Spec? Thanks for your review and feedback! : ) Actually this patch is not motivated by any limitation in spec. I was thinking that if host disables WriteBooster, which may imply that host does not want any WB related operations in device during the disabled period. However I may be wrong because host may only want not consuming any WB buffer in device during the disabled time, but still want the "flush" operation in device to clean WB buffer as quickly as possible to fulfill future high-throughput requirement after WB is re-enabled. So, I would drop this patch. Thanks, Stanley Chu > > > thanks, > Bean > > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-07 13:44 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-07 5:50 [PATCH] scsi: ufs: Enable WB flush during suspend only if WB is enabled Stanley Chu 2020-12-07 10:59 ` Bean Huo 2020-12-07 13:43 ` Stanley Chu
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).