Re: [RFC PATCH 2/2] ARM: EXYNOS: Make sure that the Exynos5420 MDMA0 clock is enabled during suspend
Hello Chanwoo, Thanks a lot for your feedback. On 03/30/2015 04:04 AM, Chanwoo Choi wrote: I faced on the similiar issue. If some clock was disabled, Exynos SoC could not enter the suspend mode But, I think it is not prpper method to resolve this issue. about that that specific clock (e.g., mdma0) is handled in this dirver. Also, ARM-64bit don't include any '../mach-exynos' directory. IMHO, some clock issue have to be handled in SoC clk driver or others. Ok, Abhilash also said that before so I just posted another RFC: [RFC PATCH v3 0/2] ARM: EXYNOS: Fix Suspend-to-RAM on Exynos5420 Please let me know if that is more aligned to what you were thinking as the proper solution. Best Regards, Chanwoo Choi Best regards, Javier -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] ARM: EXYNOS: Make sure that the Exynos5420 MDMA0 clock is enabled during suspend
Hi Javier,
On 03/27/2015 11:21 PM, Javier Martinez Canillas wrote:
Commit ae43b3289186 (ARM: 8202/1: dmaengine: pl330: Add runtime Power
Management support v12) added pm support for the pl330 dma driver but
it makes the clock for the Exynos5420 MDMA0 DMA controller to be gated
during suspend and this clock needs to remain enabled in order to make
the system resume from a system suspend state.
To make sure that the clock is enabled during suspend, enable it prior
to entering a suspend state and disable it once the system has resumed.
Thanks to Abhilash Kesavan for figuring out that this was the issue.
Fixes: ae43b32 (ARM: 8202/1: dmaengine: pl330: Add runtime Power Management
support v12)
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
arch/arm/mach-exynos/suspend.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index 1521eaf99265..6dbc0a6d1bb5 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -16,6 +16,7 @@
#include linux/init.h
#include linux/suspend.h
#include linux/syscore_ops.h
+#include linux/clk.h
#include linux/cpu_pm.h
#include linux/io.h
#include linux/irq.h
@@ -79,6 +80,7 @@ static const struct exynos_pm_data *pm_data;
static int exynos5420_cpu_state;
static unsigned int exynos_pmu_spare3;
+static struct clk *clk;
/*
* GIC wake-up support
@@ -374,6 +376,16 @@ static void exynos5420_pm_prepare(void)
{
unsigned int tmp;
+ /*
+ * Exynos5420 requires the MDMA0 controller clock to be
+ * ungated on suspend in order to be resumed correctly.
+ */
+ clk = clk_get(NULL, mdma0);
+ if (IS_ERR(clk))
+ pr_warn(Failed to get mdma0 clk (%ld)\n, PTR_ERR(clk));
+ else
+ clk_prepare_enable(clk);
+
I faced on the similiar issue. If some clock was disabled,
Exynos SoC could not enter the suspend mode
But, I think it is not prpper method to resolve this issue.
about that that specific clock (e.g., mdma0) is handled in this dirver.
Also, ARM-64bit don't include any '../mach-exynos' directory.
IMHO, some clock issue have to be handled in SoC clk driver or others.
[snip]
Best Regards,
Chanwoo Choi
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] ARM: EXYNOS: Make sure that the Exynos5420 MDMA0 clock is enabled during suspend
2015-03-27 15:21 GMT+01:00 Javier Martinez Canillas
javier.marti...@collabora.co.uk:
Commit ae43b3289186 (ARM: 8202/1: dmaengine: pl330: Add runtime Power
Management support v12) added pm support for the pl330 dma driver but
it makes the clock for the Exynos5420 MDMA0 DMA controller to be gated
during suspend and this clock needs to remain enabled in order to make
the system resume from a system suspend state.
To make sure that the clock is enabled during suspend, enable it prior
to entering a suspend state and disable it once the system has resumed.
Thanks to Abhilash Kesavan for figuring out that this was the issue.
Fixes: ae43b32 (ARM: 8202/1: dmaengine: pl330: Add runtime Power Management
support v12)
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
Hmmm... isn't a real problem elsewhere - like some clock hierarchy is
wrong or incomplete? This rather looks like workaround for the
problem.
---
arch/arm/mach-exynos/suspend.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index 1521eaf99265..6dbc0a6d1bb5 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -16,6 +16,7 @@
#include linux/init.h
#include linux/suspend.h
#include linux/syscore_ops.h
+#include linux/clk.h
#include linux/cpu_pm.h
#include linux/io.h
#include linux/irq.h
@@ -79,6 +80,7 @@ static const struct exynos_pm_data *pm_data;
static int exynos5420_cpu_state;
static unsigned int exynos_pmu_spare3;
+static struct clk *clk;
/*
* GIC wake-up support
@@ -374,6 +376,16 @@ static void exynos5420_pm_prepare(void)
{
unsigned int tmp;
+ /*
+* Exynos5420 requires the MDMA0 controller clock to be
+* ungated on suspend in order to be resumed correctly.
+*/
+ clk = clk_get(NULL, mdma0);
+ if (IS_ERR(clk))
+ pr_warn(Failed to get mdma0 clk (%ld)\n, PTR_ERR(clk));
+ else
+ clk_prepare_enable(clk);
+
/* Set wake-up mask registers */
exynos_pm_set_wakeup_mask();
@@ -516,6 +528,9 @@ static void exynos5420_pm_resume(void)
{
unsigned long tmp;
+ if (!IS_ERR_OR_NULL(clk))
+ clk_disable_unprepare(clk);
+
Missing clk_put()
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] ARM: EXYNOS: Make sure that the Exynos5420 MDMA0 clock is enabled during suspend
Hello Javier,
On 27/03/15 15:21, Javier Martinez Canillas wrote:
Commit ae43b3289186 (ARM: 8202/1: dmaengine: pl330: Add runtime Power
Management support v12) added pm support for the pl330 dma driver but
it makes the clock for the Exynos5420 MDMA0 DMA controller to be gated
during suspend and this clock needs to remain enabled in order to make
the system resume from a system suspend state.
To make sure that the clock is enabled during suspend, enable it prior
to entering a suspend state and disable it once the system has resumed.
Thanks to Abhilash Kesavan for figuring out that this was the issue.
Fixes: ae43b32 (ARM: 8202/1: dmaengine: pl330: Add runtime Power Management
support v12)
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
arch/arm/mach-exynos/suspend.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index 1521eaf99265..6dbc0a6d1bb5 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -16,6 +16,7 @@
#include linux/init.h
#include linux/suspend.h
#include linux/syscore_ops.h
+#include linux/clk.h
#include linux/cpu_pm.h
#include linux/io.h
#include linux/irq.h
@@ -79,6 +80,7 @@ static const struct exynos_pm_data *pm_data;
static int exynos5420_cpu_state;
static unsigned int exynos_pmu_spare3;
+static struct clk *clk;
/*
* GIC wake-up support
@@ -374,6 +376,16 @@ static void exynos5420_pm_prepare(void)
{
unsigned int tmp;
+ /*
+ * Exynos5420 requires the MDMA0 controller clock to be
+ * ungated on suspend in order to be resumed correctly.
+ */
+ clk = clk_get(NULL, mdma0);
+ if (IS_ERR(clk))
+ pr_warn(Failed to get mdma0 clk (%ld)\n, PTR_ERR(clk));
I suppose you want this clk_get() call in exynos_pm_init(), now there
is clk_put() missing and this will cause a memory leak.
+ else
+ clk_prepare_enable(clk);
+
/* Set wake-up mask registers */
exynos_pm_set_wakeup_mask();
@@ -516,6 +528,9 @@ static void exynos5420_pm_resume(void)
{
unsigned long tmp;
+ if (!IS_ERR_OR_NULL(clk))
This should be just IS_ERR().
+ clk_disable_unprepare(clk);
+
/* Restore the CPU0 low power state register */
tmp = pmu_raw_readl(EXYNOS5_ARM_CORE0_SYS_PWR_REG);
pmu_raw_writel(tmp | S5P_CORE_LOCAL_PWR_EN,
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] ARM: EXYNOS: Make sure that the Exynos5420 MDMA0 clock is enabled during suspend
Hello Sylwester,
Thanks a lot for your feedback.
On 03/27/2015 03:36 PM, Sylwester Nawrocki wrote:
* GIC wake-up support
@@ -374,6 +376,16 @@ static void exynos5420_pm_prepare(void)
{
unsigned int tmp;
+/*
+ * Exynos5420 requires the MDMA0 controller clock to be
+ * ungated on suspend in order to be resumed correctly.
+ */
+clk = clk_get(NULL, mdma0);
+if (IS_ERR(clk))
+pr_warn(Failed to get mdma0 clk (%ld)\n, PTR_ERR(clk));
I suppose you want this clk_get() call in exynos_pm_init(), now there
Well I wanted to do it in an exynos5420 specific function to avoid
having an of_machine_is_compatible(samsung,exynos5420) but I can
move there if that is preferred.
is clk_put() missing and this will cause a memory leak.
duh, I wonder how I missed that. Thanks for pointing it out.
+else
+clk_prepare_enable(clk);
+
/* Set wake-up mask registers */
exynos_pm_set_wakeup_mask();
@@ -516,6 +528,9 @@ static void exynos5420_pm_resume(void)
{
unsigned long tmp;
+if (!IS_ERR_OR_NULL(clk))
This should be just IS_ERR().
Ok.
+clk_disable_unprepare(clk);
+
/* Restore the CPU0 low power state register */
tmp = pmu_raw_readl(EXYNOS5_ARM_CORE0_SYS_PWR_REG);
pmu_raw_writel(tmp | S5P_CORE_LOCAL_PWR_EN,
--
Regards,
Sylwester
Best regards,
Javier
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] ARM: EXYNOS: Make sure that the Exynos5420 MDMA0 clock is enabled during suspend
Hello Krzysztof,
Thanks a lot for your feedback.
On 03/27/2015 03:38 PM, Krzysztof Kozlowski wrote:
2015-03-27 15:21 GMT+01:00 Javier Martinez Canillas
javier.marti...@collabora.co.uk:
Commit ae43b3289186 (ARM: 8202/1: dmaengine: pl330: Add runtime Power
Management support v12) added pm support for the pl330 dma driver but
it makes the clock for the Exynos5420 MDMA0 DMA controller to be gated
during suspend and this clock needs to remain enabled in order to make
the system resume from a system suspend state.
To make sure that the clock is enabled during suspend, enable it prior
to entering a suspend state and disable it once the system has resumed.
Thanks to Abhilash Kesavan for figuring out that this was the issue.
Fixes: ae43b32 (ARM: 8202/1: dmaengine: pl330: Add runtime Power Management
support v12)
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
Hmmm... isn't a real problem elsewhere - like some clock hierarchy is
wrong or incomplete? This rather looks like workaround for the
problem.
That's a very good question, that's why I sent it as an RFC. I didn't find
anything relevant in the manual but I could certainly be missing something.
@@ -516,6 +528,9 @@ static void exynos5420_pm_resume(void)
{
unsigned long tmp;
+ if (!IS_ERR_OR_NULL(clk))
+ clk_disable_unprepare(clk);
+
Missing clk_put()
Yes, Sylwester already pointed it out. Sorry for missing that.
Best regards,
Krzysztof
Best regards,
Javier
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
