[PATCH 0/4][V2] davinci: cpuidle - some cleanups

[PATCH 0/4][V2] davinci: cpuidle - some cleanups

[Daniel Lezcano]

This patchset does some cleanup. It could have been folded in a single
patch but the review would have been less clean than splitting it into
small and trivial patches.

The main purpose of this patch is to remove the usage of the driver_data
field from the state_usage structure. Len Brown is doing this cleanup in
the intel_idle.c file. With this patchset, the processor_idle.c file will
be the last user of this field.

Also, the patchset simplify the code and makes it a bit more clear to read.

Thanks to Sekhar for testing the patchset.

Changelog:
V2:
 * removed global static variable initialization to 'false'
 * added Acked-by: Sekhar Nori <nsekhar@ti.com>
V1: initial post

Daniel Lezcano (4):
  davinci: cpuidle - use global variable for ddr2 flag
  davinci: cpuidle - move code to prevent forward declaration
  davinci: cpuidle - remove the ops
  davinci: cpuidle - remove useless initialization

 arch/arm/mach-davinci/cpuidle.c |   84 ++++++++++++---------------------------
 1 file changed, 25 insertions(+), 59 deletions(-)

-- 
1.7.9.5

[PATCH 1/4][V2] davinci: cpuidle - use global variable for ddr2 flag

[Daniel Lezcano]

Replace the flag by a simple global boolean in the cpuidle.c.
That will allow to cleanup the rest of the code right after,
because the ops won't make sense.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Sekhar Nori <nsekhar@ti.com>
---
 arch/arm/mach-davinci/cpuidle.c |   23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index 9107691..5fbd470 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -26,8 +26,8 @@
 #define DAVINCI_CPUIDLE_MAX_STATES	2
 
 struct davinci_ops {
-	void (*enter) (u32 flags);
-	void (*exit) (u32 flags);
+	void (*enter) (void);
+	void (*exit) (void);
 	u32 flags;
 };
 
@@ -40,20 +40,17 @@ static int davinci_enter_idle(struct cpuidle_device *dev,
 	struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
 
 	if (ops && ops->enter)
-		ops->enter(ops->flags);
+		ops->enter();
 
 	index = cpuidle_wrap_enter(dev,	drv, index,
 				arm_cpuidle_simple_enter);
 
 	if (ops && ops->exit)
-		ops->exit(ops->flags);
+		ops->exit();
 
 	return index;
 }
 
-/* fields in davinci_ops.flags */
-#define DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN	BIT(0)
-
 static struct cpuidle_driver davinci_idle_driver = {
 	.name			= "cpuidle-davinci",
 	.owner			= THIS_MODULE,
@@ -72,6 +69,7 @@ static struct cpuidle_driver davinci_idle_driver = {
 
 static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
 static void __iomem *ddr2_reg_base;
+static bool ddr2_pdown;
 
 static void davinci_save_ddr_power(int enter, bool pdown)
 {
@@ -92,14 +90,14 @@ static void davinci_save_ddr_power(int enter, bool pdown)
 	__raw_writel(val, ddr2_reg_base + DDR2_SDRCR_OFFSET);
 }
 
-static void davinci_c2state_enter(u32 flags)
+static void davinci_c2state_enter(void)
 {
-	davinci_save_ddr_power(1, !!(flags & DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN));
+	davinci_save_ddr_power(1, ddr2_pdown);
 }
 
-static void davinci_c2state_exit(u32 flags)
+static void davinci_c2state_exit(void)
 {
-	davinci_save_ddr_power(0, !!(flags & DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN));
+	davinci_save_ddr_power(0, ddr2_pdown);
 }
 
 static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
@@ -124,8 +122,7 @@ static int __init davinci_cpuidle_probe(struct platform_device *pdev)
 
 	ddr2_reg_base = pdata->ddr2_ctlr_base;
 
-	if (pdata->ddr2_pdown)
-		davinci_states[1].flags |= DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN;
+	ddr2_pdown = pdata->ddr2_pdown;
 	cpuidle_set_statedata(&device->states_usage[1], &davinci_states[1]);
 
 	device->state_count = DAVINCI_CPUIDLE_MAX_STATES;
-- 
1.7.9.5

[PATCH 2/4][V2] davinci: cpuidle - move code to prevent forward declaration

[Daniel Lezcano]

The patch is mindless, it just moves the idle function below in the file
in order to prevent forward declaration in the next patch.

Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Acked-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
---
 arch/arm/mach-davinci/cpuidle.c |   72 +++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index 5fbd470..697febe 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -31,42 +31,6 @@ struct davinci_ops {
 	u32 flags;
 };
 
-/* Actual code that puts the SoC in different idle states */
-static int davinci_enter_idle(struct cpuidle_device *dev,
-				struct cpuidle_driver *drv,
-						int index)
-{
-	struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
-	struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
-
-	if (ops && ops->enter)
-		ops->enter();
-
-	index = cpuidle_wrap_enter(dev,	drv, index,
-				arm_cpuidle_simple_enter);
-
-	if (ops && ops->exit)
-		ops->exit();
-
-	return index;
-}
-
-static struct cpuidle_driver davinci_idle_driver = {
-	.name			= "cpuidle-davinci",
-	.owner			= THIS_MODULE,
-	.en_core_tk_irqen	= 1,
-	.states[0]		= ARM_CPUIDLE_WFI_STATE,
-	.states[1]		= {
-		.enter			= davinci_enter_idle,
-		.exit_latency		= 10,
-		.target_residency	= 100000,
-		.flags			= CPUIDLE_FLAG_TIME_VALID,
-		.name			= "DDR SR",
-		.desc			= "WFI and DDR Self Refresh",
-	},
-	.state_count = DAVINCI_CPUIDLE_MAX_STATES,
-};
-
 static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
 static void __iomem *ddr2_reg_base;
 static bool ddr2_pdown;
@@ -107,6 +71,42 @@ static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
 	},
 };
 
