linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/6] Bug fixes and bootup and shutdown improvements
@ 2020-05-20  0:30 Bhaumik Bhatt
  2020-05-20  0:30 ` [PATCH v1 1/6] bus: mhi: core: Improve shutdown handling after link down detection Bhaumik Bhatt
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Bhaumik Bhatt @ 2020-05-20  0:30 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, Bhaumik Bhatt

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 1/6] bus: mhi: core: Improve shutdown handling after link down detection
  2020-05-20  0:30 [PATCH v1 0/6] Bug fixes and bootup and shutdown improvements Bhaumik Bhatt
@ 2020-05-20  0:30 ` Bhaumik Bhatt
  2020-05-20  0:30 ` [PATCH v1 2/6] bus: mhi: core: Mark device inactive soon after host issues a shutdown Bhaumik Bhatt
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Bhaumik Bhatt @ 2020-05-20  0:30 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, Bhaumik Bhatt

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 <bbhatt@codeaurora.org>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 2/6] bus: mhi: core: Mark device inactive soon after host issues a shutdown
  2020-05-20  0:30 [PATCH v1 0/6] Bug fixes and bootup and shutdown improvements Bhaumik Bhatt
  2020-05-20  0:30 ` [PATCH v1 1/6] bus: mhi: core: Improve shutdown handling after link down detection Bhaumik Bhatt
