[PATCH 0/4] scsi: ufs-debugfs: Add UFS Exception Event reporting

[PATCH 0/4] scsi: ufs-debugfs: Add UFS Exception Event reporting

[Adrian Hunter]

Hi

Here are patches to add a tracepoint for UFS Exception Events and to allow
users to enable specific exception events without affecting the driver's
use of exception events.


Adrian Hunter (4):
      scsi: ufs: Add exception event tracepoint
      scsi: ufs: Add exception event definitions
      scsi: ufs-debugfs: Add user-defined exception_event_mask
      scsi: ufs-debugfs: Add user-defined exception event rate limiting

 drivers/scsi/ufs/ufs-debugfs.c | 90 ++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-debugfs.h |  2 +
 drivers/scsi/ufs/ufs.h         | 10 ++++-
 drivers/scsi/ufs/ufshcd.c      | 87 +++++++++++++++++++++++++---------------
 drivers/scsi/ufs/ufshcd.h      | 26 +++++++++++-
 include/trace/events/ufs.h     | 21 ++++++++++
 6 files changed, 201 insertions(+), 35 deletions(-)


Regards
Adrian

[PATCH 1/4] scsi: ufs: Add exception event tracepoint

[Adrian Hunter]

Currently, exception event status can be read from wExceptionEventStatus
attribute (sysfs file attributes/exception_event_status under the UFS host
controller device directory). Polling that attribute to track UFS exception
events is impractical, so add a tracepoint to track exception events for
testing and debugging purposes.

Note, by the time the exception event status is read, the exception event
may have cleared, so the value can be zero - see example below.

Note also, only enabled exception events can be reported. A subsequent
patch adds the ability for users to enable selected exception events via
debugfs.

Example with driver instrumented to enable all exception events:

  # echo 1 > /sys/kernel/debug/tracing/events/ufs/ufshcd_exception_event/enable

  ... do some I/O ...

  # cat /sys/kernel/debug/tracing/trace
  # tracer: nop
  #
  # entries-in-buffer/entries-written: 3/3   #P:5
  #
  #                                _-----=> irqs-off
  #                               / _----=> need-resched
  #                              | / _---=> hardirq/softirq
  #                              || / _--=> preempt-depth
  #                              ||| /     delay
  #           TASK-PID     CPU#  ||||   TIMESTAMP  FUNCTION
  #              | |         |   ||||      |         |
       kworker/2:2-173     [002] ....   731.486419: ufshcd_exception_event: 0000:00:12.5: exception event status 0x0
       kworker/2:2-173     [002] ....   732.608918: ufshcd_exception_event: 0000:00:12.5: exception event status 0x4
       kworker/2:2-173     [002] ....   732.609312: ufshcd_exception_event: 0000:00:12.5: exception event status 0x4

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/scsi/ufs/ufshcd.c  |  2 ++
 include/trace/events/ufs.h | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 46a499b7e8a8..7d46b2c278dd 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5624,6 +5624,8 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
 		goto out;
 	}
 
+	trace_ufshcd_exception_event(dev_name(hba->dev), status);
+
 	status &= hba->ee_ctrl_mask;
 
 	if (status & MASK_EE_URGENT_BKOPS)
diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
index e151477d645c..5a2586217eb6 100644
--- a/include/trace/events/ufs.h
+++ b/include/trace/events/ufs.h
@@ -349,6 +349,27 @@ TRACE_EVENT(ufshcd_upiu,
 	)
 );
 
+TRACE_EVENT(ufshcd_exception_event,
+
+	TP_PROTO(const char *dev_name, u16 status),
+
+	TP_ARGS(dev_name, status),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name)
+		__field(u16, status)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name, dev_name);
+		__entry->status = status;
+	),
+
+	TP_printk("%s: exception event status 0x%x",
+		__get_str(dev_name), __entry->status
+	)
+);
+
 #endif /* if !defined(_TRACE_UFS_H) || defined(TRACE_HEADER_MULTI_READ) */
 
 /* This part must be outside protection */
-- 
2.17.1

Re: [PATCH 1/4] scsi: ufs: Add exception event tracepoint

[Bean Huo]

