Skip to contentSkip to conversation

Add ioctl and debug utilities to UFS driver

PATCH Superseded Gilad Broner 2 participants 6 messages Mar 12, 2015

Subsystems (4)

v6 of 9 Superseded Latest is v9 →

lore
Share & tools
lore
#0 lore raw

v6 cover Add ioctl and debug utilities to UFS driver

Changes from V5:
Created a common trace event class for driver state changes instead of individual trace events definitions.

Dolev Raviv (1):
scsi: ufs: add ioctl interface for query request

Gilad Broner (1):
scsi: ufs: add trace events and dump prints for debug

Lee Susman (1):
scsi: ufs: add debugfs for ufs

 drivers/scsi/ufs/Makefile      |   1 +
 drivers/scsi/ufs/ufs-debugfs.c | 902 ++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-debugfs.h |  38 ++
 drivers/scsi/ufs/ufs-qcom.c    |  53 +++
 drivers/scsi/ufs/ufs.h         |  53 +--
 drivers/scsi/ufs/ufshcd.c      | 963 +++++++++++++++++++++++++++++++++++++++--
 drivers/scsi/ufs/ufshcd.h      | 110 +++++
 drivers/scsi/ufs/ufshci.h      |   3 +
 include/trace/events/ufs.h     | 213 +++++++++
 include/uapi/scsi/Kbuild       |   1 +
 include/uapi/scsi/ufs/Kbuild   |   3 +
 include/uapi/scsi/ufs/ioctl.h  |  57 +++
 include/uapi/scsi/ufs/ufs.h    |  66 +++
 13 files changed, 2373 insertions(+), 90 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufs-debugfs.c
 create mode 100644 drivers/scsi/ufs/ufs-debugfs.h
 create mode 100644 include/trace/events/ufs.h
 create mode 100644 include/uapi/scsi/ufs/Kbuild
 create mode 100644 include/uapi/scsi/ufs/ioctl.h
 create mode 100644 include/uapi/scsi/ufs/ufs.h
Signature · 3 lines
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw

PATCH v6 1/3 scsi: ufs: add ioctl interface for query request

From: Dolev Raviv <[email protected]>

This patch exposes the ioctl interface for UFS driver via SCSI device ioctl interface. As of now UFS driver would provide the ioctl for query interface to connected UFS device.

---
 drivers/scsi/ufs/ufs.h        |  53 +++-------
 drivers/scsi/ufs/ufshcd.c     | 225 +++++++++++++++++++++++++++++++++++++++++-
 include/uapi/scsi/Kbuild      |   1 +
 include/uapi/scsi/ufs/Kbuild  |   3 +
 include/uapi/scsi/ufs/ioctl.h |  57 +++++++++++
 include/uapi/scsi/ufs/ufs.h   |  66 +++++++++++++
 6 files changed, 361 insertions(+), 44 deletions(-)
 create mode 100644 include/uapi/scsi/ufs/Kbuild
 create mode 100644 include/uapi/scsi/ufs/ioctl.h
 create mode 100644 include/uapi/scsi/ufs/ufs.h
drivers/scsi/ufs/ufs.h +11 -42
index 42c459a..1f023c4 100644
@@ -38,6 +38,7 @@
 
 #include <linux/mutex.h>
 #include <linux/types.h>
+#include <scsi/ufs/ufs.h>
 
 #define MAX_CDB_SIZE	16
 #define GENERAL_UPIU_REQUEST_SIZE 32
@@ -71,6 +72,16 @@ enum {
 	UFS_UPIU_RPMB_WLUN		= 0xC4,
 };
 
+/**
+ * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
+ * @lun: LU number to check
+ * @return: true if the lun has a matching unit descriptor, false otherwise
+ */
+static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
+{
+	return (lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN));
+}
+
 /*
  * UFS Protocol Information Unit related definitions
  */
@@ -126,35 +137,6 @@ enum {
 	UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST          = 0x81,
 };
 