+/* Actual code that puts the SoC in different idle states */
+static int davinci_enter_idle(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv,
+						int index)
+{
+	struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
+	struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
+
+	if (ops && ops->enter)
+		ops->enter();
+
+	index = cpuidle_wrap_enter(dev,	drv, index,
+				arm_cpuidle_simple_enter);
+
+	if (ops && ops->exit)
+		ops->exit();
+
+	return index;
+}
+
+static struct cpuidle_driver davinci_idle_driver = {
+	.name			= "cpuidle-davinci",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
+	.states[0]		= ARM_CPUIDLE_WFI_STATE,
+	.states[1]		= {
+		.enter			= davinci_enter_idle,
+		.exit_latency		= 10,
+		.target_residency	= 100000,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "DDR SR",
+		.desc			= "WFI and DDR Self Refresh",
+	},
+	.state_count = DAVINCI_CPUIDLE_MAX_STATES,
+};
+
 static int __init davinci_cpuidle_probe(struct platform_device *pdev)
 {
 	int ret;
-- 
1.7.9.5

[PATCH 3/4][V2] davinci: cpuidle - remove the ops

[Daniel Lezcano]

With one function handling the idle state and a single variable,
the usage of the davinci_ops is overkill.

This patch removes these ops and simplify the code.

Furthermore, the 'driver_data' field is no longer used, we have
1 of the 3 remaining user of this field removed.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Sekhar Nori <nsekhar@ti.com>
---
 arch/arm/mach-davinci/cpuidle.c |   33 ++-------------------------------
 1 file changed, 2 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index 697febe..5e430bf 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -25,12 +25,6 @@
 
 #define DAVINCI_CPUIDLE_MAX_STATES	2
 
-struct davinci_ops {
-	void (*enter) (void);
-	void (*exit) (void);
-	u32 flags;
-};
-
 static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
 static void __iomem *ddr2_reg_base;
 static bool ddr2_pdown;
@@ -54,39 +48,17 @@ static void davinci_save_ddr_power(int enter, bool pdown)
 	__raw_writel(val, ddr2_reg_base + DDR2_SDRCR_OFFSET);
 }
 
-static void davinci_c2state_enter(void)
-{
-	davinci_save_ddr_power(1, ddr2_pdown);
-}
-
-static void davinci_c2state_exit(void)
-{
-	davinci_save_ddr_power(0, ddr2_pdown);
-}
-
-static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
-	[1] = {
-		.enter	= davinci_c2state_enter,
-		.exit	= davinci_c2state_exit,
-	},
-};
-
 /* Actual code that puts the SoC in different idle states */
 static int davinci_enter_idle(struct cpuidle_device *dev,
 				struct cpuidle_driver *drv,
 						int index)
 {
-	struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
-	struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
-
-	if (ops && ops->enter)
-		ops->enter();
+	davinci_save_ddr_power(1, ddr2_pdown);
 
 	index = cpuidle_wrap_enter(dev,	drv, index,
 				arm_cpuidle_simple_enter);
 
-	if (ops && ops->exit)
-		ops->exit();
+	davinci_save_ddr_power(0, ddr2_pdown);
 
 	return index;
 }
@@ -123,7 +95,6 @@ static int __init davinci_cpuidle_probe(struct platform_device *pdev)
 	ddr2_reg_base = pdata->ddr2_ctlr_base;
 
 	ddr2_pdown = pdata->ddr2_pdown;
-	cpuidle_set_statedata(&device->states_usage[1], &davinci_states[1]);
 
 	device->state_count = DAVINCI_CPUIDLE_MAX_STATES;
 
-- 
1.7.9.5

[PATCH 4/4][V2] davinci: cpuidle - remove useless initialization

[Daniel Lezcano]

The device->state_count is initialized in the cpuidle_register_device
function.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Sekhar Nori <nsekhar@ti.com>
---
 arch/arm/mach-davinci/cpuidle.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index 5e430bf..5ac9e93 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -96,8 +96,6 @@ static int __init davinci_cpuidle_probe(struct platform_device *pdev)
 
 	ddr2_pdown = pdata->ddr2_pdown;
 
-	device->state_count = DAVINCI_CPUIDLE_MAX_STATES;
-
 	ret = cpuidle_register_driver(&davinci_idle_driver);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register driver\n");
-- 
1.7.9.5

Re: [PATCH 0/4][V2] davinci: cpuidle - some cleanups

[Len Brown]

refreshed series applied.

thanks,
Len Brown, Intel Open Source Technology Center