[PATCH V2 0/5] rtc: tegra: cleanups to remove warning/code lines

[PATCH V2 0/5] rtc: tegra: cleanups to remove warning/code lines

[Laxman Dewangan]

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(-)

[PATCH V2 1/5] rtc: tegra: protect suspend/resume callbacks with CONFIG_PM_SLEEP

[Laxman Dewangan]

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 <ldewangan@nvidia.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
---
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

[PATCH V2 2/5] rtc: tegra: Use struct dev_pm_ops for power management

[Laxman Dewangan]

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 <ldewangan@nvidia.com>
---
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

Re: [PATCH V2 2/5] rtc: tegra: Use struct dev_pm_ops for power management

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

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 <thierry.reding@avionic-design.de>

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH V2 2/5] rtc: tegra: Use struct dev_pm_ops for power management

[Laxman Dewangan]

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

Re: [PATCH V2 2/5] rtc: tegra: Use struct dev_pm_ops for power management

[Stephen Warren]

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.

Re: [PATCH V2 2/5] rtc: tegra: Use struct dev_pm_ops for power management

[Laxman Dewangan]

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

[PATCH V2 3/5] rtc: tegra: use module_platform_driver_probe for module init/exit

[Laxman Dewangan]

Use macro module_platform_driver_probe() to reduce some of the
boilerplate code in the driver.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
---
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 <jmayo@nvidia.com>");
 MODULE_DESCRIPTION("driver for Tegra internal RTC");
-- 
1.7.1.1

[PATCH V2 4/5] rtc: tegra: set irq name as device name

[Laxman Dewangan]

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 <ldewangan@nvidia.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
---
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

[PATCH V2 5/5] rtc: tegra: use managed rtc_device_register

[Laxman Dewangan]

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 <ldewangan@nvidia.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
---
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

Re: [PATCH V2 0/5] rtc: tegra: cleanups to remove warning/code lines

[Stephen Warren]

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 <swarren@nvidia.com>