[PATCH v2 0/4] Several minor changes for UFS

[PATCH v2 0/4] Several minor changes for UFS

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Changelog:
 v1--v2:
    1. Add a new cleanup patch 1/4
    2. Make the patch 3/4 much readable by initializing a variable
    'header'.


Bean Huo (4):
  scsi: ufs: Cleanup ufshcd_add_command_trace()
  scsi: ufs: Let UPIU completion trace print RSP UPIU header
  scsi: ufs: Let command trace only for the cmd != null case
  scsi: ufs: Use UPIU query trace in devman_upiu_cmd

 drivers/scsi/ufs/ufshcd.c | 59 ++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 25 deletions(-)

-- 
2.25.1

[PATCH v2 1/4] scsi: ufs: Cleanup ufshcd_add_command_trace()

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

To consistent with trace event print, convert the value of the
variable 'lba' in the unit of LBA (logical block address).

Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/scsi/ufs/ufshcd.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 02267b090729..85590d3a719e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -25,6 +25,7 @@
 #include "ufs_bsg.h"
 #include "ufshcd-crypto.h"
 #include <asm/unaligned.h>
+#include "../sd.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/ufs.h>
@@ -364,7 +365,7 @@ static void ufshcd_add_uic_command_trace(struct ufs_hba *hba,
 static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 				     enum ufs_trace_str_t str_t)
 {
-	sector_t lba = -1;
+	u64 lba = -1;
 	u8 opcode = 0, group_id = 0;
 	u32 intr, doorbell;
 	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
@@ -382,22 +383,23 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 		/* trace UPIU also */
 		ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
 		opcode = cmd->cmnd[0];
+		lba = sectors_to_logical(cmd->device, blk_rq_pos(cmd->request));
+
 		if ((opcode == READ_10) || (opcode == WRITE_10)) {
 			/*
 			 * Currently we only fully trace read(10) and write(10)
 			 * commands
 			 */
-			if (cmd->request && cmd->request->bio)
-				lba = cmd->request->bio->bi_iter.bi_sector;
 			transfer_len = be32_to_cpu(
 				lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
 			if (opcode == WRITE_10)
 				group_id = lrbp->cmd->cmnd[6];
 		} else if (opcode == UNMAP) {
-			if (cmd->request) {
-				lba = scsi_get_lba(cmd);
-				transfer_len = blk_rq_bytes(cmd->request);
-			}
+			/*
+			 * The number of Bytes to be unmapped beginning with the
+			 * lba.
+			 */
+			transfer_len = blk_rq_bytes(cmd->request);
 		}
 	}
 
-- 
2.25.1

Re: [PATCH v2 1/4] scsi: ufs: Cleanup ufshcd_add_command_trace()

[Can Guo]

On 2021-05-31 18:43, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> To consistent with trace event print, convert the value of the
> variable 'lba' in the unit of LBA (logical block address).
> 
> Suggested-by: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Bean Huo <beanhuo@micron.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 02267b090729..85590d3a719e 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -25,6 +25,7 @@
>  #include "ufs_bsg.h"
>  #include "ufshcd-crypto.h"
>  #include <asm/unaligned.h>
> +#include "../sd.h"
> 
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/ufs.h>
> @@ -364,7 +365,7 @@ static void ufshcd_add_uic_command_trace(struct
> ufs_hba *hba,
>  static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int 
> tag,
>  				     enum ufs_trace_str_t str_t)
>  {
> -	sector_t lba = -1;
> +	u64 lba = -1;
>  	u8 opcode = 0, group_id = 0;
>  	u32 intr, doorbell;
>  	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> @@ -382,22 +383,23 @@ static void ufshcd_add_command_trace(struct
> ufs_hba *hba, unsigned int tag,
>  		/* trace UPIU also */
>  		ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
>  		opcode = cmd->cmnd[0];
> +		lba = sectors_to_logical(cmd->device, blk_rq_pos(cmd->request));
> +
>  		if ((opcode == READ_10) || (opcode == WRITE_10)) {
>  			/*
>  			 * Currently we only fully trace read(10) and write(10)
>  			 * commands
>  			 */
> -			if (cmd->request && cmd->request->bio)
> -				lba = cmd->request->bio->bi_iter.bi_sector;
>  			transfer_len = be32_to_cpu(
>  				lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
>  			if (opcode == WRITE_10)
>  				group_id = lrbp->cmd->cmnd[6];
>  		} else if (opcode == UNMAP) {
> -			if (cmd->request) {
> -				lba = scsi_get_lba(cmd);
> -				transfer_len = blk_rq_bytes(cmd->request);
> -			}
> +			/*
> +			 * The number of Bytes to be unmapped beginning with the
> +			 * lba.
> +			 */
> +			transfer_len = blk_rq_bytes(cmd->request);
>  		}
>  	}

Reviewed-by: Can Guo <cang@codeaurora.org>

[PATCH v2 2/4] scsi: ufs: Let UPIU completion trace print RSP UPIU header

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

The current UPIU completion event trace still prints the COMMAND UPIU
header, rather than the RSP UPIU header. This makes UPIU command trace
useless in problem shooting in case we receive a trace log from the
customer/field.

There are two important fields in RSP UPIU:
 1. The response field, which indicates the UFS defined overall success
    or failure of the series of Command, Data and RESPONSE UPIU’s that
    make up the execution of a task.
 2. The Status field, which contains the command set specific status for
    a specific command issued by the initiator device.

Before this patch, the UPIU paired trace events:

ufshcd_upiu: send_req: fe3b0000.ufs: HDR:01 20 00 1c 00 00 00 00 00 00 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00
ufshcd_upiu: complete_rsp: fe3b0000.ufs: HDR:01 20 00 1c 00 00 00 00 00 00 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00

After the patch:

ufshcd_upiu: send_req: fe3b0000.ufs: HDR:01 20 00 1c 00 00 00 00 00 00 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00
ufshcd_upiu: complete_rsp: fe3b0000.ufs: HDR:21 00 00 1c 00 00 00 00 00 00 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/scsi/ufs/ufshcd.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 85590d3a719e..c5754d5486c9 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -302,11 +302,17 @@ static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba, unsigned int tag,
 				      enum ufs_trace_str_t str_t)
 {
 	struct utp_upiu_req *rq = hba->lrb[tag].ucd_req_ptr;
+	struct utp_upiu_header *header;
 
 	if (!trace_ufshcd_upiu_enabled())
 		return;
 
-	trace_ufshcd_upiu(dev_name(hba->dev), str_t, &rq->header, &rq->sc.cdb,
+	if (str_t == UFS_CMD_SEND)
+		header = &rq->header;
+	else
+		header = &hba->lrb[tag].ucd_rsp_ptr->header;
+
+	trace_ufshcd_upiu(dev_name(hba->dev), str_t, header, &rq->sc.cdb,
 			  UFS_TSF_CDB);
 }
 
-- 
2.25.1

Re: [PATCH v2 2/4] scsi: ufs: Let UPIU completion trace print RSP UPIU header

[Can Guo]

On 2021-05-31 18:43, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> The current UPIU completion event trace still prints the COMMAND UPIU
> header, rather than the RSP UPIU header. This makes UPIU command trace
> useless in problem shooting in case we receive a trace log from the
> customer/field.
> 
> There are two important fields in RSP UPIU:
>  1. The response field, which indicates the UFS defined overall success
>     or failure of the series of Command, Data and RESPONSE UPIU’s that
>     make up the execution of a task.
>  2. The Status field, which contains the command set specific status 
> for
>     a specific command issued by the initiator device.
> 
> Before this patch, the UPIU paired trace events:
> 
> ufshcd_upiu: send_req: fe3b0000.ufs: HDR:01 20 00 1c 00 00 00 00 00 00
> 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00
> ufshcd_upiu: complete_rsp: fe3b0000.ufs: HDR:01 20 00 1c 00 00 00 00
> 00 00 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00
> 
> After the patch:
> 
> ufshcd_upiu: send_req: fe3b0000.ufs: HDR:01 20 00 1c 00 00 00 00 00 00
> 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00
> ufshcd_upiu: complete_rsp: fe3b0000.ufs: HDR:21 00 00 1c 00 00 00 00
> 00 00 00 00, CDB:3b e1 00 00 00 00 00 00 30 00 00 00 00 00 00 00
> 
> Signed-off-by: Bean Huo <beanhuo@micron.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 85590d3a719e..c5754d5486c9 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -302,11 +302,17 @@ static void ufshcd_add_cmd_upiu_trace(struct
> ufs_hba *hba, unsigned int tag,
>  				      enum ufs_trace_str_t str_t)
>  {
>  	struct utp_upiu_req *rq = hba->lrb[tag].ucd_req_ptr;
> +	struct utp_upiu_header *header;
> 
>  	if (!trace_ufshcd_upiu_enabled())
>  		return;
> 
> -	trace_ufshcd_upiu(dev_name(hba->dev), str_t, &rq->header, 
> &rq->sc.cdb,
> +	if (str_t == UFS_CMD_SEND)
> +		header = &rq->header;
> +	else
> +		header = &hba->lrb[tag].ucd_rsp_ptr->header;
> +
> +	trace_ufshcd_upiu(dev_name(hba->dev), str_t, header, &rq->sc.cdb,
>  			  UFS_TSF_CDB);
>  }

Reviewed-by: Can Guo <cang@codeaurora.org>

[PATCH v2 3/4] scsi: ufs: Let command trace only for the cmd != null case

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

For the query request, we already have query_trace, but in
ufshcd_send_command (), there will add two more redundant
traces. Since lrbp->cmd is null in the query request, the
below these two trace events provide nothing except the tag
and DB. Instead of letting them take up the limited trace
ring buffer, it’s better not to print these traces in case
of cmd == null.

ufshcd_command: send_req: ff3b0000.ufs: tag: 28, DB: 0x0, size: -1, IS: 0, LBA: 18446744073709551615, opcode: 0x0 (0x0), group_id: 0x0
ufshcd_command: dev_complete: ff3b0000.ufs: tag: 28, DB: 0x0, size: -1, IS: 0, LBA: 18446744073709551615, opcode: 0x0 (0x0), group_id: 0x0

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/scsi/ufs/ufshcd.c | 44 +++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c5754d5486c9..c84bd8e045f6 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -378,35 +378,33 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 	struct scsi_cmnd *cmd = lrbp->cmd;
 	int transfer_len = -1;
 
+	if (!cmd)
+		return;
+
 	if (!trace_ufshcd_command_enabled()) {
 		/* trace UPIU W/O tracing command */
-		if (cmd)
-			ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
+		ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
 		return;
 	}
 
-	if (cmd) { /* data phase exists */
-		/* trace UPIU also */
-		ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
-		opcode = cmd->cmnd[0];
-		lba = sectors_to_logical(cmd->device, blk_rq_pos(cmd->request));
+	/* trace UPIU also */
+	ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
+	opcode = cmd->cmnd[0];
+	lba = sectors_to_logical(cmd->device, blk_rq_pos(cmd->request));
 
-		if ((opcode == READ_10) || (opcode == WRITE_10)) {
-			/*
-			 * Currently we only fully trace read(10) and write(10)
-			 * commands
-			 */
-			transfer_len = be32_to_cpu(
-				lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
-			if (opcode == WRITE_10)
-				group_id = lrbp->cmd->cmnd[6];
-		} else if (opcode == UNMAP) {
-			/*
-			 * The number of Bytes to be unmapped beginning with the
-			 * lba.
-			 */
-			transfer_len = blk_rq_bytes(cmd->request);
-		}
+	if (opcode == READ_10 || opcode == WRITE_10) {
+		/*
+		 * Currently we only fully trace read(10) and write(10) commands
+		 */
+		transfer_len =
+		       be32_to_cpu(lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
+		if (opcode == WRITE_10)
+			group_id = lrbp->cmd->cmnd[6];
+	} else if (opcode == UNMAP) {
+		/*
+		 * The number of Bytes to be unmapped beginning with the lba.
+		 */
+		transfer_len = blk_rq_bytes(cmd->request);
 	}
 
 	intr = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
-- 
2.25.1

Re: [PATCH v2 3/4] scsi: ufs: Let command trace only for the cmd != null case

[Can Guo]

On 2021-05-31 18:43, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> For the query request, we already have query_trace, but in
> ufshcd_send_command (), there will add two more redundant
> traces. Since lrbp->cmd is null in the query request, the
> below these two trace events provide nothing except the tag
> and DB. Instead of letting them take up the limited trace
> ring buffer, it’s better not to print these traces in case
> of cmd == null.
> 
> ufshcd_command: send_req: ff3b0000.ufs: tag: 28, DB: 0x0, size: -1,
> IS: 0, LBA: 18446744073709551615, opcode: 0x0 (0x0), group_id: 0x0
> ufshcd_command: dev_complete: ff3b0000.ufs: tag: 28, DB: 0x0, size:
> -1, IS: 0, LBA: 18446744073709551615, opcode: 0x0 (0x0), group_id: 0x0
> 
> Signed-off-by: Bean Huo <beanhuo@micron.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 44 +++++++++++++++++++--------------------
>  1 file changed, 21 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index c5754d5486c9..c84bd8e045f6 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -378,35 +378,33 @@ static void ufshcd_add_command_trace(struct
> ufs_hba *hba, unsigned int tag,
>  	struct scsi_cmnd *cmd = lrbp->cmd;
>  	int transfer_len = -1;
> 
> +	if (!cmd)
> +		return;
> +
>  	if (!trace_ufshcd_command_enabled()) {
>  		/* trace UPIU W/O tracing command */
> -		if (cmd)
> -			ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
> +		ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
>  		return;
>  	}
> 
> -	if (cmd) { /* data phase exists */
> -		/* trace UPIU also */
> -		ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
> -		opcode = cmd->cmnd[0];
> -		lba = sectors_to_logical(cmd->device, blk_rq_pos(cmd->request));
> +	/* trace UPIU also */
> +	ufshcd_add_cmd_upiu_trace(hba, tag, str_t);
> +	opcode = cmd->cmnd[0];
> +	lba = sectors_to_logical(cmd->device, blk_rq_pos(cmd->request));
> 
> -		if ((opcode == READ_10) || (opcode == WRITE_10)) {
> -			/*
> -			 * Currently we only fully trace read(10) and write(10)
> -			 * commands
> -			 */
> -			transfer_len = be32_to_cpu(
> -				lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
> -			if (opcode == WRITE_10)
> -				group_id = lrbp->cmd->cmnd[6];
> -		} else if (opcode == UNMAP) {
> -			/*
> -			 * The number of Bytes to be unmapped beginning with the
> -			 * lba.
> -			 */
> -			transfer_len = blk_rq_bytes(cmd->request);
> -		}
> +	if (opcode == READ_10 || opcode == WRITE_10) {
> +		/*
> +		 * Currently we only fully trace read(10) and write(10) commands
> +		 */
> +		transfer_len =
> +		       be32_to_cpu(lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
> +		if (opcode == WRITE_10)
> +			group_id = lrbp->cmd->cmnd[6];
> +	} else if (opcode == UNMAP) {
> +		/*
> +		 * The number of Bytes to be unmapped beginning with the lba.
> +		 */
> +		transfer_len = blk_rq_bytes(cmd->request);
>  	}
> 
>  	intr = ufshcd_readl(hba, REG_INTERRUPT_STATUS);

Reviewed-by: Can Guo <cang@codeaurora.org>

[PATCH v2 4/4] scsi: ufs: Use UPIU query trace in devman_upiu_cmd

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Since devman_upiu_cmd is not COMMAND UPIU, and doesn't have
CDB, it is better to use UPIU query trace, which provides more
helpful information for issue shooting.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/scsi/ufs/ufshcd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c84bd8e045f6..deb9e232b349 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6701,6 +6701,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 
 	hba->dev_cmd.complete = &wait;
 
+	ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr);
 	/* Make sure descriptors are ready before ringing the doorbell */
 	wmb();
 	spin_lock_irqsave(hba->host->host_lock, flags);
@@ -6732,6 +6733,8 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 			err = -EINVAL;
 		}
 	}
+	ufshcd_add_query_upiu_trace(hba, err ? UFS_QUERY_ERR : UFS_QUERY_COMP,
+				    (struct utp_upiu_req *)lrbp->ucd_rsp_ptr);
 
 out:
 	blk_put_request(req);
-- 
2.25.1

Re: [PATCH v2 4/4] scsi: ufs: Use UPIU query trace in devman_upiu_cmd

[Can Guo]

Hi Bean,

On 2021-05-31 18:43, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> Since devman_upiu_cmd is not COMMAND UPIU, and doesn't have
> CDB, it is better to use UPIU query trace, which provides more
> helpful information for issue shooting.
> 
> Signed-off-by: Bean Huo <beanhuo@micron.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index c84bd8e045f6..deb9e232b349 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6701,6 +6701,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct
> ufs_hba *hba,
> 
>  	hba->dev_cmd.complete = &wait;
> 
> +	ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr);
>  	/* Make sure descriptors are ready before ringing the doorbell */
>  	wmb();
>  	spin_lock_irqsave(hba->host->host_lock, flags);
> @@ -6732,6 +6733,8 @@ static int ufshcd_issue_devman_upiu_cmd(struct
> ufs_hba *hba,
>  			err = -EINVAL;
>  		}
>  	}
> +	ufshcd_add_query_upiu_trace(hba, err ? UFS_QUERY_ERR : 
> UFS_QUERY_COMP,
> +				    (struct utp_upiu_req *)lrbp->ucd_rsp_ptr);
> 
>  out:
>  	blk_put_request(req);

What about ufshcd_exec_dev_cmd()?

Thanks,
Can Guo.

Re: [PATCH v2 4/4] scsi: ufs: Use UPIU query trace in devman_upiu_cmd

[Bean Huo]

On Wed, 2021-06-02 at 10:29 +0800, Can Guo wrote:
> >        spin_lock_irqsave(hba->host->host_lock, flags);
> > @@ -6732,6 +6733,8 @@ static int
> > ufshcd_issue_devman_upiu_cmd(struct
> > ufs_hba *hba,
> >                        err = -EINVAL;
> >                }
> >        }
> > +     ufshcd_add_query_upiu_trace(hba, err ? UFS_QUERY_ERR : 
> > UFS_QUERY_COMP,
> > +                                 (struct utp_upiu_req *)lrbp-
> > >ucd_rsp_ptr);
> >   out:
> >        blk_put_request(req);
> 
> 
> What about ufshcd_exec_dev_cmd()?
> 


Can,
thanks for your veiew.
yes, ufshcd_exec_dev_cmd() is only to send
UPIU command frame, and doesn't include CDB. It already uses
ufshcd_add_query_upiu_trace() to trace UPIU frame. 

Kind regards,
Bean

> Thanks,
> 
> Can Guo.

Re: [PATCH v2 4/4] scsi: ufs: Use UPIU query trace in devman_upiu_cmd

[Can Guo]

On 2021-06-03 05:05, Bean Huo wrote:
> On Wed, 2021-06-02 at 10:29 +0800, Can Guo wrote:
>> >        spin_lock_irqsave(hba->host->host_lock, flags);
>> > @@ -6732,6 +6733,8 @@ static int
>> > ufshcd_issue_devman_upiu_cmd(struct
>> > ufs_hba *hba,
>> >                        err = -EINVAL;
>> >                }
>> >        }
>> > +     ufshcd_add_query_upiu_trace(hba, err ? UFS_QUERY_ERR :
>> > UFS_QUERY_COMP,
>> > +                                 (struct utp_upiu_req *)lrbp-
>> > >ucd_rsp_ptr);
>> >   out:
>> >        blk_put_request(req);
>> 
>> 
>> What about ufshcd_exec_dev_cmd()?
>> 
> 
> 
> Can,
> thanks for your veiew.
> yes, ufshcd_exec_dev_cmd() is only to send
> UPIU command frame, and doesn't include CDB. It already uses
> ufshcd_add_query_upiu_trace() to trace UPIU frame.
> 

Oh, sorry, I missed it.

Reviewed-by: Can Guo <cang@codeaurora.org>

> Kind regards,
> Bean
> 
>> Thanks,
>> 
>> Can Guo.

Re: [PATCH v2 0/4] Several minor changes for UFS

[Martin K. Petersen]

Bean,

>     1. Add a new cleanup patch 1/4 2. Make the patch 3/4 much readable
>     by initializing a variable 'header'.

Applied to 5.14/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 0/4] Several minor changes for UFS

[Martin K. Petersen]

On Mon, 31 May 2021 12:43:04 +0200, Bean Huo wrote:

> Changelog:
>  v1--v2:
>     1. Add a new cleanup patch 1/4
>     2. Make the patch 3/4 much readable by initializing a variable
>     'header'.
> 
> 
> [...]

Applied to 5.14/scsi-queue, thanks!

[1/4] scsi: ufs: Cleanup ufshcd_add_command_trace()
      https://git.kernel.org/mkp/scsi/c/04c073feb1d7
[2/4] scsi: ufs: Let UPIU completion trace print RSP UPIU header
      https://git.kernel.org/mkp/scsi/c/89ac2c3b2835
[3/4] scsi: ufs: Let command trace only for the cmd != null case
      https://git.kernel.org/mkp/scsi/c/44b5de363524
[4/4] scsi: ufs: Use UPIU query trace in devman_upiu_cmd
      https://git.kernel.org/mkp/scsi/c/105424895c02

-- 
Martin K. Petersen	Oracle Linux Engineering