On Wed, Jun 25, 2014 at 04:06:23PM +0100, Mark Rutland wrote:
> On Wed, Jun 25, 2014 at 03:10:19PM +0100, Lorenzo Pieralisi wrote:
> > With the introduction of DT based idle states, CPUidle drivers for ARM
> > can now initialize idle states data through properties in the device tree.
> >
> > This patch adds code to the big.LITTLE CPUidle driver to dynamically
> > initialize idle states data through the updated device tree source file.
> >
> > Cc: Chander Kashyap <chander.kashyap@xxxxxxxxxx>
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> > ---
> > Documentation/devicetree/bindings/arm/vexpress.txt | 25 +++++++++++++
> > arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts | 25 +++++++++++++
> > drivers/cpuidle/Kconfig.arm | 1 +
> > drivers/cpuidle/cpuidle-big_little.c | 43 +++++++++++-----------
> > 4 files changed, 73 insertions(+), 21 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/vexpress.txt b/Documentation/devicetree/bindings/arm/vexpress.txt
> > index 39844cd..7f52ac8 100644
> > --- a/Documentation/devicetree/bindings/arm/vexpress.txt
> > +++ b/Documentation/devicetree/bindings/arm/vexpress.txt
> > @@ -67,6 +67,28 @@ with device_type = "cpu" property for every available core, eg.:
> > };
> > };
> >
> > +idle-states node
> > +----------------
> > +
> > +On Versatile Express platforms with power management capabilities, the device
> > +tree source file must contain the idle-states node[1]. As defined in [1] the
> > +idle-states node must contain an entry-method property that for Versatile
> > +Express platforms can be one of:
> > +
> > + - "arm,vexpress-v2p-ca15_a7"
>
> ... and what does this mean? It's the name we've assigned the platform
> in the Linux DT bindings, but this binding document tells me nothing
> about how this method works.
Ok, I should have omitted it, I added it to make it compliant with
current DT bindings where entry-method for idle-states is required and I
have just added TC2 compatible string to get code out for review.
I should have made the entry-method optional for arm32 and get rid of
this useless binding and entry-method string.
Thanks,
Lorenzo
> This feels like leaking Linux internals rather than a reusable
> interface.
>
> Mark.
>
> > +
> > +Versatile Express idle-states nodes example:
> > +
> > + idle-states {
> > + entry-method = "arm,vexpress-v2p-ca15_a7";
> > +
> > + cluster-sleep-0 {
> > + compatible = "arm,idle-state";
> > + entry-latency-us = <1000>;
> > + exit-latency-us = <700>;
> > + min-residency-us = <3500>;
> > + };
> > + };
> >
> > Configuration infrastructure
> > ----------------------------
> > @@ -227,3 +249,6 @@ Example of a VE tile description (simplified)
> > };
> > };
> >
> > +
> > +[1] ARM Linux Kernel documentation - Idle states bindings
> > + Documentation/devicetree/bindings/arm/idle-states.txt
> > diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
> > index a25c262..ad28242 100644
> > --- a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
> > +++ b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
> > @@ -33,11 +33,32 @@
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > + idle-states {
> > + entry-method = "arm,vexpress-v2p-ca15_a7";
> > +
> > + CLUSTER_SLEEP_BIG: cluster-sleep-big {
> > + compatible = "arm,idle-state";
> > + power-rank = <0>;
> > + entry-latency-us = <1000>;
> > + exit-latency-us = <700>;
> > + min-residency-us = <3500>;
> > + };
> > +
> > + CLUSTER_SLEEP_LITTLE: cluster-sleep-little {
> > + compatible = "arm,idle-state";
> > + power-rank = <0>;
> > + entry-latency-us = <1000>;
> > + exit-latency-us = <500>;
> > + min-residency-us = <3000>;
> > + };
> > + };
> > +
> > cpu0: cpu@0 {
> > device_type = "cpu";
> > compatible = "arm,cortex-a15";
> > reg = <0>;
> > cci-control-port = <&cci_control1>;
> > + cpu-idle-states = <&CLUSTER_SLEEP_BIG>;
> > };
> >
> > cpu1: cpu@1 {
> > @@ -45,6 +66,7 @@
> > compatible = "arm,cortex-a15";
> > reg = <1>;
> > cci-control-port = <&cci_control1>;
> > + cpu-idle-states = <&CLUSTER_SLEEP_BIG>;
> > };
> >
> > cpu2: cpu@2 {
> > @@ -52,6 +74,7 @@
> > compatible = "arm,cortex-a7";
> > reg = <0x100>;
> > cci-control-port = <&cci_control2>;
> > + cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
> > };
> >
> > cpu3: cpu@3 {
> > @@ -59,6 +82,7 @@
> > compatible = "arm,cortex-a7";
> > reg = <0x101>;
> > cci-control-port = <&cci_control2>;
> > + cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
> > };
> >
> > cpu4: cpu@4 {
> > @@ -66,6 +90,7 @@
> > compatible = "arm,cortex-a7";
> > reg = <0x102>;
> > cci-control-port = <&cci_control2>;
> > + cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
> > };
> > };
> >
> > diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> > index b6d69e8..a9b089c 100644
> > --- a/drivers/cpuidle/Kconfig.arm
> > +++ b/drivers/cpuidle/Kconfig.arm
> > @@ -12,6 +12,7 @@ config ARM_BIG_LITTLE_CPUIDLE
> > depends on ARCH_VEXPRESS_TC2_PM
> > select ARM_CPU_SUSPEND
> > select CPU_IDLE_MULTIPLE_DRIVERS
> > + select DT_IDLE_STATES
> > help
> > Select this option to enable CPU idle driver for big.LITTLE based
> > ARM systems. Driver manages CPUs coordination through MCPM and
> > diff --git a/drivers/cpuidle/cpuidle-big_little.c b/drivers/cpuidle/cpuidle-big_little.c
> > index b45fc62..6712441 100644
> > --- a/drivers/cpuidle/cpuidle-big_little.c
> > +++ b/drivers/cpuidle/cpuidle-big_little.c
> > @@ -24,6 +24,8 @@
> > #include <asm/smp_plat.h>
> > #include <asm/suspend.h>
> >
> > +#include "dt_idle_states.h"
> > +
> > static int bl_enter_powerdown(struct cpuidle_device *dev,
> > struct cpuidle_driver *drv, int idx);
> >
> > @@ -61,32 +63,12 @@ static struct cpuidle_driver bl_idle_little_driver = {
> > .name = "little_idle",
> > .owner = THIS_MODULE,
> > .states[0] = ARM_CPUIDLE_WFI_STATE,
> > - .states[1] = {
> > - .enter = bl_enter_powerdown,
> > - .exit_latency = 700,
> > - .target_residency = 2500,
> > - .flags = CPUIDLE_FLAG_TIME_VALID |
> > - CPUIDLE_FLAG_TIMER_STOP,
> > - .name = "C1",
> > - .desc = "ARM little-cluster power down",
> > - },
> > - .state_count = 2,
> > };
> >
> > static struct cpuidle_driver bl_idle_big_driver = {
> > .name = "big_idle",
> > .owner = THIS_MODULE,
> > .states[0] = ARM_CPUIDLE_WFI_STATE,
> > - .states[1] = {
> > - .enter = bl_enter_powerdown,
> > - .exit_latency = 500,
> > - .target_residency = 2000,
> > - .flags = CPUIDLE_FLAG_TIME_VALID |
> > - CPUIDLE_FLAG_TIMER_STOP,
> > - .name = "C1",
> > - .desc = "ARM big-cluster power down",
> > - },
> > - .state_count = 2,
> > };
> >
> > /*
> > @@ -165,7 +147,8 @@ static int __init bl_idle_driver_init(struct cpuidle_driver *drv, int cpu_id)
> >
> > static int __init bl_idle_init(void)
> > {
> > - int ret;
> > + int ret, i;
> > + struct cpuidle_driver *drv;
> >
> > /*
> > * Initialize the driver just for a compliant set of machines
> > @@ -187,6 +170,24 @@ static int __init bl_idle_init(void)
> > if (ret)
> > goto out_uninit_little;
> >
> > + /* Start at index 1, index 0 standard WFI */
> > + ret = dt_init_idle_driver(&bl_idle_big_driver, NULL, 1, false);
> > + if (ret)
> > + goto out_uninit_big;
> > +
> > + /* Start at index 1, index 0 standard WFI */
> > + ret = dt_init_idle_driver(&bl_idle_little_driver, NULL, 1, false);
> > + if (ret)
> > + goto out_uninit_big;
> > +
> > + drv = &bl_idle_big_driver;
> > + for (i = 1; i < drv->state_count; i++)
> > + drv->states[i].enter = bl_enter_powerdown;
> > +
> > + drv = &bl_idle_little_driver;
> > + for (i = 1; i < drv->state_count; i++)
> > + drv->states[i].enter = bl_enter_powerdown;
> > +
> > ret = cpuidle_register(&bl_idle_little_driver, NULL);
> > if (ret)
> > goto out_uninit_big;
> > --
> > 1.9.1
> >
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
