This patch series does the cleanups in the rtc driver as follows:
- Protect suspend/resume with CONFIG_PM_SLEEP.
- Use dev_pm_ops in place of legacy way for suspend/resume.
- Properly reflect teh irq name when doing cat /proc/interrupts.
- Use module_platform_probe() to remove boilerplate code.
- use devm_rtc_device_register.
Changes from V1:
- Use SIMPLE_DEV_PM_OPS
- Correctd the acronym case rtc to RTC in description.
- Add Thierry reviewed by on some of patches.
Laxman Dewangan (5):
rtc: tegra: protect suspend/resume callbacks with CONFIG_PM_SLEEP
rtc: tegra: Use struct dev_pm_ops for power management
rtc: tegra: use module_platform_driver_probe for module init/exit
rtc: tegra: set irq name as device name
rtc: tegra: use managed rtc_device_register
drivers/rtc/rtc-tegra.c | 61 +++++++++++-----------------------------------
1 files changed, 15 insertions(+), 46 deletions(-)
Use macro module_platform_driver_probe() to reduce some of the
boilerplate code in the driver.
Signed-off-by: Laxman Dewangan <[email protected]>
Reviewed-by: Thierry Reding <[email protected]>
---
Changes from V1:
- Add Thierry's reviewed by.
- No change in code.
drivers/rtc/rtc-tegra.c | 12 +-----------
1 files changed, 1 insertions(+), 11 deletions(-)
diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
index 6373eb7..c382542 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -450,17 +450,7 @@ static struct platform_driver tegra_rtc_driver = {
},
};
-static int __init tegra_rtc_init(void)
-{
- return platform_driver_probe(&tegra_rtc_driver, tegra_rtc_probe);
-}
-module_init(tegra_rtc_init);
-
-static void __exit tegra_rtc_exit(void)
-{
- platform_driver_unregister(&tegra_rtc_driver);
-}
-module_exit(tegra_rtc_exit);
+module_platform_driver_probe(tegra_rtc_driver, tegra_rtc_probe);
MODULE_AUTHOR("Jon Mayo <[email protected]>");
MODULE_DESCRIPTION("driver for Tegra internal RTC");
--
1.7.1.1
When watching the irq name of tegra rtc, it shows as "rtc alarm"
which actually does not reflect the name related to driver.
Passing the device name to have the irq names with driver name.
Signed-off-by: Laxman Dewangan <[email protected]>
Reviewed-by: Thierry Reding <[email protected]>
---
Changes from V1:
- Add Thierry's reviewed by.
- No change in code.
drivers/rtc/rtc-tegra.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
index c382542..fe16388 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -362,7 +362,7 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
ret = devm_request_irq(&pdev->dev, info->tegra_rtc_irq,
tegra_rtc_irq_handler, IRQF_TRIGGER_HIGH,
- "rtc alarm", &pdev->dev);
+ dev_name(&pdev->dev), &pdev->dev);
if (ret) {
dev_err(&pdev->dev,
"Unable to request interrupt for device (err=%d).\n",
--
1.7.1.1
Use devm_rtc_device_register for registering RTC device.
This will reduce the code for unregistering RTC device in
cleanup path and remove the implementation of remove
callback of platform driver.
Signed-off-by: Laxman Dewangan <[email protected]>
Reviewed-by: Thierry Reding <[email protected]>
---
Changes from V1:
- Formatting the code.
- Add Thierry's reviewed by.
drivers/rtc/rtc-tegra.c | 27 ++++-----------------------
1 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
index fe16388..ca01be7 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -349,13 +349,11 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
device_init_wakeup(&pdev->dev, 1);
- info->rtc_dev = rtc_device_register(
- pdev->name, &pdev->dev, &tegra_rtc_ops, THIS_MODULE);
+ info->rtc_dev = devm_rtc_device_register(dev_name(&pdev->dev),
+ &pdev->dev, &tegra_rtc_ops, THIS_MODULE);
if (IS_ERR(info->rtc_dev)) {
ret = PTR_ERR(info->rtc_dev);
- info->rtc_dev = NULL;
- dev_err(&pdev->dev,
- "Unable to register device (err=%d).\n",
+ dev_err(&pdev->dev, "Unable to register device (err=%d).\n",
ret);
return ret;
}
@@ -367,28 +365,12 @@ static int __init tegra_rtc_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"Unable to request interrupt for device (err=%d).\n",
ret);
- goto err_dev_unreg;
+ return ret;
}
dev_notice(&pdev->dev, "Tegra internal Real Time Clock\n");
return 0;
-
-err_dev_unreg:
- rtc_device_unregister(info->rtc_dev);
-
- return ret;
-}
-
-static int __exit tegra_rtc_remove(struct platform_device *pdev)
-{
- struct tegra_rtc_info *info = platform_get_drvdata(pdev);
-
- rtc_device_unregister(info->rtc_dev);
-
- platform_set_drvdata(pdev, NULL);
-
- return 0;
}
#ifdef CONFIG_PM_SLEEP
@@ -440,7 +422,6 @@ static void tegra_rtc_shutdown(struct platform_device *pdev)
MODULE_ALIAS("platform:tegra_rtc");
static struct platform_driver tegra_rtc_driver = {
- .remove = __exit_p(tegra_rtc_remove),
.shutdown = tegra_rtc_shutdown,
.driver = {
.name = "tegra_rtc",
--
1.7.1.1
Make the Tegra RTC controller driver define its PM callbacks through
a struct dev_pm_ops object rather than by using legacy PM hooks
in struct platform_driver.
Signed-off-by: Laxman Dewangan <[email protected]>
---
Changes from V1:
- Use SIMPLE_DEV_PM_OPS
drivers/rtc/rtc-tegra.c | 18 ++++++++----------
1 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
index d64cde6..6373eb7 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -26,6 +26,7 @@
#include <linux/delay.h>
#include <linux/rtc.h>
#include <linux/platform_device.h>
+#include <linux/pm.h>
/* set to 1 = busy every eight 32kHz clocks during copy of sec+msec to AHB */
#define TEGRA_RTC_REG_BUSY 0x004
@@ -391,10 +392,9 @@ static int __exit tegra_rtc_remove(struct platform_device *pdev)
}
#ifdef CONFIG_PM_SLEEP
-static int tegra_rtc_suspend(struct platform_device *pdev, pm_message_t state)
+static int tegra_rtc_suspend(struct device *dev)
{
- struct device *dev = &pdev->dev;
- struct tegra_rtc_info *info = platform_get_drvdata(pdev);
+ struct tegra_rtc_info *info = dev_get_drvdata(dev);
tegra_rtc_wait_while_busy(dev);
@@ -416,10 +416,9 @@ static int tegra_rtc_suspend(struct platform_device *pdev, pm_message_t state)
return 0;
}
-static int tegra_rtc_resume(struct platform_device *pdev)
+static int tegra_rtc_resume(struct device *dev)
{
- struct device *dev = &pdev->dev;
- struct tegra_rtc_info *info = platform_get_drvdata(pdev);
+ struct tegra_rtc_info *info = dev_get_drvdata(dev);
dev_vdbg(dev, "Resume (device_may_wakeup=%d)\n",
device_may_wakeup(dev));
@@ -431,6 +430,8 @@ static int tegra_rtc_resume(struct platform_device *pdev)
}
#endif
+SIMPLE_DEV_PM_OPS(tegra_rtc_pm_ops, tegra_rtc_suspend, tegra_rtc_resume);
+
static void tegra_rtc_shutdown(struct platform_device *pdev)
{
dev_vdbg(&pdev->dev, "disabling interrupts.\n");
@@ -445,11 +446,8 @@ static struct platform_driver tegra_rtc_driver = {
.name = "tegra_rtc",
.owner = THIS_MODULE,
.of_match_table = tegra_rtc_dt_match,
+ .pm = &tegra_rtc_pm_ops,
},
-#ifdef CONFIG_PM_SLEEP
- .suspend = tegra_rtc_suspend,
- .resume = tegra_rtc_resume,
-#endif
};
static int __init tegra_rtc_init(void)
--
1.7.1.1
The CONFIG_PM doesn't actually enable any of the PM callbacks, it
only allows to enable CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME.
This means if CONFIG_PM is used to protect system sleep callbacks
then it may end up unreferenced if only runtime PM is enabled.
Hence protecting sleep callbacks with CONFIG_PM_SLEEP.
Signed-off-by: Laxman Dewangan <[email protected]>
Reviewed-by: Thierry Reding <[email protected]>
---
Changes from V1:
- Add Thierry's reviewed by.
- No Change in code.
drivers/rtc/rtc-tegra.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
index 0020bab..d64cde6 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -390,7 +390,7 @@ static int __exit tegra_rtc_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
static int tegra_rtc_suspend(struct platform_device *pdev, pm_message_t state)
{
struct device *dev = &pdev->dev;
@@ -446,7 +446,7 @@ static struct platform_driver tegra_rtc_driver = {
.owner = THIS_MODULE,
.of_match_table = tegra_rtc_dt_match,
},
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
.suspend = tegra_rtc_suspend,
.resume = tegra_rtc_resume,
#endif
--
1.7.1.1
On Mon, Mar 11, 2013 at 02:14:25PM +0530, Laxman Dewangan wrote:
[...]
> diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
[...]
> @@ -431,6 +430,8 @@ static int tegra_rtc_resume(struct platform_device *pdev)
> }
> #endif
>
> +SIMPLE_DEV_PM_OPS(tegra_rtc_pm_ops, tegra_rtc_suspend, tegra_rtc_resume);
I should've been more explicit. Since you had this marked as static in
the previous patch, this needs to be static SIMPLE_DEV_PM_OPS.
With that change,
Reviewed-by: Thierry Reding <[email protected]>
On Monday 11 March 2013 02:21 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Mon, Mar 11, 2013 at 02:14:25PM +0530, Laxman Dewangan wrote:
> [...]
>> diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
> [...]
>> @@ -431,6 +430,8 @@ static int tegra_rtc_resume(struct platform_device *pdev)
>> }
>> #endif
>>
>> +SIMPLE_DEV_PM_OPS(tegra_rtc_pm_ops, tegra_rtc_suspend, tegra_rtc_resume);
> I should've been more explicit. Since you had this marked as static in
> the previous patch, this needs to be static SIMPLE_DEV_PM_OPS.
Yaah, I should keep the static.
I think I will need to respin this patch only in place of whole series.
Andrew,
Let me know if this is fine.
Thanks,
Laxman
On 03/11/2013 02:56 AM, Laxman Dewangan wrote:
> On Monday 11 March 2013 02:21 PM, Thierry Reding wrote:
>> * PGP Signed by an unknown key
>>
>> On Mon, Mar 11, 2013 at 02:14:25PM +0530, Laxman Dewangan wrote:
>> [...]
>>> diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
>> [...]
>>> @@ -431,6 +430,8 @@ static int tegra_rtc_resume(struct
>>> platform_device *pdev)
>>> }
>>> #endif
>>> +SIMPLE_DEV_PM_OPS(tegra_rtc_pm_ops, tegra_rtc_suspend,
>>> tegra_rtc_resume);
>> I should've been more explicit. Since you had this marked as static in
>> the previous patch, this needs to be static SIMPLE_DEV_PM_OPS.
> Yaah, I should keep the static.
>
> I think I will need to respin this patch only in place of whole series.
>
> Andrew,
>
> Let me know if this is fine.
In general, it's much easier if you repost a whole series. Otherwise,
you make the maintainer manually piece together the series from bits of
V2 and bits of V3, which is harder than just grabbing all of V3.
On 03/11/2013 02:44 AM, Laxman Dewangan wrote:
> This patch series does the cleanups in the rtc driver as follows:
> - Protect suspend/resume with CONFIG_PM_SLEEP.
> - Use dev_pm_ops in place of legacy way for suspend/resume.
> - Properly reflect teh irq name when doing cat /proc/interrupts.
> - Use module_platform_probe() to remove boilerplate code.
> - use devm_rtc_device_register.
Assuming there's a good explanation for my question re: patch 3/5, then
the series,
Reviewed-by: Stephen Warren <[email protected]>
On Monday 11 March 2013 10:48 PM, Stephen Warren wrote:
> On 03/11/2013 02:56 AM, Laxman Dewangan wrote:
>> On Monday 11 March 2013 02:21 PM, Thierry Reding wrote:
>>> * PGP Signed by an unknown key
>>>
>>> On Mon, Mar 11, 2013 at 02:14:25PM +0530, Laxman Dewangan wrote:
>>> [...]
>>>> diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
>>> [...]
>>>> @@ -431,6 +430,8 @@ static int tegra_rtc_resume(struct
>>>> platform_device *pdev)
>>>> }
>>>> #endif
>>>> +SIMPLE_DEV_PM_OPS(tegra_rtc_pm_ops, tegra_rtc_suspend,
>>>> tegra_rtc_resume);
>>> I should've been more explicit. Since you had this marked as static in
>>> the previous patch, this needs to be static SIMPLE_DEV_PM_OPS.
>> Yaah, I should keep the static.
>>
>> I think I will need to respin this patch only in place of whole series.
>>
>> Andrew,
>>
>> Let me know if this is fine.
> In general, it's much easier if you repost a whole series. Otherwise,
> you make the maintainer manually piece together the series from bits of
> V2 and bits of V3, which is harder than just grabbing all of V3.
yaah, this will be easy. I will respin 3rd version.
Thanks,
Laxman