From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Rutland Subject: Re: [PATCH v5 7/8] drivers: cpuidle: initialize Exynos driver through DT Date: Wed, 25 Jun 2014 16:13:33 +0100 Message-ID: <20140625151333.GE15240@leverpostej> References: <1403705421-17597-1-git-send-email-lorenzo.pieralisi@arm.com> <1403705421-17597-8-git-send-email-lorenzo.pieralisi@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1403705421-17597-8-git-send-email-lorenzo.pieralisi@arm.com> Sender: linux-pm-owner@vger.kernel.org To: Lorenzo Pieralisi Cc: "linux-arm-kernel@lists.infradead.org" , "linux-pm@vger.kernel.org" , "devicetree@vger.kernel.org" , Kukjin Kim , Tomasz Figa , Sudeep Holla , Catalin Marinas , Charles Garcia-Tobin , Nicolas Pitre , Rob Herring , "grant.likely@linaro.org" , Peter De Schrijver , Santosh Shilimkar , Daniel Lezcano , Amit Kucheria , Vincent Guittot , Antti Miettinen , Stephen Boyd , Kevin Hilman , Sebastian Capella List-Id: devicetree@vger.kernel.org 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 > Cc: Tomasz Figa > Signed-off-by: Lorenzo Pieralisi > --- > 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. > 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? Mark. From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.rutland@arm.com (Mark Rutland) Date: Wed, 25 Jun 2014 16:13:33 +0100 Subject: [PATCH v5 7/8] drivers: cpuidle: initialize Exynos driver through DT In-Reply-To: <1403705421-17597-8-git-send-email-lorenzo.pieralisi@arm.com> References: <1403705421-17597-1-git-send-email-lorenzo.pieralisi@arm.com> <1403705421-17597-8-git-send-email-lorenzo.pieralisi@arm.com> Message-ID: <20140625151333.GE15240@leverpostej> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 > Cc: Tomasz Figa > Signed-off-by: Lorenzo Pieralisi > --- > 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. > 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? Mark.