On Tue, 2021-01-19 at 16:15 +0200, Adrian Hunter wrote:
> Currently, exception event status can be read from
> wExceptionEventStatus
> attribute (sysfs file attributes/exception_event_status under the UFS
> host
> controller device directory). Polling that attribute to track UFS
> exception
> events is impractical, so add a tracepoint to track exception events
> for
> testing and debugging purposes.
> 
> Note, by the time the exception event status is read, the exception
> event
> may have cleared, so the value can be zero - see example below.
> 
> Note also, only enabled exception events can be reported. A
> subsequent
> patch adds the ability for users to enable selected exception events
> via
> debugfs.
> 
> Example with driver instrumented to enable all exception events:
> 
>   # echo 1 >
> /sys/kernel/debug/tracing/events/ufs/ufshcd_exception_event/enable
> 
>   ... do some I/O ...
> 
>   # cat /sys/kernel/debug/tracing/trace
>   # tracer: nop
>   #
>   # entries-in-buffer/entries-written: 3/3   #P:5
>   #
>   #                                _-----=> irqs-off
>   #                               / _----=> need-resched
>   #                              | / _---=> hardirq/softirq
>   #                              || / _--=> preempt-depth
>   #                              ||| /     delay
>   #           TASK-PID     CPU#  ||||   TIMESTAMP  FUNCTION
>   #              | |         |   ||||      |         |
>        kworker/2:2-173     [002] ....   731.486419:
> ufshcd_exception_event: 0000:00:12.5: exception event status 0x0
>        kworker/2:2-173     [002] ....   732.608918:
> ufshcd_exception_event: 0000:00:12.5: exception event status 0x4
>        kworker/2:2-173     [002] ....   732.609312:
> ufshcd_exception_event: 0000:00:12.5: exception event status 0x4

Hi Adrian

aAbove print has two trace strings "exception event" in each event
print, it is somehow redundant to me, why not replace the second one
with the event string name?


ufshcd_exception_event: 0000:00:12.5: LOW_TEMP 0x4

or just status:

ufshcd_exception_event: 0000:00:12.5: status 0x4


Bean

> 

> 

[PATCH 2/4] scsi: ufs: Add exception event definitions

[Adrian Hunter]

