[PATCH v2 0/6] Minor bug fixes and clean-up for MHI host driver

[PATCH v2 0/6] Minor bug fixes and clean-up for MHI host driver

[Bhaumik Bhatt]

This patch series serves to clean up the MHI host driver by removing an
unnecessary counter and an unused function. It also renames a function to make
it clearly worded. There is currently no user of this exported function which
makes it is safe to do so now.

Bug fixes include adding a missing EXPORT_SYMBOL_GPL to a function, and adding
a return value check to bail out of RDDM download in kernel panic path.

An outlier among the group exports the mhi_get_exec_env() API for use by
controller drivers, in case they need to determine behavior on the basis of the
current execution environment.

This set of patches was tested on arm64.

v2:
-Removed the declaration for mhi_get_exec_env() from internal.h
-Improved on the error log message in RDDM download exit case due to unknown EE

Bhaumik Bhatt (6):
  bus: mhi: core: Remove unnecessary counter from mhi_firmware_copy()
  bus: mhi: core: Add missing EXPORT_SYMBOL for mhi_get_mhi_state()
  bus: mhi: core: Expose mhi_get_exec_env() API for controllers
  bus: mhi: core: Remove unused mhi_fw_load_worker() declaration
  bus: mhi: core: Rename RDDM download function to use proper words
  bus: mhi: core: Skip RDDM download for unknown execution environment

 drivers/bus/mhi/core/boot.c     | 15 +++++++++------
 drivers/bus/mhi/core/internal.h |  2 --
 drivers/bus/mhi/core/main.c     |  2 ++
 include/linux/mhi.h             | 12 +++++++++---
 4 files changed, 20 insertions(+), 11 deletions(-)

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

[PATCH v2 1/6] bus: mhi: core: Remove unnecessary counter from mhi_firmware_copy()

[Bhaumik Bhatt]

There is an extra 'i' counter in the mhi_firmware_copy() function
which is unused. Remove it to clean-up code and reduce stack
space as well as improve efficiency of the function.

Fixes: cd457afb1667 ("bus: mhi: core: Add support for downloading firmware over BHIe")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/bus/mhi/core/boot.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
index 24422f5..6b6fd96 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -365,7 +365,6 @@ static void mhi_firmware_copy(struct mhi_controller *mhi_cntrl,
 	size_t remainder = firmware->size;
 	size_t to_cpy;
 	const u8 *buf = firmware->data;
-	int i = 0;
 	struct mhi_buf *mhi_buf = img_info->mhi_buf;
 	struct bhi_vec_entry *bhi_vec = img_info->bhi_vec;
 
@@ -377,7 +376,6 @@ static void mhi_firmware_copy(struct mhi_controller *mhi_cntrl,
 
 		buf += to_cpy;
 		remainder -= to_cpy;
-		i++;
 		bhi_vec++;
 		mhi_buf++;
 	}
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 2/6] bus: mhi: core: Add missing EXPORT_SYMBOL for mhi_get_mhi_state()

[Bhaumik Bhatt]

Add missing EXPORT_SYMBOL_GPL() declaration for mhi_get_mhi_state()
API.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/bus/mhi/core/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 2cff5dd..778897e 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -132,6 +132,7 @@ enum mhi_state mhi_get_mhi_state(struct mhi_controller *mhi_cntrl)
 				     MHISTATUS_MHISTATE_SHIFT, &state);
 	return ret ? MHI_STATE_MAX : state;
 }
+EXPORT_SYMBOL_GPL(mhi_get_mhi_state);
 
 int mhi_map_single_no_bb(struct mhi_controller *mhi_cntrl,
 			 struct mhi_buf_info *buf_info)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 3/6] bus: mhi: core: Expose mhi_get_exec_env() API for controllers

[Bhaumik Bhatt]

The mhi_get_exec_env() APIs can be used by the controller drivers
to query the execution environment of the MHI device. Expose it
so it can be used in some scenarios to determine behavior of
controllers.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
 drivers/bus/mhi/core/internal.h | 1 -
 drivers/bus/mhi/core/main.c     | 1 +
 include/linux/mhi.h             | 6 ++++++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 7989269..124c1b9 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -609,7 +609,6 @@ enum mhi_pm_state __must_check mhi_tryset_pm_state(
 					struct mhi_controller *mhi_cntrl,
 					enum mhi_pm_state state);
 const char *to_mhi_pm_state_str(enum mhi_pm_state state);