-/* Flag idn for Query Requests*/
-enum flag_idn {
-	QUERY_FLAG_IDN_FDEVICEINIT      = 0x01,
-	QUERY_FLAG_IDN_PWR_ON_WPE	= 0x03,
-	QUERY_FLAG_IDN_BKOPS_EN         = 0x04,
-};
-
-/* Attribute idn for Query requests */
-enum attr_idn {
-	QUERY_ATTR_IDN_ACTIVE_ICC_LVL	= 0x03,
-	QUERY_ATTR_IDN_BKOPS_STATUS	= 0x05,
-	QUERY_ATTR_IDN_EE_CONTROL	= 0x0D,
-	QUERY_ATTR_IDN_EE_STATUS	= 0x0E,
-};
-
-/* Descriptor idn for Query requests */
-enum desc_idn {
-	QUERY_DESC_IDN_DEVICE		= 0x0,
-	QUERY_DESC_IDN_CONFIGURAION	= 0x1,
-	QUERY_DESC_IDN_UNIT		= 0x2,
-	QUERY_DESC_IDN_RFU_0		= 0x3,
-	QUERY_DESC_IDN_INTERCONNECT	= 0x4,
-	QUERY_DESC_IDN_STRING		= 0x5,
-	QUERY_DESC_IDN_RFU_1		= 0x6,
-	QUERY_DESC_IDN_GEOMETRY		= 0x7,
-	QUERY_DESC_IDN_POWER		= 0x8,
-	QUERY_DESC_IDN_MAX,
-};
-
 enum desc_header_offset {
 	QUERY_DESC_LENGTH_OFFSET	= 0x00,
 	QUERY_DESC_DESC_TYPE_OFFSET	= 0x01,
@@ -247,19 +229,6 @@ enum bkops_status {
 	BKOPS_STATUS_MAX		 = BKOPS_STATUS_CRITICAL,
 };
 
-/* UTP QUERY Transaction Specific Fields OpCode */
-enum query_opcode {
-	UPIU_QUERY_OPCODE_NOP		= 0x0,
-	UPIU_QUERY_OPCODE_READ_DESC	= 0x1,
-	UPIU_QUERY_OPCODE_WRITE_DESC	= 0x2,
-	UPIU_QUERY_OPCODE_READ_ATTR	= 0x3,
-	UPIU_QUERY_OPCODE_WRITE_ATTR	= 0x4,
-	UPIU_QUERY_OPCODE_READ_FLAG	= 0x5,
-	UPIU_QUERY_OPCODE_SET_FLAG	= 0x6,
-	UPIU_QUERY_OPCODE_CLEAR_FLAG	= 0x7,
-	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
-};
-
 /* Query response result code */
 enum {
 	QUERY_RESULT_SUCCESS                    = 0x00,
drivers/scsi/ufs/ufshcd.c +223 -2

261 lines not on this page · raw message

include/uapi/scsi/Kbuild +1 -0
index 75746d5..d404525 100644
@@ -1,5 +1,6 @@
 # UAPI Header export list
 header-y += fc/
+header-y += ufs/
 header-y += scsi_bsg_fc.h
 header-y += scsi_netlink.h
 header-y += scsi_netlink_fc.h
include/uapi/scsi/ufs/Kbuild new file +3 -0
new file mode 100644
index 0000000..cc3ef20
@@ -0,0 +1,3 @@
+# UAPI Header export list
+header-y += ioctl.h
+header-y += ufs.h
include/uapi/scsi/ufs/ioctl.h new file +57 -0

57 lines not on this page · raw message

include/uapi/scsi/ufs/ufs.h new file +66 -0

66 lines not on this page · raw message

Signature · 3 lines
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#2 lore raw

Re: [PATCH v6 1/3] scsi: ufs: add ioctl interface for query request

2015-03-12 21:27 GMT+09:00 Gilad Broner <[email protected]>:

Quoted 10 lines

From: Dolev Raviv <[email protected]>

This patch exposes the ioctl interface for UFS driver via SCSI device ioctl interface. As of now UFS driver would provide the ioctl for query interface to connected UFS device.

Sorry to bother you again. Could you read two comments below?

Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +10 -0
+/**
+ * ufshcd_ioctl - ufs ioctl callback registered in scsi_host
+ * @dev: scsi device required for per LUN queries
+ * @cmd: command opcode
+ * @buffer: user space buffer for transferring data
+ *
+ * Supported commands:
+ * UFS_IOCTL_QUERY
+ */
+static int ufshcd_ioctl(struct scsi_device *dev, int cmd, void __user *buffer)

drivers/scsi/ufs/ufshcd.c static void ufshcd_async_scan(void *data, async_cookie_t cookie) · line 4403

Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +10 -0
+{
+       struct ufs_hba *hba = shost_priv(dev->host);
+       int err = 0;
+
+       BUG_ON(!hba);
+       if (!buffer) {
+               dev_err(hba->dev, "%s: User buffer is NULL!\n", __func__);
+               return -EINVAL;
+       }
+

Should we remove this check or move it into ufshcd_query_ioctl()? For example, BLKFLS ioctl without argument is correct usage, but it always triggers this message. (blkdev_ioctl -> __blkdev_driver_ioctl -> sd_ioctl -> scsi_ioctl -> ufshcd_ioctl)

Quoted 31 lines
drivers/scsi/ufs/ufshcd.c +21 -0
+       switch (cmd) {
+       case UFS_IOCTL_QUERY:
+               pm_runtime_get_sync(hba->dev);
+               err = ufshcd_query_ioctl(hba, ufshcd_scsi_to_upiu_lun(dev->lun),
+                               buffer);
+               pm_runtime_put_sync(hba->dev);
+               break;
+       case UFS_IOCTL_BLKROSET:
+               err = -ENOIOCTLCMD;
+               break;
+       default:
+               err = -EINVAL;
+               dev_err(hba->dev, "%s: Illegal ufs-IOCTL cmd %d\n", __func__,
+                               cmd);
+               break;
+       }
+
+       return err;
+}
+
 static struct scsi_host_template ufshcd_driver_template = {
        .module                 = THIS_MODULE,
        .name                   = UFSHCD,
@@ -4213,6 +4433,7 @@ static struct scsi_host_template ufshcd_driver_template = {
        .eh_abort_handler       = ufshcd_abort,
        .eh_device_reset_handler = ufshcd_eh_device_reset_handler,
        .eh_host_reset_handler   = ufshcd_eh_host_reset_handler,
+       .ioctl                  = ufshcd_ioctl,
        .this_id                = -1,
        .sg_tablesize           = SG_ALL,
        .cmd_per_lun            = UFSHCD_CMD_PER_LUN,

...

Quoted 6 lines
include/uapi/scsi/ufs/ioctl.h new file +0 -0
new file mode 100644
index 0000000..bc4eed7

include/uapi/scsi/ufs/ioctl.h line 10

Quoted 10 lines
include/uapi/scsi/ufs/ioctl.h new file +10 -0
@@ -0,0 +1,57 @@
+#ifndef UAPI_UFS_IOCTL_H_
+#define UAPI_UFS_IOCTL_H_
+
+#include <linux/types.h>
+
+/*
+ *  IOCTL opcode for ufs queries has the following opcode after
+ *  SCSI_IOCTL_GET_PCI
+ */
+#define UFS_IOCTL_QUERY                        0x5388

Should we also need some comments near SCSI_IOCTL_GET_PCI in include/scsi/scsi.h in order to avoid someone trying to define the same ioctl code in the future?

#3 lore raw

Re: [PATCH v6 1/3] scsi: ufs: add ioctl interface for query request

Quoted 11 lines

drivers/scsi/ufs/ufshcd.c static void ufshcd_async_scan(void *data, async_cookie_t cookie) · line 4403

Quoted 6 lines
(quoted diff) +2 -0
+       if (!buffer) {
+               dev_err(hba->dev, "%s: User buffer is NULL!\n",

__func__);

drivers/scsi/ufs/ufshcd.c +3 -0
+               return -EINVAL;
+       }
+

Should we remove this check or move it into ufshcd_query_ioctl()? For example, BLKFLS ioctl without argument is correct usage, but it always triggers this message. (blkdev_ioctl -> __blkdev_driver_ioctl -> sd_ioctl -> scsi_ioctl -> ufshcd_ioctl)

You're right, I'll move the check to ufshcd_query_ioctl().

Quoted 17 lines

+++ b/include/uapi/scsi/ufs/ioctl.h

include/uapi/scsi/ufs/ioctl.h line 10

Quoted 11 lines
include/uapi/scsi/ufs/ioctl.h new file +10 -0
@@ -0,0 +1,57 @@
+#ifndef UAPI_UFS_IOCTL_H_
+#define UAPI_UFS_IOCTL_H_
+
+#include <linux/types.h>
+
+/*
+ *  IOCTL opcode for ufs queries has the following opcode after
+ *  SCSI_IOCTL_GET_PCI
+ */
+#define UFS_IOCTL_QUERY                        0x5388

Should we also need some comments near SCSI_IOCTL_GET_PCI in include/scsi/scsi.h in order to avoid someone trying to define the same ioctl code in the future?

Indeed - I will add a comment.

Gilad.

Signature · 3 lines
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#4 lore raw

PATCH v6 2/3 scsi: ufs: add debugfs for ufs

In reply to Gilad Broner

From: Lee Susman <[email protected]>

Adding debugfs capability for ufshcd.

debugfs attributes introduced in this patch:
- View driver/controller runtime data
- Command tag statistics for performance analisis
- Dump device descriptor info
- Track recoverable errors statistics during runtime
- Change UFS power mode during runtime
entry a string in the format 'GGLLMM' where:
G - selected gear
L - number of lanes
M - power mode
(1=fast mode, 2=slow mode, 4=fast-auto mode,
5=slow-auto mode)
First letter is for RX, second is for TX.
- Get/set DME attributes

---
 drivers/scsi/ufs/Makefile      |   1 +
 drivers/scsi/ufs/ufs-debugfs.c | 902 +++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-debugfs.h |  38 ++
 drivers/scsi/ufs/ufshcd.c      | 229 ++++++++++-
 drivers/scsi/ufs/ufshcd.h      |  65 +++
 drivers/scsi/ufs/ufshci.h      |   2 +
 6 files changed, 1225 insertions(+), 12 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufs-debugfs.c
 create mode 100644 drivers/scsi/ufs/ufs-debugfs.h
drivers/scsi/ufs/Makefile +1 -0
index 8303bcc..0692314 100644
@@ -3,3 +3,4 @@ obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
 obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
 obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
 obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
+obj-$(CONFIG_DEBUG_FS) += ufs-debugfs.o
drivers/scsi/ufs/ufs-debugfs.c new file +902 -0

902 lines not on this page · raw message

drivers/scsi/ufs/ufs-debugfs.h new file +38 -0
new file mode 100644
index 0000000..7ed308d
@@ -0,0 +1,38 @@
+/* Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * UFS debugfs - add debugfs interface to the ufshcd.
+ * This is currently used for statistics collection and exporting from the
+ * UFS driver.
+ * This infrastructure can be used for debugging or direct tweaking
+ * of the driver from userspace.
+ *
+ */
+
+#ifndef _UFS_DEBUGFS_H
+#define _UFS_DEBUGFS_H
+
+#include <linux/debugfs.h>
+#include "ufshcd.h"
+
+#ifdef CONFIG_DEBUG_FS
+void ufsdbg_add_debugfs(struct ufs_hba *hba);
+void ufsdbg_remove_debugfs(struct ufs_hba *hba);
+#else
+static inline void ufsdbg_add_debugfs(struct ufs_hba *hba)
+{
+}
+static inline void ufsdbg_remove_debugfs(struct ufs_hba *hba)
+{
+}
+#endif
+
+#endif /* End of Header */
drivers/scsi/ufs/ufshcd.c +217 -12

399 lines not on this page · raw message

drivers/scsi/ufs/ufshcd.h +65 -0

99 lines not on this page · raw message

drivers/scsi/ufs/ufshci.h +2 -0
index d572119..c8b178f 100644
@@ -72,6 +72,8 @@ enum {
 	REG_UIC_COMMAND_ARG_1			= 0x94,
 	REG_UIC_COMMAND_ARG_2			= 0x98,
 	REG_UIC_COMMAND_ARG_3			= 0x9C,
+
+	UFSHCI_REG_SPACE_SIZE			= 0xA0,
 };
 
 /* Controller capability masks */
Signature · 3 lines
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#5 lore raw

PATCH v6 3/3 scsi: ufs: add trace events and dump prints for debug

In reply to Gilad Broner

Add trace events to driver to allow monitoring and profilig of activities such as PM suspend/resume, hibernate enter/exit, clock gating and clock scaling up/down.
In addition, add UFS host controller register dumps to provide detailed information in case of errors to assist in analysis of issues.

---
 drivers/scsi/ufs/ufs-qcom.c |  53 +++++
 drivers/scsi/ufs/ufshcd.c   | 511 +++++++++++++++++++++++++++++++++++++++++---
 drivers/scsi/ufs/ufshcd.h   |  49 ++++-
 drivers/scsi/ufs/ufshci.h   |   1 +
 include/trace/events/ufs.h  | 213 ++++++++++++++++++
 5 files changed, 790 insertions(+), 37 deletions(-)
 create mode 100644 include/trace/events/ufs.h
drivers/scsi/ufs/ufs-qcom.c +53 -0
index 9217af9..9fe675d 100644
@@ -30,6 +30,14 @@ static int ufs_qcom_get_bus_vote(struct ufs_qcom_host *host,
 		const char *speed_mode);
 static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote);
 
+static void ufs_qcom_dump_regs(struct ufs_hba *hba, int offset, int len,
+		char *prefix)
+{
+	print_hex_dump(KERN_ERR, prefix,
+			len > 4 ? DUMP_PREFIX_OFFSET : DUMP_PREFIX_NONE,
+			16, 4, hba->mmio_base + offset, len * 4, false);
+}
+
 static int ufs_qcom_get_connected_tx_lanes(struct ufs_hba *hba, u32 *tx_lanes)
 {
 	int err = 0;
@@ -983,6 +991,50 @@ void ufs_qcom_clk_scale_notify(struct ufs_hba *hba)
 				dev_req_params->hs_rate);
 }
 
+static void ufs_qcom_print_hw_debug_reg_all(struct ufs_hba *hba)
+{
+	u32 reg;
+
+	ufs_qcom_dump_regs(hba, UFS_UFS_DBG_RD_REG_OCSC, 44,
+			"UFS_UFS_DBG_RD_REG_OCSC ");
+
+	reg = ufshcd_readl(hba, REG_UFS_CFG1);
+	reg |= UFS_BIT(17);
+	ufshcd_writel(hba, reg, REG_UFS_CFG1);
+
+	ufs_qcom_dump_regs(hba, UFS_UFS_DBG_RD_EDTL_RAM, 32,
+			"UFS_UFS_DBG_RD_EDTL_RAM ");
+	ufs_qcom_dump_regs(hba, UFS_UFS_DBG_RD_DESC_RAM, 128,
+			"UFS_UFS_DBG_RD_DESC_RAM ");
+	ufs_qcom_dump_regs(hba, UFS_UFS_DBG_RD_PRDT_RAM, 64,
+			"UFS_UFS_DBG_RD_PRDT_RAM ");
+
+	ufshcd_writel(hba, (reg & ~UFS_BIT(17)), REG_UFS_CFG1);
+
+	ufs_qcom_dump_regs(hba, UFS_DBG_RD_REG_UAWM, 4,
+			"UFS_DBG_RD_REG_UAWM ");
+	ufs_qcom_dump_regs(hba, UFS_DBG_RD_REG_UARM, 4,
+			"UFS_DBG_RD_REG_UARM ");
+	ufs_qcom_dump_regs(hba, UFS_DBG_RD_REG_TXUC, 48,
+			"UFS_DBG_RD_REG_TXUC ");
+	ufs_qcom_dump_regs(hba, UFS_DBG_RD_REG_RXUC, 27,
+			"UFS_DBG_RD_REG_RXUC ");
+	ufs_qcom_dump_regs(hba, UFS_DBG_RD_REG_DFC, 19,
+			"UFS_DBG_RD_REG_DFC ");
+	ufs_qcom_dump_regs(hba, UFS_DBG_RD_REG_TRLUT, 34,
+			"UFS_DBG_RD_REG_TRLUT ");
+	ufs_qcom_dump_regs(hba, UFS_DBG_RD_REG_TMRLUT, 9,
+			"UFS_DBG_RD_REG_TMRLUT ");
+}
+
+static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)
+{
+	ufs_qcom_dump_regs(hba, REG_UFS_SYS1CLK_1US, 5,
+			"REG_UFS_SYS1CLK_1US ");
+
+	ufs_qcom_print_hw_debug_reg_all(hba);
+}
+
 /**
  * struct ufs_hba_qcom_vops - UFS QCOM specific variant operations
  *
@@ -1000,5 +1052,6 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
 	.pwr_change_notify	= ufs_qcom_pwr_change_notify,
 	.suspend		= ufs_qcom_suspend,
 	.resume			= ufs_qcom_resume,
+	.dbg_register_dump	= ufs_qcom_dump_dbg_regs,
 };
 EXPORT_SYMBOL(ufs_hba_qcom_vops);
drivers/scsi/ufs/ufshcd.c +476 -35

915 lines not on this page · raw message

drivers/scsi/ufs/ufshcd.h +47 -2

95 lines not on this page · raw message

drivers/scsi/ufs/ufshci.h +1 -0
index c8b178f..c5a0d19 100644
@@ -166,6 +166,7 @@ enum {
 /* UECPA - Host UIC Error Code PHY Adapter Layer 38h */
 #define UIC_PHY_ADAPTER_LAYER_ERROR			UFS_BIT(31)
 #define UIC_PHY_ADAPTER_LAYER_ERROR_CODE_MASK		0x1F
+#define UIC_PHY_ADAPTER_LAYER_LANE_ERR_MASK		0xF
 
 /* UECDL - Host UIC Error Code Data Link Layer 3Ch */
 #define UIC_DATA_LINK_LAYER_ERROR		UFS_BIT(31)
include/trace/events/ufs.h new file +213 -0

213 lines not on this page · raw message

Signature · 3 lines
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project