mbox series

[v2,0/2] scsi: ufs: Re-enable WB after device reset

Message ID 20201208135635.15326-1-stanley.chu@mediatek.com
Headers show
Series scsi: ufs: Re-enable WB after device reset | expand

Message

Stanley Chu Dec. 8, 2020, 1:56 p.m. UTC 
Hi,

This series fixes up an issue that WB is not re-enabled after device reset.

Stanley Chu (2):
  scsi: ufs: Re-enable WriteBooster after device reset
  scsi: ufs: Uninline ufshcd_vops_device_reset function

 drivers/scsi/ufs/ufshcd.c | 27 ++++++++++++++++++++++-----
 drivers/scsi/ufs/ufshcd.h | 14 +++++---------
 2 files changed, 27 insertions(+), 14 deletions(-)

Comments

Bean Huo Dec. 8, 2020, 2:13 p.m. UTC | #1 
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>
Stanley Chu Dec. 9, 2020, 1:09 a.m. UTC | #2 
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>

>
Martin K. Petersen Dec. 9, 2020, 5:12 p.m. UTC | #3 
Stanley,

> This series fixes up an issue that WB is not re-enabled after device

> reset.


Applied to 5.11/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering
Bean Huo Dec. 14, 2020, 8:32 p.m. UTC | #4 
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