-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);
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 778897e..7c45657 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -123,6 +123,7 @@ enum mhi_ee_type mhi_get_exec_env(struct mhi_controller *mhi_cntrl)
 
 	return (ret) ? MHI_EE_MAX : exec;
 }
+EXPORT_SYMBOL_GPL(mhi_get_exec_env);
 
 enum mhi_state mhi_get_mhi_state(struct mhi_controller *mhi_cntrl)
 {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index d4841e5..9225d55 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -659,6 +659,12 @@ int mhi_download_rddm_img(struct mhi_controller *mhi_cntrl, bool in_panic);
 int mhi_force_rddm_mode(struct mhi_controller *mhi_cntrl);
 
 /**
+ * mhi_get_exec_env - Get BHI execution environment of the device
+ * @mhi_cntrl: MHI controller
+ */
+enum mhi_ee_type mhi_get_exec_env(struct mhi_controller *mhi_cntrl);
+
+/**
  * mhi_get_mhi_state - Get MHI state of the device
  * @mhi_cntrl: MHI controller
  */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 3/6] bus: mhi: core: Expose mhi_get_exec_env() API for controllers

[Manivannan Sadhasivam]

On Fri, Nov 06, 2020 at 09:44:47AM -0800, Bhaumik Bhatt wrote:
> The mhi_get_exec_env() APIs can be used by the controller drivers
> to query the execution environment of the MHI device. Expose it
> so it can be used in some scenarios to determine behavior of
> controllers.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/bus/mhi/core/internal.h | 1 -
>  drivers/bus/mhi/core/main.c     | 1 +
>  include/linux/mhi.h             | 6 ++++++
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
> index 7989269..124c1b9 100644
> --- a/drivers/bus/mhi/core/internal.h
> +++ b/drivers/bus/mhi/core/internal.h
> @@ -609,7 +609,6 @@ enum mhi_pm_state __must_check mhi_tryset_pm_state(
>  					struct mhi_controller *mhi_cntrl,
>  					enum mhi_pm_state state);
>  const char *to_mhi_pm_state_str(enum mhi_pm_state state);
> -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);
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 778897e..7c45657 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -123,6 +123,7 @@ enum mhi_ee_type mhi_get_exec_env(struct mhi_controller *mhi_cntrl)
>  
>  	return (ret) ? MHI_EE_MAX : exec;
>  }
> +EXPORT_SYMBOL_GPL(mhi_get_exec_env);
>  
>  enum mhi_state mhi_get_mhi_state(struct mhi_controller *mhi_cntrl)
>  {
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index d4841e5..9225d55 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -659,6 +659,12 @@ int mhi_download_rddm_img(struct mhi_controller *mhi_cntrl, bool in_panic);
>  int mhi_force_rddm_mode(struct mhi_controller *mhi_cntrl);
>  
>  /**
> + * mhi_get_exec_env - Get BHI execution environment of the device
> + * @mhi_cntrl: MHI controller
> + */
> +enum mhi_ee_type mhi_get_exec_env(struct mhi_controller *mhi_cntrl);
> +
> +/**
>   * mhi_get_mhi_state - Get MHI state of the device
>   * @mhi_cntrl: MHI controller
>   */
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

[PATCH v2 4/6] bus: mhi: core: Remove unused mhi_fw_load_worker() declaration

[Bhaumik Bhatt]

The mhi_fw_load_worker() function no longer exists. Remove its
declaration as part of code clean-up.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/bus/mhi/core/internal.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 124c1b9..df299c7 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -613,7 +613,6 @@ 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_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);
 int mhi_pm_m0_transition(struct mhi_controller *mhi_cntrl);
 void mhi_pm_m1_transition(struct mhi_controller *mhi_cntrl);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 5/6] bus: mhi: core: Rename RDDM download function to use proper words

[Bhaumik Bhatt]

