diff mbox

[RESEND,v9,1/5] rtc: max77686: Allow the max77686 rtc to wakeup the system

Message ID 1410509864-10019-2-git-send-email-javier.martinez@collabora.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas Sept. 12, 2014, 8:17 a.m. UTC 
From: Doug Anderson <dianders@chromium.org>

The max77686 includes an RTC that keeps power during suspend.  It's
convenient to be able to use it as a wakeup source.

NOTE: due to wakeup ordering problems this patch alone doesn't work so
well on exynos5250-snow.  You also need something that brings the i2c
bus up before the max77686 wakeup runs.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---

Changes since v6: None

Changes since v5:
 - Fix $SUBJECT since the patch does not actually touch the mfd subsys.
   Suggested by Lee Jones.

Changes since v4: None

Changes since v3:
 - Keep the note that this patch needs another change due wakeup
   ordering problems.
---
 drivers/rtc/rtc-max77686.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Doug Anderson Sept. 12, 2014, 3:12 p.m. UTC | #1 
Hi,

On Fri, Sep 12, 2014 at 1:17 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> From: Doug Anderson <dianders@chromium.org>
>
> The max77686 includes an RTC that keeps power during suspend.  It's
> convenient to be able to use it as a wakeup source.
>
> NOTE: due to wakeup ordering problems this patch alone doesn't work so
> well on exynos5250-snow.  You also need something that brings the i2c
> bus up before the max77686 wakeup runs.

This note made sense when the patch was first submitted, but no longer
does since the i2c change landed.  See:

b19c195 i2c: s3c2410: resume the I2C controller earlier
Javier Martinez Canillas Sept. 12, 2014, 3:54 p.m. UTC | #2 
Hello Doug,

On 09/12/2014 05:12 PM, Doug Anderson wrote:
> Hi,
> 
> On Fri, Sep 12, 2014 at 1:17 AM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
>> From: Doug Anderson <dianders@chromium.org>
>>
>> The max77686 includes an RTC that keeps power during suspend.  It's
>> convenient to be able to use it as a wakeup source.
>>
>> NOTE: due to wakeup ordering problems this patch alone doesn't work so
>> well on exynos5250-snow.  You also need something that brings the i2c
>> bus up before the max77686 wakeup runs.
> 
> This note made sense when the patch was first submitted, but no longer
> does since the i2c change landed.  See:
> 
> b19c195 i2c: s3c2410: resume the I2C controller earlier
> 

You are right, I completely forgot to check if that actually landed and to
remove the note in that case...

Maybe when the set is applied the note can be removed from this patch or do
you think that I should re-spin the series for that?

Best regards,
Javier
Doug Anderson Sept. 12, 2014, 4:03 p.m. UTC | #3 
Hi,

On Fri, Sep 12, 2014 at 8:54 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> You are right, I completely forgot to check if that actually landed and to
> remove the note in that case...
>
> Maybe when the set is applied the note can be removed from this patch or do
> you think that I should re-spin the series for that?

This seems like something a maintainer could handle removing, but I
guess it's totally up to the maintainer.  If I were the maintainer I'd
rather remove the note myself then see a whole spin of a 5-part series
just to fix this.  ;)


-Doug
Andrew Morton Sept. 12, 2014, 10:05 p.m. UTC | #4 
On Fri, 12 Sep 2014 10:17:39 +0200 Javier Martinez Canillas <javier.martinez@collabora.co.uk> wrote:

> From: Doug Anderson <dianders@chromium.org>
> 
> The max77686 includes an RTC that keeps power during suspend.  It's
> convenient to be able to use it as a wakeup source.
> 
> NOTE: due to wakeup ordering problems this patch alone doesn't work so
> well on exynos5250-snow.  You also need something that brings the i2c
> bus up before the max77686 wakeup runs.

I removed this paragraph.

> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

And I rewrote this to Signed-off-by:, as you were on the patch delivery
path. Documentation/SubmittingPatches section 12 has the gory details.
Javier Martinez Canillas Sept. 15, 2014, 2:03 p.m. UTC | #5 
Hello Andrew,

Thanks a lot for taking a look to these patches!

On 09/13/2014 12:05 AM, Andrew Morton wrote:
> On Fri, 12 Sep 2014 10:17:39 +0200 Javier Martinez Canillas <javier.martinez@collabora.co.uk> wrote:
> 
>> From: Doug Anderson <dianders@chromium.org>
>> 
>> The max77686 includes an RTC that keeps power during suspend.  It's
>> convenient to be able to use it as a wakeup source.
>> 
>> NOTE: due to wakeup ordering problems this patch alone doesn't work so
>> well on exynos5250-snow.  You also need something that brings the i2c
>> bus up before the max77686 wakeup runs.
> 
> I removed this paragraph.
> 

Great.

>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> 
> And I rewrote this to Signed-off-by:, as you were on the patch delivery
> path. Documentation/SubmittingPatches section 12 has the gory details.
> 

Yes, originally Doug posted this patch separately but was never picked so I
included on my series but forgot to add my s-o-b and also to remove the note
paragraph.

Best regards,
Javier
diff mbox

Patch

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index d20a7f0..c1c6055 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -583,6 +583,33 @@  static void max77686_rtc_shutdown(struct platform_device *pdev)
 #endif /* MAX77686_RTC_WTSR_SMPL */
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int max77686_rtc_suspend(struct device *dev)
+{
+	if (device_may_wakeup(dev)) {
+		struct max77686_rtc_info *info = dev_get_drvdata(dev);
+
+		return enable_irq_wake(info->virq);
+	}
+
+	return 0;
+}
+
+static int max77686_rtc_resume(struct device *dev)
+{
+	if (device_may_wakeup(dev)) {
+		struct max77686_rtc_info *info = dev_get_drvdata(dev);
+
+		return disable_irq_wake(info->virq);
+	}
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(max77686_rtc_pm_ops,
+			 max77686_rtc_suspend, max77686_rtc_resume);
+
 static const struct platform_device_id rtc_id[] = {
 	{ "max77686-rtc", 0 },
 	{},
@@ -592,6 +619,7 @@  static struct platform_driver max77686_rtc_driver = {
 	.driver		= {
 		.name	= "max77686-rtc",
 		.owner	= THIS_MODULE,
+		.pm	= &max77686_rtc_pm_ops,
 	},
 	.probe		= max77686_rtc_probe,
 	.shutdown	= max77686_rtc_shutdown,