| Message ID | 1413839151-10875-3-git-send-email-javier.martinez@collabora.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Javier, On Mon, Oct 20, 2014 at 2:05 PM, Javier Martinez Canillas <javier.martinez@collabora.co.uk> 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 <dianders@chromium.org> > Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > Reviewed-by: Doug Anderson <dianders@chromium.org> I think you forgot to carry Chanwoo's review: Reviewed-by: Chanwoo Choi<cw00.choi@samsung.com> I don't think you've made any changes that would need to invalidate his review.
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 > <javier.martinez@collabora.co.uk> 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 <dianders@chromium.org> >> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> >> Reviewed-by: Doug Anderson <dianders@chromium.org> > > I think you forgot to carry Chanwoo's review: > > Reviewed-by: Chanwoo Choi<cw00.choi@samsung.com> > > 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
Hello Kukjin, On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas <javier.martinez@collabora.co.uk> 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 <dianders@chromium.org> > Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > Reviewed-by: Doug Anderson <dianders@chromium.org> > --- > arch/arm/mach-exynos/suspend.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > Any comments on this patch? Best regards, Javier
Hello Kukjin, On Thu, Oct 30, 2014 at 11:06 AM, Javier Martinez Canillas <javier@dowhile0.org> wrote: > Hello Kukjin, > > On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas > <javier.martinez@collabora.co.uk> 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 <dianders@chromium.org> >> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> >> Reviewed-by: Doug Anderson <dianders@chromium.org> >> --- >> 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
On Tue, Nov 11, 2014 at 2:23 AM, Javier Martinez Canillas <javier@dowhile0.org> wrote: > Hello Kukjin, > > On Thu, Oct 30, 2014 at 11:06 AM, Javier Martinez Canillas > <javier@dowhile0.org> wrote: >> Hello Kukjin, >> >> On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas >> <javier.martinez@collabora.co.uk> 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 <dianders@chromium.org> >>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> >>> Reviewed-by: Doug Anderson <dianders@chromium.org> >>> --- >>> 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
On 11/13/14 02:02, Olof Johansson wrote: > On Tue, Nov 11, 2014 at 2:23 AM, Javier Martinez Canillas > <javier@dowhile0.org> wrote: >> Hello Kukjin, >> >> On Thu, Oct 30, 2014 at 11:06 AM, Javier Martinez Canillas >> <javier@dowhile0.org> wrote: >>> Hello Kukjin, >>> >>> On Mon, Oct 20, 2014 at 11:05 PM, Javier Martinez Canillas >>> <javier.martinez@collabora.co.uk> 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<dianders@chromium.org> >>>> Signed-off-by: Javier Martinez Canillas<javier.martinez@collabora.co.uk> >>>> Reviewed-by: Doug Anderson<dianders@chromium.org> >>>> --- >>>> 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
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
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 = {