| Message ID | 1617313309-24035-1-git-send-email-bbhatt@codeaurora.org |
|---|---|
| State | Accepted |
| Commit | eee87072e2fb9000b12c5e752ebd4a05882da2e4 |
| Headers | show |
| Series | [RESEND] bus: mhi: core: Remove pre_init flag used for power purposes | expand |
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
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 >
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 >
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.
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
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; };
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(-)