On 06/13/2016 09:04 PM, Daniel Lezcano wrote:

On Wed, Jun 08, 2016 at 11:54:30AM -0500, Shreyas B. Prabhu wrote:

quoted

POWER ISA v3 defines a new idle processor core mechanism. In summary,
 a) new instruction named stop is added.
 b) new per thread SPR named PSSCR is added which controls the behavior
	of stop instruction.

Supported idle states and value to be written to PSSCR register to enter
any idle state is exposed via ibm,cpu-idle-state-names and
ibm,cpu-idle-state-psscr respectively. To enter an idle state,
platform provided power_stop() needs to be invoked with the appropriate
PSSCR value.

This patch adds support for this new mechanism in cpuidle powernv driver.

Cc: Rafael J. Wysocki <redacted>
Cc: Daniel Lezcano <redacted>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Lorenzo Pieralisi <redacted>
Cc: linux-pm@vger.kernel.org
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <redacted>
Cc: linuxppc-dev@lists.ozlabs.org
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Shreyas B. Prabhu <redacted>
---

[ ... ]

quoted

+	rc = of_property_read_string_array(power_mgt,
+					   "ibm,cpu-idle-state-names", names,
+					   dt_idle_states);
+	if (rc < 0) {
+		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in DT\n");
+		goto out_free_latency;
+	}
+
+	/*
+	 * If the idle states use stop instruction, probe for psscr values
+	 * which are necessary to specify required stop level.
+	 */
+	if (flags[0] & (OPAL_PM_STOP_INST_FAST | OPAL_PM_STOP_INST_DEEP)) {
+		psscr_val = kcalloc(dt_idle_states, sizeof(*psscr_val),
+				    GFP_KERNEL);

if (!psscr_val) check missing.

I ignored adding this check because this is part of initcall and we are
unlikely to run out of memory at this state. But I'll add the check in
next version.

quoted

+		rc = of_property_read_u64_array(power_mgt,
+						"ibm,cpu-idle-state-psscr",
+						psscr_val, dt_idle_states);
+		if (rc) {
+			pr_warn("cpuidle-powernv: missing ibm,cpu-idle-states-psscr in DT\n");
+			goto out_free_psscr;
+		}
+	}
 	residency_ns = kzalloc(sizeof(*residency_ns) * dt_idle_states, GFP_KERNEL);

if (!residency_ns) check missing.

I suppose the code is relying on 'of_property_read_u32_array' to check it, 
right ?

I'll add the NULL check for existing kzalloc's in the file as well in
the next version.

Thanks,
Shreyas