[v3,2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions

Commit Message

Javier Martinez Canillas Oct. 20, 2014, 9:05 p.m. UTC

The regulator framework has a set of helpers functions to be used when
the system is entering and leaving from suspend but these are not called
on Exynos platforms. This means that the .set_suspend_* function handlers
defined by regulator drivers are not called when the system is suspended.

Suggested-by: Doug Anderson <[e-mail]>
Signed-off-by: Javier Martinez Canillas <[e-mail]>
Reviewed-by: Doug Anderson <[e-mail]>
---
 arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Doug Anderson Oct. 20, 2014, 9:40 p.m. UTC | #1

Javier,

On Mon, Oct 20, 2014 at 2:05 PM, Javier Martinez Canillas
<[e-mail]> wrote:
> The regulator framework has a set of helpers functions to be used when
> the system is entering and leaving from suspend but these are not called
> on Exynos platforms. This means that the .set_suspend_* function handlers
> defined by regulator drivers are not called when the system is suspended.
>
> Suggested-by: Doug Anderson <[e-mail]>
> Signed-off-by: Javier Martinez Canillas <[e-mail]>
> Reviewed-by: Doug Anderson <[e-mail]>

I think you forgot to carry Chanwoo's review:

Reviewed-by: Chanwoo Choi<[e-mail]>

I don't think you've made any changes that would need to invalidate his review.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Javier Martinez Canillas Oct. 20, 2014, 9:47 p.m. UTC | #2

Hello Doug,

On 10/20/2014 11:40 PM, Doug Anderson wrote:
> Javier,
> 
> On Mon, Oct 20, 2014 at 2:05 PM, Javier Martinez Canillas
> <[e-mail]> wrote:
>> The regulator framework has a set of helpers functions to be used when
>> the system is entering and leaving from suspend but these are not called
>> on Exynos platforms. This means that the .set_suspend_* function handlers
>> defined by regulator drivers are not called when the system is suspended.
>>
>> Suggested-by: Doug Anderson <[e-mail]>
>> Signed-off-by: Javier Martinez Canillas <[e-mail]>
>> Reviewed-by: Doug Anderson <[e-mail]>
> 
> I think you forgot to carry Chanwoo's review:
> 
> Reviewed-by: Chanwoo Choi<[e-mail]>
> 
> I don't think you've made any changes that would need to invalidate his review.
> 

Yes, I forgot to add the tag to my local branch so it was missed when generating
the patch for the new version, sorry about that...

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Javier Martinez Canillas Oct. 30, 2014, 10:06 a.m. UTC | #3

Hello Kukjin,

On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas
<[e-mail]> wrote:
> The regulator framework has a set of helpers functions to be used when
> the system is entering and leaving from suspend but these are not called
> on Exynos platforms. This means that the .set_suspend_* function handlers
> defined by regulator drivers are not called when the system is suspended.
>
> Suggested-by: Doug Anderson <[e-mail]>
> Signed-off-by: Javier Martinez Canillas <[e-mail]>
> Reviewed-by: Doug Anderson <[e-mail]>
> ---
>  arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>

Any comments on this patch?

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Javier Martinez Canillas Nov. 11, 2014, 10:23 a.m. UTC | #4

Hello Kukjin,

On Thu, Oct 30, 2014 at 11:06 AM, Javier Martinez Canillas
<[e-mail]> wrote:
> Hello Kukjin,
>
> On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas
> <[e-mail]> wrote:
>> The regulator framework has a set of helpers functions to be used when
>> the system is entering and leaving from suspend but these are not called
>> on Exynos platforms. This means that the .set_suspend_* function handlers
>> defined by regulator drivers are not called when the system is suspended.
>>
>> Suggested-by: Doug Anderson <[e-mail]>
>> Signed-off-by: Javier Martinez Canillas <[e-mail]>
>> Reviewed-by: Doug Anderson <[e-mail]>
>> ---
>>  arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>
> Any comments on this patch?
>

Just a gentle reminder about this patch.

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Olof Johansson Nov. 12, 2014, 5:02 p.m. UTC | #5

On Tue, Nov 11, 2014 at 2:23 AM, Javier Martinez Canillas
<[e-mail]> wrote:
> Hello Kukjin,
>
> On Thu, Oct 30, 2014 at 11:06 AM, Javier Martinez Canillas
> <[e-mail]> wrote:
>> Hello Kukjin,
>>
>> On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas
>> <[e-mail]> wrote:
>>> The regulator framework has a set of helpers functions to be used when
>>> the system is entering and leaving from suspend but these are not called
>>> on Exynos platforms. This means that the .set_suspend_* function handlers
>>> defined by regulator drivers are not called when the system is suspended.
>>>
>>> Suggested-by: Doug Anderson <[e-mail]>
>>> Signed-off-by: Javier Martinez Canillas <[e-mail]>
>>> Reviewed-by: Doug Anderson <[e-mail]>
>>> ---
>>>  arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
>>>  1 file changed, 23 insertions(+)
>>>
>>
>> Any comments on this patch?
>>
>
> Just a gentle reminder about this patch.

Kukjin, should I just apply this directly since you seem to be busy?


Thanks,

-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Kim Kukjin Nov. 13, 2014, 2:12 a.m. UTC | #6

On 11/13/14 02:02, Olof Johansson wrote:
> On Tue, Nov 11, 2014 at 2:23 AM, Javier Martinez Canillas
> <[e-mail]>  wrote:
>> Hello Kukjin,
>>
>> On Thu, Oct 30, 2014 at 11:06 AM, Javier Martinez Canillas
>> <[e-mail]>  wrote:
>>> Hello Kukjin,
>>>
>>> On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas
>>> <[e-mail]>  wrote:
>>>> The regulator framework has a set of helpers functions to be used when
>>>> the system is entering and leaving from suspend but these are not called
>>>> on Exynos platforms. This means that the .set_suspend_* function handlers
>>>> defined by regulator drivers are not called when the system is suspended.
>>>>
>>>> Suggested-by: Doug Anderson<[e-mail]>
>>>> Signed-off-by: Javier Martinez Canillas<[e-mail]>
>>>> Reviewed-by: Doug Anderson<[e-mail]>
>>>> ---
>>>>   arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++
>>>>   1 file changed, 23 insertions(+)
>>>>
>>>
>>> Any comments on this patch?
>>>
>>
>> Just a gentle reminder about this patch.
>
> Kukjin, should I just apply this directly since you seem to be busy?
>
Oh, sorry. I'm here and applying this patch into samsung tree.

Thanks,
Kukjin
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Javier Martinez Canillas Nov. 13, 2014, 8:36 a.m. UTC | #7

Hello Kukjin,

On 11/13/2014 03:12 AM, Kukjin Kim wrote:
>>
> Oh, sorry. I'm here and applying this patch into samsung tree.
> 

Great, thanks a lot!

Best regards,
Javier

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index cc8d237..f8e7dcd 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -20,6 +20,7 @@ 
 #include <linux/io.h>
 #include <linux/irqchip/arm-gic.h>
 #include <linux/err.h>
+#include <linux/regulator/machine.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
@@ -443,6 +444,22 @@  static int exynos_suspend_enter(suspend_state_t state)
 
 static int exynos_suspend_prepare(void)
 {
+	int ret;
+
+	/*
+	 * REVISIT: It would be better if struct platform_suspend_ops
+	 * .prepare handler get the suspend_state_t as a parameter to
+	 * avoid hard-coding the suspend to mem state. It's safe to do
+	 * it now only because the suspend_valid_only_mem function is
+	 * used as the .valid callback used to check if a given state
+	 * is supported by the platform anyways.
+	 */
+	ret = regulator_suspend_prepare(PM_SUSPEND_MEM);
+	if (ret) {
+		pr_err("Failed to prepare regulators for suspend (%d)\n", ret);
+		return ret;
+	}
+
 	s3c_pm_check_prepare();
 
 	return 0;
@@ -450,7 +467,13 @@  static int exynos_suspend_prepare(void)
 
 static void exynos_suspend_finish(void)
 {
+	int ret;
+
 	s3c_pm_check_cleanup();
+
+	ret = regulator_suspend_finish();
+	if (ret)
+		pr_warn("Failed to resume regulators from suspend (%d)\n", ret);
 }
 
 static const struct platform_suspend_ops exynos_suspend_ops = {