[RESEND PATCH] bus: mhi: core: Remove pre_init flag used for power purposes

[RESEND PATCH] bus: mhi: core: Remove pre_init flag used for power purposes

[Bhaumik Bhatt]

Some controllers can choose to skip preparation for power up.
In that case, device context is initialized based on the pre_init
flag not being set during mhi_prepare_for_power_up(). There is no
reason MHI host driver should maintain and provide controllers
with two separate paths for preparing MHI.

Going forward, all controllers will be required to call the
mhi_prepare_for_power_up() API followed by their choice of sync
or async power up. This allows MHI host driver to get rid of the
pre_init flag and sets up a common way for all controllers to use
MHI. This also helps controllers fail early on during preparation
phase in some failure cases.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
This patch was tested on arm64 architecture.

 drivers/bus/mhi/core/init.c |  3 ---
 drivers/bus/mhi/core/pm.c   | 20 --------------------
 include/linux/mhi.h         |  2 --
 3 files changed, 25 deletions(-)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index d1d9b0d..1f61352 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -1080,8 +1080,6 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
 			mhi_rddm_prepare(mhi_cntrl, mhi_cntrl->rddm_image);
 	}
 
-	mhi_cntrl->pre_init = true;
-
 	mutex_unlock(&mhi_cntrl->pm_mutex);
 
 	return 0;
@@ -1112,7 +1110,6 @@ void mhi_unprepare_after_power_down(struct mhi_controller *mhi_cntrl)
 	}
 
 	mhi_deinit_dev_ctxt(mhi_cntrl);
-	mhi_cntrl->pre_init = false;
 }
 EXPORT_SYMBOL_GPL(mhi_unprepare_after_power_down);
 
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index e4aff77..b23eec5 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -1062,13 +1062,6 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
 	mutex_lock(&mhi_cntrl->pm_mutex);
 	mhi_cntrl->pm_state = MHI_PM_DISABLE;
 
-	if (!mhi_cntrl->pre_init) {
-		/* Setup device context */
-		ret = mhi_init_dev_ctxt(mhi_cntrl);
-		if (ret)
-			goto error_dev_ctxt;
-	}
-
 	ret = mhi_init_irq_setup(mhi_cntrl);
 	if (ret)
 		goto error_setup_irq;
@@ -1150,10 +1143,6 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
 	mhi_deinit_free_irq(mhi_cntrl);
 
 error_setup_irq:
-	if (!mhi_cntrl->pre_init)
-		mhi_deinit_dev_ctxt(mhi_cntrl);
-
-error_dev_ctxt:
 	mhi_cntrl->pm_state = MHI_PM_DISABLE;
 	mutex_unlock(&mhi_cntrl->pm_mutex);
 
@@ -1203,15 +1192,6 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful)
 	flush_work(&mhi_cntrl->st_worker);
 
 	free_irq(mhi_cntrl->irq[0], mhi_cntrl);
