Avri,
thanks for your review.
On Tue, 2022-11-08 at 13:40 +0000, Avri Altman wrote:
> > From: Bean Huo <beanhuo@xxxxxxxxxx>
> >
> > Check UFS Advanced RPMB LU enablement during ufshcd_lu_init().
> >
> > Signed-off-by: Bean Huo <beanhuo@xxxxxxxxxx>
> > ---
> > drivers/ufs/core/ufshcd.c | 4 ++++
> > include/ufs/ufs.h | 3 +++
> > 2 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index
> > ee73d7036133..d49e7a0b82ca 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -4940,6 +4940,10 @@ static void ufshcd_lu_init(struct ufs_hba
> > *hba,
> > struct scsi_device *sdev)
> > desc_buf[UNIT_DESC_PARAM_LU_WR_PROTECT] ==
> > UFS_LU_POWER_ON_WP)
> > hba->dev_info.is_lu_power_on_wp = true;
> >
> > + if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_RPMB_UNIT
> > &&
> Please remind me why do we need both UFS_RPMB_UNIT and
> UFS_UPIU_RPMB_WLUN ?
I see. they are the same value, we should remove one, will change it in
next version.
>
> > + desc_buf[UNIT_DESC_PARAM_RPMB_REGION_EN] & 1 << 4)
> (1 << 4) or BIT(4) ?
>
> > + hba->dev_info.b_advanced_rpmb_en = true;
> > +
> > kfree(desc_buf);
> > set_qdepth:
> > /*
> > diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h index
> > 1bba3fead2ce..2e617ab87750 100644
> > --- a/include/ufs/ufs.h
> > +++ b/include/ufs/ufs.h
> > @@ -199,6 +199,7 @@ enum unit_desc_param {
> > UNIT_DESC_PARAM_PSA_SENSITIVE = 0x7,
> > UNIT_DESC_PARAM_MEM_TYPE = 0x8,
> > UNIT_DESC_PARAM_DATA_RELIABILITY = 0x9,
> > + UNIT_DESC_PARAM_RPMB_REGION_EN = 0x9,
> This is awkward. Better to define it, or -
> Maybe it's time for rpmb to have its own unit descriptor - it surely
> deserve it.
>
no problem, let me think about it, will add in the next version.
Kind regards,
Bean
