diff mbox series

[3/6] drivers: firmware: psci: Decouple checker from generic ARM CPUidle

Message ID 20190722153745.32446-4-lorenzo.pieralisi@arm.com (mailing list archive)
State Superseded, archived
Headers show
Series ARM: psci: cpuidle: PSCI CPUidle rework | expand

Commit Message

Lorenzo Pieralisi July 22, 2019, 3:37 p.m. UTC
The PSCI checker currently relies on the generic ARM CPUidle
infrastructure to enter an idle state, which in turn creates
a dependency that is not really needed.

The PSCI checker code to test PSCI CPU suspend is built on
top of the CPUidle framework and can easily reuse the
struct cpuidle_state.enter() function (previously initialized
by an idle driver, with a PSCI back-end) to trigger an entry
into an idle state, decoupling the PSCI checker from the
generic ARM CPUidle infrastructure and simplyfing the code
in the process.

Convert the PSCI checker suspend entry function to use
the struct cpuidle_state.enter() function callback.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 drivers/firmware/psci/psci_checker.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Comments

Sudeep Holla Aug. 6, 2019, 3:54 p.m. UTC | #1
On Mon, Jul 22, 2019 at 04:37:42PM +0100, Lorenzo Pieralisi wrote:
> The PSCI checker currently relies on the generic ARM CPUidle
> infrastructure to enter an idle state, which in turn creates
> a dependency that is not really needed.
> 
> The PSCI checker code to test PSCI CPU suspend is built on
> top of the CPUidle framework and can easily reuse the
> struct cpuidle_state.enter() function (previously initialized
> by an idle driver, with a PSCI back-end) to trigger an entry
> into an idle state, decoupling the PSCI checker from the
> generic ARM CPUidle infrastructure and simplyfing the code
> in the process.
> 
> Convert the PSCI checker suspend entry function to use
> the struct cpuidle_state.enter() function callback.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

Not sure why we didn't take this path from the beginning.
Anyways make sense and much needed for later patches in the series.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep
Daniel Lezcano Aug. 7, 2019, 2:09 p.m. UTC | #2
On 22/07/2019 17:37, Lorenzo Pieralisi wrote:
> The PSCI checker currently relies on the generic ARM CPUidle
> infrastructure to enter an idle state, which in turn creates
> a dependency that is not really needed.
> 
> The PSCI checker code to test PSCI CPU suspend is built on
> top of the CPUidle framework and can easily reuse the
> struct cpuidle_state.enter() function (previously initialized
> by an idle driver, with a PSCI back-end) to trigger an entry
> into an idle state, decoupling the PSCI checker from the
> generic ARM CPUidle infrastructure and simplyfing the code
> in the process.
> 
> Convert the PSCI checker suspend entry function to use
> the struct cpuidle_state.enter() function callback.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>  drivers/firmware/psci/psci_checker.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/firmware/psci/psci_checker.c b/drivers/firmware/psci/psci_checker.c
> index f3659443f8c2..6a445397771c 100644
> --- a/drivers/firmware/psci/psci_checker.c
> +++ b/drivers/firmware/psci/psci_checker.c
> @@ -228,8 +228,11 @@ static int hotplug_tests(void)
>  
>  static void dummy_callback(struct timer_list *unused) {}
>  
> -static int suspend_cpu(int index, bool broadcast)
> +static int suspend_cpu(struct cpuidle_device *dev,
> +		       struct cpuidle_driver *drv, int index)
>  {
> +	struct cpuidle_state *state = &drv->states[index];
> +	bool broadcast = state->flags & CPUIDLE_FLAG_TIMER_STOP;
>  	int ret;
>  
>  	arch_cpu_idle_enter();
> @@ -254,11 +257,7 @@ static int suspend_cpu(int index, bool broadcast)
>  		}
>  	}
>  
> -	/*
> -	 * Replicate the common ARM cpuidle enter function
> -	 * (arm_enter_idle_state).
> -	 */
> -	ret = CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, index);
> +	ret = state->enter(dev, drv, index);
>  
>  	if (broadcast)
>  		tick_broadcast_exit();
> @@ -301,9 +300,8 @@ static int suspend_test_thread(void *arg)
>  		 * doesn't use PSCI).
>  		 */
>  		for (index = 1; index < drv->state_count; ++index) {
> -			struct cpuidle_state *state = &drv->states[index];
> -			bool broadcast = state->flags & CPUIDLE_FLAG_TIMER_STOP;
>  			int ret;
> +			struct cpuidle_state *state = &drv->states[index];
>  
>  			/*
>  			 * Set the timer to wake this CPU up in some time (which
> @@ -318,7 +316,7 @@ static int suspend_test_thread(void *arg)
>  			/* IRQs must be disabled during suspend operations. */
>  			local_irq_disable();
>  
> -			ret = suspend_cpu(index, broadcast);
> +			ret = suspend_cpu(dev, drv, index);
>  
>  			/*
>  			 * We have woken up. Re-enable IRQs to handle any
>
diff mbox series

Patch

diff --git a/drivers/firmware/psci/psci_checker.c b/drivers/firmware/psci/psci_checker.c
index f3659443f8c2..6a445397771c 100644
--- a/drivers/firmware/psci/psci_checker.c
+++ b/drivers/firmware/psci/psci_checker.c
@@ -228,8 +228,11 @@  static int hotplug_tests(void)
 
 static void dummy_callback(struct timer_list *unused) {}
 
-static int suspend_cpu(int index, bool broadcast)
+static int suspend_cpu(struct cpuidle_device *dev,
+		       struct cpuidle_driver *drv, int index)
 {
+	struct cpuidle_state *state = &drv->states[index];
+	bool broadcast = state->flags & CPUIDLE_FLAG_TIMER_STOP;
 	int ret;
 
 	arch_cpu_idle_enter();
@@ -254,11 +257,7 @@  static int suspend_cpu(int index, bool broadcast)
 		}
 	}
 
-	/*
-	 * Replicate the common ARM cpuidle enter function
-	 * (arm_enter_idle_state).
-	 */
-	ret = CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, index);
+	ret = state->enter(dev, drv, index);
 
 	if (broadcast)
 		tick_broadcast_exit();
@@ -301,9 +300,8 @@  static int suspend_test_thread(void *arg)
 		 * doesn't use PSCI).
 		 */
 		for (index = 1; index < drv->state_count; ++index) {
-			struct cpuidle_state *state = &drv->states[index];
-			bool broadcast = state->flags & CPUIDLE_FLAG_TIMER_STOP;
 			int ret;
+			struct cpuidle_state *state = &drv->states[index];
 
 			/*
 			 * Set the timer to wake this CPU up in some time (which
@@ -318,7 +316,7 @@  static int suspend_test_thread(void *arg)
 			/* IRQs must be disabled during suspend operations. */
 			local_irq_disable();
 
-			ret = suspend_cpu(index, broadcast);
+			ret = suspend_cpu(dev, drv, index);
 
 			/*
 			 * We have woken up. Re-enable IRQs to handle any