[RFC PATCH v2 0/2] ufs: introduce callbacks to get command information

[RFC PATCH v2 0/2] ufs: introduce callbacks to get command information

[Kiwoong Kim]

Some SoC specific might need command history for
various reasons, such as stacking command contexts
in system memory to check for debugging in the future
or scaling some DVFS knobs to boost IO throughput.

What you would do with the information could be
variant per SoC vendor.

Kiwoong Kim (2):
  ufs: introduce a callback to get info of command completion
  exynos-ufs: implement dbg_register_dump and compl_xfer_req

 drivers/scsi/ufs/Kconfig          |  14 +++
 drivers/scsi/ufs/Makefile         |   2 +-
 drivers/scsi/ufs/ufs-exynos-dbg.c | 202 ++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-exynos-if.h  |  17 ++++
 drivers/scsi/ufs/ufs-exynos.c     |  63 +++++++++++-
 drivers/scsi/ufs/ufs-exynos.h     |  13 +++
 drivers/scsi/ufs/ufshcd.c         |   2 +
 drivers/scsi/ufs/ufshcd.h         |   8 ++
 8 files changed, 318 insertions(+), 3 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufs-exynos-dbg.c
 create mode 100644 drivers/scsi/ufs/ufs-exynos-if.h

-- 
2.7.4

[RFC PATCH v2 1/2] ufs: introduce a callback to get info of command completion

[Kiwoong Kim]

Some SoC specific might need command history for
various reasons, such as stacking command contexts
in system memory to check for debugging in the future
or scaling some DVFS knobs to boost IO throughput.

