diff mbox

[2/3] RTC: omap-rtc: enable pm_runtime

Message ID 1350575631-15523-2-git-send-email-zonque@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Mack Oct. 18, 2012, 3:53 p.m. UTC
This is needed as preparation for platforms where using pm runtime usage
is mandatory.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/rtc/rtc-omap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Vaibhav Hiremath Oct. 18, 2012, 4:12 p.m. UTC | #1
On 10/18/2012 9:23 PM, Daniel Mack wrote:
> This is needed as preparation for platforms where using pm runtime usage
> is mandatory.
> 
> Signed-off-by: Daniel Mack <zonque@gmail.com>

It looks like, you just duplicated effort here.
RTC patches have been already submitted quite some time back for AM33xx,
probably you missed to do google before spending time on this.

Patch Series -
http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg23253.html


Denial,
It would be really helpful if you could test these patches and ack them.

Just FYI, we have bunch of parallel activities going inside team to
upstream all the patches/module/drivers/features, so to avoid such
duplication of effort, I suggest you to google for the patches or ping
on mailing list.

Thanks,
Vaibhav

> ---
>  drivers/rtc/rtc-omap.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index 0b614e3..baa876e 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/ioport.h>
>  #include <linux/delay.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/rtc.h>
>  #include <linux/bcd.h>
>  #include <linux/platform_device.h>
> @@ -322,6 +323,9 @@ static int __init omap_rtc_probe(struct platform_device *pdev)
>  		goto fail;
>  	}
>  
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_get_sync(&pdev->dev);
> +
>  	rtc = rtc_device_register(pdev->name, &pdev->dev,
>  			&omap_rtc_ops, THIS_MODULE);
>  	if (IS_ERR(rtc)) {
> @@ -420,6 +424,8 @@ static int __exit omap_rtc_remove(struct platform_device *pdev)
>  		free_irq(omap_rtc_alarm, rtc);
>  
>  	rtc_device_unregister(rtc);
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
>  	iounmap(rtc_base);
>  	release_mem_region(mem->start, resource_size(mem));
>  	return 0;
> @@ -442,11 +448,15 @@ static int omap_rtc_suspend(struct platform_device *pdev, pm_message_t state)
>  	else
>  		rtc_write(0, OMAP_RTC_INTERRUPTS_REG);
>  
> +	pm_runtime_put_sync(&pdev->dev);
> +
>  	return 0;
>  }
>  
>  static int omap_rtc_resume(struct platform_device *pdev)
>  {
> +	pm_runtime_get_sync(&pdev->dev);
> +
>  	if (device_may_wakeup(&pdev->dev))
>  		disable_irq_wake(omap_rtc_alarm);
>  	else
>
Daniel Mack Oct. 18, 2012, 4:19 p.m. UTC | #2
On 18.10.2012 18:12, Vaibhav Hiremath wrote:
> 
> 
> On 10/18/2012 9:23 PM, Daniel Mack wrote:
>> This is needed as preparation for platforms where using pm runtime usage
>> is mandatory.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
> 
> It looks like, you just duplicated effort here.
> RTC patches have been already submitted quite some time back for AM33xx,
> probably you missed to do google before spending time on this.
> 
> Patch Series -
> http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg23253.html

Heh, damn :) I would have thought that so shortly after the merge
window, that code either went in already or is yet missing. Oh well.

> Denial,
> It would be really helpful if you could test these patches and ack them.

Ok, will do tomorrow. What's missing there is support for writing to the
OMAP_RTC_OSC_REG (path 3/3 in my series). Would like me to rebase the
functionality? I need thatkind of setup for my board ...


> Just FYI, we have bunch of parallel activities going inside team to
> upstream all the patches/module/drivers/features, so to avoid such
> duplication of effort, I suggest you to google for the patches or ping
> on mailing list.

Yes, it's more difficult to participate in the AM33xx code than in other
platforms. I'm not complaining about other doing all the hard work
though, it's just a little different.

Anyway - thanks for the heads-up!


Daniel
Vaibhav Hiremath Oct. 18, 2012, 4:33 p.m. UTC | #3
On Thu, Oct 18, 2012 at 21:49:44, Daniel Mack wrote:
> On 18.10.2012 18:12, Vaibhav Hiremath wrote:
> > 
> > 
> > On 10/18/2012 9:23 PM, Daniel Mack wrote:
> >> This is needed as preparation for platforms where using pm runtime usage
> >> is mandatory.
> >>
> >> Signed-off-by: Daniel Mack <zonque@gmail.com>
> > 
> > It looks like, you just duplicated effort here.
> > RTC patches have been already submitted quite some time back for AM33xx,
> > probably you missed to do google before spending time on this.
> > 
> > Patch Series -
> > http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg23253.html
> 
> Heh, damn :) I would have thought that so shortly after the merge
> window, that code either went in already or is yet missing. Oh well.
> 
> > Denial,
> > It would be really helpful if you could test these patches and ack them.
> 
> Ok, will do tomorrow. What's missing there is support for writing to the
> OMAP_RTC_OSC_REG (path 3/3 in my series). Would like me to rebase the
> functionality? I need thatkind of setup for my board ...
> 
> 

