From: Jaegeuk Kim <jaegeuk@google.com>

Let's support WRITE_16, READ_16, ZBC_IN, ZBC_OUT.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 drivers/ufs/core/ufshcd.c  | 8 ++++----
 include/trace/events/ufs.h | 4 +++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index e18c9f4463ec..235d2e2d828a 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -402,15 +402,15 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
 
 	opcode = cmd->cmnd[0];
 
-	if (opcode == READ_10 || opcode == WRITE_10) {
-		/*
-		 * Currently we only fully trace read(10) and write(10) commands
-		 */
+	if (opcode == READ_10 || opcode == WRITE_10 ||
+				opcode == READ_16 || opcode == WRITE_16) {
 		transfer_len =
 		       be32_to_cpu(lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
 		lba = scsi_get_lba(cmd);
 		if (opcode == WRITE_10)
 			group_id = lrbp->cmd->cmnd[6];
+		if (opcode == WRITE_16)
+			group_id = lrbp->cmd->cmnd[14];
 	} else if (opcode == UNMAP) {
 		/*
 		 * The number of Bytes to be unmapped beginning with the lba.
diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
index 599739ee7b20..f82a9e15fd78 100644
--- a/include/trace/events/ufs.h
+++ b/include/trace/events/ufs.h
@@ -18,7 +18,9 @@
 		{ READ_16,		"READ_16" },			\
 		{ READ_10,		"READ_10" },			\
 		{ SYNCHRONIZE_CACHE,	"SYNC" },			\
-		{ UNMAP,		"UNMAP" })
+		{ UNMAP,		"UNMAP" },			\
+		{ ZBC_IN,		"ZBC_IN" },			\
+		{ ZBC_OUT,		"ZBC_OUT" })
 
 #define UFS_LINK_STATES						\
 	EM(UIC_LINK_OFF_STATE,		"UIC_LINK_OFF_STATE")		\
-- 
2.39.1.581.gbfd45094c4-goog

Why does UFS even have it's own common tracing instad of just relying
on the core SCSI one, and even worse pokes into command set specifics
which is a no-go for LLDDs.  This code simply needs to go away instead
of beeing "enhanced".

On 02/15, Christoph Hellwig wrote:
> Why does UFS even have it's own common tracing instad of just relying
> on the core SCSI one, and even worse pokes into command set specifics
> which is a no-go for LLDDs.  This code simply needs to go away instead
> of beeing "enhanced".

I'm not sure how all the other vendors use the trace tho, at least to me,
it's quite useful when debugging any UFS-specific information such as
group_id and doorbell status along with the attached scsi command, in
addition to the accurate latency measurements.

On Mon, Feb 27, 2023 at 03:15:51PM -0800, Jaegeuk Kim wrote:
> On 02/15, Christoph Hellwig wrote:
> > Why does UFS even have it's own common tracing instad of just relying
> > on the core SCSI one, and even worse pokes into command set specifics
> > which is a no-go for LLDDs.  This code simply needs to go away instead
> > of beeing "enhanced".
> 
> I'm not sure how all the other vendors use the trace tho, at least to me,
> it's quite useful when debugging any UFS-specific information such as
> group_id

The group ID isn't even ever set, how can it be useful to you?

 
> On 02/15, Christoph Hellwig wrote:
> > Why does UFS even have it's own common tracing instad of just relying
> > on the core SCSI one, and even worse pokes into command set specifics
> > which is a no-go for LLDDs.  This code simply needs to go away instead
> > of beeing "enhanced".
> 
> I'm not sure how all the other vendors use the trace tho, at least to me, it's
> quite useful when debugging any UFS-specific information such as group_id and
> doorbell status along with the attached scsi command, in addition to the
> accurate latency measurements.
For us as well.
Moreover, we are mainly using upiu tracing which is not available at scsi ML.

Thanks,
Avri

On Tue, Feb 28, 2023 at 07:28:35AM +0000, Avri Altman wrote:
> > doorbell status along with the attached scsi command, in addition to the
> > accurate latency measurements.
> For us as well.
> Moreover, we are mainly using upiu tracing which is not available at scsi ML.

So let's rework the tracepoints to work at the UPIU level and stop the
gracious poking into SCSI CDBs.

On 2/15/23 11:04, Jaegeuk Kim wrote:
> Let's support WRITE_16, READ_16, ZBC_IN, ZBC_OUT.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>