@ 2020-05-20  0:30 ` Bhaumik Bhatt
  2020-05-20  5:16   ` kbuild test robot
                     ` (2 more replies)
  2020-05-20  0:30 ` [PATCH v1 3/6] bus: mhi: core: Check for RDDM support before forcing a device crash Bhaumik Bhatt
                   ` (3 subsequent siblings)
  5 siblings, 3 replies; 10+ messages in thread
From: Bhaumik Bhatt @ 2020-05-20  0:30 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, Bhaumik Bhatt

Clients on the host may see the device in an active state for a short
period of time after the host detects a device error or power down.
Prevent any further host activity which could lead to race conditions
and timeouts seen by clients attempting to push data as they must be
notified of the host's intent sooner than later. This also allows the
host and device states to be in sync with one another and prevents
unnecessary host operations.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
 drivers/bus/mhi/core/main.c | 33 ++++++++++++++++++++++++++-------
 drivers/bus/mhi/core/pm.c   | 31 +++++++++++++++++++------------
 2 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index bafc12a..da32c23 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -376,6 +376,7 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv)
 	enum mhi_state state = MHI_STATE_MAX;
 	enum mhi_pm_state pm_state = 0;
 	enum mhi_ee_type ee = 0;
+	bool handle_rddm = false;
 
 	write_lock_irq(&mhi_cntrl->pm_lock);
 	if (!MHI_REG_ACCESS_VALID(mhi_cntrl->pm_state)) {
@@ -390,22 +391,40 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv)
 		TO_MHI_EXEC_STR(mhi_cntrl->ee), TO_MHI_EXEC_STR(ee),
 		TO_MHI_STATE_STR(state));
 
-	if (state == MHI_STATE_SYS_ERR) {
-		dev_dbg(dev, "System error detected\n");
-		pm_state = mhi_tryset_pm_state(mhi_cntrl,
-					       MHI_PM_SYS_ERR_DETECT);
-	}
-	write_unlock_irq(&mhi_cntrl->pm_lock);
-
 	 /* If device supports RDDM don't bother processing SYS error */
 	if (mhi_cntrl->rddm_image) {
+		/* host may be performing a device power down already */
+		if (!mhi_is_active(mhi_cntrl)) {
+			write_unlock_irq(&mhi_cntrl->pm_lock);
+			goto exit_intvec;
+		}
+
 		if (mhi_cntrl->ee == MHI_EE_RDDM && mhi_cntrl->ee != ee) {
+			/* prevent clients from queueing any more packets */
+			pm_state = mhi_tryset_pm_state(mhi_cntrl,
+						       MHI_PM_SYS_ERR_DETECT);
+			if (pm_state == MHI_PM_SYS_ERR_DETECT)
+				handle_rddm = true;
+		}
+
+		write_unlock_irq(&mhi_cntrl->pm_lock);
+
+		if (handle_rddm) {
+			dev_err(dev, "RDDM event occurred!\n");
 			mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_EE_RDDM);
 			wake_up_all(&mhi_cntrl->state_event);
 		}
 		goto exit_intvec;
 	}
 
+	if (state == MHI_STATE_SYS_ERR) {
+		dev_dbg(dev, "System error detected\n");
+		pm_state = mhi_tryset_pm_state(mhi_cntrl,
+					       MHI_PM_SYS_ERR_DETECT);
+	}
+
+	write_unlock_irq(&mhi_cntrl->pm_lock);
+
 	if (pm_state == MHI_PM_SYS_ERR_DETECT) {
 		wake_up_all(&mhi_cntrl->state_event);
 
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index b2b3de7..1daed86 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -465,15 +465,10 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl,
 	write_lock_irq(&mhi_cntrl->pm_lock);
 	prev_state = mhi_cntrl->pm_state;
 	cur_state = mhi_tryset_pm_state(mhi_cntrl, transition_state);
-	if (cur_state == transition_state) {
-		mhi_cntrl->ee = MHI_EE_DISABLE_TRANSITION;
+	if (cur_state == MHI_PM_SYS_ERR_PROCESS)
 		mhi_cntrl->dev_state = MHI_STATE_RESET;
-	}
 	write_unlock_irq(&mhi_cntrl->pm_lock);
 
-	/* Wake up threads waiting for state transition */
-	wake_up_all(&mhi_cntrl->state_event);
-
 	if (cur_state != transition_state) {
 		dev_err(dev, "Failed to transition to state: %s from: %s\n",
 			to_mhi_pm_state_str(transition_state),
@@ -482,6 +477,11 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl,
 		return;
 	}
 
+	mhi_cntrl->ee = MHI_EE_DISABLE_TRANSITION;
+
+	/* Wake up threads waiting for state transition */
+	wake_up_all(&mhi_cntrl->state_event);
+
 	/* Trigger MHI RESET so that the device will not access host memory */
 	if (MHI_REG_ACCESS_VALID(prev_state)) {
 		u32 in_reset = -1;
@@ -1048,22 +1048,29 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful)
 	enum dev_st_transition next_state = DEV_ST_TRANSITION_DISABLE;
 	struct device *dev = &mhi_cntrl->mhi_dev->dev;
 
+	mutex_lock(&mhi_cntrl->pm_mutex);
+	write_lock_irq(&mhi_cntrl->pm_lock);
+
 	/* If it's not a graceful shutdown, force MHI to linkdown state */
 	if (!graceful) {
-		mutex_lock(&mhi_cntrl->pm_mutex);
-		write_lock_irq(&mhi_cntrl->pm_lock);
 		cur_state = mhi_tryset_pm_state(mhi_cntrl,
 						MHI_PM_LD_ERR_FATAL_DETECT);
-		write_unlock_irq(&mhi_cntrl->pm_lock);
-		mutex_unlock(&mhi_cntrl->pm_mutex);
-		if (cur_state != MHI_PM_LD_ERR_FATAL_DETECT)
+		if (cur_state != MHI_PM_LD_ERR_FATAL_DETECT) {
 			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
+		} else {
 			next_state = DEV_ST_TRANSITION_FATAL;
+			wake_up_all(&mhi_cntrl->state_event);
+		}
 	}
 
+	/* mark device as inactive to avoid any further host processing */
+	mhi_cntrl->dev_state = MHI_STATE_RESET;
+
+	write_unlock_irq(&mhi_cntrl->pm_lock);
+	mutex_unlock(&mhi_cntrl->pm_mutex);
+
 	mhi_queue_state_transition(mhi_cntrl, next_state);
 
 	/* Wait for shutdown to complete */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 3/6] bus: mhi: core: Check for RDDM support before forcing a device crash
  2020-05-20  0:30 [PATCH v1 0/6] Bug fixes and bootup and shutdown improvements Bhaumik Bhatt
  2020-05-20  0:30 ` [PATCH v1 1/6] bus: mhi: core: Improve shutdown handling after link down detection Bhaumik Bhatt
  2020-05-20  0:30 ` [PATCH v1 2/6] bus: mhi: core: Mark device inactive soon after host issues a shutdown Bhaumik Bhatt
@ 2020-05-20  0:30 ` Bhaumik Bhatt
  2020-05-20  0:30 ` [PATCH v1 4/6] bus: mhi: core: Use common name for BHI firmware load function Bhaumik Bhatt
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Bhaumik Bhatt @ 2020-05-20  0:30 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, Bhaumik Bhatt

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 <bbhatt@codeaurora.org>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 4/6] bus: mhi: core: Use common name for BHI firmware load function
  2020-05-20  0:30 [PATCH v1 0/6] Bug fixes and bootup and shutdown improvements Bhaumik Bhatt
                   ` (2 preceding siblings ...)
  2020-05-20  0:30 ` [PATCH v1 3/6] bus: mhi: core: Check for RDDM support before forcing a device crash Bhaumik Bhatt
@ 2020-05-20  0:30 ` Bhaumik Bhatt
  2020-05-20  0:30 ` [PATCH v1 5/6] bus: mhi: core: Introduce support for manual AMSS loading Bhaumik Bhatt
  2020-05-20  0:30 ` [PATCH v1 6/6] bus: mhi: core: Process execution environment changes serially Bhaumik Bhatt
  5 siblings, 0 replies; 10+ messages in thread
From: Bhaumik Bhatt @ 2020-05-20  0:30 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, Bhaumik Bhatt

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 <bbhatt@codeaurora.org>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 5/6] bus: mhi: core: Introduce support for manual AMSS loading
  2020-05-20  0:30 [PATCH v1 0/6] Bug fixes and bootup and shutdown improvements Bhaumik Bhatt
                   ` (3 preceding siblings ...)
  2020-05-20  0:30 ` [PATCH v1 4/6] bus: mhi: core: Use common name for BHI firmware load function Bhaumik Bhatt