For readability and completeness, add exception event definitions.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/scsi/ufs/ufs.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 09c7cc8a678d..5d4a2740a159 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -348,8 +348,14 @@ enum power_desc_param_offset {
 
 /* Exception event mask values */
 enum {
-	MASK_EE_STATUS		= 0xFFFF,
-	MASK_EE_URGENT_BKOPS	= (1 << 2),
+	MASK_EE_STATUS			= 0xFFFF,
+	MASK_EE_DYNCAP_EVENT		= BIT(0),
+	MASK_EE_SYSPOOL_EVENT		= BIT(1),
+	MASK_EE_URGENT_BKOPS		= BIT(2),
+	MASK_EE_TOO_HIGH_TEMP		= BIT(3),
+	MASK_EE_TOO_LOW_TEMP		= BIT(4),
+	MASK_EE_WRITEBOOSTER_EVENT	= BIT(5),
+	MASK_EE_PERFORMANCE_THROTTLING	= BIT(6),
 };
 
 /* Background operation status */
-- 
2.17.1

Re: [PATCH 2/4] scsi: ufs: Add exception event definitions

[Bean Huo]

On Tue, 2021-01-19 at 16:15 +0200, Adrian Hunter wrote:
> For readability and completeness, add exception event definitions.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Bean Huo <beanhuo@micron.com>

[PATCH 3/4] scsi: ufs-debugfs: Add user-defined exception_event_mask

[Adrian Hunter]

Allow users to enable specific exception events via debugfs.

The bits enabled by the driver ee_drv_ctrl are separated from the bits
enabled by the user ee_usr_ctrl. The control mask ee_mask_ctrl is the
logical-or of those two. A mutex is needed to ensure that the masks match
what was written to the device.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/scsi/ufs/ufs-debugfs.c | 46 ++++++++++++++++++
 drivers/scsi/ufs/ufshcd.c      | 86 +++++++++++++++++++++-------------
 drivers/scsi/ufs/ufshcd.h      | 22 ++++++++-
 3 files changed, 120 insertions(+), 34 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-debugfs.c b/drivers/scsi/ufs/ufs-debugfs.c
index dee98dc72d29..59729073b569 100644
--- a/drivers/scsi/ufs/ufs-debugfs.c
+++ b/drivers/scsi/ufs/ufs-debugfs.c
@@ -44,10 +44,56 @@ static int ufs_debugfs_stats_show(struct seq_file *s, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(ufs_debugfs_stats);
 
+static int ee_usr_mask_get(void *data, u64 *val)
+{
+	struct ufs_hba *hba = data;
+
+	*val = hba->ee_usr_mask;
+	return 0;
+}
+
+static int ufs_debugfs_get_user_access(struct ufs_hba *hba)
+__acquires(&hba->host_sem)
+{
+	down(&hba->host_sem);
+	if (!ufshcd_is_user_access_allowed(hba)) {
+		up(&hba->host_sem);
+		return -EBUSY;
+	}
+	pm_runtime_get_sync(hba->dev);
+	return 0;
+}
+
+static void ufs_debugfs_put_user_access(struct ufs_hba *hba)
+__releases(&hba->host_sem)
+{
+	pm_runtime_put_sync(hba->dev);
+	up(&hba->host_sem);
+}
+
+static int ee_usr_mask_set(void *data, u64 val)
+{
+	struct ufs_hba *hba = data;
+	int err;
+
+	if (val & ~(u64)MASK_EE_STATUS)
+		return -EINVAL;
+	err = ufs_debugfs_get_user_access(hba);
+	if (err)
+		return err;
+	err = ufshcd_update_ee_usr_mask(hba, val, MASK_EE_STATUS);
+	ufs_debugfs_put_user_access(hba);
+	return err;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(ee_usr_mask_fops, ee_usr_mask_get, ee_usr_mask_set, "%#llx\n");
+
 void ufs_debugfs_hba_init(struct ufs_hba *hba)
 {
 	hba->debugfs_root = debugfs_create_dir(dev_name(hba->dev), ufs_debugfs_root);
 	debugfs_create_file("stats", 0400, hba->debugfs_root, hba, &ufs_debugfs_stats_fops);
+	debugfs_create_file("exception_event_mask", 0600, hba->debugfs_root,
+			    hba, &ee_usr_mask_fops);
 }
 
 void ufs_debugfs_hba_exit(struct ufs_hba *hba)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 7d46b2c278dd..d68f5ecc9b13 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5147,6 +5147,46 @@ static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba)
 	}
 }
 
+static int __ufshcd_write_ee_control(struct ufs_hba *hba, u32 ee_ctrl_mask)
+{
+	return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
+				       QUERY_ATTR_IDN_EE_CONTROL, 0, 0,
+				       &ee_ctrl_mask);
+}
+
+static int ufshcd_write_ee_control(struct ufs_hba *hba)
+{
+	int err;
+
+	mutex_lock(&hba->ee_ctrl_mutex);
+	err = __ufshcd_write_ee_control(hba, hba->ee_ctrl_mask);
+	mutex_unlock(&hba->ee_ctrl_mutex);
+	if (err)
+		dev_err(hba->dev, "%s: failed to write ee control %d\n",
+			__func__, err);
+	return err;
+}
+
+int ufshcd_update_ee_control(struct ufs_hba *hba, u16 *mask, u16 *other_mask,
+			     u16 set, u16 clr)
+{
+	u16 new_mask, ee_ctrl_mask;
+	int err = 0;
+
+	mutex_lock(&hba->ee_ctrl_mutex);
+	new_mask = (*mask & ~clr) | set;
+	ee_ctrl_mask = new_mask | *other_mask;
+	if (ee_ctrl_mask != hba->ee_ctrl_mask)
+		err = __ufshcd_write_ee_control(hba, ee_ctrl_mask);
+	/* Still need to update 'mask' even if 'ee_ctrl_mask' was unchanged */
+	if (!err) {
+		hba->ee_ctrl_mask = ee_ctrl_mask;
+		*mask = new_mask;
+	}
+	mutex_unlock(&hba->ee_ctrl_mutex);
+	return err;
+}
+
 /**
  * ufshcd_disable_ee - disable exception event
  * @hba: per-adapter instance
@@ -5157,22 +5197,9 @@ static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba)
  *
  * Returns zero on success, non-zero error value on failure.
  */
