Message ID | 20201208135635.15326-2-stanley.chu@mediatek.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | scsi: ufs: Re-enable WB after device reset | expand |
On Tue, 2020-12-08 at 21:56 +0800, Stanley Chu wrote: > index 08c8a591e6b0..36d367eb8139 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -1221,8 +1221,13 @@ static inline void > ufshcd_vops_device_reset(struct ufs_hba *hba) > if (hba->vops && hba->vops->device_reset) { > int err = hba->vops->device_reset(hba); > > - if (!err) > + if (!err) { > ufshcd_set_ufs_dev_active(hba); > + if (ufshcd_is_wb_allowed(hba)) { > + hba->wb_enabled = false; > + hba->wb_buf_flush_enabled = false; > + } > + } Stanley, how do you think group wb_buf_flush_enabled and wb_enabled to the dev_info, since they are UFS device attributes. means they are set only when UFS device flags being set. Reviewed-by: Bean Huo <beanhuo@micron.com>
On Tue, 2020-12-08 at 15:13 +0100, Bean Huo wrote: > On Tue, 2020-12-08 at 21:56 +0800, Stanley Chu wrote: > > index 08c8a591e6b0..36d367eb8139 100644 > > --- a/drivers/scsi/ufs/ufshcd.h > > +++ b/drivers/scsi/ufs/ufshcd.h > > @@ -1221,8 +1221,13 @@ static inline void > > ufshcd_vops_device_reset(struct ufs_hba *hba) > > if (hba->vops && hba->vops->device_reset) { > > int err = hba->vops->device_reset(hba); > > > > - if (!err) > > + if (!err) { > > ufshcd_set_ufs_dev_active(hba); > > + if (ufshcd_is_wb_allowed(hba)) { > > + hba->wb_enabled = false; > > + hba->wb_buf_flush_enabled = false; > > + } > > + } > > Stanley, > how do you think group wb_buf_flush_enabled and wb_enabled to the > dev_info, since they are UFS device attributes. means they are set only > when UFS device flags being set. Hi Bean, Thanks for your review. Yes, I agreed that wb related variables is a mess currently. I would like to clean them up once I have time. Feel free to post your patch if you want to take it up : ) Thanks, Stanley Chu > > Reviewed-by: Bean Huo <beanhuo@micron.com> >
On Wed, 2020-12-09 at 09:09 +0800, Stanley Chu wrote: > > > > > > - if (!err) > > > + if (!err) { > > > ufshcd_set_ufs_dev_active(hba); > > > + if (ufshcd_is_wb_allowed(hba)) { > > > + hba->wb_enabled = false; > > > + hba->wb_buf_flush_enabled = > > > false; > > > + } > > > + } > > > > Stanley, > > how do you think group wb_buf_flush_enabled and wb_enabled to the > > dev_info, since they are UFS device attributes. means they are set > > only > > when UFS device flags being set. > > Hi Bean, > > Thanks for your review. > > Yes, I agreed that wb related variables is a mess currently. I would > like to clean them up once I have time. Feel free to post your patch > if > you want to take it up : ) > Hi Stanley I updated this change in my new "Several changes for UFS WriteBooster" series patch, are you interested in reviewing that? to help Martin easier pick up the changes. Thanks, Bean
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 08c8a591e6b0..36d367eb8139 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -1221,8 +1221,13 @@ static inline void ufshcd_vops_device_reset(struct ufs_hba *hba) if (hba->vops && hba->vops->device_reset) { int err = hba->vops->device_reset(hba); - if (!err) + if (!err) { ufshcd_set_ufs_dev_active(hba); + if (ufshcd_is_wb_allowed(hba)) { + hba->wb_enabled = false; + hba->wb_buf_flush_enabled = false; + } + } if (err != -EOPNOTSUPP) ufshcd_update_evt_hist(hba, UFS_EVT_DEV_RESET, err); }
UFS 3.1 specification mentions that below WriteBooster flags will be set to default value, say disabled, after power cycle or any type of reset event. Thus we need to reset those flag variables kept in struct hba to align the device status and ensure WriteBooster related functions being configured properly after device reset. Without this fix, WriteBooster will not be enabled successfully after by ufshcd_wb_ctrl() after device reset because hba->wb_enabled remains true. Flags required to be reset to default values: - fWriteBoosterEn: hba->wb_enabled - fWriteBoosterBufferFlushEn: hba->wb_buf_flush_enabled - fWriteBoosterBufferFlushDuringHibernate: No variable mapped Fixes: 3d17b9b5ab11 ("scsi: ufs: Add write booster feature support") Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> --- drivers/scsi/ufs/ufshcd.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)