2020-05-20 00:32:52

by Bhaumik Bhatt

[permalink] [raw]
Subject: [PATCH v1 0/6] Bug fixes and bootup and shutdown improvements

Bug fixes for handling of execution environment in bootup/shutdown scenarios
and to improve host view of device state.
Introduced feature for manual mission mode image loading and allow for mission
mode image load from SBL handling.

Build dependencies:
Patch "bus: mhi: core: Mark device inactive soon after host issues a shutdown"
depends on: "bus: mhi: core: Introduce helper function to check device state".

Bhaumik Bhatt (6):
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: Check for RDDM support before forcing a device crash
bus: mhi: core: Use common name for BHI firmware load function
bus: mhi: core: Introduce support for manual AMSS loading
bus: mhi: core: Process execution environment changes serially

drivers/bus/mhi/core/boot.c | 100 ++++++++++++++++++++--------------------
drivers/bus/mhi/core/init.c | 1 +
drivers/bus/mhi/core/internal.h | 1 +
drivers/bus/mhi/core/main.c | 45 ++++++++++++------
drivers/bus/mhi/core/pm.c | 80 +++++++++++++++++++++++---------
include/linux/mhi.h | 10 ++++
6 files changed, 151 insertions(+), 86 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2020-05-20 00:33:58

by Bhaumik Bhatt

[permalink] [raw]
Subject: [PATCH v1 3/6] bus: mhi: core: Check for RDDM support before forcing a device crash

Check if the device supports RDDM from the mhi_force_rddm_mode() API
before allowing a client to force a device crash. This will ensure that
a client who is unaware does not misuse the API and expect the device to
go to ramdump collection mode after a crash is forced.

Signed-off-by: Bhaumik Bhatt <[email protected]>
---
drivers/bus/mhi/core/pm.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index 1daed86..52c290c6 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -1114,6 +1114,10 @@ int mhi_force_rddm_mode(struct mhi_controller *mhi_cntrl)
struct device *dev = &mhi_cntrl->mhi_dev->dev;
int ret;

+ /* Check if device supports RDDM */
+ if (!mhi_cntrl->rddm_image)
+ return -EINVAL;
+
/* Check if device is already in RDDM */
if (mhi_cntrl->ee == MHI_EE_RDDM)
return 0;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2020-05-20 00:35:48

by Bhaumik Bhatt

[permalink] [raw]
Subject: [PATCH v1 4/6] bus: mhi: core: Use common name for BHI firmware load function

mhi_fw_load_sbl() function is currently used to transfer SBL or EDL
images over BHI (Boot Host Interface). Moreover, its contents do not
indicate anything regarding support for a specific set of images. Since
it can be used for any image download over BHI, it can be appropriately
renamed mhi_fw_load_bhi() instead.

Signed-off-by: Bhaumik Bhatt <[email protected]>
---
drivers/bus/mhi/core/boot.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
index 24422f5..34ce102 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -218,7 +218,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)
{
@@ -245,7 +245,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 SBL/EDL 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,
@@ -446,9 +446,9 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
return;
}