This is bit tricky and required more discussion before merging it, 
let me reopen the old discussion which I had started some time back -


RTC OSC clock is required to configure clocksource systemtimer for the 
kernel, and it is important when you get into PM related features.

Please refer the below commit for more reference,

http://marc.info/?l=u-boot&m=135057734713796&w=2


> > Just FYI, we have bunch of parallel activities going inside team to
> > upstream all the patches/module/drivers/features, so to avoid such
> > duplication of effort, I suggest you to google for the patches or ping
> > on mailing list.
> 
> Yes, it's more difficult to participate in the AM33xx code than in other
> platforms. I'm not complaining about other doing all the hard work
> though, it's just a little different.
> 

No its not. Its good that we have so many developers contributing, so now 
its just working together. Recommend you to ping once before spending time, 
so that we can collaboratively work together, even I wouldn't mind you to 
pick/own on-going development and submit it.


Thanks,
Vaibhav
> Anyway - thanks for the heads-up!
> 
> 
> Daniel
> 
>
Afzal Mohammed Oct. 19, 2012, 10:58 a.m. UTC | #4
Hi Daniel,

On Thu, Oct 18, 2012 at 22:03:13, Hiremath, Vaibhav wrote:
> On Thu, Oct 18, 2012 at 21:49:44, Daniel Mack wrote:
> > On 18.10.2012 18:12, Vaibhav Hiremath wrote:

> > > It would be really helpful if you could test these patches and ack them.

> > Ok, will do tomorrow. What's missing there is support for writing to the
> > OMAP_RTC_OSC_REG (path 3/3 in my series). Would like me to rebase the
> > functionality? I need thatkind of setup for my board ...

> This is bit tricky and required more discussion before merging it, 
> let me reopen the old discussion which I had started some time back -
> 
> 
> RTC OSC clock is required to configure clocksource systemtimer for the 
> kernel, and it is important when you get into PM related features.
> 
> Please refer the below commit for more reference,
> 
> http://marc.info/?l=u-boot&m=135057734713796&w=2

Newer version (v4) of omap rtc patches for am335x has been
posted, "rtc: omap dt support (for am33xx)".
In case you can test, please do it over the new series.

You would need also need DT patch that adds DT node to
test (it has been separated from the series as it is
felt that it should not be part of same series),
"arm/dts: am33xx rtc node"

If you are using mainline uboot, you would need the fix as
mentioned by Vaibhav H (as in the link he provided above),
expecting it to reach uboot mainline soon.

If the above is being attempted to achieve in Kernel it
would cause 2-3 seconds delay in boot time (else it had
to be made a module), please refer link provided by
Vaibhav H for more details.

else you can do in current mainline uboot,

mw.l 0x44e3e06c 0x83e70b13
mw.l 0x44e3e070 0x95a4f1e0
mw.b 0x44e3e054 0x48

and then boot kernel.

Regards
Afzal
Daniel Mack Oct. 21, 2012, 7:51 p.m. UTC | #5
On 19.10.2012 12:58, Mohammed, Afzal wrote:
> Hi Daniel,
> 
> On Thu, Oct 18, 2012 at 22:03:13, Hiremath, Vaibhav wrote:
>> On Thu, Oct 18, 2012 at 21:49:44, Daniel Mack wrote:
>>> On 18.10.2012 18:12, Vaibhav Hiremath wrote:
> 
>>>> It would be really helpful if you could test these patches and ack them.
> 
>>> Ok, will do tomorrow. What's missing there is support for writing to the
>>> OMAP_RTC_OSC_REG (path 3/3 in my series). Would like me to rebase the
>>> functionality? I need thatkind of setup for my board ...
> 
>> This is bit tricky and required more discussion before merging it, 
>> let me reopen the old discussion which I had started some time back -
>>
>>
>> RTC OSC clock is required to configure clocksource systemtimer for the 
>> kernel, and it is important when you get into PM related features.
>>
>> Please refer the below commit for more reference,
>>
>> http://marc.info/?l=u-boot&m=135057734713796&w=2
> 
> Newer version (v4) of omap rtc patches for am335x has been
> posted, "rtc: omap dt support (for am33xx)".
> In case you can test, please do it over the new series.
> 
> You would need also need DT patch that adds DT node to
> test (it has been separated from the series as it is
> felt that it should not be part of same series),
> "arm/dts: am33xx rtc node"

Agreed. I tested your patches and they do basically the same thing than
mine. You can take my

  Tested-by: Daniel Mack <zonque@gmail.com>

on all patches in this series except for 2/5 which I didn't test as I
don't have any Davinci platform.

> If you are using mainline uboot, you would need the fix as
> mentioned by Vaibhav H (as in the link he provided above),
> expecting it to reach uboot mainline soon.
> 
> If the above is being attempted to achieve in Kernel it
> would cause 2-3 seconds delay in boot time (else it had
> to be made a module), please refer link provided by
> Vaibhav H for more details.
> 
> else you can do in current mainline uboot,
> 
> mw.l 0x44e3e06c 0x83e70b13
> mw.l 0x44e3e070 0x95a4f1e0
> mw.b 0x44e3e054 0x48
>
> and then boot kernel.