-static int ufshcd_disable_ee(struct ufs_hba *hba, u16 mask)
+static inline int ufshcd_disable_ee(struct ufs_hba *hba, u16 mask)
 {
-	int err = 0;
-	u32 val;
-
-	if (!(hba->ee_ctrl_mask & mask))
-		goto out;
-
-	val = hba->ee_ctrl_mask & ~mask;
-	val &= MASK_EE_STATUS;
-	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
-			QUERY_ATTR_IDN_EE_CONTROL, 0, 0, &val);
-	if (!err)
-		hba->ee_ctrl_mask &= ~mask;
-out:
-	return err;
+	return ufshcd_update_ee_drv_mask(hba, 0, mask);
 }
 
 /**
@@ -5185,22 +5212,9 @@ static int ufshcd_disable_ee(struct ufs_hba *hba, u16 mask)
  *
  * Returns zero on success, non-zero error value on failure.
  */
-static int ufshcd_enable_ee(struct ufs_hba *hba, u16 mask)
+static inline int ufshcd_enable_ee(struct ufs_hba *hba, u16 mask)
 {
-	int err = 0;
-	u32 val;
-
-	if (hba->ee_ctrl_mask & mask)
-		goto out;
-
-	val = hba->ee_ctrl_mask | mask;
-	val &= MASK_EE_STATUS;
-	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
-			QUERY_ATTR_IDN_EE_CONTROL, 0, 0, &val);
-	if (!err)
-		hba->ee_ctrl_mask |= mask;
-out:
-	return err;
+	return ufshcd_update_ee_drv_mask(hba, mask, 0);
 }
 
 /**
@@ -5626,9 +5640,7 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
 
 	trace_ufshcd_exception_event(dev_name(hba->dev), status);
 
-	status &= hba->ee_ctrl_mask;
-
-	if (status & MASK_EE_URGENT_BKOPS)
+	if (status & hba->ee_drv_mask & MASK_EE_URGENT_BKOPS)
 		ufshcd_bkops_exception_event_handler(hba);
 
 out:
@@ -7914,6 +7926,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
 	ufshcd_set_active_icc_lvl(hba);
 
 	ufshcd_wb_config(hba);
+	if (hba->ee_usr_mask)
+		ufshcd_write_ee_control(hba);
 	/* Enable Auto-Hibernate if configured */
 	ufshcd_auto_hibern8_enable(hba);
 
@@ -8916,6 +8930,9 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 		 */
 		ufshcd_urgent_bkops(hba);
 
+	if (hba->ee_usr_mask)
+		ufshcd_write_ee_control(hba);
+
 	hba->clk_gating.is_suspended = false;
 
 	if (hba->clk_scaling.is_allowed)
@@ -9362,6 +9379,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	/* Initialize mutex for device management commands */
 	mutex_init(&hba->dev_cmd.lock);
 
+	/* Initialize mutex for exception event control */
+	mutex_init(&hba->ee_ctrl_mutex);
+
 	init_rwsem(&hba->clk_scaling_lock);
 
 	ufshcd_init_clk_gating(hba);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 5bbe4715d4e9..ff9601f8d9e6 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -761,7 +761,10 @@ struct ufs_hba {
 	u32 ufshcd_state;
 	u32 eh_flags;
 	u32 intr_mask;
-	u16 ee_ctrl_mask;
+	u16 ee_ctrl_mask; /* Exception event mask */
+	u16 ee_drv_mask;  /* Exception event mask for driver */
+	u16 ee_usr_mask;  /* Exception event mask for user (via debugfs) */
+	struct mutex ee_ctrl_mutex;
 	bool is_powered;
 	bool shutting_down;
 	struct semaphore host_sem;
@@ -1273,4 +1276,21 @@ static inline u8 ufshcd_scsi_to_upiu_lun(unsigned int scsi_lun)
 int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
 		     const char *prefix);
 
+int ufshcd_update_ee_control(struct ufs_hba *hba, u16 *mask, u16 *other_mask,
+			     u16 set, u16 clr);
+
+static inline int ufshcd_update_ee_drv_mask(struct ufs_hba *hba,
+					    u16 set, u16 clr)
+{
+	return ufshcd_update_ee_control(hba, &hba->ee_drv_mask,
+					&hba->ee_usr_mask, set, clr);
+}
+
+static inline int ufshcd_update_ee_usr_mask(struct ufs_hba *hba,
+					    u16 set, u16 clr)
+{
+	return ufshcd_update_ee_control(hba, &hba->ee_usr_mask,
+					&hba->ee_drv_mask, set, clr);
+}
+
 #endif /* End of Header */
