scsi: ufs: fix Command Type issue according to UFS 2.0 spec

PATCH In review Chuanxiao Dong 3 participants 4 messages Aug 1, 2014 – Aug 18, 2014 SCSI

Share & tools

Chuanxiao Dong

author

2014-08-01 08:03 UTC #0 lore raw

PATCH 1/1 scsi: ufs: fix Command Type issue according to UFS 2.0 spec

UFS 2.0 spec defines that the command type in transfer descriptor is always 0x1 instead of different values as shown in UFSHC1.0/1.1.

This patch will distingwish v1.0/v1.1 UFSHC and later UFSHC when setting CT

Signed-off-by Chuanxiao Dong <[email protected]>

---
 drivers/scsi/ufs/ufshcd.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

drivers/scsi/ufs/ufshcd.c +21 -6

index ba27215..8635d5d 100644
@@ -706,16 +706,19 @@ static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
 /**
  * ufshcd_prepare_req_desc_hdr() - Fills the requests header
  * descriptor according to request
+ * @hba: per adapter instance
  * @lrbp: pointer to local reference block
  * @upiu_flags: flags required in the header
  * @cmd_dir: requests data direction
  */
-static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
-		u32 *upiu_flags, enum dma_data_direction cmd_dir)
+static void ufshcd_prepare_req_desc_hdr(struct ufs_hba *hba,
+		struct ufshcd_lrb *lrbp, u32 *upiu_flags,
+		enum dma_data_direction cmd_dir)
 {
 	struct utp_transfer_req_desc *req_desc = lrbp->utr_descriptor_ptr;
 	u32 data_direction;
 	u32 dword_0;
+	u8 cmd_type;
 
 	if (cmd_dir == DMA_FROM_DEVICE) {
 		data_direction = UTP_DEVICE_TO_HOST;

@@ -728,8 +731,20 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
 		*upiu_flags = UPIU_CMD_FLAGS_NONE;
 	}
 
-	dword_0 = data_direction | (lrbp->command_type
-				<< UPIU_COMMAND_TYPE_OFFSET);
+	/*
+	 * Per UFSHCI spec v1.0/v1.1, Command Type in UTP transfer
+	 * request descriptor is not the same as v2.0 spec.
+	 * In v2.0 spec, Command Type is always 1, the other values
+	 * are reserved
+	 */
+	if (hba->ufs_version == UFSHCI_VERSION_10 ||
+			hba->ufs_version == UFSHCI_VERSION_11)
+		cmd_type = lrbp->command_type;
+	else
+		cmd_type = UTP_CMD_TYPE_UFS;
+
+	dword_0 = data_direction | (cmd_type << UPIU_COMMAND_TYPE_OFFSET);
+
 	if (lrbp->intr_cmd)
 		dword_0 |= UTP_REQ_DESC_INT_CMD;

@@ -834,7 +849,7 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 	switch (lrbp->command_type) {
 	case UTP_CMD_TYPE_SCSI:
 		if (likely(lrbp->cmd)) {
-			ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags,
+			ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags,
 					lrbp->cmd->sc_data_direction);
 			ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);
 		} else {

@@ -842,7 +857,7 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 		}
 		break;
 	case UTP_CMD_TYPE_DEV_MANAGE:
-		ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, DMA_NONE);
+		ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags, DMA_NONE);
 		if (hba->dev_cmd.type == DEV_CMD_TYPE_QUERY)
 			ufshcd_prepare_utp_query_req_upiu(
 					hba, lrbp, upiu_flags);

Signature · 1 line

1.7.10.4

Dong, Chuanxiao

author

2014-08-13 15:13 UTC #1 lore raw

Any comment to this patch?

Thanks
Chuanxiao

Quoted 98 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Chuanxiao Dong
Sent: Friday, August 01, 2014 4:01 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]
Subject: [PATCH] scsi: ufs: fix Command Type issue according to UFS 2.0 spec

UFS 2.0 spec defines that the command type in transfer descriptor is always 0x1 instead of different values as shown in UFSHC1.0/1.1.