I'm surprised that survives the kernel boot, but I'll give it a try. In
case you want to add that logic to the kernel as well, please copy me in
the discussion :)


Thanks,
Daniel
Vaibhav Hiremath Oct. 22, 2012, 5:59 a.m. UTC | #6
On Mon, Oct 22, 2012 at 01:21:37, Daniel Mack wrote:
> On 19.10.2012 12:58, Mohammed, Afzal wrote:
> > Hi Daniel,
> > 
> > On Thu, Oct 18, 2012 at 22:03:13, Hiremath, Vaibhav wrote:
> >> On Thu, Oct 18, 2012 at 21:49:44, Daniel Mack wrote:
> >>> On 18.10.2012 18:12, Vaibhav Hiremath wrote:
> > 
> >>>> It would be really helpful if you could test these patches and ack them.
> > 
> >>> Ok, will do tomorrow. What's missing there is support for writing to the
> >>> OMAP_RTC_OSC_REG (path 3/3 in my series). Would like me to rebase the
> >>> functionality? I need thatkind of setup for my board ...
> > 
> >> This is bit tricky and required more discussion before merging it, 
> >> let me reopen the old discussion which I had started some time back -
> >>
> >>
> >> RTC OSC clock is required to configure clocksource systemtimer for the 
> >> kernel, and it is important when you get into PM related features.
> >>
> >> Please refer the below commit for more reference,
> >>
> >> http://marc.info/?l=u-boot&m=135057734713796&w=2
> > 
> > Newer version (v4) of omap rtc patches for am335x has been
> > posted, "rtc: omap dt support (for am33xx)".
> > In case you can test, please do it over the new series.
> > 
> > You would need also need DT patch that adds DT node to
> > test (it has been separated from the series as it is
> > felt that it should not be part of same series),
> > "arm/dts: am33xx rtc node"
> 
> Agreed. I tested your patches and they do basically the same thing than
> mine. You can take my
> 
>   Tested-by: Daniel Mack <zonque@gmail.com>
> 

Thanks Daniel for testing these patches.

> on all patches in this series except for 2/5 which I didn't test as I
> don't have any Davinci platform.
> 
> > If you are using mainline uboot, you would need the fix as
> > mentioned by Vaibhav H (as in the link he provided above),
> > expecting it to reach uboot mainline soon.
> > 
> > If the above is being attempted to achieve in Kernel it
> > would cause 2-3 seconds delay in boot time (else it had
> > to be made a module), please refer link provided by
> > Vaibhav H for more details.
> > 
> > else you can do in current mainline uboot,
> > 
> > mw.l 0x44e3e06c 0x83e70b13
> > mw.l 0x44e3e070 0x95a4f1e0
> > mw.b 0x44e3e054 0x48
> >
> > and then boot kernel.
> 
> I'm surprised that survives the kernel boot, but I'll give it a try. In
> case you want to add that logic to the kernel as well, please copy me in
> the discussion :)
> 
> 

Certainly...

Some background information for your reference,
We had some discussion in the past on this, and we decided to make this as a 
fallback mechanism considering following scenarios - 

 - System built with 32k crystal OSC, connected to RTC module

 - System without 32k crystal OSC, as RTC is not used.

 - Boot-Loader dependency for RTC clock enable (required for system-timer)

So the fallback mechanism would be something similar -

http://arago-project.org/git/projects/?p=linux-am33x.git;a=commit;h=58abec6ac010f4f8818caa4a52d16c4802e14acc


Thanks,
Vaibhav

> Thanks,
> Daniel
> 
>
diff mbox

Patch

diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index 0b614e3..baa876e 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -17,6 +17,7 @@ 
 #include <linux/module.h>
 #include <linux/ioport.h>
 #include <linux/delay.h>
+#include <linux/pm_runtime.h>
 #include <linux/rtc.h>
 #include <linux/bcd.h>
 #include <linux/platform_device.h>
@@ -322,6 +323,9 @@  static int __init omap_rtc_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_get_sync(&pdev->dev);
+
 	rtc = rtc_device_register(pdev->name, &pdev->dev,
 			&omap_rtc_ops, THIS_MODULE);
 	if (IS_ERR(rtc)) {
@@ -420,6 +424,8 @@  static int __exit omap_rtc_remove(struct platform_device *pdev)
 		free_irq(omap_rtc_alarm, rtc);
 
 	rtc_device_unregister(rtc);
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
 	iounmap(rtc_base);
 	release_mem_region(mem->start, resource_size(mem));
 	return 0;
@@ -442,11 +448,15 @@  static int omap_rtc_suspend(struct platform_device *pdev, pm_message_t state)
 	else
 		rtc_write(0, OMAP_RTC_INTERRUPTS_REG);
 
+	pm_runtime_put_sync(&pdev->dev);
+
 	return 0;
 }
 
 static int omap_rtc_resume(struct platform_device *pdev)
 {
+	pm_runtime_get_sync(&pdev->dev);
+
 	if (device_may_wakeup(&pdev->dev))
 		disable_irq_wake(omap_rtc_alarm);
 	else