-- 
2.17.1

Re: [PATCH 3/4] scsi: ufs-debugfs: Add user-defined exception_event_mask

[Bean Huo]

On Tue, 2021-01-19 at 16:15 +0200, Adrian Hunter wrote:
> Allow users to enable specific exception events via debugfs.
> 
> The bits enabled by the driver ee_drv_ctrl are separated from the
> bits
> enabled by the user ee_usr_ctrl. The control mask ee_mask_ctrl is the
> logical-or of those two. A mutex is needed to ensure that the masks
> match
> what was written to the device.

Hallo Adrian

Would you like sharing the advantage of this debugfs node comparing to
sysfs node "attributes/exception_event_control(if it is writable)"?
what is the value of this?
Also, now I can disable/enable UFS event over ufs-bsg.

Bean

Re: [PATCH 3/4] scsi: ufs-debugfs: Add user-defined exception_event_mask

[Adrian Hunter]

On 3/02/21 11:45 am, Bean Huo wrote:
> On Tue, 2021-01-19 at 16:15 +0200, Adrian Hunter wrote:
>> Allow users to enable specific exception events via debugfs.
>>
>> The bits enabled by the driver ee_drv_ctrl are separated from the
>> bits
>> enabled by the user ee_usr_ctrl. The control mask ee_mask_ctrl is the
>> logical-or of those two. A mutex is needed to ensure that the masks
>> match
>> what was written to the device.
> 
> Hallo Adrian

Hi Bean

Thanks for the review

> 
> Would you like sharing the advantage of this debugfs node comparing to
> sysfs node "attributes/exception_event_control(if it is writable)"?

Primarily this is being done as a debug interface, but the user's exception
events also need to be kept separate from the driver's ones.

> what is the value of this?

To be able to determine if the UFS device is being affected by exception events.

> Also, now I can disable/enable UFS event over ufs-bsg.

That will be overwritten by the driver when it updates the e.g. bkops
control, or sometimes also suspend/resume.

Re: [PATCH 3/4] scsi: ufs-debugfs: Add user-defined exception_event_mask

[Bean Huo]

On Wed, 2021-02-03 at 11:56 +0200, Adrian Hunter wrote:
> > 
> > Hallo Adrian
> 
> Hi Bean
> 
> Thanks for the review
> 
> > 
> > Would you like sharing the advantage of this debugfs node comparing
> > to
> > sysfs node "attributes/exception_event_control(if it is writable)"?
> 
> Primarily this is being done as a debug interface, but the user's
> exception
> events also need to be kept separate from the driver's ones.
> 
> > what is the value of this?
> 
> To be able to determine if the UFS device is being affected by
> exception events.
> 
> > Also, now I can disable/enable UFS event over ufs-bsg.
> 
> That will be overwritten by the driver when it updates the e.g. bkops
> control, or sometimes also suspend/resume.

Hi Adrian
yes, I saw that, they are not tracked by driver.

I have one question that why "exception_event_mask" cannot represent
the current QUERY_ATTR_IDN_EE_CONTROL value? only after writing it.


thanks,
Bean

Re: [PATCH 3/4] scsi: ufs-debugfs: Add user-defined exception_event_mask

[Adrian Hunter]

On 4/02/21 4:58 pm, Bean Huo wrote:
> On Wed, 2021-02-03 at 11:56 +0200, Adrian Hunter wrote:
>>>
>>> Hallo Adrian
>>
>> Hi Bean
>>
>> Thanks for the review
>>
>>>
>>> Would you like sharing the advantage of this debugfs node comparing
>>> to
>>> sysfs node "attributes/exception_event_control(if it is writable)"?
>>
>> Primarily this is being done as a debug interface, but the user's
>> exception
>> events also need to be kept separate from the driver's ones.
>>
>>> what is the value of this?
>>
>> To be able to determine if the UFS device is being affected by
>> exception events.
>>
>>> Also, now I can disable/enable UFS event over ufs-bsg.
>>
>> That will be overwritten by the driver when it updates the e.g. bkops
>> control, or sometimes also suspend/resume.
> 
> Hi Adrian
> yes, I saw that, they are not tracked by driver.
> 
> I have one question that why "exception_event_mask" cannot represent
> the current QUERY_ATTR_IDN_EE_CONTROL value? only after writing it.