This patch will distingwish v1.0/v1.1 UFSHC and later UFSHC when setting CT

Signed-off-by Chuanxiao Dong <[email protected]>

---
 drivers/scsi/ufs/ufshcd.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

drivers/scsi/ufs/ufshcd.c index renamed from drivers/scsi/ufs/ufshcd.c +0 -0

ba27215..8635d5d 100644

drivers/scsi/ufs/ufshcd.c +5 -2

@@ -706,16 +706,19 @@ static void ufshcd_disable_intr(struct ufs_hba *hba,
 u32 intrs)
 /**
  * ufshcd_prepare_req_desc_hdr() - Fills the requests header
  * descriptor according to request
+ * @hba: per adapter instance
  * @lrbp: pointer to local reference block
  * @upiu_flags: flags required in the header
  * @cmd_dir: requests data direction
  */
-static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
-		u32 *upiu_flags, enum dma_data_direction cmd_dir)
+static void ufshcd_prepare_req_desc_hdr(struct ufs_hba *hba,
+		struct ufshcd_lrb *lrbp, u32 *upiu_flags,
+		enum dma_data_direction cmd_dir)
 {
 	struct utp_transfer_req_desc *req_desc = lrbp->utr_descriptor_ptr;
 	u32 data_direction;
 	u32 dword_0;
+	u8 cmd_type;
 
 	if (cmd_dir == DMA_FROM_DEVICE) {

drivers/scsi/ufs/ufshcd.c +14 -2

 		data_direction = UTP_DEVICE_TO_HOST;

@@ -728,8 +731,20 @@ static void ufshcd_prepare_req_desc_hdr(struct
 ufshcd_lrb *lrbp,
 		*upiu_flags = UPIU_CMD_FLAGS_NONE;
 	}
 
-	dword_0 = data_direction | (lrbp->command_type
-				<< UPIU_COMMAND_TYPE_OFFSET);
+	/*
+	 * Per UFSHCI spec v1.0/v1.1, Command Type in UTP transfer
+	 * request descriptor is not the same as v2.0 spec.
+	 * In v2.0 spec, Command Type is always 1, the other values
+	 * are reserved
+	 */
+	if (hba->ufs_version == UFSHCI_VERSION_10 ||
+			hba->ufs_version == UFSHCI_VERSION_11)
+		cmd_type = lrbp->command_type;
+	else
+		cmd_type = UTP_CMD_TYPE_UFS;
+
+	dword_0 = data_direction | (cmd_type <<
 UPIU_COMMAND_TYPE_OFFSET);
+
 	if (lrbp->intr_cmd)

drivers/scsi/ufs/ufshcd.c +1 -1

 		dword_0 |= UTP_REQ_DESC_INT_CMD;

@@ -834,7 +849,7 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba,
 struct ufshcd_lrb *lrbp)
 	switch (lrbp->command_type) {
 	case UTP_CMD_TYPE_SCSI:
 		if (likely(lrbp->cmd)) {
-			ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags,
+			ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags,
 					lrbp->cmd->sc_data_direction);
 			ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);

drivers/scsi/ufs/ufshcd.c +1 -1

 		} else {

@@ -842,7 +857,7 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba,
 struct ufshcd_lrb *lrbp)
 		}
 		break;
 	case UTP_CMD_TYPE_DEV_MANAGE:
-		ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, DMA_NONE);
+		ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags, DMA_NONE);
 		if (hba->dev_cmd.type == DEV_CMD_TYPE_QUERY)
 			ufshcd_prepare_utp_query_req_upiu(

hba, lrbp, upiu_flags);
--
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html

Lee Susman

2014-08-14 13:57 UTC #2 lore raw

Hi Chuanxiao,

I have pulled and ran multiple dd runs on a build with your patch (to an external UFS device).
Did not see any issues with this patch, adding my reviewed-by and tested-by.

Reviewed-off-by Lee Susman <[email protected]>

Tested-off-by Lee Susman <[email protected]>

