[char-misc-next 1/4 V5] watchdog: mei_wdt: register wd device only if required

[char-misc-next 1/4 V5] watchdog: mei_wdt: register wd device only if required

[Tomas Winkler]

For Intel Broadwell and newer platforms, the ME device can inform
the host whether the watchdog functionality is activated or not.
If the watchdog functionality is not activated then the watchdog interface
can be not registered and eliminate unnecessary pings and hence lower the
power consumption by avoiding waking up the device.
The feature can be deactivated also without reboot
in that case the watchdog device should be unregistered at runtime.

The information regarding the deactivation is reported
in the ping response command. In runtime case the unregistration
has to be run from a worker so that the ping initiated by the watchdog
core completes. Otherwise the flow will deadlock on watchdog
core mutex which both ping and unregistration acquire.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: Rework unregistration
V3: Rebase; implement unregistraion also at runtime
V4: Rebase the code over patchset : "watchdog: Replace driver based refcounting"
V5: Rebase; Add comment explaining unregistration worker

 drivers/watchdog/mei_wdt.c | 200 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 191 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
index e7e3f144f2b0..9addf8902f74 100644
--- a/drivers/watchdog/mei_wdt.c
+++ b/drivers/watchdog/mei_wdt.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/debugfs.h>
+#include <linux/completion.h>
 #include <linux/watchdog.h>
 
 #include <linux/uuid.h>
@@ -38,27 +39,35 @@
 
 /* Sub Commands */
 #define MEI_MC_START_WD_TIMER_REQ  0x13
+#define MEI_MC_START_WD_TIMER_RES  0x83
+#define   MEI_WDT_STATUS_SUCCESS 0
+#define   MEI_WDT_WDSTATE_NOT_REQUIRED 0x1
 #define MEI_MC_STOP_WD_TIMER_REQ   0x14
 
 /**
  * enum mei_wdt_state - internal watchdog state
  *
+ * @MEI_WDT_PROBE: wd in probing stage
  * @MEI_WDT_IDLE: wd is idle and not opened
  * @MEI_WDT_START: wd was opened, start was called
  * @MEI_WDT_RUNNING: wd is expecting keep alive pings
  * @MEI_WDT_STOPPING: wd is stopping and will move to IDLE
+ * @MEI_WDT_NOT_REQUIRED: wd device is not required
  */
 enum mei_wdt_state {
+	MEI_WDT_PROBE,
 	MEI_WDT_IDLE,
 	MEI_WDT_START,
 	MEI_WDT_RUNNING,
 	MEI_WDT_STOPPING,
+	MEI_WDT_NOT_REQUIRED,
 };
 
-#if IS_ENABLED(CONFIG_DEBUG_FS)
 static const char *mei_wdt_state_str(enum mei_wdt_state state)
 {
 	switch (state) {
+	case MEI_WDT_PROBE:
+		return "PROBE";
 	case MEI_WDT_IDLE:
 		return "IDLE";
 	case MEI_WDT_START:
@@ -67,11 +76,12 @@ static const char *mei_wdt_state_str(enum mei_wdt_state state)
 		return "RUNNING";
 	case MEI_WDT_STOPPING:
 		return "STOPPING";
+	case MEI_WDT_NOT_REQUIRED:
+		return "NOT_REQUIRED";
 	default:
 		return "unknown";
 	}
 }
