diff mbox series

[v2,1/5] scsi; ufs: add device descriptor for Host Performance Booster

Message ID 20200416203126.1210-2-beanhuo@micron.com (mailing list archive)
State Changes Requested
Headers show
Series UFS Host Performance Booster (HPB v1.0) driver | expand

Commit Message

Bean Huo April 16, 2020, 8:31 p.m. UTC 
From: Bean Huo <beanhuo@micron.com>

Add HPB related attributes in devivce descriptor. HPB support is specified
by Bit7 of bUFSFeatureSupport, HPB version is indicated by wHPBVersion and
the HPB Control Mode is specified by bHPBControl in UFS device descriptor.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/scsi/ufs/ufs.h    |  6 ++++++
 drivers/scsi/ufs/ufshcd.c | 11 +++++++++++
 2 files changed, 17 insertions(+)

Comments

Bart Van Assche April 22, 2020, 11:11 p.m. UTC | #1 
On 4/16/20 1:31 PM, huobean@gmail.com wrote:
> +	if (desc_buf[DEVICE_DESC_PARAM_UFS_FEAT] & 0x80) {

Please introduce a symbolic name instead of using the number 0x80 directly.

> +		hba->dev_info.hpb_control_mode =
> +			desc_buf[DEVICE_DESC_PARAM_HPB_CTRL_MODE];
> +		hba->dev_info.hpb_ver =
> +			(u16) (desc_buf[DEVICE_DESC_PARAM_HPB_VER] << 8) |
> +			desc_buf[DEVICE_DESC_PARAM_HPB_VER + 1];

Please use get_unaligned_be16() instead of open-coding it.

Thanks,

Bart.
Bean Huo April 23, 2020, 11:01 a.m. UTC | #2 
Hi Bart
Thanks your review, I will take your suggestions in next version development.
Thanks,

Bean

> 
> On 4/16/20 1:31 PM, huobean@gmail.com wrote:
> > +	if (desc_buf[DEVICE_DESC_PARAM_UFS_FEAT] & 0x80) {
> 
> Please introduce a symbolic name instead of using the number 0x80 directly.
> 
> > +		hba->dev_info.hpb_control_mode =
> > +			desc_buf[DEVICE_DESC_PARAM_HPB_CTRL_MODE];
> > +		hba->dev_info.hpb_ver =
> > +			(u16) (desc_buf[DEVICE_DESC_PARAM_HPB_VER] << 8)
> |
> > +			desc_buf[DEVICE_DESC_PARAM_HPB_VER + 1];
> 
> Please use get_unaligned_be16() instead of open-coding it.
> 
> Thanks,
> 
> Bart.
Bart Van Assche April 23, 2020, 5:57 p.m. UTC | #3 
On 4/23/20 4:01 AM, Bean Huo (beanhuo) wrote:
> Thanks your review, I will take your suggestions in next version development.

Hi Bean,

Please make sure that the issues raised by Christoph are addressed 
before reposting this series.

Additionally, I think the following two call chains are deadlock-prone 
and should be addressed:

ufshcd_queuecommand()
-> ufshcd_comp_scsi_upiu()
   -> ufshpb_prep_fn()
     -> schedule_work(&hpb->ufshpb_req_work)
       -> ufshpb_req_workq_fn()
         -> ufshpb_subregion_activate()
           -> ufshpb_subregion_l2p_load()
             -> ufshpb_l2p_load_req()
               -> blk_execute_rq_nowait()

ufshcd_queuecommand()
-> ufshcd_comp_scsi_upiu()
   -> ufshpb_prep_fn()
     -> schedule_work(&hpb->ufshpb_req_work)
       -> ufshpb_req_workq_fn()
         -> ufshpb_subregion_activate()
         -> ufshpb_region_deactivate()
           -> ufshpb_execute_cmd()
             -> scsi_execute()

I don't know any other block or SCSI driver that calls blk_execute_rq*() 
or scsi_execute() from inside its .queue_rq() callback to queue requests 
onto the same request queue (the device mapper requeues requests onto a 
lower level request queue).

Thanks,

Bart.
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index b7fec5c73688..53a5e263f7c8 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -258,6 +258,8 @@  enum device_desc_param {
 	DEVICE_DESC_PARAM_PSA_MAX_DATA		= 0x25,
 	DEVICE_DESC_PARAM_PSA_TMT		= 0x29,
 	DEVICE_DESC_PARAM_PRDCT_REV		= 0x2A,
+	DEVICE_DESC_PARAM_HPB_VER		= 0x40,
+	DEVICE_DESC_PARAM_HPB_CTRL_MODE		= 0x42,
 };
 
 /* Interconnect descriptor parameters offsets in bytes*/
@@ -537,6 +539,10 @@  struct ufs_dev_info {
 	u8 *model;
 	u16 wspecversion;
 	u32 clk_gating_wait_us;
+	/* HPB Version */
+	u16 hpb_ver;
+	/* bHPBControl */
+	u8 hpb_control_mode;
 };
 
 /**
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 698e8d20b4ba..de13d2333f1f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6627,6 +6627,17 @@  static int ufs_get_device_desc(struct ufs_hba *hba)
 		goto out;
 	}
 
+	if (desc_buf[DEVICE_DESC_PARAM_UFS_FEAT] & 0x80) {
+		hba->dev_info.hpb_control_mode =
+			desc_buf[DEVICE_DESC_PARAM_HPB_CTRL_MODE];
+		hba->dev_info.hpb_ver =
+			(u16) (desc_buf[DEVICE_DESC_PARAM_HPB_VER] << 8) |
+			desc_buf[DEVICE_DESC_PARAM_HPB_VER + 1];
+		dev_info(hba->dev, "HPB Version: 0x%2x\n",
+			 hba->dev_info.hpb_ver);
+		dev_info(hba->dev, "HPB control mode: %d\n",
+			 hba->dev_info.hpb_control_mode);
+	}
 	/*
 	 * getting vendor (manufacturerID) and Bank Index in big endian
 	 * format