It represents only the user's exception events (ee_usr_mask), not the
driver's ones (ee_drv_mask) as well.  ee_usr_mask is updated after
successfully ensuring it is set on the device.

[PATCH 4/4] scsi: ufs-debugfs: Add user-defined exception event rate limiting

[Adrian Hunter]

An enabled user-specified exception event that does not clear quickly will
repeatedly cause the handler to run. That could unduly disturb the driver
behaviour being tested or debugged. To prevent that add debugfs file
exception_event_rate_limit_ms. When a exception event happens, it is
disabled, and then after a period of time (default 20ms) the exception
event is enabled again.

Note that if the driver also has that exception event enabled, it will not
be disabled.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/scsi/ufs/ufs-debugfs.c | 44 ++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-debugfs.h |  2 ++
 drivers/scsi/ufs/ufshcd.c      |  5 ++--
 drivers/scsi/ufs/ufshcd.h      |  4 ++++
 4 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-debugfs.c b/drivers/scsi/ufs/ufs-debugfs.c
index 59729073b569..ced9ef4d7c78 100644
--- a/drivers/scsi/ufs/ufs-debugfs.c
+++ b/drivers/scsi/ufs/ufs-debugfs.c
@@ -88,15 +88,59 @@ static int ee_usr_mask_set(void *data, u64 val)
 
 DEFINE_DEBUGFS_ATTRIBUTE(ee_usr_mask_fops, ee_usr_mask_get, ee_usr_mask_set, "%#llx\n");
 
+void ufs_debugfs_exception_event(struct ufs_hba *hba, u16 status)
+{
+	bool chgd = false;
+	u16 ee_ctrl_mask;
+	int err = 0;
+
+	if (!hba->debugfs_ee_rate_limit_ms || !status)
+		return;
+
+	mutex_lock(&hba->ee_ctrl_mutex);
+	ee_ctrl_mask = hba->ee_drv_mask | (hba->ee_usr_mask & ~status);
+	chgd = ee_ctrl_mask != hba->ee_ctrl_mask;
+	if (chgd) {
+		err = __ufshcd_write_ee_control(hba, ee_ctrl_mask);
+		if (err)
+			dev_err(hba->dev, "%s: failed to write ee control %d\n",
+				__func__, err);
+	}
+	mutex_unlock(&hba->ee_ctrl_mutex);
+
+	if (chgd && !err) {
+		unsigned long delay = msecs_to_jiffies(hba->debugfs_ee_rate_limit_ms);
+
+		queue_delayed_work(system_freezable_wq, &hba->debugfs_ee_work, delay);
+	}
+}
+
+static void ufs_debugfs_restart_ee(struct work_struct *work)
+{
+	struct ufs_hba *hba = container_of(work, struct ufs_hba, debugfs_ee_work.work);
+
+	if (!hba->ee_usr_mask || pm_runtime_suspended(hba->dev) ||
+	    ufs_debugfs_get_user_access(hba))
+		return;
+	ufshcd_write_ee_control(hba);
+	ufs_debugfs_put_user_access(hba);
+}
+
 void ufs_debugfs_hba_init(struct ufs_hba *hba)
 {
+	/* Set default exception event rate limit period to 20ms */
+	hba->debugfs_ee_rate_limit_ms = 20;
+	INIT_DELAYED_WORK(&hba->debugfs_ee_work, ufs_debugfs_restart_ee);
 	hba->debugfs_root = debugfs_create_dir(dev_name(hba->dev), ufs_debugfs_root);
 	debugfs_create_file("stats", 0400, hba->debugfs_root, hba, &ufs_debugfs_stats_fops);
 	debugfs_create_file("exception_event_mask", 0600, hba->debugfs_root,
 			    hba, &ee_usr_mask_fops);
+	debugfs_create_u32("exception_event_rate_limit_ms", 0600, hba->debugfs_root,
+			   &hba->debugfs_ee_rate_limit_ms);
 }
 
 void ufs_debugfs_hba_exit(struct ufs_hba *hba)
 {
 	debugfs_remove_recursive(hba->debugfs_root);
+	cancel_delayed_work_sync(&hba->debugfs_ee_work);
 }
