From dc844b0d99b8533d6174e5b9a369f7c2cdacfe66 Mon Sep 17 00:00:00 2001 From: Tomas Winkler Date: Mon, 11 Nov 2013 13:26:06 +0200 Subject: mei: remove flash_work_queue Cancel each work properly and remove flash_work_queue. Quoting documentation: In most situations flushing the entire workqueue is overkill; you merely need to know that a particular work item isn't queued and isn't running. In such cases you should use cancel_delayed_work_sync() or cancel_work_sync() instead. Signed-off-by: Tomas Winkler Signed-off-by: Greg Kroah-Hartman --- drivers/misc/mei/init.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'drivers/misc/mei/init.c') diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c index f7f3abbe12b6..83c879bf9967 100644 --- a/drivers/misc/mei/init.c +++ b/drivers/misc/mei/init.c @@ -131,6 +131,15 @@ err: } EXPORT_SYMBOL_GPL(mei_start); + +void mei_cancel_work(struct mei_device *dev) +{ + cancel_work_sync(&dev->init_work); + + cancel_delayed_work(&dev->timer_work); +} +EXPORT_SYMBOL_GPL(mei_cancel_work); + /** * mei_reset - resets host and fw. * @@ -215,16 +224,14 @@ void mei_stop(struct mei_device *dev) { dev_dbg(&dev->pdev->dev, "stopping the device.\n"); - flush_scheduled_work(); + mei_cancel_work(dev); - mutex_lock(&dev->device_lock); + mei_nfc_host_exit(dev); - cancel_delayed_work(&dev->timer_work); + mutex_lock(&dev->device_lock); mei_wd_stop(dev); - mei_nfc_host_exit(); - dev->dev_state = MEI_DEV_POWER_DOWN; mei_reset(dev, 0); -- cgit v1.2.3 From 544f94601409653f07ae6e22d4a39e3a90dceead Mon Sep 17 00:00:00 2001 From: Tomas Winkler Date: Wed, 8 Jan 2014 20:19:21 +0200 Subject: mei: do not run reset flow from the interrupt thread This fixes a potential deadlock in case of a firmware initiated reset mei_reset has a dialog with the interrupt thread hence it has to be run from an another work item Most of the mei_resets were called from mei_hbm_dispatch which is called in interrupt thread context so this function underwent major revamp. The error code is propagated to the interrupt thread and if needed the reset is scheduled from there. Signed-off-by: Tomas Winkler Signed-off-by: Alexander Usyskin Signed-off-by: Greg Kroah-Hartman --- drivers/misc/mei/init.c | 100 +++++++++++++++++++++++++++++------------------- 1 file changed, 61 insertions(+), 39 deletions(-) (limited to 'drivers/misc/mei/init.c') diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c index 83c879bf9967..87c077bae5d8 100644 --- a/drivers/misc/mei/init.c +++ b/drivers/misc/mei/init.c @@ -43,42 +43,6 @@ const char *mei_dev_state_str(int state) #undef MEI_DEV_STATE } -void mei_device_init(struct mei_device *dev) -{ - /* setup our list array */ - INIT_LIST_HEAD(&dev->file_list); - INIT_LIST_HEAD(&dev->device_list); - mutex_init(&dev->device_lock); - init_waitqueue_head(&dev->wait_hw_ready); - init_waitqueue_head(&dev->wait_recvd_msg); - init_waitqueue_head(&dev->wait_stop_wd); - dev->dev_state = MEI_DEV_INITIALIZING; - - mei_io_list_init(&dev->read_list); - mei_io_list_init(&dev->write_list); - mei_io_list_init(&dev->write_waiting_list); - mei_io_list_init(&dev->ctrl_wr_list); - mei_io_list_init(&dev->ctrl_rd_list); - - INIT_DELAYED_WORK(&dev->timer_work, mei_timer); - INIT_WORK(&dev->init_work, mei_host_client_init); - - 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); - - bitmap_zero(dev->host_clients_map, MEI_CLIENTS_MAX); - dev->open_handle_count = 0; - - /* - * Reserving the first client ID - * 0: Reserved for MEI Bus Message communications - */ - bitmap_set(dev->host_clients_map, 0, 1); -} -EXPORT_SYMBOL_GPL(mei_device_init); - /** * mei_start - initializes host and fw to start work. * @@ -131,10 +95,15 @@ err: } EXPORT_SYMBOL_GPL(mei_start); - +/** + * mei_cancel_work. Cancel mei background jobs + * + * @dev: the device structure + */ void mei_cancel_work(struct mei_device *dev) { cancel_work_sync(&dev->init_work); + cancel_work_sync(&dev->reset_work); cancel_delayed_work(&dev->timer_work); } @@ -215,11 +184,27 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled) dev->dev_state = MEI_DEV_INIT_CLIENTS; - mei_hbm_start_req(dev); - + ret = mei_hbm_start_req(dev); + if (ret) { + dev_err(&dev->pdev->dev, "hbm_start failed disabling the device\n"); + dev->dev_state = MEI_DEV_DISABLED; + return; + } } EXPORT_SYMBOL_GPL(mei_reset); +static void mei_reset_work(struct work_struct *work) +{ + struct mei_device *dev = + container_of(work, struct mei_device, reset_work); + + mutex_lock(&dev->device_lock); + + mei_reset(dev, true); + + mutex_unlock(&dev->device_lock); +} + void mei_stop(struct mei_device *dev) { dev_dbg(&dev->pdev->dev, "stopping the device.\n"); @@ -243,3 +228,40 @@ EXPORT_SYMBOL_GPL(mei_stop); +void mei_device_init(struct mei_device *dev) +{ + /* setup our list array */ + INIT_LIST_HEAD(&dev->file_list); + INIT_LIST_HEAD(&dev->device_list); + mutex_init(&dev->device_lock); + init_waitqueue_head(&dev->wait_hw_ready); + init_waitqueue_head(&dev->wait_recvd_msg); + init_waitqueue_head(&dev->wait_stop_wd); + dev->dev_state = MEI_DEV_INITIALIZING; + + mei_io_list_init(&dev->read_list); + mei_io_list_init(&dev->write_list); + mei_io_list_init(&dev->write_waiting_list); + mei_io_list_init(&dev->ctrl_wr_list); + mei_io_list_init(&dev->ctrl_rd_list); + + INIT_DELAYED_WORK(&dev->timer_work, mei_timer); + 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); + + bitmap_zero(dev->host_clients_map, MEI_CLIENTS_MAX); + dev->open_handle_count = 0; + + /* + * Reserving the first client ID + * 0: Reserved for MEI Bus Message communications + */ + bitmap_set(dev->host_clients_map, 0, 1); +} +EXPORT_SYMBOL_GPL(mei_device_init); + -- cgit v1.2.3 From 66ae460b13c31a176b41550259683c841a62af3e Mon Sep 17 00:00:00 2001 From: Tomas Winkler Date: Wed, 8 Jan 2014 20:19:22 +0200 Subject: mei: use hbm idle state to prevent spurious resets When reset is caused by hbm protocol mismatch or timeout we might end up in an endless reset loop and hbm protocol will never sync Cc: Signed-off-by: Tomas Winkler Signed-off-by: Alexander Usyskin Signed-off-by: Greg Kroah-Hartman --- drivers/misc/mei/init.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'drivers/misc/mei/init.c') diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c index 87c077bae5d8..c47fa273879e 100644 --- a/drivers/misc/mei/init.c +++ b/drivers/misc/mei/init.c @@ -129,14 +129,19 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled) dev_warn(&dev->pdev->dev, "unexpected reset: dev_state = %s\n", mei_dev_state_str(dev->dev_state)); + /* we're already in reset, cancel the init timer + * if the reset was called due the hbm protocol error + * we need to call it before hw start + * so the hbm watchdog won't kick in + */ + mei_hbm_idle(dev); + ret = mei_hw_reset(dev, interrupts_enabled); if (ret) { dev_err(&dev->pdev->dev, "hw reset failed disabling the device\n"); interrupts_enabled = false; - dev->dev_state = MEI_DEV_DISABLED; } - dev->hbm_state = MEI_HBM_IDLE; if (dev->dev_state != MEI_DEV_INITIALIZING && dev->dev_state != MEI_DEV_POWER_UP) { @@ -160,8 +165,6 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled) memset(&dev->wr_ext_msg, 0, sizeof(dev->wr_ext_msg)); } - /* we're already in reset, cancel the init timer */ - dev->init_clients_timer = 0; dev->me_clients_num = 0; dev->rd_msg_hdr = 0; @@ -169,6 +172,7 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled) if (!interrupts_enabled) { dev_dbg(&dev->pdev->dev, "intr not enabled end of reset\n"); + dev->dev_state = MEI_DEV_DISABLED; return; } -- cgit v1.2.3 From 33ec0826314734fc4f3c9bf37d12e98063339b31 Mon Sep 17 00:00:00 2001 From: Tomas Winkler Date: Sun, 12 Jan 2014 00:36:09 +0200 Subject: mei: revamp mei reset state machine 1. MEI_DEV_RESETTING device state spans only hardware reset flow while starting dev state is saved into a local variable for further reference, this let us to reduce big if statements in case we are trying to avoid nested resets 2. During initializations if the reset ended in MEI_DEV_DISABLED device state we bail out with -ENODEV 3. Remove redundant interrupts_enabled parameter as this can be deduced from the starting dev_state 4. mei_reset propagates error code to the caller 5. Add mei_restart function to wrap the pci resume Signed-off-by: Tomas Winkler Signed-off-by: Alexander Usyskin Signed-off-by: Greg Kroah-Hartman --- drivers/misc/mei/init.c | 209 +++++++++++++++++++++++++++++------------------- 1 file changed, 126 insertions(+), 83 deletions(-) (limited to 'drivers/misc/mei/init.c') diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c index c47fa273879e..059133d8caca 100644 --- a/drivers/misc/mei/init.c +++ b/drivers/misc/mei/init.c @@ -43,62 +43,13 @@ const char *mei_dev_state_str(int state) #undef MEI_DEV_STATE } -/** - * mei_start - initializes host and fw to start work. - * - * @dev: the device structure - * - * returns 0 on success, <0 on failure. - */ -int mei_start(struct mei_device *dev) -{ - mutex_lock(&dev->device_lock); - - /* acknowledge interrupt and stop interupts */ - mei_clear_interrupts(dev); - - mei_hw_config(dev); - - dev_dbg(&dev->pdev->dev, "reset in start the mei device.\n"); - - mei_reset(dev, 1); - - if (mei_hbm_start_wait(dev)) { - dev_err(&dev->pdev->dev, "HBM haven't started"); - goto err; - } - - if (!mei_host_is_ready(dev)) { - dev_err(&dev->pdev->dev, "host is not ready.\n"); - goto err; - } - - if (!mei_hw_is_ready(dev)) { - dev_err(&dev->pdev->dev, "ME is not ready.\n"); - goto err; - } - - if (!mei_hbm_version_is_supported(dev)) { - dev_dbg(&dev->pdev->dev, "MEI start failed.\n"); - goto err; - } - - dev_dbg(&dev->pdev->dev, "link layer has been established.\n"); - - mutex_unlock(&dev->device_lock); - return 0; -err: - dev_err(&dev->pdev->dev, "link layer initialization failed.\n"); - dev->dev_state = MEI_DEV_DISABLED; - mutex_unlock(&dev->device_lock); - return -ENODEV; -} -EXPORT_SYMBOL_GPL(mei_start); /** * mei_cancel_work. Cancel mei background jobs * * @dev: the device structure + * + * returns 0 on success or < 0 if the reset hasn't succeeded */ void mei_cancel_work(struct mei_device *dev) { @@ -113,21 +64,19 @@ EXPORT_SYMBOL_GPL(mei_cancel_work); * mei_reset - resets host and fw. * * @dev: the device structure - * @interrupts_enabled: if interrupt should be enabled after reset. */ -void mei_reset(struct mei_device *dev, int interrupts_enabled) +int mei_reset(struct mei_device *dev) { - bool unexpected; + enum mei_dev_state state = dev->dev_state; + bool interrupts_enabled; int ret; - unexpected = (dev->dev_state != MEI_DEV_INITIALIZING && - dev->dev_state != MEI_DEV_DISABLED && - dev->dev_state != MEI_DEV_POWER_DOWN && - dev->dev_state != MEI_DEV_POWER_UP); - - if (unexpected) + if (state != MEI_DEV_INITIALIZING && + state != MEI_DEV_DISABLED && + state != MEI_DEV_POWER_DOWN && + state != MEI_DEV_POWER_UP) dev_warn(&dev->pdev->dev, "unexpected reset: dev_state = %s\n", - mei_dev_state_str(dev->dev_state)); + mei_dev_state_str(state)); /* we're already in reset, cancel the init timer * if the reset was called due the hbm protocol error @@ -136,25 +85,23 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled) */ mei_hbm_idle(dev); - ret = mei_hw_reset(dev, interrupts_enabled); - if (ret) { - dev_err(&dev->pdev->dev, "hw reset failed disabling the device\n"); - interrupts_enabled = false; - } + /* enter reset flow */ + interrupts_enabled = state != MEI_DEV_POWER_DOWN; + dev->dev_state = MEI_DEV_RESETTING; + ret = mei_hw_reset(dev, interrupts_enabled); + /* fall through and remove the sw state even if hw reset has failed */ - if (dev->dev_state != MEI_DEV_INITIALIZING && - dev->dev_state != MEI_DEV_POWER_UP) { - if (dev->dev_state != MEI_DEV_DISABLED && - dev->dev_state != MEI_DEV_POWER_DOWN) - dev->dev_state = MEI_DEV_RESETTING; + /* no need to clean up software state in case of power up */ + if (state != MEI_DEV_INITIALIZING && + state != MEI_DEV_POWER_UP) { /* remove all waiting requests */ mei_cl_all_write_clear(dev); mei_cl_all_disconnect(dev); - /* wake up all readings so they can be interrupted */ + /* wake up all readers and writers so they can be interrupted */ mei_cl_all_wakeup(dev); /* remove entry if already in list */ @@ -170,33 +117,126 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled) dev->rd_msg_hdr = 0; dev->wd_pending = false; - if (!interrupts_enabled) { - dev_dbg(&dev->pdev->dev, "intr not enabled end of reset\n"); + if (ret) { + dev_err(&dev->pdev->dev, "hw_reset failed ret = %d\n", ret); dev->dev_state = MEI_DEV_DISABLED; - return; + return ret; + } + + if (state == MEI_DEV_POWER_DOWN) { + dev_dbg(&dev->pdev->dev, "powering down: end of reset\n"); + dev->dev_state = MEI_DEV_DISABLED; + return 0; } ret = mei_hw_start(dev); if (ret) { - dev_err(&dev->pdev->dev, "hw_start failed disabling the device\n"); + dev_err(&dev->pdev->dev, "hw_start failed ret = %d\n", ret); dev->dev_state = MEI_DEV_DISABLED; - return; + return ret; } dev_dbg(&dev->pdev->dev, "link is established start sending messages.\n"); - /* link is established * start sending messages. */ dev->dev_state = MEI_DEV_INIT_CLIENTS; - ret = mei_hbm_start_req(dev); if (ret) { - dev_err(&dev->pdev->dev, "hbm_start failed disabling the device\n"); + dev_err(&dev->pdev->dev, "hbm_start failed ret = %d\n", ret); dev->dev_state = MEI_DEV_DISABLED; - return; + return ret; } + + return 0; } EXPORT_SYMBOL_GPL(mei_reset); +/** + * mei_start - initializes host and fw to start work. + * + * @dev: the device structure + * + * returns 0 on success, <0 on failure. + */ +int mei_start(struct mei_device *dev) +{ + mutex_lock(&dev->device_lock); + + /* acknowledge interrupt and stop interrupts */ + mei_clear_interrupts(dev); + + mei_hw_config(dev); + + dev_dbg(&dev->pdev->dev, "reset in start the mei device.\n"); + + dev->dev_state = MEI_DEV_INITIALIZING; + mei_reset(dev); + + if (dev->dev_state == MEI_DEV_DISABLED) { + dev_err(&dev->pdev->dev, "reset failed"); + goto err; + } + + if (mei_hbm_start_wait(dev)) { + dev_err(&dev->pdev->dev, "HBM haven't started"); + goto err; + } + + if (!mei_host_is_ready(dev)) { + dev_err(&dev->pdev->dev, "host is not ready.\n"); + goto err; + } + + if (!mei_hw_is_ready(dev)) { + dev_err(&dev->pdev->dev, "ME is not ready.\n"); + goto err; + } + + if (!mei_hbm_version_is_supported(dev)) { + dev_dbg(&dev->pdev->dev, "MEI start failed.\n"); + goto err; + } + + dev_dbg(&dev->pdev->dev, "link layer has been established.\n"); + + mutex_unlock(&dev->device_lock); + return 0; +err: + dev_err(&dev->pdev->dev, "link layer initialization failed.\n"); + dev->dev_state = MEI_DEV_DISABLED; + mutex_unlock(&dev->device_lock); + return -ENODEV; +} +EXPORT_SYMBOL_GPL(mei_start); + +/** + * mei_restart - restart device after suspend + * + * @dev: the device structure + * + * returns 0 on success or -ENODEV if the restart hasn't succeeded + */ +int mei_restart(struct mei_device *dev) +{ + int err; + + mutex_lock(&dev->device_lock); + + mei_clear_interrupts(dev); + + dev->dev_state = MEI_DEV_POWER_UP; + + err = mei_reset(dev); + + mutex_unlock(&dev->device_lock); + + if (err || dev->dev_state == MEI_DEV_DISABLED) + return -ENODEV; + + return 0; +} +EXPORT_SYMBOL_GPL(mei_restart); + + static void mei_reset_work(struct work_struct *work) { struct mei_device *dev = @@ -204,9 +244,12 @@ static void mei_reset_work(struct work_struct *work) mutex_lock(&dev->device_lock); - mei_reset(dev, true); + mei_reset(dev); mutex_unlock(&dev->device_lock); + + if (dev->dev_state == MEI_DEV_DISABLED) + dev_err(&dev->pdev->dev, "reset failed"); } void mei_stop(struct mei_device *dev) @@ -222,7 +265,7 @@ void mei_stop(struct mei_device *dev) mei_wd_stop(dev); dev->dev_state = MEI_DEV_POWER_DOWN; - mei_reset(dev, 0); + mei_reset(dev); mutex_unlock(&dev->device_lock); -- cgit v1.2.3 From 6adb8efb024a7e413b93b22848fc13395b1a438a Mon Sep 17 00:00:00 2001 From: Tomas Winkler Date: Sun, 12 Jan 2014 00:36:10 +0200 Subject: mei: limit the number of consecutive resets give up reseting after 3 unsuccessful tries Signed-off-by: Tomas Winkler Signed-off-by: Alexander Usyskin Signed-off-by: Greg Kroah-Hartman --- drivers/misc/mei/init.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'drivers/misc/mei/init.c') diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c index 059133d8caca..cdd31c2a2a2b 100644 --- a/drivers/misc/mei/init.c +++ b/drivers/misc/mei/init.c @@ -89,6 +89,13 @@ int mei_reset(struct mei_device *dev) interrupts_enabled = state != MEI_DEV_POWER_DOWN; dev->dev_state = MEI_DEV_RESETTING; + dev->reset_count++; + if (dev->reset_count > MEI_MAX_CONSEC_RESET) { + dev_err(&dev->pdev->dev, "reset: reached maximal consecutive resets: disabling the device\n"); + dev->dev_state = MEI_DEV_DISABLED; + return -ENODEV; + } + ret = mei_hw_reset(dev, interrupts_enabled); /* fall through and remove the sw state even if hw reset has failed */ @@ -169,6 +176,7 @@ int mei_start(struct mei_device *dev) dev_dbg(&dev->pdev->dev, "reset in start the mei device.\n"); dev->dev_state = MEI_DEV_INITIALIZING; + dev->reset_count = 0; mei_reset(dev); if (dev->dev_state == MEI_DEV_DISABLED) { @@ -224,6 +232,7 @@ int mei_restart(struct mei_device *dev) mei_clear_interrupts(dev); dev->dev_state = MEI_DEV_POWER_UP; + dev->reset_count = 0; err = mei_reset(dev); @@ -285,6 +294,7 @@ void mei_device_init(struct mei_device *dev) init_waitqueue_head(&dev->wait_recvd_msg); init_waitqueue_head(&dev->wait_stop_wd); dev->dev_state = MEI_DEV_INITIALIZING; + dev->reset_count = 0; mei_io_list_init(&dev->read_list); mei_io_list_init(&dev->write_list); -- cgit v1.2.3