RE: [PATCH v4 6/9] scsi: ufshpb: Add hpb dev reset response

2021-03-02 Thread Avri Altman
> 
> Hi Avri,
> 
> > diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
> > index cf704b82e72a..f33aa28e0a0a 100644
> > --- a/drivers/scsi/ufs/ufshpb.c
> > +++ b/drivers/scsi/ufs/ufshpb.c
> > @@ -642,7 +642,8 @@ int ufshpb_prep(struct ufs_hba *hba, struct
> ufshcd_lrb *lrbp)
> >  if (rgn->reads == ACTIVATION_THRESHOLD)
> >  activate = true;
> >  spin_unlock_irqrestore(>rgn_lock, flags);
> > -if (activate) {
> > +if (activate ||
> > +test_and_clear_bit(RGN_FLAG_UPDATE, >rgn_flags)) {
> 
> How about merge rgn->rgn_flags to rgn_state?
This will actually not work, because a region can be e.g. active/inactive and 
dirty,
And I don't want to mess with the regions state machine.

Thanks,
Avri

> 
> Thanks,
> Daejun


RE: [PATCH v4 6/9] scsi: ufshpb: Add hpb dev reset response

2021-03-02 Thread Avri Altman
> Hi Avri,
> 
> > diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
> > index cf704b82e72a..f33aa28e0a0a 100644
> > --- a/drivers/scsi/ufs/ufshpb.c
> > +++ b/drivers/scsi/ufs/ufshpb.c
> > @@ -642,7 +642,8 @@ int ufshpb_prep(struct ufs_hba *hba, struct
> ufshcd_lrb *lrbp)
> >  if (rgn->reads == ACTIVATION_THRESHOLD)
> >  activate = true;
> >  spin_unlock_irqrestore(>rgn_lock, flags);
> > -if (activate) {
> > +if (activate ||
> > +test_and_clear_bit(RGN_FLAG_UPDATE, >rgn_flags)) {
> 
> How about merge rgn->rgn_flags to rgn_state?
Done.

Thanks,
Avri

> 
> Thanks,
> Daejun


RE: [PATCH v4 6/9] scsi: ufshpb: Add hpb dev reset response

2021-03-02 Thread Daejun Park
Hi Avri,

> diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
> index cf704b82e72a..f33aa28e0a0a 100644
> --- a/drivers/scsi/ufs/ufshpb.c
> +++ b/drivers/scsi/ufs/ufshpb.c
> @@ -642,7 +642,8 @@ int ufshpb_prep(struct ufs_hba *hba, struct ufshcd_lrb 
> *lrbp)
>  if (rgn->reads == ACTIVATION_THRESHOLD)
>  activate = true;
>  spin_unlock_irqrestore(>rgn_lock, flags);
> -if (activate) {
> +if (activate ||
> +test_and_clear_bit(RGN_FLAG_UPDATE, >rgn_flags)) {

How about merge rgn->rgn_flags to rgn_state?

Thanks,
Daejun