All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] bus: mhi: core: Fix power down latency
@ 2021-05-03  8:04 Loic Poulain
  2021-05-03  8:06 ` Loic Poulain
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Loic Poulain @ 2021-05-03  8:04 UTC (permalink / raw)
  To: mani, hemantk; +Cc: linux-arm-msm, bbhatt, Loic Poulain

On graceful power-down/disable transition, when an MHI reset is
performed, the MHI device loses its context, including interrupt
configuration. However, the current implementation is waiting for
event(irq) driven state change to confirm reset has been completed,
which never happens, and causes reset timeout, leading to unexpected
high latency of the mhi_power_down procedure (up to 45 seconds).

Fix that by moving to the recently introduced poll_reg_field method,
waiting for the reset bit to be cleared, in the same way as the
power_on procedure.

Fixes: a6e2e3522f29 ("bus: mhi: core: Add support for PM state transitions")
Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/bus/mhi/core/pm.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index e2e59a3..704a5e2 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -465,23 +465,15 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl)
 
 	/* Trigger MHI RESET so that the device will not access host memory */
 	if (!MHI_PM_IN_FATAL_STATE(mhi_cntrl->pm_state)) {
-		u32 in_reset = -1;
-		unsigned long timeout = msecs_to_jiffies(mhi_cntrl->timeout_ms);
-
 		dev_dbg(dev, "Triggering MHI Reset in device\n");
 		mhi_set_mhi_state(mhi_cntrl, MHI_STATE_RESET);
 
 		/* Wait for the reset bit to be cleared by the device */
-		ret = wait_event_timeout(mhi_cntrl->state_event,
-					 mhi_read_reg_field(mhi_cntrl,
-							    mhi_cntrl->regs,
-							    MHICTRL,
-							    MHICTRL_RESET_MASK,
-							    MHICTRL_RESET_SHIFT,
-							    &in_reset) ||
-					!in_reset, timeout);
-		if (!ret || in_reset)
-			dev_err(dev, "Device failed to exit MHI Reset state\n");
+		ret = mhi_poll_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL,
+				 MHICTRL_RESET_MASK, MHICTRL_RESET_SHIFT, 0,
+				 25000);
+		if (ret)
+			dev_err(dev, "Device failed to clear MHI Reset\n");
 
 		/*
 		 * Device will clear BHI_INTVEC as a part of RESET processing,
-- 
2.7.4


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

* Re: [PATCH v2] bus: mhi: core: Fix power down latency
  2021-05-03  8:04 [PATCH v2] bus: mhi: core: Fix power down latency Loic Poulain
@ 2021-05-03  8:06 ` Loic Poulain
  2021-05-03 17:01 ` Bhaumik Bhatt
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Loic Poulain @ 2021-05-03  8:06 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Hemant Kumar; +Cc: linux-arm-msm, Bhaumik Bhatt

Hi Manivannan and Hemant,

On Mon, 3 May 2021 at 09:55, Loic Poulain <loic.poulain@linaro.org> wrote:
>
> On graceful power-down/disable transition, when an MHI reset is
> performed, the MHI device loses its context, including interrupt
> configuration. However, the current implementation is waiting for
> event(irq) driven state change to confirm reset has been completed,
> which never happens, and causes reset timeout, leading to unexpected
> high latency of the mhi_power_down procedure (up to 45 seconds).
>
> Fix that by moving to the recently introduced poll_reg_field method,
> waiting for the reset bit to be cleared, in the same way as the
> power_on procedure.
>
> Fixes: a6e2e3522f29 ("bus: mhi: core: Add support for PM state transitions")
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

Should be considered for 5.13-rc* since it's a fix.

Regards,
Loic

> ---
>  drivers/bus/mhi/core/pm.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index e2e59a3..704a5e2 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -465,23 +465,15 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl)
>
>         /* Trigger MHI RESET so that the device will not access host memory */
>         if (!MHI_PM_IN_FATAL_STATE(mhi_cntrl->pm_state)) {
> -               u32 in_reset = -1;
> -               unsigned long timeout = msecs_to_jiffies(mhi_cntrl->timeout_ms);
> -
>                 dev_dbg(dev, "Triggering MHI Reset in device\n");
>                 mhi_set_mhi_state(mhi_cntrl, MHI_STATE_RESET);
>
>                 /* Wait for the reset bit to be cleared by the device */
> -               ret = wait_event_timeout(mhi_cntrl->state_event,
> -                                        mhi_read_reg_field(mhi_cntrl,
> -                                                           mhi_cntrl->regs,
> -                                                           MHICTRL,
> -                                                           MHICTRL_RESET_MASK,
> -                                                           MHICTRL_RESET_SHIFT,
> -                                                           &in_reset) ||
> -                                       !in_reset, timeout);
> -               if (!ret || in_reset)
> -                       dev_err(dev, "Device failed to exit MHI Reset state\n");
> +               ret = mhi_poll_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL,
> +                                MHICTRL_RESET_MASK, MHICTRL_RESET_SHIFT, 0,
> +                                25000);
> +               if (ret)
> +                       dev_err(dev, "Device failed to clear MHI Reset\n");
>
>                 /*
>                  * Device will clear BHI_INTVEC as a part of RESET processing,
> --
> 2.7.4
>

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

* Re: [PATCH v2] bus: mhi: core: Fix power down latency
  2021-05-03  8:04 [PATCH v2] bus: mhi: core: Fix power down latency Loic Poulain
  2021-05-03  8:06 ` Loic Poulain