-
-	if (!mhi_cntrl->pre_init) {
-		/* Free all allocated resources */
-		if (mhi_cntrl->fbc_image) {
-			mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
-			mhi_cntrl->fbc_image = NULL;
-		}
-		mhi_deinit_dev_ctxt(mhi_cntrl);
-	}
 }
 EXPORT_SYMBOL_GPL(mhi_power_down);
 
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index b16afd3..c9b36a3 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -354,7 +354,6 @@ struct mhi_controller_config {
  * @index: Index of the MHI controller instance
  * @bounce_buf: Use of bounce buffer
  * @fbc_download: MHI host needs to do complete image transfer (optional)
- * @pre_init: MHI host needs to do pre-initialization before power up
  * @wake_set: Device wakeup set flag
  * @irq_flags: irq flags passed to request_irq (optional)
  *
@@ -447,7 +446,6 @@ struct mhi_controller {
 	int index;
 	bool bounce_buf;
 	bool fbc_download;
-	bool pre_init;
 	bool wake_set;
 	unsigned long irq_flags;
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RESEND PATCH] bus: mhi: core: Remove pre_init flag used for power purposes

[Hemant Kumar]

On 4/1/21 2:41 PM, Bhaumik Bhatt wrote:
> Some controllers can choose to skip preparation for power up.
> In that case, device context is initialized based on the pre_init
> flag not being set during mhi_prepare_for_power_up(). There is no
> reason MHI host driver should maintain and provide controllers
> with two separate paths for preparing MHI.
> 
> Going forward, all controllers will be required to call the
> mhi_prepare_for_power_up() API followed by their choice of sync
> or async power up. This allows MHI host driver to get rid of the
> pre_init flag and sets up a common way for all controllers to use
> MHI. This also helps controllers fail early on during preparation
> phase in some failure cases.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> ---
> This patch was tested on arm64 architecture.

Change looks good as current MHI controllers ath11k and pci generic are 
not using pre_init.

Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>

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

Re: [RESEND PATCH] bus: mhi: core: Remove pre_init flag used for power purposes

[Manivannan Sadhasivam]

On Thu, Apr 01, 2021 at 02:41:49PM -0700, Bhaumik Bhatt wrote:
> Some controllers can choose to skip preparation for power up.
> In that case, device context is initialized based on the pre_init
> flag not being set during mhi_prepare_for_power_up(). There is no
> reason MHI host driver should maintain and provide controllers
> with two separate paths for preparing MHI.
> 
> Going forward, all controllers will be required to call the
> mhi_prepare_for_power_up() API followed by their choice of sync
> or async power up. This allows MHI host driver to get rid of the
> pre_init flag and sets up a common way for all controllers to use
> MHI. This also helps controllers fail early on during preparation
> phase in some failure cases.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>

I hope Jeff is also okay with this patch for AIC100. 

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

Thanks,
Mani

> ---
> This patch was tested on arm64 architecture.
> 
>  drivers/bus/mhi/core/init.c |  3 ---
>  drivers/bus/mhi/core/pm.c   | 20 --------------------
>  include/linux/mhi.h         |  2 --
>  3 files changed, 25 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index d1d9b0d..1f61352 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -1080,8 +1080,6 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
>  			mhi_rddm_prepare(mhi_cntrl, mhi_cntrl->rddm_image);
>  	}
>  
> -	mhi_cntrl->pre_init = true;
> -
>  	mutex_unlock(&mhi_cntrl->pm_mutex);
>  
>  	return 0;
> @@ -1112,7 +1110,6 @@ void mhi_unprepare_after_power_down(struct mhi_controller *mhi_cntrl)
>  	}
>  
>  	mhi_deinit_dev_ctxt(mhi_cntrl);
> -	mhi_cntrl->pre_init = false;
>  }
>  EXPORT_SYMBOL_GPL(mhi_unprepare_after_power_down);
>  
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index e4aff77..b23eec5 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -1062,13 +1062,6 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
>  	mutex_lock(&mhi_cntrl->pm_mutex);
>  	mhi_cntrl->pm_state = MHI_PM_DISABLE;
>  
> -	if (!mhi_cntrl->pre_init) {
> -		/* Setup device context */
> -		ret = mhi_init_dev_ctxt(mhi_cntrl);
> -		if (ret)
> -			goto error_dev_ctxt;
> -	}
> -
>  	ret = mhi_init_irq_setup(mhi_cntrl);
>  	if (ret)
>  		goto error_setup_irq;
> @@ -1150,10 +1143,6 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
>  	mhi_deinit_free_irq(mhi_cntrl);
>  
>  error_setup_irq:
> -	if (!mhi_cntrl->pre_init)
> -		mhi_deinit_dev_ctxt(mhi_cntrl);
> -
> -error_dev_ctxt:
>  	mhi_cntrl->pm_state = MHI_PM_DISABLE;
>  	mutex_unlock(&mhi_cntrl->pm_mutex);
>  
> @@ -1203,15 +1192,6 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful)
>  	flush_work(&mhi_cntrl->st_worker);
>  
>  	free_irq(mhi_cntrl->irq[0], mhi_cntrl);
> -
> -	if (!mhi_cntrl->pre_init) {
> -		/* Free all allocated resources */
> -		if (mhi_cntrl->fbc_image) {
> -			mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
> -			mhi_cntrl->fbc_image = NULL;
> -		}
> -		mhi_deinit_dev_ctxt(mhi_cntrl);
> -	}
>  }
>  EXPORT_SYMBOL_GPL(mhi_power_down);
>  
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index b16afd3..c9b36a3 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -354,7 +354,6 @@ struct mhi_controller_config {
>   * @index: Index of the MHI controller instance
>   * @bounce_buf: Use of bounce buffer
>   * @fbc_download: MHI host needs to do complete image transfer (optional)
> - * @pre_init: MHI host needs to do pre-initialization before power up
>   * @wake_set: Device wakeup set flag
>   * @irq_flags: irq flags passed to request_irq (optional)
>   *
> @@ -447,7 +446,6 @@ struct mhi_controller {
>  	int index;
>  	bool bounce_buf;
>  	bool fbc_download;
> -	bool pre_init;
>  	bool wake_set;
>  	unsigned long irq_flags;
>  };
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [RESEND PATCH] bus: mhi: core: Remove pre_init flag used for power purposes

[Manivannan Sadhasivam]

On Thu, Apr 01, 2021 at 02:41:49PM -0700, Bhaumik Bhatt wrote:
> Some controllers can choose to skip preparation for power up.
> In that case, device context is initialized based on the pre_init
> flag not being set during mhi_prepare_for_power_up(). There is no
> reason MHI host driver should maintain and provide controllers
> with two separate paths for preparing MHI.
> 
> Going forward, all controllers will be required to call the
> mhi_prepare_for_power_up() API followed by their choice of sync
> or async power up. This allows MHI host driver to get rid of the
> pre_init flag and sets up a common way for all controllers to use
> MHI. This also helps controllers fail early on during preparation
> phase in some failure cases.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>

Applied to mhi-next!

Thanks,
Mani

> ---
> This patch was tested on arm64 architecture.
> 
>  drivers/bus/mhi/core/init.c |  3 ---
>  drivers/bus/mhi/core/pm.c   | 20 --------------------
>  include/linux/mhi.h         |  2 --
>  3 files changed, 25 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index d1d9b0d..1f61352 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -1080,8 +1080,6 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
>  			mhi_rddm_prepare(mhi_cntrl, mhi_cntrl->rddm_image);
>  	}
>  
> -	mhi_cntrl->pre_init = true;
> -
>  	mutex_unlock(&mhi_cntrl->pm_mutex);
>  
>  	return 0;
> @@ -1112,7 +1110,6 @@ void mhi_unprepare_after_power_down(struct mhi_controller *mhi_cntrl)
>  	}
>  
>  	mhi_deinit_dev_ctxt(mhi_cntrl);
> -	mhi_cntrl->pre_init = false;
>  }
>  EXPORT_SYMBOL_GPL(mhi_unprepare_after_power_down);
>  
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index e4aff77..b23eec5 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -1062,13 +1062,6 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
>  	mutex_lock(&mhi_cntrl->pm_mutex);
>  	mhi_cntrl->pm_state = MHI_PM_DISABLE;
>  
> -	if (!mhi_cntrl->pre_init) {
> -		/* Setup device context */
> -		ret = mhi_init_dev_ctxt(mhi_cntrl);
> -		if (ret)
> -			goto error_dev_ctxt;
> -	}
> -
>  	ret = mhi_init_irq_setup(mhi_cntrl);
>  	if (ret)
>  		goto error_setup_irq;
> @@ -1150,10 +1143,6 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
>  	mhi_deinit_free_irq(mhi_cntrl);
>  
>  error_setup_irq:
> -	if (!mhi_cntrl->pre_init)
> -		mhi_deinit_dev_ctxt(mhi_cntrl);
> -
> -error_dev_ctxt:
>  	mhi_cntrl->pm_state = MHI_PM_DISABLE;
>  	mutex_unlock(&mhi_cntrl->pm_mutex);
>  
> @@ -1203,15 +1192,6 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful)
>  	flush_work(&mhi_cntrl->st_worker);
>  
>  	free_irq(mhi_cntrl->irq[0], mhi_cntrl);
> -
> -	if (!mhi_cntrl->pre_init) {
> -		/* Free all allocated resources */
> -		if (mhi_cntrl->fbc_image) {
> -			mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
> -			mhi_cntrl->fbc_image = NULL;
> -		}
> -		mhi_deinit_dev_ctxt(mhi_cntrl);
> -	}
>  }
>  EXPORT_SYMBOL_GPL(mhi_power_down);
>  
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index b16afd3..c9b36a3 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -354,7 +354,6 @@ struct mhi_controller_config {
>   * @index: Index of the MHI controller instance
>   * @bounce_buf: Use of bounce buffer
>   * @fbc_download: MHI host needs to do complete image transfer (optional)
> - * @pre_init: MHI host needs to do pre-initialization before power up
>   * @wake_set: Device wakeup set flag
>   * @irq_flags: irq flags passed to request_irq (optional)
>   *
> @@ -447,7 +446,6 @@ struct mhi_controller {
>  	int index;
>  	bool bounce_buf;
>  	bool fbc_download;
> -	bool pre_init;
>  	bool wake_set;
>  	unsigned long irq_flags;
>  };
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [RESEND PATCH] bus: mhi: core: Remove pre_init flag used for power purposes

[Jeffrey Hugo]

On 4/6/2021 11:07 PM, Manivannan Sadhasivam wrote:
> On Thu, Apr 01, 2021 at 02:41:49PM -0700, Bhaumik Bhatt wrote:
>> Some controllers can choose to skip preparation for power up.
>> In that case, device context is initialized based on the pre_init
>> flag not being set during mhi_prepare_for_power_up(). There is no
>> reason MHI host driver should maintain and provide controllers
>> with two separate paths for preparing MHI.
>>
>> Going forward, all controllers will be required to call the
>> mhi_prepare_for_power_up() API followed by their choice of sync
>> or async power up. This allows MHI host driver to get rid of the
>> pre_init flag and sets up a common way for all controllers to use
>> MHI. This also helps controllers fail early on during preparation
>> phase in some failure cases.
>>
>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> 
> I hope Jeff is also okay with this patch for AIC100.
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 

Sorry for the non-response.  This has minimal impact.  No concerns from me.

Re: [RESEND PATCH] bus: mhi: core: Remove pre_init flag used for power purposes

[patchwork-bot+linux-arm-msm]

Hello:

This patch was applied to qcom/linux.git (refs/heads/for-next):

On Thu,  1 Apr 2021 14:41:49 -0700 you wrote:
> Some controllers can choose to skip preparation for power up.
> In that case, device context is initialized based on the pre_init
> flag not being set during mhi_prepare_for_power_up(). There is no
> reason MHI host driver should maintain and provide controllers
> with two separate paths for preparing MHI.
> 
> Going forward, all controllers will be required to call the
> mhi_prepare_for_power_up() API followed by their choice of sync
> or async power up. This allows MHI host driver to get rid of the
> pre_init flag and sets up a common way for all controllers to use
> MHI. This also helps controllers fail early on during preparation
> phase in some failure cases.
> 
> [...]

Here is the summary with links:
  - [RESEND] bus: mhi: core: Remove pre_init flag used for power purposes
    https://git.kernel.org/qcom/c/eee87072e2fb

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html