-#endif /* CONFIG_DEBUG_FS */
 
 /**
  * struct mei_wdt - mei watchdog driver
@@ -79,6 +89,10 @@ static const char *mei_wdt_state_str(enum mei_wdt_state state)
  *
  * @cldev: mei watchdog client device
  * @state: watchdog internal state
+ * @resp_required: ping required response
+ * @response: ping response completion
+ * @unregister: unregister worker
+ * @reg_lock: watchdog device registration lock
  * @timeout: watchdog current timeout
  *
  * @dbgfs_dir: debugfs dir entry
@@ -88,6 +102,10 @@ struct mei_wdt {
 
 	struct mei_cl_device *cldev;
 	enum mei_wdt_state state;
+	bool resp_required;
+	struct completion response;
+	struct work_struct unregister;
+	struct mutex reg_lock;
 	u16 timeout;
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
@@ -124,6 +142,19 @@ struct mei_wdt_start_request {
 } __packed;
 
 /**
+ * struct mei_wdt_start_response watchdog start/ping response
+ *
+ * @hdr: Management Control Command Header
+ * @status: operation status
+ * @wdstate: watchdog status bit mask
+ */
+struct mei_wdt_start_response {
+	struct mei_mc_hdr hdr;
+	u8 status;
+	u8 wdstate;
+} __packed;
+
+/**
  * struct mei_wdt_stop_request - watchdog stop
  *
  * @hdr: Management Control Command Header
@@ -244,13 +275,18 @@ static int mei_wdt_ops_ping(struct watchdog_device *wdd)
 	if (wdt->state != MEI_WDT_START && wdt->state != MEI_WDT_RUNNING)
 		return 0;
 
+	if (wdt->resp_required)
+		init_completion(&wdt->response);
+
+	wdt->state = MEI_WDT_RUNNING;
 	ret = mei_wdt_ping(wdt);
 	if (ret)
 		return ret;
 
-	wdt->state = MEI_WDT_RUNNING;
+	if (wdt->resp_required)
+		ret = wait_for_completion_killable(&wdt->response);
 
-	return 0;
+	return ret;
 }
 
 /**
@@ -291,14 +327,34 @@ static struct watchdog_info wd_info = {
 };
 
 /**
+ * __mei_wdt_is_registered - check if wdt is registered
+ *
+ * @wdt: mei watchdog device
+ *
+ * Return: true if the wdt is registered with the watchdog subsystem
+ * Locking: should be called under wdt->reg_lock
+ */
+static inline bool __mei_wdt_is_registered(struct mei_wdt *wdt)
+{
+	return !!watchdog_get_drvdata(&wdt->wdd);
+}
+
+/**
  * mei_wdt_unregister - unregister from the watchdog subsystem
  *
  * @wdt: mei watchdog device
  */
 static void mei_wdt_unregister(struct mei_wdt *wdt)
 {
-	watchdog_unregister_device(&wdt->wdd);
-	watchdog_set_drvdata(&wdt->wdd, NULL);
+	mutex_lock(&wdt->reg_lock);
+
+	if (__mei_wdt_is_registered(wdt)) {
+		watchdog_unregister_device(&wdt->wdd);
+		watchdog_set_drvdata(&wdt->wdd, NULL);
+		memset(&wdt->wdd, 0, sizeof(wdt->wdd));
+	}
+
+	mutex_unlock(&wdt->reg_lock);
 }
 
 /**
@@ -318,6 +374,13 @@ static int mei_wdt_register(struct mei_wdt *wdt)
 
 	dev = &wdt->cldev->dev;
 
+	mutex_lock(&wdt->reg_lock);
+
+	if (__mei_wdt_is_registered(wdt)) {
+		ret = 0;
+		goto out;
+	}
+
 	wdt->wdd.info = &wd_info;
 	wdt->wdd.ops = &wd_ops;
 	wdt->wdd.parent = dev;
@@ -332,9 +395,106 @@ static int mei_wdt_register(struct mei_wdt *wdt)
 		watchdog_set_drvdata(&wdt->wdd, NULL);
 	}
 
+	wdt->state = MEI_WDT_IDLE;
+
+out:
+	mutex_unlock(&wdt->reg_lock);
 	return ret;
 }
 
+static void mei_wdt_unregister_work(struct work_struct *work)
+{
+	struct mei_wdt *wdt = container_of(work, struct mei_wdt, unregister);
+
+	mei_wdt_unregister(wdt);
+}
+
+/**
+ * mei_wdt_event_rx - callback for data receive
+ *
+ * @cldev: bus device
+ */
+static void mei_wdt_event_rx(struct mei_cl_device *cldev)
+{
+	struct mei_wdt *wdt = mei_cldev_get_drvdata(cldev);
+	struct mei_wdt_start_response res;
+	const size_t res_len = sizeof(res);
+	int ret;
+
+	ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len);
+	if (ret < 0) {
+		dev_err(&cldev->dev, "failure in recv %d\n", ret);
+		return;
+	}
+
+	/* Empty response can be sent on stop */
+	if (ret == 0)
+		return;
+
+	if (ret < sizeof(struct mei_mc_hdr)) {
+		dev_err(&cldev->dev, "recv small data %d\n", ret);
+		return;
+	}
+
+	if (res.hdr.command != MEI_MANAGEMENT_CONTROL ||
+	    res.hdr.versionnumber != MEI_MC_VERSION_NUMBER) {
+		dev_err(&cldev->dev, "wrong command received\n");
+		return;
+	}
+
+	if (res.hdr.subcommand != MEI_MC_START_WD_TIMER_RES) {
+		dev_warn(&cldev->dev, "unsupported command %d :%s[%d]\n",
+			 res.hdr.subcommand,
+			 mei_wdt_state_str(wdt->state),
+			 wdt->state);
+		return;
+	}
+
+	/* Run the unregistration in a worker as this can be
+	 * run only after ping completion, otherwise the flow will
+	 * deadlock on watchdog core mutex.
+	 */
+	if (wdt->state == MEI_WDT_RUNNING) {
+		if (res.wdstate & MEI_WDT_WDSTATE_NOT_REQUIRED) {
+			wdt->state = MEI_WDT_NOT_REQUIRED;
+			schedule_work(&wdt->unregister);
+		}
+		goto out;
+	}
+
+	if (wdt->state == MEI_WDT_PROBE) {
+		if (res.wdstate & MEI_WDT_WDSTATE_NOT_REQUIRED) {
+			wdt->state = MEI_WDT_NOT_REQUIRED;
+		} else {
+			/* stop the watchdog and register watchdog device */
+			mei_wdt_stop(wdt);
+			mei_wdt_register(wdt);
+		}
+		return;
+	}
+
+	dev_warn(&cldev->dev, "not in correct state %s[%d]\n",
+			 mei_wdt_state_str(wdt->state), wdt->state);
+
+out:
+	if (!completion_done(&wdt->response))
+		complete(&wdt->response);
+}
+
+/**
+ * mei_wdt_event - callback for event receive
+ *
+ * @cldev: bus device
+ * @events: event mask
+ * @context: callback context
+ */
+static void mei_wdt_event(struct mei_cl_device *cldev,
+			  u32 events, void *context)
+{
+	if (events & BIT(MEI_CL_EVENT_RX))
+		mei_wdt_event_rx(cldev);
+}
+
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 
 static ssize_t mei_dbgfs_read_state(struct file *file, char __user *ubuf,
@@ -403,8 +563,13 @@ static int mei_wdt_probe(struct mei_cl_device *cldev,
 		return -ENOMEM;
 
 	wdt->timeout = MEI_WDT_DEFAULT_TIMEOUT;
-	wdt->state = MEI_WDT_IDLE;
+	wdt->state = MEI_WDT_PROBE;
 	wdt->cldev = cldev;
+	wdt->resp_required = mei_cldev_ver(cldev) > 0x1;
+	mutex_init(&wdt->reg_lock);
+	init_completion(&wdt->response);
+	INIT_WORK(&wdt->unregister, mei_wdt_unregister_work);
+
 	mei_cldev_set_drvdata(cldev, wdt);
 
 	ret = mei_cldev_enable(cldev);
@@ -413,9 +578,20 @@ static int mei_wdt_probe(struct mei_cl_device *cldev,
 		goto err_out;
 	}
 
+	ret = mei_cldev_register_event_cb(wdt->cldev, BIT(MEI_CL_EVENT_RX),
+					  mei_wdt_event, NULL);
+	if (ret) {
+		dev_err(&cldev->dev, "Could not register event ret=%d\n", ret);
+		goto err_disable;
+	}
+
 	wd_info.firmware_version = mei_cldev_ver(cldev);
 
-	ret = mei_wdt_register(wdt);
+	if (wdt->resp_required)
+		ret = mei_wdt_ping(wdt);
+	else
+		ret = mei_wdt_register(wdt);
+
 	if (ret)
 		goto err_disable;
 
@@ -437,6 +613,12 @@ static int mei_wdt_remove(struct mei_cl_device *cldev)
 {
 	struct mei_wdt *wdt = mei_cldev_get_drvdata(cldev);
 
+	/* Free the caller in case of fw initiated or unexpected reset */
+	if (!completion_done(&wdt->response))
+		complete(&wdt->response);
+
+	cancel_work_sync(&wdt->unregister);
+
 	mei_wdt_unregister(wdt);
 
 	mei_cldev_disable(cldev);
@@ -452,7 +634,7 @@ static int mei_wdt_remove(struct mei_cl_device *cldev)
 			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
 
 static struct mei_cl_device_id mei_wdt_tbl[] = {
-	{ .uuid = MEI_UUID_WD, .version = 0x1},
+	{ .uuid = MEI_UUID_WD, .version = MEI_CL_VERSION_ANY },
 	/* required last entry */
 	{ }
 };
-- 
2.4.3

[char-misc-next 2/4 V5] watchdog: mei_wdt: add activation debugfs entry

[Tomas Winkler]

Add entry for displaying whether the device has activated or
deactivated watchdog fw application.

cat <debugfs>/mei_wdt/activation
activated | deactivated

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V3: new in the series
V4: Rebase the code over patchset : "watchdog: Replace driver based refcounting"
V5: rebase
 drivers/watchdog/mei_wdt.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
index 9addf8902f74..b93474ec34a1 100644
--- a/drivers/watchdog/mei_wdt.c
+++ b/drivers/watchdog/mei_wdt.c
@@ -497,6 +497,28 @@ static void mei_wdt_event(struct mei_cl_device *cldev,
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 
+static ssize_t mei_dbgfs_read_activation(struct file *file, char __user *ubuf,
+					size_t cnt, loff_t *ppos)
+{
+	struct mei_wdt *wdt = file->private_data;
+	const size_t bufsz = 32;
+	char buf[32];
+	ssize_t pos;
+
+	mutex_lock(&wdt->reg_lock);
+	pos = scnprintf(buf, bufsz, "%s\n",
+		__mei_wdt_is_registered(wdt) ? "activated" : "deactivated");
+	mutex_unlock(&wdt->reg_lock);
+
+	return simple_read_from_buffer(ubuf, cnt, ppos, buf, pos);
+}
+
+static const struct file_operations dbgfs_fops_activation = {
+	.open    = simple_open,
+	.read    = mei_dbgfs_read_activation,
+	.llseek  = generic_file_llseek,
+};
+
 static ssize_t mei_dbgfs_read_state(struct file *file, char __user *ubuf,
 				    size_t cnt, loff_t *ppos)
 {
@@ -536,6 +558,11 @@ static int dbgfs_register(struct mei_wdt *wdt)
 	if (!f)
 		goto err;
 
+	f = debugfs_create_file("activation",  S_IRUSR,
+				dir, wdt, &dbgfs_fops_activation);
+	if (!f)
+		goto err;
+
 	return 0;
 err:
 	dbgfs_unregister(wdt);
-- 
2.4.3

[char-misc-next 3/4 V5] watchdog: mei_wdt: re-register device on event

[Tomas Winkler]

For Intel SKL platform the ME device can inform the host via
asynchronous notification that the watchdog feature was activated
on the device. The activation doesn't require reboot.
In that case the driver registers the watchdog device with the kernel.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
V2: rework un/registration in runtime
V3: rebase, runtime unregistration was moved to BDW patch.
V4: Rebase the code over patchset : "watchdog: Replace driver based refcounting"
V5: Rebase

 drivers/watchdog/mei_wdt.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
index b93474ec34a1..630bd189f167 100644
--- a/drivers/watchdog/mei_wdt.c
+++ b/drivers/watchdog/mei_wdt.c
@@ -481,6 +481,21 @@ out:
 		complete(&wdt->response);
 }
 
+/*
+ * mei_wdt_notify_event - callback for event notification
+ *
+ * @cldev: bus device
+ */
+static void mei_wdt_notify_event(struct mei_cl_device *cldev)
+{
+	struct mei_wdt *wdt = mei_cldev_get_drvdata(cldev);
+
+	if (wdt->state != MEI_WDT_NOT_REQUIRED)
+		return;
+
+	mei_wdt_register(wdt);
+}
+
 /**
  * mei_wdt_event - callback for event receive
  *
@@ -493,6 +508,9 @@ static void mei_wdt_event(struct mei_cl_device *cldev,
 {
 	if (events & BIT(MEI_CL_EVENT_RX))
 		mei_wdt_event_rx(cldev);
+
+	if (events & BIT(MEI_CL_EVENT_NOTIF))
+		mei_wdt_notify_event(cldev);
 }
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
@@ -605,9 +623,15 @@ static int mei_wdt_probe(struct mei_cl_device *cldev,
 		goto err_out;
 	}
 
-	ret = mei_cldev_register_event_cb(wdt->cldev, BIT(MEI_CL_EVENT_RX),
+	ret = mei_cldev_register_event_cb(wdt->cldev,
+					  BIT(MEI_CL_EVENT_RX) |
+					  BIT(MEI_CL_EVENT_NOTIF),
 					  mei_wdt_event, NULL);
-	if (ret) {
+
+	/* on legacy devices notification is not supported
+	 * this doesn't fail the registration for RX event
+	 */
+	if (ret && ret != -EOPNOTSUPP) {
 		dev_err(&cldev->dev, "Could not register event ret=%d\n", ret);
 		goto err_disable;
 	}
-- 
2.4.3

[char-misc-next 4/4 RESEND] mei: trace pci configuration space io

[Tomas Winkler]

Use tracing events also for reading and writing pci configuration space
<debugfs>/tracing/events/mei/mei_pci_reg_{read,write}

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/hw-me.c     | 10 ++++++++--
 drivers/misc/mei/hw-txe.c    | 10 ++++++++--
 drivers/misc/mei/mei-trace.c |  2 ++
 drivers/misc/mei/mei-trace.h | 38 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 25b1997a62cb..e2fb44cc5c37 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -189,8 +189,11 @@ static int mei_me_fw_status(struct mei_device *dev,
 
 	fw_status->count = fw_src->count;
 	for (i = 0; i < fw_src->count && i < MEI_FW_STATUS_MAX; i++) {
-		ret = pci_read_config_dword(pdev,
-			fw_src->status[i], &fw_status->status[i]);
+		ret = pci_read_config_dword(pdev, fw_src->status[i],
+					    &fw_status->status[i]);
+		trace_mei_pci_cfg_read(dev->dev, "PCI_CFG_HSF_X",
+				       fw_src->status[i],
+				       fw_status->status[i]);
 		if (ret)
 			return ret;
 	}
@@ -215,6 +218,7 @@ static void mei_me_hw_config(struct mei_device *dev)
 
 	reg = 0;
 	pci_read_config_dword(pdev, PCI_CFG_HFS_1, &reg);
+	trace_mei_pci_cfg_read(dev->dev, "PCI_CFG_HFS_1", PCI_CFG_HFS_1, reg);
 	hw->d0i3_supported =
 		((reg & PCI_CFG_HFS_1_D0I3_MSK) == PCI_CFG_HFS_1_D0I3_MSK);
 
@@ -1248,6 +1252,7 @@ static bool mei_me_fw_type_nm(struct pci_dev *pdev)
 	u32 reg;
 
 	pci_read_config_dword(pdev, PCI_CFG_HFS_2, &reg);
+	trace_mei_pci_cfg_read(&pdev->dev, "PCI_CFG_HFS_2", PCI_CFG_HFS_2, reg);
 	/* make sure that bit 9 (NM) is up and bit 10 (DM) is down */
 	return (reg & 0x600) == 0x200;
 }
@@ -1260,6 +1265,7 @@ static bool mei_me_fw_type_sps(struct pci_dev *pdev)
 	u32 reg;
 	/* Read ME FW Status check for SPS Firmware */
 	pci_read_config_dword(pdev, PCI_CFG_HFS_1, &reg);
+	trace_mei_pci_cfg_read(&pdev->dev, "PCI_CFG_HFS_1", PCI_CFG_HFS_1, reg);
 	/* if bits [19:16] = 15, running SPS Firmware */
 	return (reg & 0xf0000) == 0xf0000;
 }
diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c
index bae680c648ff..4a6c1b85f11e 100644
--- a/drivers/misc/mei/hw-txe.c
+++ b/drivers/misc/mei/hw-txe.c
@@ -28,6 +28,9 @@
 #include "client.h"
 #include "hbm.h"
 
+#include "mei-trace.h"
+
+
 /**
  * mei_txe_reg_read - Reads 32bit data from the txe device
  *
@@ -640,8 +643,11 @@ static int mei_txe_fw_status(struct mei_device *dev,
 
 	fw_status->count = fw_src->count;
 	for (i = 0; i < fw_src->count && i < MEI_FW_STATUS_MAX; i++) {
-		ret = pci_read_config_dword(pdev,
-			fw_src->status[i], &fw_status->status[i]);
+		ret = pci_read_config_dword(pdev, fw_src->status[i],
+					    &fw_status->status[i]);
+		trace_mei_pci_cfg_read(dev->dev, "PCI_CFG_HSF_X",
+				       fw_src->status[i],
+				       fw_status->status[i]);
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/misc/mei/mei-trace.c b/drivers/misc/mei/mei-trace.c
index 388efb519138..e19e6acb191b 100644
--- a/drivers/misc/mei/mei-trace.c
+++ b/drivers/misc/mei/mei-trace.c
@@ -22,4 +22,6 @@
 
 EXPORT_TRACEPOINT_SYMBOL(mei_reg_read);
 EXPORT_TRACEPOINT_SYMBOL(mei_reg_write);
+EXPORT_TRACEPOINT_SYMBOL(mei_pci_cfg_read);
+EXPORT_TRACEPOINT_SYMBOL(mei_pci_cfg_write);
 #endif /* __CHECKER__ */
diff --git a/drivers/misc/mei/mei-trace.h b/drivers/misc/mei/mei-trace.h
index 47e1bc6551d4..86e5068837c1 100644
--- a/drivers/misc/mei/mei-trace.h
+++ b/drivers/misc/mei/mei-trace.h
@@ -64,6 +64,44 @@ TRACE_EVENT(mei_reg_write,
 		  __get_str(dev), __entry->reg,  __entry->offs, __entry->val)
 );
 
+TRACE_EVENT(mei_pci_cfg_read,
+	TP_PROTO(const struct device *dev, const char *reg, u32 offs, u32 val),
+	TP_ARGS(dev, reg, offs, val),
+	TP_STRUCT__entry(
+		__string(dev, dev_name(dev))
+		__field(const char *, reg)
+		__field(u32, offs)
+		__field(u32, val)
+	),
+	TP_fast_assign(
+		__assign_str(dev, dev_name(dev))
+		__entry->reg  = reg;
+		__entry->offs = offs;
+		__entry->val = val;
+	),
+	TP_printk("[%s] pci cfg read %s:[%#x] = %#x",
+		  __get_str(dev), __entry->reg, __entry->offs, __entry->val)
+);
+
+TRACE_EVENT(mei_pci_cfg_write,
+	TP_PROTO(const struct device *dev, const char *reg, u32 offs, u32 val),
+	TP_ARGS(dev, reg, offs, val),
+	TP_STRUCT__entry(
+		__string(dev, dev_name(dev))
+		__field(const char *, reg)
+		__field(u32, offs)
+		__field(u32, val)
+	),
+	TP_fast_assign(
+		__assign_str(dev, dev_name(dev))
+		__entry->reg = reg;
+		__entry->offs = offs;
+		__entry->val = val;
+	),
+	TP_printk("[%s] pci cfg write %s[%#x] = %#x)",
+		  __get_str(dev), __entry->reg,  __entry->offs, __entry->val)
+);
+
 #endif /* _MEI_TRACE_H_ */
 
 /* This part must be outside protection */
-- 
2.4.3