[PATCH v4 3/5] drivers: psci: refactor psci_cpu_init_idle in preparation for ACPI LPI support

On 09/06/16 14:24, Lorenzo Pieralisi wrote:

On Tue, Apr 19, 2016 at 01:30:11PM +0100, Sudeep Holla wrote:

quoted

Inorder to accomodate bot DT and ACPI LPI support in psci_cpu_init_idle,
move the device tree specific into psci_dt_cpu_init_idle.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Lorenzo Pieralisi <redacted>
Cc: linux-arm-kernel at lists.infradead.org
Signed-off-by: Sudeep Holla <redacted>
---
  drivers/firmware/psci.c | 23 +++++++++--------------
  1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 11bfee8b79a9..af6c5c839568 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c

@@ -250,11 +250,11 @@ static int __init psci_features(u32 psci_func_id)
  #ifdef CONFIG_CPU_IDLE
  static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);

-static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu)
+static int psci_dt_cpu_init_idle(unsigned int cpu)

Unfortunately you would break ARM 32-bit if you did that.

Ah right, I failed to catch this. Thanks for spotting this.

[...]

quoted

  int psci_cpu_init_idle(unsigned int cpu)
  {
-	struct device_node *cpu_node;
-	int ret;
-
-	cpu_node = of_get_cpu_node(cpu, NULL);
-	if (!cpu_node)
-		return -ENODEV;
-
-	ret = psci_dt_cpu_init_idle(cpu_node, cpu);
-
-	of_node_put(cpu_node);
-
-	return ret;
+	return psci_dt_cpu_init_idle(cpu);

How about leaving code as is and you wrap the cpu_node retrieval:

if (!acpi_disabled) {
	acpi_idle_init();
} else {
	cpu_node = of_get_cpu_node(cpu, NULL);
	if (!cpu_node)
		return -ENODEV;

	ret = psci_dt_cpu_init_idle(cpu_node, cpu);

	of_node_put(cpu_node);
}

?

Alternatively, you could create an intermediate stub
__psci_dt_cpu_init_idle(), that will be used for CONFIG_ARM
cpuidle_ops.init and psci_dt_cpu_init_idle() after retrieving
the cpu_node, which I think is slightly cleaner.

I like this approach more.

-- 
Regards,
Sudeep