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: <bdd72ef290b3c9d5492255358686a39a7aa3cde6.camel@iokpp.de>
Subject: Re: [PATCH v2 1/2] scsi: ufs: core: Remove unnecessary if statement
From:   Bean Huo <beanhuo@iokpp.de>
To:     daejun7.park@samsung.com, ALIM AKHTAR <alim.akhtar@samsung.com>,
        "avri.altman@wdc.com" <avri.altman@wdc.com>,
        "asutoshd@codeaurora.org" <asutoshd@codeaurora.org>,
        "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>,
        "cang@codeaurora.org" <cang@codeaurora.org>
Cc:     "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Date:   Tue, 11 Oct 2022 11:39:27 +0200
In-Reply-To: <20221011022153epcms2p1fcbf889099bd74279ed0d9b51ff26c1b@epcms2p1>
References: <20221010092937.520013-2-beanhuo@iokpp.de>
         <20221010092937.520013-1-beanhuo@iokpp.de>
         <CGME20221010093042epcas2p2462d3c50d81d613574332e3f39d971e1@epcms2p1>
         <20221011022153epcms2p1fcbf889099bd74279ed0d9b51ff26c1b@epcms2p1>
Organization: IOKPP
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu1 
MIME-Version: 1.0
Precedence: bulk

On Tue, 2022-10-11 at 11:21 +0900, Daejun Park wrote:
> Hi Bean=C2=A0Huo,
>=20
> I think ufs_is_valid_unit_desc_lun() is also used for
> wb_buf_alloc_units_show() in ufs-sysfs.c.
> So just removing this if-checkup=C2=A0will make different result when
> check lun value.
>=20

Hi Daejun,

Thanks for your review on the patch. Yes, I understood what you mean.
But I don't think that's the problem. Without this patch, access on
sysfs node "wb_shared_alloc_units" would get "invalid argument", while
with this patch sysfs would return 00. According to the UFS
specification:

"If this value is =E2=80=980=E2=80=99, then the WriteBooster is not support=
ed for this
LU. The Logical unit among LU0 ~ LU7 can be configured for WriteBooster
Buffer. Otherwise, whole WriteBooster Buffer configuration in this
device is invalid."

Per my understanding,  with this patch,  there is still no miss-
explanation of this sysfs node. The key purpose of this patch is to
remove any nonsense logical during the booting stage.

please have a think my comments. thanks.


Kind regards,
Bean


> Thanks,
> Daejun
>=20
> > From:=C2=A0Bean=C2=A0Huo=C2=A0<beanhuo@micron.com>
> >=20
> > LUs=C2=A0with=C2=A0WB=C2=A0potential=C2=A0support=C2=A0are=C2=A0properl=
y=C2=A0checked=C2=A0in=C2=A0ufshcd_wb_pro
> > be()
> > before=C2=A0calling=C2=A0ufshcd_read_unit_desc_param(),=C2=A0so=C2=A0re=
move=C2=A0this=C2=A0unnece
> > ssary
> > if-
> > checkup=C2=A0in=C2=A0ufs_is_valid_unit_desc_lun()=C2=A0to=C2=A0match=C2=
=A0its=C2=A0function=C2=A0defin
> > ition.