What you would do with the information could be
variant per SoC vendor.

Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 2 ++
 drivers/scsi/ufs/ufshcd.h | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 52abe82..3326236 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4882,6 +4882,7 @@ static void __ufshcd_transfer_req_compl(struct ufs_hba *hba,
 	for_each_set_bit(index, &completed_reqs, hba->nutrs) {
 		lrbp = &hba->lrb[index];
 		cmd = lrbp->cmd;
+		ufshcd_vops_compl_xfer_req(hba, index, (cmd) ? true : false);
 		if (cmd) {
 			ufshcd_add_command_trace(hba, index, "complete");
 			result = ufshcd_transfer_rsp_status(hba, lrbp);
@@ -4890,6 +4891,7 @@ static void __ufshcd_transfer_req_compl(struct ufs_hba *hba,
 			/* Mark completed command as NULL in LRB */
 			lrbp->cmd = NULL;
 			lrbp->compl_time_stamp = ktime_get();
+
 			/* Do not touch lrbp after scsi done */
 			cmd->scsi_done(cmd);
 			__ufshcd_release(hba);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index c774012..6cf46bd 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -307,6 +307,7 @@ struct ufs_hba_variant_ops {
 	void	(*config_scaling_param)(struct ufs_hba *hba,
 					struct devfreq_dev_profile *profile,
 					void *data);
+	void	(*compl_xfer_req)(struct ufs_hba *hba, int tag, bool is_scsi);
 };
 
 /* clock gating state  */
@@ -1137,6 +1138,13 @@ static inline void ufshcd_vops_config_scaling_param(struct ufs_hba *hba,
 		hba->vops->config_scaling_param(hba, profile, data);
 }
 
+static inline void ufshcd_vops_compl_xfer_req(struct ufs_hba *hba,
+					      int tag, bool is_scsi);
+{
+	if (hba->vops && hba->vops->compl_xfer_req)
+		hba->vops->compl_xfer_req(hba, tag, is_scsi);
+}
+
 extern struct ufs_pm_lvl_states ufs_pm_lvl_states[];
 
 /*
-- 
2.7.4

Re: [RFC PATCH v2 1/2] ufs: introduce a callback to get info of command completion

[Stanley Chu]

On Thu, 2020-07-02 at 11:38 +0900, Kiwoong Kim wrote:
> Some SoC specific might need command history for
> various reasons, such as stacking command contexts
> in system memory to check for debugging in the future
> or scaling some DVFS knobs to boost IO throughput.
> 
> What you would do with the information could be
> variant per SoC vendor.
> 
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>

Acked-By: Stanley Chu <stanley.chu@mediatek.com>

Re: [RFC PATCH v2 1/2] ufs: introduce a callback to get info of command completion

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]

Hi Kiwoong,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on scsi/for-next]
[also build test ERROR on mkp-scsi/for-next v5.8-rc3 next-20200701]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kiwoong-Kim/ufs-introduce-callbacks-to-get-command-information/20200702-104850
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: parisc-allmodconfig (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from drivers/scsi/ufs/tc-dwc-g210-pci.c:10:
>> drivers/scsi/ufs/ufshcd.h:1143:1: error: expected identifier or '(' before '{' token
    1143 | {
         | ^
   drivers/scsi/ufs/ufshcd.h:1141:20: warning: 'ufshcd_vops_compl_xfer_req' declared 'static' but never defined [-Wunused-function]
    1141 | static inline void ufshcd_vops_compl_xfer_req(struct ufs_hba *hba,
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~
--
   In file included from drivers/scsi/ufs/ufshcd.c:18:
>> drivers/scsi/ufs/ufshcd.h:1143:1: error: expected identifier or '(' before '{' token
    1143 | {
         | ^
>> drivers/scsi/ufs/ufshcd.h:1141:20: warning: 'ufshcd_vops_compl_xfer_req' used but never defined
    1141 | static inline void ufshcd_vops_compl_xfer_req(struct ufs_hba *hba,
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~

vim +1143 drivers/scsi/ufs/ufshcd.h

  1140	
> 1141	static inline void ufshcd_vops_compl_xfer_req(struct ufs_hba *hba,
  1142						      int tag, bool is_scsi);
> 1143	{
  1144		if (hba->vops && hba->vops->compl_xfer_req)
  1145			hba->vops->compl_xfer_req(hba, tag, is_scsi);
  1146	}
  1147	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 65115 bytes --]

Re: [RFC PATCH v2 1/2] ufs: introduce a callback to get info of command completion

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2638 bytes --]

Hi Kiwoong,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on scsi/for-next]
[also build test ERROR on mkp-scsi/for-next v5.8-rc3 next-20200702]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kiwoong-Kim/ufs-introduce-callbacks-to-get-command-information/20200702-104850
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: arm-randconfig-r012-20200701 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 003a086ffc0d1affbb8300b36225fb8150a2d40a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from drivers/scsi/ufs/ufshcd.c:18:
>> drivers/scsi/ufs/ufshcd.h:1143:1: error: expected identifier or '('
   {
   ^
>> drivers/scsi/ufs/ufshcd.c:8671:44: warning: shift count >= width of type [-Wshift-count-overflow]
                   if (!dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(64)))
                                                            ^~~~~~~~~~~~~~~~
   include/linux/dma-mapping.h:139:54: note: expanded from macro 'DMA_BIT_MASK'
   #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
                                                        ^ ~~~
   1 warning and 1 error generated.
--
   In file included from drivers/scsi/ufs/ufs-sysfs.c:10:
   In file included from drivers/scsi/ufs/ufs-sysfs.h:10:
>> drivers/scsi/ufs/ufshcd.h:1143:1: error: expected identifier or '('
   {
   ^
   1 error generated.

vim +1143 drivers/scsi/ufs/ufshcd.h

  1140	
  1141	static inline void ufshcd_vops_compl_xfer_req(struct ufs_hba *hba,
  1142						      int tag, bool is_scsi);
> 1143	{
  1144		if (hba->vops && hba->vops->compl_xfer_req)
  1145			hba->vops->compl_xfer_req(hba, tag, is_scsi);
  1146	}
  1147	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28374 bytes --]

[RFC PATCH v2 2/2] exynos-ufs: implement dbg_register_dump and compl_xfer_req

[Kiwoong Kim]

This patch implements callbacks dbg_register_dump and
compl_xfer_req to store IO contexts or print them.

This includes functions to record contexts of incoming commands
in a circular queue. ufshcd.c has already some function
tracer calls to get command history but ftrace would be
gone when system dies before you get the information,
such as panic cases.

When you turn on CONFIG_SCSI_UFS_EXYNOS_CMD_LOG,
information in the circular queue whenever ufshcd.c
invokes a dbg_register_dump callback.

Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
 drivers/scsi/ufs/Kconfig          |  14 +++
 drivers/scsi/ufs/Makefile         |   2 +-
 drivers/scsi/ufs/ufs-exynos-dbg.c | 202 ++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-exynos-if.h  |  17 ++++
 drivers/scsi/ufs/ufs-exynos.c     |  63 +++++++++++-
 drivers/scsi/ufs/ufs-exynos.h     |  13 +++
 6 files changed, 308 insertions(+), 3 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufs-exynos-dbg.c
 create mode 100644 drivers/scsi/ufs/ufs-exynos-if.h

diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index 8cd9026..ebab446 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -172,3 +172,17 @@ config SCSI_UFS_EXYNOS
 
 	  Select this if you have UFS host controller on EXYNOS chipset.
 	  If unsure, say N.
+
+config SCSI_UFS_EXYNOS_CMD_LOG
+	bool "EXYNOS specific command log"
+	default n
+	depends on SCSI_UFS_EXYNOS
+	help
+	  This selects EXYNOS specific functions to get and even print
+	  some information to see what's happening at both command
+	  issue time completion time.
+	  The information may contain gerernal things as well as
+	  EXYNOS specific, such as vendor specific hardware contexts.
+
+	  Select this if you want to get and print the information.
+	  If unsure, say N.
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index f0c5b95..d9e4da7 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -4,7 +4,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o tc-dwc-g210.
 obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o tc-dwc-g210.o
 obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
 obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
-obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o
+obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o ufs-exynos-dbg.o
 obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
 ufshcd-core-y				+= ufshcd.o ufs-sysfs.o
 ufshcd-core-$(CONFIG_SCSI_UFS_BSG)	+= ufs_bsg.o
diff --git a/drivers/scsi/ufs/ufs-exynos-dbg.c b/drivers/scsi/ufs/ufs-exynos-dbg.c
new file mode 100644
index 0000000..062fb03
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-exynos-dbg.c
@@ -0,0 +1,202 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * UFS Exynos debugging functions
+ *
+ * Copyright (C) 2020 Samsung Electronics Co., Ltd.
+ * Author: Kiwoong Kim <kwmad.kim@samsung.com>
+ *
+ */
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include "ufshcd.h"
+#include "ufs-exynos.h"
+#include "ufs-exynos-if.h"
+
+#define MAX_CMD_LOGS    32
+
+struct cmd_data {
+	unsigned int tag;
+	unsigned int sct;
+	u64 lba;
+	u64 start_time;
+	u64 end_time;
+	u64 outstanding_reqs;
+	int retries;
+	u8 op;
+};
+
+struct ufs_cmd_info {
+	u32 total;
+	u32 last;
+	struct cmd_data data[MAX_CMD_LOGS];
+	struct cmd_data *pdata[MAX_CMD_LOGS];
+};
+
+/*
+ * This structure points out several contexts on debugging
+ * per one host instant.
+ * Now command history exists in here but later handle may
+ * contains some mmio base addresses including vendor specific
+ * regions to get hardware contexts.
+ */
+struct ufs_s_dbg_mgr {
+	struct ufs_exynos_handle *handle;
+	int active;
+	u64 first_time;
+	u64 time;
+
+	/* cmd log */
+	struct ufs_cmd_info cmd_info;
+	struct cmd_data cmd_log;		/* temp buffer to put */
+	spinlock_t cmd_lock;
+};
+
+static void ufs_s_print_cmd_log(struct ufs_s_dbg_mgr *mgr, struct device *dev)
+{
+	struct ufs_cmd_info *cmd_info = &mgr->cmd_info;
+	struct cmd_data *data = cmd_info->data;
+	u32 i;
+	u32 last;
+	u32 max = MAX_CMD_LOGS;
+	unsigned long flags;
+	u32 total;
+
+	spin_lock_irqsave(&mgr->cmd_lock, flags);
+	total = cmd_info->total;
+	if (cmd_info->total < max)
+		max = cmd_info->total;
+	last = (cmd_info->last + MAX_CMD_LOGS - 1) % MAX_CMD_LOGS;
+	spin_unlock_irqrestore(&mgr->cmd_lock, flags);
+
+	dev_err(dev, ":---------------------------------------------------\n");
+	dev_err(dev, ":\t\tSCSI CMD(%u)\n", total - 1);
+	dev_err(dev, ":---------------------------------------------------\n");
+	dev_err(dev, ":OP, TAG, LBA, SCT, RETRIES, STIME, ETIME, REQS\n\n");
+
+	for (i = 0 ; i < max ; i++, data++) {
+		dev_err(dev, ": 0x%02x, %02d, 0x%08lx, 0x%04x, %d, %llu, %llu, 0x%llx %s",
+			data->op, data->tag, data->lba, data->sct, data->retries,
+			data->start_time, data->end_time, data->outstanding_reqs,
+			((last == i) ? "<--" : " "));
+		if (last == i)
+			dev_err(dev, "\n");
+	}
+}
+
+static void ufs_s_put_cmd_log(struct ufs_s_dbg_mgr *mgr,
+			      struct cmd_data *cmd_data)
+{
+	struct ufs_cmd_info *cmd_info = &mgr->cmd_info;
+	unsigned long flags;
+	struct cmd_data *pdata;
+
+	spin_lock_irqsave(&mgr->cmd_lock, flags);
+	pdata = &cmd_info->data[cmd_info->last];
+	++cmd_info->total;
+	++cmd_info->last;
+	cmd_info->last = cmd_info->last % MAX_CMD_LOGS;
+	spin_unlock_irqrestore(&mgr->cmd_lock, flags);
+
+	pdata->op = cmd_data->op;
+	pdata->tag = cmd_data->tag;
+	pdata->lba = cmd_data->lba;
+	pdata->sct = cmd_data->sct;
+	pdata->retries = cmd_data->retries;
+	pdata->start_time = cmd_data->start_time;
+	pdata->end_time = 0;
+	pdata->outstanding_reqs = cmd_data->outstanding_reqs;
+	cmd_info->pdata[cmd_data->tag] = pdata;
+}
+
+/*
+ * EXTERNAL FUNCTIONS
+ *
+ * There are two classes that are to initialize data structures for debug
+ * and to define actual behavior.
+ */
+void exynos_ufs_dump_info(struct ufs_exynos_handle *handle, struct device *dev)
+{
+	struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private;
+
+	if (mgr->active == 0)
+		goto out;
+
+	mgr->time = cpu_clock(raw_smp_processor_id());
+
+#ifdef CONFIG_SCSI_UFS_EXYNOS_CMD_LOG
+	ufs_s_print_cmd_log(mgr, dev);
+#endif
+
+	if (mgr->first_time == 0ULL)
+		mgr->first_time = mgr->time;
+out:
+	return;
+}
+
+void exynos_ufs_cmd_log_start(struct ufs_exynos_handle *handle,
+			      struct ufs_hba *hba, struct scsi_cmnd *cmd)
+{
+	struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private;
+	int cpu = raw_smp_processor_id();
+	struct cmd_data *cmd_log = &mgr->cmd_log;	/* temp buffer to put */
+	u64 lba = (cmd->cmnd[2] << 24) |
+					(cmd->cmnd[3] << 16) |
+					(cmd->cmnd[4] << 8) |
+					(cmd->cmnd[5] << 0);
+	unsigned int sct = (cmd->cmnd[7] << 8) |
+					(cmd->cmnd[8] << 0);
+
+	if (mgr->active == 0)
+		return;
+
+	cmd_log->start_time = cpu_clock(cpu);
+	cmd_log->op = cmd->cmnd[0];
+	cmd_log->tag = cmd->request->tag;
+
+	/* This function runtime is protected by spinlock from outside */
+	cmd_log->outstanding_reqs = hba->outstanding_reqs;
+
+	/* unmap */
+	if (cmd->cmnd[0] != UNMAP)
+		cmd_log->lba = lba;
+
+	cmd_log->sct = sct;
+	cmd_log->retries = cmd->allowed;
+
+	ufs_s_put_cmd_log(mgr, cmd_log);
+}
+
+void exynos_ufs_cmd_log_end(struct ufs_exynos_handle *handle,
+			    struct ufs_hba *hba, struct scsi_cmnd *cmd)
+{
+	struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private;
+	struct ufs_cmd_info *cmd_info = &mgr->cmd_info;
+	int cpu = raw_smp_processor_id();
+	int tag = cmd->request->tag;
+
+	if (mgr->active == 0)
+		return;
+
+	cmd_info->pdata[tag]->end_time = cpu_clock(cpu);
+}
+
+int exynos_ufs_init_dbg(struct ufs_exynos_handle *handle, struct device *dev)
+{
+	struct ufs_s_dbg_mgr *mgr;
+
+	mgr = devm_kzalloc(dev, sizeof(struct ufs_s_dbg_mgr), GFP_KERNEL);
+	if (!ufs)
+		return -ENOMEM;
+	handle->private = (void *)mgr;
+	mgr->handle = handle;
+	mgr->active = 1;
+
+	/* cmd log */
+	spin_lock_init(&mgr->cmd_lock);
+
+	return 0;
+}
+MODULE_AUTHOR("Kiwoong Kim <kwmad.kim@samsung.com>");
+MODULE_DESCRIPTION("Exynos UFS debug information");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("0.1");
diff --git a/drivers/scsi/ufs/ufs-exynos-if.h b/drivers/scsi/ufs/ufs-exynos-if.h
new file mode 100644
index 0000000..c746f59
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-exynos-if.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * UFS Exynos debugging functions
+ *
+ * Copyright (C) 2020 Samsung Electronics Co., Ltd.
+ * Author: Kiwoong Kim <kwmad.kim@samsung.com>
+ *
+ */
+#ifndef _UFS_EXYNOS_IF_H_
+#define _UFS_EXYNOS_IF_H_
+
+/* more members would be added in the future */
+struct ufs_exynos_handle {
+	void *private;
+};
+
+#endif /* _UFS_EXYNOS_IF_H_ */
diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
index 440f2af..7f5142e 100644
--- a/drivers/scsi/ufs/ufs-exynos.c
+++ b/drivers/scsi/ufs/ufs-exynos.c
@@ -700,12 +700,31 @@ static int exynos_ufs_post_pwr_mode(struct ufs_hba *hba,
 	return 0;
 }
 
+#ifdef CONFIG_SCSI_UFS_EXYNOS_CMD_LOG
+static void exynos_ufs_cmd_log(struct ufs_hba *hba,
+					 struct scsi_cmnd *cmd, int enter)
+{
+	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
+	struct ufs_exynos_handle *handle = ufs->handle;
+
+	if (enter == 1)
+		exynos_ufs_cmd_log_start(handle, hba, cmd);
+	else if (enter == 2)
+		exynos_ufs_cmd_log_end(handle, hba, cmd);
+}
+#endif
+
 static void exynos_ufs_specify_nexus_t_xfer_req(struct ufs_hba *hba,
 						int tag, bool op)
 {
 	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
 	u32 type;
+#ifdef CONFIG_SCSI_UFS_EXYNOS_CMD_LOG
+	struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
 
+	if (op)
+		exynos_ufs_cmd_log(hba, cmd, 1);
+#endif
 	type =  hci_readl(ufs, HCI_UTRL_NEXUS_TYPE);
 
 	if (op)
@@ -714,8 +733,7 @@ static void exynos_ufs_specify_nexus_t_xfer_req(struct ufs_hba *hba,
 		hci_writel(ufs, type & ~(1 << tag), HCI_UTRL_NEXUS_TYPE);
 }
 
-static void exynos_ufs_specify_nexus_t_tm_req(struct ufs_hba *hba,
-						int tag, u8 func)
+static void exynos_ufs_specify_nexus_t_tm_req(struct ufs_hba *hba, int tag, u8 func)
 {
 	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
 	u32 type;
@@ -1008,6 +1026,12 @@ static int exynos_ufs_init(struct ufs_hba *hba)
 		goto out;
 	exynos_ufs_specify_phy_time_attr(ufs);
 	exynos_ufs_config_smu(ufs);
+
+	/* init dbg */
+	ret = exynos_ufs_init_dbg(ufs->handle, dev);
+	if (ret)
+		return ret;
+	spin_lock_init(&ufs->dbg_lock);
 	return 0;
 
 phy_off:
@@ -1210,6 +1234,39 @@ static int exynos_ufs_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	return 0;
 }
 
+static void exynos_ufs_dbg_register_dump(struct ufs_hba *hba)
+{
+	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ufs->dbg_lock, flags);
+	if (ufs->under_dump == 0)
+		ufs->under_dump = 1;
+	else {
+		spin_unlock_irqrestore(&ufs->dbg_lock, flags);
+		goto out;
+	}
+	spin_unlock_irqrestore(&ufs->dbg_lock, flags);
+
+	exynos_ufs_dump_info(ufs->handle, hba->dev);
+
+	spin_lock_irqsave(&ufs->dbg_lock, flags);
+	ufs->under_dump = 0;
+	spin_unlock_irqrestore(&ufs->dbg_lock, flags);
+out:
+	return;
+}
+
+static void exynos_ufs_compl_xfer_req(struct ufs_hba *hba, int tag, bool op)
+{
+#ifdef CONFIG_SCSI_UFS_EXYNOS_CMD_LOG
+	struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
+
+	if (op)
+		exynos_ufs_cmd_log(hba, cmd, 2);
+#endif
+}
+
 static struct ufs_hba_variant_ops ufs_hba_exynos_ops = {
 	.name				= "exynos_ufs",
 	.init				= exynos_ufs_init,
@@ -1221,6 +1278,8 @@ static struct ufs_hba_variant_ops ufs_hba_exynos_ops = {
 	.hibern8_notify			= exynos_ufs_hibern8_notify,
 	.suspend			= exynos_ufs_suspend,
 	.resume				= exynos_ufs_resume,
+	.dbg_register_dump		= exynos_ufs_dbg_register_dump,
+	.compl_xfer_req			= exynos_ufs_compl_xfer_req,
 };
 
 static int exynos_ufs_probe(struct platform_device *pdev)
diff --git a/drivers/scsi/ufs/ufs-exynos.h b/drivers/scsi/ufs/ufs-exynos.h
index 76d6e39..0c79a30 100644
--- a/drivers/scsi/ufs/ufs-exynos.h
+++ b/drivers/scsi/ufs/ufs-exynos.h
@@ -8,6 +8,7 @@
 
 #ifndef _UFS_EXYNOS_H_
 #define _UFS_EXYNOS_H_
+#include "ufs-exynos-if.h"
 
 /*
  * UNIPRO registers
@@ -212,6 +213,10 @@ struct exynos_ufs {
 #define EXYNOS_UFS_OPT_BROKEN_AUTO_CLK_CTRL	BIT(2)
 #define EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX	BIT(3)
 #define EXYNOS_UFS_OPT_USE_SW_HIBERN8_TIMER	BIT(4)
+
+	struct ufs_exynos_handle *handle;
+	spinlock_t dbg_lock;
+	int under_dump;
 };
 
 #define for_each_ufs_rx_lane(ufs, i) \
@@ -284,4 +289,12 @@ struct exynos_ufs_uic_attr exynos7_uic_attr = {
 	.rx_hs_g3_prep_sync_len_cap	= PREP_LEN(0xf),
 	.pa_dbg_option_suite		= 0x30103,
 };
+
+/* public function declarations */
+void exynos_ufs_dump_info(struct ufs_exynos_handle *handle, struct device *dev);
+void exynos_ufs_cmd_log_start(struct ufs_exynos_handle *handle,
+			      struct ufs_hba *hba, struct scsi_cmnd *cmd);
+void exynos_ufs_cmd_log_end(struct ufs_exynos_handle *handle,
+			    struct ufs_hba *hba, struct scsi_cmnd *cmd);
+int exynos_ufs_init_dbg(struct ufs_exynos_handle *handle, struct device *dev);
 #endif /* _UFS_EXYNOS_H_ */
-- 
2.7.4

Re: [RFC PATCH v2 2/2] exynos-ufs: implement dbg_register_dump and compl_xfer_req

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 6415 bytes --]

Hi Kiwoong,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on scsi/for-next]
[also build test ERROR on mkp-scsi/for-next next-20200701]
[cannot apply to v5.8-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kiwoong-Kim/ufs-introduce-callbacks-to-get-command-information/20200702-104850
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: parisc-allmodconfig (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from drivers/scsi/ufs/ufs-exynos.c:18:
   drivers/scsi/ufs/ufshcd.h:1143:1: error: expected identifier or '(' before '{' token
    1143 | {
         | ^
   drivers/scsi/ufs/ufs-exynos.c: In function 'exynos_ufs_specify_nexus_t_xfer_req':
>> drivers/scsi/ufs/ufs-exynos.c:723:26: error: initialization of 'struct scsi_cmnd *' from incompatible pointer type 'struct scsi_cmnd **' [-Werror=incompatible-pointer-types]
     723 |  struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
         |                          ^
>> drivers/scsi/ufs/ufs-exynos.c:725:2: error: expected ',' or ';' before 'if'
     725 |  if (op)
         |  ^~
   drivers/scsi/ufs/ufs-exynos.c:723:20: warning: unused variable 'cmd' [-Wunused-variable]
     723 |  struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
         |                    ^~~
   drivers/scsi/ufs/ufs-exynos.c: In function 'exynos_ufs_compl_xfer_req':
   drivers/scsi/ufs/ufs-exynos.c:1263:26: error: initialization of 'struct scsi_cmnd *' from incompatible pointer type 'struct scsi_cmnd **' [-Werror=incompatible-pointer-types]
    1263 |  struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
         |                          ^
   drivers/scsi/ufs/ufs-exynos.c:1265:2: error: expected ',' or ';' before 'if'
    1265 |  if (op)
         |  ^~
   drivers/scsi/ufs/ufs-exynos.c:1263:20: warning: unused variable 'cmd' [-Wunused-variable]
    1263 |  struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
         |                    ^~~
   In file included from drivers/scsi/ufs/ufs-exynos.c:18:
   drivers/scsi/ufs/ufs-exynos.c: At top level:
   drivers/scsi/ufs/ufshcd.h:1141:20: warning: 'ufshcd_vops_compl_xfer_req' declared 'static' but never defined [-Wunused-function]
    1141 | static inline void ufshcd_vops_compl_xfer_req(struct ufs_hba *hba,
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/scsi/ufs/ufs-exynos.c:704:13: warning: 'exynos_ufs_cmd_log' defined but not used [-Wunused-function]
     704 | static void exynos_ufs_cmd_log(struct ufs_hba *hba,
         |             ^~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   In file included from drivers/scsi/ufs/ufs-exynos-dbg.c:11:
   drivers/scsi/ufs/ufshcd.h:1143:1: error: expected identifier or '(' before '{' token
    1143 | {
         | ^
   In file included from include/linux/device.h:15,
                    from include/linux/platform_device.h:13,
                    from drivers/scsi/ufs/ufs-exynos-dbg.c:9:
   drivers/scsi/ufs/ufs-exynos-dbg.c: In function 'ufs_s_print_cmd_log':
>> drivers/scsi/ufs/ufs-exynos-dbg.c:77:16: warning: format '%lx' expects argument of type 'long unsigned int', but argument 5 has type 'u64' {aka 'long long unsigned int'} [-Wformat=]
      77 |   dev_err(dev, ": 0x%02x, %02d, 0x%08lx, 0x%04x, %d, %llu, %llu, 0x%llx %s",
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
      19 | #define dev_fmt(fmt) fmt
         |                      ^~~
>> drivers/scsi/ufs/ufs-exynos-dbg.c:77:3: note: in expansion of macro 'dev_err'
      77 |   dev_err(dev, ": 0x%02x, %02d, 0x%08lx, 0x%04x, %d, %llu, %llu, 0x%llx %s",
         |   ^~~~~~~
   drivers/scsi/ufs/ufs-exynos-dbg.c:77:39: note: format string is defined here
      77 |   dev_err(dev, ": 0x%02x, %02d, 0x%08lx, 0x%04x, %d, %llu, %llu, 0x%llx %s",
         |                                   ~~~~^
         |                                       |
         |                                       long unsigned int
         |                                   %08llx
   drivers/scsi/ufs/ufs-exynos-dbg.c: In function 'exynos_ufs_init_dbg':
>> drivers/scsi/ufs/ufs-exynos-dbg.c:188:7: error: 'ufs' undeclared (first use in this function); did you mean 'ffs'?
     188 |  if (!ufs)
         |       ^~~
         |       ffs
   drivers/scsi/ufs/ufs-exynos-dbg.c:188:7: note: each undeclared identifier is reported only once for each function it appears in
   In file included from drivers/scsi/ufs/ufs-exynos-dbg.c:11:
   drivers/scsi/ufs/ufs-exynos-dbg.c: At top level:
   drivers/scsi/ufs/ufshcd.h:1141:20: warning: 'ufshcd_vops_compl_xfer_req' declared 'static' but never defined [-Wunused-function]
    1141 | static inline void ufshcd_vops_compl_xfer_req(struct ufs_hba *hba,
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~

vim +723 drivers/scsi/ufs/ufs-exynos.c

   716	
   717	static void exynos_ufs_specify_nexus_t_xfer_req(struct ufs_hba *hba,
   718							int tag, bool op)
   719	{
   720		struct exynos_ufs *ufs = ufshcd_get_variant(hba);
   721		u32 type;
   722	#ifdef CONFIG_SCSI_UFS_EXYNOS_CMD_LOG
 > 723		struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
   724	
 > 725		if (op)
   726			exynos_ufs_cmd_log(hba, cmd, 1);
   727	#endif
   728		type =  hci_readl(ufs, HCI_UTRL_NEXUS_TYPE);
   729	
   730		if (op)
   731			hci_writel(ufs, type | (1 << tag), HCI_UTRL_NEXUS_TYPE);
   732		else
   733			hci_writel(ufs, type & ~(1 << tag), HCI_UTRL_NEXUS_TYPE);
   734	}
   735	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 65122 bytes --]

Re: [RFC PATCH v2 2/2] exynos-ufs: implement dbg_register_dump and compl_xfer_req

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4805 bytes --]

Hi Kiwoong,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on scsi/for-next]
[also build test ERROR on mkp-scsi/for-next next-20200702]
[cannot apply to v5.8-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kiwoong-Kim/ufs-introduce-callbacks-to-get-command-information/20200702-104850
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 003a086ffc0d1affbb8300b36225fb8150a2d40a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from drivers/scsi/ufs/ufs-exynos.c:18:
   drivers/scsi/ufs/ufshcd.h:1143:1: error: expected identifier or '('
   {
   ^
>> drivers/scsi/ufs/ufs-exynos.c:723:20: error: incompatible pointer types initializing 'struct scsi_cmnd *' with an expression of type 'struct scsi_cmnd **'; remove & [-Werror,-Wincompatible-pointer-types]
           struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
                             ^     ~~~~~~~~~~~~~~~~~~
>> drivers/scsi/ufs/ufs-exynos.c:723:44: error: expected ';' at end of declaration
           struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
                                                     ^
                                                     ;
   drivers/scsi/ufs/ufs-exynos.c:1263:20: error: incompatible pointer types initializing 'struct scsi_cmnd *' with an expression of type 'struct scsi_cmnd **'; remove & [-Werror,-Wincompatible-pointer-types]
           struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
                             ^     ~~~~~~~~~~~~~~~~~~
   drivers/scsi/ufs/ufs-exynos.c:1263:44: error: expected ';' at end of declaration
           struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
                                                     ^
                                                     ;
   5 errors generated.
--
   In file included from drivers/scsi/ufs/ufs-exynos-dbg.c:11:
   drivers/scsi/ufs/ufshcd.h:1143:1: error: expected identifier or '('
   {
   ^
>> drivers/scsi/ufs/ufs-exynos-dbg.c:78:25: warning: format specifies type 'unsigned long' but the argument has type 'u64' (aka 'unsigned long long') [-Wformat]
                           data->op, data->tag, data->lba, data->sct, data->retries,
                                                ^~~~~~~~~
   include/linux/dev_printk.h:104:32: note: expanded from macro 'dev_err'
           _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
                                 ~~~     ^~~~~~~~~~~
>> drivers/scsi/ufs/ufs-exynos-dbg.c:188:7: error: use of undeclared identifier 'ufs'; did you mean 'ffs'?
           if (!ufs)
                ^~~
                ffs
   arch/x86/include/asm/bitops.h:287:28: note: 'ffs' declared here
   static __always_inline int ffs(int x)
                              ^
>> drivers/scsi/ufs/ufs-exynos-dbg.c:188:7: warning: address of function 'ffs' will always evaluate to 'true' [-Wpointer-bool-conversion]
           if (!ufs)
               ~^~~
   drivers/scsi/ufs/ufs-exynos-dbg.c:188:7: note: prefix with the address-of operator to silence this warning
           if (!ufs)
                ^
                &
   2 warnings and 2 errors generated.

vim +723 drivers/scsi/ufs/ufs-exynos.c

   716	
   717	static void exynos_ufs_specify_nexus_t_xfer_req(struct ufs_hba *hba,
   718							int tag, bool op)
   719	{
   720		struct exynos_ufs *ufs = ufshcd_get_variant(hba);
   721		u32 type;
   722	#ifdef CONFIG_SCSI_UFS_EXYNOS_CMD_LOG
 > 723		struct scsi_cmnd *cmd = &hba->lrb[tag].cmd
   724	
   725		if (op)
   726			exynos_ufs_cmd_log(hba, cmd, 1);
   727	#endif
   728		type =  hci_readl(ufs, HCI_UTRL_NEXUS_TYPE);
   729	
   730		if (op)
   731			hci_writel(ufs, type | (1 << tag), HCI_UTRL_NEXUS_TYPE);
   732		else
   733			hci_writel(ufs, type & ~(1 << tag), HCI_UTRL_NEXUS_TYPE);
   734	}
   735	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 75321 bytes --]

Re: [RFC PATCH v2 0/2] ufs: introduce callbacks to get command information

[Stanley Chu]

Hi Kiwoong,

On Thu, 2020-07-02 at 11:38 +0900, Kiwoong Kim wrote:
> Some SoC specific might need command history for
> various reasons, such as stacking command contexts
> in system memory to check for debugging in the future
> or scaling some DVFS knobs to boost IO throughput.
> 
> What you would do with the information could be
> variant per SoC vendor.
> 
> Kiwoong Kim (2):
>   ufs: introduce a callback to get info of command completion
>   exynos-ufs: implement dbg_register_dump and compl_xfer_req

Thanks for the update!

Would you please elaborate the change log in cover letter for easier
review in the future?

Thanks,
Stanley Chu

RE: [RFC PATCH v2 0/2] ufs: introduce callbacks to get command information

[Kiwoong Kim]

> Hi Kiwoong,
> 
> On Thu, 2020-07-02 at 11:38 +0900, Kiwoong Kim wrote:
> > Some SoC specific might need command history for various reasons, such
> > as stacking command contexts in system memory to check for debugging
> > in the future or scaling some DVFS knobs to boost IO throughput.
> >
> > What you would do with the information could be variant per SoC
> > vendor.
> >
> > Kiwoong Kim (2):
> >   ufs: introduce a callback to get info of command completion
> >   exynos-ufs: implement dbg_register_dump and compl_xfer_req
> 
> Thanks for the update!
> 
> Would you please elaborate the change log in cover letter for easier
> review in the future?
> 
> Thanks,
> Stanley Chu
> 

I'll keep in mind.

Thanks.
Kiwoong Kim