Thanks,
Lee

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Dong, Chuanxiao
Sent: Wednesday, August 13, 2014 6:12 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH] scsi: ufs: fix Command Type issue according to UFS 2.0 spec

Any comment to this patch?

Thanks
Chuanxiao

Quoted 48 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Chuanxiao Dong
Sent: Friday, August 01, 2014 4:01 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]
Subject: [PATCH] scsi: ufs: fix Command Type issue according to UFS 2.0 spec
UFS 2.0 spec defines that the command type in transfer descriptor is always
0x1 instead of different values as shown in UFSHC1.0/1.1.
This patch will distingwish v1.0/v1.1 UFSHC and later UFSHC when setting CT
Signed-off-by Chuanxiao Dong <[email protected]>
---
 drivers/scsi/ufs/ufshcd.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)
drivers/scsi/ufs/ufshcd.c +5 -2
index ba27215..8635d5d 100644
@@ -706,16 +706,19 @@ static void ufshcd_disable_intr(struct ufs_hba 
 *hba,
 u32 intrs)
 /**
  * ufshcd_prepare_req_desc_hdr() - Fills the requests header
  * descriptor according to request
+ * @hba: per adapter instance
  * @lrbp: pointer to local reference block
  * @upiu_flags: flags required in the header
  * @cmd_dir: requests data direction
  */
-static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
-		u32 *upiu_flags, enum dma_data_direction cmd_dir)
+static void ufshcd_prepare_req_desc_hdr(struct ufs_hba *hba,
+		struct ufshcd_lrb *lrbp, u32 *upiu_flags,
+		enum dma_data_direction cmd_dir)
 {
 	struct utp_transfer_req_desc *req_desc = lrbp->utr_descriptor_ptr;
 	u32 data_direction;
 	u32 dword_0;
+	u8 cmd_type;
 

drivers/scsi/ufs/ufshcd.c

Quoted 2 lines

drivers/scsi/ufs/ufshcd.c +0 -0

 	if (cmd_dir == DMA_FROM_DEVICE) {
 		data_direction = UTP_DEVICE_TO_HOST; @@ -728,8 +731,20 @@

static

Quoted 36 lines

void ufshcd_prepare_req_desc_hdr(struct
ufshcd_lrb *lrbp,

drivers/scsi/ufs/ufshcd.c +13 -2

 		*upiu_flags = UPIU_CMD_FLAGS_NONE;
 	}
 
-	dword_0 = data_direction | (lrbp->command_type
-				<< UPIU_COMMAND_TYPE_OFFSET);
+	/*
+	 * Per UFSHCI spec v1.0/v1.1, Command Type in UTP transfer
+	 * request descriptor is not the same as v2.0 spec.
+	 * In v2.0 spec, Command Type is always 1, the other values
+	 * are reserved
+	 */
+	if (hba->ufs_version == UFSHCI_VERSION_10 ||
+			hba->ufs_version == UFSHCI_VERSION_11)
+		cmd_type = lrbp->command_type;
+	else
+		cmd_type = UTP_CMD_TYPE_UFS;
+
+	dword_0 = data_direction | (cmd_type <<

UPIU_COMMAND_TYPE_OFFSET);

drivers/scsi/ufs/ufshcd.c +2 -1

+
 	if (lrbp->intr_cmd)
 		dword_0 |= UTP_REQ_DESC_INT_CMD;

@@ -834,7 +849,7 @@ static int ufshcd_compose_upiu(struct ufs_hba 
 *hba, struct ufshcd_lrb *lrbp)
 	switch (lrbp->command_type) {
 	case UTP_CMD_TYPE_SCSI:
 		if (likely(lrbp->cmd)) {
-			ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags,
+			ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags,
 					lrbp->cmd->sc_data_direction);
 			ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);

drivers/scsi/ufs/ufshcd.c +0 -0

 		} else {

drivers/scsi/ufs/ufshcd.c static int ufshcd_compose_upiu(struct ufs_hba · line 861

Quoted 6 lines

drivers/scsi/ufs/ufshcd.c +1 -1

@@ -842,7 +857,7 @@ static int ufshcd_compose_upiu(struct ufs_hba 
 *hba, struct ufshcd_lrb *lrbp)
 		}
 		break;
 	case UTP_CMD_TYPE_DEV_MANAGE:
-		ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, DMA_NONE);
+		ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags,

DMA_NONE);