@ 2020-05-20  0:30 ` Bhaumik Bhatt
  2020-05-20  0:30 ` [PATCH v1 6/6] bus: mhi: core: Process execution environment changes serially Bhaumik Bhatt
  5 siblings, 0 replies; 10+ messages in thread
From: Bhaumik Bhatt @ 2020-05-20  0:30 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, Bhaumik Bhatt

During full boot chain firmware load, the PM state worker called in PBL
mode waits for SBL and then does the AMSS image download. This does not
allow usage of any SBL-specific channels and should only be done when
powering the device up synchronously. If the controller plans to use any
SBL channels using an asynchronous bootup flow, SBL device creation
cannot be neglected and the option to manually load the AMSS image when
the controller is ready also becomes necessary.

To allow this, introduce an optional boolean for 'manual_amss_load' and
give the controller a callback once the bus is ready to move out of SBL.
Introduce a public API for the controller to download the AMSS image and
rename the internal download function to use the generic _bhie suffix
over _amss.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
 drivers/bus/mhi/core/boot.c | 90 ++++++++++++++++++++++-----------------------
 drivers/bus/mhi/core/pm.c   |  6 +++
 include/linux/mhi.h         | 10 +++++
 3 files changed, 61 insertions(+), 45 deletions(-)

diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
index 34ce102..2528fb3 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -171,7 +171,7 @@ int mhi_download_rddm_img(struct mhi_controller *mhi_cntrl, bool in_panic)
 }
 EXPORT_SYMBOL_GPL(mhi_download_rddm_img);
 
-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;
@@ -187,7 +187,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));
@@ -218,6 +218,34 @@ static int mhi_fw_load_amss(struct mhi_controller *mhi_cntrl,
 	return (!ret) ? -ETIMEDOUT : 0;
 }
 
+int mhi_download_amss_image(struct mhi_controller *mhi_cntrl)
+{
+	struct image_info *image_info = mhi_cntrl->fbc_image;
+	struct device *dev = &mhi_cntrl->mhi_dev->dev;
+	int ret;
+
+	if (!mhi_cntrl->fbc_download)
+		return -EINVAL;
+
+	if (!image_info)
+		return -EIO;
+
+	if (mhi_cntrl->ee != MHI_EE_SBL) {
+		dev_err(dev, "MHI could not load AMSS, EE:%s\n",
+			TO_MHI_EXEC_STR(mhi_cntrl->ee));
+		return -EINVAL;
+	}
+
+	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);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mhi_download_amss_image);
+
 static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
 			   dma_addr_t dma_addr,
 			   size_t size)
@@ -386,7 +414,6 @@ static void mhi_firmware_copy(struct mhi_controller *mhi_cntrl,
 void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 {
 	const struct firmware *firmware = NULL;
-	struct image_info *image_info;
 	struct device *dev = &mhi_cntrl->mhi_dev->dev;
 	const char *fw_name;
 	void *buf;
@@ -441,27 +468,22 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 		size = firmware->size;
 
 	buf = mhi_alloc_coherent(mhi_cntrl, size, &dma_addr, GFP_KERNEL);
-	if (!buf) {
-		release_firmware(firmware);
-		return;
-	}
+	if (!buf)
+		goto exit_fw_load;
 
 	/* Download SBL or EDL image using BHI */
 	memcpy(buf, firmware->data, 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)
-		release_firmware(firmware);
-
 	/* Error or in EDL mode, we're done */
 	if (ret) {
 		dev_err(dev, "MHI did not load SBL/EDL image, ret:%d\n", ret);
-		return;
+		goto exit_fw_load;
 	}
 
 	if (mhi_cntrl->ee == MHI_EE_EDL)
-		return;
+		goto exit_fw_load;
 
 	write_lock_irq(&mhi_cntrl->pm_lock);
 	mhi_cntrl->dev_state = MHI_STATE_RESET;
@@ -474,8 +496,10 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	if (mhi_cntrl->fbc_download) {
 		ret = mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->fbc_image,
 					   firmware->size);
-		if (ret)
-			goto error_alloc_fw_table;
+		if (ret) {
+			mhi_cntrl->fbc_image = NULL;
+			goto exit_fw_load;
+		}
 
 		/* Load the firmware into BHIE vec table */
 		mhi_firmware_copy(mhi_cntrl, firmware, mhi_cntrl->fbc_image);
@@ -484,42 +508,18 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 fw_load_ee_pthru:
 	/* Transitioning into MHI RESET->READY state */
 	ret = mhi_ready_state_transition(mhi_cntrl);
-
-	if (!mhi_cntrl->fbc_download)
-		return;
-
 	if (ret) {
 		dev_err(dev, "MHI did not enter READY state\n");
-		goto error_read;
-	}
-
-	/* Wait for the SBL event */
-	ret = wait_event_timeout(mhi_cntrl->state_event,
-				 mhi_cntrl->ee == MHI_EE_SBL ||
-				 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)) {
-		dev_err(dev, "MHI did not enter SBL\n");
-		goto error_read;
+		if (mhi_cntrl->fbc_download) {
+			mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
+			mhi_cntrl->fbc_image = NULL;
+		}
 	}
 
-	/* Start full firmware image download */
-	image_info = mhi_cntrl->fbc_image;
-	ret = mhi_fw_load_amss(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);
-
-	release_firmware(firmware);
+exit_fw_load:
+	if (firmware)
+		release_firmware(firmware);
 
 	return;
-
-error_read:
-	mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
-	mhi_cntrl->fbc_image = NULL;
-
-error_alloc_fw_table:
-	release_firmware(firmware);
 }
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index 52c290c6..5041df9 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -655,6 +655,12 @@ void mhi_pm_st_worker(struct work_struct *work)
 			 * either SBL or AMSS states
 			 */
 			mhi_create_devices(mhi_cntrl);