mhi_download_rddm_img() uses a shorter version of the word image.
Expand it and rename the function to mhi_download_rddm_image().

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/bus/mhi/core/boot.c | 4 ++--
 include/linux/mhi.h         | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
index 6b6fd96..16244cc 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -147,7 +147,7 @@ static int __mhi_download_rddm_in_panic(struct mhi_controller *mhi_cntrl)
 }
 
 /* Download RDDM image from device */
-int mhi_download_rddm_img(struct mhi_controller *mhi_cntrl, bool in_panic)
+int mhi_download_rddm_image(struct mhi_controller *mhi_cntrl, bool in_panic)
 {
 	void __iomem *base = mhi_cntrl->bhie;
 	struct device *dev = &mhi_cntrl->mhi_dev->dev;
@@ -169,7 +169,7 @@ int mhi_download_rddm_img(struct mhi_controller *mhi_cntrl, bool in_panic)
 
 	return (rx_status == BHIE_RXVECSTATUS_STATUS_XFER_COMPL) ? 0 : -EIO;
 }
-EXPORT_SYMBOL_GPL(mhi_download_rddm_img);
+EXPORT_SYMBOL_GPL(mhi_download_rddm_image);
 
 static int mhi_fw_load_amss(struct mhi_controller *mhi_cntrl,
 			    const struct mhi_buf *mhi_buf)
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 9225d55..52b3c60 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -645,12 +645,12 @@ int mhi_pm_suspend(struct mhi_controller *mhi_cntrl);
 int mhi_pm_resume(struct mhi_controller *mhi_cntrl);
 
 /**
- * mhi_download_rddm_img - Download ramdump image from device for
- *                         debugging purpose.
+ * mhi_download_rddm_image - Download ramdump image from device for
+ *                           debugging purpose.
  * @mhi_cntrl: MHI controller
  * @in_panic: Download rddm image during kernel panic
  */
-int mhi_download_rddm_img(struct mhi_controller *mhi_cntrl, bool in_panic);
+int mhi_download_rddm_image(struct mhi_controller *mhi_cntrl, bool in_panic);
 
 /**
  * mhi_force_rddm_mode - Force device into rddm mode
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 6/6] bus: mhi: core: Skip RDDM download for unknown execution environment

[Bhaumik Bhatt]

If MHI is unable to determine the execution environment during
the panic path, host must skip the RDDM download. This can happen
if the BHI offset read or the BHI_EXECENV register read fails
indicating that the underlying transport is unresponsive. Hence,
there is no need to trigger an RDDM using SYSERR or request an
SOC reset.

Suggested-by: Hemant Kumar <hemantk@codeaurora.org>
Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
 drivers/bus/mhi/core/boot.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
index 16244cc..6f0cfb9 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -92,6 +92,9 @@ static int __mhi_download_rddm_in_panic(struct mhi_controller *mhi_cntrl)
 	 * image download completion.
 	 */
 	ee = mhi_get_exec_env(mhi_cntrl);
+	if (ee == MHI_EE_MAX)
+		goto error_exit_rddm;
+
 	if (ee != MHI_EE_RDDM) {
 		dev_dbg(dev, "Trigger device into RDDM mode using SYS ERR\n");
 		mhi_set_mhi_state(mhi_cntrl, MHI_STATE_SYS_ERR);
@@ -139,10 +142,12 @@ static int __mhi_download_rddm_in_panic(struct mhi_controller *mhi_cntrl)
 	ee = mhi_get_exec_env(mhi_cntrl);
 	ret = mhi_read_reg(mhi_cntrl, base, BHIE_RXVECSTATUS_OFFS, &rx_status);
 
-	dev_err(dev, "Did not complete RDDM transfer\n");
-	dev_err(dev, "Current EE: %s\n", TO_MHI_EXEC_STR(ee));
 	dev_err(dev, "RXVEC_STATUS: 0x%x\n", rx_status);
 
+error_exit_rddm:
+	dev_err(dev, "RDDM transfer failed. Current EE: %s\n",
+		TO_MHI_EXEC_STR(ee));
+
 	return -EIO;
 }
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 6/6] bus: mhi: core: Skip RDDM download for unknown execution environment