Quoted 10 lines

drivers/scsi/ufs/ufshcd.c +0 -0
 		if (hba->dev_cmd.type == DEV_CMD_TYPE_QUERY)
 			ufshcd_prepare_utp_query_req_upiu(
hba, lrbp, upiu_flags);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html

Santosh Y

Acked-by on v1 1/1

2014-08-18 18:07 UTC #3 lore raw

In reply to Chuanxiao Dong

Quoted 79 lines · Chuanxiao Dong

UFS 2.0 spec defines that the command type in transfer descriptor is always 0x1 instead of different values as shown in UFSHC1.0/1.1.

This patch will distingwish v1.0/v1.1 UFSHC and later UFSHC when setting CT

Signed-off-by Chuanxiao Dong <[email protected]>

---
 drivers/scsi/ufs/ufshcd.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

drivers/scsi/ufs/ufshcd.c +21 -7

index ba27215..8635d5d 100644
@@ -706,16 +706,19 @@ static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
 /**
  * ufshcd_prepare_req_desc_hdr() - Fills the requests header
  * descriptor according to request
+ * @hba: per adapter instance
  * @lrbp: pointer to local reference block
  * @upiu_flags: flags required in the header
  * @cmd_dir: requests data direction
  */
-static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
-               u32 *upiu_flags, enum dma_data_direction cmd_dir)
+static void ufshcd_prepare_req_desc_hdr(struct ufs_hba *hba,
+               struct ufshcd_lrb *lrbp, u32 *upiu_flags,
+               enum dma_data_direction cmd_dir)
 {
        struct utp_transfer_req_desc *req_desc = lrbp->utr_descriptor_ptr;
        u32 data_direction;
        u32 dword_0;
+       u8 cmd_type;
 
        if (cmd_dir == DMA_FROM_DEVICE) {
                data_direction = UTP_DEVICE_TO_HOST;

@@ -728,8 +731,20 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
                *upiu_flags = UPIU_CMD_FLAGS_NONE;
        }
 
-       dword_0 = data_direction | (lrbp->command_type
-                               << UPIU_COMMAND_TYPE_OFFSET);
+       /*
+        * Per UFSHCI spec v1.0/v1.1, Command Type in UTP transfer
+        * request descriptor is not the same as v2.0 spec.
+        * In v2.0 spec, Command Type is always 1, the other values
+        * are reserved
+        */
+       if (hba->ufs_version == UFSHCI_VERSION_10 ||
+                       hba->ufs_version == UFSHCI_VERSION_11)
+               cmd_type = lrbp->command_type;
+       else
+               cmd_type = UTP_CMD_TYPE_UFS;
+
+       dword_0 = data_direction | (cmd_type << UPIU_COMMAND_TYPE_OFFSET);
+
        if (lrbp->intr_cmd)
                dword_0 |= UTP_REQ_DESC_INT_CMD;

@@ -834,7 +849,7 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
        switch (lrbp->command_type) {
        case UTP_CMD_TYPE_SCSI:
                if (likely(lrbp->cmd)) {
-                       ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags,
+                       ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags,
                                        lrbp->cmd->sc_data_direction);
                        ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);
                } else {

@@ -842,7 +857,7 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
                }
                break;
        case UTP_CMD_TYPE_DEV_MANAGE:
-               ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, DMA_NONE);
+               ufshcd_prepare_req_desc_hdr(hba, lrbp, &upiu_flags, DMA_NONE);
                if (hba->dev_cmd.type == DEV_CMD_TYPE_QUERY)
                        ufshcd_prepare_utp_query_req_upiu(
                                        hba, lrbp, upiu_flags);
--

Quoted 2 lines

1.7.10.4

Acked-by Santosh Y <[email protected]>

Signature · 1 line

~Santosh