@ 2021-05-03 17:01 ` Bhaumik Bhatt
  2021-05-03 23:44 ` Hemant Kumar
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Bhaumik Bhatt @ 2021-05-03 17:01 UTC (permalink / raw)
  To: Loic Poulain; +Cc: mani, hemantk, linux-arm-msm

On 2021-05-03 01:04 AM, Loic Poulain wrote:
> On graceful power-down/disable transition, when an MHI reset is
> performed, the MHI device loses its context, including interrupt
> configuration. However, the current implementation is waiting for
> event(irq) driven state change to confirm reset has been completed,
> which never happens, and causes reset timeout, leading to unexpected
> high latency of the mhi_power_down procedure (up to 45 seconds).
> 
> Fix that by moving to the recently introduced poll_reg_field method,
> waiting for the reset bit to be cleared, in the same way as the
> power_on procedure.
> 
> Fixes: a6e2e3522f29 ("bus: mhi: core: Add support for PM state 
> transitions")
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
Reviewed-by: Bhaumik Bhatt <bbhatt@codeaurora.org>

>  drivers/bus/mhi/core/pm.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index e2e59a3..704a5e2 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -465,23 +465,15 @@ static void mhi_pm_disable_transition(struct
> mhi_controller *mhi_cntrl)
> 
>  	/* Trigger MHI RESET so that the device will not access host memory 
> */
>  	if (!MHI_PM_IN_FATAL_STATE(mhi_cntrl->pm_state)) {
> -		u32 in_reset = -1;
> -		unsigned long timeout = msecs_to_jiffies(mhi_cntrl->timeout_ms);
> -
>  		dev_dbg(dev, "Triggering MHI Reset in device\n");
>  		mhi_set_mhi_state(mhi_cntrl, MHI_STATE_RESET);
> 
>  		/* Wait for the reset bit to be cleared by the device */
> -		ret = wait_event_timeout(mhi_cntrl->state_event,
> -					 mhi_read_reg_field(mhi_cntrl,
> -							    mhi_cntrl->regs,
> -							    MHICTRL,
> -							    MHICTRL_RESET_MASK,
> -							    MHICTRL_RESET_SHIFT,
> -							    &in_reset) ||
> -					!in_reset, timeout);
> -		if (!ret || in_reset)
> -			dev_err(dev, "Device failed to exit MHI Reset state\n");
> +		ret = mhi_poll_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL,
> +				 MHICTRL_RESET_MASK, MHICTRL_RESET_SHIFT, 0,
> +				 25000);
> +		if (ret)
> +			dev_err(dev, "Device failed to clear MHI Reset\n");
> 
>  		/*
>  		 * Device will clear BHI_INTVEC as a part of RESET processing,

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

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

* Re: [PATCH v2] bus: mhi: core: Fix power down latency
  2021-05-03  8:04 [PATCH v2] bus: mhi: core: Fix power down latency Loic Poulain
  2021-05-03  8:06 ` Loic Poulain
  2021-05-03 17:01 ` Bhaumik Bhatt
@ 2021-05-03 23:44 ` Hemant Kumar
  2021-05-21 12:29 ` Manivannan Sadhasivam
  2021-05-21 17:39 ` Manivannan Sadhasivam
  4 siblings, 0 replies; 6+ messages in thread
From: Hemant Kumar @ 2021-05-03 23:44 UTC (permalink / raw)
  To: Loic Poulain, mani; +Cc: linux-arm-msm, bbhatt



On 5/3/21 1:04 AM, Loic Poulain wrote:
> On graceful power-down/disable transition, when an MHI reset is
> performed, the MHI device loses its context, including interrupt
> configuration. However, the current implementation is waiting for
> event(irq) driven state change to confirm reset has been completed,
> which never happens, and causes reset timeout, leading to unexpected
> high latency of the mhi_power_down procedure (up to 45 seconds).
> 
> Fix that by moving to the recently introduced poll_reg_field method,
> waiting for the reset bit to be cleared, in the same way as the
> power_on procedure.
> 
> Fixes: a6e2e3522f29 ("bus: mhi: core: Add support for PM state transitions")
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

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

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

* Re: [PATCH v2] bus: mhi: core: Fix power down latency
  2021-05-03  8:04 [PATCH v2] bus: mhi: core: Fix power down latency Loic Poulain
                   ` (2 preceding siblings ...)
  2021-05-03 23:44 ` Hemant Kumar
@ 2021-05-21 12:29 ` Manivannan Sadhasivam
  2021-05-21 17:39 ` Manivannan Sadhasivam
  4 siblings, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 12:29 UTC (permalink / raw)
  To: Loic Poulain; +Cc: hemantk, linux-arm-msm, bbhatt

On Mon, May 03, 2021 at 10:04:50AM +0200, Loic Poulain wrote:
> On graceful power-down/disable transition, when an MHI reset is
> performed, the MHI device loses its context, including interrupt
> configuration. However, the current implementation is waiting for
> event(irq) driven state change to confirm reset has been completed,
> which never happens, and causes reset timeout, leading to unexpected
> high latency of the mhi_power_down procedure (up to 45 seconds).
> 
> Fix that by moving to the recently introduced poll_reg_field method,
> waiting for the reset bit to be cleared, in the same way as the
> power_on procedure.
> 
> Fixes: a6e2e3522f29 ("bus: mhi: core: Add support for PM state transitions")
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

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

Thanks,
Mani

> ---
>  drivers/bus/mhi/core/pm.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index e2e59a3..704a5e2 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -465,23 +465,15 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl)
>  
>  	/* Trigger MHI RESET so that the device will not access host memory */
>  	if (!MHI_PM_IN_FATAL_STATE(mhi_cntrl->pm_state)) {
> -		u32 in_reset = -1;
> -		unsigned long timeout = msecs_to_jiffies(mhi_cntrl->timeout_ms);
> -
>  		dev_dbg(dev, "Triggering MHI Reset in device\n");
>  		mhi_set_mhi_state(mhi_cntrl, MHI_STATE_RESET);
>  
>  		/* Wait for the reset bit to be cleared by the device */
> -		ret = wait_event_timeout(mhi_cntrl->state_event,
> -					 mhi_read_reg_field(mhi_cntrl,
> -							    mhi_cntrl->regs,
> -							    MHICTRL,
> -							    MHICTRL_RESET_MASK,
> -							    MHICTRL_RESET_SHIFT,
> -							    &in_reset) ||
> -					!in_reset, timeout);
> -		if (!ret || in_reset)
> -			dev_err(dev, "Device failed to exit MHI Reset state\n");
> +		ret = mhi_poll_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL,
> +				 MHICTRL_RESET_MASK, MHICTRL_RESET_SHIFT, 0,
> +				 25000);
> +		if (ret)
> +			dev_err(dev, "Device failed to clear MHI Reset\n");
>  
>  		/*
>  		 * Device will clear BHI_INTVEC as a part of RESET processing,
> -- 
> 2.7.4
> 

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

* Re: [PATCH v2] bus: mhi: core: Fix power down latency
  2021-05-03  8:04 [PATCH v2] bus: mhi: core: Fix power down latency Loic Poulain
                   ` (3 preceding siblings ...)
  2021-05-21 12:29 ` Manivannan Sadhasivam
@ 2021-05-21 17:39 ` Manivannan Sadhasivam
  4 siblings, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 17:39 UTC (permalink / raw)
  To: Loic Poulain; +Cc: hemantk, linux-arm-msm, bbhatt

On Mon, May 03, 2021 at 10:04:50AM +0200, Loic Poulain wrote:
> On graceful power-down/disable transition, when an MHI reset is
> performed, the MHI device loses its context, including interrupt
> configuration. However, the current implementation is waiting for
> event(irq) driven state change to confirm reset has been completed,
> which never happens, and causes reset timeout, leading to unexpected
> high latency of the mhi_power_down procedure (up to 45 seconds).
> 
> Fix that by moving to the recently introduced poll_reg_field method,
> waiting for the reset bit to be cleared, in the same way as the
> power_on procedure.
> 
> Fixes: a6e2e3522f29 ("bus: mhi: core: Add support for PM state transitions")
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

Applied to mhi-fixes!

Thanks,
Mani

> ---
>  drivers/bus/mhi/core/pm.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index e2e59a3..704a5e2 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -465,23 +465,15 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl)
>  
>  	/* Trigger MHI RESET so that the device will not access host memory */
>  	if (!MHI_PM_IN_FATAL_STATE(mhi_cntrl->pm_state)) {
> -		u32 in_reset = -1;
> -		unsigned long timeout = msecs_to_jiffies(mhi_cntrl->timeout_ms);
> -
>  		dev_dbg(dev, "Triggering MHI Reset in device\n");
>  		mhi_set_mhi_state(mhi_cntrl, MHI_STATE_RESET);
>  
>  		/* Wait for the reset bit to be cleared by the device */
> -		ret = wait_event_timeout(mhi_cntrl->state_event,
> -					 mhi_read_reg_field(mhi_cntrl,
> -							    mhi_cntrl->regs,
> -							    MHICTRL,
> -							    MHICTRL_RESET_MASK,
> -							    MHICTRL_RESET_SHIFT,
> -							    &in_reset) ||
> -					!in_reset, timeout);
> -		if (!ret || in_reset)
> -			dev_err(dev, "Device failed to exit MHI Reset state\n");
> +		ret = mhi_poll_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL,
> +				 MHICTRL_RESET_MASK, MHICTRL_RESET_SHIFT, 0,
> +				 25000);
> +		if (ret)
> +			dev_err(dev, "Device failed to clear MHI Reset\n");
>  
>  		/*
>  		 * Device will clear BHI_INTVEC as a part of RESET processing,
> -- 
> 2.7.4
> 

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

end of thread, other threads:[~2021-05-21 17:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-03  8:04 [PATCH v2] bus: mhi: core: Fix power down latency Loic Poulain
2021-05-03  8:06 ` Loic Poulain
2021-05-03 17:01 ` Bhaumik Bhatt
2021-05-03 23:44 ` Hemant Kumar
2021-05-21 12:29 ` Manivannan Sadhasivam
2021-05-21 17:39 ` Manivannan Sadhasivam

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.