[PATCH 0/3] powernv/cpuidle Device-tree parsing cleanup

[PATCH 0/3] powernv/cpuidle Device-tree parsing cleanup

[Akshay Adiga]

Device-tree parsed multiple time in powernv cpuidle and powernv
hotplug code. 

First to identify supported flags, secondly, to identify deepest_state
and first deep state, Thirdly , during cpudidle init to find the available
idle states. Any change in device-tree format will lead to make changes in
these 3 places.

This series adds code to parse device tree once and save in global structure.

Akshay Adiga (3):
  powernv/cpuidle: Parse dt idle properties into global structure
  cpuidle/powernv: Change platform init to avoid reparsing dt
  powernv/cpuidle: Use parsed device tree values for cpuidle_init

 arch/powerpc/include/asm/cpuidle.h    |  16 +++
 arch/powerpc/platforms/powernv/idle.c | 214 +++++++++++++++++++++-------------
 drivers/cpuidle/cpuidle-powernv.c     |  49 +++++---
 3 files changed, 182 insertions(+), 97 deletions(-)

-- 
2.5.5

[PATCH 1/3] powernv/cpuidle: Parse dt idle properties into global structure

[Akshay Adiga]

Device-tree parsing happens in twice, once while deciding idle state to
be used for hotplug and once during cpuidle init. Hence, parsing the
device tree and caching it will reduce code duplication. Parsing code
has been moved to pnv_parse_cpuidle_dt() from pnv_probe_idle_states().

Setting up things so that number of available idle states can be
accessible to cpuidle-powernv driver. Hence adding nr_pnv_idle_states to
track number of idle states.

Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/cpuidle.h    |  14 +++
 arch/powerpc/platforms/powernv/idle.c | 197 ++++++++++++++++++++++++----------
 2 files changed, 152 insertions(+), 59 deletions(-)

diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
index e210a83..55ee7e3 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -79,6 +79,20 @@ struct stop_sprs {
 	u64 mmcra;
 };
 
+#define PNV_IDLE_NAME_LEN    16
+struct pnv_idle_states_t {
+	char name[PNV_IDLE_NAME_LEN];
+	u32 latency_ns;
+	u32 residency_ns;
+	/*
+	 * Register value/mask used to select different idle states.
+	 * PMICR in POWER8 and PSSCR in POWER9
+	 */
+	u64 pm_ctrl_reg_val;
+	u64 pm_ctrl_reg_mask;
+	u32 flags;
+};
+
 extern u32 pnv_fastsleep_workaround_at_entry[];
 extern u32 pnv_fastsleep_workaround_at_exit[];
 
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 1c5d067..07be984 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -36,6 +36,8 @@
 #define P9_STOP_SPR_PSSCR      855
 
 static u32 supported_cpuidle_states;