- /* Download SBL image */
+ /* Download SBL or EDL 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)
@@ -456,7 +456,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 SBL/EDL image, ret:%d\n", ret);
return;
}

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2020-05-20 00:36:17

by Bhaumik Bhatt

[permalink] [raw]
Subject: [PATCH v1 1/6] bus: mhi: core: Improve shutdown handling after link down detection

If MHI were to attempt a device shutdown following an assumption
that the device is inaccessible, the host currently moves to a state
where device register accesses are allowed when they should not be.
This would end up allowing accesses to device register space when the
link is inaccessible which can result in NOC errors to be observed on
the host. Improve shutdown handling so as to prevent these outcomes
and do not move the MHI PM state to a register accessible state after
device is assumed to be inaccessible.

Signed-off-by: Bhaumik Bhatt <[email protected]>
---
drivers/bus/mhi/core/init.c | 1 +
drivers/bus/mhi/core/internal.h | 1 +
drivers/bus/mhi/core/pm.c | 20 ++++++++++++++------
3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 9b6a5173..0ae4c34 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -37,6 +37,7 @@
[DEV_ST_TRANSITION_MISSION_MODE] = "MISSION_MODE",
[DEV_ST_TRANSITION_SYS_ERR] = "SYS_ERR",
[DEV_ST_TRANSITION_DISABLE] = "DISABLE",
+ [DEV_ST_TRANSITION_FATAL] = "FATAL SHUTDOWN",
};

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 798aa483..a7203c2 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -388,6 +388,7 @@ enum dev_st_transition {
DEV_ST_TRANSITION_MISSION_MODE,
DEV_ST_TRANSITION_SYS_ERR,
DEV_ST_TRANSITION_DISABLE,
+ DEV_ST_TRANSITION_FATAL,
DEV_ST_TRANSITION_MAX,
};

diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index 783e3d5..b2b3de7 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -37,9 +37,10 @@
* M0 -> FW_DL_ERR
* M0 -> M3_ENTER -> M3 -> M3_EXIT --> M0
* L1: SYS_ERR_DETECT -> SYS_ERR_PROCESS --> POR
- * L2: SHUTDOWN_PROCESS -> DISABLE
+ * L2: SHUTDOWN_PROCESS -> LD_ERR_FATAL_DETECT
+ * SHUTDOWN_PROCESS -> DISABLE
* L3: LD_ERR_FATAL_DETECT <--> LD_ERR_FATAL_DETECT
- * LD_ERR_FATAL_DETECT -> SHUTDOWN_PROCESS
+ * LD_ERR_FATAL_DETECT -> DISABLE
*/
static struct mhi_pm_transitions const dev_state_transitions[] = {
/* L0 States */
@@ -72,7 +73,7 @@
{
MHI_PM_M3,
MHI_PM_M3_EXIT | MHI_PM_SYS_ERR_DETECT |
- MHI_PM_SHUTDOWN_PROCESS | MHI_PM_LD_ERR_FATAL_DETECT
+ MHI_PM_LD_ERR_FATAL_DETECT
},
{
MHI_PM_M3_EXIT,
@@ -103,7 +104,7 @@
/* L3 States */
{
MHI_PM_LD_ERR_FATAL_DETECT,
- MHI_PM_LD_ERR_FATAL_DETECT | MHI_PM_SHUTDOWN_PROCESS
+ MHI_PM_LD_ERR_FATAL_DETECT | MHI_PM_DISABLE
},
};

@@ -667,7 +668,11 @@ void mhi_pm_st_worker(struct work_struct *work)
break;
case DEV_ST_TRANSITION_DISABLE:
mhi_pm_disable_transition
- (mhi_cntrl, MHI_PM_SHUTDOWN_PROCESS);
+ (mhi_cntrl, MHI_PM_SHUTDOWN_PROCESS);
+ break;
+ case DEV_ST_TRANSITION_FATAL:
+ mhi_pm_disable_transition
+ (mhi_cntrl, MHI_PM_LD_ERR_FATAL_DETECT);
break;
default:
break;
@@ -1040,6 +1045,7 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful)
{
enum mhi_pm_state cur_state;
+ enum dev_st_transition next_state = DEV_ST_TRANSITION_DISABLE;
struct device *dev = &mhi_cntrl->mhi_dev->dev;

/* If it's not a graceful shutdown, force MHI to linkdown state */
@@ -1054,9 +1060,11 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful)
dev_dbg(dev, "Failed to move to state: %s from: %s\n",
to_mhi_pm_state_str(MHI_PM_LD_ERR_FATAL_DETECT),
to_mhi_pm_state_str(mhi_cntrl->pm_state));
+ else
+ next_state = DEV_ST_TRANSITION_FATAL;
}

- mhi_queue_state_transition(mhi_cntrl, DEV_ST_TRANSITION_DISABLE);
+ mhi_queue_state_transition(mhi_cntrl, next_state);

/* Wait for shutdown to complete */
flush_work(&mhi_cntrl->st_worker);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project