[PATCH v4 0/4] cpufreq-dt: add suspend frequency support

[PATCH v4 0/4] cpufreq-dt: add suspend frequency support

[Bartlomiej Zolnierkiewicz]

Hi,

This patch series adds suspend frequency support (using opp-v2
bindings and suspend-opp functionality) to cpufreq-dt driver and
then adds suspend opp for Exynos4412 based boards.

This patch series fixes suspend/resume support on Exynos4412
based Trats2 board and reboot hang on Exynos4412 based Odroid
U3 board.

Changes since v3:
- fixed dev_pm_opp_get_suspend_opp() locking
- shortened variable name in dev_pm_opp_get_suspend_opp()
- adjusted cpufreq_generic_suspend() to work with cpufreq-dt
- removed no longer needed cpufreq_dt_suspend()
- added Acked-by tag from Viresh to patch #4

Changes since v2:
- rewrote to use suspend-opp functionality

Changes since v1:
- removed superfluous ";"

Depends on:
- next-20150902 branch of linux-next kernel tree

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


Bartlomiej Zolnierkiewicz (4):
  PM / OPP: add dev_pm_opp_get_suspend_opp() helper
  cpufreq: allow cpufreq_generic_suspend() to work without suspend
    frequency
  cpufreq-dt: add suspend frequency support
  ARM: dts: add suspend opp to exynos4412

 arch/arm/boot/dts/exynos4412.dtsi |  1 +
 drivers/base/power/opp.c          | 30 ++++++++++++++++++++++++++++++
 drivers/cpufreq/cpufreq-dt.c      | 11 +++++++++++
 drivers/cpufreq/cpufreq.c         |  4 ++--
 include/linux/pm_opp.h            |  6 ++++++
 5 files changed, 50 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH v4 1/4] PM / OPP: add dev_pm_opp_get_suspend_opp() helper

[Bartlomiej Zolnierkiewicz]

