Re: [PATCH v4 7/8] qcom: cpuidle: Add cpuidle driver for QCOM cpus

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

On Thu, Aug 21, 2014 at 03:36:43PM +0100, Lina Iyer wrote:

[...]

> >>+	cpu_node = of_cpu_device_node_get(cpumask_first(drv->cpumask));
> >>+	if (!cpu_node)
> >>+		return;
> >>+
> >>+	/**
> >>+	 * Get the state description from idle-state node entry-method
> >>+	 * First state is always WFI, per spec.
> >>+	 */
> >>+	modes[0] = MSM_PM_SLEEP_MODE_WAIT_FOR_INTERRUPT;
> >>+	for (i = 1; i < drv->state_count; i++) {
> >>+		mode_name = NULL;
> >>+		state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
> >>+		of_property_read_string(state_node, "entry-method",
> >>+						&mode_name);
> >>+		for (j = 0; mode_name && (j < ARRAY_SIZE(c_states)); j++) {
> >>+			if (!strcmp(mode_name, c_states[j].name)) {
> >>+				modes[i] = c_states[j].mode;
> >>+				break;
> >>+			}
> >>+		}
> >>+	}
> >>+}
> >
> >For the function above, I believe we can do better with the 
> >idle_dt_init function to prevent to have to reparse the infos.
> >
> >I had the opportunity to discuss with Lorenzo privately and we found a 
> >solution he will submit to solve that.
> >
> Hmm. Thanks, I was asked to use the entry-method. FWIW, I dont like this
> either :)

You weren't asked to use entry-method, you were asked to do what psci
does, namely defining a per-state parameter.

Anyway, we will rely on compatible string to initialize the state enter
pointer so that we are all happy again.

The driver will pass an array of pairs {compatible, enter_function},
the dt init will initialize the idle state enter pointer accordingly.

I still have a feeling that DT parsing code should also return an array to
the CPUidle driver corresponding to DT nodes that were used to
initialize idle states, this way, if it is needed, the CPUidle driver
can parse for each index the platform specific idle state properties.

Otherwise we will end up with a function pointer per-state even if two
states are almost identical and just differ in the parameter passed.

It is more a matter of taste than anything else, I will implement the
code discussed with Daniel that will be on the lists at -rc3.

Thanks,
Lorenzo