From: Stanley Chu <stanley.chu@mediatek.com>
To: Avri Altman <Avri.Altman@wdc.com>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
"alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
"jejb@linux.ibm.com" <jejb@linux.ibm.com>,
"asutoshd@codeaurora.org" <asutoshd@codeaurora.org>,
"bvanassche@acm.org" <bvanassche@acm.org>,
"andy.teng@mediatek.com" <andy.teng@mediatek.com>,
"chun-hung.wu@mediatek.com" <chun-hung.wu@mediatek.com>,
"kuohong.wang@mediatek.com" <kuohong.wang@mediatek.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"cang@codeaurora.org" <cang@codeaurora.org>,
"linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"peter.wang@mediatek.com" <peter.wang@mediatek.com>,
"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"beanhuo@micron.com" <beanhuo@micron.com>
Subject: RE: [PATCH v4 6/8] scsi: ufs: add LU Dedicated buffer mode support for WriteBooster
Date: Sun, 3 May 2020 18:39:49 +0800 [thread overview]
Message-ID: <1588502389.3197.19.camel@mtkswgap22> (raw)
In-Reply-To: <SN6PR04MB4640977062BF81B6DE7CBE51FCA90@SN6PR04MB4640.namprd04.prod.outlook.com>
Hi Avri,
On Sun, 2020-05-03 at 08:00 +0000, Avri Altman wrote:
>
> > - if (!(hba->dev_info.b_wb_buffer_type &&
> > - hba->dev_info.d_wb_alloc_units))
> > - goto wb_disabled;
> > + if (hba->dev_info.b_wb_buffer_type == WB_BUF_MODE_SHARED) {
> > + hba->dev_info.d_wb_alloc_units =
> > + get_unaligned_be32(desc_buf +
> > + DEVICE_DESC_PARAM_WB_SHARED_ALLOC_UNITS);
> > + if (!hba->dev_info.d_wb_alloc_units)
> > + goto wb_disabled;
> > + } else {
> > + for (lun = 0; lun < hba->dev_info.max_lu_supported; lun++) {
> max_lu_supported is determined according to bMaxNumberLU in the geometry descriptor,
> which can be 32. WB buffer however, is only valid only for LU 0, ..., LU7.
> Better to add this new limit to ufs.h.
Thanks for noticing this. I will fix it in next version.
>
> > + ret = ufshcd_read_unit_desc_param(hba,
> > + lun,
> > + UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS,
> > + (u8 *)&d_lu_wb_buf_alloc,
> > + sizeof(d_lu_wb_buf_alloc));
> > + if (ret)
> > + goto wb_disabled;
> I think you should just continue here, as it is ok for the query to fail.
> The spec says:
> The WriteBooster Buffer is available only for the logical units from 0 to 7 which are configured as
> "normal memory type" (bMemoryType = 00h) and "not Boot well known logical unit" (bBootLunID =
> 00h), otherwise the Query Request shall fail and the Query Response field shall be set to "General
> Failure".
>
> Sorry for not noticing this earlier.
Always appreciate your review and always not being too late : )
The spec does not mention clearly that the Query Request shall fail for
Read or Write. Although I think it shall fail for Write only, i.e., fail
during configuration only, it is better to let fail being skipped and
check d_lu_wb_buf_alloc only anyway.
I will fix this as well in next version.
Thanks,
Stanley Chu
>
> Thanks,
> Avri
> > + if (d_lu_wb_buf_alloc) {
> > + hba->dev_info.wb_dedicated_lu = lun;
> > + break;
> > + }
> > + }
> >
> > + if (!d_lu_wb_buf_alloc)
> > + goto wb_disabled;
> > + }
> > return;
> >
> > wb_disabled:
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index 898883058e3a..f232a67fd9b3 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -861,6 +861,13 @@ static inline bool
> > ufshcd_keep_autobkops_enabled_except_suspend(
> > return hba->caps &
> > UFSHCD_CAP_KEEP_AUTO_BKOPS_ENABLED_EXCEPT_SUSPEND;
> > }
> >
> > +static inline u8 ufshcd_wb_get_flag_index(struct ufs_hba *hba)
> > +{
> > + if (hba->dev_info.b_wb_buffer_type ==
> > WB_BUF_MODE_LU_DEDICATED)
> > + return hba->dev_info.wb_dedicated_lu;
> > + return 0;
> > +}
> > +
> > extern int ufshcd_runtime_suspend(struct ufs_hba *hba);
> > extern int ufshcd_runtime_resume(struct ufs_hba *hba);
> > extern int ufshcd_runtime_idle(struct ufs_hba *hba);
> > --
> > 2.18.0
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
next prev parent reply other threads:[~2020-05-03 10:39 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-03 6:03 [PATCH v4 0/8] scsi: ufs: support LU Dedicated buffer mode for WriteBooster Stanley Chu
2020-05-03 6:03 ` [PATCH v4 1/8] scsi: ufs: enable WriteBooster on some pre-3.1 UFS devices Stanley Chu
2020-05-03 6:03 ` [PATCH v4 2/8] scsi: ufs: introduce fixup_dev_quirks vops Stanley Chu
2020-05-03 6:03 ` [PATCH v4 3/8] scsi: ufs: export ufs_fixup_device_setup() function Stanley Chu
2020-05-03 6:03 ` [PATCH v4 4/8] scsi: ufs-mediatek: add fixup_dev_quirks vops Stanley Chu
2020-05-03 6:03 ` [PATCH v4 5/8] scsi: ufs: add "index" in parameter list of ufshcd_query_flag() Stanley Chu
2020-05-03 6:03 ` [PATCH v4 6/8] scsi: ufs: add LU Dedicated buffer mode support for WriteBooster Stanley Chu
2020-05-03 8:00 ` Avri Altman
2020-05-03 10:39 ` Stanley Chu [this message]
2020-05-03 6:03 ` [PATCH v4 7/8] scsi: ufs-mediatek: enable WriteBooster capability Stanley Chu
2020-05-03 6:03 ` [PATCH v4 8/8] scsi: ufs: cleanup WriteBooster feature Stanley Chu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1588502389.3197.19.camel@mtkswgap22 \
--to=stanley.chu@mediatek.com \
--cc=Avri.Altman@wdc.com \
--cc=alim.akhtar@samsung.com \
--cc=andy.teng@mediatek.com \
--cc=asutoshd@codeaurora.org \
--cc=beanhuo@micron.com \
--cc=bvanassche@acm.org \
--cc=cang@codeaurora.org \
--cc=chun-hung.wu@mediatek.com \
--cc=jejb@linux.ibm.com \
--cc=kuohong.wang@mediatek.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=matthias.bgg@gmail.com \
--cc=peter.wang@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).