Bug fixes and improvements for MHI powerup and shutdown handling.
Firmware load function names are updated to accurately reflect their purpose.
Closed certain design gaps where the host (MHI bus) would allow clients to
operate after a power down or error detection.
Move to an error state sooner based on different scenarios.
These patches were tested on arm64 and X86_64 architectures.
v4:
-Fixed up bus: mhi: core: Move to SYS_ERROR regardless of RDDM capability patch
by removing check for EE as well since a previous guard to check if MHI pm_state
allows event ring access is already present. Event ring access should not be
allowed at the time and hence the check is safe to remove.
v3:
-Fixed bus: mhi: core: Remove MHI event ring IRQ handlers when powering down
-Mistakenly placed the free_irq() calls in mhi_pm_sys_error_transition()
-Moved it to mhi_pm_disable_transition()
v2:
-Addressed patches based on review comments and made improvements
-Added bus: mhi: core: Check for IRQ availability during registration
-Dropped bus: mhi: core: Use the IRQF_ONESHOT flag for the BHI interrupt line
-Split bus: mhi: core: Move to an error state on any firmware load failure
-Modified the following patches:
-bus: mhi: core: Disable IRQs when powering down
-bus: mhi: core: Improve shutdown handling after link down detection
-bus: mhi: core: Mark device inactive soon after host issues a shutdown
-bus: mhi: core: Move to SYS_ERROR regardless of RDDM capability
-Addressed the above as follow-up patches with improvements:
-bus: mhi: core: Prevent sending multiple RDDM entry callbacks
-bus: mhi: core: Separate system error and power down handling
-bus: mhi: core: Remove MHI event ring IRQ handlers when powering down
Bhaumik Bhatt (12):
bus: mhi: core: Use appropriate names for firmware load functions
bus: mhi: core: Move to using high priority workqueue
bus: mhi: core: Skip device wake in error or shutdown states
bus: mhi: core: Move to SYS_ERROR regardless of RDDM capability
bus: mhi: core: Prevent sending multiple RDDM entry callbacks
bus: mhi: core: Move to an error state on any firmware load failure
bus: mhi: core: Use appropriate label in firmware load handler API
bus: mhi: core: Move to an error state on mission mode failure
bus: mhi: core: Check for IRQ availability during registration
bus: mhi: core: Separate system error and power down handling
bus: mhi: core: Mark and maintain device states early on after power
down
bus: mhi: core: Remove MHI event ring IRQ handlers when powering down
drivers/bus/mhi/core/boot.c | 60 ++++++-----
drivers/bus/mhi/core/init.c | 11 ++-
drivers/bus/mhi/core/main.c | 9 +-
drivers/bus/mhi/core/pm.c | 236 ++++++++++++++++++++++++++++++++------------
include/linux/mhi.h | 2 +
5 files changed, 222 insertions(+), 96 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
If the host receives a mission mode event and by the time it can get
to processing it, the register accesses fail implying a connectivity
error, MHI should move to an error state. This helps avoid longer wait
times from a synchronous power up perspective and accurately reflects
the MHI execution environment and power management states.
Signed-off-by: Bhaumik Bhatt <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
---
drivers/bus/mhi/core/pm.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index 0299196..06adea2 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -383,10 +383,14 @@ static int mhi_pm_mission_mode_transition(struct mhi_controller *mhi_cntrl)
write_lock_irq(&mhi_cntrl->pm_lock);
if (MHI_REG_ACCESS_VALID(mhi_cntrl->pm_state))
mhi_cntrl->ee = mhi_get_exec_env(mhi_cntrl);
- write_unlock_irq(&mhi_cntrl->pm_lock);
- if (!MHI_IN_MISSION_MODE(mhi_cntrl->ee))
+ if (!MHI_IN_MISSION_MODE(mhi_cntrl->ee)) {
+ mhi_cntrl->pm_state = MHI_PM_LD_ERR_FATAL_DETECT;
+ write_unlock_irq(&mhi_cntrl->pm_lock);
+ wake_up_all(&mhi_cntrl->state_event);
return -EIO;
+ }
+ write_unlock_irq(&mhi_cntrl->pm_lock);
wake_up_all(&mhi_cntrl->state_event);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
mhi_fw_load_sbl() function is currently used to transfer SBL or EDL
images over BHI (Boot Host Interface). Same goes with mhi_fw_load_amss()
which uses BHIe. However, the contents of these functions do not
indicate support for a specific set of images. Since these can be used
for any image download over BHI or BHIe, rename them based on the
protocol used.
Signed-off-by: Bhaumik Bhatt <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
---
drivers/bus/mhi/core/boot.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
index 6f0cfb9..2d7752c 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -176,7 +176,7 @@ int mhi_download_rddm_image(struct mhi_controller *mhi_cntrl, bool in_panic)
}
EXPORT_SYMBOL_GPL(mhi_download_rddm_image);
-static int mhi_fw_load_amss(struct mhi_controller *mhi_cntrl,
+static int mhi_fw_load_bhie(struct mhi_controller *mhi_cntrl,
const struct mhi_buf *mhi_buf)
{
void __iomem *base = mhi_cntrl->bhie;
@@ -192,7 +192,7 @@ static int mhi_fw_load_amss(struct mhi_controller *mhi_cntrl,
}
sequence_id = MHI_RANDOM_U32_NONZERO(BHIE_TXVECSTATUS_SEQNUM_BMSK);
- dev_dbg(dev, "Starting AMSS download via BHIe. Sequence ID:%u\n",
+ dev_dbg(dev, "Starting image download via BHIe. Sequence ID: %u\n",
sequence_id);
mhi_write_reg(mhi_cntrl, base, BHIE_TXVECADDR_HIGH_OFFS,
upper_32_bits(mhi_buf->dma_addr));
@@ -223,7 +223,7 @@ static int mhi_fw_load_amss(struct mhi_controller *mhi_cntrl,
return (!ret) ? -ETIMEDOUT : 0;
}
-static int mhi_fw_load_sbl(struct mhi_controller *mhi_cntrl,
+static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
dma_addr_t dma_addr,
size_t size)
{
@@ -250,7 +250,7 @@ static int mhi_fw_load_sbl(struct mhi_controller *mhi_cntrl,
}
session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
- dev_dbg(dev, "Starting SBL download via BHI. Session ID:%u\n",
+ dev_dbg(dev, "Starting image download via BHI. Session ID: %u\n",
session_id);
mhi_write_reg(mhi_cntrl, base, BHI_STATUS, 0);
mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_HIGH,
@@ -449,9 +449,9 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
return;
}
- /* Download SBL image */
+ /* Download image using BHI */
memcpy(buf, firmware->data, size);
- ret = mhi_fw_load_sbl(mhi_cntrl, dma_addr, size);
+ ret = mhi_fw_load_bhi(mhi_cntrl, dma_addr, size);
mhi_free_coherent(mhi_cntrl, size, buf, dma_addr);
if (!mhi_cntrl->fbc_download || ret || mhi_cntrl->ee == MHI_EE_EDL)
@@ -459,7 +459,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
/* Error or in EDL mode, we're done */
if (ret) {
- dev_err(dev, "MHI did not load SBL, ret:%d\n", ret);
+ dev_err(dev, "MHI did not load image over BHI, ret: %d\n", ret);
return;
}
@@ -509,11 +509,12 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
/* Start full firmware image download */
image_info = mhi_cntrl->fbc_image;
- ret = mhi_fw_load_amss(mhi_cntrl,
+ ret = mhi_fw_load_bhie(mhi_cntrl,
/* Vector table is the last entry */
&image_info->mhi_buf[image_info->entries - 1]);
if (ret)
- dev_err(dev, "MHI did not load AMSS, ret:%d\n", ret);
+ dev_err(dev, "MHI did not load image over BHIe, ret: %d\n",
+ ret);
release_firmware(firmware);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
MHI work is currently scheduled on the global/system workqueue and can
encounter delays on a stressed system. To avoid those unforeseen
delays which can hamper bootup or shutdown times, use a dedicated high
priority workqueue instead of the global/system workqueue.
Signed-off-by: Bhaumik Bhatt <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
---
drivers/bus/mhi/core/init.c | 9 +++++++++
drivers/bus/mhi/core/pm.c | 2 +-
include/linux/mhi.h | 2 ++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 8cefa35..877e40c 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -880,6 +880,13 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,
INIT_WORK(&mhi_cntrl->st_worker, mhi_pm_st_worker);
init_waitqueue_head(&mhi_cntrl->state_event);
+ mhi_cntrl->hiprio_wq = alloc_ordered_workqueue
+ ("mhi_hiprio_wq", WQ_MEM_RECLAIM | WQ_HIGHPRI);
+ if (!mhi_cntrl->hiprio_wq) {
+ dev_err(mhi_cntrl->cntrl_dev, "Failed to allocate workqueue\n");
+ goto error_alloc_cmd;
+ }
+
mhi_cmd = mhi_cntrl->mhi_cmd;
for (i = 0; i < NR_OF_CMD_RINGS; i++, mhi_cmd++)
spin_lock_init(&mhi_cmd->lock);
@@ -969,6 +976,7 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,
error_alloc_cmd:
vfree(mhi_cntrl->mhi_chan);
kfree(mhi_cntrl->mhi_event);
+ destroy_workqueue(mhi_cntrl->hiprio_wq);
return ret;
}
@@ -982,6 +990,7 @@ void mhi_unregister_controller(struct mhi_controller *mhi_cntrl)
mhi_destroy_debugfs(mhi_cntrl);
+ destroy_workqueue(mhi_cntrl->hiprio_wq);
kfree(mhi_cntrl->mhi_cmd);
kfree(mhi_cntrl->mhi_event);
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index 3de7b16..805b6fa74 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -597,7 +597,7 @@ int mhi_queue_state_transition(struct mhi_controller *mhi_cntrl,
list_add_tail(&item->node, &mhi_cntrl->transition_list);
spin_unlock_irqrestore(&mhi_cntrl->transition_lock, flags);
- schedule_work(&mhi_cntrl->st_worker);
+ queue_work(mhi_cntrl->hiprio_wq, &mhi_cntrl->st_worker);
return 0;
}
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 52b3c60..1ed5f2a 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -337,6 +337,7 @@ struct mhi_controller_config {
* @wlock: Lock for protecting device wakeup
* @mhi_link_info: Device bandwidth info
* @st_worker: State transition worker
+ * @hiprio_wq: High priority workqueue for MHI work such as state transitions
* @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)
@@ -419,6 +420,7 @@ struct mhi_controller {
spinlock_t wlock;
struct mhi_link_info mhi_link_info;
struct work_struct st_worker;
+ struct workqueue_struct *hiprio_wq;
wait_queue_head_t state_event;
void (*status_cb)(struct mhi_controller *mhi_cntrl,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
On Mon, Nov 09, 2020 at 12:47:19PM -0800, Bhaumik Bhatt wrote:
> Bug fixes and improvements for MHI powerup and shutdown handling.
> Firmware load function names are updated to accurately reflect their purpose.
> Closed certain design gaps where the host (MHI bus) would allow clients to
> operate after a power down or error detection.
> Move to an error state sooner based on different scenarios.
>
> These patches were tested on arm64 and X86_64 architectures.
>
> v4:
> -Fixed up bus: mhi: core: Move to SYS_ERROR regardless of RDDM capability patch
> by removing check for EE as well since a previous guard to check if MHI pm_state
> allows event ring access is already present. Event ring access should not be
> allowed at the time and hence the check is safe to remove.
>
> v3:
> -Fixed bus: mhi: core: Remove MHI event ring IRQ handlers when powering down
> -Mistakenly placed the free_irq() calls in mhi_pm_sys_error_transition()
> -Moved it to mhi_pm_disable_transition()
>
> v2:
> -Addressed patches based on review comments and made improvements
> -Added bus: mhi: core: Check for IRQ availability during registration
> -Dropped bus: mhi: core: Use the IRQF_ONESHOT flag for the BHI interrupt line
> -Split bus: mhi: core: Move to an error state on any firmware load failure
> -Modified the following patches:
> -bus: mhi: core: Disable IRQs when powering down
> -bus: mhi: core: Improve shutdown handling after link down detection
> -bus: mhi: core: Mark device inactive soon after host issues a shutdown
> -bus: mhi: core: Move to SYS_ERROR regardless of RDDM capability
> -Addressed the above as follow-up patches with improvements:
> -bus: mhi: core: Prevent sending multiple RDDM entry callbacks
> -bus: mhi: core: Separate system error and power down handling
> -bus: mhi: core: Remove MHI event ring IRQ handlers when powering down
>
> Bhaumik Bhatt (12):
> bus: mhi: core: Use appropriate names for firmware load functions
> bus: mhi: core: Move to using high priority workqueue
> bus: mhi: core: Skip device wake in error or shutdown states
> bus: mhi: core: Move to SYS_ERROR regardless of RDDM capability
> bus: mhi: core: Prevent sending multiple RDDM entry callbacks
> bus: mhi: core: Move to an error state on any firmware load failure
> bus: mhi: core: Use appropriate label in firmware load handler API
> bus: mhi: core: Move to an error state on mission mode failure
> bus: mhi: core: Check for IRQ availability during registration
> bus: mhi: core: Separate system error and power down handling
> bus: mhi: core: Mark and maintain device states early on after power
> down
> bus: mhi: core: Remove MHI event ring IRQ handlers when powering down
Series applied to mhi-next!
Thanks,
Mani
>
> drivers/bus/mhi/core/boot.c | 60 ++++++-----
> drivers/bus/mhi/core/init.c | 11 ++-
> drivers/bus/mhi/core/main.c | 9 +-
> drivers/bus/mhi/core/pm.c | 236 ++++++++++++++++++++++++++++++++------------
> include/linux/mhi.h | 2 +
> 5 files changed, 222 insertions(+), 96 deletions(-)
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>