diff --git a/drivers/scsi/ufs/ufs-debugfs.h b/drivers/scsi/ufs/ufs-debugfs.h
index f35b39c4b4f5..3ca29d30460a 100644
--- a/drivers/scsi/ufs/ufs-debugfs.h
+++ b/drivers/scsi/ufs/ufs-debugfs.h
@@ -12,11 +12,13 @@ void __init ufs_debugfs_init(void);
 void __exit ufs_debugfs_exit(void);
 void ufs_debugfs_hba_init(struct ufs_hba *hba);
 void ufs_debugfs_hba_exit(struct ufs_hba *hba);
+void ufs_debugfs_exception_event(struct ufs_hba *hba, u16 status);
 #else
 static inline void ufs_debugfs_init(void) {}
 static inline void ufs_debugfs_exit(void) {}
 static inline void ufs_debugfs_hba_init(struct ufs_hba *hba) {}
 static inline void ufs_debugfs_hba_exit(struct ufs_hba *hba) {}
+static inline void ufs_debugfs_exception_event(struct ufs_hba *hba, u16 status) {}
 #endif
 
 #endif
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index d68f5ecc9b13..f0959b9609fd 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5147,14 +5147,14 @@ static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba)
 	}
 }
 
-static int __ufshcd_write_ee_control(struct ufs_hba *hba, u32 ee_ctrl_mask)
+int __ufshcd_write_ee_control(struct ufs_hba *hba, u32 ee_ctrl_mask)
 {
 	return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
 				       QUERY_ATTR_IDN_EE_CONTROL, 0, 0,
 				       &ee_ctrl_mask);
 }
 
-static int ufshcd_write_ee_control(struct ufs_hba *hba)
+int ufshcd_write_ee_control(struct ufs_hba *hba)
 {
 	int err;
 
@@ -5643,6 +5643,7 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
 	if (status & hba->ee_drv_mask & MASK_EE_URGENT_BKOPS)
 		ufshcd_bkops_exception_event_handler(hba);
 
+	ufs_debugfs_exception_event(hba, status);
 out:
 	ufshcd_scsi_unblock_requests(hba);
 	/*
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index ff9601f8d9e6..49eed4cf009c 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -833,6 +833,8 @@ struct ufs_hba {
 #endif
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debugfs_root;
+	struct delayed_work debugfs_ee_work;
+	u32 debugfs_ee_rate_limit_ms;
 #endif
 };
 
@@ -1276,6 +1278,8 @@ static inline u8 ufshcd_scsi_to_upiu_lun(unsigned int scsi_lun)
 int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
 		     const char *prefix);
 
+int __ufshcd_write_ee_control(struct ufs_hba *hba, u32 ee_ctrl_mask);
+int ufshcd_write_ee_control(struct ufs_hba *hba);
 int ufshcd_update_ee_control(struct ufs_hba *hba, u16 *mask, u16 *other_mask,
 			     u16 set, u16 clr);
 
-- 
2.17.1

Re: [PATCH 0/4] scsi: ufs-debugfs: Add UFS Exception Event reporting

[Adrian Hunter]

On 19/01/21 4:15 pm, Adrian Hunter wrote:
> Hi
> 
> Here are patches to add a tracepoint for UFS Exception Events and to allow
> users to enable specific exception events without affecting the driver's
> use of exception events.
> 
> 
> Adrian Hunter (4):
>       scsi: ufs: Add exception event tracepoint
>       scsi: ufs: Add exception event definitions
>       scsi: ufs-debugfs: Add user-defined exception_event_mask
>       scsi: ufs-debugfs: Add user-defined exception event rate limiting
> 
>  drivers/scsi/ufs/ufs-debugfs.c | 90 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufs-debugfs.h |  2 +
>  drivers/scsi/ufs/ufs.h         | 10 ++++-
>  drivers/scsi/ufs/ufshcd.c      | 87 +++++++++++++++++++++++++---------------
>  drivers/scsi/ufs/ufshcd.h      | 26 +++++++++++-
>  include/trace/events/ufs.h     | 21 ++++++++++
>  6 files changed, 201 insertions(+), 35 deletions(-)

Any comments on this?