Add dev_pm_opp_get_suspend_opp() helper to obtain suspend opp.

Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Thomas Abraham <thomas.ab@samsung.com>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/base/power/opp.c | 30 ++++++++++++++++++++++++++++++
 include/linux/pm_opp.h   |  6 ++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index eb25449..3d948ea 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -341,6 +341,36 @@ unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev)
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_clock_latency);
 
 /**
+ * dev_pm_opp_get_suspend_opp() - Get suspend opp
+ * @dev:	device for which we do this operation
+ *
+ * Return: This function returns pointer to the suspend opp if it is
+ * defined, otherwise it returns NULL.
+ *
+ * Locking: This function must be called under rcu_read_lock(). opp is a rcu
+ * protected pointer. The reason for the same is that the opp pointer which is
+ * returned will remain valid for use with opp_get_{voltage, freq} only while
+ * under the locked area. The pointer returned must be used prior to unlocking
+ * with rcu_read_unlock() to maintain the integrity of the pointer.
+ */
+struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev)
+{
+	struct device_opp *dev_opp;
+	struct dev_pm_opp *opp;
+
+	opp_rcu_lockdep_assert();
+
+	dev_opp = _find_device_opp(dev);
+	if (IS_ERR(dev_opp))
+		opp = NULL;
+	else
+		opp = dev_opp->suspend_opp;
+
+	return opp;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp);
+
+/**
  * dev_pm_opp_get_opp_count() - Get number of opps available in the opp list
  * @dev:	device for which we do this operation
  *
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index cab7ba5..e817722 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -34,6 +34,7 @@ bool dev_pm_opp_is_turbo(struct dev_pm_opp *opp);
 
 int dev_pm_opp_get_opp_count(struct device *dev);
 unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev);
+struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev);
 
 struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
 					      unsigned long freq,
@@ -80,6 +81,11 @@ static inline unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev)
 	return 0;
 }
 
+static inline struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev)
+{
+	return NULL;
+}
+
 static inline struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
 					unsigned long freq, bool available)
 {
-- 
1.9.1

[PATCH v4 2/4] cpufreq: allow cpufreq_generic_suspend() to work without suspend frequency

[Bartlomiej Zolnierkiewicz]

Some cpufreq drivers may set suspend frequency only for
selected setups but still would like to use the generic
suspend handler.  Thus don't treat !policy->suspend_freq
condition as an incorrect one.

Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Thomas Abraham <thomas.ab@samsung.com>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/cpufreq/cpufreq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index b3d9368..a634fcb 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1626,8 +1626,8 @@ int cpufreq_generic_suspend(struct cpufreq_policy *policy)
 	int ret;
 
 	if (!policy->suspend_freq) {
-		pr_err("%s: suspend_freq can't be zero\n", __func__);
-		return -EINVAL;
+		pr_debug("%s: suspend_freq not defined\n", __func__);
+		return 0;
 	}
 
 	pr_debug("%s: Setting suspend-freq: %u\n", __func__,
-- 
1.9.1

[PATCH v4 3/4] cpufreq-dt: add suspend frequency support

[Bartlomiej Zolnierkiewicz]

Add suspend frequency support and if needed set it to
the frequency obtained from the suspend opp (can be defined
using opp-v2 bindings and is optional).

Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Thomas Abraham <thomas.ab@samsung.com>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/cpufreq/cpufreq-dt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index c3583cd..e08ae40 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -196,6 +196,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 	struct device *cpu_dev;
 	struct regulator *cpu_reg;
 	struct clk *cpu_clk;
+	struct dev_pm_opp *suspend_opp;
 	unsigned long min_uV = ~0, max_uV = 0;
 	unsigned int transition_latency;
 	bool need_update = false;
@@ -329,6 +330,13 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 	policy->driver_data = priv;
 
 	policy->clk = cpu_clk;
+
+	rcu_read_lock();
+	suspend_opp = dev_pm_opp_get_suspend_opp(cpu_dev);
+	if (suspend_opp)
+		policy->suspend_freq = dev_pm_opp_get_freq(suspend_opp) / 1000;
+	rcu_read_unlock();
+
 	ret = cpufreq_table_validate_and_show(policy, freq_table);
 	if (ret) {
 		dev_err(cpu_dev, "%s: invalid frequency table: %d\n", __func__,
@@ -419,6 +427,9 @@ static struct cpufreq_driver dt_cpufreq_driver = {
 	.ready = cpufreq_ready,
 	.name = "cpufreq-dt",
 	.attr = cpufreq_dt_attr,
+#ifdef CONFIG_PM
+	.suspend = cpufreq_generic_suspend,
+#endif
 };
 
 static int dt_cpufreq_probe(struct platform_device *pdev)
-- 
1.9.1

Re: [PATCH v4 3/4] cpufreq-dt: add suspend frequency support

[Viresh Kumar]

On 07-09-15, 17:41, Bartlomiej Zolnierkiewicz wrote:
> +#ifdef CONFIG_PM
> +	.suspend = cpufreq_generic_suspend,
> +#endif

I don't think there is any need of the #ifdef here.

-- 
viresh

[PATCH v4 4/4] ARM: dts: add suspend opp to exynos4412

[Bartlomiej Zolnierkiewicz]

Mark 800MHz OPP as a suspend opp for Exynos4412 based
boards so effectively cpufreq-dt driver behavior w.r.t.
suspend frequency matches what the old exynos-cpufreq
driver has been doing.

This patch fixes suspend/resume support on Exynos4412 based
Trats2 board and reboot hang on Exynos4412 based Odroid U3
board.

Cc: Thomas Abraham <thomas.ab@samsung.com>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 arch/arm/boot/dts/exynos4412.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
index ca0e3c1..294cfe4 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -98,6 +98,7 @@
 			opp-hz = /bits/ 64 <800000000>;
 			opp-microvolt = <1000000>;
 			clock-latency-ns = <200000>;
+			opp-suspend;
 		};
 		opp07 {
 			opp-hz = /bits/ 64 <900000000>;
-- 
1.9.1

Re: [PATCH v4 4/4] ARM: dts: add suspend opp to exynos4412

[Krzysztof Kozlowski]

On 08.09.2015 00:41, Bartlomiej Zolnierkiewicz wrote:
> Mark 800MHz OPP as a suspend opp for Exynos4412 based
> boards so effectively cpufreq-dt driver behavior w.r.t.
> suspend frequency matches what the old exynos-cpufreq
> driver has been doing.
> 
> This patch fixes suspend/resume support on Exynos4412 based
> Trats2 board and reboot hang on Exynos4412 based Odroid U3
> board.
> 
> Cc: Thomas Abraham <thomas.ab@samsung.com>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  arch/arm/boot/dts/exynos4412.dtsi | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH v4 0/4] cpufreq-dt: add suspend frequency support

[Viresh Kumar]

On 07-09-15, 17:41, Bartlomiej Zolnierkiewicz wrote:
> Hi,
> 
> This patch series adds suspend frequency support (using opp-v2
> bindings and suspend-opp functionality) to cpufreq-dt driver and
> then adds suspend opp for Exynos4412 based boards.
> 
> This patch series fixes suspend/resume support on Exynos4412
> based Trats2 board and reboot hang on Exynos4412 based Odroid
> U3 board.
> 
> Changes since v3:
> - fixed dev_pm_opp_get_suspend_opp() locking
> - shortened variable name in dev_pm_opp_get_suspend_opp()
> - adjusted cpufreq_generic_suspend() to work with cpufreq-dt
> - removed no longer needed cpufreq_dt_suspend()
> - added Acked-by tag from Viresh to patch #4

Just a minor comment on 3/4 and after fixing that:

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v4 0/4] cpufreq-dt: add suspend frequency support

[Krzysztof Kozlowski]

On 08.09.2015 11:46, Viresh Kumar wrote:
> On 07-09-15, 17:41, Bartlomiej Zolnierkiewicz wrote:
>> Hi,
>>
>> This patch series adds suspend frequency support (using opp-v2
>> bindings and suspend-opp functionality) to cpufreq-dt driver and
>> then adds suspend opp for Exynos4412 based boards.
>>
>> This patch series fixes suspend/resume support on Exynos4412
>> based Trats2 board and reboot hang on Exynos4412 based Odroid
>> U3 board.
>>
>> Changes since v3:
>> - fixed dev_pm_opp_get_suspend_opp() locking
>> - shortened variable name in dev_pm_opp_get_suspend_opp()
>> - adjusted cpufreq_generic_suspend() to work with cpufreq-dt
>> - removed no longer needed cpufreq_dt_suspend()
>> - added Acked-by tag from Viresh to patch #4
> 
> Just a minor comment on 3/4 and after fixing that:
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Dear Kukjin,

There were no comments from your side for this patchset nor for previous
Marek's fix. After mentioned change and Bart's re-spin, do you plan to
grab this patchset and send it for current v4.3 cycle?

Best regards,
Krzysztof

Re: [PATCH v4 0/4] cpufreq-dt: add suspend frequency support

[Viresh Kumar]

On 08-09-15, 12:56, Krzysztof Kozlowski wrote:
> There were no comments from your side for this patchset nor for previous
> Marek's fix. After mentioned change and Bart's re-spin, do you plan to
> grab this patchset and send it for current v4.3 cycle?

Why do you think it should go via Kukjin's tree and not Rafael's?

-- 
viresh

Re: [PATCH v4 0/4] cpufreq-dt: add suspend frequency support

[Krzysztof Kozlowski]

On 08.09.2015 13:20, Viresh Kumar wrote:
> On 08-09-15, 12:56, Krzysztof Kozlowski wrote:
>> There were no comments from your side for this patchset nor for previous
>> Marek's fix. After mentioned change and Bart's re-spin, do you plan to
>> grab this patchset and send it for current v4.3 cycle?
> 
> Why do you think it should go via Kukjin's tree and not Rafael's?

Somehow my mind stuck on solving Exynos4x12 cpufreq issues.

Right, it should go through Rafael's, probably except DTS patch (4/4)
because it depends on previous DTS changes. These changes are still in
arm-soc, not in Linus' tree [0].

Best regards,
Krzysztof

[0]
https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=next/late&id=f44997412e5b39b6a1231f16555120c5b2aae0ed

Re: [PATCH v4 0/4] cpufreq-dt: add suspend frequency support

[Viresh Kumar]

On 08-09-15, 13:48, Krzysztof Kozlowski wrote:
> Somehow my mind stuck on solving Exynos4x12 cpufreq issues.
> 
> Right, it should go through Rafael's, probably except DTS patch (4/4)
> because it depends on previous DTS changes. These changes are still in
> arm-soc, not in Linus' tree [0].

That's bad :(

Anyway, 4/4 is kind of independent to this series, in the sense that
it wouldn't harm even if it gets applied separately.

So, go ahead and apply 4/4 to Samsung's tree and rest of this can be
picked by Rafael.

-- 
viresh

Re: [PATCH v4 0/4] cpufreq-dt: add suspend frequency support

[Kukjin Kim]

On 09/08/15 14:10, Viresh Kumar wrote:
> On 08-09-15, 13:48, Krzysztof Kozlowski wrote:
>> Somehow my mind stuck on solving Exynos4x12 cpufreq issues.
>>
>> Right, it should go through Rafael's, probably except DTS patch (4/4)
>> because it depends on previous DTS changes. These changes are still in
>> arm-soc, not in Linus' tree [0].
> 
> That's bad :(
> 
> Anyway, 4/4 is kind of independent to this series, in the sense that
> it wouldn't harm even if it gets applied separately.
> 
> So, go ahead and apply 4/4 to Samsung's tree and rest of this can be
> picked by Rafael.
> 
Applied 4/4 into samsung tree, thanks.

- Kukjin