On Tue, Oct 29, 2019 at 05:44:34PM +0100, Ulf Hansson wrote:
> In order to enable a CPU to be power managed through its PM domain, let's
> try to attach it by calling psci_dt_attach_cpu() during the cpuidle
> initialization.
>
> psci_dt_attach_cpu() returns a pointer to the attached struct device, which
> later should be used for runtime PM, hence we need to store it somewhere.
> Rather than adding yet another per CPU variable, let's create a per CPU
> struct to collect the relevant per CPU variables.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> ---
>
> Changes in v2:
> - Rebased.
>
> ---
> drivers/cpuidle/cpuidle-psci.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
> index 830995b8a56f..167249d0493f 100644
> --- a/drivers/cpuidle/cpuidle-psci.c
> +++ b/drivers/cpuidle/cpuidle-psci.c
> @@ -20,14 +20,20 @@
>
> #include <asm/cpuidle.h>
>
> +#include "cpuidle-psci.h"
> #include "dt_idle_states.h"
>
> -static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
> +struct psci_cpuidle_data {
> + u32 *psci_states;
> + struct device *dev;
> +};
> +
> +static DEFINE_PER_CPU_READ_MOSTLY(struct psci_cpuidle_data, psci_cpuidle_data);
>
> static int psci_enter_idle_state(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int idx)
> {
> - u32 *state = __this_cpu_read(psci_power_state);
> + u32 *state = __this_cpu_read(psci_cpuidle_data.psci_states);
>
> return CPU_PM_CPU_IDLE_ENTER_PARAM(psci_cpu_suspend_enter,
> idx, state[idx]);
> @@ -78,7 +84,9 @@ static int __init psci_dt_cpu_init_idle(struct device_node *cpu_node,
> {
> int i, ret = 0;
> u32 *psci_states;
> + struct device *dev;
> struct device_node *state_node;
> + struct psci_cpuidle_data *data = per_cpu_ptr(&psci_cpuidle_data, cpu);
>
> state_count++; /* Add WFI state too */
> psci_states = kcalloc(state_count, sizeof(*psci_states), GFP_KERNEL);
> @@ -104,8 +112,16 @@ static int __init psci_dt_cpu_init_idle(struct device_node *cpu_node,
> goto free_mem;
> }
>
> - /* Idle states parsed correctly, initialize per-cpu pointer */
> - per_cpu(psci_power_state, cpu) = psci_states;
> + dev = psci_dt_attach_cpu(cpu);
Why do we need to call psci_dt_attach_cpu for even PC mode and ...
> + if (IS_ERR(dev)) {
> + ret = PTR_ERR(dev);
> + goto free_mem;
> + }
> +
> + data->dev = dev;
> +
... assign NULL above. I don't see the need for one. Default it will be
NULL anyway and we can call psci_dt_attach_cpu only if psci_has_osi_support()
I will look through further patches to see if it make sense or not.
--
Regards,
Sudeep
