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.
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 | 62 ++++++++++++----------------------------------
1 files changed, 16 insertions(+), 46 deletions(-)
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]>
---
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
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]>
---
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 4cba10d..8186405 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -361,7 +361,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
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]>
---
drivers/rtc/rtc-tegra.c | 19 +++++++++----------
1 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
index d64cde6..f7718de 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -391,10 +391,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 +415,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 +429,10 @@ static int tegra_rtc_resume(struct platform_device *pdev)
}
#endif
+static const struct dev_pm_ops tegra_rtc_pm_ops = {
+ SET_SYSTEM_SLEEP_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 +447,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
Use macro module_platform_driver_probe() to reduce some of the
boilerplate code in the driver.
Signed-off-by: Laxman Dewangan <[email protected]>
---
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 f7718de..4cba10d 100644
--- a/drivers/rtc/rtc-tegra.c
+++ b/drivers/rtc/rtc-tegra.c
@@ -451,17 +451,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
On Sat, Mar 09, 2013 at 11:39:45PM +0530, Laxman Dewangan wrote:
> 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]>
> ---
> drivers/rtc/rtc-tegra.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Thierry Reding <[email protected]>
On Sat, Mar 09, 2013 at 11:39:46PM +0530, Laxman Dewangan wrote:
[...]
> @@ -431,6 +429,10 @@ static int tegra_rtc_resume(struct platform_device *pdev)
> }
> #endif
>
> +static const struct dev_pm_ops tegra_rtc_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(tegra_rtc_suspend, tegra_rtc_resume)
> +};
SIMPLE_DEV_PM_OPS?
On Sat, Mar 09, 2013 at 11:39:47PM +0530, Laxman Dewangan wrote:
> Use macro module_platform_driver_probe() to reduce some of the
> boilerplate code in the driver.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> ---
> drivers/rtc/rtc-tegra.c | 12 +-----------
> 1 files changed, 1 insertions(+), 11 deletions(-)
Reviewed-by: Thierry Reding <[email protected]>
On Sat, Mar 09, 2013 at 11:39:48PM +0530, Laxman Dewangan wrote:
> 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]>
> ---
> drivers/rtc/rtc-tegra.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Reviewed-by: Thierry Reding <[email protected]>
On 03/09/2013 11:09 AM, Laxman Dewangan wrote:
> Use macro module_platform_driver_probe() to reduce some of the
> boilerplate code in the driver.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
> ---
> 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 f7718de..4cba10d 100644
> --- a/drivers/rtc/rtc-tegra.c
> +++ b/drivers/rtc/rtc-tegra.c
> @@ -451,17 +451,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);
Out of curiosity here, why not just module_platform_driver(); can't the
instantiation of the RTC device be driven by the presence of the DT
node, and hence the creation of the platform device object, just like
any other driver/device?
On Monday 11 March 2013 10:46 PM, Stephen Warren wrote:
> On 03/09/2013 11:09 AM, Laxman Dewangan wrote:
>> Use macro module_platform_driver_probe() to reduce some of the
>> boilerplate code in the driver.
>>
>> Signed-off-by: Laxman Dewangan <[email protected]>
>> ---
>> 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 f7718de..4cba10d 100644
>> --- a/drivers/rtc/rtc-tegra.c
>> +++ b/drivers/rtc/rtc-tegra.c
>> @@ -451,17 +451,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);
> Out of curiosity here, why not just module_platform_driver(); can't the
> instantiation of the RTC device be driven by the presence of the DT
> node, and hence the creation of the platform device object, just like
> any other driver/device?
Theoretically it should work but I will work with John (original author)
about using the platform_driver_probe().
If he agrees then I will send patch to convert it to
module_platform_driver().
On 03/11/2013 01:29 PM, Laxman Dewangan wrote:
> On Monday 11 March 2013 10:46 PM, Stephen Warren wrote:
>> On 03/09/2013 11:09 AM, Laxman Dewangan wrote:
>>> Use macro module_platform_driver_probe() to reduce some of the
>>> boilerplate code in the driver.
>>> diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c
>>> -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);
>>
>> Out of curiosity here, why not just module_platform_driver(); can't the
>> instantiation of the RTC device be driven by the presence of the DT
>> node, and hence the creation of the platform device object, just like
>> any other driver/device?
>
> Theoretically it should work but I will work with John (original author)
> about using the platform_driver_probe().
> If he agrees then I will send patch to convert it to
> module_platform_driver().
OK, sounds good. Thanks.