From 44d4e063d18b87c0fef1b19f7883f10e71c544b6 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Thu, 21 May 2020 22:32:37 +0530 Subject: bus: mhi: core: Cache intmod from mhi event to mhi channel Driver is using zero initialized intmod value from mhi channel when configuring TRE for bei field. This prevents interrupt moderation to take effect in case it is supported by an event ring. Fix this by copying intmod value from associated event ring to mhi channel upon registering mhi controller. Signed-off-by: Hemant Kumar Signed-off-by: Bhaumik Bhatt Reviewed-by: Jeffrey Hugo Reviewed-by: Manivannan Sadhasivam Signed-off-by: Manivannan Sadhasivam Link: https://lore.kernel.org/r/20200521170249.21795-3-manivannan.sadhasivam@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/bus/mhi/core/init.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/bus/mhi/core/init.c') diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c index eb2ab058a01d..1a93d24efffc 100644 --- a/drivers/bus/mhi/core/init.c +++ b/drivers/bus/mhi/core/init.c @@ -863,6 +863,10 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl, mutex_init(&mhi_chan->mutex); init_completion(&mhi_chan->completion); rwlock_init(&mhi_chan->lock); + + /* used in setting bei field of TRE */ + mhi_event = &mhi_cntrl->mhi_event[mhi_chan->er_index]; + mhi_chan->intmod = mhi_event->intmod; } if (mhi_cntrl->bounce_buf) { -- cgit v1.2.3 From 560e3a045961ed0c7184ef9f6a93b95bd38c1c48 Mon Sep 17 00:00:00 2001 From: Bhaumik Bhatt Date: Thu, 21 May 2020 22:32:40 +0530 Subject: bus: mhi: core: Handle firmware load using state worker Upon power up, driver queues firmware worker thread if the execution environment is PBL. Firmware worker is blocked with a timeout until state worker gets a chance to run and unblock firmware worker. An endpoint power up failure can be seen if state worker gets a chance to run after firmware worker has timed out. Remove this dependency and handle firmware load directly using state worker thread. Signed-off-by: Bhaumik Bhatt Reviewed-by: Jeffrey Hugo Reviewed-by: Manivannan Sadhasivam Signed-off-by: Manivannan Sadhasivam Link: https://lore.kernel.org/r/20200521170249.21795-6-manivannan.sadhasivam@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/bus/mhi/core/boot.c | 18 +++--------------- drivers/bus/mhi/core/init.c | 1 - drivers/bus/mhi/core/internal.h | 1 + drivers/bus/mhi/core/pm.c | 6 +----- include/linux/mhi.h | 2 -- 5 files changed, 5 insertions(+), 23 deletions(-) (limited to 'drivers/bus/mhi/core/init.c') diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c index ebad5eb48e5a..17c636b4bc6e 100644 --- a/drivers/bus/mhi/core/boot.c +++ b/drivers/bus/mhi/core/boot.c @@ -377,30 +377,18 @@ static void mhi_firmware_copy(struct mhi_controller *mhi_cntrl, } } -void mhi_fw_load_worker(struct work_struct *work) +void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) { - struct mhi_controller *mhi_cntrl; const struct firmware *firmware = NULL; struct image_info *image_info; - struct device *dev; + struct device *dev = &mhi_cntrl->mhi_dev->dev; const char *fw_name; void *buf; dma_addr_t dma_addr; size_t size; int ret; - mhi_cntrl = container_of(work, struct mhi_controller, fw_worker); - dev = &mhi_cntrl->mhi_dev->dev; - - dev_dbg(dev, "Waiting for device to enter PBL from: %s\n", - TO_MHI_EXEC_STR(mhi_cntrl->ee)); - - ret = wait_event_timeout(mhi_cntrl->state_event, - MHI_IN_PBL(mhi_cntrl->ee) || - MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), - msecs_to_jiffies(mhi_cntrl->timeout_ms)); - - if (!ret || MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) { + if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) { dev_err(dev, "Device MHI is not in valid state\n"); return; } diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c index 1a93d24efffc..6882206ad80e 100644 --- a/drivers/bus/mhi/core/init.c +++ b/drivers/bus/mhi/core/init.c @@ -835,7 +835,6 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl, spin_lock_init(&mhi_cntrl->wlock); INIT_WORK(&mhi_cntrl->st_worker, mhi_pm_st_worker); INIT_WORK(&mhi_cntrl->syserr_worker, mhi_pm_sys_err_worker); - INIT_WORK(&mhi_cntrl->fw_worker, mhi_fw_load_worker); init_waitqueue_head(&mhi_cntrl->state_event); mhi_cmd = mhi_cntrl->mhi_cmd; diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h index 40c47f918ca3..0965ca3c9632 100644 --- a/drivers/bus/mhi/core/internal.h +++ b/drivers/bus/mhi/core/internal.h @@ -627,6 +627,7 @@ int mhi_init_irq_setup(struct mhi_controller *mhi_cntrl); void mhi_deinit_free_irq(struct mhi_controller *mhi_cntrl); void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl, struct image_info *img_info); +void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl); int mhi_prepare_channel(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan); int mhi_init_chan_ctxt(struct mhi_controller *mhi_cntrl, diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c index e7c831867a23..3cc238ad693d 100644 --- a/drivers/bus/mhi/core/pm.c +++ b/drivers/bus/mhi/core/pm.c @@ -528,7 +528,6 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl, dev_dbg(dev, "Waiting for all pending threads to complete\n"); wake_up_all(&mhi_cntrl->state_event); flush_work(&mhi_cntrl->st_worker); - flush_work(&mhi_cntrl->fw_worker); dev_dbg(dev, "Reset all active channels and remove MHI devices\n"); device_for_each_child(mhi_cntrl->cntrl_dev, NULL, mhi_destroy_device); @@ -643,7 +642,7 @@ void mhi_pm_st_worker(struct work_struct *work) mhi_cntrl->ee = mhi_get_exec_env(mhi_cntrl); write_unlock_irq(&mhi_cntrl->pm_lock); if (MHI_IN_PBL(mhi_cntrl->ee)) - wake_up_all(&mhi_cntrl->state_event); + mhi_fw_load_handler(mhi_cntrl); break; case DEV_ST_TRANSITION_SBL: write_lock_irq(&mhi_cntrl->pm_lock); @@ -976,9 +975,6 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl) next_state = MHI_IN_PBL(current_ee) ? DEV_ST_TRANSITION_PBL : DEV_ST_TRANSITION_READY; - if (next_state == DEV_ST_TRANSITION_PBL) - schedule_work(&mhi_cntrl->fw_worker); - mhi_queue_state_transition(mhi_cntrl, next_state); mutex_unlock(&mhi_cntrl->pm_mutex); diff --git a/include/linux/mhi.h b/include/linux/mhi.h index b0739ad1bae4..8289202b8cbe 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -331,7 +331,6 @@ struct mhi_controller_config { * @wlock: Lock for protecting device wakeup * @mhi_link_info: Device bandwidth info * @st_worker: State transition worker - * @fw_worker: Firmware download worker * @syserr_worker: System error worker * @state_event: State change event * @status_cb: CB function to notify power states of the device (required) @@ -412,7 +411,6 @@ struct mhi_controller { spinlock_t wlock; struct mhi_link_info mhi_link_info; struct work_struct st_worker; - struct work_struct fw_worker; struct work_struct syserr_worker; wait_queue_head_t state_event; -- cgit v1.2.3 From bc7ccce5a5192cf277da0aef05e45cd92c81c79a Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Thu, 21 May 2020 22:32:44 +0530 Subject: bus: mhi: core: Remove the system error worker thread Remove the system error worker thread and instead have the execution environment worker handle that transition to serialize processing and avoid any possible race conditions during shutdown. Signed-off-by: Hemant Kumar Reviewed-by: Jeffrey Hugo Reviewed-by: Manivannan Sadhasivam Signed-off-by: Manivannan Sadhasivam Link: https://lore.kernel.org/r/20200521170249.21795-10-manivannan.sadhasivam@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/bus/mhi/core/init.c | 2 +- drivers/bus/mhi/core/internal.h | 3 ++- drivers/bus/mhi/core/main.c | 6 +++--- drivers/bus/mhi/core/pm.c | 32 ++++++++++++++------------------ include/linux/mhi.h | 2 -- 5 files changed, 20 insertions(+), 25 deletions(-) (limited to 'drivers/bus/mhi/core/init.c') diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c index 6882206ad80e..3a853c5d2103 100644 --- a/drivers/bus/mhi/core/init.c +++ b/drivers/bus/mhi/core/init.c @@ -34,6 +34,7 @@ const char * const dev_state_tran_str[DEV_ST_TRANSITION_MAX] = { [DEV_ST_TRANSITION_READY] = "READY", [DEV_ST_TRANSITION_SBL] = "SBL", [DEV_ST_TRANSITION_MISSION_MODE] = "MISSION_MODE", + [DEV_ST_TRANSITION_SYS_ERR] = "SYS_ERR", }; const char * const mhi_state_str[MHI_STATE_MAX] = { @@ -834,7 +835,6 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl, spin_lock_init(&mhi_cntrl->transition_lock); spin_lock_init(&mhi_cntrl->wlock); INIT_WORK(&mhi_cntrl->st_worker, mhi_pm_st_worker); - INIT_WORK(&mhi_cntrl->syserr_worker, mhi_pm_sys_err_worker); init_waitqueue_head(&mhi_cntrl->state_event); mhi_cmd = mhi_cntrl->mhi_cmd; diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h index 80b32c20149c..f01283b8a451 100644 --- a/drivers/bus/mhi/core/internal.h +++ b/drivers/bus/mhi/core/internal.h @@ -386,6 +386,7 @@ enum dev_st_transition { DEV_ST_TRANSITION_READY, DEV_ST_TRANSITION_SBL, DEV_ST_TRANSITION_MISSION_MODE, + DEV_ST_TRANSITION_SYS_ERR, DEV_ST_TRANSITION_MAX, }; @@ -587,7 +588,7 @@ enum mhi_ee_type mhi_get_exec_env(struct mhi_controller *mhi_cntrl); int mhi_queue_state_transition(struct mhi_controller *mhi_cntrl, enum dev_st_transition state); void mhi_pm_st_worker(struct work_struct *work); -void mhi_pm_sys_err_worker(struct work_struct *work); +void mhi_pm_sys_err_handler(struct mhi_controller *mhi_cntrl); void mhi_fw_load_worker(struct work_struct *work); int mhi_ready_state_transition(struct mhi_controller *mhi_cntrl); void mhi_ctrl_ev_task(unsigned long data); diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c index 6a8066640e78..9ec9b3601592 100644 --- a/drivers/bus/mhi/core/main.c +++ b/drivers/bus/mhi/core/main.c @@ -406,7 +406,7 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *dev) if (MHI_IN_PBL(ee)) mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_FATAL_ERROR); else - schedule_work(&mhi_cntrl->syserr_worker); + mhi_pm_sys_err_handler(mhi_cntrl); } exit_intvec: @@ -734,7 +734,7 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller *mhi_cntrl, MHI_PM_SYS_ERR_DETECT); write_unlock_irq(&mhi_cntrl->pm_lock); if (new_state == MHI_PM_SYS_ERR_DETECT) - schedule_work(&mhi_cntrl->syserr_worker); + mhi_pm_sys_err_handler(mhi_cntrl); break; } default: @@ -920,7 +920,7 @@ void mhi_ctrl_ev_task(unsigned long data) } write_unlock_irq(&mhi_cntrl->pm_lock); if (pm_state == MHI_PM_SYS_ERR_DETECT) - schedule_work(&mhi_cntrl->syserr_worker); + mhi_pm_sys_err_handler(mhi_cntrl); } } diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c index 3cc238ad693d..d9964d4c4eed 100644 --- a/drivers/bus/mhi/core/pm.c +++ b/drivers/bus/mhi/core/pm.c @@ -449,19 +449,8 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl, to_mhi_pm_state_str(transition_state)); /* We must notify MHI control driver so it can clean up first */ - if (transition_state == MHI_PM_SYS_ERR_PROCESS) { - /* - * If controller supports RDDM, we do not process - * SYS error state, instead we will jump directly - * to RDDM state - */ - if (mhi_cntrl->rddm_image) { - dev_dbg(dev, - "Controller supports RDDM, so skip SYS_ERR\n"); - return; - } + if (transition_state == MHI_PM_SYS_ERR_PROCESS) mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_SYS_ERROR); - } mutex_lock(&mhi_cntrl->pm_mutex); write_lock_irq(&mhi_cntrl->pm_lock); @@ -527,7 +516,6 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl, mutex_unlock(&mhi_cntrl->pm_mutex); dev_dbg(dev, "Waiting for all pending threads to complete\n"); wake_up_all(&mhi_cntrl->state_event); - flush_work(&mhi_cntrl->st_worker); dev_dbg(dev, "Reset all active channels and remove MHI devices\n"); device_for_each_child(mhi_cntrl->cntrl_dev, NULL, mhi_destroy_device); @@ -607,13 +595,17 @@ int mhi_queue_state_transition(struct mhi_controller *mhi_cntrl, } /* SYS_ERR worker */ -void mhi_pm_sys_err_worker(struct work_struct *work) +void mhi_pm_sys_err_handler(struct mhi_controller *mhi_cntrl) { - struct mhi_controller *mhi_cntrl = container_of(work, - struct mhi_controller, - syserr_worker); + struct device *dev = &mhi_cntrl->mhi_dev->dev; + + /* skip if controller supports RDDM */ + if (mhi_cntrl->rddm_image) { + dev_dbg(dev, "Controller supports RDDM, skip SYS_ERROR\n"); + return; + } - mhi_pm_disable_transition(mhi_cntrl, MHI_PM_SYS_ERR_PROCESS); + mhi_queue_state_transition(mhi_cntrl, DEV_ST_TRANSITION_SYS_ERR); } /* Device State Transition worker */ @@ -661,6 +653,10 @@ void mhi_pm_st_worker(struct work_struct *work) case DEV_ST_TRANSITION_READY: mhi_ready_state_transition(mhi_cntrl); break; + case DEV_ST_TRANSITION_SYS_ERR: + mhi_pm_disable_transition + (mhi_cntrl, MHI_PM_SYS_ERR_PROCESS); + break; default: break; } diff --git a/include/linux/mhi.h b/include/linux/mhi.h index 8289202b8cbe..c4a940d98912 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -331,7 +331,6 @@ struct mhi_controller_config { * @wlock: Lock for protecting device wakeup * @mhi_link_info: Device bandwidth info * @st_worker: State transition worker - * @syserr_worker: System error worker * @state_event: State change event * @status_cb: CB function to notify power states of the device (required) * @wake_get: CB function to assert device wake (optional) @@ -411,7 +410,6 @@ struct mhi_controller { spinlock_t wlock; struct mhi_link_info mhi_link_info; struct work_struct st_worker; - struct work_struct syserr_worker; wait_queue_head_t state_event; void (*status_cb)(struct mhi_controller *mhi_cntrl, -- cgit v1.2.3 From 3c1bd0047124f506294520341cfe03e19ea773e3 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Thu, 21 May 2020 22:32:45 +0530 Subject: bus: mhi: core: Handle disable transitions in state worker Mission mode transition is handled by state worker thread but power off is not. There is a possibility while mission mode transition is in progress which calls MHI client driver probe, power off is issued by MHI controller. This results into client driver probe and remove running in parallel and causes use after free situation. By queuing disable transition work when mission mode is in progress prevents the race condition. Signed-off-by: Hemant Kumar Reviewed-by: Jeffrey Hugo Reviewed-by: Manivannan Sadhasivam Signed-off-by: Manivannan Sadhasivam Link: https://lore.kernel.org/r/20200521170249.21795-11-manivannan.sadhasivam@linaro.org Signed-off-by: Greg Kroah-Hartman --- drivers/bus/mhi/core/init.c | 1 + drivers/bus/mhi/core/internal.h | 1 + drivers/bus/mhi/core/pm.c | 11 ++++++++++- 3 files changed, 12 insertions(+), 1 deletion(-) (limited to 'drivers/bus/mhi/core/init.c') diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c index 3a853c5d2103..12207cc438aa 100644 --- a/drivers/bus/mhi/core/init.c +++ b/drivers/bus/mhi/core/init.c @@ -35,6 +35,7 @@ const char * const dev_state_tran_str[DEV_ST_TRANSITION_MAX] = { [DEV_ST_TRANSITION_SBL] = "SBL", [DEV_ST_TRANSITION_MISSION_MODE] = "MISSION_MODE", [DEV_ST_TRANSITION_SYS_ERR] = "SYS_ERR", + [DEV_ST_TRANSITION_DISABLE] = "DISABLE", }; const char * const mhi_state_str[MHI_STATE_MAX] = { diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h index f01283b8a451..b1f640b75a94 100644 --- a/drivers/bus/mhi/core/internal.h +++ b/drivers/bus/mhi/core/internal.h @@ -387,6 +387,7 @@ enum dev_st_transition { DEV_ST_TRANSITION_SBL, DEV_ST_TRANSITION_MISSION_MODE, DEV_ST_TRANSITION_SYS_ERR, + DEV_ST_TRANSITION_DISABLE, DEV_ST_TRANSITION_MAX, }; diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c index d9964d4c4eed..345f197130b8 100644 --- a/drivers/bus/mhi/core/pm.c +++ b/drivers/bus/mhi/core/pm.c @@ -657,6 +657,10 @@ void mhi_pm_st_worker(struct work_struct *work) mhi_pm_disable_transition (mhi_cntrl, MHI_PM_SYS_ERR_PROCESS); break; + case DEV_ST_TRANSITION_DISABLE: + mhi_pm_disable_transition + (mhi_cntrl, MHI_PM_SHUTDOWN_PROCESS); + break; default: break; } @@ -1011,7 +1015,12 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful) to_mhi_pm_state_str(MHI_PM_LD_ERR_FATAL_DETECT), to_mhi_pm_state_str(mhi_cntrl->pm_state)); } - mhi_pm_disable_transition(mhi_cntrl, MHI_PM_SHUTDOWN_PROCESS); + + mhi_queue_state_transition(mhi_cntrl, DEV_ST_TRANSITION_DISABLE); + + /* Wait for shutdown to complete */ + flush_work(&mhi_cntrl->st_worker); + mhi_deinit_free_irq(mhi_cntrl); if (!mhi_cntrl->pre_init) { -- cgit v1.2.3