[Manivannan Sadhasivam]

On Fri, Nov 06, 2020 at 09:44:50AM -0800, Bhaumik Bhatt wrote:
> If MHI is unable to determine the execution environment during
> the panic path, host must skip the RDDM download. This can happen
> if the BHI offset read or the BHI_EXECENV register read fails
> indicating that the underlying transport is unresponsive. Hence,
> there is no need to trigger an RDDM using SYSERR or request an
> SOC reset.
> 
> Suggested-by: Hemant Kumar <hemantk@codeaurora.org>
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/bus/mhi/core/boot.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
> index 16244cc..6f0cfb9 100644
> --- a/drivers/bus/mhi/core/boot.c
> +++ b/drivers/bus/mhi/core/boot.c
> @@ -92,6 +92,9 @@ static int __mhi_download_rddm_in_panic(struct mhi_controller *mhi_cntrl)
>  	 * image download completion.
>  	 */
>  	ee = mhi_get_exec_env(mhi_cntrl);
> +	if (ee == MHI_EE_MAX)
> +		goto error_exit_rddm;
> +
>  	if (ee != MHI_EE_RDDM) {
>  		dev_dbg(dev, "Trigger device into RDDM mode using SYS ERR\n");
>  		mhi_set_mhi_state(mhi_cntrl, MHI_STATE_SYS_ERR);
> @@ -139,10 +142,12 @@ static int __mhi_download_rddm_in_panic(struct mhi_controller *mhi_cntrl)
>  	ee = mhi_get_exec_env(mhi_cntrl);
>  	ret = mhi_read_reg(mhi_cntrl, base, BHIE_RXVECSTATUS_OFFS, &rx_status);
>  
> -	dev_err(dev, "Did not complete RDDM transfer\n");
> -	dev_err(dev, "Current EE: %s\n", TO_MHI_EXEC_STR(ee));
>  	dev_err(dev, "RXVEC_STATUS: 0x%x\n", rx_status);
>  
> +error_exit_rddm:
> +	dev_err(dev, "RDDM transfer failed. Current EE: %s\n",
> +		TO_MHI_EXEC_STR(ee));
> +
>  	return -EIO;
>  }
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH v2 0/6] Minor bug fixes and clean-up for MHI host driver

[Manivannan Sadhasivam]

On Fri, Nov 06, 2020 at 09:44:44AM -0800, Bhaumik Bhatt wrote:
> This patch series serves to clean up the MHI host driver by removing an
> unnecessary counter and an unused function. It also renames a function to make
> it clearly worded. There is currently no user of this exported function which
> makes it is safe to do so now.
> 
> Bug fixes include adding a missing EXPORT_SYMBOL_GPL to a function, and adding
> a return value check to bail out of RDDM download in kernel panic path.
> 
> An outlier among the group exports the mhi_get_exec_env() API for use by
> controller drivers, in case they need to determine behavior on the basis of the
> current execution environment.
> 
> This set of patches was tested on arm64.

Series applied to mhi-next!

Thanks,
Mani

> 
> v2:
> -Removed the declaration for mhi_get_exec_env() from internal.h
> -Improved on the error log message in RDDM download exit case due to unknown EE
> 
> Bhaumik Bhatt (6):
>   bus: mhi: core: Remove unnecessary counter from mhi_firmware_copy()
>   bus: mhi: core: Add missing EXPORT_SYMBOL for mhi_get_mhi_state()
>   bus: mhi: core: Expose mhi_get_exec_env() API for controllers
>   bus: mhi: core: Remove unused mhi_fw_load_worker() declaration
>   bus: mhi: core: Rename RDDM download function to use proper words
>   bus: mhi: core: Skip RDDM download for unknown execution environment
> 
>  drivers/bus/mhi/core/boot.c     | 15 +++++++++------
>  drivers/bus/mhi/core/internal.h |  2 --
>  drivers/bus/mhi/core/main.c     |  2 ++
>  include/linux/mhi.h             | 12 +++++++++---
>  4 files changed, 20 insertions(+), 11 deletions(-)
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>