Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1eda1c55fdf8292c2912c6d0adb741d8dd7f0a20.camel@iokpp.de>
Subject: Re: [RFC PATCH v1 1/2] ufs: core: Advanced RPMB detection
From:   Bean Huo <beanhuo@iokpp.de>
To:     Avri Altman <Avri.Altman@wdc.com>,
        "alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
        "jejb@linux.ibm.com" <jejb@linux.ibm.com>,
        "martin.petersen@oracle.com" <martin.petersen@oracle.com>,
        "stanley.chu@mediatek.com" <stanley.chu@mediatek.com>,
        "beanhuo@micron.com" <beanhuo@micron.com>,
        "bvanassche@acm.org" <bvanassche@acm.org>,
        "tomas.winkler@intel.com" <tomas.winkler@intel.com>,
        "daejun7.park@samsung.com" <daejun7.park@samsung.com>,
        "quic_cang@quicinc.com" <quic_cang@quicinc.com>,
        "quic_nguyenb@quicinc.com" <quic_nguyenb@quicinc.com>,
        "quic_xiaosenh@quicinc.com" <quic_xiaosenh@quicinc.com>,
        "quic_richardp@quicinc.com" <quic_richardp@quicinc.com>,
        "quic_asutoshd@quicinc.com" <quic_asutoshd@quicinc.com>,
        "hare@suse.de" <hare@suse.de>
Cc:     "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Date:   Tue, 08 Nov 2022 17:50:06 +0100
In-Reply-To: <DM6PR04MB6575145B168BB80F3D2910A7FC3F9@DM6PR04MB6575.namprd04.prod.outlook.com>
References: <20221107131038.201724-1-beanhuo@iokpp.de>
         <20221107131038.201724-2-beanhuo@iokpp.de>
         <DM6PR04MB6575145B168BB80F3D2910A7FC3F9@DM6PR04MB6575.namprd04.prod.outlook.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5-0ubuntu1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

Avri, 

thanks for your review.

On Tue, 2022-11-08 at 13:40 +0000, Avri Altman wrote:
> > From: Bean Huo <beanhuo@micron.com>
> > 
> > Check UFS Advanced RPMB LU enablement during ufshcd_lu_init().
> > 
> > Signed-off-by: Bean Huo <beanhuo@micron.com>
> > ---
> >  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