+struct pnv_idle_states_t *pnv_idle_states;
+int nr_pnv_idle_states;
 
 /*
  * The default stop state that will be used by ppc_md.power_save
@@ -625,45 +627,8 @@ int validate_psscr_val_mask(u64 *psscr_val, u64 *psscr_mask, u32 flags)
 static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
 					int dt_idle_states)
 {
-	u64 *psscr_val = NULL;
-	u64 *psscr_mask = NULL;
-	u32 *residency_ns = NULL;
 	u64 max_residency_ns = 0;
-	int rc = 0, i;
-
-	psscr_val = kcalloc(dt_idle_states, sizeof(*psscr_val), GFP_KERNEL);
-	psscr_mask = kcalloc(dt_idle_states, sizeof(*psscr_mask), GFP_KERNEL);
-	residency_ns = kcalloc(dt_idle_states, sizeof(*residency_ns),
-			       GFP_KERNEL);
-
-	if (!psscr_val || !psscr_mask || !residency_ns) {
-		rc = -1;
-		goto out;
-	}
-
-	if (of_property_read_u64_array(np,
-		"ibm,cpu-idle-state-psscr",
-		psscr_val, dt_idle_states)) {
-		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n");
-		rc = -1;
-		goto out;
-	}
-
-	if (of_property_read_u64_array(np,
-				       "ibm,cpu-idle-state-psscr-mask",
-				       psscr_mask, dt_idle_states)) {
-		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr-mask in DT\n");
-		rc = -1;
-		goto out;
-	}
-
-	if (of_property_read_u32_array(np,
-				       "ibm,cpu-idle-state-residency-ns",
-					residency_ns, dt_idle_states)) {
-		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-residency-ns in DT\n");
-		rc = -1;
-		goto out;
-	}
+	int i;
 
 	/*
 	 * Set pnv_first_deep_stop_state, pnv_deepest_stop_psscr_{val,mask},
@@ -681,31 +646,33 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
 	pnv_first_deep_stop_state = MAX_STOP_STATE;
 	for (i = 0; i < dt_idle_states; i++) {
 		int err;
-		u64 psscr_rl = psscr_val[i] & PSSCR_RL_MASK;
+		struct pnv_idle_states_t *state = &pnv_idle_states[i];
+		u64 psscr_rl = state->pm_ctrl_reg_val & PSSCR_RL_MASK;
 
-		if ((flags[i] & OPAL_PM_LOSE_FULL_CONTEXT) &&
-		     (pnv_first_deep_stop_state > psscr_rl))
+		if ((state->flags & OPAL_PM_LOSE_FULL_CONTEXT) &&
+		    (pnv_first_deep_stop_state > psscr_rl))
 			pnv_first_deep_stop_state = psscr_rl;
 
-		err = validate_psscr_val_mask(&psscr_val[i], &psscr_mask[i],
-					      flags[i]);
+		err = validate_psscr_val_mask(&state->pm_ctrl_reg_val,
+					      &state->pm_ctrl_reg_mask,
+					      state->flags);
 		if (err) {
-			report_invalid_psscr_val(psscr_val[i], err);
+			report_invalid_psscr_val(state->pm_ctrl_reg_val, err);
 			continue;
 		}
 
-		if (max_residency_ns < residency_ns[i]) {
-			max_residency_ns = residency_ns[i];
-			pnv_deepest_stop_psscr_val = psscr_val[i];
-			pnv_deepest_stop_psscr_mask = psscr_mask[i];
-			pnv_deepest_stop_flag = flags[i];
+		if (max_residency_ns < state->residency_ns) {
+			max_residency_ns = state->residency_ns;
+			pnv_deepest_stop_psscr_val = state->pm_ctrl_reg_val;
+			pnv_deepest_stop_psscr_mask = state->pm_ctrl_reg_mask;
+			pnv_deepest_stop_flag = state->flags;
 			deepest_stop_found = true;
 		}
 
 		if (!default_stop_found &&
-		    (flags[i] & OPAL_PM_STOP_INST_FAST)) {
-			pnv_default_stop_val = psscr_val[i];
-			pnv_default_stop_mask = psscr_mask[i];
+		    (state->flags & OPAL_PM_STOP_INST_FAST)) {
+			pnv_default_stop_val = state->pm_ctrl_reg_val;
+			pnv_default_stop_mask = state->pm_ctrl_reg_mask;
 			default_stop_found = true;
 		}
 	}
@@ -728,11 +695,8 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
 
 	pr_info("cpuidle-powernv: Requested Level (RL) value of first deep stop = 0x%llx\n",
 		pnv_first_deep_stop_state);
-out:
-	kfree(psscr_val);
-	kfree(psscr_mask);
-	kfree(residency_ns);
-	return rc;
+
+	return 0;
 }
 
 /*
@@ -776,14 +740,129 @@ static void __init pnv_probe_idle_states(void)
 out:
 	kfree(flags);
 }
-static int __init pnv_init_idle_states(void)
+
+/*
+ * This function is use to parse device-tree and populates all the information
+ * into pnv_idle_states structure. Also it sets up nr_pnv_idle_states and
+ * the number of cpuidle states discovered through device-tree.
+ */
+
+static int pnv_parse_cpuidle_dt(void)
 {
+	struct device_node *np;
+	int nr_idle_states, i;
+	u32 *temp_u32;
+	u64 *temp_u64;
+	const char **temp_string;
+
+	np = of_find_node_by_path("/ibm,opal/power-mgt");
+	if (!np) {
+		pr_warn("opal: PowerMgmt Node not found\n");
+		return -ENODEV;
+	}
+	nr_idle_states = of_property_count_u32_elems(np,
+				"ibm,cpu-idle-state-flags");
+
+	pnv_idle_states = kcalloc(nr_idle_states, sizeof(*pnv_idle_states),
+				  GFP_KERNEL);
+	temp_u32 = kcalloc(nr_idle_states, sizeof(u32),  GFP_KERNEL);
+	temp_u64 = kcalloc(nr_idle_states, sizeof(u64),  GFP_KERNEL);
+	temp_string = kcalloc(nr_idle_states, sizeof(char *),  GFP_KERNEL);
+
+	/* Read flags */
+	if (of_property_read_u32_array(np,
+			"ibm,cpu-idle-state-flags", temp_u32, nr_idle_states)) {
+		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-flags in DT\n");
+		goto out;
+	}
+	for (i = 0; i < nr_idle_states; i++)
+		pnv_idle_states[i].flags = temp_u32[i];
 
+	/* Read latencies */
+	if (of_property_read_u32_array(np,
+			"ibm,cpu-idle-state-latencies-ns", temp_u32, nr_idle_states)) {
+		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-latencies-ns in DT\n");
+		goto out;
+	}
+	for (i = 0; i < nr_idle_states; i++)
+		pnv_idle_states[i].latency_ns = temp_u32[i];
+
+	/* Read residencies */
+	if (of_property_read_u32_array(np,
+			"ibm,cpu-idle-state-residency-ns", temp_u32, nr_idle_states)) {
+		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-latencies-ns in DT\n");
+		goto out;
+	}
+	for (i = 0; i < nr_idle_states; i++)
+		pnv_idle_states[i].residency_ns = temp_u32[i];
+
+	/* For power9 */
+	if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+		/* Read pm_crtl_val */
+		if (of_property_read_u64_array(np,
+				"ibm,cpu-idle-state-psscr", temp_u64, nr_idle_states)) {
+			pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n");
+			goto out;
+		}
+		for (i = 0; i < nr_idle_states; i++)
+			pnv_idle_states[i].pm_ctrl_reg_val = temp_u64[i];
+
+		/* Read pm_crtl_mask */
+		if (of_property_read_u64_array(np,
+				"ibm,cpu-idle-state-psscr-mask", temp_u64, nr_idle_states)) {
+			pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr-mask in DT\n");
+			goto out;
+		}
+		for (i = 0; i < nr_idle_states; i++)
+			pnv_idle_states[i].pm_ctrl_reg_mask = temp_u64[i];
+	} else { /* Power8 and Power7 */
+		/* Read pm_crtl_val */
+		if (of_property_read_u64_array(np,
+				"ibm,cpu-idle-state-pmicr", temp_u64, nr_idle_states)) {
+			pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n");
+			goto out;
+		}
+		for (i = 0; i < nr_idle_states; i++)
+			pnv_idle_states[i].pm_ctrl_reg_val = temp_u64[i];
+
+		/* Read pm_crtl_mask */
+		if (of_property_read_u64_array(np,
+				"ibm,cpu-idle-state-pmicr-mask", temp_u64, nr_idle_states)) {
+			pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr-mask in DT\n");
+			goto out;
+		}
+		for (i = 0; i < nr_idle_states; i++)
+			pnv_idle_states[i].pm_ctrl_reg_mask = temp_u64[i];
+
+	}
+	if (of_property_read_string_array(np,
+			"ibm,cpu-idle-state-names", temp_string, nr_idle_states) < 0) {
+		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in DT\n");
+		goto out;
+	}
+	for (i = 0; i < nr_idle_states; i++)
+		strncpy(pnv_idle_states[i].name, temp_string[i],
+			       PNV_IDLE_NAME_LEN);
+	nr_pnv_idle_states = nr_idle_states;
+out:
+	kfree(temp_u32);
+	kfree(temp_u64);
+	kfree(temp_string);
+	return 0;
+}
+
+static int __init pnv_init_idle_states(void)
+{
+	int rc = 0;
 	supported_cpuidle_states = 0;
 
+	/* In case we error out nr_pnv_idle_states will be zero */
+	nr_pnv_idle_states = 0;
 	if (cpuidle_disable != IDLE_NO_OVERRIDE)
 		goto out;
-
+	rc = pnv_parse_cpuidle_dt();
+	if (rc)
+		return rc;
 	pnv_probe_idle_states();
 
 	if (!(supported_cpuidle_states & OPAL_PM_SLEEP_ENABLED_ER1)) {
-- 
2.5.5

Re: [PATCH 1/3] powernv/cpuidle: Parse dt idle properties into global structure

[Gautham R Shenoy]

Hi Akshay,

On Tue, Jun 19, 2018 at 10:34:26AM +0530, Akshay Adiga wrote:
> Device-tree parsing happens in twice, once while deciding idle state to
> be used for hotplug and once during cpuidle init. Hence, parsing the
> device tree and caching it will reduce code duplication. Parsing code
> has been moved to pnv_parse_cpuidle_dt() from pnv_probe_idle_states().
> 
> Setting up things so that number of available idle states can be
> accessible to cpuidle-powernv driver. Hence adding nr_pnv_idle_states to
> track number of idle states.
> 
> Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/cpuidle.h    |  14 +++
>  arch/powerpc/platforms/powernv/idle.c | 197 ++++++++++++++++++++++++----------
>  2 files changed, 152 insertions(+), 59 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index e210a83..55ee7e3 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -79,6 +79,20 @@ struct stop_sprs {
>  	u64 mmcra;
>  };
> 
> +#define PNV_IDLE_NAME_LEN    16
> +struct pnv_idle_states_t {
> +	char name[PNV_IDLE_NAME_LEN];
> +	u32 latency_ns;
> +	u32 residency_ns;
> +	/*
> +	 * Register value/mask used to select different idle states.
> +	 * PMICR in POWER8 and PSSCR in POWER9
> +	 */
> +	u64 pm_ctrl_reg_val;
> +	u64 pm_ctrl_reg_mask;

We don't use pmicr on POWER8. So I don't mind if you rename this to
psscr_val and psscr_mask.

Or atleast have
   	   union {
	   	 u64 psscr; /* P9 onwards */
		 u64 pmicr  /* P7 and P8 */
	    } val;

	   union {
	   	 u64 psscr; /* P9 onwards */
		 u64 pmicr; /* P7 and P8 */
	   } mask;


	


> +	u32 flags;
> +};
> +
>  extern u32 pnv_fastsleep_workaround_at_entry[];
>  extern u32 pnv_fastsleep_workaround_at_exit[];
> 
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 1c5d067..07be984 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -36,6 +36,8 @@
>  #define P9_STOP_SPR_PSSCR      855
> 
>  static u32 supported_cpuidle_states;
> +struct pnv_idle_states_t *pnv_idle_states;
> +int nr_pnv_idle_states;
> 
>  /*
>   * The default stop state that will be used by ppc_md.power_save
> @@ -625,45 +627,8 @@ int validate_psscr_val_mask(u64 *psscr_val, u64 *psscr_mask, u32 flags)
>  static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
>  					int dt_idle_states)

We are not using np, flags in this function right? Also dt_idle_states
can be obtained from the global variable nr_pnv_idle_states.


>  {
> -	u64 *psscr_val = NULL;
> -	u64 *psscr_mask = NULL;
> -	u32 *residency_ns = NULL;
>  	u64 max_residency_ns = 0;
> -	int rc = 0, i;
> -
> -	psscr_val = kcalloc(dt_idle_states, sizeof(*psscr_val), GFP_KERNEL);
> -	psscr_mask = kcalloc(dt_idle_states, sizeof(*psscr_mask), GFP_KERNEL);
> -	residency_ns = kcalloc(dt_idle_states, sizeof(*residency_ns),
> -			       GFP_KERNEL);
> -
> -	if (!psscr_val || !psscr_mask || !residency_ns) {
> -		rc = -1;
> -		goto out;
> -	}
> -
> -	if (of_property_read_u64_array(np,
> -		"ibm,cpu-idle-state-psscr",
> -		psscr_val, dt_idle_states)) {
> -		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n");
> -		rc = -1;
> -		goto out;
> -	}
> -
> -	if (of_property_read_u64_array(np,
> -				       "ibm,cpu-idle-state-psscr-mask",
> -				       psscr_mask, dt_idle_states)) {
> -		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr-mask in DT\n");
> -		rc = -1;
> -		goto out;
> -	}
> -
> -	if (of_property_read_u32_array(np,
> -				       "ibm,cpu-idle-state-residency-ns",
> -					residency_ns, dt_idle_states)) {
> -		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-residency-ns in DT\n");
> -		rc = -1;
> -		goto out;
> -	}
> +	int i;
> 
>  	/*
>  	 * Set pnv_first_deep_stop_state, pnv_deepest_stop_psscr_{val,mask},
> @@ -681,31 +646,33 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
>  	pnv_first_deep_stop_state = MAX_STOP_STATE;
>  	for (i = 0; i < dt_idle_states; i++) {
>  		int err;
> -		u64 psscr_rl = psscr_val[i] & PSSCR_RL_MASK;
> +		struct pnv_idle_states_t *state = &pnv_idle_states[i];
> +		u64 psscr_rl = state->pm_ctrl_reg_val & PSSCR_RL_MASK;
> 
> -		if ((flags[i] & OPAL_PM_LOSE_FULL_CONTEXT) &&
> -		     (pnv_first_deep_stop_state > psscr_rl))
> +		if ((state->flags & OPAL_PM_LOSE_FULL_CONTEXT) &&
> +		    (pnv_first_deep_stop_state > psscr_rl))
>  			pnv_first_deep_stop_state = psscr_rl;
> 
> -		err = validate_psscr_val_mask(&psscr_val[i], &psscr_mask[i],
> -					      flags[i]);
> +		err = validate_psscr_val_mask(&state->pm_ctrl_reg_val,
> +					      &state->pm_ctrl_reg_mask,
> +					      state->flags);


  We could have a "bool valid" field in the pnv_idle_states_t struct
  for explicitly recording any invalid states in order to prevent any
  other subsystems from using it. We are doing the validation of the
  psscr_val and mask twice today - once in this code and once again in
  the cpuidle code.
  
  
>  		if (err) {
> -			report_invalid_psscr_val(psscr_val[i], err);
> +			report_invalid_psscr_val(state->pm_ctrl_reg_val, err);
>  			continue;
>  		}
> 
> -		if (max_residency_ns < residency_ns[i]) {
> -			max_residency_ns = residency_ns[i];
> -			pnv_deepest_stop_psscr_val = psscr_val[i];
> -			pnv_deepest_stop_psscr_mask = psscr_mask[i];
> -			pnv_deepest_stop_flag = flags[i];
> +		if (max_residency_ns < state->residency_ns) {
> +			max_residency_ns = state->residency_ns;
> +			pnv_deepest_stop_psscr_val = state->pm_ctrl_reg_val;
> +			pnv_deepest_stop_psscr_mask = state->pm_ctrl_reg_mask;
> +			pnv_deepest_stop_flag = state->flags;
>  			deepest_stop_found = true;
>  		}
> 
>  		if (!default_stop_found &&
> -		    (flags[i] & OPAL_PM_STOP_INST_FAST)) {
> -			pnv_default_stop_val = psscr_val[i];
> -			pnv_default_stop_mask = psscr_mask[i];
> +		    (state->flags & OPAL_PM_STOP_INST_FAST)) {
> +			pnv_default_stop_val = state->pm_ctrl_reg_val;
> +			pnv_default_stop_mask = state->pm_ctrl_reg_mask;
>  			default_stop_found = true;
>  		}
>  	}
> @@ -728,11 +695,8 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
> 
>  	pr_info("cpuidle-powernv: Requested Level (RL) value of first deep stop = 0x%llx\n",
>  		pnv_first_deep_stop_state);
> -out:
> -	kfree(psscr_val);
> -	kfree(psscr_mask);
> -	kfree(residency_ns);
> -	return rc;
> +
> +	return 0;
>  }
> 
>  /*
> @@ -776,14 +740,129 @@ static void __init pnv_probe_idle_states(void)
>  out:
>  	kfree(flags);
>  }
> -static int __init pnv_init_idle_states(void)
> +
> +/*
> + * This function is use to parse device-tree and populates all the information
> + * into pnv_idle_states structure. Also it sets up nr_pnv_idle_states and
> + * the number of cpuidle states discovered through device-tree.

  "Also it sets up nr_pnv_idle_states *which is* the number of cpuidle
  states discovered through the device-tree."
  
> + */
> +
> +static int pnv_parse_cpuidle_dt(void)
>  {
> +	struct device_node *np;
> +	int nr_idle_states, i;
> +	u32 *temp_u32;
> +	u64 *temp_u64;
> +	const char **temp_string;
> +
> +	np = of_find_node_by_path("/ibm,opal/power-mgt");
> +	if (!np) {
> +		pr_warn("opal: PowerMgmt Node not found\n");
> +		return -ENODEV;
> +	}
> +	nr_idle_states = of_property_count_u32_elems(np,
> +				"ibm,cpu-idle-state-flags");
> +
> +	pnv_idle_states = kcalloc(nr_idle_states, sizeof(*pnv_idle_states),
> +				  GFP_KERNEL);
> +	temp_u32 = kcalloc(nr_idle_states, sizeof(u32),  GFP_KERNEL);
> +	temp_u64 = kcalloc(nr_idle_states, sizeof(u64),  GFP_KERNEL);
> +	temp_string = kcalloc(nr_idle_states, sizeof(char *), GFP_KERNEL);

We should ensure that the allocations has succeeded here, else
bail out and disable the idle states.
	
> +
> +	/* Read flags */
> +	if (of_property_read_u32_array(np,
> +			"ibm,cpu-idle-state-flags", temp_u32, nr_idle_states)) {
> +		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-flags in DT\n");
> +		goto out;
> +	}

There was some device-tree hardening in the cpuidle code which I think
can be moved to this place.

> +	for (i = 0; i < nr_idle_states; i++)
> +		pnv_idle_states[i].flags = temp_u32[i];
> 
> +	/* Read latencies */
> +	if (of_property_read_u32_array(np,
> +			"ibm,cpu-idle-state-latencies-ns", temp_u32, nr_idle_states)) {
> +		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-latencies-ns in DT\n");
> +		goto out;
> +	}
> +	for (i = 0; i < nr_idle_states; i++)
> +		pnv_idle_states[i].latency_ns = temp_u32[i];
> +
> +	/* Read residencies */
> +	if (of_property_read_u32_array(np,
> +			"ibm,cpu-idle-state-residency-ns", temp_u32, nr_idle_states)) {
> +		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-latencies-ns in DT\n");
> +		goto out;
> +	}
> +	for (i = 0; i < nr_idle_states; i++)
> +		pnv_idle_states[i].residency_ns = temp_u32[i];
> +
> +	/* For power9 */
> +	if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> +		/* Read pm_crtl_val */
> +		if (of_property_read_u64_array(np,
> +				"ibm,cpu-idle-state-psscr", temp_u64, nr_idle_states)) {
> +			pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n");
> +			goto out;
> +		}
> +		for (i = 0; i < nr_idle_states; i++)
> +			pnv_idle_states[i].pm_ctrl_reg_val = temp_u64[i];
> +
> +		/* Read pm_crtl_mask */
> +		if (of_property_read_u64_array(np,
> +				"ibm,cpu-idle-state-psscr-mask", temp_u64, nr_idle_states)) {
> +			pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr-mask in DT\n");
> +			goto out;
> +		}
> +		for (i = 0; i < nr_idle_states; i++)
> +			pnv_idle_states[i].pm_ctrl_reg_mask = temp_u64[i];
> +	} else { /* Power8 and Power7 */
> +		/* Read pm_crtl_val */
> +		if (of_property_read_u64_array(np,
> +				"ibm,cpu-idle-state-pmicr", temp_u64, nr_idle_states)) {
> +			pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n");
> +			goto out;
> +		}
> +		for (i = 0; i < nr_idle_states; i++)
> +			pnv_idle_states[i].pm_ctrl_reg_val = temp_u64[i];
> +
> +		/* Read pm_crtl_mask */
> +		if (of_property_read_u64_array(np,
> +				"ibm,cpu-idle-state-pmicr-mask", temp_u64, nr_idle_states)) {
> +			pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr-mask in DT\n");
> +			goto out;
> +		}
> +		for (i = 0; i < nr_idle_states; i++)
> +			pnv_idle_states[i].pm_ctrl_reg_mask = temp_u64[i];

We don't use the pmicr val and the mask in the kernel for either
POWER7 or POWER8. So, is this "else" block required ?

> +
> +	}
> +	if (of_property_read_string_array(np,
> +			"ibm,cpu-idle-state-names", temp_string, nr_idle_states) < 0) {
> +		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in DT\n");
> +		goto out;
> +	}
> +	for (i = 0; i < nr_idle_states; i++)
> +		strncpy(pnv_idle_states[i].name, temp_string[i],
> +			       PNV_IDLE_NAME_LEN);
> +	nr_pnv_idle_states = nr_idle_states;
> +out:
> +	kfree(temp_u32);
> +	kfree(temp_u64);
> +	kfree(temp_string);
> +	return 0;

We shouldn't be returning 0 we have come to "out" due to any failure
in the device-tree parsing ?

> +}
> +
> +static int __init pnv_init_idle_states(void)
> +{
> +	int rc = 0;
>  	supported_cpuidle_states = 0;
> 
> +	/* In case we error out nr_pnv_idle_states will be zero */
> +	nr_pnv_idle_states = 0;
>  	if (cpuidle_disable != IDLE_NO_OVERRIDE)
>  		goto out;
> -
> +	rc = pnv_parse_cpuidle_dt();
> +	if (rc)
> +		return rc;
>  	pnv_probe_idle_states();
>
>  	if (!(supported_cpuidle_states & OPAL_PM_SLEEP_ENABLED_ER1)) {
> -- 
> 2.5.5
> 

[PATCH 2/3] cpuidle/powernv: Change platform init to avoid reparsing dt

[Akshay Adiga]

The required data is accessible from cpuidle_states structure and
nr_cpu_idle_states. This patch makes changes to avoid reparsing and use
data from these structures.

Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/idle.c | 37 ++++++++---------------------------
 1 file changed, 8 insertions(+), 29 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 07be984..0a62611 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -624,8 +624,7 @@ int validate_psscr_val_mask(u64 *psscr_val, u64 *psscr_mask, u32 flags)
  * @dt_idle_states: Number of idle state entries
  * Returns 0 on success
  */
-static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
-					int dt_idle_states)
+static int __init pnv_power9_idle_init(void)
 {
 	u64 max_residency_ns = 0;
 	int i;
@@ -644,7 +643,7 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
 	 * the shallowest (OPAL_PM_STOP_INST_FAST) loss-less stop state.
 	 */
 	pnv_first_deep_stop_state = MAX_STOP_STATE;
-	for (i = 0; i < dt_idle_states; i++) {
+	for (i = 0; i < nr_pnv_idle_states; i++) {
 		int err;
 		struct pnv_idle_states_t *state = &pnv_idle_states[i];
 		u64 psscr_rl = state->pm_ctrl_reg_val & PSSCR_RL_MASK;
@@ -704,41 +703,21 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
  */
 static void __init pnv_probe_idle_states(void)
 {
-	struct device_node *np;
-	int dt_idle_states;
-	u32 *flags = NULL;
 	int i;
 
-	np = of_find_node_by_path("/ibm,opal/power-mgt");
-	if (!np) {
-		pr_warn("opal: PowerMgmt Node not found\n");
-		goto out;
-	}
-	dt_idle_states = of_property_count_u32_elems(np,
-			"ibm,cpu-idle-state-flags");
-	if (dt_idle_states < 0) {
+	if (nr_pnv_idle_states < 0) {
 		pr_warn("cpuidle-powernv: no idle states found in the DT\n");
-		goto out;
-	}
-
-	flags = kcalloc(dt_idle_states, sizeof(*flags),  GFP_KERNEL);
-
-	if (of_property_read_u32_array(np,
-			"ibm,cpu-idle-state-flags", flags, dt_idle_states)) {
-		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-flags in DT\n");
-		goto out;
+		return;
 	}
 
 	if (cpu_has_feature(CPU_FTR_ARCH_300)) {
-		if (pnv_power9_idle_init(np, flags, dt_idle_states))
-			goto out;
+		if (pnv_power9_idle_init())
+			return;
 	}
 
-	for (i = 0; i < dt_idle_states; i++)
-		supported_cpuidle_states |= flags[i];
+	for (i = 0; i < nr_pnv_idle_states; i++)
+		supported_cpuidle_states |= pnv_idle_states[i].flags;
 
-out:
-	kfree(flags);
 }
 
 /*
-- 
2.5.5

Re: [PATCH 2/3] cpuidle/powernv: Change platform init to avoid reparsing dt

[Gautham R Shenoy]

Hi Akshay,


On Tue, Jun 19, 2018 at 10:34:27AM +0530, Akshay Adiga wrote:
> The required data is accessible from cpuidle_states structure and
> nr_cpu_idle_states. This patch makes changes to avoid reparsing and use
> data from these structures.
> 
> Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/idle.c | 37 ++++++++---------------------------
>  1 file changed, 8 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 07be984..0a62611 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -624,8 +624,7 @@ int validate_psscr_val_mask(u64 *psscr_val, u64 *psscr_mask, u32 flags)
>   * @dt_idle_states: Number of idle state entries
>   * Returns 0 on success
>   */
> -static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
> -					int dt_idle_states)
> +static int __init pnv_power9_idle_init(void)
>  {
>  	u64 max_residency_ns = 0;
>  	int i;
> @@ -644,7 +643,7 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
>  	 * the shallowest (OPAL_PM_STOP_INST_FAST) loss-less stop state.
>  	 */
>  	pnv_first_deep_stop_state = MAX_STOP_STATE;
> -	for (i = 0; i < dt_idle_states; i++) {
> +	for (i = 0; i < nr_pnv_idle_states; i++) {
>  		int err;
>  		struct pnv_idle_states_t *state = &pnv_idle_states[i];
>  		u64 psscr_rl = state->pm_ctrl_reg_val & PSSCR_RL_MASK;

These above two hunks could be folded into the first patch.

> @@ -704,41 +703,21 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
>   */
>  static void __init pnv_probe_idle_states(void)

Can we move the remaining content of this function into the calling
function pnv_init_idle_states() ? That way...

>  {
> -	struct device_node *np;
> -	int dt_idle_states;
> -	u32 *flags = NULL;
>  	int i;
> 
> -	np = of_find_node_by_path("/ibm,opal/power-mgt");
> -	if (!np) {
> -		pr_warn("opal: PowerMgmt Node not found\n");
> -		goto out;
> -	}
> -	dt_idle_states = of_property_count_u32_elems(np,
> -			"ibm,cpu-idle-state-flags");
> -	if (dt_idle_states < 0) {
> +	if (nr_pnv_idle_states < 0) {

... This reduntant check can be removed. Because nr_pnv_idle_states is initialized to
zero) and if there is an error in parsing the device tree we don't set
up this value. If there is no error, then nr_pnv_idle_states is a
positive value. 



>  		pr_warn("cpuidle-powernv: no idle states found in the DT\n");
> -		goto out;
> -	}
> -
> -	flags = kcalloc(dt_idle_states, sizeof(*flags),  GFP_KERNEL);
> -
> -	if (of_property_read_u32_array(np,
> -			"ibm,cpu-idle-state-flags", flags, dt_idle_states)) {
> -		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-flags in DT\n");
> -		goto out;
> +		return;
>  	}
> 
>  	if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> -		if (pnv_power9_idle_init(np, flags, dt_idle_states))
> -			goto out;
> +		if (pnv_power9_idle_init())

There cannot any longer be a failure in pnv_power9_idle_init() after
the first patch, since we always return a 0. So  this if condition can be removed.

> +			return;
>  	}
> 
> -	for (i = 0; i < dt_idle_states; i++)
> -		supported_cpuidle_states |= flags[i];
> +	for (i = 0; i < nr_pnv_idle_states; i++)
> +		supported_cpuidle_states |= pnv_idle_states[i].flags;

Ideally we should be setting the supported_cpuidle_states flags after
all the validatations have been done, including the ones for stop-api
initialization.

> 
> -out:
> -	kfree(flags);
>  }
> 
>  /*
> -- 
> 2.5.5
> 

[PATCH 3/3] powernv/cpuidle: Use parsed device tree values for cpuidle_init

[Akshay Adiga]

Export pnv_idle_states and nr_pnv_idle_states so that its accessible to
cpuidle driver. Use properties from pnv_idle_states structure for powernv
cpuidle_init.

Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/cpuidle.h |  2 ++
 drivers/cpuidle/cpuidle-powernv.c  | 49 +++++++++++++++++++++++---------------
 2 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
index 55ee7e3..1446747 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -93,6 +93,8 @@ struct pnv_idle_states_t {
 	u32 flags;
 };
 
+extern struct pnv_idle_states_t *pnv_idle_states;
+extern int nr_pnv_idle_states;
 extern u32 pnv_fastsleep_workaround_at_entry[];
 extern u32 pnv_fastsleep_workaround_at_exit[];
 
diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index d29e4f0..de8ba26 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -285,6 +285,11 @@ static int powernv_add_idle_states(void)
 		goto out;
 	}
 
+	if (nr_pnv_idle_states <= 0) {
+		pr_warn("opal: No idle states found\n");
+		goto out;
+	}
+
 	/* Read values of any property to determine the num of idle states */
 	dt_idle_states = of_property_count_u32_elems(power_mgt, "ibm,cpu-idle-state-flags");
 	if (dt_idle_states < 0) {
@@ -338,7 +343,7 @@ static int powernv_add_idle_states(void)
 	 * If the idle states use stop instruction, probe for psscr values
 	 * and psscr mask which are necessary to specify required stop level.
 	 */
-	has_stop_states = (flags[0] &
+	has_stop_states = (pnv_idle_states[0].flags &
 			   (OPAL_PM_STOP_INST_FAST | OPAL_PM_STOP_INST_DEEP));
 	if (has_stop_states) {
 		count = of_property_count_u64_elems(power_mgt,
@@ -387,51 +392,55 @@ static int powernv_add_idle_states(void)
 						residency_ns, dt_idle_states);
 	}
 
-	for (i = 0; i < dt_idle_states; i++) {
+	for (i = 0; i < nr_pnv_idle_states; i++) {
 		unsigned int exit_latency, target_residency;
 		bool stops_timebase = false;
+		struct pnv_idle_states_t *state = &pnv_idle_states[i];
 
 		/*
 		 * Skip the platform idle state whose flag isn't in
-		 * the supported_cpuidle_states flag mask.
+		 * the supported_pnv_idle_states flag mask.
 		 */
-		if ((flags[i] & supported_flags) != flags[i])
+		if ((state->flags & supported_flags) !=
+				state->flags)
 			continue;
 		/*
 		 * If an idle state has exit latency beyond
 		 * POWERNV_THRESHOLD_LATENCY_NS then don't use it
 		 * in cpu-idle.
 		 */
-		if (latency_ns[i] > POWERNV_THRESHOLD_LATENCY_NS)
+		if (state->latency_ns > POWERNV_THRESHOLD_LATENCY_NS)
 			continue;
 		/*
 		 * Firmware passes residency and latency values in ns.
 		 * cpuidle expects it in us.
 		 */
-		exit_latency = DIV_ROUND_UP(latency_ns[i], 1000);
+		exit_latency = DIV_ROUND_UP(state->latency_ns, 1000);
 		if (!rc)
-			target_residency = DIV_ROUND_UP(residency_ns[i], 1000);
+			target_residency = DIV_ROUND_UP(state->residency_ns, 1000);
 		else
 			target_residency = 0;
 
 		if (has_stop_states) {
-			int err = validate_psscr_val_mask(&psscr_val[i],
-							  &psscr_mask[i],
-							  flags[i]);
+			int err;
+			err = validate_psscr_val_mask(&state->pm_ctrl_reg_val,
+						      &state->pm_ctrl_reg_mask,
+						      state->flags);
 			if (err) {
-				report_invalid_psscr_val(psscr_val[i], err);
+				report_invalid_psscr_val(state->pm_ctrl_reg_val,
+							 err);
 				continue;
 			}
 		}
 
-		if (flags[i] & OPAL_PM_TIMEBASE_STOP)
+		if (state->flags & OPAL_PM_TIMEBASE_STOP)
 			stops_timebase = true;
 
 		/*
 		 * For nap and fastsleep, use default target_residency
 		 * values if f/w does not expose it.
 		 */
-		if (flags[i] & OPAL_PM_NAP_ENABLED) {
+		if (state->flags & OPAL_PM_NAP_ENABLED) {
 			if (!rc)
 				target_residency = 100;
 			/* Add NAP state */
@@ -439,10 +448,11 @@ static int powernv_add_idle_states(void)
 					  CPUIDLE_FLAG_NONE, nap_loop,
 					  target_residency, exit_latency, 0, 0);
 		} else if (has_stop_states && !stops_timebase) {
-			add_powernv_state(nr_idle_states, names[i],
+			add_powernv_state(nr_idle_states, state->name,
 					  CPUIDLE_FLAG_NONE, stop_loop,
 					  target_residency, exit_latency,
-					  psscr_val[i], psscr_mask[i]);
+					  state->pm_ctrl_reg_val,
+					  state->pm_ctrl_reg_mask);
 		}
 
 		/*
@@ -450,8 +460,8 @@ static int powernv_add_idle_states(void)
 		 * within this config dependency check.
 		 */
 #ifdef CONFIG_TICK_ONESHOT
-		else if (flags[i] & OPAL_PM_SLEEP_ENABLED ||
-			 flags[i] & OPAL_PM_SLEEP_ENABLED_ER1) {
+		else if (state->flags & OPAL_PM_SLEEP_ENABLED ||
+			 state->flags & OPAL_PM_SLEEP_ENABLED_ER1) {
 			if (!rc)
 				target_residency = 300000;
 			/* Add FASTSLEEP state */
@@ -460,10 +470,11 @@ static int powernv_add_idle_states(void)
 					  fastsleep_loop,
 					  target_residency, exit_latency, 0, 0);
 		} else if (has_stop_states && stops_timebase) {
-			add_powernv_state(nr_idle_states, names[i],
+			add_powernv_state(nr_idle_states, state->name,
 					  CPUIDLE_FLAG_TIMER_STOP, stop_loop,
 					  target_residency, exit_latency,
-					  psscr_val[i], psscr_mask[i]);
+					  state->pm_ctrl_reg_val,
+					  state->pm_ctrl_reg_mask);
 		}
 #endif
 		else
-- 
2.5.5

Re: [PATCH 3/3] powernv/cpuidle: Use parsed device tree values for cpuidle_init

[Gautham R Shenoy]

Hi Akshay,

On Tue, Jun 19, 2018 at 10:34:28AM +0530, Akshay Adiga wrote:
> Export pnv_idle_states and nr_pnv_idle_states so that its accessible to
> cpuidle driver. Use properties from pnv_idle_states structure for powernv
> cpuidle_init.
> 
> Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/cpuidle.h |  2 ++
>  drivers/cpuidle/cpuidle-powernv.c  | 49 +++++++++++++++++++++++---------------
>  2 files changed, 32 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index 55ee7e3..1446747 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -93,6 +93,8 @@ struct pnv_idle_states_t {
>  	u32 flags;
>  };
> 
> +extern struct pnv_idle_states_t *pnv_idle_states;
> +extern int nr_pnv_idle_states;
>  extern u32 pnv_fastsleep_workaround_at_entry[];
>  extern u32 pnv_fastsleep_workaround_at_exit[];
> 
> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
> index d29e4f0..de8ba26 100644
> --- a/drivers/cpuidle/cpuidle-powernv.c
> +++ b/drivers/cpuidle/cpuidle-powernv.c
> @@ -285,6 +285,11 @@ static int powernv_add_idle_states(void)
>  		goto out;
>  	}
> 
> +	if (nr_pnv_idle_states <= 0) {
> +		pr_warn("opal: No idle states found\n");

We would have a warning of this type in the powernv/idle.c right ? We
don't need to warn again. Or at least have a different warning
something like: 

      	         pr_warn("cpuidle: Only snooze state is available\n");
> +		goto out;
> +	}

> +
>  	/* Read values of any property to determine the num of idle states */
>  	dt_idle_states = of_property_count_u32_elems(power_mgt, "ibm,cpu-idle-state-flags");


We don't need to get dt_idle_states from the device tree now that we
have nr_pnv_idle_states. 


>  	if (dt_idle_states < 0) {
> @@ -338,7 +343,7 @@ static int powernv_add_idle_states(void)
>  	 * If the idle states use stop instruction, probe for psscr values
>  	 * and psscr mask which are necessary to specify required stop level.
>  	 */
> -	has_stop_states = (flags[0] &
> +	has_stop_states = (pnv_idle_states[0].flags &
>  			   (OPAL_PM_STOP_INST_FAST | OPAL_PM_STOP_INST_DEEP));
>  	if (has_stop_states) {
>  		count = of_property_count_u64_elems(power_mgt,
> @@ -387,51 +392,55 @@ static int powernv_add_idle_states(void)
>  						residency_ns, dt_idle_states);
>  	}
> 
> -	for (i = 0; i < dt_idle_states; i++) {
> +	for (i = 0; i < nr_pnv_idle_states; i++) {
>  		unsigned int exit_latency, target_residency;
>  		bool stops_timebase = false;
> +		struct pnv_idle_states_t *state = &pnv_idle_states[i];
> 
>  		/*
>  		 * Skip the platform idle state whose flag isn't in
> -		 * the supported_cpuidle_states flag mask.
> +		 * the supported_pnv_idle_states flag mask.
>  		 */
> -		if ((flags[i] & supported_flags) != flags[i])
> +		if ((state->flags & supported_flags) !=
> +				state->flags)
>  			continue;
>  		/*
>  		 * If an idle state has exit latency beyond
>  		 * POWERNV_THRESHOLD_LATENCY_NS then don't use it
>  		 * in cpu-idle.
>  		 */
> -		if (latency_ns[i] > POWERNV_THRESHOLD_LATENCY_NS)
> +		if (state->latency_ns > POWERNV_THRESHOLD_LATENCY_NS)
>  			continue;
>  		/*
>  		 * Firmware passes residency and latency values in ns.
>  		 * cpuidle expects it in us.
>  		 */
> -		exit_latency = DIV_ROUND_UP(latency_ns[i], 1000);
> +		exit_latency = DIV_ROUND_UP(state->latency_ns, 1000);
>  		if (!rc)
> -			target_residency = DIV_ROUND_UP(residency_ns[i], 1000);
> +			target_residency = DIV_ROUND_UP(state->residency_ns, 1000);
>  		else
>  			target_residency = 0;
> 
>  		if (has_stop_states) {
> -			int err = validate_psscr_val_mask(&psscr_val[i],
> -							  &psscr_mask[i],
> -							  flags[i]);

> +			int err;
> +			err = validate_psscr_val_mask(&state->pm_ctrl_reg_val,
> +						      &state->pm_ctrl_reg_mask,
> +						      state->flags);
>  			if (err) {
> -				report_invalid_psscr_val(psscr_val[i], err);
> +				report_invalid_psscr_val(state->pm_ctrl_reg_val,
> +							 err);
>  				continue;
>  			}

We have already validated the states once. We should simply use the
result here.


>  		}
> 
> -		if (flags[i] & OPAL_PM_TIMEBASE_STOP)
> +		if (state->flags & OPAL_PM_TIMEBASE_STOP)
>  			stops_timebase = true;
> 
>  		/*
>  		 * For nap and fastsleep, use default target_residency
>  		 * values if f/w does not expose it.
>  		 */
> -		if (flags[i] & OPAL_PM_NAP_ENABLED) {
> +		if (state->flags & OPAL_PM_NAP_ENABLED) {
>  			if (!rc)
>  				target_residency = 100;
>  			/* Add NAP state */
> @@ -439,10 +448,11 @@ static int powernv_add_idle_states(void)
>  					  CPUIDLE_FLAG_NONE, nap_loop,
>  					  target_residency, exit_latency, 0, 0);
>  		} else if (has_stop_states && !stops_timebase) {
> -			add_powernv_state(nr_idle_states, names[i],
> +			add_powernv_state(nr_idle_states, state->name,
>  					  CPUIDLE_FLAG_NONE, stop_loop,
>  					  target_residency, exit_latency,
> -					  psscr_val[i], psscr_mask[i]);
> +					  state->pm_ctrl_reg_val,
> +					  state->pm_ctrl_reg_mask);
>  		}
> 
>  		/*
> @@ -450,8 +460,8 @@ static int powernv_add_idle_states(void)
>  		 * within this config dependency check.
>  		 */
>  #ifdef CONFIG_TICK_ONESHOT
> -		else if (flags[i] & OPAL_PM_SLEEP_ENABLED ||
> -			 flags[i] & OPAL_PM_SLEEP_ENABLED_ER1) {
> +		else if (state->flags & OPAL_PM_SLEEP_ENABLED ||
> +			 state->flags & OPAL_PM_SLEEP_ENABLED_ER1) {
>  			if (!rc)
>  				target_residency = 300000;
>  			/* Add FASTSLEEP state */
> @@ -460,10 +470,11 @@ static int powernv_add_idle_states(void)
>  					  fastsleep_loop,
>  					  target_residency, exit_latency, 0, 0);
>  		} else if (has_stop_states && stops_timebase) {
> -			add_powernv_state(nr_idle_states, names[i],
> +			add_powernv_state(nr_idle_states, state->name,
>  					  CPUIDLE_FLAG_TIMER_STOP, stop_loop,
>  					  target_residency, exit_latency,
> -					  psscr_val[i], psscr_mask[i]);
> +					  state->pm_ctrl_reg_val,
> +					  state->pm_ctrl_reg_mask);
>  		}
>  #endif
>  		else
> -- 
> 2.5.5
>