+
+			/* notify controller it can move out of SBL */
+			if (mhi_cntrl->manual_amss_load)
+				mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_EE_SBL);
+			else
+				mhi_download_amss_image(mhi_cntrl);
 			break;
 		case DEV_ST_TRANSITION_MISSION_MODE:
 			mhi_pm_mission_mode_transition(mhi_cntrl);
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 42e4d1e..2eb98a9 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -33,6 +33,7 @@
  * @MHI_CB_PENDING_DATA: New data available for client to process
  * @MHI_CB_LPM_ENTER: MHI host entered low power mode
  * @MHI_CB_LPM_EXIT: MHI host about to exit low power mode
+ * @MHI_CB_EE_SBL: MHI device entered SBL exec env (for manually loading AMSS)
  * @MHI_CB_EE_RDDM: MHI device entered RDDM exec env
  * @MHI_CB_EE_MISSION_MODE: MHI device entered Mission Mode exec env
  * @MHI_CB_SYS_ERROR: MHI device entered error state (may recover)
@@ -44,6 +45,7 @@ enum mhi_callback {
 	MHI_CB_PENDING_DATA,
 	MHI_CB_LPM_ENTER,
 	MHI_CB_LPM_EXIT,
+	MHI_CB_EE_SBL,
 	MHI_CB_EE_RDDM,
 	MHI_CB_EE_MISSION_MODE,
 	MHI_CB_SYS_ERROR,
@@ -354,6 +356,7 @@ struct mhi_controller_config {
  * @buffer_len: Bounce buffer length
  * @bounce_buf: Use of bounce buffer
  * @fbc_download: MHI host needs to do complete image transfer (optional)
+ * @manual_amss_load: Set to manually trigger AMSS image transfer (optional)
  * @pre_init: MHI host needs to do pre-initialization before power up
  * @wake_set: Device wakeup set flag
  *
@@ -444,6 +447,7 @@ struct mhi_controller {
 	size_t buffer_len;
 	bool bounce_buf;
 	bool fbc_download;
+	bool manual_amss_load;
 	bool pre_init;
 	bool wake_set;
 };
@@ -646,6 +650,12 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
 int mhi_download_rddm_img(struct mhi_controller *mhi_cntrl, bool in_panic);
 
 /**
+ * mhi_download_amss_image - Download AMSS/mission mode image to the device
+ * @mhi_cntrl: MHI controller
+ */
+int mhi_download_amss_image(struct mhi_controller *mhi_cntrl);
+
+/**
  * mhi_force_rddm_mode - Force device into rddm mode
  * @mhi_cntrl: MHI controller
  */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 6/6] bus: mhi: core: Process execution environment changes serially
  2020-05-20  0:30 [PATCH v1 0/6] Bug fixes and bootup and shutdown improvements Bhaumik Bhatt
                   ` (4 preceding siblings ...)
  2020-05-20  0:30 ` [PATCH v1 5/6] bus: mhi: core: Introduce support for manual AMSS loading Bhaumik Bhatt
@ 2020-05-20  0:30 ` Bhaumik Bhatt
  5 siblings, 0 replies; 10+ messages in thread
From: Bhaumik Bhatt @ 2020-05-20  0:30 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, Bhaumik Bhatt

In current design, whenever the "bhi" interrupt is fired, the execution
environment is updated. This can cause race conditions and impede any
ongoing power up or power down processing. For example, if a power down
is in progress and the host has updated the execution environment to a
local "disabled" state, any BHI interrupt could replace it with the
execution environment from the BHI EE register. Another example would
be that the device can enter mission mode while the device creation for
SBL is still ongoing leading to multiple attempts at opening the same
channel.

Ensure that EE changes are handled only from appropriate places and
occur one after another.

For RDDM, handle it as a critical event directly from the interrupt
handler and remove RDDM EE change event from the control event ring to
prevent the driver from issuing multiple callbacks. SBL handling
requires no change. For AMSS/mission mode, hold off the update until
the client is notified with a status callback.

To sum it up, ensure client readiness before processing mission mode and
move to an error state as soon as possible to avoid a bad state.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
 drivers/bus/mhi/core/main.c | 12 ++++--------
 drivers/bus/mhi/core/pm.c   | 21 +++++++++++++++++----
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index da32c23..d25f321 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -385,8 +385,7 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv)
 	}
 
 	state = mhi_get_mhi_state(mhi_cntrl);
-	ee = mhi_cntrl->ee;
-	mhi_cntrl->ee = mhi_get_exec_env(mhi_cntrl);
+	ee = mhi_get_exec_env(mhi_cntrl);
 	dev_dbg(dev, "local ee:%s device ee:%s dev_state:%s\n",
 		TO_MHI_EXEC_STR(mhi_cntrl->ee), TO_MHI_EXEC_STR(ee),
 		TO_MHI_STATE_STR(state));
@@ -399,7 +398,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv)
 			goto exit_intvec;
 		}
 
-		if (mhi_cntrl->ee == MHI_EE_RDDM && mhi_cntrl->ee != ee) {
+		if (ee == MHI_EE_RDDM && mhi_cntrl->ee != ee) {
+			mhi_cntrl->ee = MHI_EE_RDDM;
+
 			/* prevent clients from queueing any more packets */
 			pm_state = mhi_tryset_pm_state(mhi_cntrl,
 						       MHI_PM_SYS_ERR_DETECT);
@@ -794,11 +795,6 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller *mhi_cntrl,
 				st = DEV_ST_TRANSITION_MISSION_MODE;
 				break;
 			case MHI_EE_RDDM:
-				mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_EE_RDDM);
-				write_lock_irq(&mhi_cntrl->pm_lock);
-				mhi_cntrl->ee = event;
-				write_unlock_irq(&mhi_cntrl->pm_lock);
-				wake_up_all(&mhi_cntrl->state_event);
 				break;
 			default:
 				dev_err(dev,
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index 5041df9..4407338 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -377,22 +377,35 @@ static int mhi_pm_mission_mode_transition(struct mhi_controller *mhi_cntrl)
 {
 	struct mhi_event *mhi_event;
 	struct device *dev = &mhi_cntrl->mhi_dev->dev;
+	enum mhi_ee_type ee = MHI_EE_MAX;
 	int i, ret;
 
 	dev_dbg(dev, "Processing Mission Mode transition\n");
 
 	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);
+		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(ee)) {
+		dev_err(dev, "Invalid EE for Mission Mode: %s\n",
+			TO_MHI_EXEC_STR(ee));
 		return -EIO;
+	}
 
-	wake_up_all(&mhi_cntrl->state_event);
-
+	/*
+	 * let controller prepare for mission mode before making the execution
+	 * environment change so as to defer core driver activity
+	 */
 	mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_EE_MISSION_MODE);
 
+	/* ready the core driver for mission mode */
+	write_lock_irq(&mhi_cntrl->pm_lock);
+	mhi_cntrl->ee = ee;
+	write_unlock_irq(&mhi_cntrl->pm_lock);
+
+	wake_up_all(&mhi_cntrl->state_event);
+
 	/* Force MHI to be in M0 state before continuing */
 	ret = __mhi_device_get_sync(mhi_cntrl);
 	if (ret)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/6] bus: mhi: core: Mark device inactive soon after host issues a shutdown
  2020-05-20  0:30 ` [PATCH v1 2/6] bus: mhi: core: Mark device inactive soon after host issues a shutdown Bhaumik Bhatt
@ 2020-05-20  5:16   ` kbuild test robot
  2020-05-21  5:50   ` kbuild test robot
  2020-05-21 10:26   ` kbuild test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2020-05-20  5:16 UTC (permalink / raw)
  To: Bhaumik Bhatt, manivannan.sadhasivam
  Cc: kbuild-all, linux-arm-msm, hemantk, jhugo, linux-kernel, Bhaumik Bhatt

[-- Attachment #1: Type: text/plain, Size: 4083 bytes --]

Hi Bhaumik,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20200519]
[cannot apply to linus/master v5.7-rc6 v5.7-rc5 v5.7-rc4 v5.7-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Bhaumik-Bhatt/Bug-fixes-and-bootup-and-shutdown-improvements/20200520-083400
base:    fb57b1fabcb28f358901b2df90abd2b48abc1ca8
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

drivers/bus/mhi/core/main.c: In function 'mhi_intvec_threaded_handler':
>> drivers/bus/mhi/core/main.c:397:8: error: implicit declaration of function 'mhi_is_active' [-Werror=implicit-function-declaration]
397 |   if (!mhi_is_active(mhi_cntrl)) {
|        ^~~~~~~~~~~~~
cc1: some warnings being treated as errors

vim +/mhi_is_active +397 drivers/bus/mhi/core/main.c

   371	
   372	irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv)
   373	{
   374		struct mhi_controller *mhi_cntrl = priv;
   375		struct device *dev = &mhi_cntrl->mhi_dev->dev;
   376		enum mhi_state state = MHI_STATE_MAX;
   377		enum mhi_pm_state pm_state = 0;
   378		enum mhi_ee_type ee = 0;
   379		bool handle_rddm = false;
   380	
   381		write_lock_irq(&mhi_cntrl->pm_lock);
   382		if (!MHI_REG_ACCESS_VALID(mhi_cntrl->pm_state)) {
   383			write_unlock_irq(&mhi_cntrl->pm_lock);
   384			goto exit_intvec;
   385		}
   386	
   387		state = mhi_get_mhi_state(mhi_cntrl);
   388		ee = mhi_cntrl->ee;
   389		mhi_cntrl->ee = mhi_get_exec_env(mhi_cntrl);
   390		dev_dbg(dev, "local ee:%s device ee:%s dev_state:%s\n",
   391			TO_MHI_EXEC_STR(mhi_cntrl->ee), TO_MHI_EXEC_STR(ee),
   392			TO_MHI_STATE_STR(state));
   393	
   394		 /* If device supports RDDM don't bother processing SYS error */
   395		if (mhi_cntrl->rddm_image) {
   396			/* host may be performing a device power down already */
 > 397			if (!mhi_is_active(mhi_cntrl)) {
   398				write_unlock_irq(&mhi_cntrl->pm_lock);
   399				goto exit_intvec;
   400			}
   401	
   402			if (mhi_cntrl->ee == MHI_EE_RDDM && mhi_cntrl->ee != ee) {
   403				/* prevent clients from queueing any more packets */
   404				pm_state = mhi_tryset_pm_state(mhi_cntrl,
   405							       MHI_PM_SYS_ERR_DETECT);
   406				if (pm_state == MHI_PM_SYS_ERR_DETECT)
   407					handle_rddm = true;
   408			}
   409	
   410			write_unlock_irq(&mhi_cntrl->pm_lock);
   411	
   412			if (handle_rddm) {
   413				dev_err(dev, "RDDM event occurred!\n");
   414				mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_EE_RDDM);
   415				wake_up_all(&mhi_cntrl->state_event);
   416			}
   417			goto exit_intvec;
   418		}
   419	
   420		if (state == MHI_STATE_SYS_ERR) {
   421			dev_dbg(dev, "System error detected\n");
   422			pm_state = mhi_tryset_pm_state(mhi_cntrl,
   423						       MHI_PM_SYS_ERR_DETECT);
   424		}
   425	
   426		write_unlock_irq(&mhi_cntrl->pm_lock);
   427	
   428		if (pm_state == MHI_PM_SYS_ERR_DETECT) {
   429			wake_up_all(&mhi_cntrl->state_event);
   430	
   431			/* For fatal errors, we let controller decide next step */
   432			if (MHI_IN_PBL(ee))
   433				mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_FATAL_ERROR);
   434			else
   435				mhi_pm_sys_err_handler(mhi_cntrl);
   436		}
   437	
   438	exit_intvec:
   439	
   440		return IRQ_HANDLED;
   441	}
   442	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 64275 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/6] bus: mhi: core: Mark device inactive soon after host issues a shutdown
  2020-05-20  0:30 ` [PATCH v1 2/6] bus: mhi: core: Mark device inactive soon after host issues a shutdown Bhaumik Bhatt
  2020-05-20  5:16   ` kbuild test robot
@ 2020-05-21  5:50   ` kbuild test robot
  2020-05-21 10:26   ` kbuild test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2020-05-21  5:50 UTC (permalink / raw)
  To: Bhaumik Bhatt, manivannan.sadhasivam
  Cc: kbuild-all, clang-built-linux, linux-arm-msm, hemantk, jhugo,
	linux-kernel, Bhaumik Bhatt

[-- Attachment #1: Type: text/plain, Size: 4165 bytes --]

Hi Bhaumik,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20200519]
[cannot apply to linus/master v5.7-rc6 v5.7-rc5 v5.7-rc4 v5.7-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Bhaumik-Bhatt/Bug-fixes-and-bootup-and-shutdown-improvements/20200520-083400
base:    fb57b1fabcb28f358901b2df90abd2b48abc1ca8
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 3393cc4cebf9969db94dc424b7a2b6195589c33b)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/bus/mhi/core/main.c:397:8: error: implicit declaration of function 'mhi_is_active' [-Werror,-Wimplicit-function-declaration]
if (!mhi_is_active(mhi_cntrl)) {
^
1 error generated.

vim +/mhi_is_active +397 drivers/bus/mhi/core/main.c

   371	
   372	irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv)
   373	{
   374		struct mhi_controller *mhi_cntrl = priv;
   375		struct device *dev = &mhi_cntrl->mhi_dev->dev;
   376		enum mhi_state state = MHI_STATE_MAX;
   377		enum mhi_pm_state pm_state = 0;
   378		enum mhi_ee_type ee = 0;
   379		bool handle_rddm = false;
   380	
   381		write_lock_irq(&mhi_cntrl->pm_lock);
   382		if (!MHI_REG_ACCESS_VALID(mhi_cntrl->pm_state)) {
   383			write_unlock_irq(&mhi_cntrl->pm_lock);
   384			goto exit_intvec;
   385		}
   386	
   387		state = mhi_get_mhi_state(mhi_cntrl);
   388		ee = mhi_cntrl->ee;
   389		mhi_cntrl->ee = mhi_get_exec_env(mhi_cntrl);
   390		dev_dbg(dev, "local ee:%s device ee:%s dev_state:%s\n",
   391			TO_MHI_EXEC_STR(mhi_cntrl->ee), TO_MHI_EXEC_STR(ee),
   392			TO_MHI_STATE_STR(state));
   393	
   394		 /* If device supports RDDM don't bother processing SYS error */
   395		if (mhi_cntrl->rddm_image) {
   396			/* host may be performing a device power down already */
 > 397			if (!mhi_is_active(mhi_cntrl)) {
   398				write_unlock_irq(&mhi_cntrl->pm_lock);
   399				goto exit_intvec;
   400			}
   401	
   402			if (mhi_cntrl->ee == MHI_EE_RDDM && mhi_cntrl->ee != ee) {
   403				/* prevent clients from queueing any more packets */
   404				pm_state = mhi_tryset_pm_state(mhi_cntrl,
   405							       MHI_PM_SYS_ERR_DETECT);
   406				if (pm_state == MHI_PM_SYS_ERR_DETECT)
   407					handle_rddm = true;
   408			}
   409	
   410			write_unlock_irq(&mhi_cntrl->pm_lock);
   411	
   412			if (handle_rddm) {
   413				dev_err(dev, "RDDM event occurred!\n");
   414				mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_EE_RDDM);
   415				wake_up_all(&mhi_cntrl->state_event);
   416			}
   417			goto exit_intvec;
   418		}
   419	
   420		if (state == MHI_STATE_SYS_ERR) {
   421			dev_dbg(dev, "System error detected\n");
   422			pm_state = mhi_tryset_pm_state(mhi_cntrl,
   423						       MHI_PM_SYS_ERR_DETECT);
   424		}
   425	
   426		write_unlock_irq(&mhi_cntrl->pm_lock);
   427	
   428		if (pm_state == MHI_PM_SYS_ERR_DETECT) {
   429			wake_up_all(&mhi_cntrl->state_event);
   430	
   431			/* For fatal errors, we let controller decide next step */
   432			if (MHI_IN_PBL(ee))
   433				mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_FATAL_ERROR);
   434			else
   435				mhi_pm_sys_err_handler(mhi_cntrl);
   436		}
   437	
   438	exit_intvec:
   439	
   440		return IRQ_HANDLED;
   441	}
   442	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 74423 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/6] bus: mhi: core: Mark device inactive soon after host issues a shutdown
  2020-05-20  0:30 ` [PATCH v1 2/6] bus: mhi: core: Mark device inactive soon after host issues a shutdown Bhaumik Bhatt
  2020-05-20  5:16   ` kbuild test robot
  2020-05-21  5:50   ` kbuild test robot
@ 2020-05-21 10:26   ` kbuild test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2020-05-21 10:26 UTC (permalink / raw)
  To: Bhaumik Bhatt, manivannan.sadhasivam
  Cc: kbuild-all, linux-arm-msm, hemantk, jhugo, linux-kernel, Bhaumik Bhatt

[-- Attachment #1: Type: text/plain, Size: 7398 bytes --]

Hi Bhaumik,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20200519]
[cannot apply to linus/master v5.7-rc6 v5.7-rc5 v5.7-rc4 v5.7-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Bhaumik-Bhatt/Bug-fixes-and-bootup-and-shutdown-improvements/20200520-083400
base:    fb57b1fabcb28f358901b2df90abd2b48abc1ca8
config: xtensa-randconfig-r036-20200520 (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

In file included from include/linux/dev_printk.h:14,
from include/linux/device.h:15,
from drivers/bus/mhi/core/main.c:7:
include/linux/dma-mapping.h: In function 'dma_map_resource':
arch/xtensa/include/asm/page.h:193:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
193 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
|         ^~
include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
58 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
|                                                    ^~~~
include/linux/dma-mapping.h:352:2: note: in expansion of macro 'if'
352 |  if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
|  ^~
include/linux/dma-mapping.h:352:6: note: in expansion of macro 'WARN_ON_ONCE'
352 |  if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
|      ^~~~~~~~~~~~
include/linux/dma-mapping.h:352:19: note: in expansion of macro 'pfn_valid'
352 |  if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
|                   ^~~~~~~~~
arch/xtensa/include/asm/page.h:193:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
193 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
|         ^~
include/linux/compiler.h:58:61: note: in definition of macro '__trace_if_var'
58 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
|                                                             ^~~~
include/linux/dma-mapping.h:352:2: note: in expansion of macro 'if'
352 |  if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
|  ^~
include/linux/dma-mapping.h:352:6: note: in expansion of macro 'WARN_ON_ONCE'
352 |  if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
|      ^~~~~~~~~~~~
include/linux/dma-mapping.h:352:19: note: in expansion of macro 'pfn_valid'
352 |  if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
|                   ^~~~~~~~~
arch/xtensa/include/asm/page.h:193:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
193 |  ((pfn) >= ARCH_PFN_OFFSET && ((pfn) - ARCH_PFN_OFFSET) < max_mapnr)
|         ^~
include/linux/compiler.h:69:3: note: in definition of macro '__trace_if_value'
69 |  (cond) ?              |   ^~~~
include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
56 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
|                            ^~~~~~~~~~~~~~
include/linux/dma-mapping.h:352:2: note: in expansion of macro 'if'
352 |  if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
|  ^~
include/linux/dma-mapping.h:352:6: note: in expansion of macro 'WARN_ON_ONCE'
352 |  if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
|      ^~~~~~~~~~~~
include/linux/dma-mapping.h:352:19: note: in expansion of macro 'pfn_valid'
352 |  if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
|                   ^~~~~~~~~
drivers/bus/mhi/core/main.c: In function 'mhi_intvec_threaded_handler':
drivers/bus/mhi/core/main.c:397:8: error: implicit declaration of function 'mhi_is_active' [-Werror=implicit-function-declaration]
397 |   if (!mhi_is_active(mhi_cntrl)) {
|        ^~~~~~~~~~~~~
include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
58 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
|                                                    ^~~~
>> drivers/bus/mhi/core/main.c:397:3: note: in expansion of macro 'if'
397 |   if (!mhi_is_active(mhi_cntrl)) {
|   ^~
cc1: some warnings being treated as errors

vim +/if +397 drivers/bus/mhi/core/main.c

   371	
   372	irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv)
   373	{
   374		struct mhi_controller *mhi_cntrl = priv;
   375		struct device *dev = &mhi_cntrl->mhi_dev->dev;
   376		enum mhi_state state = MHI_STATE_MAX;
   377		enum mhi_pm_state pm_state = 0;
   378		enum mhi_ee_type ee = 0;
   379		bool handle_rddm = false;
   380	
   381		write_lock_irq(&mhi_cntrl->pm_lock);
   382		if (!MHI_REG_ACCESS_VALID(mhi_cntrl->pm_state)) {
   383			write_unlock_irq(&mhi_cntrl->pm_lock);
   384			goto exit_intvec;
   385		}
   386	
   387		state = mhi_get_mhi_state(mhi_cntrl);
   388		ee = mhi_cntrl->ee;
   389		mhi_cntrl->ee = mhi_get_exec_env(mhi_cntrl);
   390		dev_dbg(dev, "local ee:%s device ee:%s dev_state:%s\n",
   391			TO_MHI_EXEC_STR(mhi_cntrl->ee), TO_MHI_EXEC_STR(ee),
   392			TO_MHI_STATE_STR(state));
   393	
   394		 /* If device supports RDDM don't bother processing SYS error */
   395		if (mhi_cntrl->rddm_image) {
   396			/* host may be performing a device power down already */
 > 397			if (!mhi_is_active(mhi_cntrl)) {
   398				write_unlock_irq(&mhi_cntrl->pm_lock);
   399				goto exit_intvec;
   400			}
   401	
   402			if (mhi_cntrl->ee == MHI_EE_RDDM && mhi_cntrl->ee != ee) {
   403				/* prevent clients from queueing any more packets */
   404				pm_state = mhi_tryset_pm_state(mhi_cntrl,
   405							       MHI_PM_SYS_ERR_DETECT);
   406				if (pm_state == MHI_PM_SYS_ERR_DETECT)
   407					handle_rddm = true;
   408			}
   409	
   410			write_unlock_irq(&mhi_cntrl->pm_lock);
   411	
   412			if (handle_rddm) {
   413				dev_err(dev, "RDDM event occurred!\n");
   414				mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_EE_RDDM);
   415				wake_up_all(&mhi_cntrl->state_event);
   416			}
   417			goto exit_intvec;
   418		}
   419	
   420		if (state == MHI_STATE_SYS_ERR) {
   421			dev_dbg(dev, "System error detected\n");
   422			pm_state = mhi_tryset_pm_state(mhi_cntrl,
   423						       MHI_PM_SYS_ERR_DETECT);
   424		}
   425	
   426		write_unlock_irq(&mhi_cntrl->pm_lock);
   427	
   428		if (pm_state == MHI_PM_SYS_ERR_DETECT) {
   429			wake_up_all(&mhi_cntrl->state_event);
   430	
   431			/* For fatal errors, we let controller decide next step */
   432			if (MHI_IN_PBL(ee))
   433				mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_FATAL_ERROR);
   434			else
   435				mhi_pm_sys_err_handler(mhi_cntrl);
   436		}
   437	
   438	exit_intvec:
   439	
   440		return IRQ_HANDLED;
   441	}
   442	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29445 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-05-21 10:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20  0:30 [PATCH v1 0/6] Bug fixes and bootup and shutdown improvements Bhaumik Bhatt
2020-05-20  0:30 ` [PATCH v1 1/6] bus: mhi: core: Improve shutdown handling after link down detection Bhaumik Bhatt
2020-05-20  0:30 ` [PATCH v1 2/6] bus: mhi: core: Mark device inactive soon after host issues a shutdown Bhaumik Bhatt
2020-05-20  5:16   ` kbuild test robot
2020-05-21  5:50   ` kbuild test robot
2020-05-21 10:26   ` kbuild test robot
2020-05-20  0:30 ` [PATCH v1 3/6] bus: mhi: core: Check for RDDM support before forcing a device crash Bhaumik Bhatt
2020-05-20  0:30 ` [PATCH v1 4/6] bus: mhi: core: Use common name for BHI firmware load function Bhaumik Bhatt
2020-05-20  0:30 ` [PATCH v1 5/6] bus: mhi: core: Introduce support for manual AMSS loading Bhaumik Bhatt
2020-05-20  0:30 ` [PATCH v1 6/6] bus: mhi: core: Process execution environment changes serially Bhaumik Bhatt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).