[PATCH v5 7/8] drivers: cpuidle: initialize Exynos driver through DT

[Lorenzo Pieralisi]

On Wed, Jun 25, 2014 at 04:13:33PM +0100, Mark Rutland wrote:
> On Wed, Jun 25, 2014 at 03:10:20PM +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 Exynos CPUidle driver to dynamically
> > initialize idle states data through the updated device tree source
> > files.
> > 
> > Cc: Kukjin Kim <kgene.kim at samsung.com>
> > Cc: Tomasz Figa <t.figa at samsung.com>
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi at arm.com>
> > ---
> > Compile tested, I am not sure I patched the right dts files, please check.
> > 
> >  .../devicetree/bindings/arm/exynos/idle-states.txt | 27 ++++++++++++++++++++
> >  arch/arm/boot/dts/exynos3250.dtsi                  | 16 ++++++++++++
> >  arch/arm/boot/dts/exynos5250.dtsi                  | 15 +++++++++++
> >  arch/arm/boot/dts/exynos5410.dtsi                  | 17 +++++++++++++
> >  drivers/cpuidle/Kconfig.arm                        |  1 +
> >  drivers/cpuidle/cpuidle-exynos.c                   | 29 +++++++++++++---------
> >  6 files changed, 93 insertions(+), 12 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/arm/exynos/idle-states.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/exynos/idle-states.txt b/Documentation/devicetree/bindings/arm/exynos/idle-states.txt
> > new file mode 100644
> > index 0000000..342ecb4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/exynos/idle-states.txt
> > @@ -0,0 +1,27 @@
> > +idle-states node
> > +----------------
> > +
> > +On Exynos 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 Exynos
> > +platforms can be one of:
> > +
> > +	- "samsung,exynos"
> 
> Similarly to the TC2 binding, what does this mean?
> 
> What is a kernel expected to do when it sees this entry-method?
> 
> Using "samsung,exynos" as the entry-method feels like something that's
> going to bite us; it sounds far too wide-reaching.

Same story as TC2, it adds nothing to the patch, it is just there for
compliance with current DT bindings, but useless and will disappear.

> >  static int exynos_cpuidle_probe(struct platform_device *pdev)
> >  {
> > -	int ret;
> > +	int ret, i;
> > +	struct cpuidle_driver *drv = &exynos_idle_driver;
> >  
> >  	exynos_enter_aftr = (void *)(pdev->dev.platform_data);
> >  
> > -	ret = cpuidle_register(&exynos_idle_driver, NULL);
> > +	drv->cpumask = (struct cpumask *) cpu_possible_mask;
> 
> This assignment looks scary to me. Why do we need to do this, and why
> are we throwing away the constness of cpu_possible_mask?

Yes, that's how it is done in CPUidle core if the idle driver does not
initialize cpumask pointer, I guess it is to save some bytes, but I agree
with you, I do not like that either, I will allocate the mask and copy.

Thanks,
Lorenzo