* [char-misc-next v4 1/7] mei: wd: drop the watchdog code from the core mei driver
2016-01-07 22:49 [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver Tomas Winkler
@ 2016-01-07 22:49 ` Tomas Winkler
2016-01-07 22:49 ` [char-misc-next v4 2/7] watchdog: mei_wdt: implement MEI iAMT watchdog driver Tomas Winkler
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Tomas Winkler @ 2016-01-07 22:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Wim Van Sebroeck, Guenter Roeck
Cc: Alexander Usyskin, linux-watchdog, linux-kernel, Tomas Winkler
From: Alexander Usyskin <alexander.usyskin@intel.com>
Instead of integrating the iAMT watchdog in the mei core driver
we will create a watchdog device on the mei client bus and
create a driver for it.
This patch removes the watchdog code from the mei core driver.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2-V4: resend
drivers/misc/mei/Kconfig | 6 +-
drivers/misc/mei/Makefile | 1 -
drivers/misc/mei/client.c | 12 +-
drivers/misc/mei/client.h | 4 -
drivers/misc/mei/init.c | 10 +-
drivers/misc/mei/interrupt.c | 15 --
drivers/misc/mei/mei_dev.h | 61 +------
drivers/misc/mei/wd.c | 391 -------------------------------------------
8 files changed, 9 insertions(+), 491 deletions(-)
delete mode 100644 drivers/misc/mei/wd.c
diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index d23384dde73b..c49e1d2269af 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -1,6 +1,6 @@
config INTEL_MEI
tristate "Intel Management Engine Interface"
- depends on X86 && PCI && WATCHDOG_CORE
+ depends on X86 && PCI
help
The Intel Management Engine (Intel ME) provides Manageability,
Security and Media services for system containing Intel chipsets.
@@ -12,7 +12,7 @@ config INTEL_MEI
config INTEL_MEI_ME
tristate "ME Enabled Intel Chipsets"
select INTEL_MEI
- depends on X86 && PCI && WATCHDOG_CORE
+ depends on X86 && PCI
help
MEI support for ME Enabled Intel chipsets.
@@ -37,7 +37,7 @@ config INTEL_MEI_ME
config INTEL_MEI_TXE
tristate "Intel Trusted Execution Environment with ME Interface"
select INTEL_MEI
- depends on X86 && PCI && WATCHDOG_CORE
+ depends on X86 && PCI
help
MEI Support for Trusted Execution Environment device on Intel SoCs
diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
index 01447ca21c26..59e6b0aede34 100644
--- a/drivers/misc/mei/Makefile
+++ b/drivers/misc/mei/Makefile
@@ -9,7 +9,6 @@ mei-objs += interrupt.o
mei-objs += client.o
mei-objs += main.o
mei-objs += amthif.o
-mei-objs += wd.o
mei-objs += bus.o
mei-objs += bus-fixup.o
mei-$(CONFIG_DEBUG_FS) += debugfs.o
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 72e32615acd9..e069fcaed7aa 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -648,7 +648,7 @@ int mei_cl_unlink(struct mei_cl *cl)
if (!cl)
return 0;
- /* wd and amthif might not be initialized */
+ /* amthif might not be initialized */
if (!cl->dev)
return 0;
@@ -679,17 +679,11 @@ void mei_host_client_init(struct work_struct *work)
mutex_lock(&dev->device_lock);
-
me_cl = mei_me_cl_by_uuid(dev, &mei_amthif_guid);
if (me_cl)
mei_amthif_host_init(dev, me_cl);
mei_me_cl_put(me_cl);
- me_cl = mei_me_cl_by_uuid(dev, &mei_wd_guid);
- if (me_cl)
- mei_wd_host_init(dev, me_cl);
- mei_me_cl_put(me_cl);
-
dev->dev_state = MEI_DEV_ENABLED;
dev->reset_count = 0;
mutex_unlock(&dev->device_lock);
@@ -1153,7 +1147,7 @@ err:
*
* Return: 1 if mei_flow_ctrl_creds >0, 0 - otherwise.
*/
-int mei_cl_flow_ctrl_creds(struct mei_cl *cl)
+static int mei_cl_flow_ctrl_creds(struct mei_cl *cl)
{
int rets;
@@ -1186,7 +1180,7 @@ int mei_cl_flow_ctrl_creds(struct mei_cl *cl)
* 0 on success
* -EINVAL when ctrl credits are <= 0
*/
-int mei_cl_flow_ctrl_reduce(struct mei_cl *cl)
+static int mei_cl_flow_ctrl_reduce(struct mei_cl *cl)
{
if (WARN_ON(!cl || !cl->me_cl))
return -EINVAL;
diff --git a/drivers/misc/mei/client.h b/drivers/misc/mei/client.h
index 04e1aa39243f..2e90a25f896e 100644
--- a/drivers/misc/mei/client.h
+++ b/drivers/misc/mei/client.h
@@ -18,7 +18,6 @@
#define _MEI_CLIENT_H_
#include <linux/types.h>
-#include <linux/watchdog.h>
#include <linux/poll.h>
#include <linux/mei.h>
@@ -120,9 +119,6 @@ struct mei_cl_cb *mei_cl_alloc_cb(struct mei_cl *cl, size_t length,
enum mei_cb_file_ops type, struct file *fp);
int mei_cl_flush_queues(struct mei_cl *cl, const struct file *fp);
-int mei_cl_flow_ctrl_creds(struct mei_cl *cl);
-
-int mei_cl_flow_ctrl_reduce(struct mei_cl *cl);
/*
* MEI input output function prototype
*/
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index 3edafc8d3ad4..46a4302e5524 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -156,8 +156,7 @@ int mei_reset(struct mei_device *dev)
mei_cl_all_wakeup(dev);
/* remove entry if already in list */
- dev_dbg(dev->dev, "remove iamthif and wd from the file list.\n");
- mei_cl_unlink(&dev->wd_cl);
+ dev_dbg(dev->dev, "remove iamthif from the file list.\n");
mei_cl_unlink(&dev->iamthif_cl);
mei_amthif_reset_params(dev);
}
@@ -165,7 +164,6 @@ int mei_reset(struct mei_device *dev)
mei_hbm_reset(dev);
dev->rd_msg_hdr = 0;
- dev->wd_pending = false;
if (ret) {
dev_err(dev->dev, "hw_reset failed ret = %d\n", ret);
@@ -335,16 +333,12 @@ void mei_stop(struct mei_device *dev)
mutex_lock(&dev->device_lock);
- mei_wd_stop(dev);
-
dev->dev_state = MEI_DEV_POWER_DOWN;
mei_reset(dev);
/* move device to disabled state unconditionally */
dev->dev_state = MEI_DEV_DISABLED;
mutex_unlock(&dev->device_lock);
-
- mei_watchdog_unregister(dev);
}
EXPORT_SYMBOL_GPL(mei_stop);
@@ -394,7 +388,6 @@ void mei_device_init(struct mei_device *dev,
init_waitqueue_head(&dev->wait_hw_ready);
init_waitqueue_head(&dev->wait_pg);
init_waitqueue_head(&dev->wait_hbm_start);
- init_waitqueue_head(&dev->wait_stop_wd);
dev->dev_state = MEI_DEV_INITIALIZING;
dev->reset_count = 0;
@@ -407,7 +400,6 @@ void mei_device_init(struct mei_device *dev,
INIT_WORK(&dev->init_work, mei_host_client_init);
INIT_WORK(&dev->reset_work, mei_reset_work);
- INIT_LIST_HEAD(&dev->wd_cl.link);
INIT_LIST_HEAD(&dev->iamthif_cl.link);
mei_io_list_init(&dev->amthif_cmd_list);
mei_io_list_init(&dev->amthif_rd_complete_list);
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 64b568a0268d..6340dee33052 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -360,21 +360,6 @@ int mei_irq_write_handler(struct mei_device *dev, struct mei_cl_cb *cmpl_list)
list_move_tail(&cb->list, &cmpl_list->list);
}
- if (dev->wd_state == MEI_WD_STOPPING) {
- dev->wd_state = MEI_WD_IDLE;
- wake_up(&dev->wait_stop_wd);
- }
-
- if (mei_cl_is_connected(&dev->wd_cl)) {
- if (dev->wd_pending &&
- mei_cl_flow_ctrl_creds(&dev->wd_cl) > 0) {
- ret = mei_wd_send(dev);
- if (ret)
- return ret;
- dev->wd_pending = false;
- }
- }
-
/* complete control write list CB */
dev_dbg(dev->dev, "complete control write list cb.\n");
list_for_each_entry_safe(cb, next, &dev->ctrl_wr_list.list, list) {
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index b54d9d9cacea..da613268480c 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -18,7 +18,7 @@
#define _MEI_DEV_H_
#include <linux/types.h>
-#include <linux/watchdog.h>
+#include <linux/cdev.h>
#include <linux/poll.h>
#include <linux/mei.h>
#include <linux/mei_cl_bus.h>
@@ -26,33 +26,13 @@
#include "hw.h"
#include "hbm.h"
-/*
- * watch dog definition
- */
-#define MEI_WD_HDR_SIZE 4
-#define MEI_WD_STOP_MSG_SIZE MEI_WD_HDR_SIZE
-#define MEI_WD_START_MSG_SIZE (MEI_WD_HDR_SIZE + 16)
-
-#define MEI_WD_DEFAULT_TIMEOUT 120 /* seconds */
-#define MEI_WD_MIN_TIMEOUT 120 /* seconds */
-#define MEI_WD_MAX_TIMEOUT 65535 /* seconds */
-
-#define MEI_WD_STOP_TIMEOUT 10 /* msecs */
-
-#define MEI_WD_STATE_INDEPENDENCE_MSG_SENT (1 << 0)
-
-#define MEI_RD_MSG_BUF_SIZE (128 * sizeof(u32))
-
/*
* AMTHI Client UUID
*/
extern const uuid_le mei_amthif_guid;
-/*
- * Watchdog Client UUID
- */
-extern const uuid_le mei_wd_guid;
+#define MEI_RD_MSG_BUF_SIZE (128 * sizeof(u32))
/*
* Number of Maximum MEI Clients
@@ -78,7 +58,6 @@ extern const uuid_le mei_wd_guid;
*/
#define MEI_HOST_CLIENT_ID_ANY (-1)
#define MEI_HBM_HOST_CLIENT_ID 0 /* not used, just for documentation */
-#define MEI_WD_HOST_CLIENT_ID 1
#define MEI_IAMTHIF_HOST_CLIENT_ID 2
@@ -123,12 +102,6 @@ enum mei_file_transaction_states {
MEI_READ_COMPLETE
};
-enum mei_wd_states {
- MEI_WD_IDLE,
- MEI_WD_RUNNING,
- MEI_WD_STOPPING,
-};
-
/**
* enum mei_cb_file_ops - file operation associated with the callback
* @MEI_FOP_READ: read
@@ -404,7 +377,6 @@ const char *mei_pg_state_str(enum mei_pg_state state);
* @wait_hw_ready : wait queue for receive HW ready message form FW
* @wait_pg : wait queue for receive PG message from FW
* @wait_hbm_start : wait queue for receive HBM start message from FW
- * @wait_stop_wd : wait queue for receive WD stop message from FW
*
* @reset_count : number of consecutive resets
* @dev_state : device state
@@ -435,12 +407,6 @@ const char *mei_pg_state_str(enum mei_pg_state state);
*
* @allow_fixed_address: allow user space to connect a fixed client
*
- * @wd_cl : watchdog client
- * @wd_state : watchdog client state
- * @wd_pending : watchdog command is pending
- * @wd_timeout : watchdog expiration timeout
- * @wd_data : watchdog message buffer
- *
* @amthif_cmd_list : amthif list for cmd waiting
* @amthif_rd_complete_list : amthif list for reading completed cmd data
* @iamthif_file_object : file for current amthif operation
@@ -486,7 +452,6 @@ struct mei_device {
wait_queue_head_t wait_hw_ready;
wait_queue_head_t wait_pg;
wait_queue_head_t wait_hbm_start;
- wait_queue_head_t wait_stop_wd;
/*
* mei device states
@@ -531,13 +496,6 @@ struct mei_device {
bool allow_fixed_address;
- struct mei_cl wd_cl;
- enum mei_wd_states wd_state;
- bool wd_pending;
- u16 wd_timeout;
- unsigned char wd_data[MEI_WD_START_MSG_SIZE];
-
-
/* amthif list for cmd waiting */
struct mei_cl_cb amthif_cmd_list;
/* driver managed amthif list for reading completed amthif cmd data */
@@ -649,21 +607,6 @@ int mei_amthif_irq_read_msg(struct mei_cl *cl,
struct mei_cl_cb *complete_list);
int mei_amthif_irq_read(struct mei_device *dev, s32 *slots);
-int mei_wd_send(struct mei_device *dev);
-int mei_wd_stop(struct mei_device *dev);
-int mei_wd_host_init(struct mei_device *dev, struct mei_me_client *me_cl);
-/*
- * mei_watchdog_register - Registering watchdog interface
- * once we got connection to the WD Client
- * @dev: mei device
- */
-int mei_watchdog_register(struct mei_device *dev);
-/*
- * mei_watchdog_unregister - Unregistering watchdog interface
- * @dev: mei device
- */
-void mei_watchdog_unregister(struct mei_device *dev);
-
/*
* Register Access Function
*/
diff --git a/drivers/misc/mei/wd.c b/drivers/misc/mei/wd.c
deleted file mode 100644
index b346638833b0..000000000000
--- a/drivers/misc/mei/wd.c
+++ /dev/null
@@ -1,391 +0,0 @@
-/*
- *
- * Intel Management Engine Interface (Intel MEI) Linux driver
- * Copyright (c) 2003-2012, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope 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.
- *
- */
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/device.h>
-#include <linux/sched.h>
-#include <linux/watchdog.h>
-
-#include <linux/mei.h>
-
-#include "mei_dev.h"
-#include "hbm.h"
-#include "client.h"
-
-static const u8 mei_start_wd_params[] = { 0x02, 0x12, 0x13, 0x10 };
-static const u8 mei_stop_wd_params[] = { 0x02, 0x02, 0x14, 0x10 };
-
-/*
- * AMT Watchdog Device
- */
-#define INTEL_AMT_WATCHDOG_ID "INTCAMT"
-
-/* UUIDs for AMT F/W clients */
-const uuid_le mei_wd_guid = UUID_LE(0x05B79A6F, 0x4628, 0x4D7F, 0x89,
- 0x9D, 0xA9, 0x15, 0x14, 0xCB,
- 0x32, 0xAB);
-
-static void mei_wd_set_start_timeout(struct mei_device *dev, u16 timeout)
-{
- dev_dbg(dev->dev, "wd: set timeout=%d.\n", timeout);
- memcpy(dev->wd_data, mei_start_wd_params, MEI_WD_HDR_SIZE);
- memcpy(dev->wd_data + MEI_WD_HDR_SIZE, &timeout, sizeof(u16));
-}
-
-/**
- * mei_wd_host_init - connect to the watchdog client
- *
- * @dev: the device structure
- * @me_cl: me client
- *
- * Return: -ENOTTY if wd client cannot be found
- * -EIO if write has failed
- * 0 on success
- */
-int mei_wd_host_init(struct mei_device *dev, struct mei_me_client *me_cl)
-{
- struct mei_cl *cl = &dev->wd_cl;
- int ret;
-
- mei_cl_init(cl, dev);
-
- dev->wd_timeout = MEI_WD_DEFAULT_TIMEOUT;
- dev->wd_state = MEI_WD_IDLE;
-
- ret = mei_cl_link(cl, MEI_WD_HOST_CLIENT_ID);
- if (ret < 0) {
- dev_info(dev->dev, "wd: failed link client\n");
- return ret;
- }
-
- ret = mei_cl_connect(cl, me_cl, NULL);
- if (ret) {
- dev_err(dev->dev, "wd: failed to connect = %d\n", ret);
- mei_cl_unlink(cl);
- return ret;
- }
-
- ret = mei_watchdog_register(dev);
- if (ret) {
- mei_cl_disconnect(cl);
- mei_cl_unlink(cl);
- }
- return ret;
-}
-
-/**
- * mei_wd_send - sends watch dog message to fw.
- *
- * @dev: the device structure
- *
- * Return: 0 if success,
- * -EIO when message send fails
- * -EINVAL when invalid message is to be sent
- * -ENODEV on flow control failure
- */
-int mei_wd_send(struct mei_device *dev)
-{
- struct mei_cl *cl = &dev->wd_cl;
- struct mei_msg_hdr hdr;
- int ret;
-
- hdr.host_addr = cl->host_client_id;
- hdr.me_addr = mei_cl_me_id(cl);
- hdr.msg_complete = 1;
- hdr.reserved = 0;
- hdr.internal = 0;
-
- if (!memcmp(dev->wd_data, mei_start_wd_params, MEI_WD_HDR_SIZE))
- hdr.length = MEI_WD_START_MSG_SIZE;
- else if (!memcmp(dev->wd_data, mei_stop_wd_params, MEI_WD_HDR_SIZE))
- hdr.length = MEI_WD_STOP_MSG_SIZE;
- else {
- dev_err(dev->dev, "wd: invalid message is to be sent, aborting\n");
- return -EINVAL;
- }
-
- ret = mei_write_message(dev, &hdr, dev->wd_data);
- if (ret) {
- dev_err(dev->dev, "wd: write message failed\n");
- return ret;
- }
-
- ret = mei_cl_flow_ctrl_reduce(cl);
- if (ret) {
- dev_err(dev->dev, "wd: flow_ctrl_reduce failed.\n");
- return ret;
- }
-
- return 0;
-}
-
-/**
- * mei_wd_stop - sends watchdog stop message to fw.
- *
- * @dev: the device structure
- *
- * Return: 0 if success
- * on error:
- * -EIO when message send fails
- * -EINVAL when invalid message is to be sent
- * -ETIME on message timeout
- */
-int mei_wd_stop(struct mei_device *dev)
-{
- struct mei_cl *cl = &dev->wd_cl;
- int ret;
-
- if (!mei_cl_is_connected(cl) ||
- dev->wd_state != MEI_WD_RUNNING)
- return 0;
-
- memcpy(dev->wd_data, mei_stop_wd_params, MEI_WD_STOP_MSG_SIZE);
-
- dev->wd_state = MEI_WD_STOPPING;
-
- ret = mei_cl_flow_ctrl_creds(cl);
- if (ret < 0)
- goto err;
-
- if (ret && mei_hbuf_acquire(dev)) {
- ret = mei_wd_send(dev);
- if (ret)
- goto err;
- dev->wd_pending = false;
- } else {
- dev->wd_pending = true;
- }
-
- mutex_unlock(&dev->device_lock);
-
- ret = wait_event_timeout(dev->wait_stop_wd,
- dev->wd_state == MEI_WD_IDLE,
- msecs_to_jiffies(MEI_WD_STOP_TIMEOUT));
- mutex_lock(&dev->device_lock);
- if (dev->wd_state != MEI_WD_IDLE) {
- /* timeout */
- ret = -ETIME;
- dev_warn(dev->dev, "wd: stop failed to complete ret=%d\n", ret);
- goto err;
- }
- dev_dbg(dev->dev, "wd: stop completed after %u msec\n",
- MEI_WD_STOP_TIMEOUT - jiffies_to_msecs(ret));
- return 0;
-err:
- return ret;
-}
-
-/**
- * mei_wd_ops_start - wd start command from the watchdog core.
- *
- * @wd_dev: watchdog device struct
- *
- * Return: 0 if success, negative errno code for failure
- */
-static int mei_wd_ops_start(struct watchdog_device *wd_dev)
-{
- struct mei_device *dev;
- struct mei_cl *cl;
- int err = -ENODEV;
-
- dev = watchdog_get_drvdata(wd_dev);
- if (!dev)
- return -ENODEV;
-
- cl = &dev->wd_cl;
-
- mutex_lock(&dev->device_lock);
-
- if (dev->dev_state != MEI_DEV_ENABLED) {
- dev_dbg(dev->dev, "wd: dev_state != MEI_DEV_ENABLED dev_state = %s\n",
- mei_dev_state_str(dev->dev_state));
- goto end_unlock;
- }
-
- if (!mei_cl_is_connected(cl)) {
- cl_dbg(dev, cl, "MEI Driver is not connected to Watchdog Client\n");
- goto end_unlock;
- }
-
- mei_wd_set_start_timeout(dev, dev->wd_timeout);
-
- err = 0;
-end_unlock:
- mutex_unlock(&dev->device_lock);
- return err;
-}
-
-/**
- * mei_wd_ops_stop - wd stop command from the watchdog core.
- *
- * @wd_dev: watchdog device struct
- *
- * Return: 0 if success, negative errno code for failure
- */
-static int mei_wd_ops_stop(struct watchdog_device *wd_dev)
-{
- struct mei_device *dev;
-
- dev = watchdog_get_drvdata(wd_dev);
- if (!dev)
- return -ENODEV;
-
- mutex_lock(&dev->device_lock);
- mei_wd_stop(dev);
- mutex_unlock(&dev->device_lock);
-
- return 0;
-}
-
-/**
- * mei_wd_ops_ping - wd ping command from the watchdog core.
- *
- * @wd_dev: watchdog device struct
- *
- * Return: 0 if success, negative errno code for failure
- */
-static int mei_wd_ops_ping(struct watchdog_device *wd_dev)
-{
- struct mei_device *dev;
- struct mei_cl *cl;
- int ret;
-
- dev = watchdog_get_drvdata(wd_dev);
- if (!dev)
- return -ENODEV;
-
- cl = &dev->wd_cl;
-
- mutex_lock(&dev->device_lock);
-
- if (!mei_cl_is_connected(cl)) {
- cl_err(dev, cl, "wd: not connected.\n");
- ret = -ENODEV;
- goto end;
- }
-
- dev->wd_state = MEI_WD_RUNNING;
-
- ret = mei_cl_flow_ctrl_creds(cl);
- if (ret < 0)
- goto end;
-
- /* Check if we can send the ping to HW*/
- if (ret && mei_hbuf_acquire(dev)) {
- dev_dbg(dev->dev, "wd: sending ping\n");
-
- ret = mei_wd_send(dev);
- if (ret)
- goto end;
- dev->wd_pending = false;
- } else {
- dev->wd_pending = true;
- }
-
-end:
- mutex_unlock(&dev->device_lock);
- return ret;
-}
-
-/**
- * mei_wd_ops_set_timeout - wd set timeout command from the watchdog core.
- *
- * @wd_dev: watchdog device struct
- * @timeout: timeout value to set
- *
- * Return: 0 if success, negative errno code for failure
- */
-static int mei_wd_ops_set_timeout(struct watchdog_device *wd_dev,
- unsigned int timeout)
-{
- struct mei_device *dev;
-
- dev = watchdog_get_drvdata(wd_dev);
- if (!dev)
- return -ENODEV;
-
- /* Check Timeout value */
- if (timeout < MEI_WD_MIN_TIMEOUT || timeout > MEI_WD_MAX_TIMEOUT)
- return -EINVAL;
-
- mutex_lock(&dev->device_lock);
-
- dev->wd_timeout = timeout;
- wd_dev->timeout = timeout;
- mei_wd_set_start_timeout(dev, dev->wd_timeout);
-
- mutex_unlock(&dev->device_lock);
-
- return 0;
-}
-
-/*
- * Watchdog Device structs
- */
-static const struct watchdog_ops wd_ops = {
- .owner = THIS_MODULE,
- .start = mei_wd_ops_start,
- .stop = mei_wd_ops_stop,
- .ping = mei_wd_ops_ping,
- .set_timeout = mei_wd_ops_set_timeout,
-};
-static const struct watchdog_info wd_info = {
- .identity = INTEL_AMT_WATCHDOG_ID,
- .options = WDIOF_KEEPALIVEPING |
- WDIOF_SETTIMEOUT |
- WDIOF_ALARMONLY,
-};
-
-static struct watchdog_device amt_wd_dev = {
- .info = &wd_info,
- .ops = &wd_ops,
- .timeout = MEI_WD_DEFAULT_TIMEOUT,
- .min_timeout = MEI_WD_MIN_TIMEOUT,
- .max_timeout = MEI_WD_MAX_TIMEOUT,
-};
-
-
-int mei_watchdog_register(struct mei_device *dev)
-{
-
- int ret;
-
- amt_wd_dev.parent = dev->dev;
- /* unlock to perserve correct locking order */
- mutex_unlock(&dev->device_lock);
- ret = watchdog_register_device(&amt_wd_dev);
- mutex_lock(&dev->device_lock);
- if (ret) {
- dev_err(dev->dev, "wd: unable to register watchdog device = %d.\n",
- ret);
- return ret;
- }
-
- dev_dbg(dev->dev, "wd: successfully register watchdog interface.\n");
- watchdog_set_drvdata(&amt_wd_dev, dev);
- return 0;
-}
-
-void mei_watchdog_unregister(struct mei_device *dev)
-{
- if (watchdog_get_drvdata(&amt_wd_dev) == NULL)
- return;
-
- watchdog_set_drvdata(&amt_wd_dev, NULL);
- watchdog_unregister_device(&amt_wd_dev);
-}
-
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [char-misc-next v4 2/7] watchdog: mei_wdt: implement MEI iAMT watchdog driver
2016-01-07 22:49 [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver Tomas Winkler
2016-01-07 22:49 ` [char-misc-next v4 1/7] mei: wd: drop the watchdog code from the core mei driver Tomas Winkler
@ 2016-01-07 22:49 ` Tomas Winkler
2016-01-07 22:49 ` [char-misc-next v4 3/7] watchdog: mei_wdt: add status debugfs entry Tomas Winkler
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Tomas Winkler @ 2016-01-07 22:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Wim Van Sebroeck, Guenter Roeck
Cc: Alexander Usyskin, linux-watchdog, linux-kernel, Tomas Winkler
Create a driver with the generic watchdog interface
for the MEI iAMT watchdog device.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: The watchdog device is no longer dynamically allocated in separate structure
V3: Revert back to dynamically allocated watchdog device wrapper
V4: Rebase the code over patchset : "watchdog: Replace driver based refcounting"
drop mei_wdt_dev structure
Documentation/misc-devices/mei/mei.txt | 12 +-
MAINTAINERS | 1 +
drivers/watchdog/Kconfig | 15 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/mei_wdt.c | 404 +++++++++++++++++++++++++++++++++
5 files changed, 427 insertions(+), 6 deletions(-)
create mode 100644 drivers/watchdog/mei_wdt.c
diff --git a/Documentation/misc-devices/mei/mei.txt b/Documentation/misc-devices/mei/mei.txt
index 91c1fa34f48b..2b80a0cd621f 100644
--- a/Documentation/misc-devices/mei/mei.txt
+++ b/Documentation/misc-devices/mei/mei.txt
@@ -231,15 +231,15 @@ IT knows when a platform crashes even when there is a hard failure on the host.
The Intel AMT Watchdog is composed of two parts:
1) Firmware feature - receives the heartbeats
and sends an event when the heartbeats stop.
- 2) Intel MEI driver - connects to the watchdog feature, configures the
- watchdog and sends the heartbeats.
+ 2) Intel MEI iAMT watchdog driver - connects to the watchdog feature,
+ configures the watchdog and sends the heartbeats.
-The Intel MEI driver uses the kernel watchdog API to configure the Intel AMT
-Watchdog and to send heartbeats to it. The default timeout of the
+The Intel iAMT watchdog MEI driver uses the kernel watchdog API to configure
+the Intel AMT Watchdog and to send heartbeats to it. The default timeout of the
watchdog is 120 seconds.
-If the Intel AMT Watchdog feature does not exist (i.e. the connection failed),
-the Intel MEI driver will disable the sending of heartbeats.
+If the Intel AMT is not enabled in the firmware then the watchdog client won't enumerate
+on the me client bus and watchdog devices won't be exposed.
Supported Chipsets
diff --git a/MAINTAINERS b/MAINTAINERS
index 9bff63cf326e..e655625c2c16 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5666,6 +5666,7 @@ S: Supported
F: include/uapi/linux/mei.h
F: include/linux/mei_cl_bus.h
F: drivers/misc/mei/*
+F: drivers/watchdog/mei_wdt.c
F: Documentation/misc-devices/mei/*
INTEL MIC DRIVERS (mic)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1c427beffadd..8ac51d69785c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1154,6 +1154,21 @@ config SBC_EPX_C3_WATCHDOG
To compile this driver as a module, choose M here: the
module will be called sbc_epx_c3.
+config INTEL_MEI_WDT
+ tristate "Intel MEI iAMT Watchdog"
+ depends on INTEL_MEI && X86
+ select WATCHDOG_CORE
+ ---help---
+ A device driver for the Intel MEI iAMT watchdog.
+
+ The Intel AMT Watchdog is an OS Health (Hang/Crash) watchdog.
+ Whenever the OS hangs or crashes, iAMT will send an event
+ to any subscriber to this event. The watchdog doesn't reset the
+ the platform.
+
+ To compile this driver as a module, choose M here:
+ the module will be called mei_wdt.
+
# M32R Architecture
# M68K Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 53d4827ddfe1..9069c9dd8aa8 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -123,6 +123,7 @@ obj-$(CONFIG_MACHZ_WDT) += machzwd.o
obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
+obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
# M32R Architecture
diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
new file mode 100644
index 000000000000..32e3e1d55ef3
--- /dev/null
+++ b/drivers/watchdog/mei_wdt.c
@@ -0,0 +1,404 @@
+/*
+ * Intel Management Engine Interface (Intel MEI) Linux driver
+ * Copyright (c) 2015, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/watchdog.h>
+
+#include <linux/uuid.h>
+#include <linux/mei_cl_bus.h>
+
+/*
+ * iAMT Watchdog Device
+ */
+#define INTEL_AMT_WATCHDOG_ID "iamt_wdt"
+
+#define MEI_WDT_DEFAULT_TIMEOUT 120 /* seconds */
+#define MEI_WDT_MIN_TIMEOUT 120 /* seconds */
+#define MEI_WDT_MAX_TIMEOUT 65535 /* seconds */
+
+/* Commands */
+#define MEI_MANAGEMENT_CONTROL 0x02
+
+/* MEI Management Control version number */
+#define MEI_MC_VERSION_NUMBER 0x10
+
+/* Sub Commands */
+#define MEI_MC_START_WD_TIMER_REQ 0x13
+#define MEI_MC_STOP_WD_TIMER_REQ 0x14
+
+/**
+ * enum mei_wdt_state - internal watchdog state
+ *
+ * @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
+ */
+enum mei_wdt_state {
+ MEI_WDT_IDLE,
+ MEI_WDT_START,
+ MEI_WDT_RUNNING,
+ MEI_WDT_STOPPING,
+};
+
+/**
+ * struct mei_wdt - mei watchdog driver
+ * @wdd: watchdog device
+ *
+ * @cldev: mei watchdog client device
+ * @state: watchdog internal state
+ * @timeout: watchdog current timeout
+ */
+struct mei_wdt {
+ struct watchdog_device wdd;
+
+ struct mei_cl_device *cldev;
+ enum mei_wdt_state state;
+ u16 timeout;
+};
+
+/*
+ * struct mei_mc_hdr - Management Control Command Header
+ *
+ * @command: Management Control (0x2)
+ * @bytecount: Number of bytes in the message beyond this byte
+ * @subcommand: Management Control Subcommand
+ * @versionnumber: Management Control Version (0x10)
+ */
+struct mei_mc_hdr {
+ u8 command;
+ u8 bytecount;
+ u8 subcommand;
+ u8 versionnumber;
+};
+
+/**
+ * struct mei_wdt_start_request watchdog start/ping
+ *
+ * @hdr: Management Control Command Header
+ * @timeout: timeout value
+ * @reserved: reserved (legacy)
+ */
+struct mei_wdt_start_request {
+ struct mei_mc_hdr hdr;
+ u16 timeout;
+ u8 reserved[17];
+} __packed;
+
+/**
+ * struct mei_wdt_stop_request - watchdog stop
+ *
+ * @hdr: Management Control Command Header
+ */
+struct mei_wdt_stop_request {
+ struct mei_mc_hdr hdr;
+} __packed;
+
+/**
+ * mei_wdt_ping - send wd start/ping command
+ *
+ * @wdt: mei watchdog device
+ *
+ * Return: 0 on success,
+ * negative errno code on failure
+ */
+static int mei_wdt_ping(struct mei_wdt *wdt)
+{
+ struct mei_wdt_start_request req;
+ const size_t req_len = sizeof(req);
+ int ret;
+
+ memset(&req, 0, req_len);
+ req.hdr.command = MEI_MANAGEMENT_CONTROL;
+ req.hdr.bytecount = req_len - offsetof(struct mei_mc_hdr, subcommand);
+ req.hdr.subcommand = MEI_MC_START_WD_TIMER_REQ;
+ req.hdr.versionnumber = MEI_MC_VERSION_NUMBER;
+ req.timeout = wdt->timeout;
+
+ ret = mei_cldev_send(wdt->cldev, (u8 *)&req, req_len);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+/**
+ * mei_wdt_stop - send wd stop command
+ *
+ * @wdt: mei watchdog device
+ *
+ * Return: 0 on success,
+ * negative errno code on failure
+ */
+static int mei_wdt_stop(struct mei_wdt *wdt)
+{
+ struct mei_wdt_stop_request req;
+ const size_t req_len = sizeof(req);
+ int ret;
+
+ memset(&req, 0, req_len);
+ req.hdr.command = MEI_MANAGEMENT_CONTROL;
+ req.hdr.bytecount = req_len - offsetof(struct mei_mc_hdr, subcommand);
+ req.hdr.subcommand = MEI_MC_STOP_WD_TIMER_REQ;
+ req.hdr.versionnumber = MEI_MC_VERSION_NUMBER;
+
+ ret = mei_cldev_send(wdt->cldev, (u8 *)&req, req_len);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+/**
+ * mei_wdt_ops_start - wd start command from the watchdog core.
+ *
+ * @wdd: watchdog device
+ *
+ * Return: 0 on success or -ENODEV;
+ */
+static int mei_wdt_ops_start(struct watchdog_device *wdd)
+{
+ struct mei_wdt *wdt = watchdog_get_drvdata(wdd);
+
+ wdt->state = MEI_WDT_START;
+ wdd->timeout = wdt->timeout;
+ return 0;
+}
+
+/**
+ * mei_wdt_ops_stop - wd stop command from the watchdog core.
+ *
+ * @wdd: watchdog device
+ *
+ * Return: 0 if success, negative errno code for failure
+ */
+static int mei_wdt_ops_stop(struct watchdog_device *wdd)
+{
+ struct mei_wdt *wdt = watchdog_get_drvdata(wdd);
+ int ret;
+
+ if (wdt->state != MEI_WDT_RUNNING)
+ return 0;
+
+ wdt->state = MEI_WDT_STOPPING;
+
+ ret = mei_wdt_stop(wdt);
+ if (ret)
+ return ret;
+
+ wdt->state = MEI_WDT_IDLE;
+
+ return 0;
+}
+
+/**
+ * mei_wdt_ops_ping - wd ping command from the watchdog core.
+ *
+ * @wdd: watchdog device
+ *
+ * Return: 0 if success, negative errno code on failure
+ */
+static int mei_wdt_ops_ping(struct watchdog_device *wdd)
+{
+ struct mei_wdt *wdt = watchdog_get_drvdata(wdd);
+ int ret;
+
+ if (wdt->state != MEI_WDT_START && wdt->state != MEI_WDT_RUNNING)
+ return 0;
+
+ ret = mei_wdt_ping(wdt);
+ if (ret)
+ return ret;
+
+ wdt->state = MEI_WDT_RUNNING;
+
+ return 0;
+}
+
+/**
+ * mei_wdt_ops_set_timeout - wd set timeout command from the watchdog core.
+ *
+ * @wdd: watchdog device
+ * @timeout: timeout value to set
+ *
+ * Return: 0 if success, negative errno code for failure
+ */
+static int mei_wdt_ops_set_timeout(struct watchdog_device *wdd,
+ unsigned int timeout)
+{
+
+ struct mei_wdt *wdt = watchdog_get_drvdata(wdd);
+
+ /* valid value is already checked by the caller */
+ wdt->timeout = timeout;
+ wdd->timeout = timeout;
+
+ return 0;
+}
+
+static const struct watchdog_ops wd_ops = {
+ .owner = THIS_MODULE,
+ .start = mei_wdt_ops_start,
+ .stop = mei_wdt_ops_stop,
+ .ping = mei_wdt_ops_ping,
+ .set_timeout = mei_wdt_ops_set_timeout,
+};
+
+/* not const as the firmware_version field need to be retrieved */
+static struct watchdog_info wd_info = {
+ .identity = INTEL_AMT_WATCHDOG_ID,
+ .options = WDIOF_KEEPALIVEPING |
+ WDIOF_SETTIMEOUT |
+ WDIOF_ALARMONLY,
+};
+
+/**
+ * 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);
+}
+
+/**
+ * mei_wdt_register - register with the watchdog subsystem
+ *
+ * @wdt: mei watchdog device
+ *
+ * Return: 0 if success, negative errno code for failure
+ */
+static int mei_wdt_register(struct mei_wdt *wdt)
+{
+ struct device *dev;
+ int ret;
+
+ if (!wdt || !wdt->cldev)
+ return -EINVAL;
+
+ dev = &wdt->cldev->dev;
+
+ wdt->wdd.info = &wd_info;
+ wdt->wdd.ops = &wd_ops;
+ wdt->wdd.parent = dev;
+ wdt->wdd.timeout = MEI_WDT_DEFAULT_TIMEOUT;
+ wdt->wdd.min_timeout = MEI_WDT_MIN_TIMEOUT;
+ wdt->wdd.max_timeout = MEI_WDT_MAX_TIMEOUT;
+
+ watchdog_set_drvdata(&wdt->wdd, wdt);
+ ret = watchdog_register_device(&wdt->wdd);
+ if (ret) {
+ dev_err(dev, "unable to register watchdog device = %d.\n", ret);
+ watchdog_set_drvdata(&wdt->wdd, NULL);
+ }
+
+ return ret;
+}
+
+static int mei_wdt_probe(struct mei_cl_device *cldev,
+ const struct mei_cl_device_id *id)
+{
+ struct mei_wdt *wdt;
+ int ret;
+
+ wdt = kzalloc(sizeof(struct mei_wdt), GFP_KERNEL);
+ if (!wdt)
+ return -ENOMEM;
+
+ wdt->timeout = MEI_WDT_DEFAULT_TIMEOUT;
+ wdt->state = MEI_WDT_IDLE;
+ wdt->cldev = cldev;
+ mei_cldev_set_drvdata(cldev, wdt);
+
+ ret = mei_cldev_enable(cldev);
+ if (ret < 0) {
+ dev_err(&cldev->dev, "Could not enable cl device\n");
+ goto err_out;
+ }
+
+ wd_info.firmware_version = mei_cldev_ver(cldev);
+
+ ret = mei_wdt_register(wdt);
+ if (ret)
+ goto err_disable;
+
+ return 0;
+
+err_disable:
+ mei_cldev_disable(cldev);
+
+err_out:
+ kfree(wdt);
+
+ return ret;
+}
+
+static int mei_wdt_remove(struct mei_cl_device *cldev)
+{
+ struct mei_wdt *wdt = mei_cldev_get_drvdata(cldev);
+
+ mei_wdt_unregister(wdt);
+
+ mei_cldev_disable(cldev);
+
+ kfree(wdt);
+
+ return 0;
+}
+
+#define MEI_UUID_WD UUID_LE(0x05B79A6F, 0x4628, 0x4D7F, \
+ 0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
+
+static struct mei_cl_device_id mei_wdt_tbl[] = {
+ { .uuid = MEI_UUID_WD, .version = 0x1},
+ /* required last entry */
+ { }
+};
+MODULE_DEVICE_TABLE(mei, mei_wdt_tbl);
+
+static struct mei_cl_driver mei_wdt_driver = {
+ .id_table = mei_wdt_tbl,
+ .name = KBUILD_MODNAME,
+
+ .probe = mei_wdt_probe,
+ .remove = mei_wdt_remove,
+};
+
+static int __init mei_wdt_init(void)
+{
+ int ret;
+
+ ret = mei_cldev_driver_register(&mei_wdt_driver);
+ if (ret) {
+ pr_err(KBUILD_MODNAME ": module registration failed\n");
+ return ret;
+ }
+ return 0;
+}
+
+static void __exit mei_wdt_exit(void)
+{
+ mei_cldev_driver_unregister(&mei_wdt_driver);
+}
+
+module_init(mei_wdt_init);
+module_exit(mei_wdt_exit);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Device driver for Intel MEI iAMT watchdog");
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [char-misc-next v4 3/7] watchdog: mei_wdt: add status debugfs entry
2016-01-07 22:49 [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver Tomas Winkler
2016-01-07 22:49 ` [char-misc-next v4 1/7] mei: wd: drop the watchdog code from the core mei driver Tomas Winkler
2016-01-07 22:49 ` [char-misc-next v4 2/7] watchdog: mei_wdt: implement MEI iAMT watchdog driver Tomas Winkler
@ 2016-01-07 22:49 ` Tomas Winkler
2016-01-07 22:49 ` [char-misc-next v4 4/7] mei: bus: whitelist the watchdog client Tomas Winkler
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Tomas Winkler @ 2016-01-07 22:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Wim Van Sebroeck, Guenter Roeck
Cc: Alexander Usyskin, linux-watchdog, linux-kernel, Tomas Winkler
Add entry for displaying current watchdog internal state
cat <debugfs>/mei_wdt/state
IDLE|START|RUNNING|STOPPING
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: new in the series
V3: rebase
V4: cleanup the code
drivers/watchdog/mei_wdt.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
index 32e3e1d55ef3..e7e3f144f2b0 100644
--- a/drivers/watchdog/mei_wdt.c
+++ b/drivers/watchdog/mei_wdt.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/interrupt.h>
+#include <linux/debugfs.h>
#include <linux/watchdog.h>
#include <linux/uuid.h>
@@ -54,6 +55,24 @@ enum mei_wdt_state {
MEI_WDT_STOPPING,
};
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+static const char *mei_wdt_state_str(enum mei_wdt_state state)
+{
+ switch (state) {
+ case MEI_WDT_IDLE:
+ return "IDLE";
+ case MEI_WDT_START:
+ return "START";
+ case MEI_WDT_RUNNING:
+ return "RUNNING";
+ case MEI_WDT_STOPPING:
+ return "STOPPING";
+ default:
+ return "unknown";
+ }
+}
+#endif /* CONFIG_DEBUG_FS */
+
/**
* struct mei_wdt - mei watchdog driver
* @wdd: watchdog device
@@ -61,6 +80,8 @@ enum mei_wdt_state {
* @cldev: mei watchdog client device
* @state: watchdog internal state
* @timeout: watchdog current timeout
+ *
+ * @dbgfs_dir: debugfs dir entry
*/
struct mei_wdt {
struct watchdog_device wdd;
@@ -68,6 +89,10 @@ struct mei_wdt {
struct mei_cl_device *cldev;
enum mei_wdt_state state;
u16 timeout;
+
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+ struct dentry *dbgfs_dir;
+#endif /* CONFIG_DEBUG_FS */
};
/*
@@ -310,6 +335,63 @@ static int mei_wdt_register(struct mei_wdt *wdt)
return ret;
}
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+
+static ssize_t mei_dbgfs_read_state(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[bufsz];
+ ssize_t pos;
+
+ pos = scnprintf(buf, bufsz, "state: %s\n",
+ mei_wdt_state_str(wdt->state));
+
+ return simple_read_from_buffer(ubuf, cnt, ppos, buf, pos);
+}
+
+static const struct file_operations dbgfs_fops_state = {
+ .open = simple_open,
+ .read = mei_dbgfs_read_state,
+ .llseek = generic_file_llseek,
+};
+
+static void dbgfs_unregister(struct mei_wdt *wdt)
+{
+ debugfs_remove_recursive(wdt->dbgfs_dir);
+ wdt->dbgfs_dir = NULL;
+}
+
+static int dbgfs_register(struct mei_wdt *wdt)
+{
+ struct dentry *dir, *f;
+
+ dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
+ if (!dir)
+ return -ENOMEM;
+
+ wdt->dbgfs_dir = dir;
+ f = debugfs_create_file("state", S_IRUSR, dir, wdt, &dbgfs_fops_state);
+ if (!f)
+ goto err;
+
+ return 0;
+err:
+ dbgfs_unregister(wdt);
+ return -ENODEV;
+}
+
+#else
+
+static inline void dbgfs_unregister(struct mei_wdt *wdt) {}
+
+static inline int dbgfs_register(struct mei_wdt *wdt)
+{
+ return 0;
+}
+#endif /* CONFIG_DEBUG_FS */
+
static int mei_wdt_probe(struct mei_cl_device *cldev,
const struct mei_cl_device_id *id)
{
@@ -337,6 +419,9 @@ static int mei_wdt_probe(struct mei_cl_device *cldev,
if (ret)
goto err_disable;
+ if (dbgfs_register(wdt))
+ dev_warn(&cldev->dev, "cannot register debugfs\n");
+
return 0;
err_disable:
@@ -356,6 +441,8 @@ static int mei_wdt_remove(struct mei_cl_device *cldev)
mei_cldev_disable(cldev);
+ dbgfs_unregister(wdt);
+
kfree(wdt);
return 0;
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [char-misc-next v4 4/7] mei: bus: whitelist the watchdog client
2016-01-07 22:49 [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver Tomas Winkler
` (2 preceding siblings ...)
2016-01-07 22:49 ` [char-misc-next v4 3/7] watchdog: mei_wdt: add status debugfs entry Tomas Winkler
@ 2016-01-07 22:49 ` Tomas Winkler
2016-01-07 22:49 ` [char-misc-next v4 5/7] watchdog: mei_wdt: register wd device only if required Tomas Winkler
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Tomas Winkler @ 2016-01-07 22:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Wim Van Sebroeck, Guenter Roeck
Cc: Alexander Usyskin, linux-watchdog, linux-kernel, Tomas Winkler
The iAMT WD client has to be whitelisted sice it has two connections
and is filtered out by number_of_connections fixup.
Also the API has changed for BDW and SKL but firmware haven't updated
the protocol version.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
V2-V4: resend
drivers/misc/mei/bus-fixup.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
index b2d2a6ea576c..b87323f4bb14 100644
--- a/drivers/misc/mei/bus-fixup.c
+++ b/drivers/misc/mei/bus-fixup.c
@@ -35,6 +35,9 @@ static const uuid_le mei_nfc_info_guid = MEI_UUID_NFC_INFO;
#define MEI_UUID_NFC_HCI UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50, \
0x94, 0xd4, 0x50, 0x26, 0x67, 0x23, 0x77, 0x5c)
+#define MEI_UUID_WD UUID_LE(0x05B79A6F, 0x4628, 0x4D7F, \
+ 0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
+
#define MEI_UUID_ANY NULL_UUID_LE
/**
@@ -66,6 +69,31 @@ static void blacklist(struct mei_cl_device *cldev)
cldev->do_match = 0;
}
+/**
+ * mei_wd - wd client on the bus, change protocol version
+ * as the API has changed.
+ *
+ * @cldev: me clients device
+ */
+#if IS_ENABLED(CONFIG_INTEL_MEI_ME)
+#include <linux/pci.h>
+#include "hw-me-regs.h"
+static void mei_wd(struct mei_cl_device *cldev)
+{
+ struct pci_dev *pdev = to_pci_dev(cldev->dev.parent);
+
+ dev_dbg(&cldev->dev, "running hook %s\n", __func__);
+ if (pdev->device == MEI_DEV_ID_WPT_LP ||
+ pdev->device == MEI_DEV_ID_SPT ||
+ pdev->device == MEI_DEV_ID_SPT_H)
+ cldev->me_cl->props.protocol_version = 0x2;
+
+ cldev->do_match = 1;
+}
+#else
+static inline void mei_wd(struct mei_cl_device *cldev) {}
+#endif /* CONFIG_INTEL_MEI_ME */
+
struct mei_nfc_cmd {
u8 command;
u8 status;
@@ -280,6 +308,7 @@ static struct mei_fixup {
MEI_FIXUP(MEI_UUID_ANY, number_of_connections),
MEI_FIXUP(MEI_UUID_NFC_INFO, blacklist),
MEI_FIXUP(MEI_UUID_NFC_HCI, mei_nfc),
+ MEI_FIXUP(MEI_UUID_WD, mei_wd),
};
/**
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [char-misc-next v4 5/7] watchdog: mei_wdt: register wd device only if required
2016-01-07 22:49 [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver Tomas Winkler
` (3 preceding siblings ...)
2016-01-07 22:49 ` [char-misc-next v4 4/7] mei: bus: whitelist the watchdog client Tomas Winkler
@ 2016-01-07 22:49 ` Tomas Winkler
2016-01-17 17:13 ` [char-misc-next, v4, " Guenter Roeck
2016-01-07 22:49 ` [char-misc-next v4 6/7] watchdog: mei_wdt: add activation debugfs entry Tomas Winkler
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Tomas Winkler @ 2016-01-07 22:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Wim Van Sebroeck, Guenter Roeck
Cc: Alexander Usyskin, linux-watchdog, linux-kernel, Tomas Winkler
From: Alexander Usyskin <alexander.usyskin@intel.com>
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.
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"
drivers/watchdog/mei_wdt.c | 196 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 187 insertions(+), 9 deletions(-)
diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
index e7e3f144f2b0..85b27fc5d4ec 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,102 @@ 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;
+ }
+
+ 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 +559,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 +574,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 +609,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 +630,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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required
2016-01-07 22:49 ` [char-misc-next v4 5/7] watchdog: mei_wdt: register wd device only if required Tomas Winkler
@ 2016-01-17 17:13 ` Guenter Roeck
2016-01-17 20:54 ` Winkler, Tomas
0 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2016-01-17 17:13 UTC (permalink / raw)
To: Winkler, Tomas
Cc: Greg Kroah-Hartman, Wim Van Sebroeck, Alexander Usyskin,
linux-watchdog, linux-kernel
Hi Tomas,
On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
>
> 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.
>
> 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"
>
> drivers/watchdog/mei_wdt.c | 196 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 187 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> index e7e3f144f2b0..85b27fc5d4ec 100644
> --- a/drivers/watchdog/mei_wdt.c
> +++ b/drivers/watchdog/mei_wdt.c
>
[ ... ]
> +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);
> +}
Registration is synchronous, unregistration is asynchronous.
Assuming that is on purpose, I think it warrants an explanation.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 17+ messages in thread* RE: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required
2016-01-17 17:13 ` [char-misc-next, v4, " Guenter Roeck
@ 2016-01-17 20:54 ` Winkler, Tomas
2016-01-17 21:47 ` Guenter Roeck
0 siblings, 1 reply; 17+ messages in thread
From: Winkler, Tomas @ 2016-01-17 20:54 UTC (permalink / raw)
To: Guenter Roeck
Cc: Greg Kroah-Hartman, Wim Van Sebroeck, Usyskin, Alexander,
linux-watchdog, linux-kernel
> -----Original Message-----
> From: Guenter Roeck [mailto:linux@roeck-us.net]
> Sent: Sunday, January 17, 2016 19:13
> To: Winkler, Tomas
> Cc: Greg Kroah-Hartman; Wim Van Sebroeck; Usyskin, Alexander; linux-
> watchdog@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only
> if required
>
> Hi Tomas,
>
> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
> > From: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> > 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.
> >
> > 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"
> >
> > drivers/watchdog/mei_wdt.c | 196
> ++++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 187 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> > index e7e3f144f2b0..85b27fc5d4ec 100644
> > --- a/drivers/watchdog/mei_wdt.c
> > +++ b/drivers/watchdog/mei_wdt.c
> >
> [ ... ]
>
> > +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);
> > +}
>
> Registration is synchronous, unregistration is asynchronous.
>
> Assuming that is on purpose, I think it warrants an explanation.
>
The unregistration is detected on response from the ping, which is run under same mutex as unregistration.
Tomas
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required
2016-01-17 20:54 ` Winkler, Tomas
@ 2016-01-17 21:47 ` Guenter Roeck
2016-01-18 13:19 ` Winkler, Tomas
0 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2016-01-17 21:47 UTC (permalink / raw)
To: Winkler, Tomas
Cc: Greg Kroah-Hartman, Wim Van Sebroeck, Usyskin, Alexander,
linux-watchdog, linux-kernel
On 01/17/2016 12:54 PM, Winkler, Tomas wrote:
>
>
>> -----Original Message-----
>> From: Guenter Roeck [mailto:linux@roeck-us.net]
>> Sent: Sunday, January 17, 2016 19:13
>> To: Winkler, Tomas
>> Cc: Greg Kroah-Hartman; Wim Van Sebroeck; Usyskin, Alexander; linux-
>> watchdog@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only
>> if required
>>
>> Hi Tomas,
>>
>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
>>> From: Alexander Usyskin <alexander.usyskin@intel.com>
>>>
>>> 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.
>>>
>>> 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"
>>>
>>> drivers/watchdog/mei_wdt.c | 196
>> ++++++++++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 187 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
>>> index e7e3f144f2b0..85b27fc5d4ec 100644
>>> --- a/drivers/watchdog/mei_wdt.c
>>> +++ b/drivers/watchdog/mei_wdt.c
>>>
>> [ ... ]
>>
>>> +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);
>>> +}
>>
>> Registration is synchronous, unregistration is asynchronous.
>>
>> Assuming that is on purpose, I think it warrants an explanation.
>>
> The unregistration is detected on response from the ping, which is run under same mutex as unregistration.
> Tomas
>
>
And that explains why registration can be synchronous and unregistration
has to be asynchronous ?
Guenter
^ permalink raw reply [flat|nested] 17+ messages in thread* RE: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required
2016-01-17 21:47 ` Guenter Roeck
@ 2016-01-18 13:19 ` Winkler, Tomas
2016-01-18 15:42 ` Guenter Roeck
0 siblings, 1 reply; 17+ messages in thread
From: Winkler, Tomas @ 2016-01-18 13:19 UTC (permalink / raw)
To: Guenter Roeck
Cc: Greg Kroah-Hartman, Wim Van Sebroeck, Usyskin, Alexander,
linux-watchdog, linux-kernel
> only
> >> if required
> >>
> >> Hi Tomas,
> >>
> >> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
> >>> From: Alexander Usyskin <alexander.usyskin@intel.com>
> >>>
> >>> 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.
> >>>
> >>> 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"
> >>>
> >>> drivers/watchdog/mei_wdt.c | 196
> >> ++++++++++++++++++++++++++++++++++++++++++---
> >>> 1 file changed, 187 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> >>> index e7e3f144f2b0..85b27fc5d4ec 100644
> >>> --- a/drivers/watchdog/mei_wdt.c
> >>> +++ b/drivers/watchdog/mei_wdt.c
> >>>
> >> [ ... ]
> >>
> >>> +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);
> >>> +}
> >>
> >> Registration is synchronous, unregistration is asynchronous.
> >>
> >> Assuming that is on purpose, I think it warrants an explanation.
> >>
> > The unregistration is detected on response from the ping, which is run under
> same mutex as unregistration.
> > Tomas
> >
> >
> And that explains why registration can be synchronous and unregistration
> has to be asynchronous ?
You need to connect the dots but yes.
The registration is run from the internal ping request (in probe) or from an internal event (in runtime), so the flow is not already locked by the watchdog mutex.
Hope it helps.
Tomas
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required
2016-01-18 13:19 ` Winkler, Tomas
@ 2016-01-18 15:42 ` Guenter Roeck
2016-01-18 19:36 ` Winkler, Tomas
0 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2016-01-18 15:42 UTC (permalink / raw)
To: Winkler, Tomas
Cc: Greg Kroah-Hartman, Wim Van Sebroeck, Usyskin, Alexander,
linux-watchdog, linux-kernel
On 01/18/2016 05:19 AM, Winkler, Tomas wrote:
>
>
>> only
>>>> if required
>>>>
>>>> Hi Tomas,
>>>>
>>>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
>>>>> From: Alexander Usyskin <alexander.usyskin@intel.com>
>>>>>
>>>>> 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.
>>>>>
>>>>> 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"
>>>>>
>>>>> drivers/watchdog/mei_wdt.c | 196
>>>> ++++++++++++++++++++++++++++++++++++++++++---
>>>>> 1 file changed, 187 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
>>>>> index e7e3f144f2b0..85b27fc5d4ec 100644
>>>>> --- a/drivers/watchdog/mei_wdt.c
>>>>> +++ b/drivers/watchdog/mei_wdt.c
>>>>>
>>>> [ ... ]
>>>>
>>>>> +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);
>>>>> +}
>>>>
>>>> Registration is synchronous, unregistration is asynchronous.
>>>>
>>>> Assuming that is on purpose, I think it warrants an explanation.
>>>>
>>> The unregistration is detected on response from the ping, which is run under
>> same mutex as unregistration.
>>> Tomas
>>>
>>>
>> And that explains why registration can be synchronous and unregistration
>> has to be asynchronous ?
>
> You need to connect the dots but yes.
> The registration is run from the internal ping request (in probe) or from an internal event (in runtime), so the flow is not already locked by the watchdog mutex.
> Hope it helps.
What I was asking for is a comment such as
"We can not unregister directly because a ping operation (triggered
through the watchdog subsystem) is pending and must be completed first."
Guenter
^ permalink raw reply [flat|nested] 17+ messages in thread* RE: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required
2016-01-18 15:42 ` Guenter Roeck
@ 2016-01-18 19:36 ` Winkler, Tomas
2016-01-18 20:21 ` Guenter Roeck
0 siblings, 1 reply; 17+ messages in thread
From: Winkler, Tomas @ 2016-01-18 19:36 UTC (permalink / raw)
To: Guenter Roeck
Cc: Greg Kroah-Hartman, Wim Van Sebroeck, Usyskin, Alexander,
linux-watchdog, linux-kernel
>
> On 01/18/2016 05:19 AM, Winkler, Tomas wrote:
> >
> >
> >> only
> >>>> if required
> >>>>
> >>>> Hi Tomas,
> >>>>
> >>>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
> >>>>> From: Alexander Usyskin <alexander.usyskin@intel.com>
> >>>>>
> >>>>> 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.
> >>>>>
> >>>>> 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"
> >>>>>
> >>>>> drivers/watchdog/mei_wdt.c | 196
> >>>> ++++++++++++++++++++++++++++++++++++++++++---
> >>>>> 1 file changed, 187 insertions(+), 9 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> >>>>> index e7e3f144f2b0..85b27fc5d4ec 100644
> >>>>> --- a/drivers/watchdog/mei_wdt.c
> >>>>> +++ b/drivers/watchdog/mei_wdt.c
> >>>>>
> >>>> [ ... ]
> >>>>
> >>>>> +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);
> >>>>> +}
> >>>>
> >>>> Registration is synchronous, unregistration is asynchronous.
> >>>>
> >>>> Assuming that is on purpose, I think it warrants an explanation.
> >>>>
> >>> The unregistration is detected on response from the ping, which is run under
> >> same mutex as unregistration.
> >>> Tomas
> >>>
> >>>
> >> And that explains why registration can be synchronous and unregistration
> >> has to be asynchronous ?
> >
> > You need to connect the dots but yes.
> > The registration is run from the internal ping request (in probe) or from an
> internal event (in runtime), so the flow is not already locked by the watchdog
> mutex.
> > Hope it helps.
>
> What I was asking for is a comment such as
>
> "We can not unregister directly because a ping operation (triggered
> through the watchdog subsystem) is pending and must be completed first."
You can put it also that way, but in the bottom line you will get deadlock.
According your comment I'm guessing that you are asking me to update the commit message, please be more direct,
I'm not native English speaker and may miss little nuances.
Thanks
Tomas
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required
2016-01-18 19:36 ` Winkler, Tomas
@ 2016-01-18 20:21 ` Guenter Roeck
2016-01-18 21:52 ` Winkler, Tomas
0 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2016-01-18 20:21 UTC (permalink / raw)
To: Winkler, Tomas
Cc: Greg Kroah-Hartman, Wim Van Sebroeck, Usyskin, Alexander,
linux-watchdog, linux-kernel
On 01/18/2016 11:36 AM, Winkler, Tomas wrote:
>>
>> On 01/18/2016 05:19 AM, Winkler, Tomas wrote:
>>>
>>>
>>>> only
>>>>>> if required
>>>>>>
>>>>>> Hi Tomas,
>>>>>>
>>>>>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
>>>>>>> From: Alexander Usyskin <alexander.usyskin@intel.com>
>>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>> 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"
>>>>>>>
>>>>>>> drivers/watchdog/mei_wdt.c | 196
>>>>>> ++++++++++++++++++++++++++++++++++++++++++---
>>>>>>> 1 file changed, 187 insertions(+), 9 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
>>>>>>> index e7e3f144f2b0..85b27fc5d4ec 100644
>>>>>>> --- a/drivers/watchdog/mei_wdt.c
>>>>>>> +++ b/drivers/watchdog/mei_wdt.c
>>>>>>>
>>>>>> [ ... ]
>>>>>>
>>>>>>> +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);
>>>>>>> +}
>>>>>>
>>>>>> Registration is synchronous, unregistration is asynchronous.
>>>>>>
>>>>>> Assuming that is on purpose, I think it warrants an explanation.
>>>>>>
>>>>> The unregistration is detected on response from the ping, which is run under
>>>> same mutex as unregistration.
>>>>> Tomas
>>>>>
>>>>>
>>>> And that explains why registration can be synchronous and unregistration
>>>> has to be asynchronous ?
>>>
>>> You need to connect the dots but yes.
>>> The registration is run from the internal ping request (in probe) or from an
>> internal event (in runtime), so the flow is not already locked by the watchdog
>> mutex.
>>> Hope it helps.
>>
>> What I was asking for is a comment such as
>>
>> "We can not unregister directly because a ping operation (triggered
>> through the watchdog subsystem) is pending and must be completed first."
>
> You can put it also that way, but in the bottom line you will get deadlock.
Yes, understood. Maybe that should be part of the comment.
> According your comment I'm guessing that you are asking me to update the commit message, please be more direct,
> I'm not native English speaker and may miss little nuances.
>
Hi Tomas,
please add a comment into the source code, describing why unregistration has to be
asynchronous. It took me a while to understand the context, and we want to make
sure that others don't have to repeat that exercise.
Other than that, the series is fine with me, except that the patches
affecting the mei directory don't apply to the current mainline
as of this morning (possibly due to some other changes in that directory).
I would suggest to add the comment, wait for -rc1, rebase, and re-send
the series with my Acked-by: added to all patches.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 17+ messages in thread* RE: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required
2016-01-18 20:21 ` Guenter Roeck
@ 2016-01-18 21:52 ` Winkler, Tomas
0 siblings, 0 replies; 17+ messages in thread
From: Winkler, Tomas @ 2016-01-18 21:52 UTC (permalink / raw)
To: Guenter Roeck
Cc: Greg Kroah-Hartman, Wim Van Sebroeck, Usyskin, Alexander,
linux-watchdog, linux-kernel
>
> On 01/18/2016 11:36 AM, Winkler, Tomas wrote:
> >>
> >> On 01/18/2016 05:19 AM, Winkler, Tomas wrote:
> >>>
> >>>
> >>>> only
> >>>>>> if required
> >>>>>>
> >>>>>> Hi Tomas,
> >>>>>>
> >>>>>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
> >>>>>>> From: Alexander Usyskin <alexander.usyskin@intel.com>
> >>>>>>>
> >>>>>>> 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.
> >>>>>>>
> >>>>>>> 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"
> >>>>>>>
> >>>>>>> drivers/watchdog/mei_wdt.c | 196
> >>>>>> ++++++++++++++++++++++++++++++++++++++++++---
> >>>>>>> 1 file changed, 187 insertions(+), 9 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> >>>>>>> index e7e3f144f2b0..85b27fc5d4ec 100644
> >>>>>>> --- a/drivers/watchdog/mei_wdt.c
> >>>>>>> +++ b/drivers/watchdog/mei_wdt.c
> >>>>>>>
> >>>>>> [ ... ]
> >>>>>>
> >>>>>>> +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);
> >>>>>>> +}
> >>>>>>
> >>>>>> Registration is synchronous, unregistration is asynchronous.
> >>>>>>
> >>>>>> Assuming that is on purpose, I think it warrants an explanation.
> >>>>>>
> >>>>> The unregistration is detected on response from the ping, which is run
> under
> >>>> same mutex as unregistration.
> >>>>> Tomas
> >>>>>
> >>>>>
> >>>> And that explains why registration can be synchronous and unregistration
> >>>> has to be asynchronous ?
> >>>
> >>> You need to connect the dots but yes.
> >>> The registration is run from the internal ping request (in probe) or from an
> >> internal event (in runtime), so the flow is not already locked by the watchdog
> >> mutex.
> >>> Hope it helps.
> >>
> >> What I was asking for is a comment such as
> >>
> >> "We can not unregister directly because a ping operation (triggered
> >> through the watchdog subsystem) is pending and must be completed first."
> >
> > You can put it also that way, but in the bottom line you will get deadlock.
>
> Yes, understood. Maybe that should be part of the comment.
>
> > According your comment I'm guessing that you are asking me to update the
> commit message, please be more direct,
> > I'm not native English speaker and may miss little nuances.
> >
> Hi Tomas,
>
> please add a comment into the source code, describing why unregistration has to
> be
> asynchronous. It took me a while to understand the context, and we want to
> make
> sure that others don't have to repeat that exercise.
Sure, no problem.
> Other than that, the series is fine with me, except that the patches
> affecting the mei directory don't apply to the current mainline
> as of this morning (possibly due to some other changes in that directory).
There are other pending patches that weren't merged yet and are not related to watchdog, we see if they require rebase after the merging window is closed.
>
> I would suggest to add the comment, wait for -rc1, rebase, and re-send
> the series with my Acked-by: added to all patches.
Thanks and I appreciate your time reviewing this series.
Tomas
^ permalink raw reply [flat|nested] 17+ messages in thread
* [char-misc-next v4 6/7] watchdog: mei_wdt: add activation debugfs entry
2016-01-07 22:49 [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver Tomas Winkler
` (4 preceding siblings ...)
2016-01-07 22:49 ` [char-misc-next v4 5/7] watchdog: mei_wdt: register wd device only if required Tomas Winkler
@ 2016-01-07 22:49 ` Tomas Winkler
2016-01-07 22:49 ` [char-misc-next v4 7/7] watchdog: mei_wdt: re-register device on event Tomas Winkler
2016-02-07 6:11 ` [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver Greg Kroah-Hartman
7 siblings, 0 replies; 17+ messages in thread
From: Tomas Winkler @ 2016-01-07 22:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Wim Van Sebroeck, Guenter Roeck
Cc: Alexander Usyskin, linux-watchdog, linux-kernel, Tomas Winkler
Add entry for displaying whether the device has activated or
deactivated watchdog fw application.
cat <debugfs>/mei_wdt/activation
activated | deactivated
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"
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 85b27fc5d4ec..fe683e582566 100644
--- a/drivers/watchdog/mei_wdt.c
+++ b/drivers/watchdog/mei_wdt.c
@@ -493,6 +493,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[bufsz];
+ 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)
{
@@ -532,6 +554,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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [char-misc-next v4 7/7] watchdog: mei_wdt: re-register device on event
2016-01-07 22:49 [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver Tomas Winkler
` (5 preceding siblings ...)
2016-01-07 22:49 ` [char-misc-next v4 6/7] watchdog: mei_wdt: add activation debugfs entry Tomas Winkler
@ 2016-01-07 22:49 ` Tomas Winkler
2016-02-07 6:11 ` [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver Greg Kroah-Hartman
7 siblings, 0 replies; 17+ messages in thread
From: Tomas Winkler @ 2016-01-07 22:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Wim Van Sebroeck, Guenter Roeck
Cc: Alexander Usyskin, linux-watchdog, linux-kernel, Tomas Winkler
From: Alexander Usyskin <alexander.usyskin@intel.com>
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.
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"
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 fe683e582566..a49be916ac7f 100644
--- a/drivers/watchdog/mei_wdt.c
+++ b/drivers/watchdog/mei_wdt.c
@@ -477,6 +477,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
*
@@ -489,6 +504,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)
@@ -601,9 +619,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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver
2016-01-07 22:49 [char-misc-next v4 0/7] mei: create proper iAMT watchdog driver Tomas Winkler
` (6 preceding siblings ...)
2016-01-07 22:49 ` [char-misc-next v4 7/7] watchdog: mei_wdt: re-register device on event Tomas Winkler
@ 2016-02-07 6:11 ` Greg Kroah-Hartman
7 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2016-02-07 6:11 UTC (permalink / raw)
To: Tomas Winkler
Cc: Wim Van Sebroeck, Guenter Roeck, Alexander Usyskin,
linux-watchdog, linux-kernel
On Fri, Jan 08, 2016 at 12:49:20AM +0200, Tomas Winkler wrote:
> Instead of integrating the iAMT watchdog within the mei core driver
> we will create a watchdog device on the mei client bus and
> create a proper watchdog driver for it.
>
I applied the first 4 patches here, please fix up the rest and resend.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread